Serial console

[bwucke]

Serial console added
Serial debug now behind a configuration key

[suchmememanyskill]

Cyb

[bwucke]

Sorry, fixed. I keep thinking it's a Cheap Yellow Board...

[suchmememanyskill]

Hm, CI fails as there's too little space available on the 3.5 inch model. Might be worth removing a feature like the temp graph

[bwucke]

Reduced the input buffer from 255 to 80. Not sure if removing features would do anything; it's a specific piece of RAM that ran out. The static char[] could be replaced with one-shot malloc() if more RAM is needed elsewhere.

[suchmememanyskill]

The issue with the 3.5" model is that the entire screen buffer is statically allocated. Which eats like most of the (pre-allocated) memory :(

[bwucke]

It would take an extremely long ssid and password to fail, the rest of inputs is fixed size. It's safe against buffer overruns, it just quietly discards lines that are too long, no risk of crash. I thought about bumping the buffer to 105 = 32+64+9 (char wifi_SSID[33]; char wifi_password[65]; plus command, whitespaces, \r\n and NULL) but it was just after I committed the 80, feel free to up it; the memory overrun was by ~63 bytes so 255-63=192 if we wanted to squeeze every last byte, otherwise with buffer of 80 we still have 112 bytes for future use.

Also ofc. version bump is on your side. And any criticisms of the code, like my approach with temporary_config and stuff.

Note: LOG_F (the switchable printf) has a funny syntax, args go into double brackets, LOG_F(("c = %s",name)); - that's an easy way to get a macro that accepts optional varargs, getting a macro that tolerates both zero optional arguments and non-zero takes some crazy jumping through hoops, the (()) is an easy workaround that turns the entire thing in the brackets, brackets included into a single argument.

[suchmememanyskill]

@bwucke Testing this PR, i am experiencing some weirdness.

These numbers are supposed to be the message for how much heap space the parser had, but the message itself is missing, just the number shows. Could you look into this?

[suchmememanyskill]

Also, this will go wrong when a release happens. REPO_DEVELOPMENT is undefined if it's a release build

[bwucke]

Done.

    #if defined REPO_DEVELOPMENT  &&  REPO_DEVELOPMENT == 1
        temporary_config.debug = true;
    #else
        temporary_config.debug = false;
    #endif

[bwucke]

Somehow during search&replace a bunch of Serial.printf got swapped for LOG_LN (Serial.println wrapper) instead of LOG_F while retaining the ((...)) syntax of LOG_F
Due to the ((...)) syntax, the inner paren was interpreted as comma expression, ("A=%d B=%d\n",A,B) evaluating to B which Serial.println() was perfectly happy with.

[suchmememanyskill]

@bwucke Got another chance at testing this today, but i'm noticing something very strange. After flashing your PR, my CYD-Klipper has trouble keeping a connection to the printer (Error codes HTTPC_ERROR_CONNECTION_REFUSED and HTTPC_ERROR_READ_TIMEOUT) (https://github.com/espressif/arduino-esp32/blob/master/libraries/HTTPClient/src/HTTPClient.h#L46)

Left is your branch, right is the latest dev branch. (Also you use the main core that also renders ui, why does this affect the 2nd core pulling data...)

Can you also repro this?

[bwucke]

I'm experiencing the same problems both on my serial_console branch, and on main, without serial_console.

They occur only when I'm connected to my company's 'main' wifi network, which it seems no esp32 in existence likes, not just my CYD. I'm not sure what's the reason - 5GHz, multiple access-points, a hyphen in SSID name?

I observe none of the problems when connecting to an alternate network, 802.11g, one ap, simple name.

Tested on the old version of esp32-2432S028R (only MicroUSB) and new (USB C and MicroUSB), serial on both ports, all exhibit the same behavior, both v1.6.4 (#113, e2c2a38 ) master/origin and my serial_console hate the 5g network something fierce, and work flawlessly on the 802.11g

As for cores - I didn't pay attention to these and didn't check what runs where. I just identified most busy-wait loops during boot-up and runtime and inserted serial_console::run(); into each (with exception of OTA, don't want to be able to interrupt that!) without paying attention what context they are called from, so if I messed up, sorry, dunno how to fix.

Attaching one of more 'violent' misbehaviors of master/origin I've observed with the 5g network.
log.txt

[suchmememanyskill]

That is very strange indeed.
I wonder why when i'm on the dev branch (19cfaef) i can't repro it, across multiple reboots.
When i switch to your branch it instantly starts acting weird. Also presists across multiple reboots.
(I wonder if it's something weird like the ui core blocking the requests core for too long)

Do ESP32's even support 5ghz? Wouldn't it always drop down to 2.4?

[bwucke]

This might be power issue. I tried connecting the dual-port CYD using both ports simultaneously, USB C to 2000mA power supply port, MiniUSB to PC, and the issues got massively diminished, most of the time it connects to the 5g network just fine, occasionally has some small hiccups, one http retry, etc.

[bwucke]

Do ESP32's even support 5ghz? Wouldn't it always drop down to 2.4?

I don't know, but don't think so. Some of the access points are 2.4, one nearest is 5. I can't tell which one it connects to.

[bwucke]

I've just pulled 19cfaef, compiled, installed, and it's the same power-dependent misbehavior with the 'problem' network. And same no-problem with the 'nice' network.

[bwucke]

...what I'm considering baffling is that the serial console is extremely lightweight if there are no user inputs.

The code goes like this:

    void run()
    {
        static char cmdline[MAX_COMDLINE_SIZE + 1] = {0};
        if (!read_string_until('\n', cmdline, MAX_COMDLINE_SIZE))
            return;
     ....
    }
    
     bool read_string_until(char delimiter, char *result, int max_len)
    {
        static int index = 0;
        int c;         // read character, -1 if none
        int cnt = 100; // limit on amount of iterations in one go; we're supposed to be non-blocking!

        while ((c = Serial.read()) != -1 && cnt > 0)
        {
        ...
        }
        return false; // still waiting for delimeter
    }

The (c = Serial.read()) != -1 will be false. read_string_until returns false. run() returns. There's literally two function calls, one assignment and one comparison. Unless Serial.read() itself is heavy. It's a buffered read, but sits behind a UART_MUTEX_LOCK(); which is locked on nearly every serial() function.

I could try to utilize Serial.available() but it still uses UART_MUTEX_LOCK so I don't have high hopes.

Another thing is I could reduce the frequency at which serial_console::run() is called. Either move it to some free timer interrupt, or use millis() or some call counter to proceed to Serial.read() every n-th call. 10 calls per second would be a plenty, even 1 per second would be acceptable (uart has 64-byte buffer, and my read_string_until snarfs up to 100 characters per call if they are in the buffer.) The current execution once per main loop iteration is definite overkill, done only because I thought it's so lightweight any limiters would make it heavier.

[bwucke]

Could you put return; on the first line of serial_console::run() and see if the problem still occurs? 'cause if that fixes it, decreasing frequency of calls of it should solve the issue, but if not, it's somewhere else...

[suchmememanyskill]

Couldn't repro this today at all, wth.
Well, probably is just a me issue.

Thanks a bunch, sorry for the long wait