stdio_uart: add stdio_uart_onlcr option to convert LF -> CRLF on output
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.
Updated to fix whitespace
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.
Any feedback?
Any updates?
Sorry for having to wait so long for getting a review.
Given that the effort to enable the conversion of "\n" to "\r\n" on the RIOT side (adding one line to the Makefile) is typically the same as configuring the receiver side to use "\n" for line endings, I do not really see the benefit of it.
To me, the trade-off looks like this:
- :+1: no need to add a line to the serial program's config used. (Note that if you open the serial via
make term, RIOT will provide the correct symbol rate, the correct line ending encoding, and magic signals via DTR / RTS as needed. If your favorite program is not supported, it certainly could be added.) - :-1: a module has to be selected in addition (IMO the same work as it safes)
- :-1: it increases the size of the
.textsection by a few bytes - :-1: it adds a few CPU cycles and one byte per line of overhead
If you insist that this will be useful to you, I can review it. But to me the alternative of configuring the receiver side to except "\n" as line endings just looks more appealing.
Hacking the terminal emulator to do the wrong thing is ... just that... the wrong thing. This means that RIOT-OS can not be used easily with common systems, plugged into Windows systems, or actual terminals. . o O ( telnet towel.blinkenlights.nl )
If you are worried about efficiency of looking for \n and sending \r\n, then maybe serial communications is not for you.
Hacking the terminal emulator to do the wrong thing is ... just that... the wrong thing.
This is not technically correct. UART is a different thing than the old terminal interfaces. We have particulate matter sensors, MP3 players, GPS receivers etc. interfacing via UART. These devices use a binary request-response protocol using UART as physical layer.
There is no such thing "plug and play" for terminals with an UART interface. In addition to symbol rate, parity, and stop bits one needs to agree on that the interface is actually used as serial console. Yes, you can use UART for a serial console, but this doesn't make UART a "terminal interface". One can also use USB (e.g. via CDC ACM), SPI, I2C, or any network interface as a transport for a serial console as well.
telnet towel.blinkenlights.nl
With e.g. telnet it is actually specified that the line endings are \r\n. RIOT can provide access to the shell via telnet btw. and it does follow the standard and consequently translates \n to \r\n there.
But in any case it doesn't IMO matter to engage in a theoretical discussion here. It seems there are people that do have a use case for \n --> \r\n conversion on the device side. As OS developers we IMO should strive to enable users to address use case. So let's get this in.
(Still, rebasing and addressing the review comments is still needed to merge this.)
I haven't looked at the details of this pull request yet, but I don't want to put the fix in the UART code for all the reasons above. It needs to go into the puts() or equivalent lower-level function. I hope to fix my ignorance of this code today.
Any feedback?
I have argued that your fix is right, and I've rebased it against master.
@jimparis would you like to continue with this PR? there's consensus now that we want this. Or should we go with @mcr's rebased version?
I'm not able to work on this currently, and so I'm happy to go with @mcr 's version (thanks for picking this up!)
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: https://github.com/RIOT-OS/RIOT/pull/18731