[BUG] NSH outputting garbage

[fdcavalcanti]

Description / Steps to reproduce the issue

Hello folks!

Seems some changes on NSH have absolutely obliterated Espressif's internal CI.
I have traced the issue using git bisect to the following commit:

• 893f0c31eed10adf58ca95b52fffed56f89e43c5 (readline: remove 5x duplicated in redraw code)
• PR apps/nsh: Add support to command line editing on NSH #3657

The issue is that pytest-embedded-nuttx is parsing the command line and fails to match the regex for the echo $? command.
Analyzing the serial output as received by the tool we have this:

• Issues commands:

  • ls /dev
  • echo $?

• Before the changes

nsh> #x1B[Kls /dev
/dev:
 console
 null
 random
 ttyS0
 zero
nsh> #x1B[K
nsh> #x1B[Kecho $?
0
nsh> #x1B[K

• After the changes:

nsh> #x1B[Kl#x1B[Ks#x1B[K #x1B[K/#x1B[Kd#x1B[Ke#x1B[Kv#x1B[K
/dev:
 console
 null
 random
 ttyS0
 zero
nsh> #x1B[K
nsh> #x1B[Ke#x1B[Kc#x1B[Kh#x1B[Ko#x1B[K #x1B[K$#x1B[K?#x1B[K
0
nsh> #x1B[K

