From e0c0d3b53f8a98688198dac300de580e1d9a4a42 Mon Sep 17 00:00:00 2001 From: Quentame Date: Fri, 14 Aug 2020 15:10:13 +0200 Subject: [PATCH] Update meteo_france based on code review (#38789) * Review: if not to pop * Review: async_add_job --> async_add_executor_job * Review: const * Review: start logging messages with capital letter * Review : UTC isoformated time --> fix "Invalid date"" * Fix hail forecast condition * Review: _show_setup_form is a callback * Fix update option * Review: no icon for next_rain * Review: inline cities form * Review: store places as an instance attribute * UNDO_UPDATE_LISTENER() --- .../components/meteo_france/__init__.py | 20 +++++-- .../components/meteo_france/config_flow.py | 58 ++++++++++--------- .../components/meteo_france/const.py | 36 +++++++----- .../components/meteo_france/sensor.py | 20 ++++--- .../components/meteo_france/weather.py | 5 +- 5 files changed, 82 insertions(+), 57 deletions(-) diff --git a/homeassistant/components/meteo_france/__init__.py b/homeassistant/components/meteo_france/__init__.py index 469c66ad79f..276aac188d9 100644 --- a/homeassistant/components/meteo_france/__init__.py +++ b/homeassistant/components/meteo_france/__init__.py @@ -20,6 +20,7 @@ from .const import ( COORDINATOR_RAIN, DOMAIN, PLATFORMS, + UNDO_UPDATE_LISTENER, ) _LOGGER = logging.getLogger(__name__) @@ -77,15 +78,17 @@ async def async_setup_entry(hass: HomeAssistantType, entry: ConfigEntry) -> bool async def _async_update_data_forecast_forecast(): """Fetch data from API endpoint.""" - return await hass.async_add_job(client.get_forecast, latitude, longitude) + return await hass.async_add_executor_job( + client.get_forecast, latitude, longitude + ) async def _async_update_data_rain(): """Fetch data from API endpoint.""" - return await hass.async_add_job(client.get_rain, latitude, longitude) + return await hass.async_add_executor_job(client.get_rain, latitude, longitude) async def _async_update_data_alert(): """Fetch data from API endpoint.""" - return await hass.async_add_job( + return await hass.async_add_executor_job( client.get_warning_current_phenomenoms, department, 0, True ) @@ -156,10 +159,13 @@ async def async_setup_entry(hass: HomeAssistantType, entry: ConfigEntry) -> bool entry.title, ) + undo_listener = entry.add_update_listener(_async_update_listener) + hass.data[DOMAIN][entry.entry_id] = { COORDINATOR_FORECAST: coordinator_forecast, COORDINATOR_RAIN: coordinator_rain, COORDINATOR_ALERT: coordinator_alert, + UNDO_UPDATE_LISTENER: undo_listener, } for platform in PLATFORMS: @@ -192,8 +198,14 @@ async def async_unload_entry(hass: HomeAssistantType, entry: ConfigEntry): ) ) if unload_ok: + hass.data[DOMAIN][entry.entry_id][UNDO_UPDATE_LISTENER]() hass.data[DOMAIN].pop(entry.entry_id) - if len(hass.data[DOMAIN]) == 0: + if not hass.data[DOMAIN]: hass.data.pop(DOMAIN) return unload_ok + + +async def _async_update_listener(hass: HomeAssistantType, entry: ConfigEntry): + """Handle options update.""" + await hass.config_entries.async_reload(entry.entry_id) diff --git a/homeassistant/components/meteo_france/config_flow.py b/homeassistant/components/meteo_france/config_flow.py index 73b1ea41089..0854c280c16 100644 --- a/homeassistant/components/meteo_france/config_flow.py +++ b/homeassistant/components/meteo_france/config_flow.py @@ -21,13 +21,18 @@ class MeteoFranceFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): VERSION = 1 CONNECTION_CLASS = config_entries.CONN_CLASS_CLOUD_POLL + def __init__(self): + """Init MeteoFranceFlowHandler.""" + self.places = [] + @staticmethod @callback def async_get_options_flow(config_entry): """Get the options flow for this handler.""" return MeteoFranceOptionsFlowHandler(config_entry) - async def _show_setup_form(self, user_input=None, errors=None): + @callback + def _show_setup_form(self, user_input=None, errors=None): """Show the setup form to the user.""" if user_input is None: @@ -46,7 +51,7 @@ class MeteoFranceFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): errors = {} if user_input is None: - return await self._show_setup_form(user_input, errors) + return self._show_setup_form(user_input, errors) city = user_input[CONF_CITY] # Might be a city name or a postal code latitude = user_input.get(CONF_LATITUDE) @@ -54,13 +59,15 @@ class MeteoFranceFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): if not latitude: client = MeteoFranceClient() - places = await self.hass.async_add_executor_job(client.search_places, city) - _LOGGER.debug("places search result: %s", places) - if not places: + self.places = await self.hass.async_add_executor_job( + client.search_places, city + ) + _LOGGER.debug("Places search result: %s", self.places) + if not self.places: errors[CONF_CITY] = "empty" - return await self._show_setup_form(user_input, errors) + return self._show_setup_form(user_input, errors) - return await self.async_step_cities(places=places) + return await self.async_step_cities() # Check if already configured await self.async_set_unique_id(f"{latitude}, {longitude}") @@ -74,19 +81,27 @@ class MeteoFranceFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): """Import a config entry.""" return await self.async_step_user(user_input) - async def async_step_cities(self, user_input=None, places=None): + async def async_step_cities(self, user_input=None): """Step where the user choose the city from the API search results.""" - if places and len(places) > 1 and self.source != SOURCE_IMPORT: - places_for_form = {} - for place in places: - places_for_form[_build_place_key(place)] = f"{place}" + if not user_input: + if len(self.places) > 1 and self.source != SOURCE_IMPORT: + places_for_form = {} + for place in self.places: + places_for_form[_build_place_key(place)] = f"{place}" - return await self._show_cities_form(places_for_form) - # for import and only 1 city in the search result - if places and not user_input: - user_input = {CONF_CITY: _build_place_key(places[0])} + return self.async_show_form( + step_id="cities", + data_schema=vol.Schema( + { + vol.Required(CONF_CITY): vol.All( + vol.Coerce(str), vol.In(places_for_form) + ) + } + ), + ) + user_input = {CONF_CITY: _build_place_key(self.places[0])} - city_infos = user_input.get(CONF_CITY).split(";") + city_infos = user_input[CONF_CITY].split(";") return await self.async_step_user( { CONF_CITY: city_infos[0], @@ -95,15 +110,6 @@ class MeteoFranceFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): } ) - async def _show_cities_form(self, cities): - """Show the form to choose the city.""" - return self.async_show_form( - step_id="cities", - data_schema=vol.Schema( - {vol.Required(CONF_CITY): vol.All(vol.Coerce(str), vol.In(cities))} - ), - ) - class MeteoFranceOptionsFlowHandler(config_entries.OptionsFlow): """Handle a option flow.""" diff --git a/homeassistant/components/meteo_france/const.py b/homeassistant/components/meteo_france/const.py index d1decb54078..8e6c625e331 100644 --- a/homeassistant/components/meteo_france/const.py +++ b/homeassistant/components/meteo_france/const.py @@ -1,6 +1,9 @@ """Meteo-France component constants.""" from homeassistant.const import ( + DEVICE_CLASS_PRESSURE, + DEVICE_CLASS_TEMPERATURE, + DEVICE_CLASS_TIMESTAMP, PRESSURE_HPA, SPEED_KILOMETERS_PER_HOUR, TEMP_CELSIUS, @@ -12,6 +15,7 @@ PLATFORMS = ["sensor", "weather"] COORDINATOR_FORECAST = "coordinator_forecast" COORDINATOR_RAIN = "coordinator_rain" COORDINATOR_ALERT = "coordinator_alert" +UNDO_UPDATE_LISTENER = "undo_update_listener" ATTRIBUTION = "Data provided by Météo-France" CONF_CITY = "city" @@ -24,7 +28,7 @@ ATTR_NEXT_RAIN_1_HOUR_FORECAST = "1_hour_forecast" ENTITY_NAME = "name" ENTITY_UNIT = "unit" ENTITY_ICON = "icon" -ENTITY_CLASS = "device_class" +ENTITY_DEVICE_CLASS = "device_class" ENTITY_ENABLE = "enable" ENTITY_API_DATA_PATH = "data_path" @@ -32,8 +36,8 @@ SENSOR_TYPES = { "pressure": { ENTITY_NAME: "Pressure", ENTITY_UNIT: PRESSURE_HPA, - ENTITY_ICON: "mdi:gauge", - ENTITY_CLASS: "pressure", + ENTITY_ICON: None, + ENTITY_DEVICE_CLASS: DEVICE_CLASS_PRESSURE, ENTITY_ENABLE: False, ENTITY_API_DATA_PATH: "current_forecast:sea_level", }, @@ -41,7 +45,7 @@ SENSOR_TYPES = { ENTITY_NAME: "Rain chance", ENTITY_UNIT: UNIT_PERCENTAGE, ENTITY_ICON: "mdi:weather-rainy", - ENTITY_CLASS: None, + ENTITY_DEVICE_CLASS: None, ENTITY_ENABLE: True, ENTITY_API_DATA_PATH: "probability_forecast:rain:3h", }, @@ -49,7 +53,7 @@ SENSOR_TYPES = { ENTITY_NAME: "Snow chance", ENTITY_UNIT: UNIT_PERCENTAGE, ENTITY_ICON: "mdi:weather-snowy", - ENTITY_CLASS: None, + ENTITY_DEVICE_CLASS: None, ENTITY_ENABLE: True, ENTITY_API_DATA_PATH: "probability_forecast:snow:3h", }, @@ -57,7 +61,7 @@ SENSOR_TYPES = { ENTITY_NAME: "Freeze chance", ENTITY_UNIT: UNIT_PERCENTAGE, ENTITY_ICON: "mdi:snowflake", - ENTITY_CLASS: None, + ENTITY_DEVICE_CLASS: None, ENTITY_ENABLE: True, ENTITY_API_DATA_PATH: "probability_forecast:freezing", }, @@ -65,23 +69,23 @@ SENSOR_TYPES = { ENTITY_NAME: "Wind speed", ENTITY_UNIT: SPEED_KILOMETERS_PER_HOUR, ENTITY_ICON: "mdi:weather-windy", - ENTITY_CLASS: None, + ENTITY_DEVICE_CLASS: None, ENTITY_ENABLE: False, ENTITY_API_DATA_PATH: "current_forecast:wind:speed", }, "next_rain": { ENTITY_NAME: "Next rain", ENTITY_UNIT: None, - ENTITY_ICON: "mdi:weather-pouring", - ENTITY_CLASS: "timestamp", + ENTITY_ICON: None, + ENTITY_DEVICE_CLASS: DEVICE_CLASS_TIMESTAMP, ENTITY_ENABLE: True, ENTITY_API_DATA_PATH: None, }, "temperature": { ENTITY_NAME: "Temperature", ENTITY_UNIT: TEMP_CELSIUS, - ENTITY_ICON: "mdi:thermometer", - ENTITY_CLASS: "temperature", + ENTITY_ICON: None, + ENTITY_DEVICE_CLASS: DEVICE_CLASS_TEMPERATURE, ENTITY_ENABLE: False, ENTITY_API_DATA_PATH: "current_forecast:T:value", }, @@ -89,7 +93,7 @@ SENSOR_TYPES = { ENTITY_NAME: "UV", ENTITY_UNIT: None, ENTITY_ICON: "mdi:sunglasses", - ENTITY_CLASS: None, + ENTITY_DEVICE_CLASS: None, ENTITY_ENABLE: True, ENTITY_API_DATA_PATH: "today_forecast:uv", }, @@ -97,7 +101,7 @@ SENSOR_TYPES = { ENTITY_NAME: "Weather alert", ENTITY_UNIT: None, ENTITY_ICON: "mdi:weather-cloudy-alert", - ENTITY_CLASS: None, + ENTITY_DEVICE_CLASS: None, ENTITY_ENABLE: True, ENTITY_API_DATA_PATH: None, }, @@ -105,7 +109,7 @@ SENSOR_TYPES = { ENTITY_NAME: "Daily precipitation", ENTITY_UNIT: "mm", ENTITY_ICON: "mdi:cup-water", - ENTITY_CLASS: None, + ENTITY_DEVICE_CLASS: None, ENTITY_ENABLE: True, ENTITY_API_DATA_PATH: "today_forecast:precipitation:24h", }, @@ -113,7 +117,7 @@ SENSOR_TYPES = { ENTITY_NAME: "Cloud cover", ENTITY_UNIT: UNIT_PERCENTAGE, ENTITY_ICON: "mdi:weather-partly-cloudy", - ENTITY_CLASS: None, + ENTITY_DEVICE_CLASS: None, ENTITY_ENABLE: True, ENTITY_API_DATA_PATH: "current_forecast:clouds", }, @@ -128,7 +132,7 @@ CONDITION_CLASSES = { "Brouillard", "Brouillard givrant", ], - "hail": ["Risque de grêle"], + "hail": ["Risque de grêle", "Risque de grèle"], "lightning": ["Risque d'orages", "Orages"], "lightning-rainy": ["Pluie orageuses", "Pluies orageuses", "Averses orageuses"], "partlycloudy": [ diff --git a/homeassistant/components/meteo_france/sensor.py b/homeassistant/components/meteo_france/sensor.py index 39e33dafd65..927250c6af6 100644 --- a/homeassistant/components/meteo_france/sensor.py +++ b/homeassistant/components/meteo_france/sensor.py @@ -21,7 +21,7 @@ from .const import ( COORDINATOR_RAIN, DOMAIN, ENTITY_API_DATA_PATH, - ENTITY_CLASS, + ENTITY_DEVICE_CLASS, ENTITY_ENABLE, ENTITY_ICON, ENTITY_NAME, @@ -128,7 +128,7 @@ class MeteoFranceSensor(Entity): @property def device_class(self): """Return the device class.""" - return SENSOR_TYPES[self._type][ENTITY_CLASS] + return SENSOR_TYPES[self._type][ENTITY_DEVICE_CLASS] @property def entity_registry_enabled_default(self) -> bool: @@ -170,9 +170,15 @@ class MeteoFranceRainSensor(MeteoFranceSensor): @property def state(self): """Return the state.""" - next_rain_date_locale = self.coordinator.data.next_rain_date_locale() + # search first cadran with rain + next_rain = next( + (cadran for cadran in self.coordinator.data.forecast if cadran["rain"] > 1), + None, + ) return ( - dt_util.as_local(next_rain_date_locale) if next_rain_date_locale else None + dt_util.utc_from_timestamp(next_rain["dt"]).isoformat() + if next_rain + else None ) @property @@ -180,11 +186,7 @@ class MeteoFranceRainSensor(MeteoFranceSensor): """Return the state attributes.""" return { ATTR_NEXT_RAIN_1_HOUR_FORECAST: [ - { - dt_util.as_local( - self.coordinator.data.timestamp_to_locale_time(item["dt"]) - ).strftime("%H:%M"): item["desc"] - } + {dt_util.utc_from_timestamp(item["dt"]).isoformat(): item["desc"]} for item in self.coordinator.data.forecast ], ATTR_ATTRIBUTION: ATTRIBUTION, diff --git a/homeassistant/components/meteo_france/weather.py b/homeassistant/components/meteo_france/weather.py index a9c4840901b..e8afcea9702 100644 --- a/homeassistant/components/meteo_france/weather.py +++ b/homeassistant/components/meteo_france/weather.py @@ -16,6 +16,7 @@ from homeassistant.config_entries import ConfigEntry from homeassistant.const import CONF_MODE, TEMP_CELSIUS from homeassistant.helpers.typing import HomeAssistantType from homeassistant.helpers.update_coordinator import DataUpdateCoordinator +from homeassistant.util import dt as dt_util from .const import ( ATTRIBUTION, @@ -134,9 +135,9 @@ class MeteoFranceWeather(WeatherEntity): continue forecast_data.append( { - ATTR_FORECAST_TIME: self.coordinator.data.timestamp_to_locale_time( + ATTR_FORECAST_TIME: dt_util.utc_from_timestamp( forecast["dt"] - ), + ).isoformat(), ATTR_FORECAST_CONDITION: format_condition( forecast["weather"]["desc"] ),