Skip to content
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

No autoforward after initiated update #123

Closed
bcutter opened this issue Feb 12, 2023 · 9 comments · Fixed by #145
Closed

No autoforward after initiated update #123

bcutter opened this issue Feb 12, 2023 · 9 comments · Fixed by #145

Comments

@bcutter
Copy link
Contributor

bcutter commented Feb 12, 2023

Performing an update with /ota never reloads the page as stated ("...auto-forward...").
Request times out and page at /uploadota displays default browser message, e. g.
grafik

Firefox used.

Workaround: just manually reload the start page (not /uploadota!).

This never worked for me until now (latest update: 8.8 today).

@bcutter bcutter changed the title No autoforward after initiated updates No autoforward after initiated update Feb 12, 2023
@mundschenk-at
Copy link
Collaborator

I have also had this behaviour with Safari, but not always. Sometimes the forwarding works. I have not been able to detect a pattern.

@bcutter
Copy link
Contributor Author

bcutter commented Feb 20, 2023

For me just never worked so far. Always on Firefox. Seems like after a timeout a auto-refresh is not possible. Could be easily cross-tested with several browsers I guess.

The second irritating thing is: if you just click reload, what is gonna happen? Will the update initiated once again?

grafik

@technyon
Copy link
Owner

I've increased the timeout for OTA so it's more likely to succeed (60 to 120 seconds). Why browsers don't like this I have no idea. It's done with javascript using setTimeout(), maybe browsers kill it if runs too long or something.

@bcutter
Copy link
Contributor Author

bcutter commented Mar 6, 2023

v8.15 when updating to v8.16 still times out - only by a few seconds in my case. Both update took something from 35 to 40 seconds, unfortunately still:

grafik

@technyon
Copy link
Owner

technyon commented Mar 7, 2023

I'm aware of the issue, but not so sure what to do about it. It falls into a domain that's not my strong suit - I'd sure take a tip from a web developer if someone is around here.

@regevbr
Copy link
Contributor

regevbr commented Mar 14, 2023

Did an investigation. Here are my conclusions:

  1. Once the OTA form is submitted, the page gets redirected to /uploadota, canceling the setTimeout as we are no longer on the page it was called at (/ota)
  2. /uploadota performs the ota but never gets a chance to return an HTTP response as the esp board gets rebooted, and that is why we get those messages shown above.
  3. Current setTimeout code will only take place (if any) in case the upload took longer than 2 minutes, and I think it might hurt the upload process and cancel the ota (I'm not even sure it will even happen as the form submission actually changes the page)

To fix the issue we need to:

  1. Remove the current setTimeout code in buildOtaHtml as it is not relevant
  2. Make the request handler of /uploadota send a proper HTML page with a setTimeout for 10 seconds
  3. Delay the esp restart in Ota::updateFirmware but in an async way (and not the sync way that it is now), so the code will have the chance to return the HTTP response to the client.

@technyon if you approve of what I said here, I will be more than happy to make a PR, I can make all these changes, besides the async delay which I will need some guidance with.

regevbr added a commit to regevbr/nuki_hub that referenced this issue Mar 14, 2023
technyon added a commit that referenced this issue Mar 15, 2023
fix #123 - No autoforward after initiated update
@alexdelprete
Copy link
Collaborator

Tested with Chrome and Firefox and it worked. Had issues with Edge, but after clearing cache and 1 cookie, I restarted the upgrade and this time it worked.

Thanks a lot @regevbr, this problem bugged me for quite some time. :)

@bcutter
Copy link
Contributor Author

bcutter commented Mar 23, 2023

Which firmware contains this fix?

Will 8.19 have it? Or did I miss the release notes of latest 8.18?

@alexdelprete
Copy link
Collaborator

Which firmware contains this fix?

8.19, it's not released yet, we're testing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants