Skip to content

Conversation

@Nerivec
Copy link
Collaborator

@Nerivec Nerivec commented Sep 15, 2025

Introduces extra metadata for ZCL spec, with initial "auto update" using data from CSA ZAP (https://github.com/project-chip/zap) followed by manual re-check & completion.

Should eventually allow much better processing, e.g.:

  • proper processing of non-values & default per attribute
  • allow extending support (e.g. legacy devices missing a value must rely on default to avoid failures)
  • allow clamping numeric values as appropriate
  • allow better error handling (e.g. don't consider failure when not required)
  • allow far better frontend validation & UX (prevent "doomed-to-fail" & co)

Notes:

  • using incremental per commit updates of Clusters for easier checking
  • using a couple of "reports" for auto updates of Clusters for easier checking (present in same commit)
  • relevant commits (others are scripting-related, or typing updates):
    • 87b4a33 (couple manual fixes, needs cross-repo check though, used IDs weirdness)
    • ec7e583 (align everything before auto updates: all hex IDs with matching padding, compact formatting)
    • 0530c02 (auto update, A.K.A "the big one")
    • 1df1e99 (manual addition of pwm cluster)
    • 299df22 (cleanup typing: use shorter names, merge min/minInclusive props)
    • 142c350 (make use of some of the new metas to validate read/write of attr/param)
  • autogen datatypes commit: 5120632
  • This would have been much easier if we could have used ZAP directly to generate the spec (instead of manually scripting to edit current), but with so many name mismatches that would be created, it would end up being a very big cross-repo update... Something to keep in mind for next Z2M major though.
  • change from e.g. UINT8 to ENUM8 isn't that important (semantic), but change from e.g. UINT8 to BITMAP8 is important (no non-value for BITMAP/DATA)

Logic changes

  • attributes have to be declared writable for write op to be allowed
  • read & write ops:
    • attribute & parameter number values are checked against valid range
    • attribute & parameter string values are checked against valid length
    • default/min/max take precedence over non-value & restrictions (early return on equal)
  • read op:
    • throws if read: false
    • NaN is returned for numeric data types equal to non-value
  • write op:
    • throws if write undefined (== false)
    • NaN is converted to either attribute default (if any) or non-value for data type (throws if neither exist)
  • write/read LIST_THERMO_TRANSITIONS => was writing as uint16, spec says int16 for heat & cool set points

TODO:

  • update cluster typing
  • update clusters with ZAP data (auto)
  • check clusters from auto update against spec PDF
  • update clusters with spec data (manual, for clusters not available with auto update)
  • create logic to handle default/invalid (will handle UINT8 doesn't allow 0xff (255) as value anymore which breaks code #1498)
  • create logic to handle min/max style metas
  • create logic to handle readable/writable metas
  • remove clusters-old.ts reference (used by scripts for reporting, will fail TS build as long as it's present due to additions in Clusters)
  • update ZHC
    • fix a few cluster changes
    • set existing custom clusters to "backwards-compatible": https://gist.github.com/Nerivec/b3f514201f41eb8218f8dd3fb78217fd
    • lightingColorCtrl cluster now has two extra params in most commands (should default to 0, 0)
    • for genLevelCtrl, technically should use more restrictive genLevelCtrlForLighting in most cases (derived cluster should take priority) but with so many off-spec devices, keeping the less restrictive version and letting ZHC do the rest if needed.

TODO (future):

  • whole bunch of specific read/write for BuffaloZcl needed for various newly added params (marked TODO in cluster.ts)
  • ZHC custom clusters should set proper metadata (after "backwards-compatible" PR - see script linked above)
  • remove no longer useful scripts
  • move 3 sprut clusters to custom clusters (uses IDs not intended for custom)
  • found spec typos (error-prone): installedClosedLimitTiltDdegree, acCollTemp (needs cross-repo check when/if changed)

CC: @sjorge @kirovilya @burmistrzak @3reality-support

@Nerivec Nerivec force-pushed the zap branch 2 times, most recently from a4d76dd to 142c350 Compare September 15, 2025 21:14
@sjorge
Copy link
Contributor

sjorge commented Sep 17, 2025

Big change, I had a quick look over it so far, I love the switch to constantly using hex makes things so much easier when reading the ZCL.

So looking at the scripts, is there an XML source of all the known clusters/attribute and they are now being parsed?

@Nerivec
Copy link
Collaborator Author

Nerivec commented Sep 17, 2025

Yes, it's in the ZAP repo I linked (zap/zcl-builtin/dotdot). Though it does not have all spec clusters for some reason, so some will have to be done manually.

ZAP could technically be used to generate the whole ZCL Clusters object for ZH, that's one of its purposes, but the problem is backwards compat with name changes (& customs), that would create a massive amount of changes needed in ZHC (& external converters). So, had to build a script to merge data instead.

@Nerivec Nerivec changed the title fix: enhance ZCL specification fix!: enhance ZCL specification Oct 28, 2025
@Nerivec
Copy link
Collaborator Author

Nerivec commented Nov 11, 2025

Fixed an issue with new options fields added to most control cmds for several clusters. Many devices don't implement this (yet), so we don't want to fail if not (assumed 0 if not present). These cmds should not be received by coordinator, so should never be a problem, but... 😅

We should however update ZHC to send these two attributes (even if it's 0 for now), as it does fail payloads for e.g. in Wireshark if not present (doesn't appear to check/default to zero, so, failure).

@Nerivec
Copy link
Collaborator Author

Nerivec commented Nov 19, 2025

@Koenkk once good to go, the following script should take care of all ZHC custom clusters (will set behavior as "backwards compatible"):
https://gist.github.com/Nerivec/b3f514201f41eb8218f8dd3fb78217fd
They can be further edited in future PRs according to their respective specs by users with the knowledge to do so 😉

@Nerivec Nerivec marked this pull request as ready for review November 20, 2025 16:48
@Nerivec
Copy link
Collaborator Author

Nerivec commented Nov 20, 2025

Took this out of draft, though I suggest we wait for next release, since we're now closer to it than not 😁
Let me know if you see anything else needed before merging.

@Koenkk
Copy link
Owner

Koenkk commented Nov 20, 2025

@Nerivec perfect!

@Nerivec
Copy link
Collaborator Author

Nerivec commented Dec 7, 2025

Cleaned up a bit, figured extra bytes saved with shorter property names in Clusters was worth it since this is published over MQTT.
Also updated the script linked above to match (just writable=>write).
Removed genLevelCtrlForLightning in favor of always using genLevelCtrl (to have just 1 cluster with same ID). It's a bit less restrictive than genLevelCtrlForLightning in metadata, but ZHC can do the rest as needed (with so many off-spec devices, probably best to use the base version instead of the derived one anyway). Added comments in-place for stuff that would be different. Should avoid making a mess with cache, scenes & co.

I left the added scripts, even though most can no longer be used as-is (was meant for first-trigger), they might be useful in future...

@burmistrzak
Copy link
Contributor

@Nerivec Looking sweet! 🙌

Figuring out the correct metadata for custom clusters will be... fun. 😅
I wouldn't be surprised if some devices don't handle out-of-bound values correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants