stdio_uart: add stdio_uart_onlcr option to convert LF -> CRLF on output #18703
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add USE_MODULE += "stdio_uart_onlcr" to enable it. This is named after the "onlcr" stty flag, which does the same thing.
github-actions
bot
added
Area: build system
benpicco
reviewed
Oct 6, 2022
| @@ -87,6 +87,22 @@ ssize_t stdio_read(void* buffer, size_t count) | |||
|
|
|||
| ssize_t stdio_write(const void* buffer, size_t len) | |||
| { | |||
| #ifdef MODULE_STDIO_ETHOS | |||
| ethos_send_frame(ðos, (const uint8_t *)buffer, len, ETHOS_FRAME_TYPE_TEXT); | |||
| const uint8_t *buf = (const uint8_t *)buffer; | ||
| const uint8_t cr = '\r'; | ||
| size_t rem = len; | ||
| while (rem--) { |
There was a problem hiding this comment.
If we ever get DMA on the UART it would be faster to write whole chunks - so better search for the \n with strchr/memchr
There was a problem hiding this comment.
@maribu already sketched that in the original PR review: https://github.com/RIOT-OS/RIOT/pull/15619/files#r970405335
There was a problem hiding this comment.
That patch is fully functioning and tested ;)
|
OK, let's get this in for the next release. With the feature freeze kicking in at the end of today, I picked this up and addressed the remaining issues: #18731 |
|
Can we close this PR in favor of #18731? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add USE_MODULE += "stdio_uart_onlcr" to enable it. This is named after the "onlcr" stty flag, which does the same thing.
Jim wrote:
This adds an option, enabled by USE_MODULE += "stdio_uart_onlcr", to automatically convert LF to CRLF on stdio UART output. (\n to \r\n). This is named after the "onlcr" stty flag, which does the same thing on a Unix system.
It's an easy global fix for something which has come up a lot before, e.g.:
Wiki
PR #4899
Issue #3040
Issue #12540
This approach of converting on output is used by many other embedded OSes and lets RIOT output work without needing specific configuration on the other end's terminal software, or any changes to strings throughout the OS and application.
Contribution description
This is a rebase of #15619
Testing procedure
I compiled this code for ESP32, and saw that it resulted in an orderly output of console terminal output.
Issues/PRs references
Fixed #4899, fixes #3040, fixes #12540
fixes #18286