-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Refactor Web Server Parsing-impl.h using StreamString to avoid String Reallocations #9005
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ | |
|
|
||
| #include <Arduino.h> | ||
| #include <Stream.h> | ||
| #include <StreamString.h> | ||
|
|
||
| #define PARSE_TIMEOUT 1000 // default number of milli-seconds to wait | ||
| #define NO_SKIP_CHAR 1 // a magic char not found in a valid ASCII numeric field | ||
|
|
@@ -288,6 +289,59 @@ String Stream::readStringUntil(const char* terminator, uint32_t untilTotalNumber | |
| return ret; | ||
| } | ||
|
|
||
| String Stream::readStreamString(const ssize_t maxLen ,const oneShotMs::timeType timeoutMs) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is exactly the same as ::sendSize. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not Exactly, It constructs a String Object and a Stream Object attached to it, then populate it, then returns the String Object, it's behavior is more like ::readString `sendSize' expect you to pass a Pointer to a Stream and returns the total number of bytes.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is indeed not exactly the same. (edited:) line = client.readStreamString(size); // (no copy) can also be written as client.sendSize(S2Stream(line), size); // no copy (not tried though, meant to work, temporary is assumed) Anyway this function is not used in the following of your pull request. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, your version is much smaller 😄 Please advise on the best course of action, should I remove the unused functions ? |
||
| String ret; | ||
| S2Stream stream(ret); | ||
| sendGeneric(&stream, maxLen, -1, timeoutMs); | ||
| return ret; | ||
| } | ||
|
|
||
| String Stream::readStreamStringUntil(const int readUntilChar, const oneShotMs::timeType timeoutMs) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Based on ::readStringUntil |
||
| String ret; | ||
| S2Stream stream(ret); | ||
| sendGeneric(&stream, -1, readUntilChar, timeoutMs); | ||
| return ret; | ||
| } | ||
|
|
||
| String Stream::readStreamStringUntil (const char* terminatorString, uint32_t untilTotalNumberOfOccurrences, const oneShotMs::timeType timeoutMs) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For full genericity with any kind of stream, we should rather extend There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We Should Do that, but I am having a hard time making this modification. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Having tried implementing that...
Still, original Stream methods could be improved? idk if users care about internals here, only that String is the end result. |
||
| String ret; | ||
| S2Stream stream(ret); | ||
| uint32_t occurrences = 0; | ||
| size_t termLen = strlen(terminatorString); | ||
| size_t termIndex = 0; | ||
| // Serial.printf("S %s\n",terminatorString); | ||
| while(1){ | ||
| size_t read = sendGeneric(&stream, -1, terminatorString[termIndex], timeoutMs); | ||
| // Serial.printf("r %d, l %d, ti %d\n", read, termLen, termIndex); | ||
| if(getLastSendReport() != Report::Success) { | ||
| Serial.printf("Error %d\n", (int) getLastSendReport()); | ||
| break; | ||
| } | ||
| if(termIndex == termLen - 1){ | ||
| // Serial.printf("m %d\n", occurrences); | ||
| if(++occurrences == untilTotalNumberOfOccurrences){ | ||
| break; | ||
| }else{ | ||
| ret += terminatorString; | ||
| termIndex = 0; | ||
| continue; | ||
| } | ||
| } | ||
| int c = timedPeek(); | ||
| // Serial.printf("c %c %02X\n", c, c); | ||
| if( c >= 0 && c != terminatorString[++termIndex]){ | ||
| ret += String(terminatorString).substring(0, termIndex); | ||
| termIndex = 0; | ||
| continue; | ||
| }; | ||
| if(c < 0 || (read == 0 && termIndex == 0)) break; | ||
| } | ||
|
|
||
| return ret; | ||
| } | ||
|
|
||
|
|
||
|
|
||
| // read what can be read, immediate exit on unavailable data | ||
| // prototype similar to Arduino's `int Client::read(buf, len)` | ||
| int Stream::read (uint8_t* buffer, size_t maxLen) | ||
|
|
||