Improvements to Cover entity handling

[jeverley]

Aims to address the issue where a cover gets stuck in opening/closing state, also corrects some other bugs I've encountered whilst refactoring the code.

Changes

• Implements movement timeout and previous sate comparison to determine device instigated changes
  • The timeout is initially calculated based on expected travel distance because some devices only report new position after stopping
  • A device position update will reduce the timeout to the default (5 seconds)
  • Tilt has a lower dynamic timeout range due to shorter physical travel distance on the tilt axis
• Removed target position entity attributes (we don't need to restore these)
• An open/moving tilt state takes precedence over a static lift state
• Utilise CoverState class in const instead of string literals for state (follows approach used in core)

Fixes

• Fix issue where the cover gets stuck in opening/closing state
  • This was because previous targets were never being cleared
  • Devices could also fail to reach the target position due to physical factors
• Fix the _attr_device_class logic for Rollershade devices
  • This was resulting in the default icon mdi:window-open rather than mdi:roller-shade being used for the entity

Addresses issues

• ZHA : Cover states stays as Opening or Closing when using Set position home-assistant/core#98933
  • cover "is closing" / "is opening" not working home-assistant/core#137327
  • Refactor Aqara Roller Driver E1 as v2 quirk to expose configuration and status entities zha-device-handlers#3686 (comment)
  • ewand cover not reporting open/close state correctly through ZHA home-assistant/core#93836
  • [Device Support Request] Insta GmbH Nexentro Blinds Actuator Mini zha-device-handlers#1397
• Rollershade cover entities fail to get their device_class attribute #378

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.49%. Comparing base (29db30d) to head (467d47a).
Report is 3 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #376      +/-   ##
==========================================
+ Coverage   96.42%   96.49%   +0.06%     
==========================================
  Files          61       61              
  Lines        9644     9774     +130     
==========================================
+ Hits         9299     9431     +132     
+ Misses        345      343       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jeverley]

I've finished making amendments to this.

[puddly]

Thanks!

I've merged dev into this PR in a separate branch (dev...puddly:zha:cover-platform-entity-status), since as of yesterday we now more explicitly recompute entity capabilities in a dedicated method and have explicit lifecycle hooks for attaching/detaching listeners.

I have a ThirdReality blind and am noticing nothing unusual when testing this PR out. The only thing is the cover state sometimes jittering "opening -> open -> opening -> open" in the last second, but this is still an improvement over the previous functionality where the cover sometimes got "stuck" if an attribute report was missed.

[jeverley]

I have a ThirdReality blind and am noticing nothing unusual when testing this PR out. The only thing is the cover state sometimes jittering "opening -> open -> opening -> open" in the last second, but this is still an improvement over the previous functionality where the cover sometimes got "stuck" if an attribute report was missed.

Thanks @puddly!

Thinking about it, what you describe might be a side effect of the tolerance logic.

i.e.

• Go to 95% command
• The device reports a new position of 94% (95 - tolerance).
  • The handler would conclude it is open (close enough) and clear the target
• The device then immediately reports 95% (the actual finishing point)
  • The handler interprets this as device instigated movement (because the target had been cleared), starting a new 5 second timeout

Setting the tolerance to 0 would allow us to test this hypothesis.

If that's the root cause I suspect we could remove the tolerance because we now have the 5 second timeout after no position update.
Previously it waited forever if the device stopped early/over ran, getting stuck in opening/closing.

Edited for clarity.

[dmulcahey]

I have a ThirdReality blind and am noticing nothing unusual when testing this PR out. The only thing is the cover state sometimes jittering "opening -> open -> opening -> open" in the last second, but this is still an improvement over the previous functionality where the cover sometimes got "stuck" if an attribute report was missed.

I wonder if we can implement something like the light transitioning logic we have…. Maybe it isn’t worth the effort / complexity.

[jeverley]

I have a ThirdReality blind and am noticing nothing unusual when testing this PR out. The only thing is the cover state sometimes jittering "opening -> open -> opening -> open" in the last second, but this is still an improvement over the previous functionality where the cover sometimes got "stuck" if an attribute report was missed.

I wonder if we can implement something like the light transitioning logic we have…. Maybe it isn’t worth the effort / complexity.

I think I might have fixed this particular issue with 517822e, but I'll take a look at the methods in the light cluster handler this morning and see if there's some learnings we could apply.

[jeverley]

I'll be pushing an update to this tomorrow with split methods for handling tilt/lift transition state timeout once I've finished testing it locally.

Trying to handle them both in a single timer creates more problems than it solves with devices supporting both axis.

[jeverley]

Hi @puddly, @dmulcahey dad925c incorporates the separate timers for lift/tilt movement transitions.

This ensures we correctly handle concurrent movement of the two axis and their transition states.

I've added a test to explicitly validate this scenario performs as expected.