Repository navigation
Merge master into production (#patch) - #264
Merged
Merged
Conversation
Only r8 was checked in, so there was no way to see when a cluster, attribute or command was introduced or changed. Keep r6 and r7 next to it so a definition can be diffed against the revision it came from. r8 is published by the CSA, r7 as well; r6 is no longer offered there and comes from the Internet Archive copy of the original zigbee.org file (07-5123-06, 14 January 2016).
The Source Reference named only the r8 PDF. Name all three and say what the older two are for, so r6 and r7 get used when a definition has to match what devices in the field implement.
docs: Add the Zigbee Cluster Library r6 and r7 specifications
…rs.md AGENTS.md had grown to 520 lines, most of it a cluster definition guide whose rules no longer matched the code. The template wrote the cluster id as four-digit hex while 42 of 47 clusters return decimal, the data type reference listed a uint64 that is not exported and omitted a dozen types that are, and the section banner and command divider conventions appear in 1 and 3 of 47 files respectively. A closing checklist restated every rule a second time. It also described the outgoing frame path as Cluster.sendCommand(), which does not exist. Commands are generated onto the prototype and go out through Cluster.sendFrame(). What remains in AGENTS.md is what an agent cannot read off the code: the Node version in .nvmrc, that npm test globs test/*.js only, that index.d.ts is generated, that there is no CHANGELOG.md, the frame data flow, and the node:test callback signature that silently passes a test when written as function(done). The release notes section is dropped because README.md already documents it. The cluster guide moves to docs/adding-clusters.md, corrected against the current definitions: ids are described as the repo actually writes them, the data type list gives way to a command that prints the real set, and the 75-line template to the clusters worth copying. AGENTS.md no longer needs a Prettier exemption now that the M* markers live in docs/adding-clusters.md inside backticks.
… docs/trim-agents-md # Conflicts: # AGENTS.md
…EADME The page read as an appendix to AGENTS.md: it told an executor what not to guess and how far to let a change spread, and sent the reader to AGENTS.md for the commands. Nothing but AGENTS.md linked to it. State the conventions instead, point at Contributing for the checks, and say what happens after the definition is written: a test where the encoding is worth one, and `npm run generate-types` because `index.d.ts` is generated and committed. Link it from Contributing, where someone adding a cluster looks.
Both were duplication waiting to go stale. The extraction command told a reader how to operate a PDF from a shell, which is not something this project knows better than they do, and `grep -A 500` on a section heading matches the table of contents first anyway. The section pointer is what carries the value, so keep that alone. `.nvmrc` is the single source for the Node version and every workflow reads it with `node-version-file`, so naming the number in prose only adds a second place to update.
The snippet was presented as what `imageBlockResponse` does, but that command spreads a module-level `IMAGE_BLOCK_RESPONSE` shared with `imageBlockRequest` and `imagePageRequest`, and the real struct carries `imageType`, `dataSize` and `minimumBlockPeriod` on top of the fields shown. Present the block as the shape it is and point at the constant for the full version, so a trimmed illustration can no longer read as a quote.
docs: Trim AGENTS.md and move the cluster guide to docs/adding-clusters.md
@athombv/jsdoc-template, used only for the docs build, depends on taffydb, which has a high severity advisory and no patched release. Override it with @jsdoc/salty, the drop-in replacement jsdoc 4 itself uses, which exports the same taffy() factory. The generated docs are byte-identical.
build(deps): Replace taffydb with @jsdoc/salty
* 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.
* fix(cluster): Bound the readAttributes retry loop A record for an attribute that was not requested has no entry in the name map, so it was written into the result under the key `undefined`. Drop such records instead. The same check bounds the loop. readAttributes re-asks for whatever the remote cluster has not answered yet, which is what lets a device split a response over several frames. An attribute left the pending set only when the device reported it, so a response holding none of the requested ids left the set unchanged and the next pass sent a byte-identical request, for as long as the process ran. No device was seen doing this and no issue reports it: the loop simply had no cap and no deadline, and no timeout fires because each individual read succeeds. Stop once a pass removes nothing, logging what went unanswered. A response carrying two records for the same attribute now keeps the first rather than the last, because the second no longer matches a pending id. * test: Turn the readAttributes known-bug test into a real one The harness carries the device that only ever answers with unrequested ids as a todo test for this fix, so the todo goes. The same case in test/readAttributes.js was a duplicate and is dropped; the rest of that file now runs on the harness instead of the removed mock node.
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.
unbind replaced the binding with a new client Cluster instead of deleting it. Incoming commands for that cluster then reached a client handler, which answered UNSUP_CLUSTER_COMMAND, where an endpoint without a binding answers UNSUPPORTED_CLUSTER. Delete the binding, and turn the harness's known-bug test for this into a real one.
A reported attribute the cluster does not define has no name, so every one of them was emitted as attr.undefined. Fall back to its numeric id, which is also how readAttributes keys an attribute it has no name for.
…bute fix(cluster): Emit attr.<id> for an unknown attribute in a report
github-actions
Bot
added
the
release
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.
No description provided.