From a88335272a81cc0a68af5f1b2dff541780a4ea18 Mon Sep 17 00:00:00 2001 From: Brett Adams Date: Mon, 18 Dec 2023 11:33:26 +1000 Subject: [PATCH] Implement Review Feedback for Tessie (#105937) --- .../components/tessie/binary_sensor.py | 7 +++++- .../components/tessie/config_flow.py | 5 ++++ homeassistant/components/tessie/const.py | 1 - .../components/tessie/coordinator.py | 19 ++++++--------- homeassistant/components/tessie/sensor.py | 7 +----- homeassistant/components/tessie/strings.json | 13 ++++------ tests/components/tessie/test_coordinator.py | 24 ++++--------------- 7 files changed, 28 insertions(+), 48 deletions(-) diff --git a/homeassistant/components/tessie/binary_sensor.py b/homeassistant/components/tessie/binary_sensor.py index ca78b19a42b..297a59cac6d 100644 --- a/homeassistant/components/tessie/binary_sensor.py +++ b/homeassistant/components/tessie/binary_sensor.py @@ -14,7 +14,7 @@ from homeassistant.const import EntityCategory from homeassistant.core import HomeAssistant from homeassistant.helpers.entity_platform import AddEntitiesCallback -from .const import DOMAIN +from .const import DOMAIN, TessieStatus from .coordinator import TessieDataUpdateCoordinator from .entity import TessieEntity @@ -27,6 +27,11 @@ class TessieBinarySensorEntityDescription(BinarySensorEntityDescription): DESCRIPTIONS: tuple[TessieBinarySensorEntityDescription, ...] = ( + TessieBinarySensorEntityDescription( + key="state", + device_class=BinarySensorDeviceClass.CONNECTIVITY, + is_on=lambda x: x == TessieStatus.ONLINE, + ), TessieBinarySensorEntityDescription( key="charge_state_battery_heater_on", device_class=BinarySensorDeviceClass.HEAT, diff --git a/homeassistant/components/tessie/config_flow.py b/homeassistant/components/tessie/config_flow.py index 3e3207b264b..97d9d44af70 100644 --- a/homeassistant/components/tessie/config_flow.py +++ b/homeassistant/components/tessie/config_flow.py @@ -18,6 +18,9 @@ from homeassistant.helpers.aiohttp_client import async_get_clientsession from .const import DOMAIN TESSIE_SCHEMA = vol.Schema({vol.Required(CONF_ACCESS_TOKEN): str}) +DESCRIPTION_PLACEHOLDERS = { + "url": "[my.tessie.com/settings/api](https://my.tessie.com/settings/api)" +} class TessieConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): @@ -57,6 +60,7 @@ class TessieConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): return self.async_show_form( step_id="user", data_schema=TESSIE_SCHEMA, + description_placeholders=DESCRIPTION_PLACEHOLDERS, errors=errors, ) @@ -98,5 +102,6 @@ class TessieConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): return self.async_show_form( step_id="reauth_confirm", data_schema=TESSIE_SCHEMA, + description_placeholders=DESCRIPTION_PLACEHOLDERS, errors=errors, ) diff --git a/homeassistant/components/tessie/const.py b/homeassistant/components/tessie/const.py index dad9ba2345f..3aa7dbb185d 100644 --- a/homeassistant/components/tessie/const.py +++ b/homeassistant/components/tessie/const.py @@ -18,4 +18,3 @@ class TessieStatus(StrEnum): ASLEEP = "asleep" ONLINE = "online" - OFFLINE = "offline" diff --git a/homeassistant/components/tessie/coordinator.py b/homeassistant/components/tessie/coordinator.py index 397d9cb4dfc..0fdfbcc5345 100644 --- a/homeassistant/components/tessie/coordinator.py +++ b/homeassistant/components/tessie/coordinator.py @@ -20,7 +20,7 @@ TESSIE_SYNC_INTERVAL = 10 _LOGGER = logging.getLogger(__name__) -class TessieDataUpdateCoordinator(DataUpdateCoordinator): +class TessieDataUpdateCoordinator(DataUpdateCoordinator[dict[str, Any]]): """Class to manage fetching data from the Tessie API.""" def __init__( @@ -35,16 +35,15 @@ class TessieDataUpdateCoordinator(DataUpdateCoordinator): hass, _LOGGER, name="Tessie", - update_method=self.async_update_data, update_interval=timedelta(seconds=TESSIE_SYNC_INTERVAL), ) self.api_key = api_key self.vin = vin self.session = async_get_clientsession(hass) - self.data = self._flattern(data) + self.data = self._flatten(data) self.did_first_update = False - async def async_update_data(self) -> dict[str, Any]: + async def _async_update_data(self) -> dict[str, Any]: """Update vehicle data using Tessie API.""" try: vehicle = await get_state( @@ -54,10 +53,6 @@ class TessieDataUpdateCoordinator(DataUpdateCoordinator): use_cache=self.did_first_update, ) except ClientResponseError as e: - if e.status == HTTPStatus.REQUEST_TIMEOUT: - # Vehicle is offline, only update state and dont throw error - self.data["state"] = TessieStatus.OFFLINE - return self.data if e.status == HTTPStatus.UNAUTHORIZED: # Auth Token is no longer valid raise ConfigEntryAuthFailed from e @@ -66,22 +61,22 @@ class TessieDataUpdateCoordinator(DataUpdateCoordinator): self.did_first_update = True if vehicle["state"] == TessieStatus.ONLINE: # Vehicle is online, all data is fresh - return self._flattern(vehicle) + return self._flatten(vehicle) # Vehicle is asleep, only update state self.data["state"] = vehicle["state"] return self.data - def _flattern( + def _flatten( self, data: dict[str, Any], parent: str | None = None ) -> dict[str, Any]: - """Flattern the data structure.""" + """Flatten the data structure.""" result = {} for key, value in data.items(): if parent: key = f"{parent}_{key}" if isinstance(value, dict): - result.update(self._flattern(value, key)) + result.update(self._flatten(value, key)) else: result[key] = value return result diff --git a/homeassistant/components/tessie/sensor.py b/homeassistant/components/tessie/sensor.py index 2836b7e3931..6f79d986998 100644 --- a/homeassistant/components/tessie/sensor.py +++ b/homeassistant/components/tessie/sensor.py @@ -27,7 +27,7 @@ from homeassistant.core import HomeAssistant from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.typing import StateType -from .const import DOMAIN, TessieStatus +from .const import DOMAIN from .coordinator import TessieDataUpdateCoordinator from .entity import TessieEntity @@ -40,11 +40,6 @@ class TessieSensorEntityDescription(SensorEntityDescription): DESCRIPTIONS: tuple[TessieSensorEntityDescription, ...] = ( - TessieSensorEntityDescription( - key="state", - options=[status.value for status in TessieStatus], - device_class=SensorDeviceClass.ENUM, - ), TessieSensorEntityDescription( key="charge_state_usable_battery_level", state_class=SensorStateClass.MEASUREMENT, diff --git a/homeassistant/components/tessie/strings.json b/homeassistant/components/tessie/strings.json index 8785f2aadc3..43ddd7b4954 100644 --- a/homeassistant/components/tessie/strings.json +++ b/homeassistant/components/tessie/strings.json @@ -10,7 +10,7 @@ "data": { "access_token": "[%key:common::config_flow::data::access_token%]" }, - "description": "Enter your access token from [my.tessie.com/settings/api](https://my.tessie.com/settings/api)." + "description": "Enter your access token from {url}." }, "reauth_confirm": { "data": { @@ -23,14 +23,6 @@ }, "entity": { "sensor": { - "state": { - "name": "Status", - "state": { - "online": "Online", - "asleep": "Asleep", - "offline": "Offline" - } - }, "charge_state_usable_battery_level": { "name": "Battery level" }, @@ -96,6 +88,9 @@ } }, "binary_sensor": { + "state": { + "name": "Status" + }, "charge_state_battery_heater_on": { "name": "Battery heater" }, diff --git a/tests/components/tessie/test_coordinator.py b/tests/components/tessie/test_coordinator.py index 50a9f2f7733..6fc263e6908 100644 --- a/tests/components/tessie/test_coordinator.py +++ b/tests/components/tessie/test_coordinator.py @@ -2,15 +2,13 @@ from datetime import timedelta from homeassistant.components.tessie.coordinator import TESSIE_SYNC_INTERVAL -from homeassistant.components.tessie.sensor import TessieStatus -from homeassistant.const import STATE_UNAVAILABLE +from homeassistant.const import STATE_OFF, STATE_ON, STATE_UNAVAILABLE from homeassistant.core import HomeAssistant from homeassistant.util.dt import utcnow from .common import ( ERROR_AUTH, ERROR_CONNECTION, - ERROR_TIMEOUT, ERROR_UNKNOWN, TEST_VEHICLE_STATE_ASLEEP, TEST_VEHICLE_STATE_ONLINE, @@ -31,7 +29,7 @@ async def test_coordinator_online(hass: HomeAssistant, mock_get_state) -> None: async_fire_time_changed(hass, utcnow() + WAIT) await hass.async_block_till_done() mock_get_state.assert_called_once() - assert hass.states.get("sensor.test_status").state == TessieStatus.ONLINE + assert hass.states.get("binary_sensor.test_status").state == STATE_ON async def test_coordinator_asleep(hass: HomeAssistant, mock_get_state) -> None: @@ -43,7 +41,7 @@ async def test_coordinator_asleep(hass: HomeAssistant, mock_get_state) -> None: async_fire_time_changed(hass, utcnow() + WAIT) await hass.async_block_till_done() mock_get_state.assert_called_once() - assert hass.states.get("sensor.test_status").state == TessieStatus.ASLEEP + assert hass.states.get("binary_sensor.test_status").state == STATE_OFF async def test_coordinator_clienterror(hass: HomeAssistant, mock_get_state) -> None: @@ -55,19 +53,7 @@ async def test_coordinator_clienterror(hass: HomeAssistant, mock_get_state) -> N async_fire_time_changed(hass, utcnow() + WAIT) await hass.async_block_till_done() mock_get_state.assert_called_once() - assert hass.states.get("sensor.test_status").state == STATE_UNAVAILABLE - - -async def test_coordinator_timeout(hass: HomeAssistant, mock_get_state) -> None: - """Tests that the coordinator handles timeout errors.""" - - mock_get_state.side_effect = ERROR_TIMEOUT - await setup_platform(hass) - - async_fire_time_changed(hass, utcnow() + WAIT) - await hass.async_block_till_done() - mock_get_state.assert_called_once() - assert hass.states.get("sensor.test_status").state == TessieStatus.OFFLINE + assert hass.states.get("binary_sensor.test_status").state == STATE_UNAVAILABLE async def test_coordinator_auth(hass: HomeAssistant, mock_get_state) -> None: @@ -89,4 +75,4 @@ async def test_coordinator_connection(hass: HomeAssistant, mock_get_state) -> No async_fire_time_changed(hass, utcnow() + WAIT) await hass.async_block_till_done() mock_get_state.assert_called_once() - assert hass.states.get("sensor.test_status").state == STATE_UNAVAILABLE + assert hass.states.get("binary_sensor.test_status").state == STATE_UNAVAILABLE