Enable BMW component to be unit system aware (#17197)

* Enable BMW component to be unit system aware

* lint fixes

* use constants for config entries

* remove configuration from component and rely only on HA config of unit_system

* remove unused import

* update code to reflect feedback

* lint fixes

* remove unnecessary comments

* rework return statement to satisfy pylint

* more lint fixes

* add tests for volume utils

* lint fixes

* more lint fixes

* remove unnecessary comments

homeassistant/components/binary_sensor/bmw_connected_drive.py

@@ -8,6 +8,7 @@ import logging
 
 from homeassistant.components.binary_sensor import BinarySensorDevice
 from homeassistant.components.bmw_connected_drive import DOMAIN as BMW_DOMAIN
+from homeassistant.const import LENGTH_KILOMETERS
 
 DEPENDENCIES = ['bmw_connected_drive']
 
@@ -117,7 +118,8 @@ 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:
-                result.update(self._format_cbs_report(report))
+                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:
@@ -175,8 +177,7 @@ class BMWConnectedDriveSensor(BinarySensorDevice):
             self._state = (vehicle_state._attributes['connectionStatus'] ==
                            'CONNECTED')
 
-    @staticmethod
+    def _format_cbs_report(self, report):
-    def _format_cbs_report(report):
         result = {}
         service_type = report.service_type.lower().replace('_', ' ')
         result['{} status'.format(service_type)] = report.state.value
@@ -184,8 +185,10 @@ class BMWConnectedDriveSensor(BinarySensorDevice):
             result['{} date'.format(service_type)] = \
                 report.due_date.strftime('%Y-%m-%d')
         if report.due_distance is not None:
-            result['{} distance'.format(service_type)] = \
+            distance = round(self.hass.config.units.length(
-                '{} km'.format(report.due_distance)
+                report.due_distance, LENGTH_KILOMETERS))
+            result['{} distance'.format(service_type)] = '{} {}'.format(
+                distance, self.hass.config.units.length_unit)
         return result
 
     def update_callback(self):

homeassistant/components/bmw_connected_drive/__init__.py

@@ -9,7 +9,7 @@ import logging
 
 import voluptuous as vol
 
-from homeassistant.const import CONF_USERNAME, CONF_PASSWORD
+from homeassistant.const import (CONF_USERNAME, CONF_PASSWORD)
 from homeassistant.helpers import discovery
 from homeassistant.helpers.event import track_utc_time_change
 import homeassistant.helpers.config_validation as cv
@@ -85,6 +85,7 @@ def setup_account(account_config: dict, hass, name: str) \
     password = account_config[CONF_PASSWORD]
     region = account_config[CONF_REGION]
     read_only = account_config[CONF_READ_ONLY]
+
     _LOGGER.debug('Adding new account %s', name)
     cd_account = BMWConnectedDriveAccount(username, password, region, name,
                                           read_only)

homeassistant/components/sensor/bmw_connected_drive.py

@@ -9,18 +9,32 @@ import logging
 from homeassistant.components.bmw_connected_drive import DOMAIN as BMW_DOMAIN
 from homeassistant.helpers.entity import Entity
 from homeassistant.helpers.icon import icon_for_battery_level
+from homeassistant.const import (CONF_UNIT_SYSTEM_IMPERIAL, VOLUME_LITERS,
+                                 VOLUME_GALLONS, LENGTH_KILOMETERS,
+                                 LENGTH_MILES)
 
 DEPENDENCIES = ['bmw_connected_drive']
 
 _LOGGER = logging.getLogger(__name__)
 
-ATTR_TO_HA = {
+ATTR_TO_HA_METRIC = {
-    'mileage': ['mdi:speedometer', 'km'],
+    'mileage': ['mdi:speedometer', LENGTH_KILOMETERS],
-    'remaining_range_total': ['mdi:ruler', 'km'],
+    'remaining_range_total': ['mdi:ruler', LENGTH_KILOMETERS],
-    'remaining_range_electric': ['mdi:ruler', 'km'],
+    'remaining_range_electric': ['mdi:ruler', LENGTH_KILOMETERS],
-    'remaining_range_fuel': ['mdi:ruler', 'km'],
+    'remaining_range_fuel': ['mdi:ruler', LENGTH_KILOMETERS],
-    'max_range_electric': ['mdi:ruler', 'km'],
+    'max_range_electric': ['mdi:ruler', LENGTH_KILOMETERS],
-    'remaining_fuel': ['mdi:gas-station', 'l'],
+    'remaining_fuel': ['mdi:gas-station', VOLUME_LITERS],
+    'charging_time_remaining': ['mdi:update', 'h'],
+    'charging_status': ['mdi:battery-charging', None]
+}
+
+ATTR_TO_HA_IMPERIAL = {
+    'mileage': ['mdi:speedometer', LENGTH_MILES],
+    'remaining_range_total': ['mdi:ruler', LENGTH_MILES],
+    'remaining_range_electric': ['mdi:ruler', LENGTH_MILES],
+    'remaining_range_fuel': ['mdi:ruler', LENGTH_MILES],
+    'max_range_electric': ['mdi:ruler', LENGTH_MILES],
+    'remaining_fuel': ['mdi:gas-station', VOLUME_GALLONS],
     'charging_time_remaining': ['mdi:update', 'h'],
     'charging_status': ['mdi:battery-charging', None]
 }
@@ -28,6 +42,11 @@ ATTR_TO_HA = {
 
 def setup_platform(hass, config, add_entities, discovery_info=None):
     """Set up the BMW sensors."""
+    if hass.config.units.name == CONF_UNIT_SYSTEM_IMPERIAL:
+        attribute_info = ATTR_TO_HA_IMPERIAL
+    else:
+        attribute_info = ATTR_TO_HA_METRIC
+
     accounts = hass.data[BMW_DOMAIN]
     _LOGGER.debug('Found BMW accounts: %s',
                   ', '.join([a.name for a in accounts]))
@@ -36,9 +55,11 @@ def setup_platform(hass, config, add_entities, discovery_info=None):
         for vehicle in account.account.vehicles:
             for attribute_name in vehicle.drive_train_attributes:
                 device = BMWConnectedDriveSensor(account, vehicle,
-                                                 attribute_name)
+                                                 attribute_name,
+                                                 attribute_info)
                 devices.append(device)
-            device = BMWConnectedDriveSensor(account, vehicle, 'mileage')
+            device = BMWConnectedDriveSensor(account, vehicle, 'mileage',
+                                             attribute_info)
             devices.append(device)
     add_entities(devices, True)
 
@@ -46,7 +67,7 @@ def setup_platform(hass, config, add_entities, discovery_info=None):
 class BMWConnectedDriveSensor(Entity):
     """Representation of a BMW vehicle sensor."""
 
-    def __init__(self, account, vehicle, attribute: str):
+    def __init__(self, account, vehicle, attribute: str, attribute_info):
         """Constructor."""
         self._vehicle = vehicle
         self._account = account
@@ -54,6 +75,7 @@ class BMWConnectedDriveSensor(Entity):
         self._state = None
         self._name = '{} {}'.format(self._vehicle.name, self._attribute)
         self._unique_id = '{}-{}'.format(self._vehicle.vin, self._attribute)
+        self._attribute_info = attribute_info
 
     @property
     def should_poll(self) -> bool:
@@ -78,14 +100,14 @@ class BMWConnectedDriveSensor(Entity):
         """Icon to use in the frontend, if any."""
         from bimmer_connected.state import ChargingState
         vehicle_state = self._vehicle.state
-        charging_state = vehicle_state.charging_status in \
+        charging_state = vehicle_state.charging_status in [
-            [ChargingState.CHARGING]
+            ChargingState.CHARGING]
 
         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])
