-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Improvements to API for Concentration Measurement Clusters #28965
Improvements to API for Concentration Measurement Clusters #28965
Conversation
PR #28965: Size comparison from 0eafac9 to d58645e Increases (4 builds for linux, nrfconnect, psoc6)
Decreases (6 builds for efr32, linux, nrfconnect, psoc6, telink)
Full report (67 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
… and better documented
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.
We should consider having some code exercising the "not everything is enabled" cases.... Maybe in all-clusters-minimal-app?
src/app/clusters/concentration-measurement-server/concentration-measurement-server.h
Outdated
Show resolved
Hide resolved
src/app/clusters/concentration-measurement-server/concentration-measurement-server.h
Outdated
Show resolved
Hide resolved
PR #28965: Size comparison from 0eafac9 to 63a6f72 Full report (2 builds for cc32xx, mbed)
|
…id using ConcreteAttributePath
PR #28965: Size comparison from 0eafac9 to a5f609c Increases (1 build for psoc6)
Decreases (6 builds for efr32, linux, nrfconnect, psoc6, telink)
Full report (67 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
src/app/clusters/concentration-measurement-server/concentration-measurement-server.h
Outdated
Show resolved
Hide resolved
src/app/clusters/concentration-measurement-server/concentration-measurement-server.h
Outdated
Show resolved
Hide resolved
src/app/clusters/concentration-measurement-server/concentration-measurement-server.h
Outdated
Show resolved
Hide resolved
… removed the need for an old variable in the reporting check
PR #28965: Size comparison from 0eafac9 to 8a33f5f Increases above 0.2%:
Increases (20 builds for bl702, bl702l, cc13x4_26x4, cc32xx, linux, nrfconnect, psoc6, telink)
Decreases (9 builds for bl702, efr32, linux, nrfconnect, psoc6, telink)
Full report (67 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
…hip#28965) * Improved the use of the template args to compile out the entire function * Added a detail namespace with attribute classes that can be inherited based on feature settings * Fixed the hard to read constraints functions to be a bit more logical and better documented * Using overload of MatterReportingAttributeChangeCallback so as to avoid using ConcreteAttributePath * made the checking code in the constraints functions more succinct and removed the need for an old variable in the reporting check
…hip#28965) * Improved the use of the template args to compile out the entire function * Added a detail namespace with attribute classes that can be inherited based on feature settings * Fixed the hard to read constraints functions to be a bit more logical and better documented * Using overload of MatterReportingAttributeChangeCallback so as to avoid using ConcreteAttributePath * made the checking code in the constraints functions more succinct and removed the need for an old variable in the reporting check
Description
@bzbarsky-apple raised #28932 off the back of extra comments on #28576 and this PR is to address those comments.
This PR adds better use of the template args to "compile out" the feature functions if a feature is turned off.
It also improves the management of class variables and breaks them down into features classes in a
Detail
namespace.Links
Fixes #28932