-
Notifications
You must be signed in to change notification settings - Fork 363
fix!: enhance ZCL specification #1503
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
a4d76dd to
142c350
Compare
|
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? |
|
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. |
|
Cleaned up a bit, figured extra bytes saved with shorter property names in 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... |
|
@Nerivec Looking sweet! 🙌 Figuring out the correct metadata for custom clusters will be... fun. 😅 |
|
@burmistrzak in most cases, it will be around It's mostly an enhancement though, no doubt plenty of custom clusters will keep the default of It should be nice for the dev console in frontend, keep users from wondering why a read/write didn't work, at least with spec clusters. And should also allow to detect improper converters that may have been silently ignoring errors in the past, which may have resulted in incorrect assumptions. |
|
@Nerivec Regarding custom cluster enhancements in the frontend, would it be possible to put them first (or prefix them w/ the custom cluster's name) in the attribute dropdown? |
|
Thanks! |
|
A bit late but congrats on landing this! <3 |
The `write` parameter was introduced in zigbee2mqtt 2.7.2 to indicate attributes that are writable, otherwise they are read-only and they throw errors when trying to write to them. Koenkk/zigbee-herdsman#1503
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.:
Notes:
Logic changes
writableforwriteop to be allowedread&writeops:readop:read: falsewriteop:writeundefined (== false)TODO:
UINT8doesn't allow0xff(255) as value anymore which breaks code #1498)clusters-old.tsreference (used by scripts for reporting, will fail TS build as long as it's present due to additions in Clusters)genLevelCtrl&lightingColorCtrlclusters now have two extra params in most commands (should default to 0, 0)genLevelCtrl, technically should use more restrictivegenLevelCtrlForLightingin 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):
TODOin cluster.ts)installedClosedLimitTiltDdegree,acCollTemp(needs cross-repo check when/if changed)CC: @sjorge @kirovilya @burmistrzak @3reality-support