From ab010505b08d25462d08da2fc5e23dfa8f3c61e6 Mon Sep 17 00:00:00 2001 From: Michael Bisbjerg Date: Thu, 7 May 2026 18:22:43 +0200 Subject: [PATCH 1/2] ux: improve BLE discovery confirmation copy --- custom_components/opendisplay/config_flow.py | 76 ++++++++++--------- custom_components/opendisplay/strings.json | 4 +- .../opendisplay/translations/en.json | 4 +- 3 files changed, 46 insertions(+), 38 deletions(-) diff --git a/custom_components/opendisplay/config_flow.py b/custom_components/opendisplay/config_flow.py index 1c1ef9c..9c73192 100644 --- a/custom_components/opendisplay/config_flow.py +++ b/custom_components/opendisplay/config_flow.py @@ -39,6 +39,15 @@ ) +def _format_ble_protocol_label(protocol_type: str) -> str: + """Return a user-facing label for a BLE protocol.""" + if protocol_type == "open_display": + return "OpenDisplay (OD)" + if protocol_type == "atc": + return "OEPL / ATC" + return protocol_type + + class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): """Handle a config flow for OpenDisplay. @@ -59,6 +68,34 @@ def __init__(self) -> None: self._discovered_device: dict[str, Any] | None = {} self._dhcp_discovery_info: DhcpServiceInfo | None = None + def _bluetooth_description_placeholders( + self, + error: str | None = None, + ) -> dict[str, str]: + """Build placeholders for the Bluetooth confirmation dialog.""" + device = self._discovered_device + advertised_details = "" + if device["protocol_type"] == "atc": + battery = f"{device['battery_mv']/1000:.2f}V" if device["battery_mv"] > 0 else "Unknown" + fw_version = str(device["fw_version"]) if device["fw_version"] > 0 else "Unknown" + config_version = str(device["version"]) if device["version"] > 0 else "Unknown" + advertised_details = ( + f"\n- Battery: {battery}" + f"\n- Firmware: {fw_version}" + f"\n- Config Version: {config_version}" + ) + + placeholders = { + "name": device["name"], + "device_type": device["protocol_display"], + "address": device["address"], + "rssi": str(device["rssi"]), + "advertised_details": advertised_details, + } + if error is not None: + placeholders["error"] = error + return placeholders + async def _validate_input(self, host: str) -> tuple[dict[str, str], str | None]: """Validate the user input allows us to connect. @@ -226,6 +263,7 @@ async def async_step_bluetooth( "fw_version": advertising_data.fw_version, "version": advertising_data.version, "protocol_type": protocol.protocol_name, # Store protocol type + "protocol_display": _format_ble_protocol_label(protocol.protocol_name), } _LOGGER.debug("Discovered device info: %s", self._discovered_device) @@ -391,14 +429,7 @@ async def async_step_bluetooth_confirm( return self.async_show_form( step_id="bluetooth_confirm", errors={"base": "invalid_device_config"}, - description_placeholders={ - "name": self._discovered_device["name"], - "address": self._discovered_device["address"], - "rssi": str(self._discovered_device["rssi"]), - "battery": f"{self._discovered_device['battery_mv']/1000:.2f}V" if self._discovered_device["battery_mv"] > 0 else "Unknown", - "fw_version": str(self._discovered_device["fw_version"]) if self._discovered_device["fw_version"] > 0 else "Unknown", - "config_version": str(self._discovered_device["version"]) if self._discovered_device["version"] > 0 else "Unknown", - }, + description_placeholders=self._bluetooth_description_placeholders(), ) except (BLEConnectionError, BLEProtocolError) as e: @@ -406,15 +437,7 @@ async def async_step_bluetooth_confirm( return self.async_show_form( step_id="bluetooth_confirm", errors={"base": "interrogation_failed"}, - description_placeholders={ - "name": self._discovered_device["name"], - "address": self._discovered_device["address"], - "rssi": str(self._discovered_device["rssi"]), - "battery": f"{self._discovered_device['battery_mv']/1000:.2f}V" if self._discovered_device["battery_mv"] > 0 else "Unknown", - "fw_version": str(self._discovered_device["fw_version"]) if self._discovered_device["fw_version"] > 0 else "Unknown", - "config_version": str(self._discovered_device["version"]) if self._discovered_device["version"] > 0 else "Unknown", - "error": str(e), - }, + description_placeholders=self._bluetooth_description_placeholders(str(e)), ) except Exception as e: @@ -422,26 +445,11 @@ async def async_step_bluetooth_confirm( return self.async_show_form( step_id="bluetooth_confirm", errors={"base": "interrogation_failed"}, - description_placeholders={ - "name": self._discovered_device["name"], - "address": self._discovered_device["address"], - "rssi": str(self._discovered_device["rssi"]), - "battery": f"{self._discovered_device['battery_mv']/1000:.2f}V" if self._discovered_device["battery_mv"] > 0 else "Unknown", - "fw_version": str(self._discovered_device["fw_version"]) if self._discovered_device["fw_version"] > 0 else "Unknown", - "config_version": str(self._discovered_device["version"]) if self._discovered_device["version"] > 0 else "Unknown", - "error": str(e), - }, + description_placeholders=self._bluetooth_description_placeholders(str(e)), ) # Build description placeholders from advertising data - description_placeholders = { - "name": self._discovered_device["name"], - "address": self._discovered_device["address"], - "rssi": str(self._discovered_device["rssi"]), - "battery": f"{self._discovered_device['battery_mv']/1000:.2f}V" if self._discovered_device["battery_mv"] > 0 else "Unknown", - "fw_version": str(self._discovered_device["fw_version"]) if self._discovered_device["fw_version"] > 0 else "Unknown", - "config_version": str(self._discovered_device["version"]) if self._discovered_device["version"] > 0 else "Unknown", - } + description_placeholders = self._bluetooth_description_placeholders() return self.async_show_form( step_id="bluetooth_confirm", diff --git a/custom_components/opendisplay/strings.json b/custom_components/opendisplay/strings.json index 7a974b6..33e987b 100644 --- a/custom_components/opendisplay/strings.json +++ b/custom_components/opendisplay/strings.json @@ -2,8 +2,8 @@ "config": { "step": { "bluetooth_confirm": { - "title": "Set up BLE Tag", - "description": "Set up **{name}**?\n\n**Details:**\n- Address: {address}\n- Signal: {rssi} dBm\n- Battery: {battery}\n- Firmware: {fw_version}\n- Config Version: {config_version}" + "title": "Set up OpenDisplay Device", + "description": "Set up **{name}**?\n\n**Details:**\n- Device Type: {device_type}\n- Address: {address}\n- Signal: {rssi} dBm{advertised_details}" }, "dhcp_confirm": { "title": "Set up {hostname} ({ip})", diff --git a/custom_components/opendisplay/translations/en.json b/custom_components/opendisplay/translations/en.json index ea49cbb..b172e82 100644 --- a/custom_components/opendisplay/translations/en.json +++ b/custom_components/opendisplay/translations/en.json @@ -2,8 +2,8 @@ "config": { "step": { "bluetooth_confirm": { - "title": "Set up BLE Tag", - "description": "Set up **{name}**?\n\n**Details:**\n- Address: {address}\n- Signal: {rssi} dBm\n- Battery: {battery}\n- Firmware: {fw_version}\n- Config Version: {config_version}" + "title": "Set up OpenDisplay Device", + "description": "Set up **{name}**?\n\n**Details:**\n- Device Type: {device_type}\n- Address: {address}\n- Signal: {rssi} dBm{advertised_details}" }, "dhcp_confirm": { "title": "Set up {hostname} ({ip})", From d638eb3de6d1abed3b3de5861bb39a988fd779b7 Mon Sep 17 00:00:00 2001 From: Michael Bisbjerg Date: Thu, 7 May 2026 19:54:28 +0200 Subject: [PATCH 2/2] cleanup: address low-risk review findings --- custom_components/opendisplay/coordinator.py | 4 +--- custom_components/opendisplay/entity.py | 16 ++++++++++++++-- custom_components/opendisplay/image.py | 2 -- custom_components/opendisplay/services.py | 14 +++++--------- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/custom_components/opendisplay/coordinator.py b/custom_components/opendisplay/coordinator.py index f9f4701..a83408d 100644 --- a/custom_components/opendisplay/coordinator.py +++ b/custom_components/opendisplay/coordinator.py @@ -10,7 +10,7 @@ import websockets from homeassistant.config_entries import ConfigEntry from homeassistant.const import EVENT_HOMEASSISTANT_STOP -from homeassistant.core import HomeAssistant, CALLBACK_TYPE, callback +from homeassistant.core import HomeAssistant, CALLBACK_TYPE from homeassistant.exceptions import ConfigEntryNotReady, HomeAssistantError from homeassistant.helpers.aiohttp_client import async_get_clientsession from homeassistant.helpers.dispatcher import async_dispatcher_send @@ -458,7 +458,6 @@ async def delayed_reconnect(): except Exception as err: _LOGGER.exception("Error handling message: %s", err) - @callback async def _handle_system_message(self, sys_data: dict) -> None: """Process a system message from the AP. @@ -504,7 +503,6 @@ async def _handle_system_message(self, sys_data: dict) -> None: async_dispatcher_send(self.hass, SIGNAL_AP_UPDATE) - @callback async def _handle_tag_message(self, tag_data: dict) -> None: """Process a tag update message from the AP. diff --git a/custom_components/opendisplay/entity.py b/custom_components/opendisplay/entity.py index 1fb122f..828b1ea 100644 --- a/custom_components/opendisplay/entity.py +++ b/custom_components/opendisplay/entity.py @@ -16,6 +16,18 @@ from .runtime_data import OpenDisplayConfigEntry +def _format_tag_firmware_version(firmware_version) -> str: + """Format AP tag firmware versions without raising on malformed data.""" + if firmware_version in (None, ""): + return "Unknown" + + firmware_version_str = str(firmware_version) + try: + return f"0x{int(firmware_version_str, 16):X}" + except ValueError: + return firmware_version_str + + class OpenDisplayAPEntity(Entity): """ Base entity for AP-level entities (switch, select, text, AP sensors). @@ -103,7 +115,7 @@ def device_info(self) -> DeviceInfo: hw_type = tag_data.get("hw_type", 0) hw_string = get_hw_string(hw_type) width, height = get_hw_dimensions(hw_type) - firmware_version = str(tag_data.get("version", "")) + firmware_version = tag_data.get("version") return DeviceInfo( identifiers={(DOMAIN, self._tag_mac)}, @@ -111,7 +123,7 @@ def device_info(self) -> DeviceInfo: manufacturer="OpenEPaperLink", model=hw_string, via_device=(DOMAIN, "ap"), - sw_version=f"0x{int(firmware_version, 16):X}" if firmware_version else "Unknown", + sw_version=_format_tag_firmware_version(firmware_version), serial_number=self._tag_mac, hw_version=f"{width}x{height}", ) diff --git a/custom_components/opendisplay/image.py b/custom_components/opendisplay/image.py index 3ef5175..776b9f8 100644 --- a/custom_components/opendisplay/image.py +++ b/custom_components/opendisplay/image.py @@ -236,10 +236,8 @@ def _handle_tag_image_update(self, data) -> None: if isinstance(data, bytes): self._cached_image = data self._last_updated = datetime.now() - self.async_write_ha_state() elif data: self.hass.async_create_task(self._refresh_image()) - self.async_write_ha_state() self.async_write_ha_state() diff --git a/custom_components/opendisplay/services.py b/custom_components/opendisplay/services.py index 8225b1b..baf9446 100644 --- a/custom_components/opendisplay/services.py +++ b/custom_components/opendisplay/services.py @@ -74,8 +74,11 @@ async def get_entity_id_from_device_id(device_id: str) -> str: translation_placeholders={"device_id": device_id}, ) - domain_mac = next(iter(device.identifiers)) - if domain_mac[0] != DOMAIN: + domain_mac = next( + (identifier for identifier in device.identifiers if identifier[0] == DOMAIN), + None, + ) + if domain_mac is None: raise ServiceValidationError( translation_domain=DOMAIN, translation_key="device_not_opendisplay", @@ -267,13 +270,6 @@ async def drawcustom_service(service: ServiceCall, entity_id: str) -> None: translation_placeholders={"errors": errors_str}, ) - if device_errors: - _LOGGER.warning( - "Completed with warnings for device %s:\n%s", - entity_id, - "\n".join(device_errors) - ) - # Handle dry-run mode if service.data.get("dry-run", False): _LOGGER.info("Dry run completed for %s", entity_id)