Adds allergy/disease sensor platform from Pollen.com #11573

bachya · 2018-01-11T02:11:43Z

Description:

Adds a sensor platform for allergy and disease information from Pollen.com.

Related issue (if applicable): N/A

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#4392

Example entry for `configuration.yaml` (if applicable):

sensor:
  platform: pollen
  zip_code: 80919
  monitored_conditions:
    - allergy_average_forecasted
    - allergy_average_historical
    - allergy_index_today
    - allergy_index_tomorrow
    - allergy_index_yesterday
    - disease_average_forecasted

Checklist:

The code change is tested and works locally.

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

Documentation added/updated in home-assistant.github.io

If the code communicates with devices, web services, or third-party tools:

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

MartinHjelmare

Looks good overall. See comments below.

MartinHjelmare · 2018-01-17T08:20:50Z

homeassistant/components/sensor/pollen.py

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


Please import logging like most modules do.

MartinHjelmare · 2018-01-17T08:21:47Z

homeassistant/components/sensor/pollen.py

+        data.update()
+
+    sensors = []
+    for condition in config.get(CONF_MONITORED_CONDITIONS, []):


This is a required key in the config schema so don't use dict.get.

MartinHjelmare · 2018-01-17T08:30:58Z

homeassistant/components/sensor/pollen.py

