Fix HTTP method conflicts with Arduino core: use HTTP_Method.h enum and list-based composite#407
Fix HTTP method conflicts with Arduino core: use HTTP_Method.h enum and list-based composite#407mathieucarbou wants to merge 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Updates HTTP method handling to avoid conflicts with Arduino core method definitions by switching to platform-provided HTTP method enums and introducing a list-based composite method type.
Changes:
- Replaced the project’s bitmask-based HTTP method composite with a
WebRequestMethodCompositewrapper around platformhttp_method. - Expanded method string literals and improved method-to-string mapping for additional HTTP/WebDAV/UPnP methods.
- Added an example sketch demonstrating mixed usage with Arduino’s
HTTP_Method.h.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/literals.h | Adds string literals for additional HTTP methods (CONNECT/TRACE/WebDAV/etc.). |
| src/WebServer.cpp | Adds verbose logging when registering handlers. |
| src/WebRequest.cpp | Switches parsing/storage to platform HTTP method enum and refactors method-to-string. |
| src/WebHandlers.cpp | Updates comparisons to use platform HTTP_* constants. |
| src/WebHandlerImpl.h | Updates default handler method to HTTP_ANY. |
| src/Middleware.cpp | Updates CORS preflight check to HTTP_OPTIONS. |
| src/ESPAsyncWebServer.h | Introduces platform enum inclusion/fallback and new WebRequestMethodComposite implementation. |
| src/AsyncJson.cpp | Updates JSON handler method composite construction and GET checks. |
| platformio.ini | Adds example entry points and reorders WebDAV example line. |
| examples/HTTPMethods/HTTPMethods.ino | New example validating HTTP method compatibility across libraries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| result.concat(_methods[i]); | ||
| } | ||
| result.concat(">"); | ||
| return result; | ||
| } | ||
|
|
||
| private: |
There was a problem hiding this comment.
@copilot wrong suggestion. there is already a methodToString() existing in the code base
| private: | ||
| std::vector<WebRequestMethod> _methods; | ||
| }; |
There was a problem hiding this comment.
@copilot : considering how enums are defined, this limit the list to only 8 values right ? because a utin64_t contains 8x uint64_t ?
There was a problem hiding this comment.
@willmmiles @me-no-dev : I purposefully used a vector to not limit the size. An enum value can be contained in an uint8_t so we could have an array uint8_t[8] or a uint64_t if we want to limit, but this would not be backward compatible.
…nd list-based composite
|
replaced by #413 |
This PR is a rollback of previous committed changes and a proposal to refactor correctly our HTTP enum in order to correctly support the incoming platform enums, plus stay backward compatible.
It fixes:
This PR diff has to be compared with the latest stable commit in main: e939f7e.
Link for comparison: e939f7e...fix/402
This PR works by re-using the existing ESP-IDF enum values (so we support now more than jus tWebDAV), and, if not exist, define them like ESP-IDF defines them, plus the HTTP_ANY.
The existing enum types are set to our
WebRequestMethodtype.This PR also contains a redefinition of
WebRequestMethodCompositewhich supports a list ofWebRequestMethodplus operator overloads to support | and & to.The goal of this PR is to be backward compatible while fixing the issues above. It trades performance (removes the bit mask we had) over compatibility and I would say code sanity.