Skip to content

Add Environment Canada weather, sensor, and camera platforms#21110

Merged
balloob merged 27 commits into
home-assistant:devfrom
michaeldavie:ec_weather
Jun 6, 2019
Merged

Add Environment Canada weather, sensor, and camera platforms#21110
balloob merged 27 commits into
home-assistant:devfrom
michaeldavie:ec_weather

Conversation

@michaeldavie
Copy link
Copy Markdown
Contributor

@michaeldavie michaeldavie commented Feb 16, 2019

Description: Add platforms for Environment Canada weather data to the weather, sensor, and camera components.

**Related issue (if applicable): N/A

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

Example entry for configuration.yaml (if applicable):

weather:
  - platform: environment_canada
sensor:
  - platform: environment_canada
camera:
  - platform: environment_canada

Checklist:

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

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

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@MatthewFlamm
Copy link
Copy Markdown
Contributor

MatthewFlamm commented Feb 19, 2019

It looks like you are taking a day+night forecast and chopping out the night time piece except for the low temperature to fit within the current component choices of daily or hourly. I am working on allowing a day+night mode in the front end for my own in-progress platform. I can submit a PR for this piece now if it helps here.

See here for what my proposal is
home-assistant/architecture#75 (comment)

Also, as with every other weather platform I've looked at, this one will only show correct values in one unit system, I assume metric.

@michaeldavie
Copy link
Copy Markdown
Contributor Author

Yes, I'm only using the high and low temperatures. There is a lot more data available, but I fit it into the existing UI. You're correct that I'm only presenting the data in metric; that's the standard in Canada and is the only way the data is published.

@MatthewFlamm
Copy link
Copy Markdown
Contributor

I think at the very least this needs to be an explicit limitation in the documentation if imperial units won't be supported.

@michaeldavie
Copy link
Copy Markdown
Contributor Author

michaeldavie commented Feb 21, 2019

I tested it in imperial mode, and it does present the temperatures correctly in ºF. It doesn't look like the weather component supports units or conversions for the other types of data (i.e. wind speed, pressure, and visibility).

Comment thread homeassistant/components/environment_canada/sensor.py Outdated
@michaeldavie michaeldavie changed the title Add Environment Canada weather platform Add Environment Canada weather, sensor, and camera components Mar 9, 2019
@michaeldavie michaeldavie changed the title Add Environment Canada weather, sensor, and camera components Add Environment Canada weather, sensor, and camera platforms Mar 9, 2019
@michaeldavie
Copy link
Copy Markdown
Contributor Author

I've had a few users testing things out (see https://community.home-assistant.io/t/reviewers-needed-environment-canada-component-with-weather-sensor-and-camera-radar-platforms/59561), and I decided to merge the weather, sensor, and camera platforms under this PR. Things are running quite smoothly, and I'd like to get this merged in 0.90 if possible. Please let me know if you have any questions or concerns.

@davidbb
Copy link
Copy Markdown
Contributor

davidbb commented Mar 11, 2019

I've been testing this platform as a custom component on 0.88 and 0.89 without issues. This platform follows the style of other national weather service platforms.

Canadian Home Assistant users will be pleased to see the integration of official weather data. 🇨🇦 Thanks for the hard work @michaeldavie

@glassbase
Copy link
Copy Markdown

Great work on this. Been using since your first pull request back last year...

I think you should drop the EC from the name in sensors.py. Most people are probably only following one weather platform so distinguishing with EC in name does not make sense (to me anyways).

Comment thread homeassistant/components/environment_canada/sensor.py Outdated
@michaeldavie
Copy link
Copy Markdown
Contributor Author

@balloob, would it be possible to get this merged? It's running stably with quite a few users, and the documentation has been brought up to date. Please let me know if there's anything else I need to do. Thanks in advance for your help.

@balloob balloob merged commit fcfbdd2 into home-assistant:dev Jun 6, 2019
@balloob balloob mentioned this pull request Jun 26, 2019
@michaeldavie michaeldavie deleted the ec_weather branch July 21, 2019 03:15
@MatthewFlamm MatthewFlamm mentioned this pull request Aug 5, 2019
9 tasks
alandtse pushed a commit to alandtse/home-assistant that referenced this pull request Oct 12, 2019
…sistant#21110)

* Added Environment Canada weather platform

* Added Environment Canada weather platform

* Migrate to new folder structure

* Migrate to new folder structure

* Fix updates

* Fix updates again

* Bump env_canada to 0.0.4

* Bump env_canada to 0.0.4

* Bump env_canada to 0.0.4 in requirements_all.txt

* Change daily forecast timestamp and high/low test

* Change daily forecast timestamp and high/low test

* Bump env_canada to 0.0.5

* Break alerts into multiple sensors, bump env_canada to 0.0.6

* Bump env_canada to 0.0.7

* Remove blank line

* Remove 'ec' sensor prefix, bump env_canada to 0.0.8

* Corrections

* Change to manifests.json

* Add docstring to __init.py__

* Update CODEOWNERS

* pylint correction

* pylint correction

* Add alert details, bump env_canada to 0.0.9

* Update requirements_all.txt

* Update .coveragerc

* Bump env_canada to 0.0.10

* Update requirements_all.txt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.