-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Introduce a building block usable for all Q attributes #34266
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 term 8000 s of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce a building block usable for all Q attributes #34266
Conversation
- Q quality requires marking attributes as dirty for the purposes of reporting only when certain conditions arise. - This PR introduces a building block attribute value wrapper compatible with any nullable or non-nullable numerical attribute, which allows applying the necessary policies, and all complex policies that currently exist in the Matter spec (e.g. any CountdownTime, CurrentLevel, etc). Testing done: - Added unit tests. Integration in existing clusters will follow in a different PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes requested on sideffects api: a getter that always resets feels o 8000 ff.
PR #34266: Size comparison from c91c3e0 to c3c3c61 Increases above 0.2%:
Full report (54 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, tizen)
|
PR #34266: Size comparison from c91c3e0 to 01a065b Increases above 0.2%:
Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
PR #34266: Size comparison from c91c3e0 to f7f65b1 Increases above 0.2%:
Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
} | ||
|
||
Nullable<T> value() const { return mValue; } | ||
QuieterReportingPolicyFlags & policy() { return mPolicyFlags; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really want policy to be randomly mutable? I would expect it to be a constructor argument and immutable...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer being able to update the policy, and I don't think it's a big deal. Will leave like this.
…34266) * Introduce a building block usable for all Q attributes - Q quality requires marking attributes as dirty for the purposes of reporting only when certain conditions arise. - This PR introduces a building block attribute value wrapper compatible with any nullable or non-nullable numerical attribute, which allows applying the necessary policies, and all complex policies that currently exist in the Matter spec (e.g. any CountdownTime, CurrentLevel, etc). Testing done: - Added unit tests. Integration in existing clusters will follow in a different PR. * Reword examples * Restyled by clang-format * Address review comments * Restyled by clang-format * Restyled by gn * Address more review comments * Restyled by clang-format * Add clang-tidy exclusion due to clang-tidy bug See llvm/llvm-project#97426 * Apply review comments * Restyled by clang-format * Fix Darwin clang-tidy crash by adding an override --------- Co-authored-by: Restyled.io <commits@restyled.io>
- PR project-chip#34266 had post review comments See: project-chip#34266 (review) - Fix 0->0 case - Introduce `Timestamp` more places where it makes sense - Simplify some code lines Fixes project-chip#34334 Testing done: - Added unit test for 0->0 - Tests still pass
* Fix post-review comments on Q quality utils from #34266 - PR #34266 had post review comments See: #34266 (review) - Fix 0->0 case - Introduce `Timestamp` more places where it makes sense - Simplify some code lines Fixes #34334 Testing done: - Added unit test for 0->0 - Tests still pass * Restyled by clang-format * Address review comments * Restyled by clang-format --------- Co-authored-by: Restyled.io <commits@restyled.io>
* Fix post-review comments on Q quality utils from #34266 - PR #34266 had post review comments See: #34266 (review) - Fix 0->0 case - Introduce `Timestamp` more places where it makes sense - Simplify some code lines Fixes #34334 Testing done: - Added unit test for 0->0 - Tests still pass * Restyled by clang-format * Address review comments * Restyled by clang-format * Add Q quality to OperationalState CountdownTIme - Update operational state cluster server to report countdown time at edges. - Add a way for cluster delegate to request an update of time. Issue #34421 Testing done: - Existing tests still pass - Will add cert test when the text is finalized (week of July 22) * Restyled by clang-format * Address review comments * Address review comments * Update src/app/clusters/operational-state-server/operational-state-server.cpp * Fix several Opstate test cases * Restyled by autopep8 * Fix minor grammar typo * Restyled by autopep8 * Fix TC-OPSTATE-2.2 --------- Co-authored-by: Restyled.io <commits@restyled.io>
…roject-chip#34416) * Fix post-review comments on Q quality utils from project-chip#34266 - PR project-chip#34266 had post review comments See: project-chip#34266 (review) - Fix 0->0 case - Introduce `Timestamp` more places where it makes sense - Simplify some code lines Fixes project-chip#34334 Testing done: - Added unit test for 0->0 - Tests still pass * Restyled by clang-format * Address review comments * Restyled by clang-format --------- Co-authored-by: Restyled.io <commits@restyled.io>
* Fix post-review comments on Q quality utils from project-chip#34266 - PR project-chip#34266 had post review comments See: project-chip#34266 (review) - Fix 0->0 case - Introduce `Timestamp` more places where it makes sense - Simplify some code lines Fixes project-chip#34334 Testing done: - Added unit test for 0->0 - Tests still pass * Restyled by clang-format * Address review comments * Restyled by clang-format * Add Q quality to OperationalState CountdownTIme - Update operational state cluster server to report countdown time at edges. - Add a way for cluster delegate to request an update of time. Issue project-chip#34421 Testing done: - Existing tests still pass - Will add cert test when the text is finalized (week of July 22) * Restyled by clang-format * Address review comments * Address review comments * Update src/app/clusters/operational-state-server/operational-state-server.cpp * Fix several Opstate test cases * Restyled by autopep8 * Fix minor grammar typo * Restyled by autopep8 * Fix TC-OPSTATE-2.2 --------- Co-authored-by: Restyled.io <commits@restyled.io>
…34266) * Introduce a building block usable for all Q attributes - Q quality requires marking attributes as dirty for the purposes of reporting only when certain conditions arise. - This PR introduces a building block attribute value wrapper compatible with any nullable or non-nullable numerical attribute, which allows applying the necessary policies, and all complex policies that currently exist in the Matter spec (e.g. any CountdownTime, CurrentLevel, etc). Testing done: - Added unit tests. Integration in existing clusters will follow in a different PR. * Reword examples * Restyled by clang-format * Address review comments * Restyled by clang-format * Restyled by gn * Address more review comments * Restyled by clang-format * Add clang-tidy exclusion due to clang-tidy bug See llvm/llvm-project#97426 * Apply review comments * Restyled by clang-format * Fix Darwin clang-tidy crash by adding an override --------- Co-authored-by: Restyled.io <commits@restyled.io>
…roject-chip#34416) * Fix post-review comments on Q quality utils from project-chip#34266 - PR project-chip#34266 had post review comments See: project-chip#34266 (review) - Fix 0->0 case - Introduce `Timestamp` more places where it makes sense - Simplify some code lines Fixes project-chip#34334 Testing done: - Added unit test for 0->0 - Tests still pass * Restyled by clang-format * Address review comments * Restyled by clang-format --------- Co-authored-by: Restyled.io <commits@restyled.io>
* Fix post-review comments on Q quality utils from project-chip#34266 - PR project-chip#34266 had post review comments See: project-chip#34266 (review) - Fix 0->0 case - Introduce `Timestamp` more places where it makes sense - Simplify some code lines Fixes project-chip#34334 Testing done: - Added unit test for 0->0 - Tests still pass * Restyled by clang-format * Address review comments * Restyled by clang-format * Add Q quality to OperationalState CountdownTIme - Update operational state cluster server to report countdown time at edges. - Add a way for cluster delegate to request an update of time. Issue project-chip#34421 Testing done: - Existing tests still pass - Will add cert test when the text is finalized (week of July 22) * Restyled by clang-format * Address review comments * Address review comments * Update src/app/clusters/operational-state-server/operational-state-server.cpp * Fix several Opstate test cases * Restyled by autopep8 * Fix minor grammar typo * Restyled by autopep8 * Fix TC-OPSTATE-2.2 --------- Co-authored-by: Restyled.io <commits@restyled.io>
* Fix post-review comments on Q quality utils from #34266 - PR #34266 had post review comments See: project-chip/connectedhomeip#34266 (review) - Fix 0->0 case - Introduce `Timestamp` more places where it makes sense - Simplify some code lines Fixes #34334 Testing done: - Added unit test for 0->0 - Tests still pass * Restyled by clang-format * Address review comments * Restyled by clang-format * Add Q quality to OperationalState CountdownTIme - Update operational state cluster server to report countdown time at edges. - Add a way for cluster delegate to request an update of time. Issue #34421 Testing done: - Existing tests still pass - Will add cert test when the text is finalized (week of July 22) * Restyled by clang-format * Address review comments * Address review comments * Update src/app/clusters/operational-state-server/operational-state-server.cpp * Fix several Opstate test cases * Restyled by autopep8 * Fix minor grammar typo * Restyled by autopep8 * Fix TC-OPSTATE-2.2 --------- Co-authored-by: Restyled.io <commits@restyled.io>
Testing done: