-
Notifications
You must be signed in to change notification settings - Fork 2.3k
[OnOff] Fix OffWaitTime delayed state off #25172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[OnOff] Fix OffWaitTime delayed state off #25172
Conversation
…llback was called before OnOff off State was set causing OffWaitTime to wrongly be reseted.
PR #25172: Size comparison from 03fc0b5 to d64a06e Increases (20 builds for bl602, bl702, cc13x2_26x2, cc32xx, cyw30739, psoc6, qpg, telink)
Decreases (14 builds for bl602, bl702, cc13x2_26x2, cc32xx, psoc6, telink)
Full report (30 builds for bl602, bl702, cc13x2_26x2, cc32xx, cyw30739, linux, psoc6, qpg, telink)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems plausible, but would be good to have @Rob-Houtepen-Signify take a look....
@bzbarsky-apple I did confirm that the scenario I describe can and does happen. Well could. With this PR this timing issue doesn't happen |
…in the on off server
PR #25172: Size comparison from 03fc0b5 to caeff31 Increases (1 build for cc32xx)
Decreases (1 build for cc32xx)
Full report (1 build for cc32xx)
|
It depends how OffWithEffect is implemented. Other than MoveToLevelWithOnOff which can be interrupted, OffWithEffect cannot be interrupted. So setting the OnOff to False directly, but still have the lighting effect is also fine. |
* Fix issue where OnTime was set to 0 and the updateOnOffTimeCommand callback was called before OnOff off State was set causing OffWaitTime to wrongly be reseted. * Fix expected string in Cirque MobileDeviceTest since The log changed in the on off server
Fix #24920
There is a timing window where
OnOff::Attributes::OnTime:
is set to0
inoffWithEffectCommand
or insetOnOffValue
and the cluster timer call back ->onOffWaitTimeOffEventHandler
->updateOnOffTimeCommand
is run before theAttributes::OnOff::Set
off is done insetOnOffValue
. This problematic timing window is exacerbated byemberAfOnOffClusterLevelControlEffectCallback
delaying the Off state.Fix entails reseting OnOff::Attributes::OnTime only in
setOnOffValue
and only after the OnOff state is finally set to offTested and validate on EFR32 lighting-app by repeating the sequence
And confirmed that off-wait-time is never directly set to 0