From 98aa69fdafcee13db1d99e9083ac862c4b501f00 Mon Sep 17 00:00:00 2001 From: Marvin Wichmann Date: Sat, 29 Jan 2022 14:32:12 +0100 Subject: [PATCH] Fix KNX Expose for strings longer than 14 bytes (#63026) * Fix KNX Expose for too long strings * Fix tests * Catch exception and avoid error during config entry setup for exposures * Properly catch exceptions in knx expose * Fix pylint * Fix CI * Add test for conversion error --- homeassistant/components/knx/expose.py | 23 +++++++-- tests/components/knx/test_expose.py | 69 +++++++++++++++++++++++++- 2 files changed, 88 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/knx/expose.py b/homeassistant/components/knx/expose.py index 34949b8c0b8..0963ec3be43 100644 --- a/homeassistant/components/knx/expose.py +++ b/homeassistant/components/knx/expose.py @@ -2,10 +2,12 @@ from __future__ import annotations from collections.abc import Callable +import logging from xknx import XKNX from xknx.devices import DateTime, ExposeSensor -from xknx.dpt import DPTNumeric +from xknx.dpt import DPTNumeric, DPTString +from xknx.exceptions import ConversionError from xknx.remote_value import RemoteValueSensor from homeassistant.const import ( @@ -22,6 +24,8 @@ from homeassistant.helpers.typing import ConfigType, StateType from .const import KNX_ADDRESS from .schema import ExposeSchema +_LOGGER = logging.getLogger(__name__) + @callback def create_knx_exposure( @@ -101,7 +105,10 @@ class KNXExposeSensor: """Initialize state of the exposure.""" init_state = self.hass.states.get(self.entity_id) state_value = self._get_expose_value(init_state) - self.device.sensor_value.value = state_value + try: + self.device.sensor_value.value = state_value + except ConversionError: + _LOGGER.exception("Error during sending of expose sensor value") @callback def shutdown(self) -> None: @@ -132,6 +139,13 @@ class KNXExposeSensor: and issubclass(self.device.sensor_value.dpt_class, DPTNumeric) ): return float(value) + if ( + value is not None + and isinstance(self.device.sensor_value, RemoteValueSensor) + and issubclass(self.device.sensor_value.dpt_class, DPTString) + ): + # DPT 16.000 only allows up to 14 Bytes + return str(value)[:14] return value async def _async_entity_changed(self, event: Event) -> None: @@ -148,7 +162,10 @@ class KNXExposeSensor: async def _async_set_knx_value(self, value: StateType) -> None: """Set new value on xknx ExposeSensor.""" - await self.device.set(value) + try: + await self.device.set(value) + except ConversionError: + _LOGGER.exception("Error during sending of expose sensor value") class KNXExposeTime: diff --git a/tests/components/knx/test_expose.py b/tests/components/knx/test_expose.py index cbe174127c4..e5030eef461 100644 --- a/tests/components/knx/test_expose.py +++ b/tests/components/knx/test_expose.py @@ -128,6 +128,73 @@ async def test_expose_attribute_with_default(hass: HomeAssistant, knx: KNXTestKi await knx.assert_write("1/1/8", (0,)) +async def test_expose_string(hass: HomeAssistant, knx: KNXTestKit): + """Test an expose to send string values of up to 14 bytes only.""" + + entity_id = "fake.entity" + attribute = "fake_attribute" + await knx.setup_integration( + { + CONF_KNX_EXPOSE: { + CONF_TYPE: "string", + KNX_ADDRESS: "1/1/8", + CONF_ENTITY_ID: entity_id, + CONF_ATTRIBUTE: attribute, + ExposeSchema.CONF_KNX_EXPOSE_DEFAULT: "Test", + } + }, + ) + assert not hass.states.async_all() + + # Before init default value shall be sent as response + await knx.receive_read("1/1/8") + await knx.assert_response( + "1/1/8", (84, 101, 115, 116, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) + ) + + # Change attribute; keep state + hass.states.async_set( + entity_id, + "on", + {attribute: "This is a very long string that is larger than 14 bytes"}, + ) + await knx.assert_write( + "1/1/8", (84, 104, 105, 115, 32, 105, 115, 32, 97, 32, 118, 101, 114, 121) + ) + + +async def test_expose_conversion_exception(hass: HomeAssistant, knx: KNXTestKit): + """Test expose throws exception.""" + + entity_id = "fake.entity" + attribute = "fake_attribute" + await knx.setup_integration( + { + CONF_KNX_EXPOSE: { + CONF_TYPE: "percent", + KNX_ADDRESS: "1/1/8", + CONF_ENTITY_ID: entity_id, + CONF_ATTRIBUTE: attribute, + ExposeSchema.CONF_KNX_EXPOSE_DEFAULT: 1, + } + }, + ) + assert not hass.states.async_all() + + # Before init default value shall be sent as response + await knx.receive_read("1/1/8") + await knx.assert_response("1/1/8", (3,)) + + # Change attribute: Expect no exception + hass.states.async_set( + entity_id, + "on", + {attribute: 101}, + ) + + await knx.assert_no_telegram() + + @patch("time.localtime") async def test_expose_with_date(localtime, hass: HomeAssistant, knx: KNXTestKit): """Test an expose with a date.""" @@ -138,7 +205,7 @@ async def test_expose_with_date(localtime, hass: HomeAssistant, knx: KNXTestKit) CONF_TYPE: "datetime", KNX_ADDRESS: "1/1/8", } - }, + } ) assert not hass.states.async_all()