Remove usage of Arduino constant emptyString and use our own instead#417
Remove usage of Arduino constant emptyString and use our own instead#417mathieucarbou merged 1 commit intomainfrom
Conversation
|
@MitchBradley FYI |
There was a problem hiding this comment.
Pull request overview
This PR replaces uses of the Arduino emptyString constant with a project-owned asyncsrv::empty constant.
Changes:
- Updated default arguments across response/request APIs to use
asyncsrv::empty. - Replaced
emptyStringassignments/returns throughout request parsing and authentication helpers. - Updated examples to return/assign
asyncsrv::emptyinstead ofemptyString.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/WebResponseImpl.h | Switches AsyncBasicResponse String overload default arg to asyncsrv::empty. |
| src/WebRequest.cpp | Replaces emptyString in parsing/temp fields and several const String& return paths. |
| src/WebHandlers.cpp | Updates AsyncFileResponse content-type default argument usage. |
| src/WebAuthentication.cpp | Uses asyncsrv::empty for early-return empty String values on errors. |
| src/ESPAsyncWebServer.h | Updates multiple overload defaults and const String& accessors to use asyncsrv::empty. |
| src/AsyncEventSource.cpp | Returns asyncsrv::empty on allocation failure. |
| examples/Templates/Templates.ino | Updates template processor “no value” return to asyncsrv::empty. |
| examples/RequestContinuation/RequestContinuation.ino | Clears message using asyncsrv::empty. |
| examples/ChunkRetryResponse/ChunkRetryResponse.ino | Clears trigger string using asyncsrv::empty. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
60da0b9 to
b7b0ff1
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@mathieucarbou Thanks for addressing this. After it is merged, should I close #416 and submit a revised version, after addressing copilot's concerns? |
You just need to rebase your HOST PR on top of branch emptyString instead of main. Also please do not forget to open a separate PR for the state machine refactoring. This one is more requires more review and testing. Thank you very much for these ! |
|
One thing that occurred to me is that instead of |
|
Or perhaps a conditionally compiled LOCK macro so one could write LOCK(_client_queue_lock) |
I am not sure that the content of the conditions only apply to multicore. Right now I would leave as it is with the 3 conditions... They are at the moment more linked to the common things provided by these platforms than being multicore or not I think. |
That's the equivalent of what @willmmiles put in place in AsyncTCP. I would just focus at the Linux/Mac support with HOST macro, then you can open a new PR with the state machine. And if you are willing to improve how the locking is down using macros, please open another PR. As long as you keep the work in single isolated unit of work commits and PRs. Thanks 👍 |
6754cab to
41a0e2f
Compare
41a0e2f to
a106995
Compare
This PR is a cleanup in order to use our own empty string constant instead of Arduino one.
This will also help support
HOSTas done in PR #416