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

Cannot read status on SIN-4-FP-21_EQU #25333

Closed
vmeurisse opened this issue Dec 26, 2024 · 1 comment · Fixed by Koenkk/zigbee-herdsman#1274 or Koenkk/zigbee-herdsman-converters#8535
Closed
Labels
problem Something isn't working

Comments

@vmeurisse
Copy link

What happened?

After upgrade from 1.36.1 to 1.42.0 zigbe2mqtt is unable to read the pilot wire mode of SIN-4-FP-21_EQU.

What did you expect to happen?

No response

How to reproduce it (minimal and precise)

In device settings, disable "optimistic" mode. Change the pilot wire mode.
The physical device will change mode, but the reported mode won't change.

Zigbee2MQTT version

1.42.0

Adapter firmware version

20210708

Adapter

Sonoff Zigbee 3.0 USB Dongle Plus

Setup

add-on on home assistant on a raspbery pi

Debug log

[2024-12-26 12:39:43] �[34mdebug�[39m: 	z2m:mqtt: Received MQTT message on 'zigbee2mqtt/fp_control/get' with data '{"pilot_wire_mode":""}'
[2024-12-26 12:39:43] �[34mdebug�[39m: 	z2m: Publishing get 'get' 'pilot_wire_mode' to 'fp_control'
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:controller:endpoint: ZCL command 0x4c5bb3fffeb7fe7b/1 manuSpecificNodOnPilotWire.read([0], {"timeout":10000,"disableResponse":false,"disableRecovery":false,"disableDefaultResponse":true,"direction":0,"reservedBits":0,"manufacturerCode":4747,"writeUndiv":false})
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack: sendZclFrameToEndpointInternal 0x4c5bb3fffeb7fe7b:1483/1 (0,0,1)
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:znp: --> SREQ: AF - dataRequest - {"dstaddr":1483,"destendpoint":1,"srcendpoint":1,"clusterid":64512,"transid":171,"options":0,"radius":30,"len":7,"data":{"type":"Buffer","data":[20,139,18,5,0,0,0]}}
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:unpi:writer: --> frame [254,17,36,1,203,5,1,1,0,252,171,0,30,7,20,139,18,5,0,0,0,60]
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:unpi:parser: <-- [254,1,100,1,0,100]
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:unpi:parser: --- parseNext [254,1,100,1,0,100]
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:unpi:parser: --> parsed 1 - 3 - 4 - 1 - [0] - 100
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:znp: <-- SRSP: AF - dataRequest - {"status":0}
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:unpi:parser: --- parseNext []
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:unpi:parser: <-- [254,3,68,128,0,1,171,109]
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:unpi:parser: --- parseNext [254,3,68,128,0,1,171,109]
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:unpi:parser: --> parsed 3 - 2 - 4 - 128 - [0,1,171] - 109
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:znp: <-- AREQ: AF - dataConfirm - {"status":0,"endpoint":1,"transid":171}
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:unpi:parser: --- parseNext []
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:unpi:parser: <-- [254,3,69,196,203,5,0,76]
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:unpi:parser: --- parseNext [254,3,69,196,203,5,0,76]
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:unpi:parser: --> parsed 3 - 2 - 5 - 196 - [203,5,0] - 76
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:znp: <-- AREQ: ZDO - srcRtgInd
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:unpi:parser: --- parseNext []
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:unpi:parser: <-- [254,30,68,129,0,0,0,252,203,5,1,1,0,135,0,4,197,122,0,0,10,28,139,18,5,1,0,0,0,32,1,203,5,29,172]
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:unpi:parser: --- parseNext [254,30,68,129,0,0,0,252,203,5,1,1,0,135,0,4,197,122,0,0,10,28,139,18,5,1,0,0,0,32,1,203,5,29,172]
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:unpi:parser: --> parsed 30 - 2 - 4 - 129 - [0,0,0,252,203,5,1,1,0,135,0,4,197,122,0,0,10,28,139,18,5,1,0,0,0,32,1,203,5,29] - 172
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:znp: <-- AREQ: AF - incomingMsg - {"groupid":0,"clusterid":64512,"srcaddr":1483,"srcendpoint":1,"dstendpoint":1,"wasbroadcast":0,"linkquality":135,"securityuse":0,"timestamp":8045828,"transseqnumber":0,"len":10,"data":{"type":"Buffer","data":[28,139,18,5,1,0,0,0,32,1]}}
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:controller: Received payload: clusterID=64512, address=1483, groupID=0, endpoint=1, destinationEndpoint=1, wasBroadcast=false, linkQuality=135, frame={"header":{"frameControl":{"frameType":0,"manufacturerSpecific":true,"direction":1,"disableDefaultResponse":true,"reservedBits":0},"manufacturerCode":4747,"transactionSequenceNumber":5,"commandIdentifier":1},"payload":[{"attrId":0,"status":0,"dataType":32,"attrData":1}],"command":{"ID":1,"name":"readRsp","parameters":[{"name":"attrId","type":33},{"name":"status","type":32},{"name":"dataType","type":32,"conditions":[{"type":"statusEquals","value":0}]},{"name":"attrData","type":1000,"conditions":[{"type":"statusEquals","value":0}]}]}}
[2024-12-26 12:39:43] �[34mdebug�[39m: 	zh:zstack:unpi:parser: --- parseNext []
[2024-12-26 12:39:43] �[34mdebug�[39m: 	z2m: Received Zigbee message from 'fp_control', type 'readResponse', cluster 'manuSpecificAssaDoorLock', data '{"0":1}' from endpoint 1 with groupID 0
[2024-12-26 12:39:43] �[34mdebug�[39m: 	z2m: No converter available for 'SIN-4-FP-21_EQU' with cluster 'manuSpecificAssaDoorLock' and type 'readResponse' and data '{"0":1}'
@vmeurisse vmeurisse added the problem Something isn't working label Dec 26, 2024
@vmeurisse
Copy link
Author

The issue seems to come from Koenkk/zigbee-herdsman@7316247#diff-bc89a730c378c181a37d00497b5e8bf42f3bc8da7224200964770ce2edd1d6ecR41-R68

New code will match cluster manufacturerCode instead of manuSpecificNodOnPilotWire.

Previous code:

function getClusterDefinition(
    key: string | number,
    manufacturerCode: number | null,
    customClusters: CustomClusters,
): {name: string, cluster: ClusterDefinition} {
    let name: string;
    const clusters: CustomClusters = {
        ...customClusters, // Custom clusters have a higher priority than Zcl clusters (custom cluster has a DIFFERENT name than Zcl clusters)
        ...Clusters,
        ...customClusters, // Custom clusters should override Zcl clusters (custom cluster has the SAME name than Zcl clusters)
    };
    if (typeof key === 'number') {
        if (manufacturerCode) {
            name = Object.entries(clusters)
                .find((e) => e[1].ID === key && e[1].manufacturerCode === manufacturerCode)?.[0];
        } 
        
        if (!name) {
            name = Object.entries(clusters).find((e) => e[1].ID === key && !e[1].manufacturerCode)?.[0];
        }
        if (!name) {
            name = Object.entries(clusters).find((e) => e[1].ID === key)?.[0];
        }
    } else {
        name = key;
    }
    let cluster = clusters[name];
    if (!cluster) {
        if (typeof key === 'number') {
            name = key.toString();
            cluster = {attributes: {}, commands: {}, commandsResponse: {}, manufacturerCode: null, ID: key};
        } else {
            throw new Error(`Cluster with name '${key}' does not exist`);
        }
    }
    return {name, cluster};
}

New code:

‎function findClusterNameByID(id: number, manufacturerCode: number | null, clusters: CustomClusters): [name: string, partialMatch: boolean] {
    let name: string;
    // if manufacturer code is given, consider partial match if didn't match against manufacturer code
    let partialMatch: boolean = (manufacturerCode != null);
    for (const clusterName in clusters) {
        const cluster = clusters[clusterName as ClusterName];
        if (cluster.ID === id) {
            // priority on first match when matching only ID
            /* istanbul ignore else */
            if (name == undefined) {
                name = clusterName;
            }
            if (manufacturerCode && (cluster.manufacturerCode === manufacturerCode)) {
                name = clusterName;
                partialMatch = false;
                break;
            } else if (!cluster.manufacturerCode) {
                name = clusterName;
                break;
            }
        }
    }
    return [name, partialMatch];
}
function getClusterDefinition(key: string | number, manufacturerCode: number | null, customClusters: CustomClusters)
    : {name: string, cluster: ClusterDefinition} {
    let name: string;
    if (typeof key === 'number') {
        let partialMatch: boolean;
        // custom clusters have priority over Zcl clusters, except in case of better match (see below)
        [name, partialMatch] = findClusterNameByID(key, manufacturerCode, customClusters);
        if (!name) {
            [name, partialMatch] = findClusterNameByID(key, manufacturerCode, Clusters);
        } else if (partialMatch) {
            let zclName: string;
            [zclName, partialMatch] = findClusterNameByID(key, manufacturerCode, Clusters);
            // Zcl clusters contain a better match, use that one
            if (zclName != undefined && !partialMatch) {
                name = zclName;
            }
        }
    } else {
        name = key;
    }
    let cluster = (name != undefined) && hasCustomClusters(customClusters) ?
        {
            ...Clusters[name as ClusterName],
            ...customClusters[name],// should override Zcl clusters
        }
        : Clusters[name as ClusterName];
    if (!cluster) {
        if (typeof key === 'number') {
            name = key.toString();
            cluster = {attributes: {}, commands: {}, commandsResponse: {}, manufacturerCode: null, ID: key};
        } else {
            throw new Error(`Cluster with name '${key}' does not exist`);
        }
    }
    return {name, cluster};
}

The old code will match cluster without manufacturerCode only after testing all cluster with manufacturerCode.
The new code will match cluster without manufacturerCode if it comes before the one with manufacturerCode.
I fixed the issue on my side by just removing the second break. However t will cause the function to return the last cluster without manufacturerCode. I'm not sure how dangerous it is for other devices.

function findClusterNameByID(id: number, manufacturerCode: number | null, clusters: CustomClusters): [name: string, partialMatch: boolean] {
    let name: string;
    // if manufacturer code is given, consider partial match if didn't match against manufacturer code
    let partialMatch: boolean = (manufacturerCode != null);
    for (const clusterName in clusters) {
        const cluster = clusters[clusterName as ClusterName];
        if (cluster.ID === id) {
            // priority on first match when matching only ID
            /* istanbul ignore else */
            if (name == undefined) {
                name = clusterName;
            }
            if (manufacturerCode && (cluster.manufacturerCode === manufacturerCode)) {
                name = clusterName;
                partialMatch = false;
                break;
            } else if (!cluster.manufacturerCode) {
                name = clusterName;
-               break;
            }
        }
    }
    return [name, partialMatch];
}

vmeurisse added a commit to vmeurisse/zigbee-herdsman that referenced this issue Dec 27, 2024
Previous logic stopped on the first cluster with no manufacturerCode.

Fixes Koenkk/zigbee2mqtt#25333
vmeurisse added a commit to vmeurisse/zigbee-herdsman-converters that referenced this issue Dec 29, 2024
Moved the cluster and zigbee parsers to dedicated file to avoid confusion with manuSpecificAssaDoorLock in findClusterNameById.

Affect: SIN-4-FP-20, SIN-4-FP-21, SIN-4-FP-21_EQU

Fixes Koenkk/zigbee2mqtt#25333
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
problem Something isn't working
Projects
None yet
1 participant