Miflora #3053

ghost · 2016-08-30T05:14:24Z

Description: Support for Mi Flora plant sensor

Related issue (if applicable): fixes #

Pull request in home-assistant.io with documentation (if applicable): not yet done

Example entry for configuration.yaml (if applicable):

sensor
  platform: miflora
  mac: xx:xx:xx:xx:xx:xx
  name: Flower 1
  force_update: false
  median: 3
  monitored_conditions:
   - moisture
   - light
   - temperature
   - conductivity

Checklist:

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated in home-assistant.io

If code communicates with devices, web services, or a:

Local tests with tox run successfully. Your PR cannot be merged unless tests pass
New dependencies have been added to the REQUIREMENTS variable ([example][ex-requir]).
New dependencies are only imported inside functions that use them ([example][ex-import]).
New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
New files were added to .coveragerc.

Get name from sensor, if not defined

…into miflora

balloobbot · 2016-08-30T05:15:09Z

homeassistant/components/sensor/miflora.py

+        return self._unit
+
+    @property
+    def force_update(self):


farcy v1.1

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

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

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 :-)

Danielhiversen · 2016-08-30T07:30:20Z

homeassistant/components/sensor/miflora.py

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


'default' is unnecessary when the option is required.

Danielhiversen · 2016-08-30T07:33:21Z

homeassistant/components/sensor/miflora.py

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


Should there be a 'return' here?

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

Danielhiversen · 2016-08-30T07:34:28Z

Would it be possible to support several sensors?

ghost · 2016-08-30T07:35:20Z

Yes, you can just define multiple sensors in the configuration. Every sensor has a different MAC address.

Danielhiversen · 2016-08-30T07:45:17Z

Have you tested with multiple sensors?

ghost · 2016-08-30T07:46:30Z

No, I only have a single sensor here (waiting for more to be delivered), but I don't see any reason why it shouldn't work with multiple sensors.

fabaff · 2016-08-30T18:54:42Z

homeassistant/components/sensor/miflora.py

+MIN_TIME_BETWEEN_UPDATES = timedelta(seconds=900)
+CONF_MAC = "mac"
+CONF_FORCE_UPDATE = 'force_update'
+DEFAULT_NAME = ""


How about DEFAULT_NAME = "Mi Flora"?

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.

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

That's right. I will change this.

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).

…into miflora

Teagan42 · 2016-08-30T23:57:46Z

homeassistant/components/sensor/miflora.py

+
+LOGGER = logging.getLogger(__name__)
+MIN_TIME_BETWEEN_UPDATES = timedelta(seconds=900)
+CONF_MAC = "mac"


Mixing ' and "

Teagan42 · 2016-08-31T00:00:55Z

You will need to add unit test or add the files to .coveragerc - tests are much preferred.

Teagan42 · 2016-08-31T00:01:19Z

Unrelated - I wish I knew about these a few months ago when I started my indoor garden. Maybe next go round :-)

ghost · 2016-08-31T04:38:11Z

Units tests can't be implemented here as this would not only require a Linux system with a Bluetooth stack but also a working sensor. Both isn't available on the test system.

Teagan42 · 2016-08-31T04:47:03Z

Then you need to add the files to .coveragerc

On Aug 30, 2016 10:38 PM, "Open Home Automation" notifications@github.com
wrote:

Units tests can't be implemented here as this would not only require a
Linux system with a Bluetooth stack but also a working sensor. Both isn't
available on the test system.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#3053 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AC2fZf3VR_Ei4UJ5x1DupmC65T2UVQyCks5qlQU9gaJpZM4JwJvn
.

ghost · 2016-08-31T04:49:12Z

The sensor file is already added to .coveragerc

Teagan42 · 2016-08-31T05:51:51Z

Fuck I'm tired - totally missed it.

On Tue, Aug 30, 2016 at 10:49 PM, Open Home Automation <
notifications@github.com> wrote:

The sensor file is already added to .coveragerc

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#3053 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AC2fZYPuA-KMZnX-ZWXsseBKRy-U-Glkks5qlQfSgaJpZM4JwJvn
.

Teagan42 · 2016-08-31T05:52:39Z

So the only real issue is the mixing of quotes.

…into miflora

- set state to None if no data could be polled from sensor

more logging

Teagan42 · 2016-09-04T16:41:36Z

homeassistant/components/sensor/miflora.py

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


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)

Danielhiversen · 2016-09-11T17:04:25Z

Is this ready to merge now?

ghost · 2016-09-12T16:40:52Z

@Danielhiversen From my point of view it is ready to merge.

balloob · 2016-09-14T05:37:55Z

Looks good! 🐬

@Teagan42

* First version of the MiFlora sensor (not yet finished) * First workign version * Added some documentation Get name from sensor, if not defined * Ignore IOError * Added force_update option * Updated comments * Renamed fertility to conductivity (what it really is) * MiFlora library update * Updated helper files * Formatting * Fixed pylint errors * Removed default from monitored conditions * Removed KeyError handling as a KeyError should never be raised * Added a return when no data is received * emoved unnecessary return statement * Changed default name * Changes quotes and string operation ( @Teagan42 ) * - number of samples for median calculation is now configurable - set state to None if no data could be polled from sensor * Bugfix in library more logging * Fixed miflora version number

