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
73.94k stars 30.99k forks source link

Homekit integration send wrong tilt value to Velux Gateway #115096

Closed adattada-samuel closed 3 months ago

adattada-samuel commented 7 months ago

The problem

Homekit integration send wrong value for "Tile close" to Velux Gateway.

As a result I am not able to close/darken the venetian blinds from HomeAssistant.

Thanks for any help, Samuel

Observations: When using Somfy remote or Velux Gateway the tilt command leads to expected 90/-90 values received by the homekit integration.

2024-04-07 10:31:26.563 DEBUG (MainThread) [aiohomekit.controller.abstract] callback ev:{(15, 14): {'value': 90}}

2024-04-07 10:31:32.762 DEBUG (MainThread) [aiohomekit.controller.abstract] callback ev:{(2, 13): {'value': 60.0}}

2024-04-07 10:31:32.765 DEBUG (MainThread) [aiohomekit.controller.abstract] callback ev:{(15, 14): {'value': -90}}

When sending tilt from HomeAssistant, the log from Homekit integration shows 90/0 instead of 90/-90:

2024-04-07 10:33:58.063 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.0.164: raw request: b'PUT /characteristics HTTP/1.1\r\nHost: 192.168.0.164\r\nContent-Length: 52\r\nContent-Type: application/hap+json\r\n\r\n{"characteristics":[{"aid":15,"iid":14,"value":90}]}'

2024-04-07 10:34:01.711 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.0.164: raw request: b'PUT /characteristics HTTP/1.1\r\nHost: 192.168.0.164\r\nContent-Length: 51\r\nContent-Type: application/hap+json\r\n\r\n{"characteristics":[{"aid":15,"iid":14,"value":0}]}'

What version of Home Assistant Core has the issue?

core-2024.1.6

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant Container

Integration causing the issue

Homekit

Link to integration documentation on our website

https://www.home-assistant.io/integrations/homekit_controller

Diagnostics information

home-assistant_homekit_controller_2024-04-07T10-34-27.000Z.log

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 7 months ago

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

Code owner commands Code owners of `homekit_controller` 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 homekit_controller` 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)


homekit_controller documentation homekit_controller source (message by IssueLinks)

adattada-samuel commented 7 months ago

Problem found and solved locally by cleaning up below function in 'cover.py' - now less lines of code and covering all scenarios.

async def async_set_cover_tilt_position(self, **kwargs: Any) -> None:
    """Move the cover tilt to a specific position."""
    tilt_position = kwargs[ATTR_TILT_POSITION]

    if self.is_vertical_tilt:
        minValue = self.service[CharacteristicsTypes.VERTICAL_TILT_CURRENT].minValue
        maxValue = self.service[CharacteristicsTypes.VERTICAL_TILT_CURRENT].maxValue
        # Recalculate tilt_position. Convert from 1-100 scale to arc degree scale respecting possible min/max Values.
        totalRange = maxValue - minValue
        tilt_position = int(tilt_position / 100 * totalRange + minValue)
        # Set tilt_position.
        await self.async_put_characteristics(
            {CharacteristicsTypes.VERTICAL_TILT_TARGET: tilt_position}
        )
    elif self.is_horizontal_tilt:
        minValue = self.service[CharacteristicsTypes.HORIZONTAL_TILT_TARGET].minValue
        maxValue = self.service[CharacteristicsTypes.HORIZONTAL_TILT_TARGET].maxValue  
        # Recalculate tilt_position. Convert from 1-100 scale to arc degree scale respecting possible min/max Values.
        totalRange = maxValue - minValue
        tilt_position = int(tilt_position / 100 * totalRange + minValue)     
        # Set tilt_position.   
        await self.async_put_characteristics(
            {CharacteristicsTypes.HORIZONTAL_TILT_TARGET: tilt_position}
        ) 
red-island commented 6 months ago

Hi @Jc2k, @bdraco - can you please create a PR or give me the permission to do so?

swa72 commented 5 months ago

I think the problem is still present - 2024.6.3

bdraco commented 5 months ago

Hi @Jc2k, @bdraco - can you please create a PR or give me the permission to do so?

No need to ask for permission, feel free to open a PR

adattada-samuel commented 5 months ago

@bdraco Happy to do so, but I believe I do not have the permission to create a branch in this repo ... at least I get a permission error in the console.

Jc2k commented 5 months ago

Happy to do so, but I believe I do not have the permission to create a branch in this repo ... at least I get a permission error in the console.

In GitHub speak, you create a fork. You can make as many branches as you like in your own fork, then make a PR from your repo into ours. None of this requires us to give you permission. It is very bad security practice to give strangers access to your own GitHub repositories directly. Their contributions need to come via pull requests from their personal forks.

Any changes you make will need to pass the tests. It looks like the existing tests aren't adequate and because we don't have the problematic devices, would be really glad to add more tests.

adattada-samuel commented 5 months ago

Ok. Fine with this approach. Will do so.

adattada-samuel commented 3 months ago

@Jc2k, @bdraco - pull request finally done today. Thanks for taking it forward. https://github.com/home-assistant/core/pull/123532