[CodeDriven] Migrate Relative Humidity Measurement cluster to code-driven implementation#71424
Open
Elen777300 wants to merge 19 commits intoproject-chip:masterfrom
Open
Conversation
![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 implements the Relative Humidity Measurement cluster as a code-driven server. The changes include the core cluster logic with attribute constraint validation, integration delegates for the codegen data model, and a suite of unit tests. A review comment identifies a likely compilation error in the Attributes method where kMandatoryMetadata requires a namespace prefix to be correctly resolved.
src/app/clusters/relative-humidity-measurement-server/RelativeHumidityMeasurementCluster.cpp
Show resolved
Hide resolved
Elen777300
force-pushed
the
relative-humidity-measurement-code-driven
branch
from
3298e5c to
a2a644e
Compare
Elen777300
force-pushed
the
relative-humidity-measurement-code-driven
branch
2 times, most recently
from
ff4f9ed to
c956ccd
Compare
|
PR #71424: Size comparison from da57c2e to c956ccd Full report (9 builds for cc13x4_26x4, cc32xx, realtek, stm32)
|
Elen777300
force-pushed
the
relative-humidity-measurement-code-driven
branch
from
c956ccd to
cdc9eb6
Compare
Elen777300
changed the title
Relative Humidity Measurement cluster to code-driven implementation
|
PR #71424: Size comparison from da57c2e to dd7daa4 Full report (21 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, nrfconnect, psoc6, qpg, realtek, stm32)
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #71424 +/- ##
==========================================
+ Coverage 54.32% 54.33% +0.01%
==========================================
Files 1577 1580 +3
Lines 108257 108344 +87
Branches 13401 13403 +2
==========================================
+ Hits 58806 58872 +66
- Misses 49451 49472 +21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
PR #71424: Size comparison from da57c2e to ba2e81c Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Elen777300
force-pushed
the
relative-humidity-measurement-code-driven
branch
from
ba2e81c to
f1609ea
Compare
|
PR #71424: Size comparison from 5f91738 to f1609ea Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
src/app/clusters/relative-humidity-measurement-server/RelativeHumidityMeasurementCluster.h
Show resolved
Hide resolved
|
PR #71424: Size comparison from 5f91738 to 4366a9b Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
andy31415
reviewed
Apr 7, 2026
examples/air-purifier-app/air-purifier-common/include/relative-humidity-sensor-manager.h
Outdated
Show resolved
Hide resolved
andy31415
reviewed
Apr 7, 2026
src/app/clusters/relative-humidity-measurement-server/CodegenIntegration.cpp
Outdated
Show resolved
Hide resolved
andy31415
reviewed
Apr 7, 2026
src/app/clusters/relative-humidity-measurement-server/RelativeHumidityMeasurementCluster.cpp
Outdated
Show resolved
Hide resolved
andy31415
approved these changes
Apr 7, 2026
Contributor
|
@Elen777300 the flash cost of this seems to be 1.4K for air purifier. Could you check where the cost comes from? I wonder if we have some combination of pure ember (table driven) to code which is a cost but also if we have more validation code than before. Anything we can do to make this smaller in size? |
Elen777300
force-pushed
the
relative-humidity-measurement-code-driven
branch
from
f497eeb to
4ab2609
Compare
|
PR #71424: Size comparison from cdb458b to 67e490f Full report (5 builds for cc32xx, realtek, stm32)
|
|
PR #71424: Size comparison from cdb458b to 69d577d Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
src/app/clusters/relative-humidity-measurement-server/RelativeHumidityMeasurementCluster.cpp
Show resolved
Hide resolved
| RelativeHumidityMeasurementCluster::RelativeHumidityMeasurementCluster(EndpointId endpointId, const Config & config) : | ||
| DefaultServerCluster({ endpointId, RelativeHumidityMeasurement::Id }), mOptionalAttributeSet(config.mOptionalAttributeSet) | ||
| { | ||
| if (!config.minMeasuredValue.IsNull()) |
Contributor
There was a problem hiding this comment.
These bounds check are duplicated here and in the Set methods.
Maybe see if a helper method method would make this easier in the anon namespace?
Contributor
Author
There was a problem hiding this comment.
The checks are actually different
The constructor validates the hardware config once at startup.
SetMeasuredValue validates each new sensor reading against that config at runtime.
They check different things so the helper won't really remove duplication
Contributor
There was a problem hiding this comment.
Would something like this help?
namespace {
bool IsValueInHumidityRange(uint16_t value,
const DataModel::Nullable<uint16_t>& min,
const DataModel::Nullable<uint16_t>& max)
{
if (value > kMeasuredValueMax) return false;
if (!min.IsNull() && value < min.Value()) return false;
if (!max.IsNull() && value > max.Value()) return false;
return true;
}
} // namespace
// Usage in SetMeasuredValue (Returning Error)
if (!measuredValue.IsNull()) {
VerifyOrReturnError(IsValueInHumidityRange(measuredValue.Value(), mMinMeasuredValue, mMaxMeasuredValue),
CHIP_IM_GLOBAL_STATUS(ConstraintError));
}
// Usage in Constructor (Crashing)
if (!config.minMeasuredValue.IsNull()) {
VerifyOrDie(config.minMeasuredValue.Value() <= kMinMeasuredValueMax);
if (!config.maxMeasuredValue.IsNull()) {
VerifyOrDie(IsValueInHumidityRange(config.maxMeasuredValue.Value(), config.minMeasuredValue,
DataModel::MakeNullable(kMeasuredValueMax)));
}
}
soares-sergio
approved these changes
Apr 10, 2026
soares-sergio
added
sdk-maintainer-approved
| namespace app { | ||
| namespace Clusters { | ||
|
|
||
| class RelativeHumiditySensorManager |
Contributor
There was a problem hiding this comment.
Code size still seems high. Could you post the size diff here as comment so we can double check?
Contributor
Author
There was a problem hiding this comment.
ELF Size Diff Breakdown (+1330 bytes)
| Type | Size | Function | Size1 | Size2 |
|---|---|---|---|---|
| REMOVED | -61 | chip::app::Clusters::AirPurifierManager::Init |
543 | 604 |
| REMOVED | -52 | chip::app::Clusters::RelativeHumidityMeasurement::Attributes::MaxMeasuredValue::Set |
0 | 52 |
| REMOVED | -52 | chip::app::Clusters::RelativeHumidityMeasurement::Attributes::MinMeasuredValue::Set |
0 | 52 |
| REMOVED | -49 | chip::app::Clusters::RelativeHumidityMeasurement::Attributes::MeasuredValue::Set |
0 | 49 |
| CHANGED | -8 | attributeData |
86 | 94 |
| CHANGED | 8 | __frame_dummy_init_array_entry |
272 | 264 |
| ADDED | 10 | chip::app::Clusters::RelativeHumidityMeasurementCluster::~RelativeHumidityMeasurementCluster |
10 | 0 |
| ADDED | 11 | std::_Optional_payload_base<unsigned short>::_M_reset |
11 | 0 |
| CHANGED | 15 | MatterClusterServerShutdownCallback |
220 | 205 |
| CHANGED | 16 | MatterClusterServerInitCallback |
218 | 202 |
| ADDED | 24 | chip::app::Clusters::RelativeHumidityMeasurement::Attributes::kMandatoryMetadata |
24 | 0 |
| ADDED | 55 | chip::app::Clusters::RelativeHumidityMeasurement::Attributes::Tolerance::Get |
55 | 0 |
| ADDED | 56 | chip::app::Clusters::RelativeHumidityMeasurement::FindClusterOnEndpoint |
56 | 0 |
| ADDED | 58 | MatterRelativeHumidityMeasurementClusterShutdownCallback |
58 | 0 |
| ADDED | 59 | chip::app::Clusters::RelativeHumidityMeasurement::SetMeasuredValue |
59 | 0 |
| ADDED | 69 | chip::app::Clusters::RelativeHumidityMeasurementCluster::Attributes |
69 | 0 |
| ADDED | 71 | MatterRelativeHumidityMeasurementClusterInitCallback |
71 | 0 |
| ADDED | 81 | chip::app::Clusters::RelativeHumidityMeasurement::Attributes::MaxMeasuredValue::Get |
81 | 0 |
| ADDED | 81 | chip::app::Clusters::RelativeHumidityMeasurement::Attributes::MinMeasuredValue::Get |
81 | 0 |
| ADDED | 100 | chip::app::LazyRegisteredServerCluster<chip::app::Clusters::RelativeHumidityMeasurementCluster>::Destroy |
100 | 0 |
| ADDED | 132 | chip::app::Clusters::RelativeHumidityMeasurementCluster::SetMeasuredValue |
132 | 0 |
| ADDED | 133 | chip::ChipError chip::app::AttributeValueEncoder::Encode<chip::app::DataModel::Nullable<unsigned short>, true> |
133 | 0 |
| ADDED | 136 | vtable for chip::app::Clusters::RelativeHumidityMeasurementCluster |
136 | 0 |
| ADDED | 148 | chip::app::Clusters::RelativeHumidityMeasurementCluster::ReadAttribute |
148 | 0 |
| ADDED | 289 | chip::app::Clusters::RelativeHumidityMeasurementCluster::RelativeHumidityMeasurementCluster |
289 | 0 |
| TOTAL | 1330 | 923885 | 922555 |
src/app/clusters/relative-humidity-measurement-server/RelativeHumidityMeasurementCluster.h
Outdated
Show resolved
Hide resolved
src/app/clusters/relative-humidity-measurement-server/README.md
Outdated
Show resolved
Hide resolved
| This cluster uses the code-driven approach. The Ember attribute accessors for | ||
| `MeasuredValue`, `MinMeasuredValue`, `MaxMeasuredValue`, and `Tolerance` are no | ||
| longer available. | ||
|
|
Contributor
There was a problem hiding this comment.
Should also include info on instantiating the cluster directly in their application, instead of relying on CodegenIntegration.
|
PR #71424: Size comparison from cdb458b to e2e7c0b Full report (21 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, nrfconnect, psoc6, qpg, realtek, stm32)
|
Elen777300
force-pushed
the
relative-humidity-measurement-code-driven
branch
from
f47a68a to
2396c9f
Compare
|
PR #71424: Size comparison from 6cfa747 to b31a09d Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
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
5 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
Converts the Relative Humidity Measurement cluster from a pure-Ember implementation to the code-driven pattern using DefaultServerCluster.
Related issues
#71423
Testing
Readability checklist