Skip to content

added doorsensor to NukiLock#61

Merged
pschmitt merged 4 commits into
pschmitt:masterfrom
pree:doorsensor
Sep 15, 2020
Merged

added doorsensor to NukiLock#61
pschmitt merged 4 commits into
pschmitt:masterfrom
pree:doorsensor

Conversation

@pree
Copy link
Copy Markdown
Contributor

@pree pree commented Sep 4, 2020

I've added the doorsensor state & name to the NukiLock class.
I saw in a different PR that you'd like to have a NukiLockV2 object, since the doorsensor is not present on the V1 Locks.

For this reason I asked in the nuki developer forum what the Bridge-API does with V1-Locks: https://developer.nuki.io/t/door-sensor-state-on-lock-v1/7157

If it just reports as "unknown" or "deactivated" I don't see the needs to split this into two different classes. What do you think about this?

@pschmitt
Copy link
Copy Markdown
Owner

pschmitt commented Sep 4, 2020

I wasn't aware that it would return "unknown". That alleviates most of my concerns.

@pschmitt pschmitt self-requested a review September 4, 2020 18:43
Copy link
Copy Markdown
Owner

@pschmitt pschmitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take a look at what the v1 lock actually returns when I find the time to do so (next week probably - ping me if I forget).

Comment thread pynuki/lock.py Outdated
@pschmitt
Copy link
Copy Markdown
Owner

pschmitt commented Sep 7, 2020

Okay, so I check with my v1 lock, here's the JSON state:

{'deviceType': 0,
 'nukiId': REPLACED,
 'name': 'Front Door',
 'firmwareVersion': '1.9.6',
 'mode': 2,
 'state': 1,
 'stateName': 'locked',
 'batteryCritical': False}

As you can see there are no door sensor fields. So what we really want to do in is_door_sensor_activated is to check for None as well.

@pree
Copy link
Copy Markdown
Contributor Author

pree commented Sep 7, 2020

Thanks for testing!

I've added Unknown & None to the check. Could you test if this is working for V1, it's working fine for V2

@pree
Copy link
Copy Markdown
Contributor Author

pree commented Sep 15, 2020

We've also got an official answer from NUKI that unsupported values are not included in the request. https://developer.nuki.io/t/door-sensor-state-on-lock-v1/7157/2?u=pree

Comment thread pynuki/lock.py Outdated
Comment on lines +13 to +19
@property
def is_door_sensor_activated(self):
return (
self.door_sensor_state != const.STATE_DOORSENSOR_DEACTIVATED
or self.door_sensor_state != const.STATE_DOORSENSOR_UNKNOWN
or self.door_sensor_state != None
)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried your PR with my v1 lock.

In its current state is_door_sensor_activated returns True for my lock.
I suggest to update the method as follows:

Suggested change
@property
def is_door_sensor_activated(self):
return (
self.door_sensor_state != const.STATE_DOORSENSOR_DEACTIVATED
or self.door_sensor_state != const.STATE_DOORSENSOR_UNKNOWN
or self.door_sensor_state != None
)
@property
def is_door_sensor_activated(self):
# Nuki v1 locks don't have a door sensor, therefore the
# door_sensor_state will is unset for them.
if (
not self.door_sensor_state
or self.door_sensor_state == const.STATE_DOORSENSOR_UNKNOWN
):
return
return self.door_sensor_state != const.STATE_DOORSENSOR_DEACTIVATED

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, I added your suggestion. When you're ready I would stash my commits and we can merge :)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I squashed the PR. Thanks for your contribution :)

@pschmitt pschmitt merged commit e69db4d into pschmitt:master Sep 15, 2020
@pschmitt pschmitt mentioned this pull request Sep 15, 2020
@pschmitt
Copy link
Copy Markdown
Owner

Included in 1.4.0: https://pypi.org/project/pynuki/1.4.0/

@chpego
Copy link
Copy Markdown
Contributor

chpego commented Sep 16, 2020

Included too Nuki component HA ? :)

@pschmitt
Copy link
Copy Markdown
Owner

pschmitt commented Sep 16, 2020

Included too Nuki component HA ? :)

No. Not yet. I won't be issuing a PR for this.

@jghaanstra
Copy link
Copy Markdown

jghaanstra commented Sep 22, 2020

I won't be issuing a PR for this.

Just wondering why it will not be included in HA?

@pschmitt
Copy link
Copy Markdown
Owner

Just wondering why it will not be included in HA?

No reason. Feel free to implement it yourself. I have no way to properly test this, that's all.

@pree
Copy link
Copy Markdown
Contributor Author

pree commented Sep 22, 2020

I have a rouge configuration for HA running on my RPi. I will clean it up and create a PR for HA.

@pree pree deleted the doorsensor branch September 29, 2020 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants