Re: 答复: 答复: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver

2023-03-11 Thread Tomek CEDRO
ACK! :-)

--
CeDeROM, SQ7MHZ, http://www.tomek.cedro.info

On Sat, Mar 11, 2023, 23:53 Xiang Xiao  wrote:

> On Sun, Mar 12, 2023 at 2:09 AM Tomek CEDRO  wrote:
>
> > Please consult Unix manual and source code (for instance any of the BSD).
> > Linux was never a reference in Unix world, it was about to mimic Unix,
> but
> > was not even self-compatible. Not a good reference point. See how big
> mess
> > it introduced in current drivers implementation (i.e. drm kms) and their
> > propagation to other OS. Is this similar situation here?
> >
> >
> This change isn't specific to Linux:
> https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/termios.h.html
>
>
> > Device driver should not provide any additional processing / silent
> rewrite
> > of data, just provide raw data, that is then processed by the
> application.
> > Driver may be configured by ioctl/sysclt, to enable additional / specific
> > behavior (i.e. debug, buffer size, latency, etc), so the user always
> knows
> > exactly what the driver does.
> >
> > I am pro Greg approach, so driver is always simple and generic,
> everything
> > else is up to application. This is the Unix way and you will know how
> most
> > things work, because they all work the same generic standardized way, as
> > simple as possible, nothing else happens "in the background unnoticed".
> >
> > If a specific device needs a specific handling by a generic driver, then
> > "quirks" may be used. That explicitly mark "specific" behavior for a
> given
> > device (and only this device).
> >
> >
> All changes are controlled by ioctl and specified by standard, please read
> the spec:
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/tcsetattr.html
> https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/termios.h.html
>
>
> > --
> > CeDeROM, SQ7MHZ, http://www.tomek.cedro.info
> >
>


Re: [Breaking change] Echo character by serial driver

2023-03-11 Thread Nathan Hartman
Generally I like to lean towards low flash footprint. However I accept the
arguments in favor of POSIX compliance. How to find a balance? I think
Xiang Xiao's idea makes sense: always enable the parts that have a minimal
impact on flash footprint, while leaving the heavier parts conditional on a
Kconfig. So we get the best of both worlds.

So yes, we will still have Kconfig and low footprint:-)

Cheers
Nathan

On Sat, Mar 11, 2023 at 12:38 PM Tomek CEDRO  wrote:

> +1 :-)
>
> --
> CeDeROM, SQ7MHZ, http://www.tomek.cedro.info
>
> On Sat, Mar 11, 2023, 12:54 Alin Jerpelea:
>
> > Hi,
> >
> > In my opinion we should keep the memory usage as low as possible
> > +1 for menu config
> >
> > Best Regards
> > Alin
>
>
> >
>


Re: 答复: 答复: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver

2023-03-11 Thread Xiang Xiao
On Sun, Mar 12, 2023 at 2:09 AM Tomek CEDRO  wrote:

> Please consult Unix manual and source code (for instance any of the BSD).
> Linux was never a reference in Unix world, it was about to mimic Unix, but
> was not even self-compatible. Not a good reference point. See how big mess
> it introduced in current drivers implementation (i.e. drm kms) and their
> propagation to other OS. Is this similar situation here?
>
>
This change isn't specific to Linux:
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/termios.h.html


> Device driver should not provide any additional processing / silent rewrite
> of data, just provide raw data, that is then processed by the application.
> Driver may be configured by ioctl/sysclt, to enable additional / specific
> behavior (i.e. debug, buffer size, latency, etc), so the user always knows
> exactly what the driver does.
>
> I am pro Greg approach, so driver is always simple and generic, everything
> else is up to application. This is the Unix way and you will know how most
> things work, because they all work the same generic standardized way, as
> simple as possible, nothing else happens "in the background unnoticed".
>
> If a specific device needs a specific handling by a generic driver, then
> "quirks" may be used. That explicitly mark "specific" behavior for a given
> device (and only this device).
>
>
All changes are controlled by ioctl and specified by standard, please read
the spec:
https://pubs.opengroup.org/onlinepubs/9699919799/functions/tcsetattr.html
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/termios.h.html


