zha-device-handlers icon indicating copy to clipboard operation
zha-device-handlers copied to clipboard

Disable schedule on Tuya/Saswell TRV (TS601/_TZE200_c88teujp)

Open blazewicz opened this issue 2 years ago • 7 comments

This work-around is intended to fix #1815. Its based on solution used in Zigbee2MQTT linked below:

https://github.com/Koenkk/zigbee-herdsman-converters/blob/master/converters/toZigbee.js#L5877:L5879

I've tested it locally with 6 devices and it fixed the issue.

blazewicz avatar Oct 09 '22 16:10 blazewicz

Codecov Report

Base: 80.00% // Head: 80.18% // Increases project coverage by +0.18% :tada:

Coverage data is based on head (77e45ba) compared to base (2292c4e). Patch coverage: 69.56% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1816      +/-   ##
==========================================
+ Coverage   80.00%   80.18%   +0.18%     
==========================================
  Files         239      240       +1     
  Lines        7430     7428       -2     
==========================================
+ Hits         5944     5956      +12     
+ Misses       1486     1472      -14     
Impacted Files Coverage Δ
zhaquirks/tuya/ts0601_trv_sas.py 66.12% <69.56%> (+11.58%) :arrow_up:
zhaquirks/tuya/air/ts0601_air_quality.py 100.00% <0.00%> (ø)
zhaquirks/gledopto/glc009.py 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov-commenter avatar Oct 09 '22 16:10 codecov-commenter

