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

Checking Xiaomi Aqara devices unavailability states #11631

Conversation

PaulAnnekov
Copy link
Contributor

Description:

This PR makes xiaomi aqara component track if each connected device is still available and update state accordingly.

Why is it needed?

Because any device connected to gateway is totally useless w/o this feature, especially such critical ones as leakage, gas, smoke, motion and door sensors. Why useless? Because they can switch off at any point of time because of bug/battery drain and you won't know about this. For days and months you will think they are still working, but they won't. You will understand they are not working, when you will come back from a holiday and see your house is a pool full of water :). I think I don't need further explanation of "Why?", I'm sure you will agree this check is a MUST for EVERY device connected to smart home.

How is it working

Each device is given 2.5 hours timeframe to send report/heartbeat or respond to read. This time is enough to get at least 2 heartbeats. When device shows it's alive making any of these actions - we reset the timer. If it's not - state is set to STATE_UNAVAILABLE. It will trigger state change + you will see the state in UI (screenshot below). When device shows it's alive again, we set state to None (like constructor does).

selection_001

How to get notification when device becomes unavailable?

E.g. use automation to track unavailable state and send an email.

Average heartbeat time periods

magnet 1 hour
motion/sensor_motion.aq2 55 min
switch 1 hour
sensor_wleak.aq1 50 min
gateway 10 sec
sensor_ht 50 min - 1 hour
plug 7 min

Checklist:

  • Remove motion sensor ignores from PyXiaomiGateway.
  • The code change is tested and works locally.

@Danielhiversen this code Danielhiversen/PyXiaomiGateway@973a21d makes motion sensors switch to unavailable state because we ignore heartbeats. I see two issues with this commit:

  • You have written a general-purpose lib to talk to gateway, but you're ignoring essential part of gateway's API. I think it's not responsibility of your library to filter these messages. Users of library should decide what to do with them by themselves.
  • Is this issue still relevant? There were several firmware updates since that moment.

I suggest to remove this filter completely and:

  • Add a config option to the component to ignore heartbeats for motion sensors. That's in case users will find this issue is still relevant and want a fast fix. It's not easy task, because push_data receives data-only, it doesn't know if it's heartbeat or report, we will need to change PyXiaomiGateway.
  • Do nothing. There is a big chance this issue was fixed in the latest firmwares. If users will complain - we will implement 1st option.

What do you this about this?

if is_data or is_voltage:
self.schedule_update_ha_state()
# There is a chance this function will be called simultaneously by 2 different threads
# (SyncThreads) when 'heartbeat' and 'report' messages arrived at the same time.

Choose a reason for hiding this comment

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

line too long (88 > 79 characters)

is_voltage = self.parse_voltage(data)
if is_data or is_voltage:
self.schedule_update_ha_state()
# There is a chance this function will be called simultaneously by 2 different threads

Choose a reason for hiding this comment

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

line too long (94 > 79 characters)

if self._remove_unavailability_tracker:
self._remove_unavailability_tracker()
self._remove_unavailability_tracker = async_track_point_in_utc_time(
self._hass, self._set_unavailable, utcnow() + TIME_TILL_UNAVAILABLE)

Choose a reason for hiding this comment

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

line too long (80 > 79 characters)

from homeassistant.const import (ATTR_BATTERY_LEVEL, EVENT_HOMEASSISTANT_STOP,
CONF_MAC, CONF_HOST, CONF_PORT)
CONF_MAC, CONF_HOST, CONF_PORT, STATE_UNAVAILABLE)

Choose a reason for hiding this comment

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

line too long (83 > 79 characters)

elif model == '86plug':
devices.append(XiaomiGenericSwitch(device, 'Wall Plug',
'status', True, gateway))
'status', True, hass, gateway))

Choose a reason for hiding this comment

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

line too long (82 > 79 characters)

self._remove_unavailability_tracker = None
self._state = STATE_UNAVAILABLE
self.schedule_update_ha_state()

Choose a reason for hiding this comment

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

blank line contains whitespace

@Danielhiversen
Copy link
Member

Great.
What is wrong with Travis?

@PaulAnnekov
Copy link
Contributor Author

@Danielhiversen it's ready to be merged. Merge my PR in PyXiaomiGateway, I will update version here.
P.S. I don't understand how could I add this old submodule in my commit f51ff3d#diff-d3eb3a918c38d80bb0a48ed78d8488ef

@PaulAnnekov
Copy link
Contributor Author

@Danielhiversen you can merge it.

self._is_available = False
self.async_schedule_update_ha_state()

def _track_unavailable(self):
Copy link
Member

Choose a reason for hiding this comment

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

Please add the callback annotation and add async in the naem: _async_track_unavailable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.
I thought these annotations and prefixes are required only when methods are called by internal HA functions, because they check them (at least annotations).

@@ -225,13 +247,33 @@ def device_state_attributes(self):
"""Return the state attributes."""
return self._device_state_attributes

def push_data(self, data):
@callback
def _set_unavailable(self, now):
Copy link
Member

Choose a reason for hiding this comment

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

Please rename this to _async_set_unavailable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

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

Two small comments. Can be merged after they have been addressed 👍 Awesome work 🐬

@Danielhiversen
Copy link
Member

👍

@Danielhiversen Danielhiversen merged commit 3417c6a into home-assistant:dev Jan 23, 2018
self.parse_voltage(device['data'])

def _add_push_data_job(self, *args):
self.hass.async_add_job(self.push_data, *args)
Copy link
Member

Choose a reason for hiding this comment

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

Following the source, it looks like this callback will be called in a thread, so here we should use self.hass.add_job, and not the async version of the same method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it will be called in a thread. I will fix it in 5 hours.

Copy link
Member

Choose a reason for hiding this comment

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

👍

@balloob balloob mentioned this pull request Jan 26, 2018
@home-assistant home-assistant locked and limited conversation to collaborators May 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants