[BUG] Tuya/MEOS Thermostat - the time gets unsynchronized making built-in schedules unusable.

sstefanowski commented 3 years ago

The problem: The Tuya/MEOS Thermostat support built-in heating schedules (and change temperature up to 6 times a day). Unfortunately, the clock inside the device is not very precise and the time gets unsynchronized after a few days. ZHA should take care of the device time and re-synchronize it with some default frequency (e.g. once a day). The device should always receive a local date/time from ZHA. This also should be taken into consideration when summer/winter time changes

Similar Zigbee solutions take care of the device clock e.g. Tuya platform and Zigbee2MQT.\ The solutions for the same issue in Zigbee2MQTT: https://github.com/Koenkk/zigbee2mqtt/issues/5916 https://github.com/Koenkk/zigbee2mqtt/issues/5620

To Reproduce Steps to reproduce the behaviour:

Set a correct time on the Tuya/MEOS Thermostat device manually
Leave it for a few days
Check the time on the device after a few days
The time on the device is no longer correct

Expected behaviour When checking the time on the device it should be always correct. ZHA should take care of date/time settings and re-synchronize them periodically as the other Zigee platforms do. The device should always receive a local date/time from ZHA. This also should be taken into consideration when summer/winter time changes

Screenshots N/A

Additional context N/A

Adminiuga commented 3 years ago

Tuya should have just sticked to the standards. Nominally nthe device is a zigbee device, but really is using a proprietary protocol.

sstefanowski commented 3 years ago

Yeah, I understand this is a nasty thing, but.... are we going to do something with it?

Edit no 1. I just did another test with strange results... Another strange thing is that I just did a re-pair test of the Thermostat and it got a strange time, so it looks like re-pairing with ZHA flashes some time to the device but the time is strange...

