Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Spec-required level control interactions were removed from on/off cluster #13168

Closed
bzbarsky-apple opened this issue Dec 20, 2021 · 3 comments · Fixed by #13338
Closed

Spec-required level control interactions were removed from on/off cluster #13168

bzbarsky-apple opened this issue Dec 20, 2021 · 3 comments · Fixed by #13338
Labels
spec Mismatch between spec and implementation

Comments

@bzbarsky-apple
Copy link
Contributor

Problem

#12507 removed some level control bits from the on/off cluster, claiming they are not spec compliant. But as far as I can tell they are in fact required by the spec.

The requirement is a COMEFROM-style requirement in the level control cluster, so it's not obvious from reading just the on/off cluster. The relevant spec section is "1.6.4.1.1. Effect of On/Off Commands on the CurrentLevel Attribute" in the appclusters spec. The PR says no automated test regressions were caused, but all that says to me is that we have no test coverage for this part of the spec and should add some.

Proposed Solution

Double-check what implements this spec section in our code, write tests for it, make sure we pass those tests.
@bzbarsky-apple bzbarsky-apple added the spec Mismatch between spec and implementation label Dec 20, 2021
@bzbarsky-apple
Copy link
Contributor Author

@mkardous-silabs

@bzbarsky-apple bzbarsky-apple mentioned this issue Dec 20, 2021
Merged
@bzbarsky-apple
Copy link
Contributor Author

@jmartinez-silabs @Damian-Nordic

@mkardous-silabs
Copy link
Contributor

mkardous-silabs commented Dec 20, 2021 •
edited
Loading

@bzbarsky-apple Fix is on it's way for after the holidays with an update on the level control and the OnOff cluster.

@jmartinez-silabs jmartinez-silabs mentioned this issue Jan 6, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
spec Mismatch between spec and implementation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update/Add Level control features based on lastest cluster revision jmartinez-silabs/connectedhomeip
2 participants
@bzbarsky-apple @mkardous-silabs