[Draft] Conversion of Thermostat to Code-Driven Cluster#42325
Draft
hasty wants to merge 128 commits into
Draft
Conversation
…ter tests to change it
…r_driven_thermostat
github-actions
Bot
added
examples
scripts
darwin
app
tests
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 undertakes a significant refactoring of the Thermostat cluster, converting it to the modern code-driven implementation style. This is a large change that touches many example applications, ZAP configurations, and the core cluster logic itself. The changes correctly move attribute and command handling from the old attribute access interface and Ember callbacks into a new ThermostatCluster class. The new implementation is well-structured, separating concerns into different files for read, write, setpoints, presets, and atomic operations.
My review has identified a couple of areas for improvement:
- A copy-paste error in a new header file that should be corrected.
- An inconsistency in error logging during cluster initialization that could hinder debugging.
Overall, this is a solid first draft for this major conversion. The new structure is much cleaner and aligns with the current SDK architecture.
|
PR #42325: Size comparison from efdf99d to c2d2a9d Full report (3 builds for realtek, stm32)
|
…driven_thermostat # Conflicts: # examples/all-clusters-app/linux/main-common.cpp
|
PR #42325: Size comparison from efdf99d to c1a8831 Full report (3 builds for realtek, stm32)
|
|
PR #42325: Size comparison from efdf99d to 0438f4a Full report (3 builds for realtek, stm32)
|
|
PR #42325: Size comparison from efdf99d to 6b81f9b Full report (3 builds for realtek, stm32)
|
|
PR #42325: Size comparison from cb38d7b to 623dfa6 Full report (3 builds for realtek, stm32)
|
|
PR #42325: Size comparison from cb38d7b to 9c92d71 Full report (21 builds for bl602, bl702, bl702l, cc13x4_26x4, realtek, stm32, telink)
|
|
PR #42325: Size comparison from 852d166 to a262f00 Full report (21 builds for bl602, bl702, bl702l, cc13x4_26x4, realtek, stm32, telink)
|
|
PR #42325: Size comparison from b2c44f5 to 1de2b7c Full report (3 builds for realtek, stm32)
|
|
PR #42325: Size comparison from 089617b to 64b0640 Full report (13 builds for bl602, bl702, bl702l, cc13x4_26x4, realtek, stm32)
|
hasty
force-pushed
the
granbery/code_driven_thermostat
branch
from
f79aa02 to
823d966
Compare
|
PR #42325: Size comparison from 8a162c6 to 42b7c97 Full report (23 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, nrfconnect, realtek, stm32, telink)
|
|
PR #42325: Size comparison from 8a162c6 to 7367915 Full report (23 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, nrfconnect, realtek, stm32, telink)
|
|
|
|
PR #42325: Size comparison from 8a162c6 to ca989c8 Increases above 0.2%:
Full report (32 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, esp32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
|
PR #42325: Size comparison from 8a162c6 to 6965e19 Increases above 0.2%:
Full report (35 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…y/code_driven_thermostat # Conflicts: # src/app/clusters/thermostat-server/Setpoint.h # src/app/clusters/thermostat-server/Setpoints.cpp # src/app/clusters/thermostat-server/Setpoints.h
|
PR #42325: Size comparison from ca3e67c to 74ec094 Increases above 0.2%:
Full report (35 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
|
PR #42325: Size comparison from ca3e67c to 2c72e3c Increases above 0.2%:
Full report (35 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
|
PR #42325: Size comparison from f1767a8 to f067ae6 Increases above 0.2%:
Full report (35 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
|
PR #42325: Size comparison from 9cf1485 to e9a971d Increases above 0.2%:
Full report (25 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, psoc6, qpg, realtek, stm32)
|
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
3 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
This is the initial conversion of the Thermostat cluster to the code-driven style. It replaces the existing functionality which
was previously handled in ember callbacks and the attribute access interface.
It is meant to be merged after #41739.
Testing
Ran test suite