> --
> CeDeROM, SQ7MHZ, http://www.tomek.cedro.info
>


Re: 答复: 答复: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver

2023-03-11 Thread Gregory Nutt


It is true that Linux is not fully POSIX compliant (e.g., 
https://linuxhint.com/is_linux_posix_compliant/).  But Linux folk have 
gone to a lot of work to clean up a Linux specification with the LSB: 
https://refspecs.linuxfoundation.org/lsb.shtml and that generally adopts 
POSIX standards.


   We use the the term "Linux" loosely here.  POSIX specifies the
   interface between applications and an operating system.  However,
   the user application never interacts directly with Linux which
   resides in kernel space but rather with various user space libraries
   like libc that, in turn, interface with Linux via call gates
   (uncontrolled and not not prototype-able C interfaces).  So more
   correctly, it is the GNU libraries that determine the user
   application interface. Calling this Gnu/Linux is more accurate but
   that phrase manages to piss off everyone.

NuttX has evolved over the years in regard to what it uses as its 
standards base.


1. Originally, in the beginning, NuttX was a very tiny OS (think 
FreeRTOS or ChibiOS) with a very limited user interface but drawing 
primarily from POSIX interface definitions.  Since NuttX provides both 
the OS and the libc, we can safely call this interface NuttX.


2. Later the project adopted OpenGroup.org as the standards base.  
OpenGroup.org is a good clean POSIX definition and no user interface 
could come in that was not specified by OpenGroup.org.  OpenGroup.org is 
THE Unix spcification.  Documentation (like the Inviolables.md) still 
claim that OpenGroup.org is the standards base.


3. Most NuttX users are also Linux users so there has always been 
pressure to follow Linux interface definitions (the LSB).  After Xiaomi 
adopted NuttX, the pressure to adopt the LSB as the specification base 
(NSB?) was irresistible.


From my point of view, I am just happy that we can point to 
specifications and say, "NuttX is compliant with that" -- whether that 
is OpenGroup.org or the LSB.


I have led technical teams for several decades and one thing I have 
learned is that everyone does thing differently and as a technical 
leader, you have to stand back and simply ask does the propose solution 
work?  Does the proposed solution meet the need?  If yes, then you just 
have to accept the solution and let the rest go as personal preference.


Unless you are the BDFL as I was for so many years.

Also, the TERMIOS changes are POSIX and compliant with the LSB, 
OpenGroup.org and, I am sure, BSD.


A bunch of caveats.
- Use of the term POSIX compliance OS in regard to any OS that is not 
POSIX certified is a trademark violation, but people say this all of the 
time.

- http://get.posixcertified.ieee.org/certification_guide.html
- POSIX is a trademark of the IEEE
- Unix is a trademark of OpenGroup.org
- Linux is a trademark of Linux Torvalds

On 3/11/2023 12:08 PM, Tomek CEDRO wrote:

Please consult Unix manual and source code (for instance any of the BSD).
Linux was never a reference in Unix world, it was about to mimic Unix, but
was not even self-compatible. Not a good reference point. See how big mess
it introduced in current drivers implementation (i.e. drm kms) and their
propagation to other OS. Is this similar situation here?

Device driver should not provide any additional processing / silent rewrite
of data, just provide raw data, that is then processed by the application.
Driver may be configured by ioctl/sysclt, to enable additional / specific
behavior (i.e. debug, buffer size, latency, etc), so the user always knows
exactly what the driver does.

I am pro Greg approach, so driver is always simple and generic, everything
else is up to application. This is the Unix way and you will know how most
things work, because they all work the same generic standardized way, as
simple as possible, nothing else happens "in the background unnoticed".

If a specific device needs a specific handling by a generic driver, then
"quirks" may be used. That explicitly mark "specific" behavior for a given
device (and only this device).

--
CeDeROM, SQ7MHZ,http://www.tomek.cedro.info



Re: 答复: 答复: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver

2023-03-11 Thread Tomek CEDRO
Please consult Unix manual and source code (for instance any of the BSD).
Linux was never a reference in Unix world, it was about to mimic Unix, but
was not even self-compatible. Not a good reference point. See how big mess
it introduced in current drivers implementation (i.e. drm kms) and their
propagation to other OS. Is this similar situation here?

Device driver should not provide any additional processing / silent rewrite
of data, just provide raw data, that is then processed by the application.
Driver may be configured by ioctl/sysclt, to enable additional / specific
behavior (i.e. debug, buffer size, latency, etc), so the user always knows
exactly what the driver does.

I am pro Greg approach, so driver is always simple and generic, everything
else is up to application. This is the Unix way and you will know how most
things work, because they all work the same generic standardized way, as
simple as possible, nothing else happens "in the background unnoticed".

If a specific device needs a specific handling by a generic driver, then
"quirks" may be used. That explicitly mark "specific" behavior for a given
device (and only this device).

--
CeDeROM, SQ7MHZ, http://www.tomek.cedro.info


Re: [Breaking change] Echo character by serial driver

2023-03-11 Thread Tomek CEDRO
+1 :-)