+    sensors = []
+    for condition in config.get(CONF_MONITORED_CONDITIONS, []):
+        name, sensor_class, data_key, params, icon = CONDITIONS[condition]
+        sensors.append(globals()[sensor_class](


I don't think you need to use globals. Is there a specific reason you want that?

This so that I can dynamically instantiate objects of a certain type from the CONDITIONS dict. Open to better suggestions!

Can't you just call sensor_class and instantiate? I think that should work.

Sorry I wasn't clear. If you replace the string representing the class name in CONDITIONS with the actual class name you could call it directly from here. You might have to move the conditions dict below the class defines though, so that might be uglier. You decide.

Ah, I see. Putting CONDITIONS at the bottom of the file feels confusing to me (given all other constants are at the top). Given that, are you okay with me keeping as-is?

Yeah, I'm ok with it.

MartinHjelmare · 2018-01-17T08:34:35Z

homeassistant/components/sensor/pollen.py

+
+def average_of_list(list_of_nums, decimal_places=1):
+    """Return the average of a list of ints."""
+    return round(sum(list_of_nums, 0.0)/len(list_of_nums), decimal_places)


Try to use https://docs.python.org/3.4/library/statistics.html#statistics.mean.

MartinHjelmare · 2018-01-17T08:37:01Z

homeassistant/components/sensor/pollen.py

+
+
+def calculate_trend(list_of_nums):
+    """Return the average of a list of ints."""


Stale docstring.

MartinHjelmare · 2018-01-17T08:47:10Z

homeassistant/components/sensor/pollen.py

+
+def calculate_trend(list_of_nums):
+    """Return the average of a list of ints."""
+    ratings = list(


You can achieve the same with a two level nested generator expression and next. That wouldn't be super readable either but I'd prefer that over this.

ratings = next(( r['label'] for n in list_of_nums for r in RATING_MAPPING if r['minimum'] <= n <= r['maximum']), None)

MartinHjelmare · 2018-01-17T08:51:45Z

homeassistant/components/sensor/pollen.py

+    @property
+    def device_state_attributes(self):
+        """Return the device state attributes."""
+        return merge_two_dicts(


Why do you need this? Can't you just update the attributes?

I'm merging common attributes of the base class (e.g. ATTR_ATTRIBUTION) why (pseudo-)dynamic attributes of the subclasses. I've done this before; not good?

Why do you need to copy before updating? You can just do:

self._attr.update({...}) return self._attr

Oh wow, silly on me. Thanks!

MartinHjelmare · 2018-01-17T09:01:52Z

homeassistant/components/sensor/pollen.py

+        from pypollencom.exceptions import HTTPError
+
+        try:
+            self.extended_data = self._client.allergens.extended()


You could break out a function to access a client method inside a try except block, since you do the same thing 6 times.

MartinHjelmare · 2018-01-17T09:05:16Z

homeassistant/components/sensor/pollen.py

+
+    @Throttle(MIN_TIME_UPDATE_INDICES)
+    def update(self):
+        """Update with new AirVisual data."""


Stale docstring.

MartinHjelmare · 2018-01-17T09:05:31Z

homeassistant/components/sensor/pollen.py

+
+    @Throttle(MIN_TIME_UPDATE_INDICES)
+    def update(self):
+        """Update with new AirVisual data."""


Same as above.

bachya · 2018-01-26T17:29:24Z

@MartinHjelmare Need anything else from me on this one?

MartinHjelmare

Looks great!

grantalewis · 2018-02-15T16:37:59Z

I'm getting data from

    monitored_conditions:
      - allergy_index_today
      - allergy_index_tomorrow

but not from

      - allergy_average_forecasted

Anyone else experiencing this?

bachya · 2018-02-15T16:45:03Z

@grantalewis Please open a new bug report and we'll be able to help debug.

bachya requested a review from andrey-git as a code owner January 11, 2018 02:11

homeassistant added new-platform platform: sensor.pollen cla-signed labels Jan 11, 2018

bachya mentioned this pull request Jan 11, 2018

Adds docs for Pollen.com sensor platform home-assistant/home-assistant.io#4392

Closed

2 tasks

MartinHjelmare reviewed Jan 17, 2018

View reviewed changes

bachya force-pushed the allergen-disease branch from 06849e9 to ddd7976 Compare January 18, 2018 20:14

homeassistant added the cla-signed label Jan 18, 2018

bachya added 7 commits January 22, 2018 22:11

Base platform in place

e042787

Logic in place

c3c6d7a

Requirements and coverage

da07fa2

Fixed some linting issues

49cddcf

Small attribute reorganization

c661409

Collaborator-requested changes round 1

75bddfb

Updated documentation

b47223a

bachya force-pushed the allergen-disease branch from ddd7976 to b47223a Compare January 23, 2018 05:11

homeassistant added the cla-signed label Jan 23, 2018

MartinHjelmare approved these changes Jan 26, 2018

View reviewed changes

MartinHjelmare merged commit 5af7666 into home-assistant:dev Jan 26, 2018

bachya mentioned this pull request Jan 26, 2018

Adds docs for Pollen.com sensor platform home-assistant/home-assistant.io#4524

Merged

2 tasks

bachya deleted the allergen-disease branch February 4, 2018 17:36

balloob mentioned this pull request Feb 9, 2018

0.63 #12267

Merged

home-assistant locked and limited conversation to collaborators May 29, 2018

ghost added integration: pollen and removed platform: sensor.pollen labels Mar 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds allergy/disease sensor platform from Pollen.com #11573

Adds allergy/disease sensor platform from Pollen.com #11573

bachya commented Jan 11, 2018 •

edited

Loading

MartinHjelmare left a comment

MartinHjelmare Jan 17, 2018

MartinHjelmare Jan 17, 2018

MartinHjelmare Jan 17, 2018

bachya Jan 17, 2018

MartinHjelmare Jan 17, 2018

MartinHjelmare Jan 17, 2018

bachya Jan 18, 2018

MartinHjelmare Jan 18, 2018

MartinHjelmare Jan 17, 2018

MartinHjelmare Jan 17, 2018

MartinHjelmare Jan 17, 2018

MartinHjelmare Jan 17, 2018

bachya Jan 17, 2018

MartinHjelmare Jan 17, 2018

bachya Jan 18, 2018

MartinHjelmare Jan 17, 2018

MartinHjelmare Jan 17, 2018

MartinHjelmare Jan 17, 2018

bachya commented Jan 26, 2018

MartinHjelmare left a comment

grantalewis commented Feb 15, 2018

bachya commented Feb 15, 2018



		def calculate_trend(list_of_nums):
		"""Return the average of a list of ints."""

Adds allergy/disease sensor platform from Pollen.com #11573

Adds allergy/disease sensor platform from Pollen.com #11573

Conversation

bachya commented Jan 11, 2018 • edited Loading

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

MartinHjelmare left a 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

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

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

bachya commented Jan 26, 2018

MartinHjelmare left a comment

Choose a reason for hiding this comment

grantalewis commented Feb 15, 2018

bachya commented Feb 15, 2018

bachya commented Jan 11, 2018 •

edited

Loading

Example entry for `configuration.yaml` (if applicable):