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.

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

AverageWearCount attribute got merged throughout SDK without existing in spec, needs to be removed #30002

Closed
tcarmelveilleux opened this issue Oct 25, 2023 · 4 comments · Fixed by #30125
Labels
Spec XML align SDK XML does not match the spec (including naming, etc) spec Mismatch between spec and implementation

Comments

@tcarmelveilleux
Copy link
Contributor

Problem

The AverageWearCount attribute of the General Diagnostics cluster got introduced throughout the SDK, (possibly including the test harness), in Matter 1.2. This happened in #29285 that predated the 1.2.0 tag, so it is included in that tag.

The AverageWearCount attribute is not present in spec. It was merged from a work-in-progress PR, in the SDK, before the spec had been updated to include it. The spec PR itself (https://github.com/CHIP-Specifications/connectedhomeip-spec/pull/7535) got closed without merging, because it was a new feature that did not follow the NFR/TCR process.

It is likely that real products will be certified to 1.2 that will include the AverageWearCount "non standard" attribute that is in standard range, but not tested or certified.

Proposed solution

  • Remove AverageWearCount attribute from General Diagnostics cluster XML
  • Reserve attribute ID 9 in General Diagnostics cluster, including in spec (do so in rev 3)
  • Remove AverageWearCount from General Diagnostics cluster implementation and associated YAML tests, until it appears in the spec.
  • Make an urgent CCB to mention that this attribute ID should not be found.
@tcarmelveilleux tcarmelveilleux added spec Mismatch between spec and implementation Spec XML align SDK XML does not match the spec (including naming, etc) labels Oct 25, 2023
@tcarmelveilleux
Copy link
Contributor Author

@tcarmelveilleux
Copy link
Contributor Author

tcarmelveilleux commented Oct 25, 2023

The SHA used by TH (according to @cjandhyala) is 19771ed. This includes the new attribute. This means it will not be detected by TC-IDM-10.1 as a reserved/invalid attribute.

@cecille
Copy link
Contributor

cecille commented Oct 25, 2023

Per discussion here, we'd like to propose a one-off errata test that should be back ported to 1.2 testing with a test house notification. @naveenkommareddi FYI.

If we go forward with the proposal to mark the attribute ID as reserved and disallowed in the spec going forward, the conformance tests that are landing for 1.3 should catch it automatically.

@cecille
Copy link
Contributor

cecille commented Oct 26, 2023

I've added a proposal for a test for 1.2 - please see linked PRs. Given that the plan is to fix this in the spec for 1.3 by marking this attribute as disallowed, we should be able to remove this test for the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Spec XML align SDK XML does not match the spec (including naming, etc) spec Mismatch between spec and implementation
Projects
Development

Successfully merging a pull request may close this issue.

2 participants