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

Swedish weather institute weather component #16717

Merged
merged 7 commits into from Oct 8, 2018

Conversation

@helto4real
Contributor

helto4real commented Sep 19, 2018

Description:

This is a new weather component that access public API data from the Swedish weather institute (SMHI). (Creative Commons Attribution 4.0 International License)

This component will only accept locations in Sweden.

This component use the new config_flow only. It supports using several locations by providing a coordinate (lat/lon) can be configured and weather data will be provided for these locations.

Swedish people are used to trust the public service institute and I think a component like this will be welcome.

I personally use this for automation and both SMHI and Yr.no has this as percent of sky that has clouds. Second best to use light sensors.

Related issue (if applicable): fixes #

**Pull request in home-assistant.github.io with documentation: home-assistant/home-assistant.io#6375

Example entry for configuration.yaml (if applicable):

No configuration. Only config through config_flow

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

Not able to run the whole tox suite on windows dev machine. Ran tests related to weather components and passed.

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][ex-requir]).
  • New dependencies are only imported inside functions that use them ([example][ex-import]).
  • 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.

My first PR ever so be gentle ladies and gentlemen :)

@helto4real helto4real requested a review from home-assistant/core as a code owner Sep 19, 2018

@homeassistant

This comment has been minimized.

homeassistant commented Sep 19, 2018

Hi @helto4real,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

Show resolved Hide resolved homeassistant/components/weather/smhi.py Outdated
Show resolved Hide resolved homeassistant/components/weather/smhi.py Outdated
Show resolved Hide resolved homeassistant/components/smhi/config_flow.py
Show resolved Hide resolved homeassistant/components/smhi/__init__.py Outdated
Show resolved Hide resolved homeassistant/components/smhi/__init__.py Outdated
Show resolved Hide resolved homeassistant/components/smhi/__init__.py Outdated

@homeassistant homeassistant added cla-signed and removed cla-needed labels Sep 19, 2018

Show resolved Hide resolved homeassistant/components/weather/__init__.py Outdated
Show resolved Hide resolved homeassistant/components/weather/smhi.py Outdated
Show resolved Hide resolved homeassistant/components/weather/smhi.py Outdated
Show resolved Hide resolved homeassistant/components/weather/smhi.py Outdated
Show resolved Hide resolved homeassistant/components/weather/smhi.py Outdated
Show resolved Hide resolved tests/components/weather/test_smhi.py Outdated

@frenck frenck added the docs-missing label Sep 21, 2018

@frenck

This comment has been minimized.

Member

frenck commented Sep 21, 2018

Could not find a related documentation PR, added docs-missing label.

@helto4real helto4real changed the title from WIP - Swedish weather institute weather component to Swedish weather institute weather component Sep 23, 2018

@MartinHjelmare

What's the setup.txt file? Is that intended?

Show resolved Hide resolved homeassistant/components/smhi/const.py Outdated
Show resolved Hide resolved homeassistant/components/smhi/const.py Outdated
Show resolved Hide resolved homeassistant/components/weather/demo.py Outdated
Show resolved Hide resolved homeassistant/components/weather/smhi.py Outdated
Show resolved Hide resolved homeassistant/components/weather/smhi.py Outdated
Show resolved Hide resolved homeassistant/components/weather/smhi.py Outdated
Show resolved Hide resolved homeassistant/components/weather/smhi.py
Show resolved Hide resolved homeassistant/components/weather/smhi.py Outdated
Show resolved Hide resolved homeassistant/components/weather/smhi.py Outdated
Show resolved Hide resolved homeassistant/components/weather/smhi.py Outdated
Show resolved Hide resolved homeassistant/components/weather/smhi.py Outdated
Show resolved Hide resolved homeassistant/components/smhi/config_flow.py Outdated
Show resolved Hide resolved homeassistant/components/smhi/config_flow.py Outdated
Show resolved Hide resolved homeassistant/components/smhi/config_flow.py Outdated
Show resolved Hide resolved homeassistant/config_entries.py Outdated
Show resolved Hide resolved script/gen_requirements_all.py Outdated

@Kane610 Kane610 added the in progress label Oct 2, 2018

@Kane610 Kane610 changed the title from Swedish weather institute weather component to WIP: Swedish weather institute weather component Oct 2, 2018

@Kane610 Kane610 changed the title from WIP: Swedish weather institute weather component to Swedish weather institute weather component Oct 2, 2018

@MartinHjelmare

This comment has been minimized.

Member

MartinHjelmare commented Oct 2, 2018

Do an interactive rebase and reword the commit messages that contain WIP, removing that word.

@helto4real

This comment has been minimized.

Contributor

helto4real commented Oct 2, 2018

Had to redo the history due to bad rebase in past. Only one commit now

@MartinHjelmare

I'm approving this now. But I'd would like a second pair of eyes before merge, specifically on config flow and tests.

@helto4real

This comment has been minimized.

Contributor

helto4real commented Oct 3, 2018

Ok Thanks @MartinHjelmare hopefully @balloob have time to help out even I suspect he will be busy during hacktoberfest hehe

@Kane610

Only lesser comments from me, looks good!

Show resolved Hide resolved homeassistant/components/smhi/config_flow.py
Show resolved Hide resolved tests/components/weather/test_smhi.py Outdated
Show resolved Hide resolved homeassistant/components/weather/smhi.py Outdated
Show resolved Hide resolved homeassistant/components/smhi/config_flow.py Outdated
@Kane610

This comment has been minimized.

Contributor

Kane610 commented Oct 7, 2018

@MartinHjelmare @balloob config flow looks ok! Im not too confident in my test skills to give a +2 there though

@MartinHjelmare

This comment has been minimized.

Member

MartinHjelmare commented Oct 7, 2018

Let's merge this. Ok @Kane610?

@helto4real

This comment has been minimized.

Contributor

helto4real commented Oct 8, 2018

Awesome guys! Thanks @MartinHjelmare @Kane610 and all others that helped out on the PR. Great feedbacks.

@MartinHjelmare

Ok, maybe this is it? 😄

@helto4real

This comment has been minimized.

Contributor

helto4real commented Oct 8, 2018

Lets hope so hehe. You think I should implement support for the "Available" as @Kane610 suggested after @dersger review? @MartinHjelmare ? No other weather component does that but it seem reasonable. But merge it if you are ok with it

@MartinHjelmare

This comment has been minimized.

Member

MartinHjelmare commented Oct 8, 2018

I think it's a good addition. But it can also be done in another PR. You decide.

@helto4real

This comment has been minimized.

Contributor

helto4real commented Oct 8, 2018

Lets make that in a separate PR then.

@MartinHjelmare MartinHjelmare merged commit 540d22d into home-assistant:dev Oct 8, 2018

4 checks passed

Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@wafflebot wafflebot bot removed the in progress label Oct 8, 2018

@helto4real helto4real deleted the helto4real:smhi-component branch Oct 9, 2018

@balloob balloob referenced this pull request Oct 26, 2018

Merged

0.81 #17809

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