Pull Request Test Coverage Report for Build 3255727846

  • 16 of 23 (69.57%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 80.183%

Changes Missing Coverage Covered Lines Changed/Added Lines %
zhaquirks/tuya/ts0601_trv_sas.py 16 23 69.57%
<!-- Total: 16 23
Totals Coverage Status
Change from base Build 3234730911: 0.1%
Covered Lines: 5956
Relevant Lines: 7428

💛 - Coveralls

coveralls avatar Oct 09 '22 16:10 coveralls

For future references, these are the relevant lines:

  • https://github.com/Koenkk/zigbee-herdsman-converters/blob/96563772e92eb459bf52c10e241c3a725968bcab/converters/toZigbee.js#L5924-L5934

javicalle avatar Oct 15 '22 17:10 javicalle

Just to be sure.

Are that change indented to be applied to all the devices in that quirk?

That will be all these devices:

            ("_TZE200_c88teujp", "TS0601"),
            ("_TZE200_azqp6ssj", "TS0601"),
            ("_TZE200_yw7cahqs", "TS0601"),
            ("_TZE200_9gvruqf5", "TS0601"),
            ("_TZE200_zuhszj9s", "TS0601"),
            ("_TZE200_2ekuz3dz", "TS0601"),
            ("_TYST11_KGbxAXL2", "GbxAXL2"),
            ("_TYST11_c88teujp", "88teujp"),
            ("_TYST11_azqp6ssj", "zqp6ssj"),
            ("_TYST11_yw7cahqs", "w7cahqs"),
            ("_TYST11_9gvruqf5", "gvruqf5"),
            ("_TYST11_zuhszj9s", "uhszj9s"),

Fair point, I've assumed these were all clones.

All except these three share the same configuration in Z2M:

  • _TZE200_2ekuz3dz
  • _TYST11_azqp6ssj
  • _TYST11_9gvruqf5

Here's the list:

https://github.com/Koenkk/zigbee-herdsman-converters/blob/96563772e92eb459bf52c10e241c3a725968bcab/devices/saswell.js#L11:L22

The _TYST11_azqp6ssj seems to be yet another sibling of _TZE200_c88teujp according to https://zigbee.blakadder.com/RTX_ZB-RT1.html.

I couldn't find any information regarding _TYST11_9gvruqf5.

The _TZE200_2ekuz3dz seems to be a completely different device type and I think it should get a separate quirk. More on that device: https://zigbee.blakadder.com/Beok_TGR85-ZB.html

I have no way of testing them all of course.

blazewicz avatar Oct 16 '22 10:10 blazewicz

Here's the configuration for _TZE200_2ekuz3dz from Z2M:

https://github.com/Koenkk/zigbee-herdsman-converters/blob/177326f7af1c42cd5e2f89998a03effd042780c6/devices/tuya.js#L2803:L2840

blazewicz avatar Oct 16 '22 10:10 blazewicz

But... according to Z2M, your device also have some kind of scheduler:

  • https://github.com/Koenkk/zigbee-herdsman-converters/blob/96563772e92eb459bf52c10e241c3a725968bcab/devices/saswell.js#L34-L40

and:

        toZigbee: [tz.saswell_thermostat_current_heating_setpoint, ... , tz.tuya_thermostat_weekly_schedule],

javicalle avatar Oct 16 '22 21:10 javicalle

You have said that the scheduled was setted in the Tuya application, right?

But as the quirk can not manage the scheduled for the device you need to disable it always to prevent that undesired behavior when HA interacts with the device.

If HA could manage the scheduler (in a good manner) that quirk will not be needed.

Would that be more or less the purpose of the quirk?

javicalle avatar Oct 16 '22 21:10 javicalle

How to i disable schedule???

Quidamdk avatar Oct 18 '22 23:10 Quidamdk

You have said that the scheduled was setted in the Tuya application, right?

But as the quirk can not manage the scheduled for the device you need to disable it always to prevent that undesired behavior when HA interacts with the device.

If HA could manage the scheduler (in a good manner) that quirk will not be needed.

Would that be more or less the purpose of the quirk?

Yes, that is correct.

Whenever someone finds time and will to implement the full support for the schedules this quirk may be upgraded. Until then my patch makes it usable in the basic form.

blazewicz avatar Oct 19 '22 14:10 blazewicz

Mvh.Jørgen HansenDen 19. okt. 2022 kl. 16.43 skrev Krzysztof Błażewicz @.***>:

You have said that the scheduled was setted in the Tuya application, right? But as the quirk can not manage the scheduled for the device you need to disable it always to prevent that undesired behavior when HA interacts with the device. If HA could manage the scheduler (in a good manner) that quirk will not be needed. Would that be more or less the purpose of the quirk?

Yes, that is correct. Whenever someone finds time and will to implement the full support for the schedules this quirk may be upgraded. Until then my patch makes it usable in the basic form.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.***>

Quidamdk avatar Oct 19 '22 15:10 Quidamdk

Would be really great if this could be merged soon. We just hit 0°C and my thermostats are not turning on because of this.

jclsn avatar Nov 19 '22 17:11 jclsn

I feel you buddy, winter has come to Poland as well. I hope you'll be able to enjoy this patch soon, I've been using it ever since I've published this PR and it works fine.

blazewicz avatar Nov 19 '22 19:11 blazewicz

@blazewicz one question, the disable schedule is a 'one time' action, or is needed to call every time?

I was thinking that we can update the attribute in the device binding? That will be enough?

javicalle avatar Nov 20 '22 02:11 javicalle

@javicalle I understand your concerns and of course I'll try to help if any issue arise from my changes.

I was thinking that we can update the attribute in the device binding? That will be enough?

Thanks for the suggestion, It might be similar to the issue handled in ParksideTuyaValveManufCluster.bind():

https://github.com/zigpy/zha-device-handlers/blob/8d0bcc6e549e3a5c650ec0dadcbff9a35ae5e7e2/zhaquirks/tuya/ts0601_valve.py#L204:L213

    async def bind(self):
        """
        Bind cluster.

        When adding this device tuya gateway issues factory reset,
        we just need to reset the frost lock, because its state is unknown to us.
        """
        result = await super().bind()
        await self.write_attributes({self.attributes_by_name["frost_lock_reset"].id: 0})
        return result

I've moved schedule disable command to bind() method, the diff:

diff --git a/zhaquirks/tuya/ts0601_trv_sas.py b/zhaquirks/tuya/ts0601_trv_sas.py
index a5304a2..4cd5249 100644
--- a/zhaquirks/tuya/ts0601_trv_sas.py
+++ b/zhaquirks/tuya/ts0601_trv_sas.py
@@ -73,6 +73,15 @@ class ManufacturerThermostatCluster(TuyaManufClusterAttributes):
         SASSWEL_LOCAL_TEMP_ATTR: "local_temperature",
     }

