search

kardia-as / zigpy-zboss

GNU General Public License v3.0
40 stars 4 forks source link

Rename permit_with_key to permit_with_link_key #36

Closed natexornate closed 9 months ago

natexornate commented 10 months ago

I guess the Zigpy project changed this method, so we need to keep up.

Before this change, I got an error like this:

File "/home/nate/.local/lib/python3.10/site-packages/zigpy_cli/radio.py", line 66, in radio app = app_cls(config) TypeError: Can't instantiate abstract class ControllerApplication with abstract method permit_with_link_key

I assume the method was renamed upstream, so let's just rename it too.

giuliocorradini commented 10 months ago

Hello, I tried your patch on the latest version of the project installed via PyPI and it works delightfully. Without this the configuration of ZHA using zigpy-zboss can't be concluded.

You are indeed right, as the discussed method was renamed from permit_with_key to permit_with_link_key with commit b0ffd5b6 on the zigpy repository.

Thanks again!

puddly commented 10 months ago

The method wasn't merely renamed: the signature is different and the purpose is different. Not that it makes a difference if the library doesn't implement the method, but the parameter names should remain consistent so that it can be called properly.

natexornate commented 10 months ago

Good point, I renamed the link_key argument, hopefully that is better.

natexornate commented 9 months ago

Is there more I need to do here?

natexornate commented 9 months ago

Thanks!