fix(cluster): Split readAttributes requests to fit a frame

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,

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,
 };

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();

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',