Address late review comments for Tankerkoenig (#72672)

* address late review comment from #72654

* use entry_id instead of unique_id

* remove not needed `_hass` property

* fix skiping failing stations

* remove not neccessary error log

* set DeviceEntryType.SERVICE

* fix use entry_id instead of unique_id

* apply suggestions on tests

* add return value also to other tests

* invert data check to early return user form
pull/72682/head
Michael 2022-05-29 20:57:47 +02:00 committed by GitHub
parent 7ff1b53d4f
commit 1ed7e226c6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 55 additions and 54 deletions

View File

@ -11,6 +11,7 @@ import voluptuous as vol
from homeassistant.config_entries import SOURCE_IMPORT, ConfigEntry
from homeassistant.const import (
ATTR_ID,
CONF_API_KEY,
CONF_LATITUDE,
CONF_LOCATION,
@ -24,8 +25,14 @@ from homeassistant.const import (
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import ConfigEntryNotReady
import homeassistant.helpers.config_validation as cv
from homeassistant.helpers.device_registry import DeviceEntryType
from homeassistant.helpers.entity import DeviceInfo
from homeassistant.helpers.typing import ConfigType
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
from homeassistant.helpers.update_coordinator import (
CoordinatorEntity,
DataUpdateCoordinator,
UpdateFailed,
)
from .const import (
CONF_FUEL_TYPES,
@ -109,9 +116,7 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
"""Set a tankerkoenig configuration entry up."""
hass.data.setdefault(DOMAIN, {})
hass.data[DOMAIN][
entry.unique_id
] = coordinator = TankerkoenigDataUpdateCoordinator(
hass.data[DOMAIN][entry.entry_id] = coordinator = TankerkoenigDataUpdateCoordinator(
hass,
entry,
_LOGGER,
@ -140,7 +145,7 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
"""Unload Tankerkoenig config entry."""
unload_ok = await hass.config_entries.async_unload_platforms(entry, PLATFORMS)
if unload_ok:
hass.data[DOMAIN].pop(entry.unique_id)
hass.data[DOMAIN].pop(entry.entry_id)
return unload_ok
@ -172,7 +177,6 @@ class TankerkoenigDataUpdateCoordinator(DataUpdateCoordinator):
self._api_key: str = entry.data[CONF_API_KEY]
self._selected_stations: list[str] = entry.data[CONF_STATIONS]
self._hass = hass
self.stations: dict[str, dict] = {}
self.fuel_types: list[str] = entry.data[CONF_FUEL_TYPES]
self.show_on_map: bool = entry.options[CONF_SHOW_ON_MAP]
@ -195,7 +199,7 @@ class TankerkoenigDataUpdateCoordinator(DataUpdateCoordinator):
station_id,
station_data["message"],
)
return False
continue
self.add_station(station_data["station"])
if len(self.stations) > 10:
_LOGGER.warning(
@ -215,7 +219,7 @@ class TankerkoenigDataUpdateCoordinator(DataUpdateCoordinator):
# The API seems to only return at most 10 results, so split the list in chunks of 10
# and merge it together.
for index in range(ceil(len(station_ids) / 10)):
data = await self._hass.async_add_executor_job(
data = await self.hass.async_add_executor_job(
pytankerkoenig.getPriceList,
self._api_key,
station_ids[index * 10 : (index + 1) * 10],
@ -223,13 +227,11 @@ class TankerkoenigDataUpdateCoordinator(DataUpdateCoordinator):
_LOGGER.debug("Received data: %s", data)
if not data["ok"]:
_LOGGER.error(
"Error fetching data from tankerkoenig.de: %s", data["message"]
)
raise UpdateFailed(data["message"])
if "prices" not in data:
_LOGGER.error("Did not receive price information from tankerkoenig.de")
raise UpdateFailed("No prices in data")
raise UpdateFailed(
"Did not receive price information from tankerkoenig.de"
)
prices.update(data["prices"])
return prices
@ -244,3 +246,20 @@ class TankerkoenigDataUpdateCoordinator(DataUpdateCoordinator):
self.stations[station_id] = station
_LOGGER.debug("add_station called for station: %s", station)
class TankerkoenigCoordinatorEntity(CoordinatorEntity):
"""Tankerkoenig base entity."""
def __init__(
self, coordinator: TankerkoenigDataUpdateCoordinator, station: dict
) -> None:
"""Initialize the Tankerkoenig base entity."""
super().__init__(coordinator)
self._attr_device_info = DeviceInfo(
identifiers={(ATTR_ID, station["id"])},
name=f"{station['brand']} {station['street']} {station['houseNumber']}",
model=station["brand"],
configuration_url="https://www.tankerkoenig.de",
entry_type=DeviceEntryType.SERVICE,
)

View File

@ -8,13 +8,11 @@ from homeassistant.components.binary_sensor import (
BinarySensorEntity,
)
from homeassistant.config_entries import ConfigEntry
from homeassistant.const import ATTR_ID, ATTR_LATITUDE, ATTR_LONGITUDE
from homeassistant.const import ATTR_LATITUDE, ATTR_LONGITUDE
from homeassistant.core import HomeAssistant
from homeassistant.helpers.entity import DeviceInfo
from homeassistant.helpers.entity_platform import AddEntitiesCallback
from homeassistant.helpers.update_coordinator import CoordinatorEntity
from . import TankerkoenigDataUpdateCoordinator
from . import TankerkoenigCoordinatorEntity, TankerkoenigDataUpdateCoordinator
from .const import DOMAIN
_LOGGER = logging.getLogger(__name__)
@ -25,7 +23,7 @@ async def async_setup_entry(
) -> None:
"""Set up the tankerkoenig binary sensors."""
coordinator: TankerkoenigDataUpdateCoordinator = hass.data[DOMAIN][entry.unique_id]
coordinator: TankerkoenigDataUpdateCoordinator = hass.data[DOMAIN][entry.entry_id]
stations = coordinator.stations.values()
entities = []
@ -41,7 +39,7 @@ async def async_setup_entry(
async_add_entities(entities)
class StationOpenBinarySensorEntity(CoordinatorEntity, BinarySensorEntity):
class StationOpenBinarySensorEntity(TankerkoenigCoordinatorEntity, BinarySensorEntity):
"""Shows if a station is open or closed."""
_attr_device_class = BinarySensorDeviceClass.DOOR
@ -53,18 +51,12 @@ class StationOpenBinarySensorEntity(CoordinatorEntity, BinarySensorEntity):
show_on_map: bool,
) -> None:
"""Initialize the sensor."""
super().__init__(coordinator)
super().__init__(coordinator, station)
self._station_id = station["id"]
self._attr_name = (
f"{station['brand']} {station['street']} {station['houseNumber']} status"
)
self._attr_unique_id = f"{station['id']}_status"
self._attr_device_info = DeviceInfo(
identifiers={(ATTR_ID, station["id"])},
name=f"{station['brand']} {station['street']} {station['houseNumber']}",
model=station["brand"],
configuration_url="https://www.tankerkoenig.de",
)
if show_on_map:
self._attr_extra_state_attributes = {
ATTR_LATITUDE: station["lat"],

View File

@ -1,6 +1,7 @@
"""Config flow for Tankerkoenig."""
from __future__ import annotations
from collections.abc import Mapping
from typing import Any
from pytankerkoenig import customException, getNearbyStations
@ -30,7 +31,7 @@ from .const import CONF_FUEL_TYPES, CONF_STATIONS, DEFAULT_RADIUS, DOMAIN, FUEL_
async def async_get_nearby_stations(
hass: HomeAssistant, data: dict[str, Any]
hass: HomeAssistant, data: Mapping[str, Any]
) -> dict[str, Any]:
"""Fetch nearby stations."""
try:
@ -114,14 +115,12 @@ class FlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
return self._show_form_user(
user_input, errors={CONF_API_KEY: "invalid_auth"}
)
if stations := data.get("stations"):
for station in stations:
self._stations[
station["id"]
] = f"{station['brand']} {station['street']} {station['houseNumber']} - ({station['dist']}km)"
else:
if len(stations := data.get("stations", [])) == 0:
return self._show_form_user(user_input, errors={CONF_RADIUS: "no_stations"})
for station in stations:
self._stations[
station["id"]
] = f"{station['brand']} {station['street']} {station['houseNumber']} - ({station['dist']}km)"
self._data = user_input
@ -180,7 +179,7 @@ class FlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
CONF_RADIUS, default=user_input.get(CONF_RADIUS, DEFAULT_RADIUS)
): NumberSelector(
NumberSelectorConfig(
min=0.1,
min=1.0,
max=25,
step=0.1,
unit_of_measurement=LENGTH_KILOMETERS,
@ -224,7 +223,7 @@ class OptionsFlowHandler(config_entries.OptionsFlow):
return self.async_create_entry(title="", data=user_input)
nearby_stations = await async_get_nearby_stations(
self.hass, dict(self.config_entry.data)
self.hass, self.config_entry.data
)
if stations := nearby_stations.get("stations"):
for station in stations:

View File

@ -7,17 +7,14 @@ from homeassistant.components.sensor import SensorEntity, SensorStateClass
from homeassistant.config_entries import ConfigEntry
from homeassistant.const import (
ATTR_ATTRIBUTION,
ATTR_ID,
ATTR_LATITUDE,
ATTR_LONGITUDE,
CURRENCY_EURO,
)
from homeassistant.core import HomeAssistant
from homeassistant.helpers.entity import DeviceInfo
from homeassistant.helpers.entity_platform import AddEntitiesCallback
from homeassistant.helpers.update_coordinator import CoordinatorEntity
from . import TankerkoenigDataUpdateCoordinator
from . import TankerkoenigCoordinatorEntity, TankerkoenigDataUpdateCoordinator
from .const import (
ATTR_BRAND,
ATTR_CITY,
@ -39,7 +36,7 @@ async def async_setup_entry(
) -> None:
"""Set up the tankerkoenig sensors."""
coordinator: TankerkoenigDataUpdateCoordinator = hass.data[DOMAIN][entry.unique_id]
coordinator: TankerkoenigDataUpdateCoordinator = hass.data[DOMAIN][entry.entry_id]
stations = coordinator.stations.values()
entities = []
@ -62,7 +59,7 @@ async def async_setup_entry(
async_add_entities(entities)
class FuelPriceSensor(CoordinatorEntity, SensorEntity):
class FuelPriceSensor(TankerkoenigCoordinatorEntity, SensorEntity):
"""Contains prices for fuel in a given station."""
_attr_state_class = SensorStateClass.MEASUREMENT
@ -70,19 +67,12 @@ class FuelPriceSensor(CoordinatorEntity, SensorEntity):
def __init__(self, fuel_type, station, coordinator, show_on_map):
"""Initialize the sensor."""
super().__init__(coordinator)
super().__init__(coordinator, station)
self._station_id = station["id"]
self._fuel_type = fuel_type
self._attr_name = f"{station['brand']} {station['street']} {station['houseNumber']} {FUEL_TYPES[fuel_type]}"
self._attr_native_unit_of_measurement = CURRENCY_EURO
self._attr_unique_id = f"{station['id']}_{fuel_type}"
self._attr_device_info = DeviceInfo(
identifiers={(ATTR_ID, station["id"])},
name=f"{station['brand']} {station['street']} {station['houseNumber']}",
model=station["brand"],
configuration_url="https://www.tankerkoenig.de",
)
attrs = {
ATTR_ATTRIBUTION: ATTRIBUTION,
ATTR_BRAND: station["brand"],

View File

@ -95,7 +95,7 @@ async def test_user(hass: HomeAssistant):
assert result["step_id"] == "user"
with patch(
"homeassistant.components.tankerkoenig.async_setup_entry"
"homeassistant.components.tankerkoenig.async_setup_entry", return_value=True
) as mock_setup_entry, patch(
"homeassistant.components.tankerkoenig.config_flow.getNearbyStations",
return_value=MOCK_NEARVY_STATIONS_OK,
@ -147,6 +147,7 @@ async def test_user_already_configured(hass: HomeAssistant):
)
assert result["type"] == RESULT_TYPE_ABORT
assert result["reason"] == "already_configured"
async def test_exception_security(hass: HomeAssistant):
@ -193,7 +194,7 @@ async def test_user_no_stations(hass: HomeAssistant):
async def test_import(hass: HomeAssistant):
"""Test starting a flow by import."""
with patch(
"homeassistant.components.tankerkoenig.async_setup_entry"
"homeassistant.components.tankerkoenig.async_setup_entry", return_value=True
) as mock_setup_entry, patch(
"homeassistant.components.tankerkoenig.config_flow.getNearbyStations",
return_value=MOCK_NEARVY_STATIONS_OK,
@ -233,12 +234,12 @@ async def test_options_flow(hass: HomeAssistant):
mock_config.add_to_hass(hass)
with patch(
"homeassistant.components.tankerkoenig.async_setup_entry"
"homeassistant.components.tankerkoenig.async_setup_entry", return_value=True
) as mock_setup_entry, patch(
"homeassistant.components.tankerkoenig.config_flow.getNearbyStations",
return_value=MOCK_NEARVY_STATIONS_OK,
):
await mock_config.async_setup(hass)
await hass.config_entries.async_setup(mock_config.entry_id)
await hass.async_block_till_done()
assert mock_setup_entry.called