espressif / arduino-esp32

Arduino core for the ESP32
GNU Lesser General Public License v2.1
13.28k stars 7.35k forks source link

StreamString class should also override availableForWrite() otherwise it will always return 0 #8648

Open Jeroen88 opened 11 months ago

Jeroen88 commented 11 months ago

Board

Does not matter

Device Description

Does not matter

Hardware Configuration

Does not matter

Version

latest master (checkout manually)

IDE Name

Arduino IDE v2, does not matter

Operating System

Ubuntu, does not matter

Flash frequency

40 MHz, does not matter

PSRAM enabled

yes

Upload speed

115200, does not matter

Description

If I call availableForWrite() on an instance of the StreamString class this will return 0 (zero) because the virtual function in Print.h (here) is not overridden in StreamString. A minor issue but incorrect IMHO.

Sketch

#include <StreamString.h>

void setup() {
  // put your setup code here, to run once:
  Serial.begin(115200);
  StreamString s;
  Serial.printf("Available for write is %d\n", s.availableForWrite());
}

void loop() {
  // put your main code here, to run repeatedly:

}

Debug Message

Available for write is 0

Other Steps to Reproduce

No response

I have checked existing issues, online documentation and the Troubleshooting Guide

SuGlider commented 11 months ago

@Jeroen88 - This is an interesting issue. availableForWrite() should return the remaining space in the String buffer. It may use String::capacity(), but String has a flexible allocated buffer size. If necessary, it will discard the current buffer and allocate a new one, bigger. Therefore, it is not a good indication of the writable space.

By the way, a recently created String will have capacity() Zero.... https://github.com/espressif/arduino-esp32/blob/b98255d8d1be7280e767f15a8c759e13e00f4b4a/cores/esp32/WString.cpp#L149-L155

StreamString::write() will just reallocate (reserve()) the buffer to make it fit, as long as there is enough HEAP. https://github.com/espressif/arduino-esp32/blob/bd57ff4ab4011fa88be9b191d9217c363927cc45/cores/esp32/StreamString.cpp#L26-L38

Do you have any suggestion?

SuGlider commented 11 months ago

I have run a testing sketch and I found out that the String maximum size is std::numeric_limits<uint16_t>::max() = 65536 bytes.

This may be a good way to calculate the possible StreamString::availableForWrite() space. But, if HEAP is low... it may be also lower. This is not a perfect way to calculate it.

#include "StreamString.h"

void setup() {
  Serial.begin(115200);
}

String myStr("01234567890123456789012345678901234567890123456789");
StreamString myStreamStr;

void loop() {
  // put your main code here, to run repeatedly:
  myStr += "01234567890123456789012345678901234567890123456789"; // 50 chars
  Serial.printf("String :: Size = %d\n",myStr.length());
  myStreamStr.write((uint8_t *)"01234567890123456789", 20); // 20 chars
  Serial.printf("StreamString :: Size = %d\n", myStreamStr.available());
}
SuGlider commented 11 months ago

For the records, Max Capacity of StreamString object is 65518 bytes, when the SoC has no PSRAM. When it has PSRAM, PSRAM will hold the String content that may use all of its free space.... Thus, it depends on the SoC PSRAM and String::CAPACITY_MAX ESP32 with PSRAM goes up to 3MB (CAPACITY_MAX = 3145728) ESP32-S2 with 2MB PSRAM goes up to 1.9MB (all PSRAM) When PSRAM is disabled, it goes up to 65518 (CAPACITY_MAX = 65535). It also may change depending if HEAP or PSRAM has space left...

https://github.com/espressif/arduino-esp32/blob/628b668c5c4c6ecb6d9b685a1159618c01928108/cores/esp32/WString.h#L311-#L315

Jeroen88 commented 11 months ago

Hi @SuGlider, well the exact value does not matter I think, as long as it is big enough for most use cases and for sure not zero, even for a newly created empty StreamString. Maybe it should return an "at least" available amount?

I found out because I took a StreamString to test a real interface that is going to use Serial. Before writing to the Stream I want to test if I can write a buffer. "Normal" streams will return the available buffer size (which is much smaller than CAPACITY_MAX), that is why I think the exact value is not very important.