level_control: bump TestLevelControlDeviceLogic cluster revision to 6#453
Merged
ivmarkov merged 1 commit intoMay 18, 2026
Merged
Conversation
`LevelControlHandler::validate()` panics unless `H::CLUSTER.revision` is 6, and the Matter 1.5.1 IDL declares LevelControl at revision 6. The in-tree `TestLevelControlDeviceLogic` still declared revision 5, so any caller (including `examples/src/bin/speaker.rs`) that exercised `init()` panicked at startup with "incorrect version number: expected 6 got 5". The sibling `TestOnOffDeviceLogic` already tracks revision 6; this brings LevelControl in line. Also adds a small regression test that constructs the LevelControl + OnOff handler pair the same way `speaker.rs` does, so future drift between `TestLevelControlDeviceLogic::CLUSTER` and what `LevelControlHandler::validate()` requires is caught at CI time rather than at first runtime use.
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the LevelControl cluster revision from 5 to 6 in the test module and adds a new test case, test_logic_passes_handler_validate, to ensure consistency between the device logic and the handler during initialization. I have no feedback to provide.
|
PR #453: Size comparison from 7d96427 to 85318f1 Increases above 0.2%:
Full report (8 builds for (core), dimmable-light, onoff-light, onoff-light-bt, speaker)
|
ivmarkov
approved these changes
May 18, 2026
snabb
added a commit
to snabb/rs-matter
that referenced
this pull request
May 23, 2026
Matter 1.5.1 §1.6 lists the LevelControl cluster at revision 7. Upstream project-chip#453 bumped the local `validate()` check and the `TestLevelControlDeviceLogic` reference impl from 5 to 6, but the spec value is one higher. Bring both to 7 so device-side `validate()` accepts spec-conformant cluster definitions.
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
LevelControlHandler::validate()panics unlessH::CLUSTER.revision == 6, and the Matter 1.5.1 IDL declares LevelControl at revision 6. The in-treeTestLevelControlDeviceLogicstill declared revision 5, so any caller that exercisesinit()panics at startup with:examples/src/bin/speaker.rsL99–115 hits this — it constructs the handler withnew(...) + .init(...). The siblingTestOnOffDeviceLogicalready tracks revision 6, so this just brings LevelControl in line.Changes
rs-matter/src/dm/clusters/app/level_control.rs:1778—with_revision(5)→with_revision(6).speaker.rsdoes, so future drift betweenTestLevelControlDeviceLogic::CLUSTERand whatLevelControlHandler::validate()requires is caught at CI time rather than at first runtime use.References
rs-matter/src/dm/clusters/app/level_control.rsL290–296 — validator (if H::CLUSTER.revision != 6 { panic!(...) }).rs-matter-codegen/src/idl/parser/controller-clusters-V1.5.1.0.matterL557–558 — IDL says revision 6.Test plan
cargo test --package rs-matter --lib dm::clusters::app::level_control::tests::— passes.