earlephilhower / arduino-pico

Raspberry Pi Pico Arduino core, for all RP2040 boards
GNU Lesser General Public License v2.1
1.88k stars 394 forks source link

feat: Implemented support for filters and removable routes in WebServer #2225

Closed ayushsharma82 closed 1 month ago

ayushsharma82 commented 1 month ago

This PR implements filters and removable routes in RP2040 arduino core, making it API compatible with recent changes to ESP32 & ESP8266 WebServer API.

[!NOTE] Please merge when all ESP32 repo pull requests mentioned below are merged.

Reference (Recent additions to ESP32 WebServer API):

Reference (Recent addition to ESP8266 WebServer API):

Noteable Changes:

  1. Request Filters (setFilter)
  2. Removable Routes (removeRoute, removeHandler, _removeRequestHandler)
earlephilhower commented 1 month ago

You need to run tools/restyle.sh to get the formatting correct for the repo here, and (void) out an unused parameter to get through CI. OTW, seems nice and thanks for including an example!

ayushsharma82 commented 1 month ago

Apparently, a lot of files weren't conforming to suggested format. I ran tests/restyle.sh.

ayushsharma82 commented 1 month ago

CI style check failed. Can this be due to different astyle package version on my local machine? I have this on my PC: Artistic Style Version 3.5.

ayushsharma82 commented 1 month ago

Fixed example & formatting.

PS: I formatted WebServer folder only instead of whole library.

earlephilhower commented 1 month ago

Weird about the astyle version differences, thx for fixing it. CI is using whatever the GH runner has installed which seems to match my own version for output (but not revision number).

For the callback build error, I haven't dug into it yet but in this core I split out the HTTPServer from the Webserver. This lets us support HTTPS and HTTP servers with no extra code. Separation of duties, the template (WebServer/WebServerSecure) handles incoming connections and passes it on to the HTTP parsing engine.

ayushsharma82 commented 1 month ago

Fixed everything and tested locally. This should pass all CI tests.

Also, ESP32 PRs are merged therefore this PR is ready for merger too.