-
Notifications
You must be signed in to change notification settings - Fork 13.3k
HTTPUpdateServer callbacks for progress/errors #2694
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
Conversation
Semantically identical, but moving it in a single clean commit to ease review. We're going to introduce new privates that rely on public definitions. Signed-off-by: Karl Palsson <karlp@tweak.net.au>
No semantic change. Added {} for all single line ifs. Avoids future mistakes. Consistently use 4 spaces for all indents, instead of a mix of 2 and 4. This makes it more inline with other libraries, and easier on the eyes for newcomers to the codebase. Signed-off-by: Karl Palsson <karlp@tweak.net.au>
Use a single point wrapper function. Eventually we can let the user provide their own debug handler. Signed-off-by: Karl Palsson <karlp@tweak.net.au>
Provide basic callbacks for update start, end, error and progress. Progress callback provides as much information as we can, but because the filehandler processs as it receives data, we don't always know the entire file size in the first call. Signed-off-by: Karl Palsson <karlp@tweak.net.au>
Current coverage is 27.80% (diff: 100%)@@ master #2694 diff @@
==========================================
Files 20 20
Lines 3625 3625
Methods 335 335
Messages 0 0
Branches 656 656
==========================================
Hits 1008 1008
Misses 2441 2441
Partials 176 176
|
if there's some docs on how coverage testiing works, I'd be happy to work on that too |
Thanks for your PR, but the core and libraries have changed enough that this PR now has a merge conflict. Could you merge it manually with the latest core, so we can consider it for future releases? |
and wait another three years? I'll consider it if I'm working on another arduino esp8266 project again, but there's none on the table right now. |
Replaces abandoned esp8266#1817 and esp8266#2694 Add optional std::function callback (so it supports lambdas and normal functions) via ::onStart, ::onEnd, ::onProgress, and ::onError methods. Update example with their use. From @baruch's original pull request: The callback is called when the upgrade actually starts rather than just the initial query so that the user can know that it will not take longer and can also prepare for the upgrade by shutting down other works. From @karlp's original pull request: Incomplete: I've not updated any documentation yet. If this style looks good, I'll happily go and update the documentation (likewise for the examples)
Replaces abandoned #1817 and #2694 Add optional std::function callback (so it supports lambdas and normal functions) via ::onStart, ::onEnd, ::onProgress, and ::onError methods. Update example with their use. From @baruch's original pull request: The callback is called when the upgrade actually starts rather than just the initial query so that the user can know that it will not take longer and can also prepare for the upgrade by shutting down other works. From @karlp's original pull request: Incomplete: I've not updated any documentation yet. If this style looks good, I'll happily go and update the documentation (likewise for the examples)
Incomplete: I've not updated any documentation yet. If this style looks good, I'll happily go and update the documentation (likewise for the examples)
One thing I've noticed, I'm using NeoPixelBus library as well, and when the update starts, I can't use the pixels anymore. I was hoping to animate some leds for the progress handler, but it's as if something is killing off internal timers or something? Any ideas? Serial printf works just fine in the progress handlers.
This is an alternative implementation of #1817
Given that there was no existing style guide, I did restyle this library to match some other libraries and be at least self consistent. Those can be dropped of course if necessary.