Skip to content

Added DWD WarnApp Sensor#8657

Merged
balloob merged 18 commits into
home-assistant:devfrom
runningman84:dwdwarnapp
Sep 5, 2017
Merged

Added DWD WarnApp Sensor#8657
balloob merged 18 commits into
home-assistant:devfrom
runningman84:dwdwarnapp

Conversation

@runningman84
Copy link
Copy Markdown
Contributor

@runningman84 runningman84 commented Jul 26, 2017

Description:

This sensor adds support for current and advance weather warnings from DWD (Deutsche Wetter Dienst) for Germany. This sensor uses the same data as the WarnWetter Android/IOS App.

The warning level is between 0 (no danger) and 4:

  • Warnungen vor extremem Unwetter (Stufe 4)
  • Unwetterwarnungen (Stufe 3)
  • Warnungen vor markantem Wetter (Stufe 2)
  • Wetterwarnungen (Stufe 1)

It can be a bit tricky to find out the correct region name because the source json files only shows a region if a current or advance warning is there:
https://www.dwd.de/DWD/warnungen/warnapp_landkreise/json/warnings.json?jsonp=loadWarnings
The region_name should match the name of the regions listed on this page:
https://www.dwd.de/DE/wetter/warnungen_landkreise/warnWetter_node.html?ort=Hamburg

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

Example entry for configuration.yaml (if applicable):

sensor:
  - platform: dwd_weather_warnings
    region_name: 'Hansestadt Hamburg'   # optional

Here is a screenshot:
https://community.home-assistant.io/t/dwd-warnapp-sensor-amtliche-warnungen-des-deutschen-wetterdienstes/22699/1

Checklist:

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

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.

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

@homeassistant
Copy link
Copy Markdown
Contributor

Hi @runningman84,

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!

data['{}_warning_count'.format(mykey)] = len(my_warnings)
data['{}_warnings'.format(mykey)] = my_warnings

_LOGGER.debug("Found {} {} local DWD warnings".format(len(my_warnings), mykey))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (95 > 79 characters)


# Get max warning level
for event in my_warnings:
if(event['level'] >= data['{}_warning_level'.format(mykey)]):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (81 > 79 characters)

continue
my_warnings = json_obj[myvalue][key]
self.region_id = key
self.region_state = json_obj[myvalue][key][0]['stateShort']
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (83 > 79 characters)

else:
# loop through all items to find warnings, region_id and region_state for region_name
for key in json_obj[myvalue]:
if json_obj[myvalue][key][0]['regionName'] != self.region_name:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (87 > 79 characters)

my_warnings = json_obj[myvalue][self.region_id]

else:
# loop through all items to find warnings, region_id and region_state for region_name
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (105 > 79 characters)

# data['region_map_url'] = 'https://www.dwd.de/DWD/warnungen/warnapp_gemeinden/json/warnungen_gemeinde_map_' + str(data['region_state'].lower()) + '.png'

if self._api.data['time'] is not None:
data['last_update'] = dt_util.as_local(dt_util.utc_from_timestamp(self._api.data['time'] / 1000))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (110 > 79 characters)
multiple spaces before operator


if self._api.region_state is not None:
data['region_state'] = self._api.region_state
# data['region_map_url'] = 'https://www.dwd.de/DWD/warnungen/warnapp_gemeinden/json/warnungen_gemeinde_map_' + str(data['region_state'].lower()) + '.png'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (165 > 79 characters)

if self._api.region_id is not None:
data['region_id'] = self._api.region_id

if self._api.region_state is not None:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

multiple spaces after keyword

data = {}
data['region_name'] = self._api.region_name

if self._api.region_id is not None:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

multiple spaces after keyword

'current_warning_level': ['Current Warning Level',
None, 'mdi:close-octagon-outline'],
'advance_warning_level': ['Advance Warning Level',
None, 'mdi:close-octagon-outline'],
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

continuation line under-indented for visual indent

import homeassistant.util.dt as dt_util

_LOGGER = logging.getLogger(__name__)
_ENDPOINT = '/DWD/warnungen/warnapp_landkreise/json/warnings.json?jsonp=loadWarnings'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (85 > 79 characters)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how ca we fix this one? I cannot change the corresponding url, splitting it into multiple lines seems odd to me.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix it by not hardcoding the url arguments in the endpoint.

import logging
import json
import time
import datetime
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'datetime' imported but unused

