DataModelProvider: add mark dirty for WriteAttribute function #36988
+42
−3
Conversation
This file contains 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
|
Review changes with |
|
PR #36988: Size comparison from dedef1f to 80145a3 Full report (70 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
andy31415
requested changes
Jan 10, 2025
There was a problem hiding this comment.
This needs some design considerations: data model providers should own the data and I am unsure about the syntax of control of when something is dirty: why is the write itself not able to determine dirtyness of things.
This feels like a workaround/hackish at a first glance: adding more things into parameters and we should stop that practice.
Worst case (and I am not sure we should) this kind of flag should be part of the WriteAttributeRequest structure instead since that contains the write request parameters.
|
@wqx6 could you provide me with some code links and references about the behaviour that causes issues here? It seems like we want quiet reporting in LevelControl ... but then could we solve it at the cluster level using https://github.com/project-chip/connectedhomeip/blob/master/src/app/cluster-building-blocks/QuieterReporting.h ? |
https://github.com/project-chip/connectedhomeip/blob/master/src/app/clusters/level-control/level-control.cpp#L412 In level control or color control cluster, we are using AttributeAccessors(such as |
|
PR #36988: Size comparison from 5bd63d5 to ecea71f Full report (70 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
Why would you be replacing that with a WriteAttribute? Wouldn't you just update the cluster state as needed? |
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.
Some write operations in data model provider should not mark the attribute as dirty.
For example, CurrentLevel attribute in LevelControl cluster should not be reported frequently during a transition after the device receives a LevelMove command. Some changes during the transition should be quiet and not reported to the subscriber. So we need to add a markDirty for the WriteAttribute function in DataModelProvider.
Testing
Unit test should pass