Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

webserial.print displays truncated value for long String value #110

Closed
Tomgault1959 opened this issue Jan 18, 2025 · 10 comments
Closed

webserial.print displays truncated value for long String value #110

Tomgault1959 opened this issue Jan 18, 2025 · 10 comments

Comments

@Tomgault1959
Copy link

Hi,

I am migrating all my sketches from version 1.4 to version 2.0.8 and values ​​assigned to String are truncated on display or cause a system crash when the values ​​are very long. I create a testing sketch for this subject.

I use this library with ESP32

Version Webserial 2.0.8
ESPAsyncWebServer by Mathieu Carbou 3.6.0
AsyncTCP by Mathieu Carbou 3.3.2
ESP32 core 3.1.1
Compile with Arduino IDE 2.3.4 on Windows

For testing purpose, enter the command ? to display help information store in bufferPrint
string. The result will give a truncated value on webserial html view or a system crash depend how long the string is. The serial monitor display value correctly all the time. All work fine with on version 1.4

The V2.0.8 test sketch here :
V2.0.8.txt

#include <Arduino.h>
#if defined(ESP8266)
#include <ESP8266WiFi.h>
#include <ESPAsyncTCP.h>
#elif defined(ESP32)
#include <AsyncTCP.h>
#endif
#include <ESPAsyncWebServer.h>
#include <WebSerial.h>

AsyncWebServer server(80);

const char* ssid = ""; // Your WiFi SSID
const char* password = ""; // Your WiFi Password
const char* http_username = "admin"; //Login (must be set)
const char* http_password = "admin"; //Login password (must be set)

void setup() {
Serial.begin(115200);
WiFi.mode(WIFI_STA);
WiFi.begin(ssid, password);

if (WiFi.waitForConnectResult() != WL_CONNECTED) {
Serial.printf("WiFi Failed!\n");
return;
}

// Once connected, print IP
Serial.print("IP Address: ");
Serial.println(WiFi.localIP());

server.on("/", HTTP_GET, [](AsyncWebServerRequest *request) {
request->send(200, "text/plain", "Hi! This is WebSerial demo. You can access webserial interface at http://" + WiFi.localIP().toString() + "/webserial");
});

// WebSerial is accessible at "/webserial" in browser
WebSerial.begin(&server);
// Set Authentication Credentials
//WebSerial.setAuthentication(http_username, http_password);

/* Attach Message Callback */
WebSerial.onMessage([&](uint8_t *data, size_t len) {
Serial.printf("Received %lu bytes from WebSerial: ", len);
Serial.write(data, len);
Serial.println();
WebSerial.println("Received Data...");
String d = "";
for(size_t i=0; i < len; i++){
d += char(data[i]);
}
WebSerial.println(d);

String bufferPrint = "";  
  
if (d == "?")
{
  bufferPrint += " R              : Redémarrage du service\n";
  bufferPrint += " N              : Liste des réseaux disponibles\n";
  bufferPrint += " L              : Lister les paramètres réseaux modifiables\n";
  bufferPrint += " E              : Effacer tous les paramètres de configuration réseau\n";
  bufferPrint += " T              : Afficher les données RTC\n";
  bufferPrint += " ST             : Synchroniser l'horloge RTC à partir du temps NTP\n";
  bufferPrint += " D              : Démarrer la génératrice à essence\n";
  bufferPrint += " P              : Arrêt de la génératrice à essence\n";
  bufferPrint += " BS             : Power OFF du robot qui gère la génératrice à esssence\n";
  bufferPrint += " BD             : Power ON du robot qui gère la génératrice à esssence\n";

//Uncomment next line cause system to crash and reboot
/*
bufferPrint += " RRSI : Force du signal Wi-Fi\n";
bufferPrint += " LOAD : LOAD - Lister la valeur du controleur 1=ON 0=OFF\n";
bufferPrint += " S : SLEEP MODE 0=Mode normal sans interruption 1=Deep Sleep activé\n";
bufferPrint += " HA : Heure avancée, valeur possible 0 ou 1. 0 = Heure normale, 1 = heure avancée\n";
bufferPrint += " GMT : Correction de l'heure UTC\n";
bufferPrint += " PING : Vérification de la connexion internet et du serveur chalet\n";
bufferPrint += " TEMP : Température maximale déclenchant l'arrêt de la génératrice\n\n";
bufferPrint += " setSSID : Modifier le nom du réseau Wi-Fi\n";
bufferPrint += " setPASS : Modifier le mot de passe du réseau Wi-Fi\n";
bufferPrint += " setLOAD : LOAD Assigner la valeur du controleur - 1=ON 0=OFF\n";
bufferPrint += " setDEBUG : DEBUG Assigner la valeur du controleur - 0=prd 1=debug\n";
bufferPrint += " setSLEEP : SLEEP MODE Assigner le mode d'économie d'énergie - 0=Mode normal 1=Deep Sleep\n";
bufferPrint += " setGMT : Modifier la correction de l'heure UTC\n";
bufferPrint += " setHA : Heure avancée, valeur possible 0 ou 1. 0 = Heure normale, 1 = heure avancée\n";
bufferPrint += " setTEMP : Modifier la température maximale permise avant l'arrêt de la génératrice\n";
bufferPrint += "\n- Toujours redémarrer le service pour relire les nouveaux paramètres de configuration réseau IP\n";
*/
WebSerial.println(bufferPrint);
Serial.println(bufferPrint);

}
});

// Start server
server.begin();
}

