Refactor (#223)

* Typing * WIP * Combine and short circuit multiple if's * Remove unused code in sensor * Move to minimum HA 2023.8.0 * Lint issues & warnings * Tidy up file operations * Fix config flow after change to library updater * Use extend method rather than loop * Update readme * Tidy up sensors * Fix entity id's getting translated where system language not english * Versino bump
andrew-codechimp · Jan 3, 2024 · 88e17f8 · 88e17f8
1 parent 8f73a1c
commit 88e17f8
Show file tree

Hide file tree

Showing 12 changed files with 207 additions and 259 deletions.
diff --git a/README.md b/README.md
@@ -118,11 +118,12 @@ To add a device definition to the battery library so that it will be automatical
 
 ### Submit Definition via GitHub Issues Form
 
-To add a new device via GitHub Issues, fill out [this form (BETA)](https://github.com/andrew-codechimp/HA-Battery-Notes/issues/new?template=new_device_request.yml&title=[Device]%3A+).
+To add a new device via GitHub Issues, fill out [this form](https://github.com/andrew-codechimp/HA-Battery-Notes/issues/new?template=new_device_request.yml&title=[Device]%3A+).
 Upon submission of the issue, GitHub will attempt to make the required code changes automatically.
 
 ### Submit Definition via Pull Request
-
+<details>
+<summary>More Details</summary>
 If you have issues with the form, or if you feel more comfortable editing JSON data, you can directly add definitions to [the library.json file](custom_components/battery_notes/data/library.json).
 Fork the repository, add your device details to the JSON document `custom_components/battery_notes/data/library.json`, and then submit a pull request.
 Do not enable GitHub Actions (disabled by default) on your fork as this will mess with the pull request and they are unnecessary for a library submission.
@@ -148,6 +149,7 @@ For the example image below, your JSON entry will look like this:
 
 ![Device Details](https://github.com/andrew-codechimp/HA-Battery-Notes/blob/main/images/screenshot-device-info.png "Device Details")
 <!---->
+</details>
 
 ## Contributions are welcome!
 

diff --git a/custom_components/battery_notes/__init__.py b/custom_components/battery_notes/__init__.py
@@ -15,14 +15,13 @@
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.core import HomeAssistant, callback
 from homeassistant.const import __version__ as HA_VERSION  # noqa: N812
-from homeassistant.helpers.aiohttp_client import async_get_clientsession
 from homeassistant.helpers.typing import ConfigType
 from homeassistant.helpers import device_registry as dr
+from homeassistant.util import dt as dt_util
 
 from .discovery import DiscoveryManager
-from .library_coordinator import BatteryNotesLibraryUpdateCoordinator
 from .library_updater import (
-    LibraryUpdaterClient,
+    LibraryUpdater,
 )
 from .coordinator import BatteryNotesCoordinator
 from .store import (
@@ -35,7 +34,7 @@
     PLATFORMS,
     CONF_ENABLE_AUTODISCOVERY,
     CONF_USER_LIBRARY,
-    DATA_UPDATE_COORDINATOR,
+    DATA_LIBRARY_UPDATER,
     CONF_SHOW_ALL_DEVICES,
     CONF_ENABLE_REPLACED,
     SERVICE_BATTERY_REPLACED,
@@ -93,12 +92,11 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
     coordinator = BatteryNotesCoordinator(hass, store)
     hass.data[DOMAIN][DATA_COORDINATOR] = coordinator
 
-    library_coordinator = BatteryNotesLibraryUpdateCoordinator(
-        hass=hass,
-        client=LibraryUpdaterClient(session=async_get_clientsession(hass)),
-    )
+    library_updater = LibraryUpdater(hass)
+
+    await library_updater.get_library_updates(dt_util.utcnow())
 
-    hass.data[DOMAIN][DATA_UPDATE_COORDINATOR] = library_coordinator
+    hass.data[DOMAIN][DATA_LIBRARY_UPDATER] = library_updater
 
     await coordinator.async_refresh()
 
@@ -110,7 +108,6 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
 
     return True
 
-
 async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     """Set up a config entry."""
 
@@ -132,7 +129,7 @@ async def async_remove_entry(hass: HomeAssistant, config_entry: ConfigEntry) ->
 
     device_id = config_entry.data["device_id"]
 
-    coordinator = hass.data[DOMAIN][DATA_COORDINATOR]
+    coordinator: BatteryNotesCoordinator = hass.data[DOMAIN][DATA_COORDINATOR]
     data = {ATTR_REMOVE: True}
 
     coordinator.async_update_device_config(device_id=device_id, data=data)
@@ -171,14 +168,13 @@ async def handle_battery_replaced(call):
             ) and entry.domain == DOMAIN:
                 date_replaced = datetime.utcnow()
 
-                coordinator = hass.data[DOMAIN][DATA_COORDINATOR]
+                coordinator: BatteryNotesCoordinator = hass.data[DOMAIN][DATA_COORDINATOR]
                 device_entry = {"battery_last_replaced": date_replaced}
 
                 coordinator.async_update_device_config(
                     device_id=device_id, data=device_entry
                 )
 
-                await coordinator._async_update_data()
                 await coordinator.async_request_refresh()
 
                 _LOGGER.debug(

diff --git a/custom_components/battery_notes/button.py b/custom_components/battery_notes/button.py
@@ -41,6 +41,8 @@
     CONF_ENABLE_REPLACED,
 )
 
+from .coordinator import BatteryNotesCoordinator
+
 from .entity import (
     BatteryNotesEntityDescription,
 )
@@ -56,17 +58,6 @@ class BatteryNotesButtonEntityDescription(
     unique_id_suffix: str
 
 
-ENTITY_DESCRIPTIONS: tuple[BatteryNotesButtonEntityDescription, ...] = (
-    BatteryNotesButtonEntityDescription(
-        unique_id_suffix="_battery_replaced_button",
-        key="battery_replaced",
-        translation_key="battery_replaced",
-        icon="mdi:battery-sync",
-        entity_category=EntityCategory.DIAGNOSTIC,
-        entity_registry_enabled_default = False,
-    ),
-)
-
 PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend(
     {vol.Optional(CONF_NAME): cv.string, vol.Required(CONF_DEVICE_ID): cv.string}
 )
@@ -132,7 +123,7 @@ async def async_registry_updated(event: Event) -> None:
 
     enable_replaced = True
     if DOMAIN_CONFIG in hass.data[DOMAIN]:
-        domain_config = hass.data[DOMAIN][DOMAIN_CONFIG]
+        domain_config: dict = hass.data[DOMAIN][DOMAIN_CONFIG]
         enable_replaced = domain_config.get(CONF_ENABLE_REPLACED, True)
 
     description = BatteryNotesButtonEntityDescription(
@@ -141,27 +132,29 @@ async def async_registry_updated(event: Event) -> None:
         translation_key="battery_replaced",
         icon="mdi:battery-sync",
         entity_category=EntityCategory.DIAGNOSTIC,
-        entity_registry_enabled_default = enable_replaced,
+        entity_registry_enabled_default=enable_replaced,
     )
 
     async_add_entities(
         [
             BatteryNotesButton(
-            hass,
-            description,
-            f"{config_entry.entry_id}{description.unique_id_suffix}",
-            device_id,
+                hass,
+                description,
+                f"{config_entry.entry_id}{description.unique_id_suffix}",
+                device_id,
             )
         ]
     )
 
+
 async def async_setup_platform(
     hass: HomeAssistant,
 ) -> None:
     """Set up the battery note sensor."""
 
     await async_setup_reload_service(hass, DOMAIN, PLATFORMS)
 
+
 class BatteryNotesButton(ButtonEntity):
     """Represents a battery replaced button."""
 
@@ -190,11 +183,12 @@ def __init__(
                 identifiers=device.identifiers,
             )
 
+            self.entity_id = f"button.{device.name}_{description.key}"
+
     async def async_added_to_hass(self) -> None:
         """Handle added to Hass."""
         registry = er.async_get(self.hass)
         if registry.async_get(self.entity_id) is not None:
-
             registry.async_update_entity_options(
                 self.entity_id,
                 DOMAIN,
@@ -207,8 +201,6 @@ async def async_press(self) -> None:
 
         device_entry = {"battery_last_replaced": datetime.utcnow()}
 
-        coordinator = self.hass.data[DOMAIN][DATA_COORDINATOR]
+        coordinator: BatteryNotesCoordinator = self.hass.data[DOMAIN][DATA_COORDINATOR]
         coordinator.async_update_device_config(device_id=device_id, data=device_entry)
-        await coordinator._async_update_data()
         await coordinator.async_request_refresh()
-
diff --git a/custom_components/battery_notes/config_flow.py b/custom_components/battery_notes/config_flow.py
@@ -16,21 +16,23 @@
 from homeassistant.const import Platform
 from homeassistant.components.sensor import SensorDeviceClass
 import homeassistant.helpers.device_registry as dr
+from homeassistant.util import dt as dt_util
 
 from homeassistant.const import (
     CONF_NAME,
     CONF_DEVICE_ID,
 )
 
 from .library import Library
+from .library_updater import LibraryUpdater
 
 from .const import (
     DOMAIN,
     CONF_BATTERY_TYPE,
     CONF_DEVICE_NAME,
     CONF_MANUFACTURER,
     CONF_MODEL,
-    DATA_UPDATE_COORDINATOR,
+    DATA_LIBRARY_UPDATER,
     DOMAIN_CONFIG,
     CONF_SHOW_ALL_DEVICES,
 )
@@ -114,10 +116,13 @@ async def async_step_user(
 
             device_id = user_input[CONF_DEVICE_ID]
 
-            if DOMAIN in self.hass.data:
-                if DATA_UPDATE_COORDINATOR in self.hass.data[DOMAIN]:
-                    coordinator = self.hass.data[DOMAIN][DATA_UPDATE_COORDINATOR]
-                    await coordinator.async_refresh()
+            if (
+                DOMAIN in self.hass.data
+                and DATA_LIBRARY_UPDATER in self.hass.data[DOMAIN]
+            ):
+                library_updater: LibraryUpdater  = self.hass.data[DOMAIN][DATA_LIBRARY_UPDATER]
+                await library_updater.get_library_updates(dt_util.utcnow())
+
 
             device_registry = dr.async_get(self.hass)
             device_entry = device_registry.async_get(device_id)
@@ -132,24 +137,28 @@ async def async_step_user(
                 device_entry.manufacturer, device_entry.model
             )
 
-            if device_battery_details:
-                if not device_battery_details.is_manual:
-                    _LOGGER.debug(
-                        "Found device %s %s", device_entry.manufacturer, device_entry.model
-                    )
-                    self.data[
-                        CONF_BATTERY_TYPE
-                    ] = device_battery_details.battery_type_and_quantity
+            if (
+                device_battery_details
+                and not  device_battery_details.is_manual
+            ):
+                _LOGGER.debug(
+                    "Found device %s %s", device_entry.manufacturer, device_entry.model
+                )
+                self.data[
+                    CONF_BATTERY_TYPE
+                ] = device_battery_details.battery_type_and_quantity
 
             return await self.async_step_battery()
 
         schema = DEVICE_SCHEMA
         # If show_all_devices = is specified and true, don't filter
-        if DOMAIN in self.hass.data:
-            if DOMAIN_CONFIG in self.hass.data[DOMAIN]:
-                domain_config = self.hass.data[DOMAIN][DOMAIN_CONFIG]
-                if domain_config.get(CONF_SHOW_ALL_DEVICES, False):
-                    schema = DEVICE_SCHEMA_ALL
+        if (
+            DOMAIN in self.hass.data
+            and DOMAIN_CONFIG in self.hass.data[DOMAIN]
+            ):
+            domain_config: dict = self.hass.data[DOMAIN][DOMAIN_CONFIG]
+            if domain_config.get(CONF_SHOW_ALL_DEVICES, False):
+                schema = DEVICE_SCHEMA_ALL
 
         return self.async_show_form(
             step_id="user",

diff --git a/custom_components/battery_notes/const.py b/custom_components/battery_notes/const.py
@@ -39,7 +39,7 @@
 DATA_DISCOVERED_ENTITIES = "discovered_entities"
 DATA_DOMAIN_ENTITIES = "domain_entities"
 DATA_LIBRARY = "library"
-DATA_UPDATE_COORDINATOR = "update_coordinator"
+DATA_LIBRARY_UPDATER = "library_updater"
 DATA_LIBRARY_LAST_UPDATE = "library_last_update"
 DATA_COORDINATOR = "coordinator"
 DATA_STORE = "store"

diff --git a/custom_components/battery_notes/library.py b/custom_components/battery_notes/library.py
@@ -29,27 +29,29 @@ def __init__(self, hass: HomeAssistant) -> None:
         """Init."""
 
         # User Library
-        if DOMAIN_CONFIG in hass.data[DOMAIN]:
-            if CONF_USER_LIBRARY in hass.data[DOMAIN][DOMAIN_CONFIG]:
-                user_library_filename = hass.data[DOMAIN][DOMAIN_CONFIG].get(CONF_USER_LIBRARY)
-                if  user_library_filename != "":
-                    json_user_path = os.path.join(
-                        BUILT_IN_DATA_DIRECTORY,
-                        user_library_filename,
+        if (
+            DOMAIN_CONFIG in hass.data[DOMAIN]
+            and CONF_USER_LIBRARY in hass.data[DOMAIN][DOMAIN_CONFIG]
+            ):
+            user_library_filename = hass.data[DOMAIN][DOMAIN_CONFIG].get(CONF_USER_LIBRARY)
+            if  user_library_filename != "":
+                json_user_path = os.path.join(
+                    BUILT_IN_DATA_DIRECTORY,
+                    user_library_filename,
+                )
+                _LOGGER.debug("Using user library file at %s", json_user_path)
+
+                try:
+                    with open(json_user_path, encoding="utf-8") as user_file:
+                        user_json_data = json.load(user_file)
+                        self._devices = user_json_data["devices"]
+                        user_file.close()
+
+                except FileNotFoundError:
+                    _LOGGER.error(
+                        "User library file not found at %s",
+                        json_user_path,
                     )
-                    _LOGGER.debug("Using user library file at %s", json_user_path)
-
-                    try:
-                        with open(json_user_path, encoding="utf-8") as user_file:
-                            user_json_data = json.load(user_file)
-                            self._devices = user_json_data["devices"]
-                            user_file.close()
-
-                    except FileNotFoundError:
-                        _LOGGER.error(
-                            "User library file not found at %s",
-                            json_user_path,
-                        )
 
         # Default Library
         json_default_path = os.path.join(
@@ -61,8 +63,7 @@ def __init__(self, hass: HomeAssistant) -> None:
         try:
             with open(json_default_path, encoding="utf-8") as default_file:
                 default_json_data = json.load(default_file)
-                for i in default_json_data["devices"]:
-                    self._devices.append(i)
+                self._devices.extend(default_json_data["devices"])
                 default_file.close()
 
         except FileNotFoundError: