Skip to content

Add a start callback for HTTP updater #1817

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

Closed
wants to merge 1 commit into from

Conversation

baruch
Copy link
Contributor

@baruch baruch commented Mar 25, 2016

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.

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.
@baruch baruch force-pushed the hooks-in-http-update branch from e20554c to ee87ac0 Compare March 25, 2016 14:03
@igrr
Copy link
Member

igrr commented Mar 27, 2016

This almost looks good to me, but I would propose to replace plain C function pointer with an std::function<void(void)>, similar to what we do in ESP8266WebServer library.
Obvious benefit is that you can bind a member function to a pointer/reference using std::bind, and pass the result into std::function. With plain C function pointer there is no way to do that.

@mangelajo
Copy link
Contributor

If you had time to add progress and end callbacks like in ArduinoOTA, that'd be great.

Cheers!,

@baruch
Copy link
Contributor Author

baruch commented Mar 28, 2016

The problem with progress and end is that at that time there is no TCP/UDP active as they are cancelled by the upgrade process.

As for the method calls, I can change to that, they are not that common in Arduino code though and all other callbacks throughout the system is normal function calls too.

@mangelajo
Copy link
Contributor

@baruch , but you don't need any other open TCP/UDP connection to show progress, the use case is device itself showing on a display/leds. I use it in all my projects so the end user can have some indication of the upgrade progress.

I'd be glad to see an standard interface for those callbacks for consistency within esp8266-arduino at least.

@ghost
Copy link

ghost commented Apr 11, 2016

Can one of the admins verify this patch?

@krzychb
Copy link
Contributor

krzychb commented Apr 11, 2016

@esp8266-arduino-bot,
You are not a human, are you?
Great avatar 😄 👍

@igrr
Copy link
Member

igrr commented Apr 11, 2016

Sorry for the noise. My regression testing setup is misbehaving.

@igrr
Copy link
Member

igrr commented Apr 12, 2016

@baruch just to clarify regarding callback functions: if an argument is an std::function<void(void)>, you can pass plain C function pointer and it will be implicitly converted. This will behave just like other Arduino functions, attachInterrupt for example. However, this also allows the user to pass in any other function object (lambda function, anything that std::bind returns, or even something custom). So in general, replacing a C function pointer argument with a corresponding std::function won't hurt.

@karlp
Copy link
Contributor

karlp commented Nov 13, 2016

After just running into this lack in the http update server, I've started work on this based on the ArduinoOTA style.

@earlephilhower earlephilhower added merge-conflict PR has a merge conflict that needs manual correction waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. labels Oct 1, 2019
@earlephilhower
Copy link
Collaborator

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?

@earlephilhower
Copy link
Collaborator

Closing as abandoned for #6796

earlephilhower added a commit to earlephilhower/Arduino that referenced this pull request Nov 17, 2019
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)
devyte pushed a commit that referenced this pull request Nov 19, 2019
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-conflict PR has a merge conflict that needs manual correction waiting for feedback Waiting on additional info. If it's not received, the issue may be closed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants