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

Add optional location based region to dwd_weather_warnings #96027

Merged
merged 13 commits into from
Apr 22, 2024
45 changes: 40 additions & 5 deletions homeassistant/components/dwd_weather_warnings/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,57 @@
from __future__ import annotations

from dwdwfsapi import DwdWeatherWarningsAPI
import voluptuous as vol

from homeassistant.config_entries import ConfigEntry
from homeassistant.core import HomeAssistant
from homeassistant.helpers import entity_registry as er

from .const import CONF_REGION_IDENTIFIER, DOMAIN, PLATFORMS
from .const import (
CONF_REGION_DEVICE_TRACKER,
CONF_REGION_IDENTIFIER,
DOMAIN,
LOGGER,
PLATFORMS,
)
from .coordinator import DwdWeatherWarningsCoordinator
from .exceptions import EntityNotFoundError
from .util import get_position_data


async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
"""Set up a config entry."""
region_identifier: str = entry.data[CONF_REGION_IDENTIFIER]
region_identifier: str = entry.data.get(CONF_REGION_IDENTIFIER, None)
device_tracker: str = entry.data.get(CONF_REGION_DEVICE_TRACKER, None)
emontnemery marked this conversation as resolved.
Show resolved Hide resolved

# Initialize the API and coordinator.
api = await hass.async_add_executor_job(DwdWeatherWarningsAPI, region_identifier)
coordinator = DwdWeatherWarningsCoordinator(hass, api)
# Initialize the API and coordinator based on the specified data.
if region_identifier is not None:
emontnemery marked this conversation as resolved.
Show resolved Hide resolved
api = await hass.async_add_executor_job(
DwdWeatherWarningsAPI, region_identifier
)
elif device_tracker is not None:
emontnemery marked this conversation as resolved.
Show resolved Hide resolved
registry = er.async_get(hass)

try:
device_tracker = er.async_validate_entity_id(registry, device_tracker)
except vol.Invalid:
# The entity/UUID is invalid or not associated with an entity registry item.
LOGGER.error(
"Failed to setup dwd_weather_warnings for unknown entity %s",
device_tracker,
)
return False
andarotajo marked this conversation as resolved.
Show resolved Hide resolved
emontnemery marked this conversation as resolved.
Show resolved Hide resolved

try:
position = get_position_data(hass, device_tracker)
except (EntityNotFoundError, AttributeError) as err:
# The provided device_tracker is not available or missing required attributes.
LOGGER.error(f"Failed to setup dwd_weather_warnings: {repr(err)}")
return False

api = await hass.async_add_executor_job(DwdWeatherWarningsAPI, position)
Copy link
Contributor

Choose a reason for hiding this comment

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

All of this seems like duplicated logic which the coordinator also needs to handle, can you try to make it happen as a result of calling DwdWeatherWarningsCoordinator.async_config_entry_first_refresh instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll see what I can do, not familiar with that method though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Took a few tries but I got it (see last 3 commits)


coordinator = DwdWeatherWarningsCoordinator(hass, api)
await coordinator.async_config_entry_first_refresh()

hass.data.setdefault(DOMAIN, {})[entry.entry_id] = coordinator
Expand Down
75 changes: 62 additions & 13 deletions homeassistant/components/dwd_weather_warnings/config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,13 @@
import voluptuous as vol

from homeassistant.config_entries import ConfigFlow, ConfigFlowResult
from homeassistant.helpers import entity_registry as er
import homeassistant.helpers.config_validation as cv
from homeassistant.helpers.selector import EntitySelector, EntitySelectorConfig

from .const import CONF_REGION_IDENTIFIER, DOMAIN
from .const import CONF_REGION_DEVICE_TRACKER, CONF_REGION_IDENTIFIER, DOMAIN
from .exceptions import EntityNotFoundError
from .util import get_position_data


