[HVAC] Rename thermostat-server to ThermostatCluster#41739
Open
hasty wants to merge 6 commits intoproject-chip:masterfrom
Open
[HVAC] Rename thermostat-server to ThermostatCluster#41739hasty wants to merge 6 commits intoproject-chip:masterfrom
hasty wants to merge 6 commits intoproject-chip:masterfrom
Conversation
|
PR #41739: Size comparison from 5d26b5d to dea25b8 Full report (4 builds for nrfconnect, realtek, stm32)
|
|
PR #41739: Size comparison from 5d26b5d to c01e80a Full report (35 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
…ter tests to change it
…r_driven_thermostat
|
PR #41739: Size comparison from cfc90c8 to 88f9f42 Full report (35 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
|
This was referenced Dec 8, 2025
…driven_thermostat # Conflicts: # examples/all-clusters-app/linux/main-common.cpp
|
PR #41739: Size comparison from efdf99d to 19a5426 Full report (35 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
hasty
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This draft PR renames the thermostat-server cluster implementation to follow the code-driven cluster naming template (ThermostatCluster pattern), similar to other modernized clusters. The PR also changes the default MinSetpointDeadBand value from 25 (2.5°C) to 20 (2.0°C) across all configuration files and updates the corresponding test suite to restore this value after testing.
Key Changes
- Renamed thermostat-server files to ThermostatCluster naming pattern (e.g.,
thermostat-server.cpp→ThermostatCluster.cpp,thermostat-delegate.h→ThermostatDelegate.h) - Updated MinSetpointDeadBand default from 0x19 (25, representing 2.5°C) to 0x14 (20, representing 2.0°C) in server code and all configuration files
- Enhanced test suite TC_TSTAT_2_2.py to properly restore the MinSetpointDeadBand attribute after testing
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/app/clusters/thermostat-server/app_config_dependent_sources.gni | Updates build file list with renamed source files; missing ThermostatSuggestionStructWithOwnedMembers files |
| src/app/clusters/thermostat-server/app_config_dependent_sources.cmake | Updates CMake build file list with renamed source files; inconsistent ordering with .gni file |
| src/app/clusters/thermostat-server/ThermostatDelegate.h | Newly added file (renamed from thermostat-delegate.h) |
| src/app/clusters/thermostat-server/ThermostatClusterSuggestions.cpp | Updates include paths to use renamed header files |
| src/app/clusters/thermostat-server/ThermostatClusterPresets.h | Updates include path for ThermostatDelegate.h |
| src/app/clusters/thermostat-server/ThermostatClusterPresets.cpp | Updates include paths to use renamed header files |
| src/app/clusters/thermostat-server/ThermostatClusterAtomic.cpp | Updates include path for ThermostatCluster.h |
| src/app/clusters/thermostat-server/ThermostatCluster.h | Updates include path for ThermostatDelegate.h |
| src/app/clusters/thermostat-server/ThermostatCluster.cpp | Updates include path and changes kDefaultDeadBand from 25 to 20 |
| src/python_testing/TC_TSTAT_2_2.py | Updates MinSetpointDeadBandValue constant and adds restoration logic after testing |
| scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h | Updates MinSetpointDeadBand default value from 0x19 to 0x14 |
| scripts/tools/zap/tests/inputs/all-clusters-app.zap | Updates MinSetpointDeadBand defaultValue from 0x19 to 0x14 |
| scripts/py_matter_idl/matter/idl/tests/inputs/large_all_clusters_app.matter | Updates MinSetpointDeadBand default and removes trailing blank lines |
| examples/thermostat/thermostat-common/thermostat.zap | Updates MinSetpointDeadBand defaultValue from 0x19 to 0x14 |
| examples/thermostat/thermostat-common/thermostat.matter | Updates MinSetpointDeadBand default from 0x19 to 0x14 |
| examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h | Updates include path for ThermostatDelegate.h |
| examples/thermostat/qpg/zap/thermostaticRadiatorValve.zap | Updates MinSetpointDeadBand defaultValue from 0x19 to 0x14 |
| examples/thermostat/qpg/zap/thermostaticRadiatorValve.matter | Updates MinSetpointDeadBand default from 0x19 to 0x14 |
| examples/thermostat/linux/main.cpp | Updates include path for ThermostatCluster.h |
| examples/placeholder/linux/apps/app2/config.zap | Updates MinSetpointDeadBand defaultValue from 0x19 to 0x14 |
| examples/placeholder/linux/apps/app2/config.matter | Updates MinSetpointDeadBand default from 0x19 to 0x14 |
| examples/placeholder/linux/apps/app1/config.zap | Updates MinSetpointDeadBand defaultValue from 0x19 to 0x14 |
| examples/placeholder/linux/apps/app1/config.matter | Updates MinSetpointDeadBand default from 0x19 to 0x14 |
| examples/all-clusters-app/realtek/data_model/all-clusters-app.zap | Updates MinSetpointDeadBand defaultValue from 0x19 to 0x14 |
| examples/all-clusters-app/realtek/data_model/all-clusters-app.matter | Updates MinSetpointDeadBand default from 0x19 to 0x14 |
| examples/all-clusters-app/linux/main-common.cpp | Updates include path for ThermostatCluster.h |
| examples/all-clusters-app/all-clusters-common/all-clusters-app.zap | Updates MinSetpointDeadBand defaultValue from 0x19 to 0x14 |
| examples/all-clusters-app/all-clusters-common/all-clusters-app.matter | Updates MinSetpointDeadBand default from 0x19 to 0x14 |
| "ThermostatClusterPresets.cpp", | ||
| "ThermostatClusterPresets.h", | ||
| "ThermostatClusterSuggestions.cpp", | ||
| "ThermostatDelegate.h", |
There was a problem hiding this comment.
The file ordering is inconsistent between app_config_dependent_sources.gni and app_config_dependent_sources.cmake. The .gni file is missing ThermostatSuggestionStructWithOwnedMembers.cpp and ThermostatSuggestionStructWithOwnedMembers.h which are present in the .cmake file (lines 28-29).
Both build configuration files should include the same source files. Add these two files to the .gni file to maintain consistency.
Suggested change
| "ThermostatDelegate.h", | |
| "ThermostatDelegate.h", | |
| "ThermostatSuggestionStructWithOwnedMembers.cpp", | |
| "ThermostatSuggestionStructWithOwnedMembers.h", |
Comment on lines
14
to
24
| app_config_dependent_sources = [ | ||
| "PresetStructWithOwnedMembers.cpp", | ||
| "PresetStructWithOwnedMembers.h", | ||
| "thermostat-delegate.h", | ||
| "thermostat-server-atomic.cpp", | ||
| "thermostat-server-presets.cpp", | ||
| "thermostat-server-presets.h", | ||
| "thermostat-server-suggestions.cpp", | ||
| "thermostat-server.cpp", | ||
| "thermostat-server.h", | ||
| "ThermostatCluster.cpp", | ||
| "ThermostatCluster.h", | ||
| "ThermostatClusterAtomic.cpp", | ||
| "ThermostatClusterPresets.cpp", | ||
| "ThermostatClusterPresets.h", | ||
| "ThermostatClusterSuggestions.cpp", | ||
| "ThermostatDelegate.h", | ||
| ] |
There was a problem hiding this comment.
The file ordering differs between app_config_dependent_sources.gni and app_config_dependent_sources.cmake. While both should include the same files, they should also maintain a consistent order for better maintainability.
In the .gni file:
- ThermostatCluster.cpp/h come first (lines 17-18)
- ThermostatDelegate.h comes last (line 23)
In the .cmake file:
- ThermostatDelegate.h comes first (line 21)
- ThermostatCluster.cpp/h come last (lines 26-27)
Consider adopting a consistent alphabetical ordering or logical grouping (e.g., main cluster files first, then auxiliary files, then delegate interfaces) across both build systems.
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
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 a draft PR for renaming thermostat-manager to more closely fit the code-driven cluster template.
Testing
Ran test-suite locally.
Readability checklist
The checklist below will help the reviewer finish PR review in time and keep the
code readable:
descriptive
“When in Rome…”
rule (coding style)