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

Implementing safe default for devices not requiring RESET/Passive functionality - Fix for bug in 4.1.0 #1022

Merged
merged 5 commits into from
Oct 11, 2022

Conversation

sibowler
Copy link

@sibowler sibowler commented Sep 7, 2022

Disables RESET functionality unless RESET DPIDs are specified in the device config.

Hopefully fixes issues #1019 #1015

For all those requiring RESET (generally for plugs with power monitoring), try 18,19,20 if your device isn't responding.

Have done brief regression test on local system. If others could assist with testing this change to confirm the issue has been correctly identified, that would be appreciated.

@sibowler sibowler mentioned this pull request Sep 7, 2022
@sibowler sibowler changed the title Implementing safe default for devices not requiring RESET Implementing safe default for devices not requiring RESET - Fix for bugs in 4.1.0 Sep 7, 2022
@sibowler sibowler changed the title Implementing safe default for devices not requiring RESET - Fix for bugs in 4.1.0 Implementing safe default for devices not requiring RESET - Fix for bug in 4.1.0 Sep 7, 2022
@sibowler
Copy link
Author

sibowler commented Sep 9, 2022

Added some further changes to only enable the Passive DPS restore logic if specifically configured for an entity as there seems to be broader compatibility issues across the gamut of tuya devices.

This is currently only available for simple entity types (switch/number/select). Other entity types will require specific testing and can be extended at a later stage.

@sibowler
Copy link
Author

sibowler commented Sep 9, 2022

Still TODO: Update the documentation to match the new config entry

@besiktas97
Copy link

Are the changes merged in the master integration or do we need to wait for a new version in HACS for local tuya?

@sibowler
Copy link
Author

Are the changes merged in the master integration or do we need to wait for a new version in HACS for local tuya?

Not yet - Aiming to get this code/fix tested to make sure it addresses all the issues. If you could help test, that would be appreciated.

@fotis3d
Copy link

fotis3d commented Sep 10, 2022

@sibowler in order to test it we need to upgrade to the latest and place/replace the red lines you have here at those files right ?

Just to be sure :)

@sibowler
Copy link
Author

@sibowler in order to test it we need to upgrade to the latest and place/replace the red lines you have here at those files right ?

Essentially yep - You need to make sure all the changes are made that are highlighted by the red/green view.

@fotis3d
Copy link

fotis3d commented Sep 10, 2022

@sibowler in order to test it we need to upgrade to the latest and place/replace the red lines you have here at those files right ?

Essentially yep - You need to make sure all the changes are made that are highlighted by the red/green view.

I did that but maybe I made a mistake somewhere cause Local Tuya couldn't load on restart. I will recheck. Is it possible to upload somehow and share the changed files to just replace them and test?

@sibowler sibowler changed the title Implementing safe default for devices not requiring RESET - Fix for bug in 4.1.0 Implementing safe default for devices not requiring RESET/Passive functionality - Fix for bug in 4.1.0 Sep 10, 2022
@joe-sydney
Copy link

Seems to have fixed my issue where the fan turned on whenever HA was restarted.

@JLOFPV
Copy link

JLOFPV commented Sep 11, 2022

I just suffered a blackout again from my circuit breaker configured in your local after the Home Assistant update

@sibowler
Copy link
Author

I just suffered a blackout again from my circuit breaker configured in your local after the Home Assistant update

Is that after installing this PR? IF so, do you have any logs from around the incident?

@JLOFPV
Copy link

JLOFPV commented Sep 12, 2022

Acabo de sufrir un apagón nuevamente desde mi disyuntor configurado en su local después de la actualización de Home Assistant

¿Eso es después de instalar este PR? Si es así, ¿tiene algún registro de todo el incidente?

Forgive my ignorance. How can I extract a record to facilitate it?

@rospogrigio
Copy link
Owner

Will start testing this PR now on my setup. Why didn't you introduce the Passive option in cover platform too?

@sibowler
Copy link
Author

Will start testing this PR now on my setup. Why didn't you introduce the Passive option in cover platform too?

@rospogrigio - Thanks, that would be great.

Re the passive option for cover. I've disabled the passive functionality for covers in the 4.1.0 release as I this will need some specific testing (and cover devices that are passive) - unfortunately I don't have any Tuya cover devices, so the safer bet seemed to be to disable it for the moment. The framework is there though for when there's a need.

@rospogrigio
Copy link
Owner

Re the passive option for cover. I've disabled the passive functionality for covers in the 4.1.0 release as I this will need some specific testing (and cover devices that are passive) - unfortunately I don't have any Tuya cover devices, so the safer bet seemed to be to disable it for the moment. The framework is there though for when there's a need.

Ah ok, I see, thank you. Everything seems to run smoothly for the moment, I am planning to merge and release probably tomorrow. More feedbacks would be more than welcome in the meantime.

@fbloemhof
Copy link

Is it possible that this solves a part of the issues in #1012 ?

I would like to disable the state restoring after unavailable for fans but not sure where I should do this.

@pplucky
Copy link

pplucky commented Oct 9, 2022

Seems to have fixed my issue where the fan turned on whenever HA was restarted.

Same here, I had a fan which was turning itself on after every unavailability or HA restart. Fixed after manually implementing this PR.

@rospogrigio
Copy link
Owner

OK after positive feedbacks and me testing for some time without issues, let's merge and release this PR and see if it solves some problems for some users.

@rospogrigio rospogrigio merged commit 154b960 into rospogrigio:master Oct 11, 2022
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 this pull request may close these issues.

8 participants