--
CeDeROM, SQ7MHZ, http://www.tomek.cedro.info

On Sat, Mar 11, 2023, 12:54 Alin Jerpelea:

> Hi,
>
> In my opinion we should keep the memory usage as low as possible
> +1 for menu config
>
> Best Regards
> Alin


>


Re: [Breaking change] Echo character by serial driver

2023-03-11 Thread Alan C. Assis
Agree!

+1

On 3/11/23, Alin Jerpelea  wrote:
> Hi,
>
> In my opinion we should keep the memory usage as low as possible
> +1 for menu config
>
> Best Regards
> Alin
>
> On Sat, 11 Mar 2023, 05:13 Xiang Xiao,  wrote:
>
>> On Sat, Mar 11, 2023 at 9:51 AM Gregory Nutt  wrote:
>>
>> >
>> > On 3/10/2023 7:44 PM, Huang Qi wrote:
>> > > >>  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.
>> > >
>> > > > The those Kconfig files should select TERMIOS_SERIAL, right?
>> > >
>> > > Yes, this is a chioce, or we can make TERMIOS enabled forcely, but
>> > > will cause a extra 1KB overhead for all target.
>> > >
>> > That 1Kb number seems large to me.  Most of that is in the lower-half,
>> > UART driver, right?  If so then the size would vary dramatically from
>> > chip-to-chip.
>> >
>> >
>> Terminal setting include two part:
>>
>>1. Hardware related setting(e.g. baud rate, parity check etc)
>>2. Software related setting(e.g. echo, \r\n<->\n etc)
>>
>> The major code size increase comes from the first item, but it's
>> unfortunate that TERMIOS_SERIAL controls both settings.
>> So, here is my suggestion:
>>
>>1. TERMIOS_SERIAL only control the hardware related setting
>>2. The software setting is always enabled
>>3. isconsole decide the initial software setting
>>   - isconsole equals false, disable all special process
>>   - isconsole equals true, enable \r\n<->\n, echo and crtl+c handling
>>4. terminal aware function or application change the terminal to raw
>>mode and restore to the original setting before exit
>>5. other normal application could assume that the terminal do all
>>special process
>>
>> This could achieve POSIX compliance with the minimal cost.
>>
>>
>> >
>> > As a percentage growth, I suppose even 1Kb is not so large.  Probably
>> > less then 2%
>> >
>>
>


Re: [Breaking change] Echo character by serial driver

2023-03-11 Thread Alin Jerpelea
Hi,

In my opinion we should keep the memory usage as low as possible
+1 for menu config

Best Regards
Alin

On Sat, 11 Mar 2023, 05:13 Xiang Xiao,  wrote:

