uriyacovy / ESPHome_nuki_lock

ESPHome lock platform for Nuki Smartlock
MIT License
64 stars 20 forks source link

Support door sensor of keyturner #4

Closed mapero closed 2 years ago

mapero commented 2 years ago

Hi,

i added two additional sensors to represent the door sensor.

To map the nuki states to a binary sensor, I decided to use DoorClosed as "false" and all other cases as "true". This ensures that if there is something wrong with the door sensor, it shows open instead of closed (more secure).

Thanks for providing this custom component.

best regards Jochen

uriyacovy commented 2 years ago

Hi @mapero,

Great contribution, thanks! Two things I noticed that should be fixed before pulling:

  1. The new configuration is not really optional. If it's not specified in the yaml, the module does not work. I guess it is related to this->door_sensor_->publish_state but please have a look. It is better not to have breaking changes if they can be avoided.
  2. Please update the first part of README.md, preferably with a screenshot of the entities (including the new ones). Unfortunately, I don't have the door sensor, therefore I cannot put a meaningful screenshot.

Thanks, Uri

mapero commented 2 years ago

Hi @uriyacovy

thanks for the feedback. I have changed the code for the optional sensors (also for the existing ones, they had the same problem) to only publish in case they are present in the configuration.

Updated readme and screenshot accordingly.

Regards Jochen

uriyacovy commented 2 years ago

Looks very good, thanks.