From c13fdd23c1d221cd887422e6b71df1419a7e6e8e Mon Sep 17 00:00:00 2001 From: Will W Date: Sun, 2 Jul 2017 05:30:39 +0900 Subject: [PATCH] components.knx - KNXMultiAddressDevice corrections (#8275) 1. The has_attributes was comparing names to addresses 2. Some errors outside of an except block were using _LOGGER.except. This will cause an exception itself because there is no trance context available to the logger 3. Added alias names for the address and state addresses so that they can be accessed with the same 4. Added return values for the set_int_value and set_percentage methods to allow error checking similar to the set_value method 5. Added the name of the configured object to the log messages to make them more meaningful (otherwise multiple similar log messages are received without any hint as to the target device) --- homeassistant/components/knx.py | 91 ++++++++++++++++++++++++++------- 1 file changed, 72 insertions(+), 19 deletions(-) diff --git a/homeassistant/components/knx.py b/homeassistant/components/knx.py index 10286f84b3a..9155304def7 100644 --- a/homeassistant/components/knx.py +++ b/homeassistant/components/knx.py @@ -51,7 +51,7 @@ def setup(hass, config): res = KNXTUNNEL.connect() _LOGGER.debug("Res = %s", res) if not res: - _LOGGER.exception("Could not connect to KNX/IP interface %s", host) + _LOGGER.error("Could not connect to KNX/IP interface %s", host) return False except KNXException as ex: @@ -127,7 +127,10 @@ class KNXGroupAddress(Entity): self._config = config self._state = False self._data = None - _LOGGER.debug("Initalizing KNX group address %s", self.address) + _LOGGER.debug( + "Initalizing KNX group address for %s (%s)", + self.name, self.address + ) def handle_knx_message(addr, data): """Handle an incoming KNX frame. @@ -198,11 +201,15 @@ class KNXGroupAddress(Entity): self._data = res else: _LOGGER.debug( - "Unable to read from KNX address: %s (None)", self.address) + "%s: unable to read from KNX address: %s (None)", + self.name, self.address + ) except KNXException: _LOGGER.exception( - "Unable to read from KNX address: %s", self.address) + "%s: unable to read from KNX address: %s", + self.name, self.address + ) return False @@ -229,20 +236,44 @@ class KNXMultiAddressDevice(Entity): self._config = config self._state = False self._data = None - _LOGGER.debug("Initalizing KNX multi address device") + _LOGGER.debug( + "%s: initalizing KNX multi address device", + self.name + ) settings = self._config.config + if config.address: + _LOGGER.debug( + "%s: base address: address=%s", + self.name, settings.get('address') + ) + self.names[config.address] = 'base' + if config.state_address: + _LOGGER.debug( + "%s, state address: state_address=%s", + self.name, settings.get('state_address') + ) + self.names[config.state_address] = 'state' + # parse required addresses for name in required: - _LOGGER.info(name) paramname = '{}{}'.format(name, '_address') addr = settings.get(paramname) if addr is None: - _LOGGER.exception( - "Required KNX group address %s missing", paramname) + _LOGGER.error( + "%s: Required KNX group address %s missing", + self.name, paramname + ) raise KNXException( - "Group address for %s missing in configuration", paramname) - _LOGGER.debug("%s: %s=%s", settings.get('name'), paramname, addr) + "%s: Group address for {} missing in " + "configuration for {}".format( + self.name, paramname + ) + ) + _LOGGER.debug( + "%s: (required parameter) %s=%s", + self.name, paramname, addr + ) addr = parse_group_address(addr) self.names[addr] = name @@ -250,12 +281,18 @@ class KNXMultiAddressDevice(Entity): for name in optional: paramname = '{}{}'.format(name, '_address') addr = settings.get(paramname) - _LOGGER.debug("%s: %s=%s", settings.get('name'), paramname, addr) + _LOGGER.debug( + "%s: (optional parameter) %s=%s", + self.name, paramname, addr + ) if addr: try: addr = parse_group_address(addr) except KNXException: - _LOGGER.exception("Cannot parse group address %s", addr) + _LOGGER.exception( + "%s: cannot parse group address %s", + self.name, addr + ) self.names[addr] = name @property @@ -283,7 +320,7 @@ class KNXMultiAddressDevice(Entity): This is mostly important for optional addresses. """ - for attributename, dummy_attribute in self.names.items(): + for attributename in self.names.values(): if attributename == name: return True return False @@ -296,7 +333,7 @@ class KNXMultiAddressDevice(Entity): percentage = abs(percentage) # only accept positive values scaled_value = percentage * 255 / 100 value = min(255, scaled_value) - self.set_int_value(name, value) + return self.set_int_value(name, value) def get_percentage(self, name): """Get a percentage from knx for a given attribute. @@ -312,7 +349,7 @@ class KNXMultiAddressDevice(Entity): # KNX packets are big endian value = round(value) # only accept integers b_value = value.to_bytes(num_bytes, byteorder='big') - self.set_value(name, list(b_value)) + return self.set_value(name, list(b_value)) def get_int_value(self, name): """Get an integer value for a given attribute.""" @@ -340,13 +377,21 @@ class KNXMultiAddressDevice(Entity): addr = attributeaddress if addr is None: - _LOGGER.exception("Attribute %s undefined", name) + _LOGGER.error("%s: attribute '%s' undefined", + self.name, name) + _LOGGER.debug( + "%s: defined attributes: %s", + self.name, str(self.names) + ) return False try: res = KNXTUNNEL.group_read(addr, use_cache=self.cache) except KNXException: - _LOGGER.exception("Unable to read from KNX address: %s", addr) + _LOGGER.exception( + "%s: unable to read from KNX address: %s", + self.name, addr + ) return False return res @@ -361,13 +406,21 @@ class KNXMultiAddressDevice(Entity): addr = attributeaddress if addr is None: - _LOGGER.exception("Attribute %s undefined", name) + _LOGGER.error("%s: attribute '%s' undefined", + self.name, name) + _LOGGER.debug( + "%s: defined attributes: %s", + self.name, str(self.names) + ) return False try: KNXTUNNEL.group_write(addr, value) except KNXException: - _LOGGER.exception("Unable to write to KNX address: %s", addr) + _LOGGER.exception( + "%s: unable to write to KNX address: %s", + self.name, addr + ) return False return True