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

Miflora #3053

Merged
merged 23 commits into from Sep 14, 2016
Merged

Miflora #3053

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .coveragerc
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ omit =
homeassistant/components/sensor/lastfm.py
homeassistant/components/sensor/loopenergy.py
homeassistant/components/sensor/mhz19.py
homeassistant/components/sensor/miflora.py
homeassistant/components/sensor/mqtt_room.py
homeassistant/components/sensor/neurio_energy.py
homeassistant/components/sensor/nzbget.py
Expand Down
144 changes: 144 additions & 0 deletions homeassistant/components/sensor/miflora.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
"""
Support for Xiaomi Mi Flora BLE plant sensor.

For more details about this platform, please refer to the documentation at
https://home-assistant.io/components/sensor.miflora/
"""
from datetime import timedelta
import logging

import voluptuous as vol

from homeassistant.components.sensor import PLATFORM_SCHEMA
from homeassistant.helpers.entity import Entity
import homeassistant.helpers.config_validation as cv
from homeassistant.util import Throttle
from homeassistant.const import CONF_MONITORED_CONDITIONS, CONF_NAME


REQUIREMENTS = ['miflora==0.1.4']

LOGGER = logging.getLogger(__name__)
MIN_TIME_BETWEEN_UPDATES = timedelta(seconds=900)
CONF_MAC = "mac"
Copy link
Contributor

Choose a reason for hiding this comment

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

Mixing ' and "

CONF_FORCE_UPDATE = 'force_update'
DEFAULT_NAME = ""
Copy link
Member

Choose a reason for hiding this comment

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

How about DEFAULT_NAME = "Mi Flora"?

Copy link
Author

Choose a reason for hiding this comment

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

If no default is given, the name is retrieved from the device itself. Personally, I like this more than a hard-coded default name.
However, if you're have more than a single plant and your using more than a single sensor, you will usually configure a new for each of them anyway.

Copy link
Member

Choose a reason for hiding this comment

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

So, DEFAULT_NAME would not be needed at all and instead doing the test for an empty string it could be done against None.

Copy link
Author

Choose a reason for hiding this comment

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

That's right. I will change this.

Copy link
Author

Choose a reason for hiding this comment

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

I was thinking about this again. Reading the name from the sensor during initialization isn't a good idea when the sensors temporary isn't available. Therefore I have changed the code name to use a hard-coded default name. (The name in the sensor is usually also always the same, therefore it doesn't help to differentiate between sensors).


# Sensor types are defined like: Name, units
SENSOR_TYPES = {
'temperature': ['Temperature', '°C'],
'light': ['Light intensity', 'lux'],
'moisture': ['Moisture', '%'],
'conductivity': ['Conductivity', 'µS/cm'],
}

PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
vol.Required(CONF_MAC): cv.string,
vol.Required(CONF_MONITORED_CONDITIONS, default=[]):
Copy link
Member

Choose a reason for hiding this comment

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

'default' is unnecessary when the option is required.

Copy link
Author

Choose a reason for hiding this comment

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

Ok

vol.All(cv.ensure_list, [vol.In(SENSOR_TYPES)]),
vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string,
vol.Optional(CONF_FORCE_UPDATE, default=False): cv.boolean,
})


def setup_platform(hass, config, add_devices, discovery_info=None):
"""Setup the MiFlora sensor."""
from miflora import miflora_poller

poller = miflora_poller.MiFloraPoller(config.get(CONF_MAC))
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to catch connection errors and alike if possible during the setup and make the platform setup fail. Otherwise the users will end up with a non-working platform in their instance.

Copy link
Author

Choose a reason for hiding this comment

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

BLE devices support only a single connection. Therefore it is always possible that a lookup fails. Making the platform fail due to a temporary problem will leave the user without a sensor that might be available a few seconds later again. I find this much better than removing a sensor just because it is not available during startup.

Copy link
Member

Choose a reason for hiding this comment

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

Even better if the platform is able to detect changes or recover from a failure during the setup.

Copy link
Author

@ghost ghost Aug 30, 2016

Choose a reason for hiding this comment

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

