-
Notifications
You must be signed in to change notification settings - Fork 1.3k
mimxrt10xx: add one-directional UART #2934
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
Conversation
Going to follow this and #2898 up with a general Busio testing and cleanup PR that brings style in line across all three and enables proper pin resetting / never reset now that those functions are enabled. |
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.
Just a few minor things. Thanks!
I have nothing more to add on top of Scott's comments :) |
@tannewt ready for another look. |
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.
A suggestion for the xor but good otherwise. Generally I find the implicit bool logic in C to be confusing because it's the inverse of bash (I think.) I much prefer always using ==
or !=
to produce a boolean value.
@tannewt all set, thanks for the suggestion |
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.
This looks good! Just needs a translation update and the merge should bring in a fix for the esp build failure too.
Oh, and @arturo182's suggesting of raising ValueError is good too. Please do that as well. |
@tannewt should be all set |
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.
Looks good! Thank you!
This PR enables one-directional UART to Busio, allowing for the definition of a UART line that has only the TX or RX pin.
Throws an error if you try to do this while also defining pins for flow control, since the two are basically incompatible as far as I'm aware. Also adds the reserved_uart table so the same peripheral instance can't be used twice, and fixes some bugs I found in the stm32 UART implementation while using it for reference (whoops).
resolves #2484