> On Sat, Mar 11, 2023 at 9:51 AM Gregory Nutt  wrote:
>
> >
> > On 3/10/2023 7:44 PM, Huang Qi wrote:
> > > >>  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.
> > >
> > > > The those Kconfig files should select TERMIOS_SERIAL, right?
> > >
> > > Yes, this is a chioce, or we can make TERMIOS enabled forcely, but
> > > will cause a extra 1KB overhead for all target.
> > >
> > That 1Kb number seems large to me.  Most of that is in the lower-half,
> > UART driver, right?  If so then the size would vary dramatically from
> > chip-to-chip.
> >
> >
> Terminal setting include two part:
>
>1. Hardware related setting(e.g. baud rate, parity check etc)
>2. Software related setting(e.g. echo, \r\n<->\n etc)
>
> The major code size increase comes from the first item, but it's
> unfortunate that TERMIOS_SERIAL controls both settings.
> So, here is my suggestion:
>
>1. TERMIOS_SERIAL only control the hardware related setting
>2. The software setting is always enabled
>3. isconsole decide the initial software setting
>   - isconsole equals false, disable all special process
>   - isconsole equals true, enable \r\n<->\n, echo and crtl+c handling
>4. terminal aware function or application change the terminal to raw
>mode and restore to the original setting before exit
>5. other normal application could assume that the terminal do all
>special process
>
> This could achieve POSIX compliance with the minimal cost.
>
>
> >
> > As a percentage growth, I suppose even 1Kb is not so large.  Probably
> > less then 2%
> >
>


Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Xiang Xiao
On Sat, Mar 11, 2023 at 9:51 AM Gregory Nutt  wrote:

>
> On 3/10/2023 7:44 PM, Huang Qi wrote:
> > >>  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.
> >
> > > The those Kconfig files should select TERMIOS_SERIAL, right?
> >
> > Yes, this is a chioce, or we can make TERMIOS enabled forcely, but
> > will cause a extra 1KB overhead for all target.
> >
> That 1Kb number seems large to me.  Most of that is in the lower-half,
> UART driver, right?  If so then the size would vary dramatically from
> chip-to-chip.
>
>
Terminal setting include two part:

   1. Hardware related setting(e.g. baud rate, parity check etc)
   2. Software related setting(e.g. echo, \r\n<->\n etc)

The major code size increase comes from the first item, but it's
unfortunate that TERMIOS_SERIAL controls both settings.
So, here is my suggestion:

   1. TERMIOS_SERIAL only control the hardware related setting
   2. The software setting is always enabled
   3. isconsole decide the initial software setting
  - isconsole equals false, disable all special process
  - isconsole equals true, enable \r\n<->\n, echo and crtl+c handling
   4. terminal aware function or application change the terminal to raw
   mode and restore to the original setting before exit
   5. other normal application could assume that the terminal do all
   special process

This could achieve POSIX compliance with the minimal cost.


>
> As a percentage growth, I suppose even 1Kb is not so large.  Probably
> less then 2%
>


RE: [Breaking change] Echo character by serial driver

2023-03-10 Thread Huang Qi
> That 1Kb number seems large to me.  Most of that is in the lower-half,
> UART driver, right?  If so then the size would vary dramatically from
> chip-to-chip.

I prefer to select it by Kconfig, most morden platforms doesn't make sense for 
1Kb but still many chips in source tree with very small flash size.

BTW, for this thread, I think we can try to optimize the new approach about 
ECHO and fix all issue found.

If it's not possible or not the right way, let's revert the change.

What's your opinion?

On 3/10/2023 7:44 PM, Huang Qi wrote:
> >>  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.
>
> > The those Kconfig files should select TERMIOS_SERIAL, right?
>
> Yes, this is a chioce, or we can make TERMIOS enabled forcely, but
> will cause a extra 1KB overhead for all target.
>
That 1Kb number seems large to me.  Most of that is in the lower-half,
UART driver, right?  If so then the size would vary dramatically from
chip-to-chip.


As a percentage growth, I suppose even 1Kb is not so large.  Probably
less then 2%


Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Gregory Nutt


On 3/10/2023 7:44 PM, Huang Qi wrote:

>>  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.

> The those Kconfig files should select TERMIOS_SERIAL, right?

Yes, this is a chioce, or we can make TERMIOS enabled forcely, but 
will cause a extra 1KB overhead for all target.


