-
Notifications
You must be signed in to change notification settings - Fork 13.3k
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
Implemented support for filters and removable routes in ESP8266WebServer #9152
Conversation
@me-no-dev , Please review this as well. |
Not an Espressif repo (maybe ping @d-a-v). Will merge based on esp32 review, though. |
…/Arduino into webserver-extended
Oh. I thought me-no-dev is a maintainer in this repo too. I'll check documentation. |
Long time ago :) |
Any idea why LwIP CI tests are failing? Compiling on Arduino IDE locally doesn't show any error. |
|
Co-authored-by: Max Prokhorov <[email protected]>
Great! Everything compiles now, what should we do about style test? |
server.on("/", [&]() { | ||
digitalWrite(led, 1); | ||
server.send(200, "text/plain", "Hi!, This route is accessible for STA clients only"); | ||
digitalWrite(led, 0); | ||
}) | ||
.setFilter(ON_STA_FILTER); | ||
|
||
// This route will be accessible by AP clients only | ||
server.on("/", [&]() { | ||
digitalWrite(led, 1); | ||
server.send(200, "text/plain", "Hi!, This route is accessible for AP clients only"); | ||
digitalWrite(led, 0); | ||
}) | ||
.setFilter(ON_AP_FILTER); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/esp8266/Arduino/actions/runs/9516245776/job/26232190207#step:4:400
./tests/restyle.sh runs clang-format-15 on everything, chaining makes extra indentation
lose [&]
and maybe do something explicit with 'on' return value?
auto& onlyAp = ...;
onlyAp.setFilter(ON_AP_FILTER);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried running ./tests/restyle.sh
on my PC, odd enough that did nothing. Do we have to install anything for it to run?
Regarding Example:
I think it's better if we keep it same as ESP32 Filters.ino example, that way people are not confused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-format-15
in $PATH / env CLANG_FORMAT=path-to-clang-format ./tests/restyle.sh
(e.g. https://github.com/muttleyxd/clang-tools-static-binaries)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better if we keep it same as ESP32 Filters.ino example, that way people are not confused.
Right, but it definitely got me confused
[&]
meant to capture by reference, but server callback should not do that. if you do it in setup() and capture value from there, later callback in loop() references invalid memory location- nothing is actually captured, don't wanna do that anyway
Specifically for us here and clang-format, without chaining - there are less spaces.
…ver (esp8266#9152) * feat: added filters and removeable routes * Update Filters.ino * fix: clang-format * chore: updated docs * chore: updated doc * fix: use new implementation * fix: filters.ino example * fix: filters.ino * fix: formatting * fix: formatting (2)
This PR implements filters and removable routes in ESP8266 arduino core, making it API compatible with recent changes to ESP32 WebServer API.
Reference (Recent additions to ESP32 WebServer API):
Noteable Changes:
setFilter
)removeRoute
,removeHandler
,_removeRequestHandler
)