Matter Window Covering: Support the tilt feature

[nickolas-deboom]

Type of Change

• WWST Certification Request
  • If this is your first time contributing code:
    • I have reviewed the README.md file
    • I have reviewed the CODE_OF_CONDUCT.md file
    • I have signed the CLA
  • I plan on entering a WWST Certification Request or have entered a request through the WWST Certification console at developer.smartthings.com
• Bug fix
• New feature
• Refactor

Checklist

• I have performed a self-review of my code
• I have commented my code in hard-to-understand areas
• I have verified my changes by testing with a device or have communicated a plan for testing
• I am adding new behavior, such as adding a sub-driver, and have added and run new unit tests to cover the new behavior

Description of Change

CHAD-14539

This change adds support for the tilt feature in the WindowCovering cluster, to subscribe to the CurrentPositionTiltPercent100ths attribute and map this value to the the windowShadeTiltLevel capability.

Summary of Completed Tests

Tested with the VDA Window Covering device. This has recently been updated to support tilt in addition to lift. Also see new unit tests.

[github-actions]

Duplicate profile check: Passed - no duplicate profiles detected.

[github-actions]

Channel deleted.

[github-actions]

Test Results

   64 files    409 suites   0s ⏱️
2 032 tests 2 032 ✅ 0 💤 0 ❌
3 511 runs  3 511 ✅ 0 💤 0 ❌

Results for commit 57ffd97.

♻️ This comment has been updated with latest results.

[github-actions]

File	Coverage
All files	75%	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-window-covering/src/init.lua	91%	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-window-covering/src/matter-window-covering-position-updates-while-moving/init.lua	36%	❌

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against 57ffd97

[SathvikGatti]

@nickolas-deboom I think the position values of both tilt and lift must be considered on every report when deciding the window shade's status as currently the shade's status is based solely on the attribute that was last reported.
For example:

1. Lift is at 100%. Window Shade says "Open".
2. Move Tilt to 0%. Window Shade would now be shown as "Closed".

Please let me know if I'm overlooking anything, or if this is intentional.

[nickolas-deboom]

Hi @SathvikGatti , I think you are correct, and I made an update to consider both lift and tilt on each position update. The lift/tilt values are captured each time current_pos_handler runs, and new logic is introduced with a few considerations for shades that support both lift and tilt. For example, if the lift is at 100%, the tilt value does not matter, the shade will be considered open. Similarly, if the lift is at 0%, the shade is only considered closed if the tilt value is also 0% (it would be partially open for any other tilt value).

[SathvikGatti]

@nickolas-deboom, thanks for the update! There is a scenario where the issue persists when a device with both Tilt and Lift features that is also handled by sub-driver is onboarded.

This issue is seen as the lift attribute handling is done in the sub-driver, while the tilt reports are handled in the base/main driver. Due to this, the window shade status shown is based on the attribute that was last reported.

Currently, only the Virtual Device App is listed in the sub-driver's can_handle. Should this be handled as well?

CC: @HunsupJung

[HunsupJung]

@SathvikGatti
I'm against modifying to sub driver. VDA's role is to test drivers before actual products are released. Virtual window covering is using a sub driver, which is only used to test edge cases. I don't think it's a good direction. VDA should use the main driver used by most real devices. There are two ways to do that.

1. Remove the VDA from the sub-driver and modify the VDA to operate like a device that uses a main driver.
2. Create a new virtual window covering using the main driver.

I think we should decide between the two.

[nickolas-deboom]

@HunsupJung I agree with you and think the VDA should probably be updated to be compatible with the main driver.

[nickolas-deboom]

I agree and that way the VDA will continue to work in the meantime!

[SathvikGatti]

@nickolas-deboom sometimes there's an issue seen with window shade status observed with VDA window covering having tilt feature and probably could be seen with real devices.
Currently VDA window covering reports tilt and lift values once the movement is complete. I'll try to explain with an example, pardon me for the lengthy explanation :)

Assuming lift and tilt are set to 100, on invoking "Close", tilt and lift move towards 0. Depending on what attribute's report is received (tilt/lift) different window shade status can be emitted twice in a short span of time.
Driver receives tilt's report set to 0 first:
windowShadeTiltLevel="0"
windowShade="\open"

The window shade status set to "Open" is updated as current tilt "0" is compared with previous lift "100". This is because the latest value of lift is not received yet.

Then on receiving lift's report(0) from device "closed" is emitted
shadeLevel="0"
windowShade="\closed"

Since there were two window shade statuses emitted within a short span (arround 200ms) sometimes capability events displayed in ST-App's event history are not in order and the first sent status I.e "Open" is shown.

CC: @HunsupJung

[hcarter-775]

@SathvikGatti so if I'm understanding correctly, there are two orders that these events can be sent in.

The first:

1. tilt received for 0 - leaving it as "open"
2. lift received for 0 - correctly showing it as "closed"

The second:

1. lift received for 0 - showing it as "partially open"
2. tilt received for 0 - correctly showing it as "closed"

Both of these paths leave the device in the "closed" state in the end, so I am confused what case you are describing.

Edit: I think I see what you are saying now; because the VDA sends reports multiple times while closing, it is possible that it one of its open commands is received after the final closed command.

If this is what you are describing, this should be seen as an error with the VDA device. There is no reason to be sending updates that quickly where issues like this arise, and real devices generally should not be handled like this.

[nickolas-deboom]

I think they meant that there is no guaranteed order of the capabilities being processed and so the final state of the capability in the app could appear as either open or closed (for scenario 1) after both capabilities are emitted. This is a good point and I think we may need to make an update with this consideration.

[SathvikGatti]

yes, even though the right window shade status is reported at the end, sometimes the capability events are not processed in the right order and the final status shown is not correct.

[hcarter-775]

one discussion is still open, but I think this is good to go since it is a VDA issue we are discussing there.

[nickolas-deboom]

After testing with the VDA, which now supports tilt and also works with the main driver now, I was not able to reproduce the issue where capabilities are processed out of order on the cloud side. This is possible, but probably unlikely with a real window covering device, since they wouldn't be likely to have lift and tilt finish updating at exactly the same time like the VDA does. Therefore, I would like to move forward with this PR and re-address the potential side-affect noted in this PR if we see this occur with a real device.

[nickolas-deboom]

Also, the subdriver can now be removed by a separate PR, since the VDA was updated to work with the main driver.