Re: [Breaking change] Echo character by serial driver
I'm Qi Huang, changed to a new mailbox to avoid messing massge in mail titile. > If isconsole is set and TERMIOS is not, then it will always echo the > input. The CR-LF behavior only depends on isconsole and can't be > changed even if TERMIOS is enabled. If TERMIOS enabled, the CR-LF behavior only determined by ifags/oflags in termios' setting, it can be changed even if isconsole is set. > The ECHO behavior can be disabled ONLY if TERMIOS are enabled. > TERIMIOS is now required by POSIX and, further, if TERMIOS is not > system, many features are now broken. Like hiding the password when > logging into NSH. Yes maybe we should select TERMIOS if some features really need it ? Most apps don't need it, only termcurse/nsh login rely on it now in my known. > I have never seen that behavior. That bug is probably introduced recently. I see the echo by telnet can be disabled by special command and it sended, but it don't works now.
答复: 答复: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver
> I think my biggest concern with the solution is that it > effectively makes > it impossible to received any out-of-band data: Any > encoded, > context-dependent, escaped data will be echoed back to the > host > unconditionally making it impossible to receive cleanly. But this behavior can be disabled by termios if needed, and it only affect console port by default. And cr/lf convertion also break this. > We have already seen this in escape sequences sent from the > host appearing > appearing on the user console. An awful solution > was adopted: Just remove > the escape sequence. That is called > "sweeping the problem under the rug." > This will be a serious > limitation that will effect our capability to > implement many serial > protocols and bad thing to happen to the OS. The current approach could be improved, such as let readline/cle to erase the escape sequence on console, what's your suggestion ? In general, I think it's a step forward, why we submit such a patch? 1.Telnet before this patch always echo input twice since telnet echo character itself by default. 2.Many application must do echo itself, such as readline/cle/bas. But the bigge problem is, for some software ported from other posix compatible system, they don't do echo by themself, for these applications like WAMR's repl (respacially with REPL) you can't see the input unless you modify the source to adapt it to NuttX. 发件人: Qi3 Huang 黄齐 发送时间: 2023年3月11日 0:05:41 收件人: dev@nuttx.apache.org 主题: 答复: 答复: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver Thanks for point that, so my change will affect them by serial.c since it's in common logic (excatly, is in uart_read()). 发件人: Gregory Nutt 发送时间: 2023年3月10日 23:58:29 收件人: dev@nuttx.apache.org 主题: Re: 答复: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver [外部邮件] 此邮件来源于小米公司外部，请谨慎处理。若对邮件安全性存疑，请将邮件转发给mi...@xiaomi.com进行反馈 > Emm, I confirmed that one of my test platform (esp32c3), its cdcacm driver > implement the uart_ops, but rp2040 seems not. > > > CLE should works since it will override the content from driver ECHO if over > serial driver, but readline shouldn't echo input. > > > I'll test more to confirm it. cdcacm.c is the part that is similar to serial.c. It implements the serial driver interface and also interfaces with the lower level USB device driver. cdcacm.c resides at drivers/usbdev/cdcacm.c and it common for all MCUs. The MCU-specif logic has no idea that there is a cdcacm "gadget" attached to it. cdcacm.c implements the common uart_ops that are used by all MCUs to support CDC/ACM serial. pl2303.c is another USB serial device. It also implements uart_ops and has its own TERMIOS implementation. #/**本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!**/#
答复: 答复: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver
Thanks for point that, so my change will affect them by serial.c since it's in common logic (excatly, is in uart_read()). 发件人: Gregory Nutt 发送时间: 2023年3月10日 23:58:29 收件人: dev@nuttx.apache.org 主题: Re: 答复: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver [外部邮件] 此邮件来源于小米公司外部，请谨慎处理。若对邮件安全性存疑，请将邮件转发给mi...@xiaomi.com进行反馈 > Emm, I confirmed that one of my test platform (esp32c3), its cdcacm driver > implement the uart_ops, but rp2040 seems not. > > > CLE should works since it will override the content from driver ECHO if over > serial driver, but readline shouldn't echo input. > > > I'll test more to confirm it. cdcacm.c is the part that is similar to serial.c. It implements the serial driver interface and also interfaces with the lower level USB device driver. cdcacm.c resides at drivers/usbdev/cdcacm.c and it common for all MCUs. The MCU-specif logic has no idea that there is a cdcacm "gadget" attached to it. cdcacm.c implements the common uart_ops that are used by all MCUs to support CDC/ACM serial. pl2303.c is another USB serial device. It also implements uart_ops and has its own TERMIOS implementation. #/**本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!**/#
答复: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver
> Telnet is the same. It does not use any of the changes to serial.c. It > redirects stdin and stdout to the socket interface exchages raw data > with the host. It has now TERMIOS support. I'm sure telnet will echo input itself, so it works with some display issues before. 发件人: Gregory Nutt 发送时间: 2023年3月10日 23:47:52 收件人: dev@nuttx.apache.org 主题: Re: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver [外部邮件] 此邮件来源于小米公司外部，请谨慎处理。若对邮件安全性存疑，请将邮件转发给mi...@xiaomi.com进行反馈 > >> nsh over usb works with readline/cle now. >> >> >> In face, isconsole in current implementation is more like a very tiny >> `line driver` inside the serial driver. > > That is very interesting. The serial driver > (drivers/usbdev/cdcacm.c). Does not include any of your change. It > does not use serial.c . It has its own serial interface and its own > TERMIOS implementation that does not use your changes. > > So I would expect serial.c to be better behaved without the ECHO as well. > Telnet is the same. It does not use any of the changes to serial.c. It redirects stdin and stdout to the socket interface exchages raw data with the host. It has now TERMIOS support. #/**本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!**/#
答复: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver
Emm, I confirmed that one of my test platform (esp32c3), its cdcacm driver implement the uart_ops, but rp2040 seems not. CLE should works since it will override the content from driver ECHO if over serial driver, but readline shouldn't echo input. I'll test more to confirm it. 发件人: Gregory Nutt 发送时间: 2023年3月10日 23:22:45 收件人: dev@nuttx.apache.org 主题: Re: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver [外部邮件] 此邮件来源于小米公司外部，请谨慎处理。若对邮件安全性存疑，请将邮件转发给mi...@xiaomi.com进行反馈 > nsh over usb works with readline/cle now. > > > In face, isconsole in current implementation is more like a very tiny `line > driver` inside the serial driver. That is very interesting. The serial driver (drivers/usbdev/cdcacm.c). Does not include any of your change. It does not use serial.c . It has its own serial interface and its own TERMIOS implementation that does not use your changes. So I would expect serial.c to be better behaved without the ECHO as well. #/**本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!**/#
答复: [External Mail]Re: [Breaking change] Echo character by serial driver
nsh over usb works with readline/cle now. In face, isconsole in current implementation is more like a very tiny `line driver` inside the serial driver. 发件人: Gregory Nutt 发送时间: 2023年3月10日 23:09:12 收件人: dev@nuttx.apache.org 主题: [External Mail]Re: [Breaking change] Echo character by serial driver [外部邮件] 此邮件来源于小米公司外部，请谨慎处理。若对邮件安全性存疑，请将邮件转发给mi...@xiaomi.com进行反馈 >>> Isn't the default setting of ECHO disabled? My understanding is the >>> ECHO is always /disabled /unless it is specifically /enabled /in the >>> TERMIOS settings. >>> >> No, it depends on isconsole: >> https://github.com/apache/nuttx/blob/master/drivers/serial/serial.c#L1303-L1308 >> https://github.com/apache/nuttx/blob/master/drivers/serial/serial.c#L1821-L1840 >> https://github.com/apache/nuttx/blob/master/drivers/serial/serial.c#L339-L344 >> All terminal special handling(\r\n<->\n, ECHO CRTL+C) is activated if >> isconsole equals true. >> And all serial ports designed for shell set isconsole=true statically. It would be a good thing to eliminate isconsole. Any tty device should be capable of being a console. Has anyone tests the USB serial drivers? Does the console still work over USB? > The first and last deal with CR-LF expansion as I mentioned. The > second is a little different. This is 1) enabling signal handling to > support Ctrl-C behavior and enabling CR-LF expansion. Oddly, it also > enables echo. Those, of course, never did anything in the past: > CR-FL expansion was controlled by the isonsole flag and t/*here was no > implementation of ECHO before 68384e9db42*/. So, yes, you are right > about some of that. Those "odd behaviors", including select ECHO where added by Huanq Qi in 3e41bd8b359 and 68384e9db42 and never existed in the original code. It is part of the disastrous implementation of ECHO and never should have been there at all. As I said originally, isconsole was only added as a kludge to get CR-LF. #/**本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!**/#
答复: [External Mail]Re: [Breaking change] Echo character by serial driver
I think I should give a simple brief of questions mentioned: 1. How to handle so many vt100 commands ? There aren't many vt100 commands actually used, current implementation can meet nearly all requirment in apps (only pdcurse need test, I'll have a test soon). If we really need to handle all vt100 command someday, we can add a line driver on top of serial driver like linux does. 2. Does CLE break? CLE and termcurse (VI) works fine, but in pdcurse, console was set to raw mode and ECHO restored after exit to nsh. CLE meet a issue on specific hardware due to driver, but have a fix https://github.com/apache/nuttx-apps/pull/1650 , it should works, wait for the feedback. 3. TERMIOS In my test now, only termcurse rely on raw mode (maybe another is pdcurse), so the must set raw mode for them. Enable TERMIOS will take about more ~840 bytes text and few bytes of bss for armv7m (other platform should close to it), so we can select it for these library by Kconfig. 发件人: Xiang Xiao 发送时间: 2023年3月10日 22:45:58 收件人: dev@nuttx.apache.org 主题: [External Mail]Re: [Breaking change] Echo character by serial driver [外部邮件] 此邮件来源于小米公司外部，请谨慎处理。若对邮件安全性存疑，请将邮件转发给mi...@xiaomi.com进行反馈 On Fri, Mar 10, 2023 at 10:39 PM Gregory Nutt wrote: > > On 3/10/2023 8:12 AM, Xiang Xiao wrote: > > The only problem is that terminal is optional: > > > https://github.com/apache/nuttx/blob/master/drivers/serial/Kconfig#L167-L174 > > So, if the user doesn't enable CONFIG_SERIAL_TERMIOS in defconfig, > readline > > will stop working due to failure to disable ECHO. > > Isn't the default setting of ECHO disabled? My understanding is the > ECHO is always /disabled /unless it is specifically /enabled /in the > TERMIOS settings. > No, it depends on isconsole: https://github.com/apache/nuttx/blob/master/drivers/serial/serial.c#L1303-L1308 https://github.com/apache/nuttx/blob/master/drivers/serial/serial.c#L1821-L1840 https://github.com/apache/nuttx/blob/master/drivers/serial/serial.c#L339-L344 All terminal special handling(\r\n<->\n, ECHO CRTL+C) is activated if isconsole equals true. And all serial ports designed for shell set isconsole=true statically. > > Three option here: > > > > 1. Remove SERIAL_TERMIOS and enable terminal setting always > I think so. I previously argued that TERMIOS should not be optional. > It is required by POSIX. There was discussion about the size increase, > but I don't think there is any significant size increase due to this. > We could measure the increase with TERMIOS enabled and disabled to see. > > 2. readline or special tool designed for raw mode select > SERIAL_TERMIOS > > in their config > That would be insufficient. Other logic needs TERMIOS too. Other than > a size increase, there is no harm in enabling TERMIOS. But if TERMIOS > is not enabled, then that can be a source of errors. > > 3. Keep SERIAL_TERMIOS option, but simulate terminal setting by > change > > isconsole flag > My recollection is that the isconsole flag was added ONLY to assure that > lines sent from the console terminated in CR-LF as required by most > terminal emulators. It never had anything to do with ECHO or other > TERMIOS settings recently. That extension of the semantics is not good > and introduces lots of problems. Just because a serial device is the > console device, that should not generally effect its behavior. > > Both code size and change is small for option 3. > > Except I don't think it is correct. > > #/**本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!**/#
[Breaking change] Echo character by serial driver
Hello everyone: I had submit a PR in last week https://github.com/apache/nuttx/pull/8691, this change makes NuttX align with other posix compatible system like linux. This improved experience like telnet and WAMR's repl, but also introduced some issues: 1. Don't echo again in application layer since it's done by driver now, refer to https://github.com/apache/nuttx-apps/pull/1608 If any other issues found, please inform me, thanks! BR, Huang Qi #/**本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!**/#
Changes in Common Line Editor
Hi all: I've submitted a new patch to GitHub (https://github.com/apache/nuttx-apps/pull/1650), in this is to maintain the cursor position in cle's internal instead of query from terminal, this avoid the query operation before each line read operation. Then the cle's behavior is much more like readline, and also avoid some potential issues if the connection between nuttx and PC is not so stable (serial driver? or cable link?). But, this patch assume that the nsh prompt is always shown at line start (ensure it by clear line command in vt100), is there any side effect for you? BR, Huang Qi #/**本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!**/#
