From 2e7b5dcd196782e2005577a6229ce4f4d168d814 Mon Sep 17 00:00:00 2001
From: Gerard <gerard33@users.noreply.github.com>
Date: Tue, 15 May 2018 20:47:32 +0200
Subject: [PATCH] BMW code cleanup (#14424)

* Some cleanup for BMW sensors

* Changed dict sort

* Updates based on review and Travis
---
 .../binary_sensor/bmw_connected_drive.py      | 24 ++++++-----
 .../components/sensor/bmw_connected_drive.py  | 40 ++++++++-----------
 2 files changed, 31 insertions(+), 33 deletions(-)

diff --git a/homeassistant/components/binary_sensor/bmw_connected_drive.py b/homeassistant/components/binary_sensor/bmw_connected_drive.py
index af3ebd53b80..e214610f46d 100644
--- a/homeassistant/components/binary_sensor/bmw_connected_drive.py
+++ b/homeassistant/components/binary_sensor/bmw_connected_drive.py
@@ -115,14 +115,7 @@ class BMWConnectedDriveSensor(BinarySensorDevice):
             result['lights_parking'] = vehicle_state.parking_lights.value
         elif self._attribute == 'condition_based_services':
             for report in vehicle_state.condition_based_services:
-                service_type = report.service_type.lower().replace('_', ' ')
-                result['{} status'.format(service_type)] = report.state.value
-                if report.due_date is not None:
-                    result['{} date'.format(service_type)] = \
-                        report.due_date.strftime('%Y-%m-%d')
-                if report.due_distance is not None:
-                    result['{} distance'.format(service_type)] = \
-                        '{} km'.format(report.due_distance)
+                result.update(self._format_cbs_report(report))
         elif self._attribute == 'check_control_messages':
             check_control_messages = vehicle_state.check_control_messages
             if not check_control_messages:
@@ -139,7 +132,7 @@ class BMWConnectedDriveSensor(BinarySensorDevice):
             result['connection_status'] = \
                 vehicle_state._attributes['connectionStatus']
 
-        return result
+        return sorted(result.items())
 
     def update(self):
         """Read new state data from the library."""
@@ -177,6 +170,19 @@ class BMWConnectedDriveSensor(BinarySensorDevice):
             self._state = (vehicle_state._attributes['connectionStatus'] ==
                            'CONNECTED')
 
+    @staticmethod
+    def _format_cbs_report(report):
+        result = {}
+        service_type = report.service_type.lower().replace('_', ' ')
+        result['{} status'.format(service_type)] = report.state.value
+        if report.due_date is not None:
+            result['{} date'.format(service_type)] = \
+                report.due_date.strftime('%Y-%m-%d')
+        if report.due_distance is not None:
+            result['{} distance'.format(service_type)] = \
+                '{} km'.format(report.due_distance)
+        return result
+
     def update_callback(self):
         """Schedule a state update."""
         self.schedule_update_ha_state(True)
diff --git a/homeassistant/components/sensor/bmw_connected_drive.py b/homeassistant/components/sensor/bmw_connected_drive.py
index 8e06836b102..e3331cdc763 100644
--- a/homeassistant/components/sensor/bmw_connected_drive.py
+++ b/homeassistant/components/sensor/bmw_connected_drive.py
@@ -15,6 +15,17 @@ DEPENDENCIES = ['bmw_connected_drive']
 
 _LOGGER = logging.getLogger(__name__)
 
+ATTR_TO_HA = {
+    'mileage': ['mdi:speedometer', 'km'],
+    'remaining_range_total': ['mdi:ruler', 'km'],
+    'remaining_range_electric': ['mdi:ruler', 'km'],
+    'remaining_range_fuel': ['mdi:ruler', 'km'],
+    'max_range_electric': ['mdi:ruler', 'km'],
+    'remaining_fuel': ['mdi:gas-station', 'l'],
+    'charging_time_remaining': ['mdi:update', 'h'],
+    'charging_status': ['mdi:battery-charging', None]
+}
+
 
 def setup_platform(hass, config, add_devices, discovery_info=None):
     """Set up the BMW sensors."""
@@ -68,22 +79,12 @@ class BMWConnectedDriveSensor(Entity):
         charging_state = vehicle_state.charging_status in \
             [ChargingState.CHARGING]
 
-        if self._attribute == 'mileage':
-            return 'mdi:speedometer'
-        elif self._attribute in (
-                'remaining_range_total', 'remaining_range_electric',
-                'remaining_range_fuel', 'max_range_electric'):
-            return 'mdi:ruler'
-        elif self._attribute == 'remaining_fuel':
-            return 'mdi:gas-station'
-        elif self._attribute == 'charging_time_remaining':
-            return 'mdi:update'
-        elif self._attribute == 'charging_status':
-            return 'mdi:battery-charging'
-        elif self._attribute == 'charging_level_hv':
+        if self._attribute == 'charging_level_hv':
             return icon_for_battery_level(
                 battery_level=vehicle_state.charging_level_hv,
                 charging=charging_state)
+        icon, _ = ATTR_TO_HA.get(self._attribute, [None, None])
+        return icon
 
     @property
     def state(self):
@@ -97,17 +98,8 @@ class BMWConnectedDriveSensor(Entity):
     @property
     def unit_of_measurement(self) -> str:
         """Get the unit of measurement."""
-        if self._attribute in (
-                'mileage', 'remaining_range_total', 'remaining_range_electric',
-                'remaining_range_fuel', 'max_range_electric'):
-            return 'km'
-        elif self._attribute == 'remaining_fuel':
-            return 'l'
-        elif self._attribute == 'charging_time_remaining':
-            return 'h'
-        elif self._attribute == 'charging_level_hv':
-            return '%'
-        return None
+        _, unit = ATTR_TO_HA.get(self._attribute, [None, None])
+        return unit
 
     @property
     def device_state_attributes(self):