Scenario: I removed Thermostat from the ZHA network at 6 PM and I changed the time to 9 AM manually on the device. After re-paring, the Thermostat got strange time from ZHA, 3 PM (3 hours late to my local time - I'm in CET (GMT+2) and the thermostat set GMT-1)

. Isn't it strange?

Edit no 2. Wooow.... It's even worse. Now, whenever I change the time manually on the device to 6PM it gets back to 3PM automatically. looks like it's syncing with ZHA time but.... incorrectly(!) - 3 hour late to local time.

Edit no 3. Now it's getting very strange the thermostat, which was recently re-paired got somehow local time, it's 6 PM on this device (I've restarted HA nad Sonoff Zigbee bridge with Tasmota in meantime). But the other thermostats still have incorrect time. most of them are 3h late.

Any other tests I could do?

mallorca2288 commented 3 years ago

@sstefanowski What thermostat model do you have? I have a _TZE200_aoclfnxz thermostat and I'm having the same problem.

I'm not sure, but I believe that the command to set the time from ZHA to the device is already in the code, but it's only applied to some thermostatic valves and not to all tuya thermostats.

mallorca2288 commented 3 years ago

I was looking at the code and I think I've fixed it for my device. I've just added this line of code: set_time_offset = 1970 in this line

So now the code looks like this:

class MoesBHTThermostat(TuyaThermostatCluster):
    """Thermostat cluster for some electric heating controllers."""

    set_time_offset = 1970

    def map_attribute(self, attribute, value):
        """Map standardized attribute value to dict of manufacturer values."""

I waited a few minutes until the thermostat asked for a time sync to the server: 2021-04-01 00:30:35 DEBUG (MainThread) [zhaquirks.tuya] [0xd829:1:0xef00] Got set time request (command 0x0024)

After that, I checked the thermostat and the time was correct. I'll keep an eye on it for the next few days to see if the problem was solved.

sstefanowski commented 3 years ago

BTW. My Thermostat is heater valve _TZE200_ywdxldoj

OK. I can see the code and it looks like this handler should work but it is not working...

I enabled debug in configuration.yaml yesterday and... NOT a single log entry with [zhaquirks.tuya] [0xd829:1:0xef00] Got set time request (command 0x0024) was generated. I can see just updated temperature readings generated by[zhaquirks.tuya]

but... I can see periodic ERRORS in a log as follows:

Mar 31 23:52:23 raspberrypi hass[19459]: 2021-03-31 23:52:23 ERROR (MainThread) [zigpy.device] Failed to parse message (b'191c24004b') on cluster 61184, because Data is too short to contain 1 bytes
Mar 31 23:52:50 raspberrypi hass[19459]: 2021-03-31 23:52:50 ERROR (MainThread) [zigpy.device] Failed to parse message (b'195324004c') on cluster 61184, because Data is too short to contain 1 bytes
Mar 31 23:52:54 raspberrypi hass[19459]: 2021-03-31 23:52:54 ERROR (MainThread) [zigpy.device] Failed to parse message (b'1913240058') on cluster 61184, because Data is too short to contain 1 bytes
(...)
Mar 31 23:53:57 raspberrypi hass[19459]: 2021-03-31 23:53:57 ERROR (MainThread) [zigpy.device] Failed to parse message (b'19562403ea') on cluster 61184, because Data is too short to contain 1 bytes
(...)
Apr  1 00:52:49 raspberrypi hass[19459]: 2021-04-01 00:52:49 ERROR (MainThread) [zigpy.device] Failed to parse message (b'192124004e') on cluster 61184, because Data is too short to contain 1 bytes
Apr  1 00:53:21 raspberrypi hass[19459]: 2021-04-01 00:53:21 ERROR (MainThread) [zigpy.device] Failed to parse message (b'195b240050') on cluster 61184, because Data is too short to contain 1 bytes
Apr  1 00:53:26 raspberrypi hass[19459]: 2021-04-01 00:53:26 ERROR (MainThread) [zigpy.device] Failed to parse message (b'1926240062') on cluster 61184, because Data is too short to contain 1 bytes

and 61184=0xef00 (MeosManufCluster) and all the messages listed contain static (b'19??24????') while 0x0024is set_time_requestcommand in this cluster.

Can these ERRORS be related to missing time sync? I think we are close to the solution, but I don't know how to fix it...

Isn't it something wrong with set_time_request command definition? Another custom behaviour of Tuya or just a bug?

https://github.com/zigpy/zha-device-handlers/blob/2f9b0c33e08dfbddb703e44ce017e0a0a9315562/zhaquirks/tuya/__init__.py#L69

https://github.com/zigpy/zha-device-handlers/blob/2f9b0c33e08dfbddb703e44ce017e0a0a9315562/zhaquirks/tuya/__init__.py#L96

the comment in the code suggests that the size of the payload of set_time_request sent from the device should be 0

    """ Time sync command (It's transparent between MCU and server)
            Time request device -> server
               payloadSize = 0

while set_time_request definition indicates some TuyaTimePayload. Isn't it a bug causing deserialization error message ...because Data is too short to contain 1 bytes?

    class Command(t.Struct):
        """Tuya manufacturer cluster command."""

        status: t.uint8_t
        tsn: t.uint8_t
        command_id: t.uint16_t
        function: t.uint8_t
        data: Data
(...)
    manufacturer_client_commands = {
        0x0001: ("get_data", (Command,), True),
        0x0002: ("set_data_response", (Command,), True),
        0x0024: ("set_time_request", (TuyaTimePayload,), True),
    }
(...)

Any hints how to fix it?

sstefanowski commented 3 years ago

Update 1: I've changed a line https://github.com/zigpy/zha-device-handlers/blob/2f9b0c33e08dfbddb703e44ce017e0a0a9315562/zhaquirks/tuya/__init__.py#L135

to

0x0024: ("set_time_request", (t.NoData,), True),

and now... I can see in the log Apr 01 11:00:37 raspberrypi hass[21528]: 2021-04-01 11:00:37 DEBUG (MainThread) [zhaquirks.tuya] [0xd17c:1:0xef00] Got set time request (command 0x0024) and one of my thermostats has valid time. I'll recheck remaining devices after some time.

Question to @Adminiuga and @xonestonex who is the main contributor to the ZHA quirk for Moes Thermostatic valve. What do you think? Is this a proper way of fixing this?

Update 2: Note: After applying a change all my MEOS Thermostat valves got the correct time. So there was a bug with using TuyaTime Payload for set_time_request. but using t.NoData is not a good solution as it generates the following WARNING in a log:

Apr  2 00:04:31 raspberrypi hass[21528]: 2021-04-02 00:04:31 WARNING (MainThread) [zigpy.zcl] [0xcc53:1:0xef00] Data remains after deserializing ZCL frame
Apr  2 00:04:31 raspberrypi hass[21528]: 2021-04-02 00:04:31 DEBUG (MainThread) [zhaquirks.tuya] [0xcc53:1:0xef00] Got set time request (command 0x0024)

So I changed it to t.data16and the WARNING is gone. Now it's just handling the set_time_request correctly. So the final version of the line https://github.com/zigpy/zha-device-handlers/blob/dev/zhaquirks/tuya/__init__.py#L135 is as follows: 0x0024: ("set_time_request", (t.data16,), True),

I usedt.data16 as I don't know the meaning of the payload (spec says it should be 0 and... I can see the payload is ignored in 0x0024 command handler. I just wanted the length of the payload to be correct.

I don't know if this is a fully correct fix. but having this is FAR more accurate than having a BUG and unsynchronized time.

I have no code branched. Can any developer implement this change and create a pull request for this, so we could have this in next release?

almico commented 3 years ago

I too have the _TZE200_ywdxldoj and would really love to see this fix applied! Please, please, please 😄

almico commented 3 years ago

I can confirm that changing line 135 to 0x0024: ("set_time_request", (t.data16,), True), worked here too. Thank you!

almico commented 3 years ago

I just received a new HY368 and its name is _TZE200_cwnjrr72...

MattWestb commented 3 years ago

I was adding it in https://github.com/zigpy/zha-device-handlers/pull/820 if the user putting all info OK it shall working out of the box.

sstefanowski commented 3 years ago

Hi @MattWestb, I wonder.... If you have a source code already branched maybe you could update line 135 above to fix the time synchronization and create pull-request so this could to be analyzed by main ZHA contributors?

sstefanowski commented 3 years ago

Hi @MattWestb,

Thanks for an update but...

I can see you committed: 0x0024: ("set_time_request", (t.NoData,), True), while this setting generates a WARN in the log about unconsumed payload in a buffer. t.NoData means that it's not expecting any data but there are unconsumed bytes remaining (I guess 2 bytes) in the request.

Changing it to ("set_time_request", (t.data16,), True), makes everything working without a WARN - remaining last bytes are consumed as expected but just not "interpreted".

Also, I don't meaning of these bytes but the previous entry using TuyaTimePayload for set_time_request was a real bug.

BTW. Maybe I should comment on a pull request directly what do you think?

sstefanowski commented 3 years ago

OK, I'll comment on Pull Request I can justify the commit

MattWestb commented 3 years ago

Updated PR

sstefanowski commented 3 years ago

@MattWestb, Stupid question, how to install this commit? Do you mean just relacing the local file with the file from your repo?

MattWestb commented 3 years ago

Download the commit as zip file or copy only the file changed and putting it in your system so we can see its working.

sstefanowski commented 3 years ago

This is in release 0,57 so we can close the issue...

zigpy / zha-device-handlers

[BUG] Tuya/MEOS Thermostat - the time gets unsynchronized making built-in schedules unusable. #831