zha-device-handlers: [BUG] / Request send default response in wrong direction from quirk.

Describe the bug Our beloved TS004F Dimmer switch is re sending commands also after sending default response to it. Its described in this PR https://github.com/zigpy/zha-device-handlers/pull/1437.

To Reproduce Have sniffing tuya ZBGW and they is doing the same

Expected behavior Default response shall working OK but the device is doing all possible things wrong and this is one firmware bug. Reading little more i was finding that Smart things is sending default response for EP2-4 like this:

Frame Control Field: Profile-wide (0x00)
    .... ..00 = Frame Type: Profile-wide (0x0)
    .... .0.. = Manufacturer Specific: False
    .... 0... = Direction: Client to Server
    ...0 .... = Disable Default Response: False

And Zigbee standard shall being like this and tuya ZBGW and ZHA is doing it so:

Frame Control Field: Profile-wide (0x18)
    .... ..00 = Frame Type: Profile-wide (0x0)
    .... .0.. = Manufacturer Specific: False
    .... 1... = Direction: Server to Client
    ...1 .... = Disable Default Response: True

ST is doing one thing wrong you is not sending one default response for one default response but there is sending it server to client and the device is very happy and is not re sending the command and is sending the next without delay.

Screenshots If applicable, add screenshots to help explain your problem.

Additional context

Z2M look have implanting it and looks working OK but they is sending 4 DR.

I like getting hep implanting sending “revers” DR from quirks (force Client to Server and perhaps in the future the opposite if needed) so we can getting the device responding for multiple commands without delay (is now 3 seconds between commands).

One funny but not unexpected thing is that the device is triggering reporting battery then pressing the button for EP2 and its start reporting battery on EP1 and getting one normal DR and its OK and then sending the “on” command from EP2 without delay but is being repeated then the device like having it in the reversed direction (Client to Server and not the standard Server to Client).

Sniff of attribute reported the pressing button 2 and reporting battery attribute and the command:

EP2 Event:
First its reporting battery from EP1:

ZigBee Network Layer Data, Dst: 0x0000, Src: 0xc29e
ZigBee Application Support Layer Data, Dst Endpt: 1, Src Endpt: 1
ZigBee Cluster Library Frame, Command: Report Attributes, Seq: 73
    Frame Control Field: Profile-wide (0x08)
        .... ..00 = Frame Type: Profile-wide (0x0)
        .... .0.. = Manufacturer Specific: False
        .... 1... = Direction: Server to Client
        ...0 .... = Disable Default Response: False
    Sequence Number: 73
    Command: Report Attributes (0x0a)
    Attribute Field
        Attribute: Battery Percentage Remaining (0x0021)
        Data Type: 8-Bit Unsigned Integer (0x20)
        Remaining Battery Percentage: 94,0 [%]


ZigBee Network Layer Data, Dst: 0x0000, Src: 0xc29e
ZigBee Application Support Layer Data, Dst Endpt: 1, Src Endpt: 1
ZigBee Cluster Library Frame, Command: Report Attributes, Seq: 73
    Frame Control Field: Profile-wide (0x08)
        .... ..00 = Frame Type: Profile-wide (0x0)
        .... .0.. = Manufacturer Specific: False
        .... 1... = Direction: Server to Client
        ...0 .... = Disable Default Response: False
    Sequence Number: 73
    Command: Report Attributes (0x0a)
    Attribute Field
        Attribute: Battery Percentage Remaining (0x0021)
        Data Type: 8-Bit Unsigned Integer (0x20)
        Remaining Battery Percentage: 94,0 [%]


Then is sending the command from EP2:

ZigBee Network Layer Data, Dst: 0x0000, Src: 0xc29e
ZigBee Application Support Layer Data, Dst Endpt: 1, Src Endpt: 2
ZigBee Cluster Library Frame
    Frame Control Field: Cluster-specific (0x01)
        .... ..01 = Frame Type: Cluster-specific (0x1)
        .... .0.. = Manufacturer Specific: False
        .... 0... = Direction: Client to Server
        ...0 .... = Disable Default Response: False
    Sequence Number: 74
    Command: Unknown (0xfd)
    Payload


ZigBee Network Layer Data, Dst: 0xc29e, Src: 0x0000
ZigBee Application Support Layer Data, Dst Endpt: 2, Src Endpt: 2
ZigBee Cluster Library Frame, Command: Default Response, Seq: 74
    Frame Control Field: Profile-wide (0x18)
        .... ..00 = Frame Type: Profile-wide (0x0)
        .... .0.. = Manufacturer Specific: False
        .... 1... = Direction: Server to Client
        ...1 .... = Disable Default Response: True
    Sequence Number: 74
    Command: Default Response (0x0b)
    Response to Command: 0xfd
    Status: Success (0x00)

I think we have finding the 10 bugs / bad / not working faulty things in this device but perhaps we is finding more in the future.

About this issue

Original URL
State: open
Created 2 years ago
Comments: 19 (11 by maintainers)

Most upvoted comments

I know you are doing a lots of tests and changes and I don’t want to generate any additional noise to them. There is no hurry, but when you test if you can get logs from the retries I will take another look. I believe that the current code have a couple more of interesting debug logs.