That 1Kb number seems large to me.  Most of that is in the lower-half, 
UART driver, right?  If so then the size would vary dramatically from 
chip-to-chip.



As a percentage growth, I suppose even 1Kb is not so large.  Probably 
less then 2%


Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Gregory Nutt


On 3/10/2023 7:44 PM, Huang Qi wrote:

>>  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.

> The those Kconfig files should select TERMIOS_SERIAL, right?

Yes, this is a chioce, or we can make TERMIOS enabled forcely, but 
will cause a extra 1KB overhead for all target.


That 1Kb number seems large to me.  Most of that is in the lower-half, 
UART driver, right?  If so then the size would vary dramatically from 
chip-to-chip.



As a percentage growth, I suppose even 1Kb is not so large.  Probably 
less then 2%


RE: [Breaking change] Echo character by serial driver

2023-03-10 Thread Huang Qi
>>   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.

> The those Kconfig files should select TERMIOS_SERIAL, right?

Yes, this is a chioce, or we can make TERMIOS enabled forcely, but will cause a 
extra 1KB overhead for all target.


发件人: Gregory Nutt 
发送时间: 2023年3月11日 09:33
收件人: Qi Huang 
主题: Re: [Breaking change] Echo character by serial driver


>>   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.
The those Kconfig files should select TERMIOS_SERIAL, right?


Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread 黄 齐
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.



Re: 答复: 答复: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Gregory Nutt

On 3/10/2023 5:30 PM, Qi3 Huang 黄齐 wrote:

I have to agree that the code is looking better all of the time. Many of 
the obvious problems that I saw just a couple of days ago have been 
addressed.  Many of the things I have complained about in this thread 
have been fixed.  But there are probably more issues that will show up.


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.

That is not always true.

 * 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.

   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.

 * Also, the trick of disabling ECHO is not a general solution. You
   cannot disable TERMIOS for asynchronous, out of band input because
   you don't know when that input is going to arrive.

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 the case of readline, we know when the ESC sequence is coming.
   readline() asked for the position and the host responded.  For all
   such synchronous input,  we could control the ECHO with TERMIOS. But
   we cannot for other asynchronous, out-of-band input.


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.


I have never seen that behavior.  That bug is probably introduced recently.


答复: 答复: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Qi3 Huang 黄齐
> 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!**/#


Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Gregory Nutt
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.


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.  New bugs will be showing up for many months due to 
this change.


That is not a good thing to let happen to the OS.



Re: 答复: 答复: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Gregory Nutt

On 3/10/2023 10:05 AM, Qi3 Huang 黄齐 wrote:

Thanks for point that, so my change will affect them by serial.c since it's in 
common logic (excatly, is in uart_read()).

You are right.  But that is not true of telnet, is it?


答复: 答复: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Qi3 Huang 黄齐
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!**/#


Re: 答复: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Gregory Nutt




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.




答复: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Qi3 Huang 黄齐
> 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!**/#


Re: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Gregory Nutt






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.





答复: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Qi3 Huang 黄齐

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!**/#


Re: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Gregory Nutt




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.



Re: 答复: [External Mail]Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Gregory Nutt

On 3/10/2023 9:08 AM, Qi3 Huang 黄齐 wrote:

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.
We do not want to break the capability of any future use either.  It is 
capability that can be used in must ways and must not be compromised.

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.
Let's please resolve the bigger issues with the design before putting 
bandaids all over the code to work around a poorly thought out solution.

3. TERMIOS


In my test now, only termcurse rely on raw mode (maybe another is pdcurse), so 
the must set raw mode for them.
There has been no raw mode in the past.  readline() simulated rawmode by 
reading and writing directly to the serial driver.  That worked because 
the serial driver was basically always in raw mode. So raw mode was 
there in a sense.  You change breaks that behavior.




答复: [External Mail]Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Qi3 Huang 黄齐
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!**/#


Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Gregory Nutt



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.




答复: [External Mail]Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Qi3 Huang 黄齐
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!**/#


Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Gregory Nutt



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.
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.

Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Xiang Xiao
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.
>
>


Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Gregory Nutt


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.



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.



Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Sebastien Lorquet
Definitely a good thing we get the opportunity to talk about these 
changes on this list, then.


