Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Documentation] Spec vs XML difference BooleanStateCluster #25605

Closed
Apollon77 opened this issue Mar 10, 2023 · 7 comments · Fixed by #25626
Closed

[Documentation] Spec vs XML difference BooleanStateCluster #25605

Apollon77 opened this issue Mar 10, 2023 · 7 comments · Fixed by #25626
Labels
Spec XML align SDK XML does not match the spec (including naming, etc)

Comments

@Apollon77
Copy link
Contributor

Documentation issues

I found the following differences between specifications (1.0) and the Chiptool XML for the BooleanStateCluster

XML: https://github.com/project-chip/connectedhomeip/blob/master/src/app/zap-templates/zcl/data-model/chip/boolean-state-cluster.xml

  • the XML defined the value "false" as default falue, the specification do not state any default value
  • The XML defines the event as NOT optional, the specification state optional. With the info in the specs "This event SHALL be generated when the StateValue attribute changes." it feels that the specs are wrong in the table in ApplicationCluster Specs 1.7.5

Platform

other

Anything else?

No response
@bzbarsky-apple bzbarsky-apple added the Spec XML align SDK XML does not match the spec (including naming, etc) label Mar 10, 2023
@bzbarsky-apple
Copy link
Contributor

@Apollon77 Thank you for doing this audit!

the XML defined the value "false" as default falue, the specification do not state any default value

Yep, good catch.

The XML defines the event as NOT optional, the specification state optional.

See https://github.com/CHIP-Specifications/connectedhomeip-spec/issues/6270 opened a month or so ago.

@Apollon77
Copy link
Contributor Author

See https://github.com/CHIP-Specifications/connectedhomeip-spec/issues/6270 opened a month or so ago.

This is not accessible for "Normal GitHub users" it seems

@bzbarsky-apple
Copy link
Contributor

Yes, you have to be a CSA member, and even then it's somewhat restricted. Deeply unfortunate. What the issue says is:

The table says optional, the prose says "This event SHALL be generated when the StateValue attribute changes." as a blanket statement. We should either make it mandatory, or change the prose to say "If this event is supported, then this event SHALL be generated when ..." or something.

@Apollon77
Copy link
Contributor Author

Apollon77 commented Mar 10, 2023 •
edited
Loading

Ok, so ... is the Event now Optional or Mandatory? :-) When I read this then it is "most likely optional"

Apollon77 referenced this issue in mfucci/node-matter Mar 10, 2023
@Apollon77
add todo renames
3a595f0
@bzbarsky-apple
Copy link
Contributor

The issue is still open, so I don't have an answer to that question....

@bzbarsky-apple
Copy link
Contributor

Thinking about the "default" thing some more: "default" in the XML right now does not mean the same concept as "default" does in the spec. So there's no particular reason they should be aligned, and in some cases aligning them will be actively confusing.

@bzbarsky-apple
Copy link
Contributor

That said, in this case removing the default="0" is probably fine.

bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Mar 10, 2023
@bzbarsky-apple
Fix various mis-alignments wrt spec in cluster XML.
88c1e7a 
1) Remove default that is not listed in spec table in Boolean State.
   * Fixes project-chip#25605
2) Remove length constraint on list in Fixed Label and User Label.
   * Fixes project-chip#25610
3) Remove length constraint on list in Network Commissioning.
   * Partially addresses project-chip#25612
4) Remove incorrect max values in Occupancy Sensing.
   * Fixes project-chip#25614
5) Add min/max for SupportedFabrics in Operational Credentials
   * Fixes project-chip#25616
6) Remove defaults and "reportable" annotations that are not in the spec table
   in Pressure Measurement
   * Fixes project-chip#25617
7) Fix max value of Tolerance attribute in Temperature Measurement
   * Fixes project-chip#25618
8) Remove stray "reportable" on Tolerance in Relative Humidity Measurement
   * Fixes project-chip#25620
@bzbarsky-apple bzbarsky-apple mentioned this issue Mar 10, 2023
Merged
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Mar 10, 2023
@bzbarsky-apple
Fix various mis-alignments wrt spec in cluster XML.
05f1c92 
1) Remove default that is not listed in spec table in Boolean State.
   * Fixes project-chip#25605
2) Remove length constraint on list in Fixed Label and User Label.
   * Fixes project-chip#25610
3) Remove length constraint on list in Network Commissioning.
   * Partially addresses project-chip#25612
4) Remove incorrect max values in Occupancy Sensing.
   * Fixes project-chip#25614
5) Add min/max for SupportedFabrics in Operational Credentials
   * Fixes project-chip#25616
6) Remove defaults and "reportable" annotations that are not in the spec table
   in Pressure Measurement
   * Fixes project-chip#25617
7) Fix max value of Tolerance attribute in Temperature Measurement
   * Fixes project-chip#25618
8) Remove stray "reportable" on Tolerance in Relative Humidity Measurement
   * Fixes project-chip#25620
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Mar 10, 2023
@bzbarsky-apple
Fix various mis-alignments wrt spec in cluster XML.
599ce0f 
1) Remove default that is not listed in spec table in Boolean State.
   * Fixes project-chip#25605