+        icon, _ = self._attribute_info.get(self._attribute, [None, None])
         return icon
 
     @property
@@ -100,7 +122,7 @@ class BMWConnectedDriveSensor(Entity):
     @property
     def unit_of_measurement(self) -> str:
         """Get the unit of measurement."""
-        _, unit = ATTR_TO_HA.get(self._attribute, [None, None])
+        _, unit = self._attribute_info.get(self._attribute, [None, None])
         return unit
 
     @property
@@ -116,6 +138,16 @@ class BMWConnectedDriveSensor(Entity):
         vehicle_state = self._vehicle.state
         if self._attribute == 'charging_status':
             self._state = getattr(vehicle_state, self._attribute).value
+        elif self.unit_of_measurement == VOLUME_GALLONS:
+            value = getattr(vehicle_state, self._attribute)
+            value_converted = self.hass.config.units.volume(value,
+                                                            VOLUME_LITERS)
+            self._state = round(value_converted)
+        elif self.unit_of_measurement == LENGTH_MILES:
+            value = getattr(vehicle_state, self._attribute)
+            value_converted = self.hass.config.units.length(value,
+                                                            LENGTH_KILOMETERS)
+            self._state = round(value_converted)
         else:
             self._state = getattr(vehicle_state, self._attribute)
 

homeassistant/util/unit_system.py

@@ -13,6 +13,7 @@ from homeassistant.const import (
     TEMPERATURE, UNIT_NOT_RECOGNIZED_TEMPLATE)
 from homeassistant.util import temperature as temperature_util
 from homeassistant.util import distance as distance_util
+from homeassistant.util import volume as volume_util
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -108,6 +109,13 @@ class UnitSystem:
         return distance_util.convert(length, from_unit,
                                      self.length_unit)
 
