Arduino 3 / ESP-IDF 5 compatibility

[mathieucarbou]

This PR updates some project files (CI and library.json) to ensure compatibility with Arduino 3 / ESP-IDF 5.

The project itself does not require any change (I tested the benchmark example on ESP32S3 with Arduino 3 RC1), except the library.json for PlatformIO projects, but these dependencies must be used, which have been updated to support Arduino 3 / ESP-IDF 5:

• https://github.com/mathieucarbou/AsyncTCP
• https://github.com/mathieucarbou/ESPAsyncWebServer

Example:

lib_deps = 
  mathieucarbou/Async TCP @ ^3.0.2
  mathieucarbou/ESP Async WebServer @ 2.9.3

You can see the PR build in my fork with the new CI files: https://github.com/mathieucarbou/ayushsharma82-ESP-DASH/actions/runs/8774146477

[ayushsharma82]

@mathieucarbou , Thanks, I'll merge this PR soon.

I'm re-working WebSerial which might delay some things. It's getting a complete makeover 😄.

[mathieucarbou]

@mathieucarbou , Thanks, I'll merge this PR soon.

I'm re-working WebSerial which might delay some things. It's getting a complete makeover 😄.

FYI the dependencies must be updated since - I will refresh have refreshed the PR.

Good to know for WebSerial !
I am using an equivalent one on 2 big projets so if you want me to include it to show case and test for bugs let me know :-)

I added some things like logo, local command history with arrow up & down, etc

[ayushsharma82]

@mathieucarbou Sure, your feedback will be helpful. I'll send it to you this week.

Local command history sounds like a wonderful idea! I'll see if it can be implemented after roll out.

[ayushsharma82]

Here's a sneak peek:

[mathieucarbou]

I've updated the PR again to account for the new Arduino v3 rc2, and espressif platform 6.7.0

[mathieucarbou]

Switched to 3.0.0-rc3.

FYI Arduino 3 is supposed to be released next week if everything is ok, but I doubt it because with someone else we found a serious bug (wifi not working on a lot of boards) so it might be delayed.

In any case, I don't you might want to wait for Arduino 3 release before merging it... Should be soon.

[ayushsharma82]

I'll be merging it in a 2-3 days - no time to test myself.

Btw, WebSerial V2 release candidate is in v2 branch: https://github.com/ayushsharma82/WebSerial/tree/v2

[mathieucarbou]

Ok! I've looked a little at the source code and I have a few comments: - you could provide a callback with a String (or char*) that would be easier to process because anyway the only possible thing the WS callback would receive is a command typedef std::function<void(AsyncWebSocketClient* client, const String& msg)> RecvMsgHandler; - introducing the need to call loop() => it can be avoided (with 2 points below) - WS client cleanup could be called on the write(buffer) or before calling the ws api to send message (this is a quick call, and if you loop at the cleanup code, it does not loop but just try to find one to remove and close, not all if there are more than 1 client than the max). - I am really wondering about the double buffering mechanism (with a mutex)... This adds up on heap memory to queue some data that are just delayed to go and be copied again in the WS message queue (sending a WS message does not send it but enqueues it). Sending WS messages is cheap (since put in a queue). What is important is to avoid sending one message per character (write(int)) or per small buffer. The only buffering that makes sense I think would be to buffer the incoming writes to wait for a EOL (\n) to arrive, and then, flush it. I have already tried such a design but it performs badly and slows down the code if put in WebSerial because of the mutex at a central place, especially on dual cores. Worse, it does not solve the issue of 2 tasks on 2 different cores logging sending data at the same time: core 0 could write, pause, then core 1 write, pause, and then core 0 finish his write. So what I resorted to is to instead put that responsibility to the user. I.e. in my apps, I use a logging library (MycilaLogger) which supports forwarding the logging line to several Print streams. So I am buffering the log line in this library, and print it both in Serial and WebSerial. This way, WebSerial only receives full lines that can be sent to the WS queue directly and it does not exhaust the WS queeu size. There is also no concurrency issue and no shared access between 2 cores. But doing this buffering in the WebSerial lib is more complex and has performance lost because of the fact that you need to manage a central buffer, with a mutex, and also consider that even if you have a mutex, you can have code form 2 different cores that are calling write() at different time. So it does not solve the concurrency problem of 2 messages sent from 2 cores. So I think this concurrency problem just needs to be explained, but not handled within WebSerial because this is too deep in the code. It better be handled on the caller side. Mathieu.

[ayushsharma82]

you could provide a callback with a String (or char*) that would be
easier to process because anyway the only possible thing the WS callback
would receive is a command

Yeah, I can add it as another callback.