lwis · 2016-09-19T11:26:24Z

@open-homeautomation could you add the website docs for this, please?

ghost · 2016-09-19T11:34:08Z

Yes, I will work on this.

ghost · 2016-09-19T12:08:36Z

home-assistant/home-assistant.io#960

growers12 and others added 10 commits August 25, 2016 14:45

First version of the MiFlora sensor (not yet finished)

4ce655b

First workign version

1315b29

Added some documentation

7fb0f39

Get name from sensor, if not defined

Ignore IOError

b7627e5

Added force_update option

9bbaf9f

Merge branch 'dev' of https://github.com/home-assistant/home-assistant …

b2348be

…into miflora

Updated comments

a2cb7c4

Renamed fertility to conductivity (what it really is)

f21ee64

MiFlora library update

e4bd461

Updated helper files

5d734f8

balloobbot reviewed Aug 30, 2016
View reviewed changes

Daniel Matuschek and others added 2 commits August 30, 2016 07:19

Formatting

9ae9078

Fixed pylint errors

6c6561c

Danielhiversen reviewed Aug 30, 2016
View reviewed changes

Removed default from monitored conditions

55f073f

Danielhiversen reviewed Aug 30, 2016
View reviewed changes

hifiberry added 2 commits August 30, 2016 09:38

Removed KeyError handling as a KeyError should never be raised

ed65d27

Added a return when no data is received

7476045

fabaff reviewed Aug 30, 2016
View reviewed changes

Daniel Matuschek added 3 commits August 30, 2016 21:04

emoved unnecessary return statement

40e1199

Merge branch 'dev' of https://github.com/home-assistant/home-assistant …

be1ffac

…into miflora

Changed default name

8577a75

Teagan42 reviewed Aug 30, 2016
View reviewed changes

growers12 and others added 5 commits August 31, 2016 10:09

Merge branch 'dev' of https://github.com/home-assistant/home-assistant …

0f98d1d

…into miflora

Changes quotes and string operation ( @Teagan42 )

cd475c8

- number of samples for median calculation is now configurable

babc288

- set state to None if no data could be polled from sensor

Bugfix in library

376e300

more logging

Fixed miflora version number

04f5c1b

Teagan42 reviewed Sep 4, 2016
View reviewed changes

Danielhiversen added this to the 0.29 milestone Sep 13, 2016

balloob merged commit 0c7c85d into home-assistant:dev Sep 14, 2016

home-assistant locked and limited conversation to collaborators Mar 17, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Miflora #3053

Miflora #3053

ghost commented Aug 30, 2016 •

edited by ghost

Loading

balloobbot Aug 30, 2016

ghost Aug 30, 2016

Teagan42 Aug 30, 2016

Danielhiversen Aug 30, 2016

ghost Aug 30, 2016

Danielhiversen Aug 30, 2016

ghost Aug 30, 2016

Danielhiversen commented Aug 30, 2016

ghost commented Aug 30, 2016 •

edited by ghost

Loading

Danielhiversen commented Aug 30, 2016

ghost commented Aug 30, 2016 •

edited by ghost

Loading

fabaff Aug 30, 2016

ghost Aug 30, 2016

fabaff Aug 30, 2016

ghost Aug 30, 2016

ghost Aug 30, 2016

Teagan42 Aug 30, 2016

Teagan42 commented Aug 31, 2016

Teagan42 commented Aug 31, 2016

ghost commented Aug 31, 2016

Teagan42 commented Aug 31, 2016

ghost commented Aug 31, 2016

Teagan42 commented Aug 31, 2016

Teagan42 commented Aug 31, 2016

Teagan42 Sep 4, 2016

Danielhiversen commented Sep 11, 2016

ghost commented Sep 12, 2016

balloob commented Sep 14, 2016

lwis commented Sep 19, 2016

ghost commented Sep 19, 2016

ghost commented Sep 19, 2016

Miflora #3053

Miflora #3053

Conversation

ghost commented Aug 30, 2016 • edited by ghost Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Danielhiversen commented Aug 30, 2016

ghost commented Aug 30, 2016 • edited by ghost Loading

Danielhiversen commented Aug 30, 2016

ghost commented Aug 30, 2016 • edited by ghost Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Teagan42 commented Aug 31, 2016

Teagan42 commented Aug 31, 2016

ghost commented Aug 31, 2016

Teagan42 commented Aug 31, 2016

ghost commented Aug 31, 2016

Teagan42 commented Aug 31, 2016

Teagan42 commented Aug 31, 2016

Choose a reason for hiding this comment

Danielhiversen commented Sep 11, 2016

ghost commented Sep 12, 2016

balloob commented Sep 14, 2016

lwis commented Sep 19, 2016

ghost commented Sep 19, 2016

ghost commented Sep 19, 2016

ghost commented Aug 30, 2016 •

edited by ghost

Loading

ghost commented Aug 30, 2016 •

edited by ghost

Loading

ghost commented Aug 30, 2016 •

edited by ghost

Loading