-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UARTs: select pins choosen by manufacturer #41
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "default" UART settings for UART0, UART1 and UART2 are correct according to my test build. Testing for a BigTreetech SKR 1.4 Turbo. That board needed mentioned define UART3_P4_28 set, to override the default for UART3. In this manner UART3 is also working. This is a new (optional) define, for which I added support in this pull-request.
This fix does not affect in any way boards that depend on the "choosen" default pins for UARTs, thus I feel it is save to merge. People that need different pins for their UART on their board can set the appropriate #define and do not need to "hack" HardwareSerial.h
Hey thanks for looking into this, the framework was written for a specific board layout so isn't very generalised, The updated definition naming is better and links better with the frameworks pin naming convention, the main thing is to make sure that it doesn't change the default behaviour. |
That was exactly what I had in mind. for the BTT SKR 1.4, without any defines UART3 is not working. The others are. With my proper define also UART3 is working. I checked carefully the default definitions for UART3. They are not touched, and the other default UARTs correctness is confirmed by my working board. |
With the update of the README,MD, I hope it can be merged soon, as without defines there is no effect. |
The LPC1769 allows functions to be multiplexed to various pins. Board manufacturers choose which pins are used for which function. This allows to choose any pin combination for UARTs to be defined by additional optional symbols. It can resolve the BigTreeTech used different pins for UART3, compared to other manufaturers. In this case defineing UART_PIN_82_85 enables UART3, which is available at the Wifi port. The symbols per UART are: For UART0: There is only one set of pins: pin 98 and 99, so no define needed For UART1: default is pin 62 and 63 define UART1_PIN_75_74 to use pin 75 and 74 For UART2: default is pin 48 and 49 define UART2_PIN_65_64 to use pin 65 and 44 For UART3: default is pin 46 and 47 define UART3_PIN_7_6 to use pin 7 and 6 define UART3_PIN_82_85 to use pin 82 and 85 DISCLASIMER Currently I have *only* tested correct functioning of this setting for UART3 with UART3_PIN_82_85 for my BTT SKR 1.4 Turbo. All settings were derived from the hardware documentation: https://github.com/bigtreetech/BIGTREETECH-SKR-V1.3/blob/master/BTT%20SKR%20V1.4/Datasheet/LPC17-32-IC.pdf
…efine's, I now use the port and the bit number of the TX UART pin for the name of the #define. The logic is not changed. In this manner the #define names do not reflect a specific packaging variant, which makes more sense.
4091be1
to
cc276da
Compare
Can you test with my cleanup changes, All I really did was add a namespace to the defines (and a little cleanup) as its going to be global, |
@p3p Thanks for looking into this. I cannot perform a testbuild until next month (am abroad, away from the hardware) but I inspected the code and I did not see any issues. |
Somehow this change has broken UART3 completely. While the HarwareSerial hack was indeed a solution for my Esp WiFi on SKR1.4 now it does not work. I have tried compiling it in it's intended way and event editing harwareserial and removing all other definitions - no luck. |
I cannot see the full environment to determine "how it broke", but you seem to raise the comment coming from Marlin. For me, updating the Marlin files was just one element of the solution (i.e. where you put the define). In order to have the define properly taken into account (and implemented, you need to update for platformio framework files (for compiling for LPC176x) accordingly. In "%userprofile%.platformio\packages\framework-arduino-lpc176x\cores\arduino\HardwareSerial.h" you can see the define that is needed for UART3 in the section after:
(line 140 in my case). The defines for UART0, UART1, UART2 are not needed, their default definition was and is OK. Please confirm that you platformio LPC176x framework indeed supports the "new" definitions that were merged. |
The definitions are there and seem to do the job. I can see with M43 command that both 4.28 and 4.29 are assigned to TX/RX 3. |
I see: my contribution was only related to getting UART3 work from the framework perspective. In your case, this seems OK, but something in Marlin 2.0.7 (perhaps the pull request on debug pins?) seems to break configuration how UART3 is used / defined. I used UART 3 externally, for hooking up an MMU2. This pull request that you posted to deals with the platformio framework, which seems OK from your observations. I cannot help you with this, but suggest you try 2.0.7 while taking out the changes of Marlin#19475 to see if that indeed caused it and if so pursuing it in Marlin. |
Thanks for your reply. Do definitions in pinsDebug.h actually assign pins or do they just give them names? |
The LPC1769 allows functions to be multiplexed to various pins.
Board manufacturers choose which pins are used for which function.
This allows to choose any pin combination for UARTs to be defined by additional optional symbols.
It can resolve the BigTreeTech used different pins for UART3, compared to other manufaturers.
In this case defineing UART_PIN_82_85 enables UART3, which is available at the Wifi port.
The symbols per UART are:
DISCLASIMER
Currently I have only tested correct functioning of this setting for UART3 with UART3_PIN_82_85 for my BTT SKR 1.4 Turbo.
All settings were derived from the hardware documentation:
https://github.com/bigtreetech/BIGTREETECH-SKR-V1.3/blob/master/BTT%20SKR%20V1.4/Datasheet/LPC17-32-IC.pdf