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

Added new sensor meteoalarm to get weather alerts in Europe #23663

Merged
merged 25 commits into from May 7, 2019

Conversation

Projects
None yet
6 participants
@rolfberkenbosch
Copy link
Contributor

commented May 4, 2019

Description:

The MeteoAlarm platform allows one to watch for weather alerts in europe from MeteoAlarm (EUMETNET).

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

Example entry for configuration.yaml (if applicable):

sensor:
  - platform: meteoalarm
    country: "NL"
    province: "Groningen"
    language: 'nl'

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

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

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

  • The manifest file has all fields filled out correctly (example).
  • New dependencies have been added to requirements in the manifest (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.
Show resolved Hide resolved homeassistant/components/meteoalarm/sensor.py Outdated
Show resolved Hide resolved homeassistant/components/meteoalarm/manifest.json Outdated
Show resolved Hide resolved homeassistant/components/meteoalarm/sensor.py Outdated

rolfberkenbosch added some commits May 4, 2019

@amelchio
Copy link
Contributor

left a comment

Looks good, I just found some minor issues.

Show resolved Hide resolved homeassistant/components/meteoalarm/sensor.py Outdated
Show resolved Hide resolved homeassistant/components/meteoalarm/sensor.py Outdated
Show resolved Hide resolved homeassistant/components/meteoalarm/sensor.py Outdated
Show resolved Hide resolved homeassistant/components/meteoalarm/sensor.py Outdated
Show resolved Hide resolved homeassistant/components/meteoalarm/sensor.py Outdated
Show resolved Hide resolved homeassistant/components/meteoalarm/sensor.py Outdated
Show resolved Hide resolved homeassistant/components/meteoalarm/sensor.py Outdated
Show resolved Hide resolved homeassistant/components/meteoalarm/sensor.py Outdated
Show resolved Hide resolved homeassistant/components/meteoalarm/sensor.py Outdated

rolfberkenbosch added some commits May 5, 2019

@amelchio
Copy link
Contributor

left a comment

Looks good 👍

@amelchio
Copy link
Contributor

left a comment

I agree with @fabaff, it makes sense to convert it to a binary sensor.

rolfberkenbosch added some commits May 6, 2019

Update sensor.py
Change sensor to binarysensor
Update binary_sensor.py
Removed BinarySensorDevice from class
Update binary_sensor.py
Made a mistake with BinarySensorDevice
Update binary_sensor.py
clean up white spaces
Update binary_sensor.py
Fix BinarySensorDevice
Update binary_sensor.py
cleanup the import libs
@rolfberkenbosch

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

@fabaff @amelchio, I change the code, i get no errors. But sadly also no binary_sensor.meteoalarm is coming up in the dev-state list. Any idea ?

@rolfberkenbosch

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

Found the problem, then binary sensor is now working.

fix
@amelchio
Copy link
Contributor

left a comment

I am sorry that you had to do this rework 😢 but glad you got it working 😊.

It's looking very good at this point, I noticed just a few small issues.

Show resolved Hide resolved .coveragerc Outdated
Show resolved Hide resolved homeassistant/components/meteoalarm/binary_sensor.py Outdated
Show resolved Hide resolved homeassistant/components/meteoalarm/binary_sensor.py Outdated
Show resolved Hide resolved homeassistant/components/meteoalarm/binary_sensor.py Outdated
Show resolved Hide resolved homeassistant/components/meteoalarm/binary_sensor.py Outdated

rolfberkenbosch added some commits May 6, 2019

rolfberkenbosch added some commits May 6, 2019

@amelchio
Copy link
Contributor

left a comment

Please test before pushing :-/

Show resolved Hide resolved homeassistant/components/meteoalarm/binary_sensor.py Outdated
Show resolved Hide resolved homeassistant/components/meteoalarm/binary_sensor.py Outdated
@rolfberkenbosch

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

@amelchio your right. I did now a test first, everything seems ok now.

@amelchio
Copy link
Contributor

left a comment

🎉

@rolfberkenbosch

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

@fabaff is everything now also "ok" for you? Then i can update the documentation, since it is now a binary sensor.

@amelchio amelchio merged commit c07c557 into home-assistant:dev May 7, 2019

14 checks passed

build Workflow: build
Details
ci/circleci: pre-install-all-requirements Your tests passed on CircleCI!
Details
ci/circleci: pre-test 3.5.5 Your tests passed on CircleCI!
Details
ci/circleci: pre-test 3.6 Your tests passed on CircleCI!
Details
ci/circleci: pre-test 3.7 Your tests passed on CircleCI!
Details
ci/circleci: pylint Your tests passed on CircleCI!
Details
ci/circleci: static-check Your tests passed on CircleCI!
Details
ci/circleci: test 3.5.5 Your tests passed on CircleCI!
Details
ci/circleci: test 3.6 Your tests passed on CircleCI!
Details
ci/circleci: test 3.7 Your tests passed on CircleCI!
Details
cla-bot Everyone involved has signed the CLA
codecov/patch Coverage not affected when comparing 8fc3056...ed0108e
Details
codecov/project 93.9% (target 90%)
Details
home-assistant Build #20190506.49 succeeded
Details

mxworm added a commit to mxworm/home-assistant that referenced this pull request May 7, 2019

Merge branch 'dev' into current
* dev: (52 commits)
  Add abbreviation for position_topic (home-assistant#23740)
  Upgrade aiodns to 2.0.0 (home-assistant#23743)
  Fix effects on HUE integration for Osram bulbs (home-assistant#22789)
  Fix Hue sensor integration with multiple bridges. (home-assistant#23713)
  Add HEOS sign-in/out services (home-assistant#23729)
  Add debug prints to sun (home-assistant#23598)
  Add water_heater to geniushub, bump client library (home-assistant#23384)
  add abbreviation for current_temperature_template (home-assistant#23733)
  Added new binary sensor meteoalarm to get weather alerts in Europe (home-assistant#23663)
  Add TTL support and custom headers support. (home-assistant#22988)
  Share snmp constants across all platforms (home-assistant#23678)
  Fix SmartThings Samsung Air Conditioner Support (home-assistant#23706)
  Upgrade switchbot , fixes home-assistant#23702 (home-assistant#23716)
  Use the URL provided by the Wink API for subscriptions. (home-assistant#23710)
  huawei_lte: handle icons for None sensor values gracefully (home-assistant#23649)
  Update IDs after firmware upgrade in HEOS (home-assistant#23641)
  bump zha-quirks (home-assistant#23714)
  Upgrade sendgrid to 6.0.5 (home-assistant#23711)
  Use local constant in Daikin for STATE_OFF (home-assistant#23712)
  Catch thethingsnetwork TypeError (home-assistant#23667)
  ...

@balloob balloob referenced this pull request May 14, 2019

Merged

0.93.0 #23864

@martinhoess

This comment has been minimized.

Copy link

commented May 16, 2019

Available countries (extracted from https://www.meteoalarm.eu/ATOM/root.xml)

Germany (DE) and Cyprus (CY) are currently not working! (I've sent them an email - meteoalarm@zamg.ac.at)

AT - Austria
BA - Bosnia and Herzegovina
BE - Belgium
BG - Bulgaria
CH - Switzerland
CY - Cyprus
CZ - Czechia
DE - Germany
DK - Denmark
EE - Estonia
ES - Spain
FI - Finland
FR - France
GR - Greece
HR - Croatia
HU - Hungary
IE - Ireland
IS - Island
IT - Italy
LT - Lithuania
LU - Luxembourg
LV - Latvia
MD - Moldova, Republic of
ME - Montenegro
MK - North Macedonia
MT - Malta
NL - Netherlands
NO - Norway
PL - Poland
PT - Portugal
RO - Romania
RS - Serbia
SE - Sweden
SI - Slovenia
SK - Slovakia
UK - United Kingdom (UK ist not the valid ISO country code!)
@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented May 16, 2019

Please open an issue if you suspect a bug.

If you want to suggest an enhancement please open a feature request in the Feature Requests section of our community forum.

Merged PRs should not be used for enhancement discussion or bug reports. If you've found a bug it's ok to make a review with inline comments and link to an issue that reports the bug.

Thanks!

@home-assistant home-assistant locked as resolved and limited conversation to collaborators May 16, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.