Ignore duplicate state changes GarageDoor HomeKit

[cdce8p]

Description:

Ignore duplicate state chage calls from HomeKit for GarageDoor accessories. Those could happen with the use of HomeKit automations. This PR is necessary after I had to revert the HAP-python update #18069. It's also largely based on the work @adrum did with #17453. Just added the tests for it.

Related issue (if applicable): fixes #17557

Checklist:

• The code change is tested and works locally.
• Local tests pass with tox. Your PR cannot be merged unless tests pass
• Tests have been added to verify that the new code works.

---

CC: @wasp100

[wasp100]

Thanks for addressing this bug. Can you please also implement for locks?

[cdce8p]

@wasp100 I would, but why do you think this is necessary? For garage doors I could reproduce your issue, but that doesn't exist for locks, or am I missing something?

[wasp100]

In my previous testing, when using the homekit lock function, when running a scene to lock all doors at night, it would trigger the relay even though the door was locked. Essentially the same issue that exists with the garage door.

[cdce8p]

Yeah, I does. But that happens with every accessory and I don't really see the negative consequences that exists for the garage door representation.

[wasp100]

Correct, it has no negative effect on the implementation of the garage door opener. The lock function should also be addressed because it poses a security risk, ie, it unlocks an already locked door when running an automation to ensure the doors are locked.

[cdce8p]

I don't see that. Since the lock implementation uses no transitioning states in contrast to the garage door, the only thing that happens is an additional call to lock an already locked door.

[wasp100]

I’ll test again and revert back to you.

[adrum]

@cdce8p HomeKit supports transitioning states for locks. I never got around to experiencing/fixing this scenario because I'm still waiting on a fix for my ZWave locks to properly function.

[cdce8p]

@adrum Technical correct, however we don't set them like we do with the garage door here: https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/homekit/type_covers.py#L48-L53
That's why for locks it's just setting the same state again, whereas for the garage door we would go to opening or closing first.

[adrum]

Yes I assumed this might be the case without digging into it. Should we add support for this temporary state scenario around here: https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/homekit/type_locks.py#L52

[cdce8p]

There is no need to add anything for locks. As I've said, I would just call lock again without any consequences. Thus there is no need to ignore the call. Especially considering that we don't know if the internal state is correct: #17453 (comment)

[adrum]

I now have 2 questions:

1. Since we don't know if the internal state is correct, should we change the code in this PR to only ignore setting the state to opening/closing and still allow the service call to go through? That way, it would continue to function the same as locks do, without getting into the weird state situation.

2. I have absolutely no knowledge on this subject (beyond the scope of this issue as well), but is there anything we can do to improve the reliability of the internal state? Are there any issues/PRs I could read through to get up to speed?

[cdce8p]

To be honest, in most situations the internal state should be correct. At least I haven't seen anything else, since I started using HA.

IMO we should be fine with the way it's now, but if you like to, I can allow the service_call.

[cdce8p]

@adrum I've update the PR to account for 1)

[wasp100]

Hi @cdce8p,
Just updated to 0.83 to test the change for the garage door opener.
This is still not working correctly for me. If the door is closed and I run a scene like Good Night (which closes the open garage, switches off lights etc), the relay is still being triggered, which results in the garage door opening.
The same happens with the lock function. When running the scene, the relay triggers which results in the door unlocking.
Is there any way to not send any command to the device if the last state was closed/ locked, etc??
Thanks for your help thus far.
It worked perfectly when the initial changes were made, but I understand that it was causing problems with lights.

[wasp100]

Would ignoring the service call help resolve this as I’ve never found the internal state to be incorrect.
Could you apply this (ignore service call) to locks as well please?

[cdce8p]

@wasp100 We shouldn't discuss this here, since the PR is already closed. Please open a new issue for this and link me.

However I don't want to give you false hope, I tested it again a moment ago an everything works as expected. I believe the issue is with your setup (more specific the relays). Please post the configs for those too if you open an issue.