Address FireServiceRota late code review (#43741)

* Address review comment from PR #38206

* Address review comment from PR #43638

* Address review comment from PR #43700

* isort fixed

* Better code for duty entity update

* Removed all pylint relative-beyond-top-level

* Removed logger entry from entity state method

homeassistant/components/fireservicerota/__init__.py

@@ -75,7 +75,9 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
 async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     """Unload FireServiceRota config entry."""
 
-    hass.data[DOMAIN][entry.entry_id].websocket.stop_listener()
+    await hass.async_add_executor_job(
+        hass.data[DOMAIN][entry.entry_id].websocket.stop_listener
+    )
 
     unload_ok = all(
         await asyncio.gather(
@@ -115,7 +117,7 @@ class FireServiceRotaOauth:
 
         except (InvalidAuthError, InvalidTokenError):
             _LOGGER.error("Error refreshing tokens, triggered reauth workflow")
-            self._hass.add_job(
+            self._hass.async_create_task(
                 self._hass.config_entries.flow.async_init(
                     DOMAIN,
                     context={"source": SOURCE_REAUTH},
@@ -150,7 +152,7 @@ class FireServiceRotaWebSocket:
         self._entry = entry
 
         self._fsr_incidents = FireServiceRotaIncidents(on_incident=self._on_incident)
-        self._incident_data = None
+        self.incident_data = None
 
     def _construct_url(self) -> str:
         """Return URL with latest access token."""
@@ -158,14 +160,10 @@ class FireServiceRotaWebSocket:
             self._entry.data[CONF_URL], self._entry.data[CONF_TOKEN]["access_token"]
         )
 
-    def incident_data(self) -> object:
-        """Return incident data."""
-        return self._incident_data
-
     def _on_incident(self, data) -> None:
         """Received new incident, update data."""
         _LOGGER.debug("Received new incident via websocket: %s", data)
-        self._incident_data = data
+        self.incident_data = data
         dispatcher_send(self._hass, f"{DOMAIN}_{self._entry.entry_id}_update")
 
     def start_listener(self) -> None:
@@ -190,6 +188,9 @@ class FireServiceRotaClient:
         self._url = entry.data[CONF_URL]
         self._tokens = entry.data[CONF_TOKEN]
 
+        self.entry_id = entry.entry_id
+        self.unique_id = entry.unique_id
+
         self.token_refresh_failure = False
         self.incident_id = None
         self.on_duty = False
@@ -216,12 +217,12 @@ class FireServiceRotaClient:
         try:
             return await self._hass.async_add_executor_job(func, *args)
         except (ExpiredTokenError, InvalidTokenError):
-            self.websocket.stop_listener()
+            await self._hass.async_add_executor_job(self.websocket.stop_listener)
             self.token_refresh_failure = True
 
             if await self.oauth.async_refresh_tokens():
                 self.token_refresh_failure = False
-                self.websocket.start_listener()
+                await self._hass.async_add_executor_job(self.websocket.start_listener)
 
                 return await self._hass.async_add_executor_job(func, *args)
 
@@ -231,6 +232,9 @@ class FireServiceRotaClient:
             self.fsr.get_availability, str(self._hass.config.time_zone)
         )
 
+        if not data:
+            return
+
         self.on_duty = bool(data.get("available"))
 
         _LOGGER.debug("Updated availability data: %s", data)

homeassistant/components/fireservicerota/binary_sensor.py

@@ -63,7 +63,6 @@ class ResponseBinarySensor(CoordinatorEntity, BinarySensorEntity):
 
         self._state = self._client.on_duty
 
-        _LOGGER.debug("Set state of entity 'Duty Binary Sensor' to '%s'", self._state)
         return self._state
 
     @property
@@ -89,5 +88,4 @@ class ResponseBinarySensor(CoordinatorEntity, BinarySensorEntity):
             if key in data
         }
 
-        _LOGGER.debug("Set attributes of entity 'Duty Binary Sensor' to '%s'", attr)
         return attr

homeassistant/components/fireservicerota/config_flow.py

@@ -5,7 +5,6 @@ import voluptuous as vol
 from homeassistant import config_entries
 from homeassistant.const import CONF_PASSWORD, CONF_TOKEN, CONF_URL, CONF_USERNAME
 
-# pylint: disable=relative-beyond-top-level
 from .const import DOMAIN, URL_LIST
 
 DATA_SCHEMA = vol.Schema(
@@ -57,14 +56,14 @@ class FireServiceRotaFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
             await self.async_set_unique_id(self._username)
             self._abort_if_unique_id_configured()
 
-        try:
+        self.api = FireServiceRota(
-            self.api = FireServiceRota(
+            base_url=self._base_url,
-                base_url=self._base_url,
+            username=self._username,
-                username=self._username,
+            password=self._password,
-                password=self._password,
+        )
-            )
-            token_info = await self.hass.async_add_executor_job(self.api.request_tokens)
 
+        try:
+            token_info = await self.hass.async_add_executor_job(self.api.request_tokens)
         except InvalidAuthError:
             self.api = None
             return self.async_show_form(

homeassistant/components/fireservicerota/sensor.py

@@ -7,7 +7,6 @@ from homeassistant.helpers.dispatcher import async_dispatcher_connect
 from homeassistant.helpers.restore_state import RestoreEntity
 from homeassistant.helpers.typing import HomeAssistantType
 
-# pylint: disable=relative-beyond-top-level
 from .const import DATA_CLIENT, DOMAIN as FIRESERVICEROTA_DOMAIN
 
 _LOGGER = logging.getLogger(__name__)
@@ -28,8 +27,8 @@ class IncidentsSensor(RestoreEntity):
     def __init__(self, client):
         """Initialize."""
         self._client = client
-        self._entry_id = self._client._entry.entry_id
+        self._entry_id = self._client.entry_id
-        self._unique_id = f"{self._client._entry.unique_id}_Incidents"
+        self._unique_id = f"{self._client.unique_id}_Incidents"
         self._state = None
         self._state_attributes = {}
 
@@ -123,7 +122,7 @@ class IncidentsSensor(RestoreEntity):
     @callback
     def client_update(self) -> None:
         """Handle updated data from the data client."""
-        data = self._client.websocket.incident_data()
+        data = self._client.websocket.incident_data
         if not data or "body" not in data:
             return
 

homeassistant/components/fireservicerota/switch.py

@@ -7,7 +7,6 @@ from homeassistant.core import callback
 from homeassistant.helpers.dispatcher import async_dispatcher_connect
 from homeassistant.helpers.typing import HomeAssistantType
 
-# pylint: disable=relative-beyond-top-level
 from .const import DATA_CLIENT, DATA_COORDINATOR, DOMAIN as FIRESERVICEROTA_DOMAIN
 
 _LOGGER = logging.getLogger(__name__)
@@ -97,7 +96,6 @@ class ResponseSwitch(SwitchEntity):
             if key in data
         }
 
-        _LOGGER.debug("Set attributes of entity 'Response Switch' to '%s'", attr)
         return attr
 
     async def async_turn_on(self, **kwargs) -> None:
@@ -128,12 +126,9 @@ class ResponseSwitch(SwitchEntity):
                 self.client_update,
             )
         )
-        self.async_on_remove(self._coordinator.async_add_listener(self.on_duty_update))
+        self.async_on_remove(
-
+            self._coordinator.async_add_listener(self.async_write_ha_state)
-    @callback
+        )
-    def on_duty_update(self):
-        """Trigger on a on duty update."""
-        self.async_write_ha_state()
 
     @callback
     def client_update(self) -> None:

tests/components/fireservicerota/test_config_flow.py

@@ -2,9 +2,7 @@
 from pyfireservicerota import InvalidAuthError
 
 from homeassistant import data_entry_flow
-from homeassistant.components.fireservicerota.const import (  # pylint: disable=unused-import
+from homeassistant.components.fireservicerota.const import DOMAIN
-    DOMAIN,
-)
 from homeassistant.const import CONF_PASSWORD, CONF_URL, CONF_USERNAME
 
 from tests.async_mock import patch
@@ -16,7 +14,6 @@ MOCK_CONF = {
     CONF_URL: "www.brandweerrooster.nl",
 }
 
-
 MOCK_DATA = {
     "auth_implementation": DOMAIN,
     CONF_URL: MOCK_CONF[CONF_URL],
@@ -79,14 +76,14 @@ async def test_step_user(hass):
 
     with patch(
         "homeassistant.components.fireservicerota.config_flow.FireServiceRota"
-    ) as MockFireServiceRota, patch(
+    ) as mock_fsr, patch(
         "homeassistant.components.fireservicerota.async_setup", return_value=True
     ) as mock_setup, patch(
         "homeassistant.components.fireservicerota.async_setup_entry",
         return_value=True,
     ) as mock_setup_entry:
 
-        mock_fireservicerota = MockFireServiceRota.return_value
+        mock_fireservicerota = mock_fsr.return_value
         mock_fireservicerota.request_tokens.return_value = MOCK_TOKEN_INFO
 
         result = await hass.config_entries.flow.async_init(