tuya / tuya-device-sharing-sdk

Tuya Device Sharing SDK
MIT License
19 stars 6 forks source link

Include list of updated status properties in SharingDeviceListener.update_device #17

Closed lanthaler closed 3 months ago

lanthaler commented 3 months ago

In the current form, no information what has been updated is shared with SharingDeviceListener. This makes it impossible to implement event-based integrations as required by switches for instance as it is unclear which state the device reported if it didn't change.

As an example, a switch such as the one depicted below which implements the instruction set described at https://developer.tuya.com/en/docs/iot/s?id=Kbeoa9fkv6brp can't be implemented as it is impossible to detect events on switch_mode1 if the user just keeps clicking (click value) instead of alternating between click and press.

switch

lanthaler commented 3 months ago

@shihuimiao made this PR target the dev branch instead as I saw that 0.2.0 has already been pushed to pypi. Let me know if you want me to target the main branch instead.

It would be great if this could be pushed as I have a Home Assistant change that adds support for switch devices that depends on this. Thanks

yuqiufeng commented 3 months ago

In the current form, no information what has been updated is shared with SharingDeviceListener. This makes it impossible to implement event-based integrations as required by switches for instance as it is unclear which state the device reported if it didn't change.

As an example, a switch such as the one depicted below which implements the instruction set described at https://developer.tuya.com/en/docs/iot/s?id=Kbeoa9fkv6brp can't be implemented as it is impossible to detect events on switch_mode1 if the user just keeps clicking (click value) instead of alternating between click and press.

switch

First of all,Thank you for your PR. We understand that the logic of your submitted PR is to return a list of specific device status codes for the changes, in addition to returning the latest status of the device when a status change is detected. However, we do not understand where exactly you intend to utilize the list of device status codes at the upper level application. If possible, please describe a specific use case so that we can provide a clearer and more significant change when push the PR to the HA official project.

lanthaler commented 3 months ago

Thanks for the review @yuqiufeng. I have a pull request for Home Assistant out which would need this (I currently patched the class there till this change is merged).

When you look at that change, you'll see that I updated the DeviceListener to send the list of updated status properties. This is necessary to be able to fire the right event in the newly added support for Event entities.

Without this change, all I could do is look at the current state (and potentially compare it to the previous state). The problem is that it might look exactly the same as the previous one as the value of switch_mode1 would remain click (and all other entries wouldn't change either) if the user just keeps clicking. So I have no way to figure out which button the user pressed on the remote switch.

Please let me know if anything is still unclear

yuqiufeng commented 3 months ago

Thanks for the review @yuqiufeng. I have a pull request for Home Assistant out which would need this (I currently patched the class there till this change is merged).

When you look at that change, you'll see that I updated the DeviceListener to send the list of updated status properties. This is necessary to be able to fire the right event in the newly added support for Event entities.

Without this change, all I could do is look at the current state (and potentially compare it to the previous state). The problem is that it might look exactly the same as the previous one as the value of switch_mode1 would remain click (and all other entries wouldn't change either) if the user just keeps clicking. So I have no way to figure out which button the user pressed on the remote switch.

Please let me know if anything is still unclear

Alright, we understand that this PR provides good support for extending HA's Event entities. This week, we will update a version and push it to the PyPI repository.

yuqiufeng commented 3 months ago

Thanks for the review @yuqiufeng. I have a pull request for Home Assistant out which would need this (I currently patched the class there till this change is merged). When you look at that change, you'll see that I updated the DeviceListener to send the list of updated status properties. This is necessary to be able to fire the right event in the newly added support for Event entities. Without this change, all I could do is look at the current state (and potentially compare it to the previous state). The problem is that it might look exactly the same as the previous one as the value of switch_mode1 would remain click (and all other entries wouldn't change either) if the user just keeps clicking. So I have no way to figure out which button the user pressed on the remote switch. Please let me know if anything is still unclear

Alright, we understand that this PR provides good support for extending HA's Event entities. This week, we will update a version and push it to the PyPI repository.

The new version has been released, with the version number being: 0.2.1

lanthaler commented 3 months ago

Awesome, thanks so much @yuqiufeng