From fa98685b1ec368390bf6f07c57652bd4ec47f34c Mon Sep 17 00:00:00 2001 From: r01k Date: Wed, 7 Dec 2022 00:07:22 -0700 Subject: [PATCH] Refactor Fully Kiosk and add logging details (#83028) * - Refactor fully_kiosk/services.py with a less repetitive structure. - Log exception details in config_flow.py. * Log config_flow.py connection exception details * Appropriate logging level including stack trace. Co-authored-by: Martin Hjelmare * Finish setting appropriate logging level when recording stack trace. * Log unknown exception with stack trace Co-authored-by: Martin Hjelmare * test_config_flow.py now passes. * All pytests passing. Co-authored-by: Martin Hjelmare --- .../components/fully_kiosk/config_flow.py | 11 ++- .../components/fully_kiosk/services.py | 94 ++++++++++--------- 2 files changed, 60 insertions(+), 45 deletions(-) diff --git a/homeassistant/components/fully_kiosk/config_flow.py b/homeassistant/components/fully_kiosk/config_flow.py index 5257030ecf0..785948f5632 100644 --- a/homeassistant/components/fully_kiosk/config_flow.py +++ b/homeassistant/components/fully_kiosk/config_flow.py @@ -41,10 +41,15 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): try: async with timeout(15): device_info = await fully.getDeviceInfo() - except (ClientConnectorError, FullyKioskError, asyncio.TimeoutError): + except ( + ClientConnectorError, + FullyKioskError, + asyncio.TimeoutError, + ) as error: + LOGGER.debug(error.args, exc_info=True) errors["base"] = "cannot_connect" - except Exception: # pylint: disable=broad-except - LOGGER.exception("Unexpected exception") + except Exception as error: # pylint: disable=broad-except + LOGGER.exception("Unexpected exception: %s", error) errors["base"] = "unknown" else: await self.async_set_unique_id(device_info["deviceID"]) diff --git a/homeassistant/components/fully_kiosk/services.py b/homeassistant/components/fully_kiosk/services.py index 2283904dfa9..3d63bf4f23c 100644 --- a/homeassistant/components/fully_kiosk/services.py +++ b/homeassistant/components/fully_kiosk/services.py @@ -1,6 +1,10 @@ """Services for the Fully Kiosk Browser integration.""" from __future__ import annotations +from collections.abc import Callable +from typing import Any + +from fullykiosk import FullyKiosk import voluptuous as vol from homeassistant.const import ATTR_DEVICE_ID @@ -12,6 +16,7 @@ from .const import ( ATTR_APPLICATION, ATTR_URL, DOMAIN, + LOGGER, SERVICE_LOAD_URL, SERVICE_START_APPLICATION, ) @@ -20,54 +25,59 @@ from .const import ( async def async_setup_services(hass: HomeAssistant) -> None: """Set up the services for the Fully Kiosk Browser integration.""" - async def async_load_url(call: ServiceCall) -> None: - """Load a URL on the Fully Kiosk Browser.""" + async def execute_service( + call: ServiceCall, + fully_method: Callable, + *args: list[str], + **kwargs: dict[str, Any], + ) -> None: + """ + Execute a Fully service call. + + :param call: {ServiceCall} HA service call. + :param fully_method: {Callable} A method of the FullyKiosk class. + :param args: Arguments for fully_method. + :param kwargs: Key-word arguments for fully_method. + :return: None + """ + LOGGER.debug( + "Calling Fully service %s with args: %s, %s", ServiceCall, args, kwargs + ) registry = dr.async_get(hass) for target in call.data[ATTR_DEVICE_ID]: - device = registry.async_get(target) if device: coordinator = hass.data[DOMAIN][list(device.config_entries)[0]] - await coordinator.fully.loadUrl(call.data[ATTR_URL]) + # fully_method(coordinator.fully, *args, **kwargs) would make + # test_services.py fail. + await getattr(coordinator.fully, fully_method.__name__)(*args, **kwargs) + + async def async_load_url(call: ServiceCall) -> None: + """Load a URL on the Fully Kiosk Browser.""" + await execute_service(call, FullyKiosk.loadUrl, call.data[ATTR_URL]) async def async_start_app(call: ServiceCall) -> None: """Start an app on the device.""" - registry = dr.async_get(hass) - for target in call.data[ATTR_DEVICE_ID]: + await execute_service( + call, FullyKiosk.startApplication, call.data[ATTR_APPLICATION] + ) - device = registry.async_get(target) - if device: - coordinator = hass.data[DOMAIN][list(device.config_entries)[0]] - await coordinator.fully.startApplication(call.data[ATTR_APPLICATION]) - - hass.services.async_register( - DOMAIN, - SERVICE_LOAD_URL, - async_load_url, - schema=vol.Schema( - vol.All( - { - vol.Required(ATTR_DEVICE_ID): cv.ensure_list, - vol.Required( - ATTR_URL, - ): cv.string, - }, - ) - ), - ) - - hass.services.async_register( - DOMAIN, - SERVICE_START_APPLICATION, - async_start_app, - schema=vol.Schema( - vol.All( - { - vol.Required(ATTR_DEVICE_ID): cv.ensure_list, - vol.Required( - ATTR_APPLICATION, - ): cv.string, - }, - ) - ), - ) + # Register all the above services + service_mapping = [ + (async_load_url, SERVICE_LOAD_URL, ATTR_URL), + (async_start_app, SERVICE_START_APPLICATION, ATTR_APPLICATION), + ] + for service_handler, service_name, attrib in service_mapping: + hass.services.async_register( + DOMAIN, + service_name, + service_handler, + schema=vol.Schema( + vol.All( + { + vol.Required(ATTR_DEVICE_ID): cv.ensure_list, + vol.Required(attrib): cv.string, + } + ) + ), + )