Matter Switch: Improve Profile Checking in infoChanged handler#2812
Open
hcarter-775 wants to merge 1 commit intomainfrom
Open
Matter Switch: Improve Profile Checking in infoChanged handler#2812hcarter-775 wants to merge 1 commit intomainfrom
hcarter-775 wants to merge 1 commit intomainfrom
Conversation
hcarter-775
requested review from
NoahCornell,
ctowns,
gharveymn,
greens and
nickolas-deboom
|
Invitation URL: |
Test Results 72 files 487 suites 0s ⏱️ For more details on these failures, see this check. Results for commit 221d26c. |
|
matter-lock_coverage.xml
matter-sensor_coverage.xml
matter-switch_coverage.xml
Minimum allowed coverage is Generated by 🐒 cobertura-action against 221d26c |
gharveymn
reviewed
Feb 25, 2026
Comment on lines
+241
to
+263
| local function profile_changed(latest_profile, previous_profile) | ||
| if latest_profile.id ~= previous_profile.id then | ||
| return true | ||
| end | ||
| for component_id, synced_component in pairs(latest_profile.components or {}) do | ||
| local prev_component = previous_profile.components[component_id] | ||
| if prev_component == nil then | ||
| return true | ||
| end | ||
| -- Build a table of capability IDs from the previous component. Then, use this map to check | ||
| -- that all capabilities in the synced component existed in the previous component. | ||
| local prev_cap_ids = {} | ||
| for _, capability in pairs(prev_component.capabilities or {}) do | ||
| prev_cap_ids[capability.id] = true | ||
| end | ||
| for _, capability in pairs(synced_component.capabilities or {}) do | ||
| if not prev_cap_ids[capability.id] then | ||
| return true | ||
| end | ||
| end | ||
| end | ||
| return false | ||
| end |
Contributor
There was a problem hiding this comment.
What do we want to happen if latest_profile is a strict subset of previous_profile? It looks like this function will say that the profile hasn't changed in that case.
Contributor
Author
There was a problem hiding this comment.
oh duh, the previous version of the function checked that the size of the two was equal, but that was removed in this latest iteration since #table doesn't work for keyed tables. Gonna fix that up.
Contributor
There was a problem hiding this comment.
Ah ok, I think you'll also need to do the same in the comparison for synced_component vs prev_component. Thinking we should add a deep_compare to the utils...
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
2 participants
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.
Description of Change
After hotfixing a revert to these changes, this PR re-adds the work from #2775 + the fixes in #2807.
Remove the MODULAR_PROFILE_UPDATED flag, which is prone to race-y conditions, and replace it with the more robust profile_changed() function across all Matter drivers that currently utilize the flag.
Further, update the current profile_changed() implementation to more accurately identify component-capability differences.
Summary of Completed Tests
Unit test added to specify that the profile_changed function works as expected.