Migrate Brother to use DataUpdateCoordinator (#32800)

* Use DataCoordinator to update Brother data

* Simplify error text

* Improve tests

* Rename DEFAULT_SCAN_INTERVAL to SCAN_INTERVAL

* Add entity_registry_enabled_default property

* Add quality_scale to manifest.json file

* Remove misleading coordinator.data.data

.coveragerc

@@ -84,9 +84,6 @@ omit =
     homeassistant/components/broadlink/remote.py
     homeassistant/components/broadlink/sensor.py
     homeassistant/components/broadlink/switch.py
-    homeassistant/components/brother/__init__.py
-    homeassistant/components/brother/sensor.py
-    homeassistant/components/brother/const.py
     homeassistant/components/brottsplatskartan/sensor.py
     homeassistant/components/browser/*
     homeassistant/components/brunt/cover.py

homeassistant/components/brother/__init__.py

@@ -9,20 +9,19 @@ from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import CONF_HOST, CONF_TYPE
 from homeassistant.core import Config, HomeAssistant
 from homeassistant.exceptions import ConfigEntryNotReady
-from homeassistant.util import Throttle
+from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
 
 from .const import DOMAIN
 
 PLATFORMS = ["sensor"]
 
-DEFAULT_SCAN_INTERVAL = timedelta(seconds=30)
+SCAN_INTERVAL = timedelta(seconds=30)
 
 _LOGGER = logging.getLogger(__name__)
 
 
 async def async_setup(hass: HomeAssistant, config: Config):
     """Set up the Brother component."""
-    hass.data[DOMAIN] = {}
     return True
 
 
@@ -31,14 +30,14 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry):
     host = entry.data[CONF_HOST]
     kind = entry.data[CONF_TYPE]
 
-    brother = BrotherPrinterData(host, kind)
+    coordinator = BrotherDataUpdateCoordinator(hass, host=host, kind=kind)
+    await coordinator.async_refresh()
 
-    await brother.async_update()
+    if not coordinator.last_update_success:
+        raise ConfigEntryNotReady
 
-    if not brother.available:
+    hass.data.setdefault(DOMAIN, {})
-        raise ConfigEntryNotReady()
+    hass.data[DOMAIN][entry.entry_id] = coordinator
-
-    hass.data[DOMAIN][entry.entry_id] = brother
 
     for component in PLATFORMS:
         hass.async_create_task(
@@ -64,39 +63,21 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry):
     return unload_ok
 
 
-class BrotherPrinterData:
+class BrotherDataUpdateCoordinator(DataUpdateCoordinator):
-    """Define an object to hold sensor data."""
+    """Class to manage fetching Brother data from the printer."""
 
-    def __init__(self, host, kind):
+    def __init__(self, hass, host, kind):
         """Initialize."""
-        self._brother = Brother(host, kind=kind)
+        self.brother = Brother(host, kind=kind)
-        self.host = host
-        self.model = None
-        self.serial = None
-        self.firmware = None
-        self.available = False
-        self.data = {}
-        self.unavailable_logged = False
 
-    @Throttle(DEFAULT_SCAN_INTERVAL)
+        super().__init__(
-    async def async_update(self):
+            hass, _LOGGER, name=DOMAIN, update_interval=SCAN_INTERVAL,
+        )
+
+    async def _async_update_data(self):
         """Update data via library."""
         try:
-            await self._brother.async_update()
+            await self.brother.async_update()
         except (ConnectionError, SnmpError, UnsupportedModel) as error:
-            if not self.unavailable_logged:
+            raise UpdateFailed(error)
-                _LOGGER.error(
+        return self.brother.data
-                    "Could not fetch data from %s, error: %s", self.host, error
-                )
-                self.unavailable_logged = True
-            self.available = self._brother.available
-            return
-
-        self.model = self._brother.model
-        self.serial = self._brother.serial
-        self.firmware = self._brother.firmware
-        self.available = self._brother.available
-        self.data = self._brother.data
-        if self.available and self.unavailable_logged:
-            _LOGGER.info("Printer %s is available again", self.host)
-            self.unavailable_logged = False

homeassistant/components/brother/manifest.json

@@ -6,5 +6,6 @@
   "codeowners": ["@bieniu"],
   "requirements": ["brother==0.1.9"],
   "zeroconf": ["_printer._tcp.local."],
-  "config_flow": true
+  "config_flow": true,
+  "quality_scale": "platinum"
 }

homeassistant/components/brother/sensor.py

@@ -28,54 +28,55 @@ from .const import (
 )
 
 ATTR_COUNTER = "counter"
+ATTR_FIRMWARE = "firmware"
+ATTR_MODEL = "model"
 ATTR_REMAINING_PAGES = "remaining_pages"
+ATTR_SERIAL = "serial"
 
 _LOGGER = logging.getLogger(__name__)
 
 
 async def async_setup_entry(hass, config_entry, async_add_entities):
     """Add Brother entities from a config_entry."""
-    brother = hass.data[DOMAIN][config_entry.entry_id]
+    coordinator = hass.data[DOMAIN][config_entry.entry_id]
 
     sensors = []
 
-    name = brother.model
     device_info = {
-        "identifiers": {(DOMAIN, brother.serial)},
+        "identifiers": {(DOMAIN, coordinator.data[ATTR_SERIAL])},
-        "name": brother.model,
+        "name": coordinator.data[ATTR_MODEL],
         "manufacturer": ATTR_MANUFACTURER,
-        "model": brother.model,
+        "model": coordinator.data[ATTR_MODEL],
-        "sw_version": brother.firmware,
+        "sw_version": coordinator.data.get(ATTR_FIRMWARE),
     }
 
     for sensor in SENSOR_TYPES:
-        if sensor in brother.data:
+        if sensor in coordinator.data:
-            sensors.append(BrotherPrinterSensor(brother, name, sensor, device_info))
+            sensors.append(BrotherPrinterSensor(coordinator, sensor, device_info))
-    async_add_entities(sensors, True)
+    async_add_entities(sensors, False)
 
 
 class BrotherPrinterSensor(Entity):
     """Define an Brother Printer sensor."""
 
-    def __init__(self, printer, name, kind, device_info):
+    def __init__(self, coordinator, kind, device_info):
         """Initialize."""
-        self.printer = printer
+        self._name = f"{coordinator.data[ATTR_MODEL]} {SENSOR_TYPES[kind][ATTR_LABEL]}"
-        self._name = name
+        self._unique_id = f"{coordinator.data[ATTR_SERIAL].lower()}_{kind}"
         self._device_info = device_info
-        self._unique_id = f"{self.printer.serial.lower()}_{kind}"
+        self.coordinator = coordinator
         self.kind = kind
-        self._state = None
         self._attrs = {}
 
     @property
     def name(self):
         """Return the name."""
-        return f"{self._name} {SENSOR_TYPES[self.kind][ATTR_LABEL]}"
+        return self._name
 
     @property
     def state(self):
         """Return the state."""
-        return self._state
+        return self.coordinator.data.get(self.kind)
 
     @property
     def device_state_attributes(self):
@@ -98,8 +99,10 @@ class BrotherPrinterSensor(Entity):
             remaining_pages = ATTR_YELLOW_DRUM_REMAINING_PAGES
             drum_counter = ATTR_YELLOW_DRUM_COUNTER
         if remaining_pages and drum_counter:
-            self._attrs[ATTR_REMAINING_PAGES] = self.printer.data.get(remaining_pages)
+            self._attrs[ATTR_REMAINING_PAGES] = self.coordinator.data.get(
-            self._attrs[ATTR_COUNTER] = self.printer.data.get(drum_counter)
+                remaining_pages
+            )
+            self._attrs[ATTR_COUNTER] = self.coordinator.data.get(drum_counter)
         return self._attrs
 
     @property
@@ -120,15 +123,27 @@ class BrotherPrinterSensor(Entity):
     @property
     def available(self):
         """Return True if entity is available."""
-        return self.printer.available
+        return self.coordinator.last_update_success
+
+    @property
+    def should_poll(self):
+        """Return the polling requirement of the entity."""
+        return False
 
     @property
     def device_info(self):
         """Return the device info."""
         return self._device_info
 
-    async def async_update(self):
+    @property
-        """Update the data from printer."""
+    def entity_registry_enabled_default(self):
-        await self.printer.async_update()
+        """Return if the entity should be enabled when first added to the entity registry."""
+        return True
 
-        self._state = self.printer.data.get(self.kind)
+    async def async_added_to_hass(self):
+        """Connect to dispatcher listening for entity data notifications."""
+        self.coordinator.async_add_listener(self.async_write_ha_state)
+
+    async def async_will_remove_from_hass(self):
+        """Disconnect from update signal."""
+        self.coordinator.async_remove_listener(self.async_write_ha_state)

tests/components/brother/test_init.py

@@ -0,0 +1,112 @@
+"""Test init of Brother integration."""
+from datetime import timedelta
+import json
+
+from asynctest import patch
+import pytest
+
+import homeassistant.components.brother as brother
+from homeassistant.components.brother.const import DOMAIN
+from homeassistant.const import CONF_HOST, CONF_TYPE, STATE_UNAVAILABLE
+from homeassistant.exceptions import ConfigEntryNotReady
+from homeassistant.util.dt import utcnow
+
+from tests.common import MockConfigEntry, async_fire_time_changed, load_fixture
+
+
+async def test_async_setup_entry(hass):
+    """Test a successful setup entry."""
+    entry = MockConfigEntry(
+        domain=DOMAIN,
+        title="HL-L2340DW 0123456789",
+        data={CONF_HOST: "localhost", CONF_TYPE: "laser"},
+    )
+    with patch(
+        "brother.Brother._get_data",
+        return_value=json.loads(load_fixture("brother_printer_data.json")),
+    ):
+        entry.add_to_hass(hass)
+        assert await hass.config_entries.async_setup(entry.entry_id)
+        await hass.async_block_till_done()
+
+        state = hass.states.get("sensor.hl_l2340dw_status")
+        assert state is not None
+        assert state.state != STATE_UNAVAILABLE
+        assert state.state == "waiting"
+
+
+async def test_config_not_ready(hass):
+    """Test for setup failure if connection to broker is missing."""
+    entry = MockConfigEntry(
+        domain=DOMAIN,
+        title="HL-L2340DW 0123456789",
+        data={CONF_HOST: "localhost", CONF_TYPE: "laser"},
+    )
+    with patch(
+        "brother.Brother._get_data", side_effect=ConnectionError()
+    ), pytest.raises(ConfigEntryNotReady):
+        await brother.async_setup_entry(hass, entry)
+
+
+async def test_unload_entry(hass):
+    """Test successful unload of entry."""
+    entry = MockConfigEntry(
+        domain=DOMAIN,
+        title="HL-L2340DW 0123456789",
+        data={CONF_HOST: "localhost", CONF_TYPE: "laser"},
+    )
+    with patch(
+        "brother.Brother._get_data",
+        return_value=json.loads(load_fixture("brother_printer_data.json")),
+    ):
+        entry.add_to_hass(hass)
+        assert await hass.config_entries.async_setup(entry.entry_id)
+        await hass.async_block_till_done()
+
+        assert hass.data[DOMAIN][entry.entry_id]
+
+        assert await hass.config_entries.async_unload(entry.entry_id)
+        assert not hass.data[DOMAIN]
+
+
+async def test_availability(hass):
+    """Ensure that we mark the entities unavailable correctly when device is offline."""
+    entry = MockConfigEntry(
+        domain=DOMAIN,
+        title="HL-L2340DW 0123456789",
+        data={CONF_HOST: "localhost", CONF_TYPE: "laser"},
+    )
+    with patch(
+        "brother.Brother._get_data",
+        return_value=json.loads(load_fixture("brother_printer_data.json")),
+    ):
+        entry.add_to_hass(hass)
+        assert await hass.config_entries.async_setup(entry.entry_id)
+        await hass.async_block_till_done()
+
+        state = hass.states.get("sensor.hl_l2340dw_status")
+        assert state is not None
+        assert state.state != STATE_UNAVAILABLE
+        assert state.state == "waiting"
+
+    future = utcnow() + timedelta(minutes=5)
+    with patch("brother.Brother._get_data", side_effect=ConnectionError()):
+        async_fire_time_changed(hass, future)
+        await hass.async_block_till_done()
+
+        state = hass.states.get("sensor.hl_l2340dw_status")
+        assert state is not None
+        assert state.state == STATE_UNAVAILABLE
+
+    future = utcnow() + timedelta(minutes=10)
+    with patch(
+        "brother.Brother._get_data",
+        return_value=json.loads(load_fixture("brother_printer_data.json")),
+    ):
+        async_fire_time_changed(hass, future)
+        await hass.async_block_till_done()
+
+        state = hass.states.get("sensor.hl_l2340dw_status")
+        assert state is not None
+        assert state.state != STATE_UNAVAILABLE
+        assert state.state == "waiting"

tests/fixtures/brother_printer_data.json

@@ -8,10 +8,24 @@
       "31010400000001",
       "6f010400001d4c",
       "81010400000050",
-      "8601040000000a"
+      "8601040000000a",
+      "7e01040000064b",
+      "7301040000064b",
+      "7401040000064b",
+      "7501040000064b",
+      "790104000023f0",
+      "7a0104000023f0",
+      "7b0104000023f0",
+      "800104000023f0"
     ],
     "1.3.6.1.4.1.2435.2.3.9.1.1.7.0": "MFG:Brother;CMD:PJL,HBP,URF;MDL:HL-L2340DW series;CLS:PRINTER;CID:Brother Laser Type1;URF:W8,CP1,IS4-1,MT1-3-4-5-8,OB10,PQ4,RS300-600,V1.3,DM1;",
-    "1.3.6.1.4.1.2435.2.3.9.4.2.1.5.5.11.0": ["82010400002b06"],
+    "1.3.6.1.4.1.2435.2.3.9.4.2.1.5.5.11.0": [
+      "82010400002b06",
+      "a4010400004005",
+      "a5010400004005",
+      "a6010400004005",
+      "a7010400004005"
+    ],
     "1.3.6.1.4.1.2435.2.3.9.4.2.1.5.5.1.0": "0123456789",
     "1.3.6.1.4.1.2435.2.3.9.4.2.1.5.4.5.2.0": "WAITING     "
   }