-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[shelly] Fix thing re-init after power cycle for firmware update #17163
Conversation
...hab.binding.shelly/src/main/java/org/openhab/binding/shelly/internal/api2/Shelly2ApiRpc.java
Outdated
Show resolved
Hide resolved
...hab.binding.shelly/src/main/java/org/openhab/binding/shelly/internal/api2/Shelly2ApiRpc.java
Show resolved
Hide resolved
@lsiepel changes applied please also consider as back-Ort to 4.2 |
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.
Thanks for the fix. I have added a few comments as well. (FYI @lsiepel)
...nhab.binding.shelly/src/main/java/org/openhab/binding/shelly/internal/api2/ShellyBluApi.java
Outdated
Show resolved
Hide resolved
...ding.shelly/src/main/java/org/openhab/binding/shelly/internal/handler/ShellyBaseHandler.java
Outdated
Show resolved
Hide resolved
...hab.binding.shelly/src/main/java/org/openhab/binding/shelly/internal/api2/Shelly2ApiRpc.java
Show resolved
Hide resolved
...hab.binding.shelly/src/main/java/org/openhab/binding/shelly/internal/api2/Shelly2ApiRpc.java
Show resolved
Hide resolved
...hab.binding.shelly/src/main/java/org/openhab/binding/shelly/internal/api2/Shelly2ApiRpc.java
Show resolved
Hide resolved
...hab.binding.shelly/src/main/java/org/openhab/binding/shelly/internal/api2/Shelly2ApiRpc.java
Outdated
Show resolved
Hide resolved
...hab.binding.shelly/src/main/java/org/openhab/binding/shelly/internal/api2/Shelly2ApiRpc.java
Outdated
Show resolved
Hide resolved
review changes are applied, please check |
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.
Thanks for addressing comments. I have added two additional findings.
@lsiepel - anything more to add, I see your two comments are already resolved?
@markus7017, please remove the label "additional testing preferred" when verified and ready to be merged.
...ding.shelly/src/main/java/org/openhab/binding/shelly/internal/handler/ShellyBaseHandler.java
Outdated
Show resolved
Hide resolved
@jlaur changes pushed |
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.
LGTM
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.
LGTM. Was on the road last couple of days so just had a quick look. See that other comments are made and i have nothing to add.
Edit: Will merge (and backport) once tests confirmed.
Ping @markus7017 |
I'm running the build since several days with my 60 Shellys, looks good. I'll ask for additional testing, just to make sure |
For the avoidance of doubt, do you mean that you are going to ask user(s) to test this, then get back to this PR with confirmation when it has been succesfully tested and is ready to be merged? Just to reiterate:
|
on ota_success message. Signed-off-by: Markus Michels <markus7017@gmail.com>
Signed-off-by: Markus Michels <markus7017@gmail.com>
further events throughout the upgrade process Signed-off-by: Markus Michels <markus7017@gmail.com>
Signed-off-by: Markus Michels <markus7017@gmail.com>
Signed-off-by: Markus Michels <markus7017@gmail.com>
Signed-off-by: Markus Michels <markus7017@gmail.com>
Signed-off-by: Markus Michels <markus7017@gmail.com>
184ebd6
to
b01fede
Compare
This pull request has been mentioned on openHAB Community. There might be relevant details there: |
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur we are good to go |
As i’m on holiday, not able to backport now. With the upcoming point release don’t wait for me. |
) * Fixes thing re-init during/after firmware update. API will not be closed on ota_success message. * Don't disconnect from device during firmware update, this ensures to get further events throughout the upgrade process * suspress COMMUNICATION_ERROR on expected restart after fw upgrade Signed-off-by: Markus Michels <markus7017@gmail.com>
…nhab#17163) * Fixes thing re-init during/after firmware update. API will not be closed on ota_success message. * Don't disconnect from device during firmware update, this ensures to get further events throughout the upgrade process * suspress COMMUNICATION_ERROR on expected restart after fw upgrade Signed-off-by: Markus Michels <markus7017@gmail.com>
…nhab#17163) * Fixes thing re-init during/after firmware update. API will not be closed on ota_success message. * Don't disconnect from device during firmware update, this ensures to get further events throughout the upgrade process * suspress COMMUNICATION_ERROR on expected restart after fw upgrade Signed-off-by: Markus Michels <markus7017@gmail.com> Signed-off-by: Patrik Gfeller <patrik.gfeller@proton.me>
…nhab#17163) * Fixes thing re-init during/after firmware update. API will not be closed on ota_success message. * Don't disconnect from device during firmware update, this ensures to get further events throughout the upgrade process * suspress COMMUNICATION_ERROR on expected restart after fw upgrade Signed-off-by: Markus Michels <markus7017@gmail.com>
…nhab#17163) * Fixes thing re-init during/after firmware update. API will not be closed on ota_success message. * Don't disconnect from device during firmware update, this ensures to get further events throughout the upgrade process * suspress COMMUNICATION_ERROR on expected restart after fw upgrade Signed-off-by: Markus Michels <markus7017@gmail.com>
…nhab#17163) * Fixes thing re-init during/after firmware update. API will not be closed on ota_success message. * Don't disconnect from device during firmware update, this ensures to get further events throughout the upgrade process * suspress COMMUNICATION_ERROR on expected restart after fw upgrade Signed-off-by: Markus Michels <markus7017@gmail.com> Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
After a power cycle all connected devices should be re-initialized and thing should go ONLINE. This was broken.
Same after a firmware upgrade. Thing went to FIRMWARE_UPDATING, but never got back ONLINE.
Closing
#17014