+    def volume(self, volume: Optional[float], from_unit: str) -> float:
+        """Convert the given volume to this unit system."""
+        if not isinstance(volume, Number):
+            raise TypeError('{} is not a numeric value.'.format(str(volume)))
+
+        return volume_util.convert(volume, from_unit, self.volume_unit)
+
     def as_dict(self) -> dict:
         """Convert the unit system to a dictionary."""
         return {

homeassistant/util/volume.py

@@ -0,0 +1,45 @@
+"""Volume conversion util functions."""
+
+import logging
+from numbers import Number
+from homeassistant.const import (VOLUME_LITERS, VOLUME_MILLILITERS,
+                                 VOLUME_GALLONS, VOLUME_FLUID_OUNCE,
+                                 VOLUME, UNIT_NOT_RECOGNIZED_TEMPLATE)
+
+_LOGGER = logging.getLogger(__name__)
+
+VALID_UNITS = [VOLUME_LITERS, VOLUME_MILLILITERS, VOLUME_GALLONS,
+               VOLUME_FLUID_OUNCE]
+
+
+def __liter_to_gallon(liter: float) -> float:
+    """Convert a volume measurement in Liter to Gallon."""
+    return liter * .2642
+
+
+def __gallon_to_liter(gallon: float) -> float:
+    """Convert a volume measurement in Gallon to Liter."""
+    return gallon * 3.785
+
+
+def convert(volume: float, from_unit: str, to_unit: str) -> float:
+    """Convert a temperature from one unit to another."""
+    if from_unit not in VALID_UNITS:
+        raise ValueError(UNIT_NOT_RECOGNIZED_TEMPLATE.format(from_unit,
+                                                             VOLUME))
+    if to_unit not in VALID_UNITS:
+        raise ValueError(UNIT_NOT_RECOGNIZED_TEMPLATE.format(to_unit, VOLUME))
+
+    if not isinstance(volume, Number):
+        raise TypeError('{} is not of numeric type'.format(volume))
+
+    if from_unit == to_unit:
+        return volume
+
+    result = volume
+    if from_unit == VOLUME_LITERS and to_unit == VOLUME_GALLONS:
+        result = __liter_to_gallon(volume)
+    elif from_unit == VOLUME_GALLONS and to_unit == VOLUME_LITERS:
+        result = __gallon_to_liter(volume)
+
+    return result

tests/util/test_volume.py

@@ -0,0 +1,49 @@
+"""Test homeassistant volume utility functions."""
+
+import unittest
+import homeassistant.util.volume as volume_util
+from homeassistant.const import (VOLUME_LITERS, VOLUME_MILLILITERS,
+                                 VOLUME_GALLONS, VOLUME_FLUID_OUNCE)
+
+INVALID_SYMBOL = 'bob'
+VALID_SYMBOL = VOLUME_LITERS
+
+
+class TestVolumeUtil(unittest.TestCase):
+    """Test the volume utility functions."""
+
+    def test_convert_same_unit(self):
+        """Test conversion from any unit to same unit."""
+        self.assertEqual(2, volume_util.convert(2, VOLUME_LITERS,
+                                                VOLUME_LITERS))
+        self.assertEqual(3, volume_util.convert(3, VOLUME_MILLILITERS,
+                                                VOLUME_MILLILITERS))
+        self.assertEqual(4, volume_util.convert(4, VOLUME_GALLONS,
+                                                VOLUME_GALLONS))
+        self.assertEqual(5, volume_util.convert(5, VOLUME_FLUID_OUNCE,
+                                                VOLUME_FLUID_OUNCE))
+
+    def test_convert_invalid_unit(self):
+        """Test exception is thrown for invalid units."""
+        with self.assertRaises(ValueError):
+            volume_util.convert(5, INVALID_SYMBOL, VALID_SYMBOL)
+
+        with self.assertRaises(ValueError):
+            volume_util.convert(5, VALID_SYMBOL, INVALID_SYMBOL)
+
+    def test_convert_nonnumeric_value(self):
+        """Test exception is thrown for nonnumeric type."""
+        with self.assertRaises(TypeError):
+            volume_util.convert('a', VOLUME_GALLONS, VOLUME_LITERS)
+
+    def test_convert_from_liters(self):
+        """Test conversion from liters to other units."""
+        liters = 5
+        self.assertEqual(volume_util.convert(liters, VOLUME_LITERS,
+                                             VOLUME_GALLONS), 1.321)
+
+    def test_convert_from_gallons(self):
+        """Test conversion from gallons to other units."""
+        gallons = 5
+        self.assertEqual(volume_util.convert(gallons, VOLUME_GALLONS,
+                                             VOLUME_LITERS), 18.925)