If a sensor is completely removed, it won't notice that. However, if the sensor is only unavailable for some time, it will get data from it again as soon as it becomes available again (e.g. if the battery is empty).

force_update = config.get(CONF_FORCE_UPDATE)

devs = []
try:
for parameter in config[CONF_MONITORED_CONDITIONS]:
name = SENSOR_TYPES[parameter][0]
unit = SENSOR_TYPES[parameter][1]

prefix = config.get(CONF_NAME)

if prefix == "":
# If no name is given, retrieve the name from the device
# this is usually "Flower mate"
prefix = poller.name()

if len(prefix) > 0:
name = prefix + " " + name
devs.append(MiFloraSensor(poller,
parameter,
name, unit,
force_update))
except KeyError as err:
Copy link
Member

Choose a reason for hiding this comment

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

Can this happen?
Voluptuous makes sure config only has valid sensor types.

Copy link
Author

Choose a reason for hiding this comment

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

Ok

LOGGER.error("Sensor type %s unknown", err)
return False

add_devices(devs)

return True
Copy link
Member

Choose a reason for hiding this comment

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

I think that's not needed.

Copy link
Author

Choose a reason for hiding this comment

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

done



class MiFloraSensor(Entity):
"""Implementing the MiFlora sensor."""

# pylint: disable=too-many-instance-attributes,too-many-arguments
def __init__(self, poller, parameter, name, unit, force_update):
"""Initialize the sensor."""
self.poller = poller
self.parameter = parameter
self._unit = unit
self._name = name
self._state = None
self.data = []
self._force_update = force_update
# Median is used to filter out outliers. median of 3 will filter
# single outliers, while median of 5 will filter double outliers
# Use median_count = 1 if no filtering is required.
self.median_count = 3

@property
def name(self):
"""Return the name of the sensor."""
return self._name

@property
def state(self):
"""Return the state of the sensor."""
return self._state

@property
def unit_of_measurement(self):
"""Return the units of measurement."""
return self._unit
Copy link
Contributor

Choose a reason for hiding this comment

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

If the HASS configured unit system is imperial , we should be doing a conversion from C to F (both unit of measurement and state properties)


@property
def force_update(self):

Choose a reason for hiding this comment

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

farcy v1.1

  • D400: First line should end with a period (not 'e')

Copy link
Author

Choose a reason for hiding this comment

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

This has been fixed, but it seems that this comment isn't updated automatically.

Copy link
Contributor

Choose a reason for hiding this comment

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

Farcy comments on the method definition - your code change is the line below - it's a terrible little bot, has bunch of bugs. Don't worry about it :-)

"""Force update."""
return self._force_update

@Throttle(MIN_TIME_BETWEEN_UPDATES)
def update(self):

Choose a reason for hiding this comment

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

farcy v1.1

  • D202: No blank lines allowed after function docstring (found 1)

"""
Update current conditions.

This uses a rolling median over 3 values to filter out outliers.
"""
try:
data = self.poller.parameter_value(self.parameter)
except IOError:
return

if data:
LOGGER.debug("%s = %s", self.name, data)
self.data.append(data)
else:
LOGGER.debug("Did not receive any data for %s", self.name)

Copy link
Member

Choose a reason for hiding this comment

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

Should there be a 'return' here?

Copy link
Author

Choose a reason for hiding this comment

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

It's not necessary as update won't change anything in this case. But I can add a return, it doesn't hurt.

LOGGER.debug("Data collected: %s", self.data)
if len(self.data) > self.median_count:
self.data = self.data[1:]

if len(self.data) == self.median_count:
median = sorted(self.data)[int((self.median_count - 1) / 2)]
LOGGER.debug("Median is: %s", median)
self._state = median
else:
LOGGER.debug("Not yet enough data for median calculation")
3 changes: 3 additions & 0 deletions requirements_all.txt
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,9 @@ messagebird==1.2.0
# homeassistant.components.switch.mfi
mficlient==0.3.0

# homeassistant.components.sensor.miflora
miflora==0.1.4

# homeassistant.components.discovery
netdisco==0.7.1

Expand Down