+    async def bind(self):
+        """Bind cluster."""
+        # this quirk does not support programmig modes so we force the schedule mode to be always off
+        # more details: https://github.com/zigpy/zha-device-handlers/issues/1815
+        result = await super().bind()
+        _LOGGER.warning("ManufacturerThermostatCluster.bind()")
+        # await self.write_attributes({SASSWEL_SCHEDULE_ENABLE_ATTR: 0})
+        return result
+
     def _update_attribute(self, attrid, value):
         super()._update_attribute(attrid, value)
         if attrid in self.TEMPERATURE_ATTRS:
@@ -131,12 +140,10 @@ class ThermostatCluster(TuyaThermostatCluster):
             return {SASSWEL_HEATING_SETPOINT_ATTR: round(value / 10)}

         if attribute == "system_mode":
-            # this quirk does not support programmig modes so we force the schedule mode to be always off
-            # more details: https://github.com/zigpy/zha-device-handlers/issues/1815
             if value == self.SystemMode.Off:
-                return {SASSWEL_STATE_ATTR: 0, SASSWEL_SCHEDULE_ENABLE_ATTR: 0}
+                return {SASSWEL_STATE_ATTR: 0}
             if value == self.SystemMode.Heat:
-                return {SASSWEL_STATE_ATTR: 1, SASSWEL_SCHEDULE_ENABLE_ATTR: 0}
+                return {SASSWEL_STATE_ATTR: 1}


 class Thermostat_TYST11_c88teujp(TuyaThermostat):

Here's my testing procedure:

  1. Rollback to vanilla 0.0.85.
  2. Reconfigure 3 of my TRV's in ZHA (by adding the same device once again).
  3. Confirm that all TRV's are set to the default 16 °C according to the default schedule.
  4. Change temperature setpoint to 15 °C.
  5. Confirm that setpoint changes back to 16 °C automatically.
  6. Apply code changes, restart HA.
  7. Repeat points 2 through 4.

I've also added extra debug logs to confirm that request is sent to the device and proper response is received - it is. As strange as it looks, all devices properly report schedule state to be off, but the schedule is still active. Temperature setpoint eventually changes to 16 °C and a clock icon is displayed on the LED display, presumably indicating that current value is set by the schedule.

IMG_20221120_125016

I've tried adding a 3 second delay between call to .bind() and sending the request to disable the schedule and I've observed the same result. This is how I've implemented the delay:

diff --git a/zhaquirks/tuya/ts0601_trv_sas.py b/zhaquirks/tuya/ts0601_trv_sas.py
index 4cd5249..179ad1c 100644
--- a/zhaquirks/tuya/ts0601_trv_sas.py
+++ b/zhaquirks/tuya/ts0601_trv_sas.py
@@ -1,5 +1,6 @@
 """Saswell (Tuya whitelabel) 88teujp thermostat valve quirk."""

+import asyncio
 import logging

 from zigpy.profiles import zha
@@ -77,9 +78,16 @@ class ManufacturerThermostatCluster(TuyaManufClusterAttributes):
         """Bind cluster."""
         # this quirk does not support programmig modes so we force the schedule mode to be always off
         # more details: https://github.com/zigpy/zha-device-handlers/issues/1815
+        _LOGGER.warning("ManufacturerThermostatCluster.bind(): starting bind procedure")
         result = await super().bind()
-        _LOGGER.warning("ManufacturerThermostatCluster.bind()")
-        # await self.write_attributes({SASSWEL_SCHEDULE_ENABLE_ATTR: 0})
+
+        async def disable_schedule():
+            await asyncio.sleep(3)
+            _LOGGER.warning("ManufacturerThermostatCluster.bind(): requesting schedule disable")
+            await self.write_attributes({SASSWEL_SCHEDULE_ENABLE_ATTR: 0})
+
+        asyncio.create_task(disable_schedule())
+
         return result

     def _update_attribute(self, attrid, value):
@@ -100,6 +108,8 @@ class ManufacturerThermostatCluster(TuyaManufClusterAttributes):
             self.endpoint.device.battery_bus.listener_event(
                 "battery_change", 0 if value == 1 else 100
             )
+        elif attrid == SASSWEL_SCHEDULE_ENABLE_ATTR:
+            _LOGGER.warning("Reported schedule state change: %s", value)


 class ThermostatCluster(TuyaThermostatCluster):

Then I've rolled back to the version from this PR, restarted HA (reconfigure is not required) and confirmed schedule is disabled on all TRV's.

IMG_20221120_125640

blazewicz avatar Nov 20 '22 11:11 blazewicz

Thanks for the work here!

dmulcahey avatar Nov 20 '22 12:11 dmulcahey

Thanks for testing.

javicalle avatar Nov 20 '22 17:11 javicalle

Thanks from me as well. Can't wait! :)

jclsn avatar Nov 23 '22 10:11 jclsn

Just to be sure. Are that change indented to be applied to all the devices in that quirk? That will be all these devices:

            ("_TZE200_c88teujp", "TS0601"),
            ("_TZE200_azqp6ssj", "TS0601"),
            ("_TZE200_yw7cahqs", "TS0601"),
            ("_TZE200_9gvruqf5", "TS0601"),
            ("_TZE200_zuhszj9s", "TS0601"),
            ("_TZE200_2ekuz3dz", "TS0601"),
            ("_TYST11_KGbxAXL2", "GbxAXL2"),
            ("_TYST11_c88teujp", "88teujp"),
            ("_TYST11_azqp6ssj", "zqp6ssj"),
            ("_TYST11_yw7cahqs", "w7cahqs"),
            ("_TYST11_9gvruqf5", "gvruqf5"),
            ("_TYST11_zuhszj9s", "uhszj9s"),

Fair point, I've assumed these were all clones.

All except these three share the same configuration in Z2M:

  • _TZE200_2ekuz3dz
  • _TYST11_azqp6ssj
  • _TYST11_9gvruqf5

Here's the list:

https://github.com/Koenkk/zigbee-herdsman-converters/blob/96563772e92eb459bf52c10e241c3a725968bcab/devices/saswell.js#L11:L22

The _TYST11_azqp6ssj seems to be yet another sibling of _TZE200_c88teujp according to https://zigbee.blakadder.com/RTX_ZB-RT1.html.

I couldn't find any information regarding _TYST11_9gvruqf5.

The _TZE200_2ekuz3dz seems to be a completely different device type and I think it should get a separate quirk. More on that device: https://zigbee.blakadder.com/Beok_TGR85-ZB.html

I have no way of testing them all of course.

Unfortunately 2ekuz3dz is not working at all using this quirk. Not a single entity is available.

Hilpas avatar Dec 09 '22 09:12 Hilpas

Unfortunately 2ekuz3dz is not working at all using this quirk. Not a single entity is available.

@Hilpas thats probably because of missing output clusters, like I said, this one should have a separate quirk on its own since it's not a TRV.

Could you provide a list of supported clusters for this device?

blazewicz avatar Dec 09 '22 09:12 blazewicz

Unfortunately 2ekuz3dz is not working at all using this quirk. Not a single entity is available.

@Hilpas thats probably because of missing output clusters, like I said, this one should have a separate quirk on its own since it's not a TRV.

Could you provide a list of supported clusters for this device?

This might help, otherwise i could try to provide some Information later. https://github.com/zigpy/zha-device-handlers/issues/1721

Hilpas avatar Dec 09 '22 10:12 Hilpas

But #1721 was opened on August 30-th, almost 3 months before this PR was merged in.

blazewicz avatar Dec 09 '22 10:12 blazewicz

My device signaturen is exactly the same though. https://github.com/zigpy/zha-device-handlers/pull/1609 Using this quirk i can at least read temperatures no control though.

Hilpas avatar Dec 11 '22 20:12 Hilpas