home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
70.93k stars 29.58k forks source link

`HMCover.current_cover_position` has a problem with type error or type mismatch #115639

Open marinelay opened 4 months ago

marinelay commented 4 months ago

The problem

https://github.com/home-assistant/core/blob/15ecd3ae31a150ea47a2b22d0ecc21a9146d1d10/homeassistant/components/homematic/cover.py#L46-L67

The return type of current_cover_position is annotated as int or None, but it actually returns only int type in this code.

I believe that it is appropriate to handle when self._hm_get_state() is None type because is_closed checked whether current_cover_position is None or not at line 65.

If not, type annotation should be fixed to None.

Thank you.

What version of Home Assistant Core has the issue?

homeassistant-2024.5.0.dev0

What was the last working version of Home Assistant Core?

homeassistant-2024.5.0.dev0

What type of installation are you running?

Home Assistant Core

Integration causing the issue

No response

Link to integration documentation on our website

No response

Diagnostics information

No response

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

No response

home-assistant[bot] commented 4 months ago

Hey there @pvizeli, mind taking a look at this issue as it has been labeled with an integration (homematic) you are listed as a code owner for? Thanks!

Code owner commands Code owners of `homematic` can trigger bot actions by commenting: - `@home-assistant close` Closes the issue. - `@home-assistant rename Awesome new title` Renames the issue. - `@home-assistant reopen` Reopen the issue. - `@home-assistant unassign homematic` Removes the current integration label and assignees on the issue, add the integration domain after the command. - `@home-assistant add-label needs-more-information` Add a label (needs-more-information, problem in dependency, problem in custom component) to the issue. - `@home-assistant remove-label needs-more-information` Remove a label (needs-more-information, problem in dependency, problem in custom component) on the issue.

(message by CodeOwnersMention)


homematic documentation homematic source (message by IssueLinks)

alexanderkraus commented 2 months ago

I can confirm this issue with one of my HmIP covers. Once the device is in the "None" state, it will not be controllable via Homeassistant anymore until the state is manually reset.

I'm not that proficient in Python, but changing the code as follows does fix it for me:

@property
def current_cover_position(self) -> int | None:                                                                                                                                                                                 
    """Return current position of cover.                                                                                                                                                                                        

    None is unknown, 0 is closed, 100 is fully open.                                                                                                                                                                            
    """                                                                                                                                                                                                                         
+    value = self._hm_get_state()                                                                                                                                                                                                
+    value = None if (not bool(value) and value != 0) else int( value * 100)                                                                                                                                                     
+    return value
-    return int(self._hm_get_state() * 100)