10000 shared/readline: handle \r and \n correctly by maass-hamburg · Pull Request #18713 · micropython/micropython · GitHub
[go: up one dir, main page]
Skip to content

shared/readline: handle \r and \n correctly #18713

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
maass-hamburg wants to merge 1 commit into
base: master
Choose a base branch
from
Open

shared/readline: handle \r and \n correctly #18713

maass-hamburg wants to merge 1 commit into from
+19 −1

Conversation

@maass-hamburg
Copy link
@maass-hamburg maass-hamburg commented Jan 22, 2026

Summary

the old code was only handeling \r and ignoring
\n, which is a problem for systems that only send \n.

this handels it better. btw zephyr uses a similar logic in its shell.

Testing

tested on the zephyr port

Trade-offs and Alternatives

@maass-hamburg maass-hamburg force-pushed the readline_handle_n_r branch 2 times, most recently from da20223 to 1ec37f4 Compare January 22, 2026 16:39
@maass-hamburg
shared/readline: Handle \r and \n correctly.
9a417b2 
The old code was only handeling `\r` and ignoring
`\n`, which is a problem for systems that only send `\n`.

this handels it better. btw zephyr uses a similar logic
in its shell.

Signed-off-by: Fin Maaß <f.maass@vogl-electronic.com>
@github-actions
Copy link
github-actions bot commented Jan 22, 2026

Code size report:

Reference:  tests/float/complex1.py: Fix CPython 3.14 deprecation. [b14d129]
Comparison: shared/readline: Handle \r and \n correctly. [merge of 9a417b2]
  mpy-cross:    +0 +0.000% 
   bare-arm:    +0 +0.000% 
minimal x86:   -20 -0.011% [incl +8(bss)]
   unix x64:   +24 +0.003% standard[incl +8(bss)]
      stm32:   -16 -0.004% PYBV10[incl +4(bss)]
      esp32:   +40 +0.002% ESP32_GENERIC[incl +8(bss)]
     mimxrt:   -16 -0.004% TEENSY40
        rp2:    +4 +0.000% RPI_PICO_W[incl +4(bss)]
       samd:   -16 -0.006% ADAFRUIT_ITSYBITSY_M4_EXPRESS[incl +4(bss)]
  qemu rv32:    +4 +0.001% VIRT_RV32[incl +4(bss)]

@codecov
Copy link
codecov bot commented Jan 22, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.38%. Comparing base (b14d129) to head (9a417b2).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master   #18713   +/-   ##
=======================================
  Coverage   98.38%   98.38%           
=======================================
  Files         171      171           
  Lines       22298    22298           
=======================================
  Hits        21937    21937           
  Misses        361      361

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Josverl
Copy link
Contributor
Josverl commented Jan 22, 2026

which is a problem for systems that only send \n.

Can you elaborate the issue?
Which systems,
How do they connect to MicroPython?

Is this a terminal? AFAIK the terminal protocol explicitly specified /r/n sequences that may seem out of place for those reared on unix, but are actually part of the formal spec.

@maass-hamburg
Copy link
Author
maass-hamburg commented Jan 23, 2026

Is this a terminal? AFAIK the terminal protocol explicitly specified /r/n sequences that may seem out of place for those reared on unix, but are actually part of the formal spec.

Which terminal protocol?

Btw one alternative would be to use \n instead and ignore \r. That would at least ensure bigger compatibility.

@Josverl
Copy link
Contributor
Josverl commented Jan 23, 2026

Which terminal protocol?

Termios default settings

But could you please answer the other questions, as I'm grasping in the dark what problem you are trying to solve.

@dpgeorge dpgeorge added the shared Relates to shared/ directory in source label Jan 25, 2026
@maass-hamburg
Copy link
Author
maass-hamburg commented Jan 26, 2026

Can you elaborate the issue?
Which systems,
How do they connect to MicroPython?

I used https://github.com/enjoy-digital/litex and the included litex_term, because I use the serial boot functionality to load the micropython zephyr image into the ram during boot and then start from it.

litex_term only sends \n. And it took me pretty long to understand, that that was the problem and that micropython was waiting for \r and not for \n. While it could also be changed in litex, I think it would be good, if micropython would support the single \n too, in case other terminals wouldn't send \r. Also lot of serial terminals have options to ether use \r, \r\n or \n, which would indicate that it isn't that standardized.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

shared Relates to shared/ directory in source

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@maass-hamburg @Josverl @dpgeorge
0