-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
nrf: Fix a few issues with UART REPL #17212
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
@dpgeorge I will look at that, but today I'm a little bit busy. Tomorrow I'll have plenty of time. |
Thanks. There's no hurry. |
#if defined(NRF52832) | ||
// The nRF52832 cannot write more than 255 bytes at a time. | ||
mp_uint_t chunk = MIN(255, remaining); | ||
#else |
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.
Would it make sense to have this 255 number configurable?
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.
I've now made this a macro.
Lacking the PCA boards I could not do too much of testing. I tested some aspect with a Arduino Nanon 3 BLE, NRF 52840 and ran that PR on Micro:BIt V1. The latter uses the single UART for REPL.
A few notes not related to this PR:
|
Thanks for testing and review!
I did update #15909 to work on all ports, or at least it should work. It detects if
Yes, I also noticed that. I think we can fix that in a separate PR.
Oh, that's interesting, I didn't see that behaviour. Is that with ARDUINO_NANO_33_BLE_SENSE? |
That's with NRF52840 boards, tested ARDUINO_NANO_33_BLE_SENSE and SEEED_XIAO_NRF52. |
I investigated this issue. Indeed it is there. The following script triggers the bug (run via import sys,time
sys.stdout.write('a'*317+'\r\n')
for _ in range(100):
time.sleep(.01)
print('done') It doesn't print anything until after the script finishes. It seems that 320 bytes is the magic limit (the above test writes out 319 bytes before sleeping). The issue is due to LTO in combination with So that seems like a separate issue to the ones addressed in this PR. This PR is related to UART REPL. |
This commit adds an `attached_to_repl` property to each UART, and makes sure that property is correctly set/unset when the UART is attached to or detached from the REPL. That property is then used to make sure incoming characters on the UART are only checked for the interrupt character if the UART is attached to the REPL. Otherwise a board without REPL on UART can have its code interrupted if ctrl-C is received on the UART. Also, put incoming UART characters on to `stdin_ringbuf` instead of the UARTs ring buffer (the former is much larger than the latter). Signed-off-by: Damien George <damien@micropython.org>
Some MCUs cannot write more than 255 bytes to the UART at once. Eg writing 256 bytes gets truncated to 0, writing 257 gets truncated to 1, etc. Signed-off-by: Damien George <damien@micropython.org>
To workaround issues with JLink CDC. Signed-off-by: Damien George <damien@micropython.org>
53a0844
to
cc7eb1a
Compare
Summary
This PR fixes three things with the nRF UART REPL:
Testing
Tested on PCA10028, PCA10040 and ARDUINO_NANO_33_BLE_SENSE, using the script from #15909.