Fixes for issues in #3648

[jmattsson]

Fixes #3648.

• [x] This PR is for the dev-esp32 branch rather than for the release branch.
• [x] This PR is compliant with the other contributing guidelines as well (if not, please describe why).
• [x] I have thoroughly tested my contribution.
• [x] The code changes are reflected in the documentation at docs/*.

It was pointed out that the switch to use the IDF's stdin inadvertently broke some IDE's due to different CR/LF handling, as well as breaking uart.on('data') handling on the console uart. This PR addresses both. The first by changing the default IDF CR/LF handling to do no translation which should match what we used to do, and the second by refactoring the uart.on('data') handling and hooking it into the new console task in addition to the uart task. In the future, we should probably consider adding a console.on('data') module/function to allow this sort of functionality also on models which use CDC or USB-Serial-JTAG consoles (at which point the uart.on('data') should just be a forward to that, for the console uart).

Additionally I've added node.chipmodel() to allow querying the model of ESP32 that NodeMCU is running on.

I've tested this on ESP32-C3 as that was what I had on my desk at the moment, and as far as I can tell it now works correctly. CR/LF handling is madly confusing with local terminal line discipline, ttyUSB line discipline, and ESP internal CR/LF translation all acting together and causing confusion.

@serg3295 could you please take this for a spin? I do not have VS Code to test with, and I can't find an ESPlorer version that supports io.open() instead of the old file.open() to test file uploads there.

[serg3295]

Thanks for the PR!

a uart.on() callback handler works now. But uart.start()/uart.stop() are mandatory. The test code that works:

uart.start(0)
uart.on(0,"data",3,function(d)
  uart.write(0,"test\n")
  uart.on(0,"data")
  uart.stop(0)
  end, 0)
uart.write(0,"Ready\n")

ESPlorer is not working. I guess, because it doesn't send uart.start|stop.

I will try to figure out the CR/LF handling.

[serg3295]

I fixed nodemcu-tools (VSCode extension). Now it works with this PR. nodemcu-tool (CLI utility) also works for both ESP8266 and ESP32.

However, the issue of determining the architecture of the esp chip remained unresolved when launching the nodemcu-tool and nodemcu-tools applications. This is required for further correct operation of applications.

The following functions can be used to determine:

function	esp8266	esp32	esp32xx
node.info()	+	-	-
node.chipid()	+	+	-
node.chipmodel()	-	+	+

From the table above, it can be seen that none of the functions currently allows you to determine all possible types of chips at once with one command.

If node.chipid() will return string "0x_unknown" for esp32xx chips (PR #3651), then Lua will not throw an error. Handling this exception is a non-trivial task in vscode-nodemcu-tools due to the complexity of the internal protocol. This is much easier to do in nodemcu-tool.

What's wrong with returning such a string? We are not deceiving the user. The hexadecimal digit is really unknown. :-)

ChipID: 0x_unknown will be output instead of ChipID: 0x837c81111119

❯ ./nodemcu-tool fsinfo
[NodeMCU-Tool]~ Connected 
[device]      ~ Arch: esp32 | Version: unknown | ChipID: 0x837c81111119 | FlashID: 0x_unknown 
[device]      ~ Free Disk Space: 403 KB | Total: 404 KB | 1 Files 
[device]      ~ Files stored into Flash (SPIFFS) 
[device]      ~  - wifi32start.lua (321 Bytes) 
[NodeMCU-Tool]~ disconnecting 

The new node.chipmodel() function will allow a more accurate display of the esp 32 model after the architecture is defined.

[marcelstoer]

@serg3295 Thanks for testing these combinations! Would you maybe have the bandwidth to address #2759? It would allow us to add a few more '+' to your table above 😄

[serg3295]

@serg3295 Thanks for testing these combinations! Would you maybe have the bandwidth to address #2759? It would allow us to add a few more '+' to your table above 😄

I'll try, but I don't know C very well. I'm afraid that my code will need to be heavily corrected 😄

[serg3295]

another problem. I send binary file to ESP with the contents

ESP replaces all 0D with 0A. I think it's a bug.

resulting file on SPIFFS

The code of receiver:

__f = io.open("crlf.bin", "w")
uart.start(0)
uart.on("data", 10, function(d)
  __f:write(d)
  uart.on("data")
  __f:close()
  __f = nil
  uart.stop(0)
end, 0)

[serg3295]

I suggest revert commit to default

• Line ending for UART output (CRLF)
• Line ending for UART input (CR)

because some log messages will be displayed more correctly in IDEs.

All IDEs works fine with these CRLF/CR settings.

[jmattsson]

@serg3295 Is there something wrong with using e.g. local model = node.chipmodel and node.chipmodel() or node.chipid() to avoid errors being thrown?

As for the CR+LF to LF issue, that is (part of) what this PR is intended to fix. As I just mentioned over on the issue itself, you may not have gotten the fix activated as I only updated sdkconfig.defaults. If you do make menuconfig and then go into Component config > Newlib and ensure that at the very least the input setting is set to LF only, you should not suffer this translation issue.

Having the output direction also configured to LF fixes the issue with a double-prompt which has been present ever since we switched to using the IDF's console driver. I wasn't aware there were logging issues with fixes that. Do you have more detailed information please?

[jmattsson]

Also, uart.start() used to be mandatory before too, that much has not changed. I also have conflicting advice on ESPlorer now - over on the issue report a have a success report with this PR.

[serg3295]

If you do make menuconfig and then go into Component config > Newlib and ensure that at the very least the input setting is set to LF only, you should not suffer this translation issue.

I have tried both cr lf/cr and lf/lf. In any case, 10 is returned.

I run the test code in the terminal:

uart.start(0)uart.on("data",3,function(d)for i=1,#d do ch=tostring(d:sub(i,i))print('ch:',string.byte(ch))end uart.on("data")uart.stop(0)end,0)

then type 111

I ran the program in debugger. In uart_feed_data function

 need_len: 3
 buf: 49 //'1'

another test case: type <cr><cr><cr>

need_len: 3
 buf: 10 

I wasn't aware there were logging issues with fixes that. Do you have more detailed information please?

There is no problem. It's just that the message is displayed more correctly. pay attention to the line: main_task: Started...

crlf_cr:

lf_lf

[jmattsson]

Huh. That's odd. I'll see if I can get some more time to do a deep-dive into the IDF internals then and figure out what's going on...

[serg3295]

Is there something wrong with using e.g. local model = node.chipmodel and node.chipmodel() or node.chipid() to avoid errors being thrown?

You are absolutely right. I don't even understand why this obvious option didn't occur to me :-/

[serg3295]

I've made small changes.

diff --git a/components/base_nodemcu/user_main.c b/components/base_nodemcu/user_main.c
index c72cc654..60ddc0bb 100644
--- a/components/base_nodemcu/user_main.c
+++ b/components/base_nodemcu/user_main.c
@@ -214,7 +214,7 @@ static void console_init(void)
   /* Based on console/advanced example */

   esp_vfs_dev_uart_port_set_rx_line_endings(
-    CONFIG_ESP_CONSOLE_UART_NUM, ESP_LINE_ENDINGS_CR);
+    CONFIG_ESP_CONSOLE_UART_NUM, ESP_LINE_ENDINGS_LF);
   esp_vfs_dev_uart_port_set_tx_line_endings(
     CONFIG_ESP_CONSOLE_UART_NUM, ESP_LINE_ENDINGS_CRLF);

Now the applications are working fine.

[jmattsson]

@serg3295 Oh gosh, those should be using the correct macros from Kconfig, not be hard-coded! Cut-n-paste fail on my behalf, good catch, will fix!

[serg3295]

@jmattsson I fixed nodemcu-tool and nodemcu-tools and tested them with settings: rx: ESP_LINE_ENDINGS_LF tx: ESP_LINE_ENDINGS_CRLF

I also tested ESPlorer a bit. Everything works. I believe that setting for tx should stay crlf.

EDIT. Hmm. Esplorer works with tx: ESP_LINE_ENDINGS_LF

I will try to fix nodemcu-tool(s) to get only LF and not expect CRLF, if it needs to be done for any reason.

[jmattsson]

@serg3295 Okay, I've made the necessary changes to actually honour the Kconfig settings. Let me know if that looks good on your end, please?

[serg3295]

I have corrected the code of nodemcu-tool/nodemcu-tools so that they also work with the LF/LF settings. The changes for the Kconfig settings also work for me.

[jmattsson]

@marcelstoer since it looks like Philip is busy, and there's been external review and feedback and fixing, any objections to me merging this?