You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nuttx.apache.org by Sebastien Lorquet <se...@lorquet.fr> on 2023/03/07 15:45:22 UTC

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] [<cmd>]

----------------------------------

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

Posted by Sebastien Lorquet <se...@lorquet.fr>.
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] [<cmd>]
>>
>> ----------------------------------
>>
>> 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

Posted by Sebastien Lorquet <se...@lorquet.fr>.
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 <sp...@gmail.com> 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

Posted by Tomek CEDRO <to...@cedro.info>.
On Wed, Mar 8, 2023 at 11:14 PM Nathan Hartman wrote:
> On Wed, Mar 8, 2023 at 4:02 PM Sebastien Lorquet <se...@lorquet.fr>
> 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

Posted by Tomek CEDRO <to...@cedro.info>.
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&chfieldfrom=24h&list_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

NuttX Hardware

Posted by Sebastien Lorquet <se...@lorquet.fr>.
This needs to be in its own thread.

Your collection of boards probably got some historical value.

Sebastien

On 3/8/23 23:31, Gregory Nutt wrote:
> 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

Posted by Tomek CEDRO <to...@cedro.info>.
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

Posted by Nathan Hartman <ha...@gmail.com>.
On Wed, Mar 8, 2023 at 5:31 PM Gregory Nutt <sp...@gmail.com> 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

Posted by Gregory Nutt <sp...@gmail.com>.
>> 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

Posted by Nathan Hartman <ha...@gmail.com>.
On Wed, Mar 8, 2023 at 4:02 PM Sebastien Lorquet <se...@lorquet.fr>
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

Posted by Gregory Nutt <sp...@gmail.com>.
> 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

Posted by Sebastien Lorquet <se...@lorquet.fr>.
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 <sp...@gmail.com> 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

Posted by Gregory Nutt <sp...@gmail.com>.
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 <sp...@gmail.com> 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

Posted by Nathan Hartman <ha...@gmail.com>.
On Wed, Mar 8, 2023 at 3:20 PM Gregory Nutt <sp...@gmail.com> 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

Posted by Gregory Nutt <sp...@gmail.com>.
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 <xi...@gmail.com> 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

Posted by Nathan Hartman <ha...@gmail.com>.
On Wed, Mar 8, 2023 at 2:26 PM Xiang Xiao <xi...@gmail.com> 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

Posted by Gregory Nutt <sp...@gmail.com>.
>> 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

Posted by Xiang Xiao <xi...@gmail.com>.
On Thu, Mar 9, 2023 at 3:07 AM Gregory Nutt <sp...@gmail.com> 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

Posted by Gregory Nutt <sp...@gmail.com>.
>>>> 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

Posted by Xiang Xiao <xi...@gmail.com>.
On Thu, Mar 9, 2023 at 2:17 AM Gregory Nutt <sp...@gmail.com> 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

Posted by Gregory Nutt <sp...@gmail.com>.
>> 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

Posted by Xiang Xiao <xi...@gmail.com>.
On Wed, Mar 8, 2023 at 12:09 AM Gregory Nutt <sp...@gmail.com> 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] [<cmd>]
> >
> > ----------------------------------
> >
> > 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

Posted by Gregory Nutt <sp...@gmail.com>.
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] [<cmd>]
>
> ----------------------------------
>
> 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
>