I´m not aware if those are ANSI or UTF-something escape values, but seems there is a symbol repeating before each character that is passed to the serial output (#x1B[K).

Please if someone can check and confirm if that is expected. If so, how should we parse it?

Thanks in advance.

On which OS does this issue occur?

[OS: Linux]

What is the version of your OS?

Mint 22

NuttX Version

master

Issue Architecture

[Arch: risc-v]

Issue Area

[Area: NSH]

Host information

Board used is esp32c3-devkit:nsh.

Verification

• I have verified before submitting the report.

[acassis]

@fdcavalcanti probably it is happening because you are not using a terminal that supports VT102/VT220 ANSI escape sequence. Before this patch the serial driver was blocking most of the VT102 and VT220 byte sequences. This is exactly to avoid the serial driver consuming/blocking it blindly.

I don't remember all the details, but I think pytest uses pyserial, so you need to filter out these sequences:
https://www.google.com/search?q=how+to+filter+out+VT102%2FVT220+ANSI+escape+sequence+on+pyserial%3F

[fdcavalcanti]

@fdcavalcanti probably it is happening because you are not using a terminal that supports VT102/VT220 ANSI escape sequence. Before this patch the serial driver was blocking most of the VT102 and VT220 byte sequences. This is exactly to avoid the serial driver consuming/blocking it blindly.

I don't remember all the details, but I think pytest uses pyserial, so you need to filter out these sequences: https://www.google.com/search?q=how+to+filter+out+VT102%2FVT220+ANSI+escape+sequence+on+pyserial%3F

I think the feature is great. But should it be like that for every character typed even without using arrows, tab, etc?
I'm surprised no one else has had this problem as it is a clear issue when parsing, at least on Python.

#x1B[K is an ANSI escape code (specifically the "Erase in Line" command) combined with a literal hash symbol,
used to clear text from the cursor position to the end of a terminal line.

[acassis]

@fdcavalcanti probably it is happening because you are not using a terminal that supports VT102/VT220 ANSI escape sequence. Before this patch the serial driver was blocking most of the VT102 and VT220 byte sequences. This is exactly to avoid the serial driver consuming/blocking it blindly.
I don't remember all the details, but I think pytest uses pyserial, so you need to filter out these sequences: https://www.google.com/search?q=how+to+filter+out+VT102%2FVT220+ANSI+escape+sequence+on+pyserial%3F

I think the feature is great. But should it be like that for every character typed even without using arrows, tab, etc? I'm surprised no one else has had this problem as it is a clear issue when parsing, at least on Python.

Exactly, this is the standardized way to pass these special keys. And you are right (just change the words order, hehehe) : "that is a clear Python issue"

There is a Python library just to fix it:

[fdcavalcanti]

@acassis there is some confusion going on.

You showed an example of parsing raw input data. That is not my case, as the issue is in the data echoed back.

Look at the example where my test simply does serial.write('ls /dev'):

• Before:
  nsh> #x1B[Kls /dev
• After:
  nsh> #x1B[Kl#x1B[Ks#x1B[K #x1B[K/#x1B[Kd#x1B[Ke#x1B[Kv#x1B[K

So please check if this understanding is right:

• You are saying #x1B[K should be before every single character
• VT102 is not UTF-8 compatible
• Your suggested approach is to fix on Python side?
• Google says No, \x1b[1K (or #x1B[Kl) should not appear before every character. It is an ANSI/VT102 control sequence meant to clear a line, and repeating it constantly is redundant and indicates a bug in your output stream or logging system. Is that incorrect?

Thanks for your help, I'm trying to get CI back up as soon as possible!

[acassis]

Hi @fdcavalcanti understood, so the issue is because now the escape code is echoed before each character sent. Yes, that is wrong.

I think I didn't see this issue because the serial console handles it correctly. Please try this fix and let me know if it works for you:

diff --git a/system/readline/readline_common.c b/system/readline/readline_common.c
index 1642e035e..3d0f570d7 100644
--- a/system/readline/readline_common.c
+++ b/system/readline/readline_common.c
@@ -1598,7 +1598,10 @@ ssize_t readline_common(FAR struct rl_common_s *vtbl, FAR char *buf,
 
               buf[cursor] = (char)ch; nch++; cursor++;
 #  ifdef CONFIG_READLINE_ECHO
-              redraw_tail(vtbl, buf, nch, cursor);
+              if (cursor < nch)
+                {
+                  redraw_tail(vtbl, buf, nch, cursor);
+		}
 #  endif
             }
 #else
-- 
2.43.0

Also as you can see "Before: nsh> #x1B[Kls /dev", the you already had that escape printed. I suggest you to filter it out:

   ANSI_CSI = re.compile(rb'\x1b\[[0-?]*[ -/]*[@-~]')
   clean = ANSI_CSI.sub(b'', raw)

[fdcavalcanti]

Hi @fdcavalcanti understood, so the issue is because now the escape code is echoed before each character sent. Yes, that is wrong.

I think I didn't see this issue because the serial console handles it correctly. Please try this fix and let me know if it works for you:

diff --git a/system/readline/readline_common.c b/system/readline/readline_common.c
index 1642e035e..3d0f570d7 100644
--- a/system/readline/readline_common.c
+++ b/system/readline/readline_common.c
@@ -1598,7 +1598,10 @@ ssize_t readline_common(FAR struct rl_common_s *vtbl, FAR char *buf,
 
               buf[cursor] = (char)ch; nch++; cursor++;
 #  ifdef CONFIG_READLINE_ECHO
-              redraw_tail(vtbl, buf, nch, cursor);
+              if (cursor < nch)
+                {
+                  redraw_tail(vtbl, buf, nch, cursor);
+		}
 #  endif
             }
 #else
-- 
2.43.0

Also as you can see "Before: nsh> #x1B[Kls /dev", the you already had that escape printed. I suggest you to filter it out:

   ANSI_CSI = re.compile(rb'\x1b\[[0-?]*[ -/]*[@-~]')
   clean = ANSI_CSI.sub(b'', raw)

Hi @acassis, unfortunately it did not work. Seems the escape sequence is still showing up before each character:

total segments stored 7

NuttShell (NSH) NuttX-10.4.0
nsh> #x1B[K2026-07-31 16:34:06,400 - conftest - INFO - Ready
l#x1B[Ks#x1B[K #x1B[K/#x1B[Kd#x1B[Ke#x1B[Kv#x1B[K
/dev:
 console
 null
 random
 ttyS0
 zero
nsh> #x1B[Ke#x1B[Kc#x1B[Kh#x1B[Ko#x1B[K #x1B[K$#x1B[K?#x1B[K
0
nsh> #x1B[K