Matter Window Covering: Update shade status via latest shade level when movement halts#3132
Matter Window Covering: Update shade status via latest shade level when movement halts#3132hcarter-775 wants to merge 1 commit into
Conversation
59036d0 to
e364737
Compare
|
Invitation URL: |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against e364737 |
Test Results 73 files 536 suites 0s ⏱️ Results for commit e364737. |
hcarter-775
changed the title
hcarter-775
changed the title
| local preset_position = device:get_field(PRESET_LEVEL_KEY) or | ||
| (device.preferences ~= nil and device.preferences.presetPosition) or | ||
| DEFAULT_PRESET_LEVEL | ||
| local preset_position = device.preferences.presetPosition or DEFAULT_PRESET_LEVEL |
There was a problem hiding this comment.
Whats the difference here between use of the preference and use of windowShadePreset? This is removing the use of the windowShadePreset capability; perhaps we should be trying to get the state for the preset position rather than just using the preference value.
Any idea why both the capability and preference exist?
There was a problem hiding this comment.
In this work several months ago, this preference was ripped out: 07532ad in favor of the new capability. So this stuff is only being triggered in the case that a device has not yet populated the capability. Once that is done, the capability is the only thing that is evaluated.
There was a problem hiding this comment.
Thanks for the context on that. I see the get_latest_state check now too, that makes it so this only runs if the cap isnt populated.
2 participants
Description of Change
If an operational status of 0 is received (not moving), we should emit an event setting the
windowShadeLeveloperational status (open, closed, etc). This is to ensure that a final status is handled in the case that a device sends any kind of "movement" operational status after the last current position is sent.This is a follow-up to this PR that also attempts to resolve: #3043, without adding a timer to the handling. This timer is less ideal, since responsiveness is slow by definition of the timer with little value (as there is another route), reads do fail at times, and read timers adds extra time state to keep in mind, which can complicate the handlers.
Tag-along updates:
Several fields are not needed, so I have removed them to add greater clarity to the updated handling.
CURRENT_LIFT/CURRENT_TILTare fields that store some latest attribute states during runtime. A field is not required for this.REVERSE_POLARITYstored the state ofdevice.preferences.reverse, which is already stored in the data model. Therefore this is not needed. As it is persisted, it needs to be made nil to remove.PRESET_LEVEL_KEYis only ever used (get_field) when the capability state is nil, and is therefore nil in that context as well. So we don't need this.In
info_changed, we should not try to update the device profile on any non-profile update, as the inventories can be synced for many reasons. In fact, the only case this would be required for is a software update, so I have altered the logic to explicitly check for that- this has been validated with identical handling in Matter Switch, and a unit test has been updated to validate this as well.Summary of Completed Tests
Unit tests updated.