search

uriyacovy / ESPHome_nuki_lock

ESPHome lock platform for Nuki Smartlock
MIT License
60 stars 19 forks source link

Support for multiple locks #2

Open Savjee opened 1 year ago

Savjee commented 1 year ago

Hi,

First up: awesome work! Works flawlessly!

Would it be possible to pair multiple Nuki locks to the same ESP? I'm planning on adding a second lock (frontdoor + backdoor) and place my ESP32 in the middle.

uriyacovy commented 1 year ago

Hi Savjee, Thank you for the feedback.

Regarding your request, I was already considering this but I think it is a bit problematic due to several reasons:

  1. The pairing/unpairing process would be more complicated to the average user.
  2. Testing of this feature would be difficult (while I plan to buy another Nuki, I don't own two devices currently).
  3. It is difficult to assess how good multiple devices are supported by the underlying library (https://github.com/I-Connect/NukiBleEsp32)

Overall, considering that an ESP32 module costs ~$5, I think it does not worth the effort. Nevertheless, I'll keep the issue open (as a feature request), and wait for additional comments.

Uri

Savjee commented 1 year ago

Thanks for your reply!

I don't have a lot of experience with developing for ESPHome, and zero knowledge of BLE, but I do want to give this a go. Here are my initial thoughts, and I'd love your feedback/guidance.

We could create multiple instances of your Nuki implementation and set their deviceId in the YAML. I believe this is supported by the NukiBLE library you're using (not sure though how you set the deviceId in your code. Seems like it's fixed to 2020002?)

Example:

lock:
  - platform: nuki_lock
    name: Front door
    device_id: 1234567  # Available in Nuki app
  - platform: nuki_lock
    name: Back door
    device_id: 987654  # Available in Nuki app

As for your other remarks:

  1. I believe the current pairing process could be kept. The logic would be: if the user has not defined a device_id himself, follow the regular pairing process.
  2. Agreed. I'm planning on buying a second one soon, so I could test it if you want.
  3. That's true. However, at first glance, it's seems well designed.

I know that ESP32's are cheap, and I have several of them around the house. But it seems wasteful to have 1 for each lock. I like to consolidate as much as possible.

uriyacovy commented 1 year ago
  1. The device id should be fixed, since it represents the bridge device id and not the smart lock device id (see "Authorization Data" in the Nuki bridge API doc).
  2. If everything is implemented right, the only thing you need to do to support multiple locks is to pass a unique device name for each lock. You can do this by initializing Nuki::NukiBle in nuki_lock.cpp with _this->getname() instead of _this->deviceName__

I suggest that once you get the 2nd nuki, test the changes and I'll pull the PR if everything goes right.

mkotek commented 1 year ago

+1 on 2 locks spport

uriyacovy commented 1 year ago

I appreciate the feedback and the request, but currently, due to scheduling, IO and other complexities when working with BT and ESPHome, I do not plan support for two locks. I'd leave the issue open to get further feedback.