"""
import logging
import json
import time
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'time' imported but unused


if self._api.region_state is not None:
data['region_state'] = self._api.region_state
# data['region_map_url'] = 'https://www.dwd.de/DWD/warnungen/warnapp_gemeinden/json/warnungen_gemeinde_map_' + str(data['region_state'].lower()) + '.png'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (165 > 79 characters)

Copy link
Copy Markdown
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a lot of options in here that I don't really see why they would make sense for querying a web service (verify SSL, use SSL etc)

DEFAULT_METHOD = 'GET'
DEFAULT_NAME = 'DWD-Warnapp'
DEFAULT_SSL = True
DEFAULT_VERIFY_SSL = True
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you implement this option? Doesn't it default fetch dwd.de which I assume has SSL working?

self.available = True
self.update()

#@Throttle(SCAN_INTERVAL)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You will need to implement throttle or you're hammering their API

"""Initialize the data object."""
from homeassistant.components.sensor.rest import RestData

uri_scheme = 'https://' if use_ssl else 'http://'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this an option? Isn't the website always the same

self.available = True
self.update()

@Throttle(SCAN_INTERVAL)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

undefined name 'Throttle'

import homeassistant.helpers.config_validation as cv
from homeassistant.helpers.entity import Entity
from homeassistant.components.sensor import PLATFORM_SCHEMA
from homeassistant.const import (
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'homeassistant.const.CONF_HOST' imported but unused
'homeassistant.const.CONF_SSL' imported but unused
'homeassistant.const.CONF_VERIFY_SSL' imported but unused


# Get max warning level
for event in my_warnings:
if(event['level'] >= data['{}_warning_level'.format(mykey)]):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (81 > 79 characters)

continue
my_warnings = json_obj[myvalue][key]
self.region_id = key
self.region_state = json_obj[myvalue][key][0]['stateShort']
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (83 > 79 characters)

else:
# loop through all items to find warnings, region_id and region_state for region_name
for key in json_obj[myvalue]:
if json_obj[myvalue][key][0]['regionName'] != self.region_name:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (87 > 79 characters)

my_warnings = json_obj[myvalue][self.region_id]

else:
# loop through all items to find warnings, region_id and region_state for region_name
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (105 > 79 characters)


data['time'] = json_obj['time']

for mykey, myvalue in {'current': 'warnings', 'advance': 'vorabInformation'}.items():
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (97 > 79 characters)


if self._api.region_state is not None:
data['region_state'] = self._api.region_state
# data['region_map_url'] = 'https://www.dwd.de/DWD/warnungen/warnapp_gemeinden/json/warnungen_gemeinde_map_' + str(data['region_state'].lower()) + '.png'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (165 > 79 characters)

@runningman84
Copy link
Copy Markdown
Contributor Author

@balloob I added the throttle function and renamed it to dwd weather warnings.
I do not have a good idea how to shorten the lines to be below 80 chars. Is that a hard limit? If yes please provide some advice how to do this...

@balloob
Copy link
Copy Markdown
Member

balloob commented Aug 6, 2017

 if (event['level'] >=
         data['{}_warning_level'.format(mykey)]):

@balloob
Copy link
Copy Markdown
Member

balloob commented Aug 6, 2017

Yes, it is a hard limit.

# loop through all items to find warnings, region_id
# and region_state for region_name
for key in json_obj[myvalue]:
if json_obj[myvalue][key][0]['regionName'] != self.region_name:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (87 > 79 characters)


_LOGGER = logging.getLogger(__name__)

DEFAULT_NAME = 'DWD-Wetter-Warnungen'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be in English. If the user can set name: if he/she want it in German.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

DEFAULT_NAME = 'DWD-Wetter-Warnungen'

CONF_REGION_NAME = 'region_name'
DEFAULT_REGION_NAME = 'Hansestadt Hamburg'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This default does only make sense for a small group of users.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will remove the default region


# pylint: disable=no-member
@property
def device_state_attributes(self):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add an attribution?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you mean?


def __init__(self, hass, api, name, variable):
"""Initialize a DWD-Weather-Warnings sensor."""
self._hass = hass
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You doesn't seems to use self._hass.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will remove it


def __init__(self, region_name):
"""Initialize the data object."""
from homeassistant.components.sensor.rest import RestData
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please place this in the file header.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

return self._api.data[self._var_id]

# pylint: disable=no-member
@property
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redefinition of unused 'device_state_attributes' from line 96

@runningman84
Copy link
Copy Markdown
Contributor Author

@fabaff can you accept the pull request or is still something missing?

@fabaff
Copy link
Copy Markdown
Member

fabaff commented Aug 21, 2017

There are still lint issues.

data['{}_warnings'.format(mykey)] = my_warnings

_LOGGER.debug("Found %d %s local DWD warnings",
len(my_warnings), mykey)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

continuation line under-indented for visual indent

}.items():

_LOGGER.debug("Found %d %s global DWD warnings",
len(json_obj[myvalue]), mykey)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

continuation line under-indented for visual indent

elif self._var_id == 'advance_warning_level':
prefix = 'advance'

data['warning_count'] = self._api.data[prefix + '_warning_count']
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Local variable 'prefix' might be referenced before assignment. Compare L122, L124.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok I will add a else path and throw an exception there...


def __init__(self, region_name):
"""Initialize the data object."""

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just remove this blank line.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

json_string = self._rest.data[24:len(self._rest.data) - 2]
json_obj = json.loads(json_string)

data = {}
Copy link
Copy Markdown
Member

@syssi syssi Sep 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could save a line here. My IDE complains about. ;-)

- data = {}
-
- data['time'] = json_obj['time']
+ data = {'time': json_obj['time']}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

@property
def device_state_attributes(self):
"""Return the state attributes of the DWD-Weather-Warnings."""
data = {}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here:

        data = {
            ATTR_ATTRIBUTION: ATTRIBUTION,
            'region_name': self._api.region_name,
        }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

@syssi
Copy link
Copy Markdown
Member

syssi commented Sep 3, 2017

Good job! 👍

@runningman84
Copy link
Copy Markdown
Contributor Author

@fabaff and @balloob all checks have passed

@balloob
Copy link
Copy Markdown
Member

balloob commented Sep 5, 2017

Thanks! 🐬

@balloob balloob merged commit 9ede0f5 into home-assistant:dev Sep 5, 2017
@balloob balloob mentioned this pull request Sep 7, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Dec 11, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants