-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
WIP: Swapping in async web server #178
Conversation
@bau-sec You will see 👇 that there are pre-commit checks that need to pass before changes can be merged. You can run them locally by installing pre-commit and running |
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.
Love to make this change, pending testing to ensure that this does not cause problems.
Things to test would be to:
- Verify captive portal works
- Ensure that BLE scanning and accessing the website doesn't cause crashes.
- Verify all pages in website work without crashes
- Ride verification, ensuring BLE works during a ride with website access
- Deploying firmware update from website works
- @doudar More???
@@ -12,6 +12,7 @@ lib_deps = | |||
;https://github.com/h2zero/NimBLE-Arduino.git#master //Nimble is tracked and including this will probably cause issues. | |||
teemuatlut/TMCStepper@^0.7.1 | |||
bblanchon/ArduinoJson@^6.17.2 | |||
ESP Async WebServer |
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.
It is a good practice to pin to a version, if possible, so that we builds can be idempotent.
How did I miss this? Weird sorry I’m just getting to it now. Big thing is really just getting around h2zero/NimBLE-Arduino#167 It rears it’s ugly head at basically any time during run and probably has something to do with TLS. There might be a fix upstream but every time (so far) there’s been hope, our dreams have been crushed shortly thereafter. There is some hope in that thread though so maybe it’s been resolved/worked around recently? @h2zero , any input on the stability of async/tls with nimble? Have others finally gotten it working? |
The issue as far as I know still exists due to the many factors involved. 2 asynchronous radio stacks fighting over a single antenna is a difficult puzzle to solve, there may be work-a rounds as mentioned in the linked issue but nothing concrete that I am aware of. I wanted to put some more work into the solution but so much of the issue resides in a closed source binary that even if I found it I couldn't fix it 😢. |
@h2zero any thoughts as to why the sync version of the web server works well? |
The simple answer is likely complexity. In sync it's a simple request/response that can be managed, in asyc there can be multiples of both happening and the controller needs to be handled appropriately. As alluded to in the issue thread, it seems much of the problem is related to the asyc queue and timeslotting of the antenna, which should be managed lower level. However, I have a suspicion that the freertos tick rate plays a significant role in this and if set to something like 100hz instead of 1000hz in arduino the problem would solve itself (I have done this and it seems to work well). |
Interesting. We don't need incredibly fast ble or wifi communications on this project so that might be a great solution. |
Seen this? me-no-dev/AsyncTCP#116 |
Agreed, this is not near ready. I was mostly putting it up so people could see progress. WIP = work in progress, not sure if I've seen you guys use that convention; sorry if intent wasn't clear. With bluetooth scans things are very crashy. I might switch over to a non-async websocket lib to start playing around with pushing data out to the page from the server. It hurts to switch though, the ESPAsyncWebServer syntax and features are so nice. If I switch ws libraries I'll do it in another branch/pr so this code is around if the underlying issue between nimBLE and ESPAsyncWebServer is resolved. |
@kadaan Thanks for the link, quite insightful. Hopefully async will be improved in the future, sadly it seems to be nearly abandoned at this point (last commit almost 18 months ago). There are a few issues there with suggested solutions to try, unfortunately they are buried in the "closed" (bot) section. |
@bau-sec It would be sad not to use a better webserver. There is quite a lot to be gained by caching, compression, and being able to write directly to the stream. |
I wouldn't completely lose hope on it - It would be so nice to be able to use a better webserver |
I'm curious to hear of any testing results with the new arduino master and NimBLE master. They have both been updated to use the latest IDF version and it may resolve the underlying issues from the controller co-existence (wishful thinking maybe). If any of you try it please post the results. |
This weekend I'll take a stab at rebasing this branch to get it current and combining the Arduino and NimBLE master branches to see the state of things. |
Just following up again. @bau-sec , have you had any time to mess around with this? |
Sorry, I've been pretty busy the last few months and have slacked off on my opensource projects. If anyone wants to take over looking into this that's fine w/ me. Hopefully I'll be able to circle back and help out more in the future. |
No problem! I'll just close this PR for now as I don't really have the time to look into it currently either. Feel free to open it in the future if you work on it. Thanks! |
Switching to ESPAsyncWebServer, currently compiles and hosts the web app, but still needs more work and testing