[Code Driven Cluster] temperature measurement cluster#43204
Open
arielsz71 wants to merge 16 commits intoproject-chip:masterfrom
Open
[Code Driven Cluster] temperature measurement cluster#43204arielsz71 wants to merge 16 commits intoproject-chip:masterfrom
arielsz71 wants to merge 16 commits intoproject-chip:masterfrom
Conversation
github-actions
bot
added
examples
scripts
platform
app
examples chef
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request decouples the Temperature Measurement cluster by migrating it to a code-driven implementation. The changes are extensive, touching many example applications to adopt the new API for setting temperature values. New cluster implementation files, tests, and build configurations have been added. The overall approach is solid and the implementation appears correct and consistent across the codebase. I have one suggestion to improve the formatting of the new README file for better readability.
Comment on lines
+1
to
+24
| # This cluster is currently following a code driven approach. | ||
|
|
||
| # This means that the Accessors for the attribute MeasuredValue are no longer available. | ||
|
|
||
| # Now to set the value for this attribute the following code change applies: | ||
|
|
||
| # BEFORE (using the Accessors) | ||
|
|
||
| app::Clusters::TemperatureMeasurement::Attributes::MeasuredValue::Set(1, value); | ||
|
|
||
| # CURRENT (using the code driven approach) | ||
|
|
||
| auto temperatureMeasurement = app::Clusters::TemperatureMeasurement::FindClusterOnEndpoint(1); | ||
| VerifyOrReturn(temperatureMeasurement != nullptr); | ||
|
|
||
| CHIP_ERROR err = temperatureMeasurement->SetMeasuredValue(value); | ||
| if (err == CHIP_NO_ERROR) | ||
| { | ||
| // SetMeasuredValue() succeeded | ||
| } | ||
| else | ||
| { | ||
| // SetMeasuredValue() failed | ||
| } |
Contributor
There was a problem hiding this comment.
The markdown formatting in this file seems incorrect. Each line starting with a # will render as a top-level heading, which is likely not the intended formatting. I've suggested a change to use proper markdown for headers, paragraphs, and fenced code blocks to improve readability.
This cluster is currently following a code driven approach.
This means that the Accessors for the attribute `MeasuredValue` are no longer available.
Now to set the value for this attribute the following code change applies:
### BEFORE (using the Accessors)
```cpp
app::Clusters::TemperatureMeasurement::Attributes::MeasuredValue::Set(1, value);CURRENT (using the code driven approach)
auto temperatureMeasurement = app::Clusters::TemperatureMeasurement::FindClusterOnEndpoint(1);
VerifyOrReturn(temperatureMeasurement != nullptr);
CHIP_ERROR err = temperatureMeasurement->SetMeasuredValue(value);
if (err == CHIP_NO_ERROR)
{
// SetMeasuredValue() succeeded
}
else
{
// SetMeasuredValue() failed
}
|
PR #43204: Size comparison from 3ba5c51 to 173848d Full report (20 builds for cc13x4_26x4, cc32xx, efr32, nrfconnect, nxp, psoc6, qpg, realtek, stm32)
|
andy31415
reviewed
Feb 18, 2026
|
|
||
| // update the temp attribute here for hardcoded endpoint 1 | ||
| chip::app::Clusters::TemperatureMeasurement::Attributes::MeasuredValue::Set(1, static_cast<int16_t>(n * 100)); | ||
| TEMPORARY_RETURN_IGNORED temperatureMeasurement->SetMeasuredValue(static_cast<int16_t>(n * 100)); |
Contributor
There was a problem hiding this comment.
Maybe LogErrorOnFailure instead of temporary ignored? I would avoid adding new TEMPORARY_RETURN_IGNORED for new code.
|
PR #43204: Size comparison from c1553ce to 6b7957d Full report (31 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
arielsz71
changed the title
|
PR #43204: Size comparison from c1553ce to 934d157 Increases above 0.2%:
Full report (33 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #43204: Size comparison from 4d6f12f to 248baea Full report (34 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
| @@ -0,0 +1,24 @@ | |||
| # This cluster is currently following a code driven approach. | |||
|
|
|||
| # This means that the Accessors for the attribute MeasuredValue are no longer available. | |||
Contributor
There was a problem hiding this comment.
Getter is still there, right? Just not setter. And getter will not get the "current" value, just the default one.
But if we stop getting it in CodegenIntegration, we could make the getter go away too, right?
Comment on lines
+52
to
+53
| DataModel::Nullable<int16_t> measuredValue{}; | ||
| MeasuredValue::Get(endpointId, measuredValue); |
Contributor
There was a problem hiding this comment.
I'm not sure it makes sense to have a ZAP-configured MeasuredValue... I think we should just start the cluster off with null until the app actually provides a measurement.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Decoupling temperature measurement cluster (code driven cluster implementation)
Related issues
Fixes #43113
Testing
Tested using chip-all-clusters-app and matter-repl.