2) Remove length constraint on list in Fixed Label and User Label.
   * Fixes project-chip#25610
3) Remove length constraint on list in Network Commissioning.
   * Partially addresses project-chip#25612
4) Remove incorrect max values in Occupancy Sensing.
   * Fixes project-chip#25614
5) Add min/max for SupportedFabrics in Operational Credentials
   * Fixes project-chip#25616
6) Remove defaults and "reportable" annotations that are not in the spec table
   in Pressure Measurement
   * Fixes project-chip#25617
7) Fix max value of Tolerance attribute in Temperature Measurement
   * Fixes project-chip#25618
8) Remove stray "reportable" on Tolerance in Relative Humidity Measurement
   * Fixes project-chip#25620
jmartinez-silabs pushed a commit that referenced this issue Mar 14, 2023
@bzbarsky-apple
Fix various mis-alignments wrt spec in cluster XML. (#25626)
0c5d81d 
1) Remove default that is not listed in spec table in Boolean State.
   * Fixes #25605
2) Remove length constraint on list in Fixed Label and User Label.
   * Fixes #25610
3) Remove length constraint on list in Network Commissioning.
   * Partially addresses #25612
4) Remove incorrect max values in Occupancy Sensing.
   * Fixes #25614
5) Add min/max for SupportedFabrics in Operational Credentials
   * Fixes #25616
6) Remove defaults and "reportable" annotations that are not in the spec table
   in Pressure Measurement
   * Fixes #25617
7) Fix max value of Tolerance attribute in Temperature Measurement
   * Fixes #25618
8) Remove stray "reportable" on Tolerance in Relative Humidity Measurement
   * Fixes #25620
kkasperczyk-no pushed a commit to kkasperczyk-no/sdk-connectedhomeip that referenced this issue Mar 15, 2023
@bzbarsky-apple @kkasperczyk-no
Fix various mis-alignments wrt spec in cluster XML. (#25626)
0e8d736 
1) Remove default that is not listed in spec table in Boolean State.
   * Fixes project-chip/connectedhomeip#25605
2) Remove length constraint on list in Fixed Label and User Label.
   * Fixes project-chip/connectedhomeip#25610
3) Remove length constraint on list in Network Commissioning.
   * Partially addresses project-chip/connectedhomeip#25612
4) Remove incorrect max values in Occupancy Sensing.
   * Fixes project-chip/connectedhomeip#25614
5) Add min/max for SupportedFabrics in Operational Credentials
   * Fixes project-chip/connectedhomeip#25616
6) Remove defaults and "reportable" annotations that are not in the spec table
   in Pressure Measurement
   * Fixes project-chip/connectedhomeip#25617
7) Fix max value of Tolerance attribute in Temperature Measurement
   * Fixes project-chip/connectedhomeip#25618
8) Remove stray "reportable" on Tolerance in Relative Humidity Measurement
   * Fixes project-chip/connectedhomeip#25620
lecndav pushed a commit to lecndav/connectedhomeip that referenced this issue Mar 22, 2023
@bzbarsky-apple
Fix various mis-alignments wrt spec in cluster XML. (project-chip#25626)
a6169f9 
1) Remove default that is not listed in spec table in Boolean State.
   * Fixes project-chip#25605
2) Remove length constraint on list in Fixed Label and User Label.
   * Fixes project-chip#25610
3) Remove length constraint on list in Network Commissioning.
   * Partially addresses project-chip#25612
4) Remove incorrect max values in Occupancy Sensing.
   * Fixes project-chip#25614
5) Add min/max for SupportedFabrics in Operational Credentials
   * Fixes project-chip#25616
6) Remove defaults and "reportable" annotations that are not in the spec table
   in Pressure Measurement
   * Fixes project-chip#25617
7) Fix max value of Tolerance attribute in Temperature Measurement
   * Fixes project-chip#25618
8) Remove stray "reportable" on Tolerance in Relative Humidity Measurement
   * Fixes project-chip#25620
mwswartwout pushed a commit to mwswartwout/connectedhomeip that referenced this issue Mar 27, 2023
@bzbarsky-apple @mwswartwout
Fix various mis-alignments wrt spec in cluster XML. (project-chip#25626)
9d72411 
1) Remove default that is not listed in spec table in Boolean State.
   * Fixes project-chip#25605
2) Remove length constraint on list in Fixed Label and User Label.
   * Fixes project-chip#25610
3) Remove length constraint on list in Network Commissioning.
   * Partially addresses project-chip#25612
4) Remove incorrect max values in Occupancy Sensing.
   * Fixes project-chip#25614
5) Add min/max for SupportedFabrics in Operational Credentials
   * Fixes project-chip#25616
6) Remove defaults and "reportable" annotations that are not in the spec table
   in Pressure Measurement
   * Fixes project-chip#25617
7) Fix max value of Tolerance attribute in Temperature Measurement
   * Fixes project-chip#25618
8) Remove stray "reportable" on Tolerance in Relative Humidity Measurement
   * Fixes project-chip#25620
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Spec XML align SDK XML does not match the spec (including naming, etc)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix various mis-alignments wrt spec in cluster XML. bzbarsky-apple/connectedhomeip
2 participants
@Apollon77 @bzbarsky-apple