I am really wondering about the double buffering mechanism

Really didn't want to use double buffering mechanism too but the main focus of this release was to make it stable somehow, It ultimately came to an over-engineered approach but it is stable afaik. If you know of any other approach, we can still revamp it.

The Print Problem

Even if we are using println or printf functions - we are actually getting single byte write commands in the library. It was not sending the whole log line in one go, this made it necessary to buffer these into chunks.

The very first version of WebSerial had no buffering which caused a lot of problems. You know why - because people tend to use a lot of print functions and the WS queue is overflown quickly.

Dual Buffer Roles

Role of Print Buffer:

Buffers the incoming single byte writes into a whole message packet and regularly flushes the received data into global buffer by marking it a as a 'row'. ( This is quite essential in WebSerial Pro because timestamps are tagged now ). It also lets us free on depending for a \n new line character to close the row.

Role of Global Buffer:

The only reason for using a global buffer was 'performance' and WS queue limitation. ESP really sucks in sending bursts of data via Websockets, therefore we batch all rows into a single global buffer that decreases load on the WebSocket server.

[ayushsharma82]

Regarding Buffer Mutex

This was really just a shot in the dark. What are the approaches to this - apart from leaving it to the user?

[mathieucarbou]

Yes there is a lot of tradeoffs here I agree.

For what is worth, write(c) is not called.

My logger impl if buffering and forwarding to registered Print implementations:

and in the WebSerial class I only have:

Once write(const uint8_t* buffer, size_t size) is overridden, most of the Print methods are using it.

[mathieucarbou]

Also, to solve the dual core issue with potentially 2 tasks on different cores calling write(), if buffering is not handled on caller site, and each core can write as they want, the common buffer behind can contain a mix of characters from both. To correctly fix that you would need to maintain a buffer per core, and get the current core with xPortGetCoreID().

So that's a double buffer...

Personally, here is what I would do:

• fail in write(c):

size_t WebSerialClass::write(uint8_t m) {
  log_e("use the buffered method instead to not overload WS queue, or set #define WS_ALLOW_WRITE_CHAR to really allow this, knowing what you are doing, or fix the caller stack to go through the write(buffer) method, writing lines instead of characters");
  assert(false);
}

FYI: this is what I was using for several months locally and it worked fine. This also helped me correctly find callers I needed to fix thanks to the call stack

• Then I would provide another utility class to help people buffering:

Something similar to a print buffer collector, but which could also detect EOL and flush them in the write method.

For example, such class could be improved to detect EOL and then flush the content to the write method of the delegate passed as parameter, which could be any print or WebSerial itself.

  class WebSerialBuffer : public Print {
    public:
      WebSerialBuffer(Print delegate) { _buffer.reserve(WS_BUFFER_SIZE); };
      size_t write(const uint8_t* p, size_t n) override { return _buffer.concat(reinterpret_cast<const char*>(p), n) ? n : 0; }
      size_t write(uint8_t c) override { return _buffer.concat(static_cast<char>(c)) ? 1 : 0; }
      const String& buffer() const { return _buffer; }

    private:
      String _buffer;
  };

Such buffering would occur on caller side and be memory-collected once not needed anymore.

[mathieucarbou]

@ayushsharma82 : Arduino 3 is out, I've updated the CI build. It is not yet available through espressif / PIO (will take more time for that) - see espressif/arduino-esp32#8796 (comment)

[ayushsharma82]

@mathieucarbou I guess I'll just leave buffer mutex issue to the user itself. Seems very hard to tackle on our own.

Also, the time has come - I'll update all my major libraries to use your fork of AsyncWebServer :)

[ayushsharma82]

@mathieucarbou , I updated CI of v2 branch of WebSerial as well but it failed at arduino step, what could be the issue?

[mathieucarbou]

@mathieucarbou , I updated CI of v2 branch of WebSerial as well but it failed at arduino step, what could be the issue?

That's the Arduino CI part that is failing. I am wondering if this is caused by your fork of the gitbub action ?

FYI here is the CI setup I use in my WebSerial fork (I dropped support for esp8266 - you would just need to add it back)

https://github.com/mathieucarbou/WebSerialLite/blob/main/.github/workflows/ci.yml

[mathieucarbou]

It is also failing for ESP-DASH....

[mathieucarbou]

I have updated my fork and I have the same issue... https://github.com/mathieucarbou/ayushsharma82-ESP-DASH/actions/runs/9289998149

Wondering if something has changed on their side...

[ayushsharma82]

Probably something is down or changed on their side.

[mathieucarbou]

I have fixed it in PR #214 by using the same mechanism I use in my other projects. This is more verbose but it works and gives more control.