Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New functionality: consider config.on for state updates #66

Closed
jasperslits opened this issue May 25, 2020 · 13 comments
Closed

New functionality: consider config.on for state updates #66

jasperslits opened this issue May 25, 2020 · 13 comments

Comments

@jasperslits
Copy link

Hi,

First of all thanks for your work on Deconz integration with HA.

One of the annoying bits of the Deconz REST API by dresden elektronik is that state updates are still sent for binary sensors which are turned on as mentioned here: dresden-elektronik/deconz-rest-plugin#90

My use-case is that I have a Philips motion sensor (SML001) in the living room which I want to silence when I am at home during the day and use it only in the night and early morning to trigger some automations. If it's not silenced it spams the log and pollutes HA.

I managed to get this to work by editing the DeconzBinarySensor class in /homeassistant/components/deconz/binary_sensor.py and changed async_update_callback to check 'on':

if self._device.on and ( force_update or self._device.changed_keys.intersection(keys) ):

This works but it seems better if it was implemented in pydeconz/sensor.py class Presence and the state method returns true and considers the config.on

I am happy to make a PR for either pydeconz or homeassistant if you're OK with the suggested modification. It's cleaner if it's implemented in Deconz REST API directly but that seems to take forever and I my C++ skills are rusty.

I use REST commands to turn the config.on to true / false so it's a bit more cumbersome but it works.

@Kane610
Copy link
Owner

Kane610 commented May 25, 2020

Sure, I can take that into consideration, can you clarify if this would work the same for all types of entities?

@jasperslits
Copy link
Author

According http://dresden-elektronik.github.io/deconz-rest-doc/sensors/ upon sensor creation it defaults config.on to True and it's part of the JSON GET response upon querying sensors.

So it's safe to assume that it's available for all sensors and it doesn't seem to distinct between the types one has available in HA.

@jasperslits
Copy link
Author

For the PR I can create: do you think this change should go into pydeconz package or the deconz/binary_sensor in the HA component?

@Kane610
Copy link
Owner

Kane610 commented May 26, 2020

I would like the change to be as generic as possible so I need to look into that

@jasperslits
Copy link
Author

Got it, I have quite a few sensors (presence, temperature from Philips and Xiaomi etc) linked to Deconz so I can help with testing if you want to implement it yourself

@Kane610
Copy link
Owner

Kane610 commented May 26, 2020

Awesome!

@jasperslits
Copy link
Author

Any idea when you'll have time for it?

I gave the C++ change a try (rest_sensors.cpp) and the code compiles yet it doesn't emit anything in the debugging log. And as Deconz is running in Docker, replacing the .so plugin is quite complex. It should be possible to suppress the firing of the event according to the Deconz Discord channel but it seems quite tricky. So I think the Python solution would probably be easier to going

@Kane610
Copy link
Owner

Kane610 commented Jun 30, 2020

No sorry, I'm currently on vacation, also focusing on the Axis integration

@jasperslits
Copy link
Author

Hi,

How do you want to go forward with this?
I modified is_tripped method of the Presence class in /usr/local/lib/python3.8/site-packages/pydeconz/sensor.py to read:

   @property    
    def is_tripped(self):
        """Sensor is tripped."""
        return self.presence and self.on

That works as expected and I can create a PR for that unless you think that handling this config.on should be handled in the Deconz cpp code? This change is limited to the motion sensor because I dont know a good user story for the config.on with other sensors like temperature, light level or water etc.

@Kane610
Copy link
Owner

Kane610 commented Sep 15, 2020

That is way too specific to do. Im looking into if I can better fit this into hass integration rather than library

@Kane610
Copy link
Owner

Kane610 commented Sep 23, 2020

Have you considered to just use the "on" attribute as a condition in your automations

@jasperslits
Copy link
Author

Great suggestion and Version 0.115 to the rescue as I can now put a condition on the state attribute “on” with true / false via the UX.

I had 0.115 running since launch but didn’t realize this “problem” could be solved with just config.

Closing this as this new functionality request is no longer relevant.

@Kane610
Copy link
Owner

Kane610 commented Sep 23, 2020

Awesome!

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

No branches or pull requests

2 participants