Repository navigation
fix(cluster): Split readAttributes requests to fit a frame - #259
Merged
Merged
Conversation
RobinBol
added a commit
that referenced
this pull request
Oct 27, 2026
* test: Add a virtual device harness for misbehaving-device edge cases Real devices behave too well to reproduce bugs like the readAttributes retry loop. createVirtualDevice puts the device on its own node behind an in-memory link that records every frame, enforces Homey's 82-byte ZCL frame limit, and lets a test drop, rewrite or inject frames. test/virtualDevice.js keeps four bugs that reproduce on master as todo tests: the readAttributes loop on unrequested ids (#258), readAttributes([]) on metering overflowing a frame (#259), Endpoint.unbind leaving a Cluster in bindings, and attr.undefined emitted for unknown ids in a report. Refs #258 Refs #259 * test: Move every test onto the virtual device harness The suite had four ways to fake a node: createMockNode, createConnectedNodePair, MOCK_DEVICES and hand-built Node stubs. All of them are now createVirtualDevice, and test/util/mockNode.js is gone. Migrating exposed tests that could not fail. They now can, each checked by breaking the library code it covers: - waitForResponse: false was passed as command args, so the test waited anyway. - The mixed-manufacturer tests accepted any error, including one from a binding left over from an earlier test. - Not answering a default response was never reached, because our own default responses set Disable Default Response; the device now sends one without it. - Assertions inside handlers became error responses or hung done callbacks; tests now assert after await or settle(), and on wire frames instead of spies. - The attribute 0x0000 test asserted nothing; it now decodes a noData report. Duplicates of the ZCLError and Default Response tests are removed, the four tests of MOCK_DEVICES itself are dropped, and OTA runs the way Homey uses it: the device requests, the node serves. The harness gains frame meta, eager parsing and a 1000-frame cap that turns a request/response loop into a failure. * docs: Document the virtual device test harness docs/testing.md explains both sides of createVirtualDevice, its options and return values, recipes for misbehaving devices, and the rules that keep a test able to fail. AGENTS.md and the README point to it. * test: Turn the Xiaomi lifeline script into a real test test/manual/xiaomiHacks.js registered a Basic cluster that declares its own data type for Xiaomi's attribute 0xff01, but never decoded a frame, so it checked nothing. The new test has the virtual device send a report shaped like the ones Xiaomi devices send and asserts the decoded records. It fails when the library decodes with the type on the wire instead of the declared one, which is what such a custom cluster relies on. * test: Deliver harness frames after sendFrame resolves Frames were delivered while sendFrame was still running, so every reply arrived before the command armed its response timeout: setting that timeout to 0 failed one test. Frames now arrive on the next turn of the event loop, as on Homey where sendFrame resolves on the APS ACK and the response comes later. The same mutation now fails 33 tests. A loop between two handlers only debug-logged the frame cap error, so the test passed. settle() now rethrows a failed delivery, including the cap. An interceptor that returns a non-Buffer throws instead of being dropped silently. frameToClient() and createSingleClusterDevice() replace the hand-built headers and per-file single-cluster helpers. frames[].error, which the node never let through, is gone, and docs/testing.md matches the code. * test: Import the harness directly instead of through test/util/index.js The index only re-exported test/util/virtualDevice.js. docs/adding-clusters.md still pointed at the removed mock node helpers there; it now links the testing guide. * docs: Cut the testing guide to what the code does not already say The diagram repeated the bullets under it, frameToClient was explained three times, and the misbehaving-device snippet was plain BoundCluster use. * docs: Explain the test harness as a concept and point to examples Option and return value details now live as JSDoc on createVirtualDevice, next to the code they describe, so they cannot drift from it. The guide keeps the idea, a table of example tests per scenario, and the rules.
RobinBol
force-pushed
the
fix/chunk-read-attributes-requests
branch
3 times, most recently
from
October 27, 2026 13:18
60d6dba to
9511256
Compare
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.
RobinBol
force-pushed
the
fix/chunk-read-attributes-requests
branch
from
October 27, 2026 13:18
9511256 to
733d2ef
Compare
RobinBol
marked this pull request as ready for review
October 27, 2026 13:19
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
readAttributesasks for at most 30 attribute ids per request; the rest follow in the next one. Afull read put every id in one frame, which fails above 39 attributes: Homey's stack rejects a ZCL
frame over 82 bytes (doorLock, 45), and metering (175) threw
RangeError: The value of "offset" is out of rangebefore sending anything. The 82-byte limit is nowMAX_ZCL_FRAME_BYTESinlib/constants.js. On an IKEA INSPELNING plug, metering now reads in 8 requests.