void loop() {
static unsigned long last_print_time = millis();

// Print every 20 seconds (non-blocking)
if ((unsigned long)(millis() - last_print_time) > 20000) {
WebSerial.print(F("IP address: "));
WebSerial.println(WiFi.localIP());
WebSerial.printf("Uptime: %lums\n", millis());
WebSerial.printf("Free heap: %u\n", ESP.getFreeHeap());
last_print_time = millis();
}

WebSerial.loop();
}

The V1.4 test sketch here :
V1.4.txt

Regards,

Jean

@mathieucarbou
Copy link
Contributor

@Tomgault1959 : please paste the decoded stack trace (files and line numbers) about the crash. Thanks!

@mathieucarbou
Copy link
Contributor

mathieucarbou commented Jan 18, 2025

@Tomgault1959 : fyi I was able to reproduce and confirm that the issue is not caused neither by our fork of AsyncTCP and ESPAAsyncWS, but from a change in @ayushsharma82 code from v1: the new WebSerial is now doing some (IMHO) shady buffering and send logic which can make the sending goes from the main loop instead.

This is a breaking change, and now you need to add WebSerial.loop() in the loop.

Since you forgot it, the data is not sent.

FYI, I have contributed a high performance mode as an alternative of this implementation which is not using this loop addition, and just buffers the data until \n is found then the line is sent.

It can be activated with: -D WSL_HIGH_PERF and just modify your code to add:

  WebSerial.setBuffer(128); // because your longer line seems to be less than that
  WebSerial.begin(&server);

Now this buffer could even be omitted, if you were not using printf or functions writing single a char, and make sure to only use functions like print("string\n"), println(), etc which are printing complete full lines ending with \n, then you don't need buffering at all, and the line would be directly sent.

@Tomgault1959
Copy link
Author

Thanks Mathieu for the quick response,

WebSerial.loop() is at the end of loop section. How to activate stack trace ? Core Debug Level : "Debug" ? I tried with WebSerial.setBuffer(128); without success.

Regards,

Jean

@mathieucarbou
Copy link
Contributor

You need -D WSL_HIGH_PERF also. I don't use Arduino IDE so I don't know how to pass build flags and activate stack trace, but I know this is possible.

@Tomgault1959
Copy link
Author

Ok I'll check and come back as soon as possible.

@Tomgault1959
Copy link
Author

Hi Mathieu,

I pass WSL_HIGH_PERF flag (add #define WSL_HIGH_PERF to webserial.h), add WebSerial.setBuffer(128); to sketch and comment WebSerial.loop(). Now all work fine.

Regards,

Merci milles fois Mathieu pour ton incroyable support encore une fois et ta rapidité d'intervention.

Jean,
Beaupré Québec

@ayushsharma82
Copy link
Owner

@mathieucarbou , I was thinking of updating the library with your high performance approach only so that no flag is needed. It's just inconvenient that we ask users for additional setup in library but the main issue is that I want to cover "printf or functions writing single a char" condition as well.

What if we override the print and printf methods of base class? Let me know if you can come up with a nice 2-in-1 solution to this issue. WebSerial should ideally handle all scenarios.

@mathieucarbou
Copy link
Contributor

mathieucarbou commented Jan 18, 2025

What if we override the print and printf methods of base class? Let me know if you can come up with a nice 2-in-1 solution to this issue. WebSerial should ideally handle all scenarios.

The implementation I did overrides the write(c) function which crashes with an error message in case -D WSL_HIGH_PERF is used but write(c) is called (which is usually the case with printf and println).

"Non-buffered write is not supported. Please use write(const uint8_t* buffer, size_t size) instead."

So the message could be improved to specifically say: "write(c) is not supported, see http://webserial.pro/limitations to know the limitations and how to work around them." or "Non-buffered write is not supported: use webSerial.setBuffer(size_t), or write(const uint8_t* buffer, size_t size), or print(...)"

Currently, with -D WSL_HIGH_PERF, line #148 (when using the write(buffer) implementation but not having set a buffer with setBuffer(), there is no check ensuring that the buffer ends with \n. I would also add this check to make sure users without a buffer do NOT call write(buffer) consecutively wit ha few chars inside, and fail with "\n is required at line end, see http://webserial.pro/limitations to know the limitations and how to work around them."

Forget the above, this is wrong. \n are removed when sending to WS, and the way I did the API, I remove the \n: size = buffer[size - 1] == '\n' ? size - 1 : size; (line 148), which is OK and supports client-side buffering like the high perf example:

    buffer += "";
    for (int i = 0; i < r; i++) {
      buffer += dict[random(0, 62)];
    }
    WebSerial.print(buffer);

The client does not need to add the ending \n: we consider that a buffer represents 1 line.

Also, if users want even more perf, they have access to makeBuffer() and send() to avoid a memory copy and directly put data into the websocket buffer.

@ayushsharma82
Copy link
Owner

ayushsharma82 commented Jan 19, 2025

I was thinking of making use of a simple buffered approach when we encounter write(c) function and leaving the same high-perf implementation for write(buffer, size).

We should not throw error on write(c) as it's still used in some libraries for logging so we can't just ignore that.

@mathieucarbou
Copy link
Contributor

mathieucarbou commented Jan 19, 2025

That is why the high perf mode has an optional buffering.
If set, write(c) can be used. Otherwise, not.

The problem by using a buffet is what I have explained to you before: it works only in specific cases because it is a shared resource at a static location which can be shared amongst callers.

For logger implementation like you said, and this is also the use case I do in my apps, buffering should be done on caller side to avoid log collision and no buffer should be used on webserial side. This is the only way to avoid interleaving log lines.

You can have a look at MycilaLogger (the logging lib I use) and how I couple it with Serial and Webserial. I use for exemple a Stream buffer to accumulate the log line from printf in a buffer that will be sent at once to webserial in its own buffer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants