From 9c5bbfe96d8f20b6221238847c26bd5bfbbf1774 Mon Sep 17 00:00:00 2001
From: Mattias Welponer <mattias@welponer.net>
Date: Wed, 18 Jul 2018 12:19:08 +0200
Subject: [PATCH] Cleanup of HomematicIP Cloud code (#15475)

* Check if device supports lowBat and shows it only if battery is low

* Show empty battery icon if lowBat is true

* Default return None

* Sabotage attribute and icon if device has this feature

* Bug fix and cleanup

* Use dedicated function for security state

* Cleanup of sensor attributes and icons

* Empty
---
 .../alarm_control_panel/homematicip_cloud.py  | 18 +++++++-----------
 .../components/homematicip_cloud/device.py    | 19 +++++++++++++++----
 .../components/sensor/homematicip_cloud.py    |  7 ++-----
 3 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/homeassistant/components/alarm_control_panel/homematicip_cloud.py b/homeassistant/components/alarm_control_panel/homematicip_cloud.py
index 893fa76c44b..43cb4494978 100644
--- a/homeassistant/components/alarm_control_panel/homematicip_cloud.py
+++ b/homeassistant/components/alarm_control_panel/homematicip_cloud.py
@@ -20,7 +20,6 @@ DEPENDENCIES = ['homematicip_cloud']
 
 _LOGGER = logging.getLogger(__name__)
 
-HMIP_OPEN = 'OPEN'
 HMIP_ZONE_AWAY = 'EXTERNAL'
 HMIP_ZONE_HOME = 'INTERNAL'
 
@@ -57,14 +56,18 @@ class HomematicipSecurityZone(HomematicipGenericDevice, AlarmControlPanel):
     @property
     def state(self):
         """Return the state of the device."""
+        from homematicip.base.enums import WindowState
+
         if self._device.active:
             if (self._device.sabotage or self._device.motionDetected or
-                    self._device.windowState == HMIP_OPEN):
+                    self._device.windowState == WindowState.OPEN):
                 return STATE_ALARM_TRIGGERED
 
-            if self._device.label == HMIP_ZONE_HOME:
+            active = self._home.get_security_zones_activation()
+            if active == (True, True):
+                return STATE_ALARM_ARMED_AWAY
+            elif active == (False, True):
                 return STATE_ALARM_ARMED_HOME
-            return STATE_ALARM_ARMED_AWAY
 
         return STATE_ALARM_DISARMED
 
@@ -79,10 +82,3 @@ class HomematicipSecurityZone(HomematicipGenericDevice, AlarmControlPanel):
     async def async_alarm_arm_away(self, code=None):
         """Send arm away command."""
         await self._home.set_security_zones_activation(True, True)
-
-    @property
-    def device_state_attributes(self):
-        """Return the state attributes of the alarm control device."""
-        # The base class is loading the battery property, but device doesn't
-        # have this property - base class needs clean-up.
-        return None
diff --git a/homeassistant/components/homematicip_cloud/device.py b/homeassistant/components/homematicip_cloud/device.py
index 94fe5f40be8..bb21e1df3d5 100644
--- a/homeassistant/components/homematicip_cloud/device.py
+++ b/homeassistant/components/homematicip_cloud/device.py
@@ -62,10 +62,21 @@ class HomematicipGenericDevice(Entity):
         """Device available."""
         return not self._device.unreach
 
+    @property
+    def icon(self):
+        """Return the icon."""
+        if hasattr(self._device, 'lowBat') and self._device.lowBat:
+            return 'mdi:battery-outline'
+        if hasattr(self._device, 'sabotage') and self._device.sabotage:
+            return 'mdi:alert'
+        return None
+
     @property
     def device_state_attributes(self):
         """Return the state attributes of the generic device."""
-        return {
-            ATTR_LOW_BATTERY: self._device.lowBat,
-            ATTR_MODEL_TYPE: self._device.modelType
-        }
+        attr = {ATTR_MODEL_TYPE: self._device.modelType}
+        if hasattr(self._device, 'lowBat') and self._device.lowBat:
+            attr.update({ATTR_LOW_BATTERY: self._device.lowBat})
+        if hasattr(self._device, 'sabotage') and self._device.sabotage:
+            attr.update({ATTR_SABOTAGE: self._device.sabotage})
+        return attr
diff --git a/homeassistant/components/sensor/homematicip_cloud.py b/homeassistant/components/sensor/homematicip_cloud.py
index 87021e9c7c5..7292e3b2f40 100644
--- a/homeassistant/components/sensor/homematicip_cloud.py
+++ b/homeassistant/components/sensor/homematicip_cloud.py
@@ -80,11 +80,6 @@ class HomematicipAccesspointStatus(HomematicipGenericDevice):
         """Return the unit this state is expressed in."""
         return '%'
 
-    @property
-    def device_state_attributes(self):
-        """Return the state attributes of the access point."""
-        return {}
-
 
 class HomematicipHeatingThermostat(HomematicipGenericDevice):
     """MomematicIP heating thermostat representation."""
@@ -98,6 +93,8 @@ class HomematicipHeatingThermostat(HomematicipGenericDevice):
         """Return the icon."""
         from homematicip.base.enums import ValveState
 
+        if super().icon:
+            return super().icon
         if self._device.valveState != ValveState.ADAPTION_DONE:
             return 'mdi:alert'
         return 'mdi:radiator'