[ColorControl]Add Quiet reporting to the CurrentHue, CurrentSaturation, EnhancedCur… #34544
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
…rentHue, CurrentX, CurrentY and RemainingTime attributes of the colorcontrol cluster server implementation
|
Review changes with SemanticDiff. |
pullapprove
bot
requested review from
andyg-apple,
anush-apple,
arkq,
axelnxp,
bauerschwan,
bzbarsky-apple,
carol-apple,
chapongatien,
chrisdecenzo,
chshu,
chulspro,
cliffamzn,
Damian-Nordic,
dhrishi,
doru91,
fessehaeve,
harimau-qirex,
harsha-rajendran,
hawk248,
hicklin,
jepenven-silabs,
jmeg-sfy,
joonhaengHeo,
jtung-apple,
kiel-apple and
kkasperczyk-no
pullapprove
bot
requested review from
sharadb-amazon,
tcarmelveilleux,
tecimovic,
tehampson,
tima-q,
tobiasgraf,
turon,
vivien-apple,
wiba-nordic,
woody-apple,
younghak-hwang,
yufengwangca and
yunhanw-google
|
PR #34544: Size comparison from 693ffba to 4f0dcc4 Increases above 0.2%:
Full report (67 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink)
|
lpbeliveau-silabs
approved these changes
Jul 26, 2024
|
PR #34544: Size comparison from 693ffba to b5521d2 Increases above 0.2%:
Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
andy31415
reviewed
Jul 26, 2024
|
PR #34544: Size comparison from 693ffba to 5b6cd83 Increases above 0.2%:
Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
andy31415
approved these changes
Jul 29, 2024
| dirtyState = quietReporter.SetValue(newValue, now, predicate); | ||
| } | ||
|
|
||
| return (dirtyState == AttributeDirtyState::kMustReport) ? MarkAttributeDirty::kIfChanged : MarkAttributeDirty::kNo; |
There was a problem hiding this comment.
This looks wrong. If kMustReport is true, you want MarkAttributeDirty::kYes, because the IfChanged value will do the wrong thing because it does not know when things were last reported.
There was a problem hiding this comment.
Filed #34618 to track.
| // - kMarkDirtyOnIncrement : When the value increases. | ||
| if (quietRemainingTime[epIndex].SetValue(newRemainingTime, now) == AttributeDirtyState::kMustReport) | ||
| { | ||
| markDirty = MarkAttributeDirty::kIfChanged; |
j-ororke
pushed a commit
to j-ororke/connectedhomeip
that referenced
this pull request
Jul 31, 2024
…n, EnhancedCur… (project-chip#34544) * WIP * Add Quiet reporting to the CurrentHue, CurrentSaturation, EnhancedCurrentHue, CurrentX, CurrentY and RemainingTime attributes of the colorcontrol cluster server implementation * Add a constructor in QuieterReporting that works with arrays * address comment
rochaferraz
pushed a commit
to rochaferraz/connectedhomeip
that referenced
this pull request
Jul 31, 2024
…n, EnhancedCur… (project-chip#34544) * WIP * Add Quiet reporting to the CurrentHue, CurrentSaturation, EnhancedCurrentHue, CurrentX, CurrentY and RemainingTime attributes of the colorcontrol cluster server implementation * Add a constructor in QuieterReporting that works with arrays * address comment
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.
…rentHue, CurrentX, CurrentY and RemainingTime attributes of the color control cluster server implementation
CurrentHue, CurrentSaturation, EnhancedCurrentHue, CurrentX and CurrentY attributes are reported on at most every second, but usually 4 times throughout the transition or at transition start and end
RemainingTime is reported on value increase, or when it changes from or to 0
fixes #33863
Tests:
Existing Color control cert tests.
Manual test by subscribing to the attributes