From 733d2efc30ca60c769444dfaa81c7b0238d2aa46 Mon Sep 17 00:00:00 2001 From: Robin Bolscher Date: Tue, 27 Oct 2026 14:16:51 +0100 Subject: [PATCH] fix(cluster): Split readAttributes requests to fit a frame readAttributes([]) put every attribute of the cluster in one Read Attributes request. Homey's Zigbee stack rejects a ZCL frame over 82 bytes, so a full read failed on any cluster with more than 39 attributes: doorLock (45) with "ZCL frame too long", and metering (175) even earlier, in the library's own serializer, with a RangeError before anything was sent. Ask for at most 30 ids per request. The loop already re-asks for whatever is still pending, so the ids a cut-short response left out go in the next request. Define the 82-byte limit once, as MAX_ZCL_FRAME_BYTES in lib/constants.js, and have the test harness read it there. The harness's known-bug test for this now passes. --- lib/Cluster.js | 12 ++++++++--- lib/constants.js | 7 +++++++ test/util/virtualDevice.js | 6 ++---- test/virtualDevice.js | 42 +++++++++++++++++--------------------- 4 files changed, 37 insertions(+), 30 deletions(-) diff --git a/lib/Cluster.js b/lib/Cluster.js index 348028f..9a8692c 100644 --- a/lib/Cluster.js +++ b/lib/Cluster.js @@ -15,6 +15,10 @@ const { } = require('./zclFrames'); const { ZCLStruct, ZCLDataTypes } = require('./zclTypes'); +// 30 ids fit a request frame. The response often does not: the device cuts it short, and the ids +// it left out are asked for again in the next request. +const MAX_ATTRIBUTE_IDS_PER_READ = 30; + const GLOBAL_ATTRIBUTES = { clusterRevision: { id: 0xfffd, type: ZCLDataTypes.uint16 }, attributeReportingStatus: { @@ -474,18 +478,20 @@ class Cluster extends EventEmitter { const resultObj = {}; while (attrIds.size) { + const requestIds = [...attrIds].slice(0, MAX_ATTRIBUTE_IDS_PER_READ); + // Check if command should get manufacturerSpecific flag - const manufacturerId = this._checkForManufacturerSpecificAttributes(Array.from(attrIds)); + const manufacturerId = this._checkForManufacturerSpecificAttributes(requestIds); debug( this.logId, 'read attributes', - [...attrIds], + requestIds, manufacturerId ? `manufacturer specific id ${manufacturerId}` : '', ); const response = await super.readAttributes( { - attributes: [...attrIds], + attributes: requestIds, manufacturerId, }, opts, diff --git a/lib/constants.js b/lib/constants.js index dcafbf5..9bf9973 100644 --- a/lib/constants.js +++ b/lib/constants.js @@ -90,8 +90,15 @@ const IAS_ZONE_TYPE = { INVALID_ZONE_TYPE: 0xffff, }; +/** + * Longest ZCL frame, header included, that Homey's Zigbee stack sends; it rejects a longer one. + * Raise it here when a stack allows larger frames. + */ +const MAX_ZCL_FRAME_BYTES = 82; + module.exports = { ZIGBEE_PROFILE_ID, ZIGBEE_DEVICE_ID, IAS_ZONE_TYPE, + MAX_ZCL_FRAME_BYTES, }; diff --git a/test/util/virtualDevice.js b/test/util/virtualDevice.js index db2b552..a323c53 100644 --- a/test/util/virtualDevice.js +++ b/test/util/virtualDevice.js @@ -7,9 +7,7 @@ const Node = require('../../lib/Node'); const Endpoint = require('../../lib/Endpoint'); const BoundCluster = require('../../lib/BoundCluster'); const { ZCLStandardHeader } = require('../../lib/zclFrames'); - -// Homey's Zigbee stack rejects a longer ZCL frame (MAX_ZCL_BYTES in @athombv/z3gateway). -const HOMEY_MAX_ZCL_BYTES = 82; +const { MAX_ZCL_FRAME_BYTES } = require('../../lib/constants'); // ponytail: counts every frame of a device, raise it when a legit test (e.g. an OTA transfer) // sends more @@ -47,7 +45,7 @@ function boundClusterWithAttributes(attributes) { * `sendFrame`; `settle()`, which waits until no frame is in flight and rejects when a delivery * failed. */ -function createVirtualDevice({ endpoints, maxFrameSize = HOMEY_MAX_ZCL_BYTES }) { +function createVirtualDevice({ endpoints, maxFrameSize = MAX_ZCL_FRAME_BYTES }) { const frames = []; const interceptors = []; const inFlight = new Set(); diff --git a/test/virtualDevice.js b/test/virtualDevice.js index 436e68e..b66e7c4 100644 --- a/test/virtualDevice.js +++ b/test/virtualDevice.js @@ -122,32 +122,28 @@ describe('Virtual device: known bugs on master', function () { assert.deepStrictEqual(result, {}); }); - it( - 'readAttributes([]) on metering splits the request to fit a frame', - { todo: 'fixed by #259' }, - async function () { - const { zclNode } = createVirtualDevice({ - endpoints: [ - { - endpointId: 1, - inputClusters: [MeteringCluster.ID], - clusters: { - metering: { currentSummationDelivered: 1234, multiplier: 1, divisor: 1000 }, - }, + it('readAttributes([]) on metering splits the request to fit a frame', async function () { + const { zclNode } = createVirtualDevice({ + endpoints: [ + { + endpointId: 1, + inputClusters: [MeteringCluster.ID], + clusters: { + metering: { currentSummationDelivered: 1234, multiplier: 1, divisor: 1000 }, }, - ], - }); + }, + ], + }); - // Every request must also fit the Homey stack, which the virtual link enforces. - const { currentSummationDelivered, multiplier, divisor } = - await zclNode.endpoints[1].clusters.metering.readAttributes([]); + // Every request must also fit the Homey stack, which the virtual link enforces. + const { currentSummationDelivered, multiplier, divisor } = + await zclNode.endpoints[1].clusters.metering.readAttributes([]); - assert.deepStrictEqual( - { currentSummationDelivered, multiplier, divisor }, - { currentSummationDelivered: 1234, multiplier: 1, divisor: 1000 }, - ); - }, - ); + assert.deepStrictEqual( + { currentSummationDelivered, multiplier, divisor }, + { currentSummationDelivered: 1234, multiplier: 1, divisor: 1000 }, + ); + }); it( 'Endpoint.unbind removes the binding, so the cluster is unsupported',