javicalle on Mar 25, 2023

Another round to check the repeated responses.

If I have understood correctly the device expects the response to the commands to have the direction reversed. I think it would be possible to override it as follows (just added the command function):

TuyaSmartRemoteOnOffCluster

class TuyaSmartRemoteOnOffCluster(OnOff, EventableCluster):
    """TuyaSmartRemoteOnOffCluster: fire events corresponding to press type."""

    rotate_type = {
        0x00: RIGHT,
        0x01: LEFT,
        0x02: STOP,
    }
    press_type = {
        0x00: SHORT_PRESS,
        0x01: DOUBLE_PRESS,
        0x02: LONG_PRESS,
    }
    name = "TS004X_cluster"
    ep_attribute = "TS004X_cluster"
    attributes = OnOff.attributes.copy()
    attributes.update({0x8001: ("backlight_mode", SwitchBackLight)})
    attributes.update({0x8002: ("power_on_state", PowerOnState)})
    attributes.update({0x8004: ("switch_mode", SwitchMode)})

    def __init__(self, *args, **kwargs):
        """Init."""
        self.last_tsn = -1
        super().__init__(*args, **kwargs)

    server_commands = OnOff.server_commands.copy()
    server_commands.update(
        {
            0xFC: foundation.ZCLCommandDef(
                "rotate_type",
                {"rotate_type": t.uint8_t},
                False,
                is_manufacturer_specific=True,
            ),
            0xFD: foundation.ZCLCommandDef(
                "press_type",
                {"press_type": t.uint8_t},
                False,
                is_manufacturer_specific=True,
            ),
        }
    )

    def handle_cluster_request(
        self,
        hdr: foundation.ZCLHeader,
        args: List[Any],
        *,
        dst_addressing: Optional[
            Union[t.Addressing.Group, t.Addressing.IEEE, t.Addressing.NWK]
        ] = None,
    ):
        """Handle press_types command."""
        # normally if default response sent, TS004x wouldn't send such repeated zclframe (with same sequence number),
        # but for stability reasons (e. g. the case the response doesn't arrive the device), we can simply ignore it
        if hdr.tsn == self.last_tsn:
            _LOGGER.debug("TS004X: ignoring duplicate frame")
            return
        # save last sequence number
        self.last_tsn = hdr.tsn

        # send default response (as soon as possible), so avoid repeated zclframe from device
        if not hdr.frame_control.disable_default_response:
            self.debug("TS004X: send default response")
            self.send_default_rsp(hdr, status=foundation.Status.SUCCESS)
        # handle command
        if hdr.command_id == 0xFC:
            rotate_type = args[0]
            self.listener_event(
                ZHA_SEND_EVENT, self.rotate_type.get(rotate_type, "unknown"), []
            )
        elif hdr.command_id == 0xFD:
            press_type = args[0]
            self.listener_event(
                ZHA_SEND_EVENT, self.press_type.get(press_type, "unknown"), []
            )

    async def command(
        self,
        command_id: Union[foundation.GeneralCommand, int, t.uint8_t],
        *args,
        manufacturer: Optional[Union[int, t.uint16_t]] = None,
        expect_reply: bool = True,
        tsn: Optional[Union[int, t.uint8_t]] = None,
        **kwargs: Any,
    ):
        """Override the default Cluster command."""
        self.debug(
            "Inverting response direction. Cluster Command is %x, Arguments are %s - %s",
            command_id,
            args,
            kwargs,
        )

        def_response = await super().command(
            command_id, *args, manufacturer, expect_reply, tsn, **kwargs
        )
        self.debug(
            "Command response: %s",
            def_response,
        )

        def_response.direction=foundation.Direction.Server_to_Client

        return def_response

Totally untested and just trying things. If needed we can also overwrite the `expect_reply` variable.

javicalle on Mar 26, 2023

Sorry i was decalking my kaffe machines and was getting water in the power connection and the apartment was shutting down the power so have running around and doing all clocks one more time and helping some Zigbee devices that was having problems (only 3 of round 100). This 2 lines is the interesting:

2023-03-26 10:49:51.877 DEBUG (MainThread) [zigpy.zcl] [0x6E40:3:0x0006] Sending reply header: ZCLHeader(frame_control=FrameControl(frame_type=<FrameType.GLOBAL_COMMAND: 0>, is_manufacturer_specific=False, direction=<Direction.Client_to_Server: 1>, disable_default_response=1, reserved=0, *is_cluster=False, *is_general=True, *is_reply=True), tsn=118, command_id=<GeneralCommand.Default_Response: 11>, *direction=<Direction.Client_to_Server: 1>, *is_reply=True)
2023-03-26 10:49:51.880 DEBUG (MainThread) [zigpy.zcl] [0x6E40:3:0x0006] Sending reply: Default_Response(command_id=253, status=<Status.SUCCESS: 0>)

And this is that parameter that need being 0 and not 1 *direction=<Direction.Client_to_Server: 1>.

I have not testing your last code then i need doing changes in the HA container and its taking little time doing right and not braking HA (if copy the INIT to the wrong folder).

I doing it later or tomorrow and reporting back 😃))

MattWestb on Mar 26, 2023