esp32/uart: Add flow keyword argument to enable hardware flow control.#6902
esp32/uart: Add flow keyword argument to enable hardware flow control.#6902willsowerbutts wants to merge 2 commits intomicropython:masterfrom
Conversation
|
This differs from #5608 because flow control is not enabled implicitly when the RTS/CTS pins are specified, only when the caller explicitly requests it with the new "flow" constructor keyword. |
|
Yes, we had that discussion in the PR. My point was more, that this PR and another one for this feature is stalling since then. |
|
Well I hope one of them gets merged, hardware flow control is broken on esp32. I'm open to suggestions for any way that I can help, I'd love to see this fixed as my project depends upon it. |
willsowerbutts
force-pushed
the
master
branch
3 times, most recently
from
3fe789d to
c1e5be4
Compare
|
I took another look at this, and at the other attempts at hardware flow control (#5608 and #5567), trying to learn from the previous feedback. I've updated the new "flow" constructor keyword parameter to use constants named "RTS" and "CTS" (without the "FLOW_" prefix I used previously) in order to match what the STM32 port already does as closely as possible. I've updated the machine.UART documentation to describe the interface offered by both STM32 and ESP32. I also explain briefly how RTS and CTS are used as the signal names alone can be confusing. #5567 combines both UART modes and hardware flow control. These are two independent issues and I am not addressing UART modes in this patch. I believe many of the comments made about #5567 are addressed in this patch. I've changed the RTS threshold from 50% to 75% of the UART hardware FIFO size, as recommended in #5608. I would be happy to add an additional keyword parameter to allow the user to specify this, but I believe 75% is a sensible value. |
willsowerbutts
force-pushed
the
master
branch
2 times, most recently
from
54bdd26 to
71d2916
Compare
This enables optional support for the hardware UART to use the RTS and/or CTS pins for flow control. The new "flow" constructor keyword specifies a bitmask of RTS and/or CTS. This matches the interface used by machine.UART on STM32. Previously on ESP32 it was possible to specify which pins to use for the RTS and CTS signals, but hardware flow control was never functional: CTS was not checked before transmitting bytes, and RTS was always driven high (signalling no buffer space available). With this patch, CTS and RTS both operate as expected. This also includes an update to the machine.UART documentation. Signed-off-by: Will Sowerbutts <will@sowerbutts.com>
willsowerbutts
force-pushed
the
master
branch
from
71d2916 to
e894f09
Compare
|
LGTM at a glance... |
|
Thank you for the contribution, this looks really good. Merged in a367529 (with minor change to the docs, because rp2 now also supports flow). |
Add `coproc` module
4 participants
This enables optional support for the hardware UART to use the RTS
and/or CTS pins for flow control.
The new "flow" constructor keyword specifies a bitmask of RTS and CTS.
Previously it was possible to specify which pins to use for the RTS and
CTS signals, but hardware flow control was never functional: CTS was not
checked before transmitting bytes, and RTS was always driven high
(signalling no buffer space available). With this patch, CTS and RTS
both operate as expected.
Signed-off-by: Will Sowerbutts will@sowerbutts.com