We need technical overview from several people to determine the best way 
to do things.


Sebastien

Le 10/03/2023 à 14:49, Gregory Nutt a écrit :



Thank you for this good change for better posix compliance.


The TERMIOS changes are basically good and does help with POSIX 
compliance, but the console ECHO behavior is not and has nothing to do 
with POSIX compliance.  POSIX does not deal with implementation.  We 
can look at standard implementations for guidance, however:


Standard implementations of readline place the console in raw mode in 
which any echo from the driver is disabled (ECHO is not selected in 
the TERMIOS).  All echo is preformed selectively by the readline 
implementation without trashing the users terminal.





Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Xiang Xiao
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.
Three option here:

   1. Remove SERIAL_TERMIOS and enable terminal setting always
   2. readline or special tool designed for raw mode select SERIAL_TERMIOS
   in their config
   3. Keep SERIAL_TERMIOS option, but simulate terminal setting by change
   isconsole flag

Both code size and change is small for option 3.

On Fri, Mar 10, 2023 at 9:52 PM Gregory Nutt  wrote:

>
> > Thank you for this good change for better posix compliance.
>
> The TERMIOS changes are basically good and does help with POSIX
> compliance, but the console ECHO behavior is not and has nothing to do
> with POSIX compliance.  POSIX does not deal with implementation.  We can
> look at standard implementations for guidance, however:
>
> Standard implementations of readline place the console in raw mode in
> which any echo from the driver is disabled (ECHO is not selected in the
> TERMIOS).  All echo is preformed selectively by the readline
> implementation without trashing the users terminal.
>
>
>


Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Gregory Nutt




Thank you for this good change for better posix compliance.


The TERMIOS changes are basically good and does help with POSIX 
compliance, but the console ECHO behavior is not and has nothing to do 
with POSIX compliance.  POSIX does not deal with implementation.  We can 
look at standard implementations for guidance, however:


Standard implementations of readline place the console in raw mode in 
which any echo from the driver is disabled (ECHO is not selected in the 
TERMIOS).  All echo is preformed selectively by the readline 
implementation without trashing the users terminal.





Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Gregory Nutt



How do you plan to handle incoming vt100 escape sequences.  I see you 
kludged around one, but there are hundreds.  TheY are used in BAS, 
pdcurses and other apps extensively.


You are ruining that critical vt100 capability and breaking a lot of 
things


Has anyone done any serious CLE editing yet.  The entire editor is 
probably broken because all edit commands will be returned to the host.


There are many other cases that need to accept input on the console 
without echoing.  For example, serial file transfer protocols which will 
dump huge files over the serial console.


You are breaking meaning years of hard work, usage, and testing for 
nothing.  At least nothing more than you ego and opinion about how it 
should world.


Standard readline implementations run in raw mode with character echo 
disabled.




RE: [Breaking change] Echo character by serial driver

2023-03-10 Thread spudaneco
How do you plan to handle incoming vt100 escape sequences.  I see you kludged 
around one, but there are hundreds.  TheY are used in BAS, pdcurses and other 
apps extensively.You are ruinking that critical vt100 capability and breaking a 
lot of thingsSent from my Galaxy
 Original message From: Qi3 Huang 黄齐 
 Date: 3/10/23  3:43 AM  (GMT-06:00) To: 
dev@nuttx.apache.org Subject: [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!**/#

Re: [Breaking change] Echo character by serial driver

2023-03-10 Thread Sebastien Lorquet

Hello,

Thank you for this good change for better posix compliance.

In my setup nsh is used via a stm32h7 serial port.

I have my updated custom apps with your changes and everything works as 
expected. I am using readline.


For better coverage I also tested CLE (with history) with NSH and did 
not notice problems excepta very minor problem. I describe it in the 
other thread.


I did not really know about CLE and your message made me discover it. I 
will use CLE now. Thank you.


Best regards,

Sebastien


Le 10/03/2023 à 10:43, Qi3 Huang 黄齐 a écrit :

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!**/#