Re: Serial console BROKEN
If CONFIG_SERIAL_TERMIOS is not selected then non-consoles will do nothing. In order to restore legacy behavior you would have to do this: +#ifdef CONFIG_SERIAL_TERMIOS if ( -#ifdef CONFIG_SERIAL_TERMIOS dev->tc_iflag & ECHO -#else - dev->isconsole -#endif ) ... } +#endif That should eliminate the double character echo. So the command line tool has to do the different thing(echo or non echo by self) based on CONFIG_SERIAL_TERMIOS? No, the legacy behavior is that the serial driver never echos by itself unless ECHO is selected.
Re: Serial console BROKEN
On Thu, Mar 9, 2023 at 2:14 AM Nathan Hartman wrote: > I guess you have to assess the intended lifecycle of your product and > over-provision your MCU accordingly to the expected rate of growth of the > firmware. I would stick to the smallest-possible-core-kernel-and-base-by-design and put everything else optional so we do not obsolete-some-hardware-by-design :-) -- CeDeROM, SQ7MHZ, http://www.tomek.cedro.info
Re: Serial console BROKEN
On Wed, Mar 8, 2023 at 5:31 PM Gregory Nutt wrote: > > >> At some point NuttX will grow too large for deep embedded platforms. > >> > >> > >> My concern exactly. Yes, POSIX compliance is super important because it > >> provides portability: I regularly write a program and run it on PC and > >> embedded with almost no changes. This is one of the big selling points > of > >> NuttX for me. > > I don't that the specific TERMIOS configuration we are talking about > here is an significant issue. I don't expect any large code size change > as a consequence of enabling TERMIOS because the extent of logic enabled > is not enormous. Perhaps we should try to strike a reasonable balance: areas with a bigger flash footprint impact should be removable by Kconfig, especially if they're not always needed. Areas that don't eat too much flash could be always included, especially if they are in common use. In other words try to optimize for 80% of the impact and not for 20% of the impact. Related: This would be an issue for people who have to support a product > for an extended life. In the early 2010's, for example, there were > products using NuttX based on MCUs with 32Kb of FLASH memory. I suspect > those would already be in trouble. Exactly. And: The moral of this is: If you have to support the same hardware for > years and years, you really need to freeze the OS version and backport > any critical fixes as necessary. So, the problem here is that backports only get more difficult over time as the codebase grows and changes. I guess you have to assess the intended lifecycle of your product and over-provision your MCU accordingly to the expected rate of growth of the firmware. Nathan
Re: Serial console BROKEN
On Wed, Mar 8, 2023 at 11:31 PM Gregory Nutt wrote: > Related: This would be an issue for people who have to support a product > for an extended life. In the early 2010's, for example, there were > products using NuttX based on MCUs with 32Kb of FLASH memory. I suspect > those would already be in trouble. > > The moral of this is: If you have to support the same hardware for > years and years, you really need to freeze the OS version and backport > any critical fixes as necessary. ..and for retro computer folks that want to bring new life to old machines with a recent version of NuttX would be lost.. old is not always bad :-) > >> Maybe, like FreeBSD, we should declare support "tiers" for different > >> architectures and MCUs, with Tier 1 meaning supported fully and other tiers > >> for things like partial support (experimental?) or deprecated... just a > >> thought. > > Good idea... but I doubt that we could ever staff this. It could be simply implemented by assigning "maintainers" to a given platform / application / code, like FreeBSD Ports do :-) In general, person mostly interested in particular application and/or hardware voluntarily applies to be "maintainer" of this specific part of code / application / drivers (probably the author). Then all questions / updates / requests / flames are directed towards that person as the guru knowing the subject best. When person is not interested in maintaining anymore, then another maintainer is being searched, while the part of the code is marked as maintained by the community and supervised by the port tree maintainers team. Maybe there can be tags used in the code comments that would automate status generation in the doc/web. Commits and changes introduced to a specific "port" must be approved by its "maintainer" in the first place. Then they can be verified and committed to the source tree by "port tree maintainers" if the update is fine, or request a fix with hints on how to fix. That split of duties allows adding and updating single ports by less experienced users that are only interested in a specific change (i.e. application update), and it is then verified by people that are most experienced with the whole port tree organization. This is usually done with Bugzilla but I can see it may happen over GitHub too. https://bugs.freebsd.org/bugzilla/buglist.cgi?chfield=%5BBug%20creation%5D=24h_id=591118 https://github.com/freebsd/freebsd-ports/pulls?q=is%3Apr Working with kernel is similar, but all changes need to be carefully explained and reviewed and approved by several "core team members" (usually at least three) before commit. I think this is done with Phabricator but it may happen over GitHub too. https://github.com/freebsd/freebsd-src/pulls?q=is%3Apr > Slightly different topic: I have almost every board that ever ran NuttX > from about 2005 through 2020 or so. That is probably several hundred > boards. I don't use them any more and am thinking about just dumping > them to make space. Is there anyone willing to pay the shipping from > Costa Rica on a massive dump of older but interesting hardware? TREASURES!! =) What would be the cost??? Priv plz plz :-) -- CeDeROM, SQ7MHZ, http://www.tomek.cedro.info
Re: Serial console BROKEN
On Wed, Mar 8, 2023 at 11:14 PM Nathan Hartman wrote: > On Wed, Mar 8, 2023 at 4:02 PM Sebastien Lorquet > wrote: > > You are right about posix compliance, this is a valuable goal, but at > > the same time it raises the hard remark: > > At some point NuttX will grow too large for deep embedded platforms. > > My concern exactly. Yes, POSIX compliance is super important because it > provides portability: I regularly write a program and run it on PC and > embedded with almost no changes. This is one of the big selling points of > NuttX for me. > > At the same time, there's another kind of portability: between MCU models > and families. Before NuttX if I had a firmware running perfectly on PIC32 > and wanted to move it to Tiva I had to rewrite everything. NuttX support > for many MCU models eliminates this (except the occasional bugfix in low > level driver code or something). That's a huge advantage. Supporting lots > of architectures means the next project is likely to find one that will > work. > > If we won't fit on the smaller micros, we'll lose some of that advantage. > Maybe it doesn't matter. Maybe the days of super small MCUs are numbered > and things are moving to bigger flash, 64-bit, and Raspberry Pi class > embedded systems. Maybe that's okay. But, if that's the direction to go, > then we just need to be clear about what our future holds so that users > won't build designs around micros that are not going to be supported for > much longer. Well for me this is the biggest advantage of NuttX over other RTOS! It is advertised as working on 8-bit CPU/MCU upwards and it would be really nice that things stayed that way. I would love to port NuttX one day to MC68000 and MOS6502. Probably others will also switch to NuttX because of that scalability. Loosing that will cause NuttX loose its strongest point. Why not stay with microkernel that can fit into kilobytes of flash and ram and keep all other stuff into module / library / application layer? That way extreme scalability would be maintained and new features could show up with no conflict? For instance, lets take old Atari, or other MCU, with 64KB of RAM and similar (possibly smaller) Flash size. Kernel + custom application would fit in. Data can be provided in a raw form. Additional processing can be done by additional optional application/module/library at the cost of required memory. Additional processing can be easily done by external machine / platform (i.e. terminal application on a pc). But if we want to add a web server then more powerful MCU and/or additional memory is required. Still it would be possible to fit core functionality on a tiny MCU. Worst case (?) is when we have tiny kernel and all required functionalities (provided with libraries) are packed into application that determines memory requirements for the hardware. I guess this is most common approach in RTOS anyway? > Maybe, like FreeBSD, we should declare support "tiers" for different > architectures and MCUs, with Tier 1 meaning supported fully and other tiers > for things like partial support (experimental?) or deprecated... just a > thought. This is a very nice idea that would extend current Supported Platforms [1] to know what is the actual status of the hardware support at first glance. I like to know things at first glance. This would not really introduce any new bonds or obligations but provide better insight for the potential (new) users :-) [1] list on https://nuttx.apache.org/docs/latest/introduction/supported_platforms.html -- CeDeROM, SQ7MHZ, http://www.tomek.cedro.info
Re: Serial console BROKEN
At some point NuttX will grow too large for deep embedded platforms. My concern exactly. Yes, POSIX compliance is super important because it provides portability: I regularly write a program and run it on PC and embedded with almost no changes. This is one of the big selling points of NuttX for me. I don't that the specific TERMIOS configuration we are talking about here is an significant issue. I don't expect any large code size change as a consequence of enabling TERMIOS because the extent of logic enabled is not enormous. But the general principle and the agreement on the project values is more significant. At the same time, there's another kind of portability: between MCU models and families. Before NuttX if I had a firmware running perfectly on PIC32 and wanted to move it to Tiva I had to rewrite everything. NuttX support for many MCU models eliminates this (except the occasional bugfix in low level driver code or something). That's a huge advantage. Supporting lots of architectures means the next project is likely to find one that will work. Related: This would be an issue for people who have to support a product for an extended life. In the early 2010's, for example, there were products using NuttX based on MCUs with 32Kb of FLASH memory. I suspect those would already be in trouble. The moral of this is: If you have to support the same hardware for years and years, you really need to freeze the OS version and backport any critical fixes as necessary. If we won't fit on the smaller micros, we'll lose some of that advantage. Maybe it doesn't matter. Maybe the days of super small MCUs are numbered and things are moving to bigger flash, 64-bit, and Raspberry Pi class embedded systems. Maybe that's okay. But, if that's the direction to go, then we just need to be clear about what our future holds so that users won't build designs around micros that are not going to be supported for much longer. We can't compete in size with RTOSs like FreeRTOS, mbed, ChibiOS. Never could. Those things only need about 4Kb of memory and would be the RTOS of choice for use with really small MCUs. Maybe, like FreeBSD, we should declare support "tiers" for different architectures and MCUs, with Tier 1 meaning supported fully and other tiers for things like partial support (experimental?) or deprecated... just a thought. Good idea... but I doubt that we could ever staff this. Slightly different topic: I have almost every board that ever ran NuttX from about 2005 through 2020 or so. That is probably several hundred boards. I don't use them any more and am thinking about just dumping them to make space. Is there anyone willing to pay the shipping from Costa Rica on a massive dump of older but interesting hardware?
Re: Serial console BROKEN
On Wed, Mar 8, 2023 at 4:02 PM Sebastien Lorquet wrote: > You are right about posix compliance, this is a valuable goal, but at > the same time it raises the hard remark: > > At some point NuttX will grow too large for deep embedded platforms. My concern exactly. Yes, POSIX compliance is super important because it provides portability: I regularly write a program and run it on PC and embedded with almost no changes. This is one of the big selling points of NuttX for me. At the same time, there's another kind of portability: between MCU models and families. Before NuttX if I had a firmware running perfectly on PIC32 and wanted to move it to Tiva I had to rewrite everything. NuttX support for many MCU models eliminates this (except the occasional bugfix in low level driver code or something). That's a huge advantage. Supporting lots of architectures means the next project is likely to find one that will work. If we won't fit on the smaller micros, we'll lose some of that advantage. Maybe it doesn't matter. Maybe the days of super small MCUs are numbered and things are moving to bigger flash, 64-bit, and Raspberry Pi class embedded systems. Maybe that's okay. But, if that's the direction to go, then we just need to be clear about what our future holds so that users won't build designs around micros that are not going to be supported for much longer. Maybe, like FreeBSD, we should declare support "tiers" for different architectures and MCUs, with Tier 1 meaning supported fully and other tiers for things like partial support (experimental?) or deprecated... just a thought. Cheers Nathan
Re: Serial console BROKEN
You are right about posix compliance, this is a valuable goal, but at the same time it raises the hard remark: At some point NuttX will grow too large for deep embedded platforms. That may or may not be true. Certainly NuttX has outgrown most old architectures with 16-bit address space limitations. Other old retro architectures with memory limitations are also at risk. But, in general, I would say that deeply embedded hardware memory resources have outpaced the growth in NuttX size. I would tend to bet that that growing too large for the mainstream won't happen. I don't think this is an issue for anyone but retro hardware fans. But then, "All users matter." These are just the sort of tough tradeoffs that should be being addressed in this list. Remember when NuttX with NSH would fit in 12Kb?
Re: Serial console BROKEN
You are right about posix compliance, this is a valuable goal, but at the same time it raises the hard remark: At some point NuttX will grow too large for deep embedded platforms. Sebastien On 3/8/23 21:41, Gregory Nutt wrote: Historically, whenever we find a POSIX issue we have always fixed it in order to as compliant as possible. In the hierarchy of values, POSIX is probably at the top of the list and well above personal preferences and novel solutions. In the name of POSIX compliance, we have eliminated architecture support, increased code size, forced redesigns, etc. So I think it is pretty hard to come with a truly convincing argument why we should support a clearly non-compliant behavior (or lack or behavior). On 3/8/2023 2:35 PM, Nathan Hartman wrote: On Wed, Mar 8, 2023 at 3:20 PM Gregory Nutt wrote: POSIX defines the TERMIOS options and, I suspect that those TERMIOS options are required, not optional (need to check that). If that is true, then there should be no CONFIG_SERIAL_TERMIOS option; it should always be enabled. Unless the user (like me) knows termios will never be used. I have both scenarios, used (serial config changes at runtime -- connected to external equipment) and never used (serial config set once at build time -- connected to a chip on the circuit board). Even though we want as close to POSIX compliant as possible, we still should allow to eliminate code to reduce flash footprint whenever possible. Otherwise we won't be for deeply embedded anymore. Cheers Nathan
Re: Serial console BROKEN
Historically, whenever we find a POSIX issue we have always fixed it in order to as compliant as possible. In the hierarchy of values, POSIX is probably at the top of the list and well above personal preferences and novel solutions. In the name of POSIX compliance, we have eliminated architecture support, increased code size, forced redesigns, etc. So I think it is pretty hard to come with a truly convincing argument why we should support a clearly non-compliant behavior (or lack or behavior). On 3/8/2023 2:35 PM, Nathan Hartman wrote: On Wed, Mar 8, 2023 at 3:20 PM Gregory Nutt wrote: POSIX defines the TERMIOS options and, I suspect that those TERMIOS options are required, not optional (need to check that). If that is true, then there should be no CONFIG_SERIAL_TERMIOS option; it should always be enabled. Unless the user (like me) knows termios will never be used. I have both scenarios, used (serial config changes at runtime -- connected to external equipment) and never used (serial config set once at build time -- connected to a chip on the circuit board). Even though we want as close to POSIX compliant as possible, we still should allow to eliminate code to reduce flash footprint whenever possible. Otherwise we won't be for deeply embedded anymore. Cheers Nathan
Re: Serial console BROKEN
On Wed, Mar 8, 2023 at 3:20 PM Gregory Nutt wrote: > POSIX defines the TERMIOS options and, I suspect that those TERMIOS > options are required, not optional (need to check that). If that is > true, then there should be no CONFIG_SERIAL_TERMIOS option; it should > always be enabled. Unless the user (like me) knows termios will never be used. I have both scenarios, used (serial config changes at runtime -- connected to external equipment) and never used (serial config set once at build time -- connected to a chip on the circuit board). Even though we want as close to POSIX compliant as possible, we still should allow to eliminate code to reduce flash footprint whenever possible. Otherwise we won't be for deeply embedded anymore. Cheers Nathan
Re: Serial console BROKEN
POSIX defines the TERMIOS options and, I suspect that those TERMIOS options are required, not optional (need to check that). If that is true, then there should be no CONFIG_SERIAL_TERMIOS option; it should always be enabled. On 3/8/2023 2:15 PM, Nathan Hartman wrote: On Wed, Mar 8, 2023 at 2:26 PM Xiang Xiao wrote: Since the code to handle the special process is very small, is it better to always allow application change ECHO and OPOST through TCSETS? So, the special function or program can disable ECHO/OPOST programmatically. Only if termios support is enabled in Kconfig (CONFIG_SERIAL_TERMIOS), I think. Else all serial setup is static and cannot be altered programmatically -- this makes sense when connected to a dedicated device, such as MCU UART connected directly to another MCU UART or some sensor chip or interface chip with serial setup that never changes. Nathan
Re: Serial console BROKEN
Hi, This is a good idea to better follow posix. This is typically the kind of stuff that would have deserved a message on the dev list to say: Hey friends we are improving terminals, expect bugs because it's hard to get right, oh btw apps need to be updated too, otherwise strange things may happen like double echos. This is not doable in a pull request named "termios improvement" or something similar, lost in the middle of tens of other requests. It conveys no meaning and no information that it is a profound change with wide consequences. It is easy to miss, hard and long to find, but it would take you seconds to drop a message on the list. This would help A LOT to have your work understood by the community. It would also have avoided several github issues. We could contribute by doing more useful testing of corner cases and come up with more testing ideas. Like, proper community behaviour on our nice common project? Isnt that a normal thing to do? Sebastien On 3/8/23 20:26, Xiang Xiao wrote: On Thu, Mar 9, 2023 at 3:07 AM Gregory Nutt wrote: I imagine that this is occurring because readline also echos the input: https://github.com/apache/nuttx-apps/blob/master/system/readline/readline_common.c#L644 The low-level serial driver should not echo just because /isconsole /is true. Console echo is always handled by the application. I would say that that is a bug. From the spec: https://pubs.opengroup.org/onlinepubs/7908799/xsh/termios.h.html the serial driver should echo the char if ECHO is enabled. Since NuttX doesn't follow the spec, it makes many POSIX compliant interactive shells doesn't work very well. We have to make the breaking change to fix the wrong initial implementation. I still seems like the isconsole does not belong in the conditional: https://github.com/apache/nuttx/commit/68384e9db42e254b2cf6720bc3380aebdd2b298c if (dev->isconsole #ifdef CONFIG_SERIAL_TERMIOS || (dev->tc_iflag & ECHO) #endif ) { Currently: 893 if ( 894 #ifdef CONFIG_SERIAL_TERMIOS 895 dev->tc_iflag & ECHO 896 #else 897 dev->isconsole 898 #endif 899 ) 900 { If CONFIG_SERIAL_TERMIOS is not set then shouldn't the entire 'if ' condition should be removed? From my understanding, when CONFIG_SERIAL_TERMIOS isn't set whether serial driver do some special terminal handling(e.g. \n<->\r\n and echo) is controlled by isconsole flag: 1. isconsole equals false, all special handling is disable 2. isconsole equals true, all special handling is enable So, the check needs to be kept to ensure that non-console uart doesn't add the unxpected process. If CONFIG_SERIAL_TERMIOS is not selected then non-consoles will do nothing. In order to restore legacy behavior you would have to do this: +#ifdef CONFIG_SERIAL_TERMIOS if ( -#ifdef CONFIG_SERIAL_TERMIOS dev->tc_iflag & ECHO -#else - dev->isconsole -#endif ) ... } +#endif That should eliminate the double character echo. So the command line tool has to do the different thing(echo or non echo by self) based on CONFIG_SERIAL_TERMIOS? I suppose that you could also eliminate the echo in readline() (and cle()???) but that would probably mess up the command line editing since the in the edit commands would also be echoed by the serial driver. Since the code to handle the special process is very small, is it better to always allow application change ECHO and OPOST through TCSETS? So, the special function or program can disable ECHO/OPOST programmatically. By the way, the reason that this is implemented in a non-standard way like this with a flag is historical. The serial driver is one of the older parts of the system. There was no TERMIOS support in NuttX until much, much later in the development. The serial driver was just a flat, read/write character device like any other. readline() fakes all the the necessary operations of raw mode and other termios settings. Yes, but it's good time to follow the POSIX spec that moves the special process from readline to serial driver and make TCSETS for ECHO and OPOST always exist.
Re: Serial console BROKEN
On Wed, Mar 8, 2023 at 2:26 PM Xiang Xiao wrote: > > Since the code to handle the special process is very small, is it better to > always allow application change ECHO and OPOST through TCSETS? So, the > special function or program can disable ECHO/OPOST programmatically. Only if termios support is enabled in Kconfig (CONFIG_SERIAL_TERMIOS), I think. Else all serial setup is static and cannot be altered programmatically -- this makes sense when connected to a dedicated device, such as MCU UART connected directly to another MCU UART or some sensor chip or interface chip with serial setup that never changes. Nathan
Re: Serial console BROKEN
On Thu, Mar 9, 2023 at 3:07 AM Gregory Nutt wrote: > > I imagine that this is occurring because readline also echos the > input: > > > > >> > https://github.com/apache/nuttx-apps/blob/master/system/readline/readline_common.c#L644 > The low-level serial driver should not echo just because /isconsole > /is > true. Console echo is always handled by the application. I would say > that that is a bug. > > >>> From the spec: > >>> https://pubs.opengroup.org/onlinepubs/7908799/xsh/termios.h.html > >>> the serial driver should echo the char if ECHO is enabled. Since NuttX > >>> doesn't follow the spec, it makes many POSIX compliant interactive > shells > >>> doesn't work very well. > >>> We have to make the breaking change to fix the wrong initial > >> implementation. > >> > >> I still seems like the isconsole does not belong in the conditional: > >> > >> > >> > https://github.com/apache/nuttx/commit/68384e9db42e254b2cf6720bc3380aebdd2b298c > >> > >> if (dev->isconsole > >> #ifdef CONFIG_SERIAL_TERMIOS > >> || (dev->tc_iflag & ECHO) > >> #endif > >>) > >> { > >> > >> Currently: > >> > >>893 if ( > >>894 #ifdef CONFIG_SERIAL_TERMIOS > >>895 dev->tc_iflag & ECHO > >>896 #else > >>897 dev->isconsole > >>898 #endif > >>899 ) > >>900 { > >> > >> If CONFIG_SERIAL_TERMIOS is not set then shouldn't the entire 'if ' > >> condition should be removed? > >> > > From my understanding, when CONFIG_SERIAL_TERMIOS isn't set whether > serial > > driver do some special terminal handling(e.g. \n<->\r\n and echo) is > > controlled by isconsole flag: > > > > 1. isconsole equals false, all special handling is disable > > 2. isconsole equals true, all special handling is enable > > > > So, the check needs to be kept to ensure that non-console uart doesn't > add > > the unxpected process. > > > If CONFIG_SERIAL_TERMIOS is not selected then non-consoles will do > nothing. In order to restore legacy behavior you would have to do this: > >+#ifdef CONFIG_SERIAL_TERMIOS >if ( >-#ifdef CONFIG_SERIAL_TERMIOS > dev->tc_iflag & ECHO > -#else - dev->isconsole -#endif > ) >... > } > +#endif > > That should eliminate the double character echo. > > So the command line tool has to do the different thing(echo or non echo by self) based on CONFIG_SERIAL_TERMIOS? > I suppose that you could also eliminate the echo in readline() (and > cle()???) but that would probably mess up the command line editing since > the in the edit commands would also be echoed by the serial driver. > > Since the code to handle the special process is very small, is it better to always allow application change ECHO and OPOST through TCSETS? So, the special function or program can disable ECHO/OPOST programmatically. > By the way, the reason that this is implemented in a non-standard way like > this with a flag is historical. The serial driver is one of the older > parts of the system. There was no TERMIOS support in NuttX until much, > much later in the development. The serial driver was just a flat, > read/write character device like any other. readline() fakes all the the > necessary operations of raw mode and other termios settings. > > Yes, but it's good time to follow the POSIX spec that moves the special process from readline to serial driver and make TCSETS for ECHO and OPOST always exist.
Re: Serial console BROKEN
I imagine that this is occurring because readline also echos the input: https://github.com/apache/nuttx-apps/blob/master/system/readline/readline_common.c#L644 The low-level serial driver should not echo just because /isconsole /is true. Console echo is always handled by the application. I would say that that is a bug. From the spec: https://pubs.opengroup.org/onlinepubs/7908799/xsh/termios.h.html the serial driver should echo the char if ECHO is enabled. Since NuttX doesn't follow the spec, it makes many POSIX compliant interactive shells doesn't work very well. We have to make the breaking change to fix the wrong initial implementation. I still seems like the isconsole does not belong in the conditional: https://github.com/apache/nuttx/commit/68384e9db42e254b2cf6720bc3380aebdd2b298c if (dev->isconsole #ifdef CONFIG_SERIAL_TERMIOS || (dev->tc_iflag & ECHO) #endif ) { Currently: 893 if ( 894 #ifdef CONFIG_SERIAL_TERMIOS 895 dev->tc_iflag & ECHO 896 #else 897 dev->isconsole 898 #endif 899 ) 900 { If CONFIG_SERIAL_TERMIOS is not set then shouldn't the entire 'if ' condition should be removed? From my understanding, when CONFIG_SERIAL_TERMIOS isn't set whether serial driver do some special terminal handling(e.g. \n<->\r\n and echo) is controlled by isconsole flag: 1. isconsole equals false, all special handling is disable 2. isconsole equals true, all special handling is enable So, the check needs to be kept to ensure that non-console uart doesn't add the unxpected process. If CONFIG_SERIAL_TERMIOS is not selected then non-consoles will do nothing. In order to restore legacy behavior you would have to do this: +#ifdef CONFIG_SERIAL_TERMIOS if ( -#ifdef CONFIG_SERIAL_TERMIOS dev->tc_iflag & ECHO -#else - dev->isconsole -#endif ) ... } +#endif That should eliminate the double character echo. I suppose that you could also eliminate the echo in readline() (and cle()???) but that would probably mess up the command line editing since the in the edit commands would also be echoed by the serial driver. By the way, the reason that this is implemented in a non-standard way like this with a flag is historical. The serial driver is one of the older parts of the system. There was no TERMIOS support in NuttX until much, much later in the development. The serial driver was just a flat, read/write character device like any other. readline() fakes all the the necessary operations of raw mode and other termios settings.
Re: Serial console BROKEN
On Thu, Mar 9, 2023 at 2:17 AM Gregory Nutt wrote: > > >> I imagine that this is occurring because readline also echos the input: > >> > >> > >> > https://github.com/apache/nuttx-apps/blob/master/system/readline/readline_common.c#L644 > >> > >> The low-level serial driver should not echo just because /isconsole /is > >> true. Console echo is always handled by the application. I would say > >> that that is a bug. > >> > > From the spec: > > https://pubs.opengroup.org/onlinepubs/7908799/xsh/termios.h.html > > the serial driver should echo the char if ECHO is enabled. Since NuttX > > doesn't follow the spec, it makes many POSIX compliant interactive shells > > doesn't work very well. > > We have to make the breaking change to fix the wrong initial > implementation. > > I still seems like the isconsole does not belong in the conditional: > > > https://github.com/apache/nuttx/commit/68384e9db42e254b2cf6720bc3380aebdd2b298c > >if (dev->isconsole > #ifdef CONFIG_SERIAL_TERMIOS >|| (dev->tc_iflag & ECHO) > #endif > ) > { > > Currently: > > 893 if ( > 894 #ifdef CONFIG_SERIAL_TERMIOS > 895 dev->tc_iflag & ECHO > 896 #else > 897 dev->isconsole > 898 #endif > 899 ) > 900 { > > If CONFIG_SERIAL_TERMIOS is not set then shouldn't the entire 'if ' > condition should be removed? > >From my understanding, when CONFIG_SERIAL_TERMIOS isn't set whether serial driver do some special terminal handling(e.g. \n<->\r\n and echo) is controlled by isconsole flag: 1. isconsole equals false, all special handling is disable 2. isconsole equals true, all special handling is enable So, the check needs to be kept to ensure that non-console uart doesn't add the unxpected process.
Re: Serial console BROKEN
I imagine that this is occurring because readline also echos the input: https://github.com/apache/nuttx-apps/blob/master/system/readline/readline_common.c#L644 The low-level serial driver should not echo just because /isconsole /is true. Console echo is always handled by the application. I would say that that is a bug. From the spec: https://pubs.opengroup.org/onlinepubs/7908799/xsh/termios.h.html the serial driver should echo the char if ECHO is enabled. Since NuttX doesn't follow the spec, it makes many POSIX compliant interactive shells doesn't work very well. We have to make the breaking change to fix the wrong initial implementation. I still seems like the isconsole does not belong in the conditional: https://github.com/apache/nuttx/commit/68384e9db42e254b2cf6720bc3380aebdd2b298c if (dev->isconsole #ifdef CONFIG_SERIAL_TERMIOS || (dev->tc_iflag & ECHO) #endif ) { Currently: 893 if ( 894 #ifdef CONFIG_SERIAL_TERMIOS 895 dev->tc_iflag & ECHO 896 #else 897 dev->isconsole 898 #endif 899 ) 900 { If CONFIG_SERIAL_TERMIOS is not set then shouldn't the entire 'if ' condition should be removed?
Re: Serial console BROKEN
On Wed, Mar 8, 2023 at 12:09 AM Gregory Nutt wrote: > I imagine that this is occurring because readline also echos the input: > > > https://github.com/apache/nuttx-apps/blob/master/system/readline/readline_common.c#L644 > > The low-level serial driver should not echo just because /isconsole /is > true. Console echo is always handled by the application. I would say > that that is a bug. > >From the spec: https://pubs.opengroup.org/onlinepubs/7908799/xsh/termios.h.html the serial driver should echo the char if ECHO is enabled. Since NuttX doesn't follow the spec, it makes many POSIX compliant interactive shells doesn't work very well. We have to make the breaking change to fix the wrong initial implementation. > On 3/7/2023 9:45 AM, Sebastien Lorquet wrote: > > Hi, > > > > I am connecting to my board using > > > > python3 -m serial --raw --eol cr /dev/ttyUSB0 115200 > > > > This has ALWAYS worked. > > > > Today it does not work anymore. > > > > Every character I send to NSH is echoed TWICE, see below just typing > > help: > > > > -- > > > > seb@lap:~$ python3 -m serial --raw /dev/ttyUSB0 115200 > > --- Miniterm on /dev/ttyUSB0 115200,8,N,1 --- > > --- Quit: Ctrl+] | Menu: Ctrl+T | Help: Ctrl+T followed by Ctrl+H --- > > hheellpp > > > > help usage: help [-v] [] > > > > -- > > > > Only the echo is double, normal output is OK and characters are only > > inputed once > > > > Miniterm is not doing any echo. This worked fine before. > > > > > > I have found that this commit is the problem > > > > > https://github.com/apache/nuttx/commit/68384e9db42e254b2cf6720bc3380aebdd2b298c > > > > > > Reverting this fixes the dual echo. > > > > > > I dont know what dev->isconsole does but > > > > Just HOW can such a general breakage be missed by the super duper cool > > testing that takes hours to complete? > > > > > > Please STOP breaking NuttX, AGAIN > > > > This is a lot of issues for a single day work. > > > > > > Thank you > > > > Sebatien > > >
Re: Serial console BROKEN
Many people are affected by this double echo https://github.com/apache/nuttx/issues/8731 Sebastien Le 07/03/2023 à 17:09, Gregory Nutt a écrit : I imagine that this is occurring because readline also echos the input: https://github.com/apache/nuttx-apps/blob/master/system/readline/readline_common.c#L644 The low-level serial driver should not echo just because /isconsole /is true. Console echo is always handled by the application. I would say that that is a bug. On 3/7/2023 9:45 AM, Sebastien Lorquet wrote: Hi, I am connecting to my board using python3 -m serial --raw --eol cr /dev/ttyUSB0 115200 This has ALWAYS worked. Today it does not work anymore. Every character I send to NSH is echoed TWICE, see below just typing help: -- seb@lap:~$ python3 -m serial --raw /dev/ttyUSB0 115200 --- Miniterm on /dev/ttyUSB0 115200,8,N,1 --- --- Quit: Ctrl+] | Menu: Ctrl+T | Help: Ctrl+T followed by Ctrl+H --- hheellpp help usage: help [-v] [] -- Only the echo is double, normal output is OK and characters are only inputed once Miniterm is not doing any echo. This worked fine before. I have found that this commit is the problem https://github.com/apache/nuttx/commit/68384e9db42e254b2cf6720bc3380aebdd2b298c Reverting this fixes the dual echo. I dont know what dev->isconsole does but Just HOW can such a general breakage be missed by the super duper cool testing that takes hours to complete? Please STOP breaking NuttX, AGAIN This is a lot of issues for a single day work. Thank you Sebatien
Re: Serial console BROKEN
I imagine that this is occurring because readline also echos the input: https://github.com/apache/nuttx-apps/blob/master/system/readline/readline_common.c#L644 The low-level serial driver should not echo just because /isconsole /is true. Console echo is always handled by the application. I would say that that is a bug. On 3/7/2023 9:45 AM, Sebastien Lorquet wrote: Hi, I am connecting to my board using python3 -m serial --raw --eol cr /dev/ttyUSB0 115200 This has ALWAYS worked. Today it does not work anymore. Every character I send to NSH is echoed TWICE, see below just typing help: -- seb@lap:~$ python3 -m serial --raw /dev/ttyUSB0 115200 --- Miniterm on /dev/ttyUSB0 115200,8,N,1 --- --- Quit: Ctrl+] | Menu: Ctrl+T | Help: Ctrl+T followed by Ctrl+H --- hheellpp help usage: help [-v] [] -- Only the echo is double, normal output is OK and characters are only inputed once Miniterm is not doing any echo. This worked fine before. I have found that this commit is the problem https://github.com/apache/nuttx/commit/68384e9db42e254b2cf6720bc3380aebdd2b298c Reverting this fixes the dual echo. I dont know what dev->isconsole does but Just HOW can such a general breakage be missed by the super duper cool testing that takes hours to complete? Please STOP breaking NuttX, AGAIN This is a lot of issues for a single day work. Thank you Sebatien
Serial console BROKEN
Hi, I am connecting to my board using python3 -m serial --raw --eol cr /dev/ttyUSB0 115200 This has ALWAYS worked. Today it does not work anymore. Every character I send to NSH is echoed TWICE, see below just typing help: -- seb@lap:~$ python3 -m serial --raw /dev/ttyUSB0 115200 --- Miniterm on /dev/ttyUSB0 115200,8,N,1 --- --- Quit: Ctrl+] | Menu: Ctrl+T | Help: Ctrl+T followed by Ctrl+H --- hheellpp help usage: help [-v] [] -- Only the echo is double, normal output is OK and characters are only inputed once Miniterm is not doing any echo. This worked fine before. I have found that this commit is the problem https://github.com/apache/nuttx/commit/68384e9db42e254b2cf6720bc3380aebdd2b298c Reverting this fixes the dual echo. I dont know what dev->isconsole does but Just HOW can such a general breakage be missed by the super duper cool testing that takes hours to complete? Please STOP breaking NuttX, AGAIN This is a lot of issues for a single day work. Thank you Sebatien