class DwdWeatherWarningsConfigFlow(ConfigFlow, domain=DOMAIN):
Expand All @@ -25,27 +29,72 @@ async def async_step_user(
errors: dict = {}

if user_input is not None:
region_identifier = user_input[CONF_REGION_IDENTIFIER]
valid_options = (CONF_REGION_IDENTIFIER, CONF_REGION_DEVICE_TRACKER)
emontnemery marked this conversation as resolved.
Show resolved Hide resolved

# Validate region identifier using the API
if not await self.hass.async_add_executor_job(
DwdWeatherWarningsAPI, region_identifier
):
errors["base"] = "invalid_identifier"
# Check, if either CONF_REGION_IDENTIFIER or CONF_GPS_TRACKER has been set.
if all(k not in user_input for k in valid_options):
errors["base"] = "no_identifier"
elif all(k in user_input for k in valid_options):
errors["base"] = "ambiguous_identifier"
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems a bit unnecessary. Maybe just do a set intersection and check the length?
Not sure if my suggestion is more readable though.

EXCLUSIVE_OPTIONS = {CONF_REGION_IDENTIFIER, CONF_REGION_DEVICE_TRACKER}
options = user_input.keys() & EXCLUSIVE_OPTIONS
if not options:
    ...
elif len(options) == 2:
   ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my opinion the current solution is easier to understand though not as efficient probably

Copy link
Contributor

Choose a reason for hiding this comment

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

That's fine, it's a matter of taste 👍

elif CONF_REGION_IDENTIFIER in user_input:
# Validate region identifier using the API
identifier = user_input[CONF_REGION_IDENTIFIER]

if not errors:
# Set the unique ID for this config entry.
await self.async_set_unique_id(region_identifier)
self._abort_if_unique_id_configured()
if not await self.hass.async_add_executor_job(
DwdWeatherWarningsAPI, identifier
):
errors["base"] = "invalid_identifier"

return self.async_create_entry(title=region_identifier, data=user_input)
if not errors:
# Set the unique ID for this config entry.
await self.async_set_unique_id(identifier)
self._abort_if_unique_id_configured()

return self.async_create_entry(title=identifier, data=user_input)
elif CONF_REGION_DEVICE_TRACKER in user_input:
emontnemery marked this conversation as resolved.
Show resolved Hide resolved
device_tracker = user_input[CONF_REGION_DEVICE_TRACKER]
registry = er.async_get(self.hass)
entity_entry = registry.async_get(device_tracker)

if entity_entry is None:
errors["base"] = "entity_not_found"
else:
try:
position = get_position_data(self.hass, entity_entry.id)
except EntityNotFoundError:
errors["base"] = "entity_not_found"
except AttributeError:
errors["base"] = "attribute_not_found"
else:
# Validate position using the API
if not await self.hass.async_add_executor_job(
DwdWeatherWarningsAPI, position
):
errors["base"] = "invalid_identifier"

# Position is valid here, because the API call was successful.
if not errors and position is not None and entity_entry is not None:
# Set the unique ID for this config entry.
await self.async_set_unique_id(entity_entry.id)
self._abort_if_unique_id_configured()

# Replace entity ID with registry ID for more stability.
user_input[CONF_REGION_DEVICE_TRACKER] = entity_entry.id

return self.async_create_entry(
title=device_tracker.removeprefix("device_tracker."),
data=user_input,
)

return self.async_show_form(
step_id="user",
errors=errors,
data_schema=vol.Schema(
{
vol.Required(CONF_REGION_IDENTIFIER): cv.string,
vol.Optional(CONF_REGION_IDENTIFIER): cv.string,
vol.Optional(CONF_REGION_DEVICE_TRACKER): EntitySelector(
EntitySelectorConfig(domain="device_tracker")
MartinHjelmare marked this conversation as resolved.
Show resolved Hide resolved
),
}
),
)
1 change: 1 addition & 0 deletions homeassistant/components/dwd_weather_warnings/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

CONF_REGION_NAME: Final = "region_name"
CONF_REGION_IDENTIFIER: Final = "region_identifier"
CONF_REGION_DEVICE_TRACKER: Final = "region_device_tracker"

ATTR_REGION_NAME: Final = "region_name"
ATTR_REGION_ID: Final = "region_id"
Expand Down
21 changes: 18 additions & 3 deletions homeassistant/components/dwd_weather_warnings/coordinator.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,20 @@

from dwdwfsapi import DwdWeatherWarningsAPI

from homeassistant.config_entries import ConfigEntry
from homeassistant.core import HomeAssistant
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed

from .const import DEFAULT_SCAN_INTERVAL, DOMAIN, LOGGER
from .const import CONF_REGION_DEVICE_TRACKER, DEFAULT_SCAN_INTERVAL, DOMAIN, LOGGER
from .exceptions import EntityNotFoundError
from .util import get_position_data


class DwdWeatherWarningsCoordinator(DataUpdateCoordinator[None]):
"""Custom coordinator for the dwd_weather_warnings integration."""

config_entry: ConfigEntry

def __init__(self, hass: HomeAssistant, api: DwdWeatherWarningsAPI) -> None:
"""Initialize the dwd_weather_warnings coordinator."""
super().__init__(
Expand All @@ -23,4 +28,14 @@ def __init__(self, hass: HomeAssistant, api: DwdWeatherWarningsAPI) -> None:

async def _async_update_data(self) -> None:
"""Get the latest data from the DWD Weather Warnings API."""
await self.hass.async_add_executor_job(self.api.update)
if device_tracker := self.config_entry.data.get(CONF_REGION_DEVICE_TRACKER):
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest to store device_tracker in the object instead, and just do like this here:

if self._device_tracker:

Copy link
Contributor Author

@andarotajo andarotajo Mar 11, 2024

Choose a reason for hiding this comment

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

I'm not following completely. Do you mean having it defined in the __init__ as self.device_tracker = None and doing the initialization once in async_config_entry_first_refresh?

Edit: Nevermind got it

try:
position = get_position_data(self.hass, device_tracker)
except (EntityNotFoundError, AttributeError) as err:
raise UpdateFailed(f"Error fetching position: {repr(err)}") from err

self.api = await self.hass.async_add_executor_job(
DwdWeatherWarningsAPI, position
)
else:
await self.hass.async_add_executor_job(self.api.update)
6 changes: 6 additions & 0 deletions homeassistant/components/dwd_weather_warnings/exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
"""Exceptions for the dwd_weather_warnings integration."""
from homeassistant.exceptions import HomeAssistantError
emontnemery marked this conversation as resolved.
Show resolved Hide resolved


class EntityNotFoundError(HomeAssistantError):
"""When a referenced entity was not found."""
6 changes: 4 additions & 2 deletions homeassistant/components/dwd_weather_warnings/sensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

from __future__ import annotations

from typing import Any

from homeassistant.components.sensor import SensorEntity, SensorEntityDescription
from homeassistant.config_entries import ConfigEntry
from homeassistant.core import HomeAssistant
Expand Down Expand Up @@ -96,15 +98,15 @@ def __init__(
self.api = coordinator.api

@property
def native_value(self):
def native_value(self) -> int | None:
"""Return the state of the sensor."""
if self.entity_description.key == CURRENT_WARNING_SENSOR:
return self.api.current_warning_level

return self.api.expected_warning_level

@property
def extra_state_attributes(self):
def extra_state_attributes(self) -> dict[str, Any]:
"""Return the state attributes of the sensor."""
data = {
ATTR_REGION_NAME: self.api.warncell_name,
Expand Down
13 changes: 9 additions & 4 deletions homeassistant/components/dwd_weather_warnings/strings.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,22 @@
"config": {
"step": {
"user": {
"description": "To identify the desired region, the warncell ID / name is required.",
"description": "To identify the desired region, either the warncell ID / name or device tracker is required. The provided device tracker has to contain the attributes 'latitude' and 'longitude'.",
"data": {
"region_identifier": "Warncell ID or name"
"region_identifier": "Warncell ID or name",
"region_device_tracker": "Device tracker entity"
}
}
},
"error": {
"invalid_identifier": "The specified region identifier is invalid."
"no_identifier": "Either the region identifier or device tracker is required.",
"ambiguous_identifier": "The region identifier and device tracker can not be specified together.",
"invalid_identifier": "The specified region identifier / device tracker is invalid.",
"entity_not_found": "The specified device tracker entity was not found.",
"attribute_not_found": "The required `latitude` or `longitude` attribute was not found in the specified device tracker."
},
"abort": {
"already_configured": "Warncell ID / name is already configured.",
"already_configured": "[%key:common::config_flow::abort::already_configured_device%]",
"invalid_identifier": "[%key:component::dwd_weather_warnings::config::error::invalid_identifier%]"
}
},
Expand Down
39 changes: 39 additions & 0 deletions homeassistant/components/dwd_weather_warnings/util.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
"""Util functions for the dwd_weather_warnings integration."""

from __future__ import annotations

from homeassistant.const import ATTR_LATITUDE, ATTR_LONGITUDE
from homeassistant.core import HomeAssistant
from homeassistant.helpers import entity_registry as er

from .exceptions import EntityNotFoundError


def get_position_data(
hass: HomeAssistant, registry_id: str
) -> tuple[float, float] | None:
"""Extract longitude and latitude from a device tracker."""
registry = er.async_get(hass)
registry_entry = registry.async_get(registry_id)
if registry_entry is None:
raise EntityNotFoundError(f"Failed to find registry entry {registry_id}")

entity = hass.states.get(registry_entry.entity_id)
if entity is None:
raise EntityNotFoundError(f"Failed to find entity {registry_entry.entity_id}")

latitude = entity.attributes.get(ATTR_LATITUDE)
if not latitude:
raise AttributeError(
f"Failed to find attribute '{ATTR_LATITUDE}' in {registry_entry.entity_id}",
ATTR_LATITUDE,
)

longitude = entity.attributes.get(ATTR_LONGITUDE)
if not longitude:
raise AttributeError(

Check warning on line 34 in homeassistant/components/dwd_weather_warnings/util.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/dwd_weather_warnings/util.py#L34

Added line #L34 was not covered by tests
f"Failed to find attribute '{ATTR_LONGITUDE}' in {registry_entry.entity_id}",
ATTR_LONGITUDE,
)

return (latitude, longitude)
Loading
Loading