Skip to content

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

nrf: Fix a few issues with UART REPL #17212

wants to merge 3 commits into from
+63 −21

Conversation

dpgeorge
Copy link
Member

Summary

This PR fixes three things with the nRF UART REPL:

  • Only process interrupt chars on UARTs used for REPL. Otherwise a board without REPL on UART can have its code interrupted if ctrl-C is received on the UART.
  • Fix UART write on parts that can't write more than 255 bytes.
  • Use 64 byte raw-paste buffer on PCA10028 and PCA10040, due to bug in JLink CDC.

Testing

Tested on PCA10028, PCA10040 and ARDUINO_NANO_33_BLE_SENSE, using the script from #15909.

dpgeorge added 3 commits April 30, 2025 10:31
@dpgeorge
nrf: Only process interrupt chars on UARTs used for REPL.
ff89c90 
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>
@dpgeorge
nrf: Fix UART write on parts that can't write more than 255 bytes.
6d280c0 
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>
@dpgeorge
nrf/boards: Use 64 byte raw-paste buffer on PCA10028 and PCA10040.
53a0844 
To workaround issues with JLink CDC.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge dpgeorge requested a review from robert-hh April 30, 2025 00:40
@robert-hh
Copy link
Contributor

@dpgeorge I will look at that, but today I'm a little bit busy. Tomorrow I'll have plenty of time.

@dpgeorge
Copy link
Member Author

Thanks. There's no hurry.

robert-hh
robert-hh reviewed May 1, 2025
View reviewed changes
ports/nrf/modules/machine/uart.c
#if defined(NRF52832)
// The nRF52832 cannot write more than 255 bytes at a time.
mp_uint_t chunk = MIN(255, remaining);
#else
Copy link
Contributor

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?

@robert-hh
Copy link
Contributor

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.

  • I verified that Ctrl-C does not act as keyboard-interrupt unless REPL is assigned ot UART.
  • The UART still works within the limits of this port. Receive works if the small 64Byte RX buffer is taken into account and the speed does not exceed 230400 baud. The test code you mentioned is for the STM32 port, so I could not use it.
  • The code looks correct.

A few notes not related to this PR:

  • At the NRF52840 running code cannot be interrupted by Ctrl-C at the USB interface. Ctrl-C is only recognized once something is sent to the console output.
  • Console output is not sent to the PC until at least 128 bytes are accumulated. Then 128 bytes are sent. Console Output via UART is sent immediately.

@dpgeorge
Copy link
Member Author

dpgeorge commented May 2, 2025

Thanks for testing and review!

The test code you mentioned is for the STM32 port, so I could not use it.

I did update #15909 to work on all ports, or at least it should work. It detects if pyb is available or not, and falls back to sys.stdXXX. It also uses sys.stdout.buffer or just sys.stdout if buffer is not available. So it should work with the nrf port.

  • At the NRF52840 running code cannot be interrupted by Ctrl-C at the USB interface.

Yes, I also noticed that. I think we can fix that in a separate PR.

  • Console output is not sent to the PC until at least 128 bytes are accumulated

Oh, that's interesting, I didn't see that behaviour. Is that with ARDUINO_NANO_33_BLE_SENSE?

@robert-hh
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robert-hh robert-hh robert-hh approved these changes

Assignees
No one assigned
Labels
port-nrf
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dpgeorge @robert-hh