Fix displayed units for BMW Connected Drive (#76613)

* Fix displayed units

* Add tests for unit conversion

* Streamline test config entry init

* Refactor test to pytest fixture

* Fix renamed mock

Co-authored-by: rikroe <rikroe@users.noreply.github.com>

homeassistant/components/bmw_connected_drive/sensor.py

@@ -15,13 +15,7 @@ from homeassistant.components.sensor import (
     SensorEntityDescription,
 )
 from homeassistant.config_entries import ConfigEntry
-from homeassistant.const import (
-    LENGTH_KILOMETERS,
-    LENGTH_MILES,
-    PERCENTAGE,
-    VOLUME_GALLONS,
-    VOLUME_LITERS,
-)
+from homeassistant.const import LENGTH, PERCENTAGE, VOLUME
 from homeassistant.core import HomeAssistant, callback
 from homeassistant.helpers.entity_platform import AddEntitiesCallback
 from homeassistant.helpers.typing import StateType
@@ -39,8 +33,7 @@ class BMWSensorEntityDescription(SensorEntityDescription):
     """Describes BMW sensor entity."""
 
     key_class: str | None = None
-    unit_metric: str | None = None
-    unit_imperial: str | None = None
+    unit_type: str | None = None
     value: Callable = lambda x, y: x
 
 
@@ -81,56 +74,49 @@ SENSOR_TYPES: dict[str, BMWSensorEntityDescription] = {
     "remaining_battery_percent": BMWSensorEntityDescription(
         key="remaining_battery_percent",
         key_class="fuel_and_battery",
-        unit_metric=PERCENTAGE,
-        unit_imperial=PERCENTAGE,
+        unit_type=PERCENTAGE,
         device_class=SensorDeviceClass.BATTERY,
     ),
     # --- Specific ---
     "mileage": BMWSensorEntityDescription(
         key="mileage",
         icon="mdi:speedometer",
-        unit_metric=LENGTH_KILOMETERS,
-        unit_imperial=LENGTH_MILES,
+        unit_type=LENGTH,
         value=lambda x, hass: convert_and_round(x, hass.config.units.length, 2),
     ),
     "remaining_range_total": BMWSensorEntityDescription(
         key="remaining_range_total",
         key_class="fuel_and_battery",
         icon="mdi:map-marker-distance",
-        unit_metric=LENGTH_KILOMETERS,
-        unit_imperial=LENGTH_MILES,
+        unit_type=LENGTH,
         value=lambda x, hass: convert_and_round(x, hass.config.units.length, 2),
     ),
     "remaining_range_electric": BMWSensorEntityDescription(
         key="remaining_range_electric",
         key_class="fuel_and_battery",
         icon="mdi:map-marker-distance",
-        unit_metric=LENGTH_KILOMETERS,
-        unit_imperial=LENGTH_MILES,
+        unit_type=LENGTH,
         value=lambda x, hass: convert_and_round(x, hass.config.units.length, 2),
     ),
     "remaining_range_fuel": BMWSensorEntityDescription(
         key="remaining_range_fuel",
         key_class="fuel_and_battery",
         icon="mdi:map-marker-distance",
-        unit_metric=LENGTH_KILOMETERS,
-        unit_imperial=LENGTH_MILES,
+        unit_type=LENGTH,
         value=lambda x, hass: convert_and_round(x, hass.config.units.length, 2),
     ),
     "remaining_fuel": BMWSensorEntityDescription(
         key="remaining_fuel",
         key_class="fuel_and_battery",
         icon="mdi:gas-station",
-        unit_metric=VOLUME_LITERS,
-        unit_imperial=VOLUME_GALLONS,
+        unit_type=VOLUME,
         value=lambda x, hass: convert_and_round(x, hass.config.units.volume, 2),
     ),
     "remaining_fuel_percent": BMWSensorEntityDescription(
         key="remaining_fuel_percent",
         key_class="fuel_and_battery",
         icon="mdi:gas-station",
-        unit_metric=PERCENTAGE,
-        unit_imperial=PERCENTAGE,
+        unit_type=PERCENTAGE,
     ),
 }
 
@@ -177,8 +163,12 @@ class BMWSensor(BMWBaseEntity, SensorEntity):
         self._attr_name = f"{vehicle.name} {description.key}"
         self._attr_unique_id = f"{vehicle.vin}-{description.key}"
 
-        # Force metric system as BMW API apparently only returns metric values now
-        self._attr_native_unit_of_measurement = description.unit_metric
+        # Set the correct unit of measurement based on the unit_type
+        if description.unit_type:
+            self._attr_native_unit_of_measurement = (
+                coordinator.hass.config.units.as_dict().get(description.unit_type)
+                or description.unit_type
+            )
 
     @callback
     def _handle_coordinator_update(self) -> None:

tests/components/bmw_connected_drive/__init__.py

@@ -1,17 +1,29 @@
 """Tests for the for the BMW Connected Drive integration."""
 
+import json
+from pathlib import Path
+
+from bimmer_connected.account import MyBMWAccount
+from bimmer_connected.api.utils import log_to_to_file
+
 from homeassistant import config_entries
 from homeassistant.components.bmw_connected_drive.const import (
     CONF_READ_ONLY,
+    CONF_REFRESH_TOKEN,
     DOMAIN as BMW_DOMAIN,
 )
 from homeassistant.const import CONF_PASSWORD, CONF_REGION, CONF_USERNAME
+from homeassistant.core import HomeAssistant
+from homeassistant.util.dt import utcnow
+
+from tests.common import MockConfigEntry, get_fixture_path, load_fixture
 
 FIXTURE_USER_INPUT = {
     CONF_USERNAME: "user@domain.com",
     CONF_PASSWORD: "p4ssw0rd",
     CONF_REGION: "rest_of_world",
 }
+FIXTURE_REFRESH_TOKEN = "SOME_REFRESH_TOKEN"
 
 FIXTURE_CONFIG_ENTRY = {
     "entry_id": "1",
@@ -21,8 +33,82 @@ FIXTURE_CONFIG_ENTRY = {
         CONF_USERNAME: FIXTURE_USER_INPUT[CONF_USERNAME],
         CONF_PASSWORD: FIXTURE_USER_INPUT[CONF_PASSWORD],
         CONF_REGION: FIXTURE_USER_INPUT[CONF_REGION],
+        CONF_REFRESH_TOKEN: FIXTURE_REFRESH_TOKEN,
     },
     "options": {CONF_READ_ONLY: False},
     "source": config_entries.SOURCE_USER,
     "unique_id": f"{FIXTURE_USER_INPUT[CONF_REGION]}-{FIXTURE_USER_INPUT[CONF_REGION]}",
 }
+
+
+async def mock_vehicles_from_fixture(account: MyBMWAccount) -> None:
+    """Load MyBMWVehicle from fixtures and add them to the account."""
+
+    fixture_path = Path(get_fixture_path("", integration=BMW_DOMAIN))
+
+    fixture_vehicles_bmw = list(fixture_path.rglob("vehicles_v2_bmw_*.json"))
+    fixture_vehicles_mini = list(fixture_path.rglob("vehicles_v2_mini_*.json"))
+
+    # Load vehicle base lists as provided by vehicles/v2 API
+    vehicles = {
+        "bmw": [
+            vehicle
+            for bmw_file in fixture_vehicles_bmw
+            for vehicle in json.loads(load_fixture(bmw_file, integration=BMW_DOMAIN))
+        ],
+        "mini": [
+            vehicle
+            for mini_file in fixture_vehicles_mini
+            for vehicle in json.loads(load_fixture(mini_file, integration=BMW_DOMAIN))
+        ],
+    }
+    fetched_at = utcnow()
+
+    # simulate storing fingerprints
+    if account.config.log_response_path:
+        for brand in ["bmw", "mini"]:
+            log_to_to_file(
+                json.dumps(vehicles[brand]),
+                account.config.log_response_path,
+                f"vehicles_v2_{brand}",
+            )
+
+    # Create a vehicle with base + specific state as provided by state/VIN API
+    for vehicle_base in [vehicle for brand in vehicles.values() for vehicle in brand]:
+        vehicle_state_path = (
+            Path("vehicles")
+            / vehicle_base["attributes"]["bodyType"]
+            / f"state_{vehicle_base['vin']}_0.json"
+        )
+        vehicle_state = json.loads(
+            load_fixture(
+                vehicle_state_path,
+                integration=BMW_DOMAIN,
+            )
+        )
+
+        account.add_vehicle(
+            vehicle_base,
+            vehicle_state,
+            fetched_at,
+        )
+
+        # simulate storing fingerprints
+        if account.config.log_response_path:
+            log_to_to_file(
+                json.dumps(vehicle_state),
+                account.config.log_response_path,
+                f"state_{vehicle_base['vin']}",
+            )
+
+
+async def setup_mocked_integration(hass: HomeAssistant) -> MockConfigEntry:
+    """Mock a fully setup config entry and all components based on fixtures."""
+
+    mock_config_entry = MockConfigEntry(**FIXTURE_CONFIG_ENTRY)
+    mock_config_entry.add_to_hass(hass)
+
+    assert await hass.config_entries.async_setup(mock_config_entry.entry_id)
+    await hass.async_block_till_done()
+
+    return mock_config_entry

tests/components/bmw_connected_drive/conftest.py

@@ -0,0 +1,12 @@
+"""Fixtures for BMW tests."""
+
+from bimmer_connected.account import MyBMWAccount
+import pytest
+
+from . import mock_vehicles_from_fixture
+
+
+@pytest.fixture
+async def bmw_fixture(monkeypatch):
+    """Patch the vehicle fixtures into a MyBMWAccount."""
+    monkeypatch.setattr(MyBMWAccount, "get_vehicles", mock_vehicles_from_fixture)

tests/components/bmw_connected_drive/fixtures/vehicles/I01/state_WBY00000000REXI01_0.json

@@ -0,0 +1,206 @@
+{
+  "capabilities": {
+    "climateFunction": "AIR_CONDITIONING",
+    "climateNow": true,
+    "climateTimerTrigger": "DEPARTURE_TIMER",
+    "horn": true,
+    "isBmwChargingSupported": true,
+    "isCarSharingSupported": false,
+    "isChargeNowForBusinessSupported": false,
+    "isChargingHistorySupported": true,
+    "isChargingHospitalityEnabled": false,
+    "isChargingLoudnessEnabled": false,
+    "isChargingPlanSupported": true,
+    "isChargingPowerLimitEnabled": false,
+    "isChargingSettingsEnabled": false,
+    "isChargingTargetSocEnabled": false,
+    "isClimateTimerSupported": true,
+    "isCustomerEsimSupported": false,
+    "isDCSContractManagementSupported": true,
+    "isDataPrivacyEnabled": false,
+    "isEasyChargeEnabled": false,
+    "isEvGoChargingSupported": false,
+    "isMiniChargingSupported": false,
+    "isNonLscFeatureEnabled": false,
+    "isRemoteEngineStartSupported": false,
+    "isRemoteHistoryDeletionSupported": false,
+    "isRemoteHistorySupported": true,
+    "isRemoteParkingSupported": false,
+    "isRemoteServicesActivationRequired": false,
+    "isRemoteServicesBookingRequired": false,
+    "isScanAndChargeSupported": false,
+    "isSustainabilitySupported": false,
+    "isWifiHotspotServiceSupported": false,
+    "lastStateCallState": "ACTIVATED",
+    "lights": true,
+    "lock": true,
+    "remoteChargingCommands": {},
+    "sendPoi": true,
+    "specialThemeSupport": [],
+    "unlock": true,
+    "vehicleFinder": false,
+    "vehicleStateSource": "LAST_STATE_CALL"
+  },
+  "state": {
+    "chargingProfile": {
+      "chargingControlType": "WEEKLY_PLANNER",
+      "chargingMode": "DELAYED_CHARGING",
+      "chargingPreference": "CHARGING_WINDOW",
+      "chargingSettings": {
+        "hospitality": "NO_ACTION",
+        "idcc": "NO_ACTION",
+        "targetSoc": 100
+      },
+      "climatisationOn": false,
+      "departureTimes": [
+        {
+          "action": "DEACTIVATE",
+          "id": 1,
+          "timeStamp": {
+            "hour": 7,
+            "minute": 35
+          },
+          "timerWeekDays": [
+            "MONDAY",
+            "TUESDAY",
+            "WEDNESDAY",
+            "THURSDAY",
+            "FRIDAY"
+          ]
+        },
+        {
+          "action": "DEACTIVATE",
+          "id": 2,
+          "timeStamp": {
+            "hour": 18,
+            "minute": 0
+          },
+          "timerWeekDays": [
+            "MONDAY",
+            "TUESDAY",
+            "WEDNESDAY",
+            "THURSDAY",
+            "FRIDAY",
+            "SATURDAY",
+            "SUNDAY"
+          ]
+        },
+        {
+          "action": "DEACTIVATE",
+          "id": 3,
+          "timeStamp": {
+            "hour": 7,
+            "minute": 0
+          },
+          "timerWeekDays": []
+        },
+        {
+          "action": "DEACTIVATE",
+          "id": 4,
+          "timerWeekDays": []
+        }
+      ],
+      "reductionOfChargeCurrent": {
+        "end": {
+          "hour": 1,
+          "minute": 30
+        },
+        "start": {
+          "hour": 18,
+          "minute": 1
+        }
+      }
+    },
+    "checkControlMessages": [],
+    "climateTimers": [
+      {
+        "departureTime": {
+          "hour": 6,
+          "minute": 40
+        },
+        "isWeeklyTimer": true,
+        "timerAction": "ACTIVATE",
+        "timerWeekDays": ["THURSDAY", "SUNDAY"]
+      },
+      {
+        "departureTime": {
+          "hour": 12,
+          "minute": 50
+        },
+        "isWeeklyTimer": false,
+        "timerAction": "ACTIVATE",
+        "timerWeekDays": ["MONDAY"]
+      },
+      {
+        "departureTime": {
+          "hour": 18,
+          "minute": 59
+        },
+        "isWeeklyTimer": true,
+        "timerAction": "DEACTIVATE",
+        "timerWeekDays": ["WEDNESDAY"]
+      }
+    ],
+    "combustionFuelLevel": {
+      "range": 105,
+      "remainingFuelLiters": 6,
+      "remainingFuelPercent": 65
+    },
+    "currentMileage": 137009,
+    "doorsState": {
+      "combinedSecurityState": "UNLOCKED",
+      "combinedState": "CLOSED",
+      "hood": "CLOSED",
+      "leftFront": "CLOSED",
+      "leftRear": "CLOSED",
+      "rightFront": "CLOSED",
+      "rightRear": "CLOSED",
+      "trunk": "CLOSED"
+    },
+    "driverPreferences": {
+      "lscPrivacyMode": "OFF"
+    },
+    "electricChargingState": {
+      "chargingConnectionType": "CONDUCTIVE",
+      "chargingLevelPercent": 82,
+      "chargingStatus": "WAITING_FOR_CHARGING",
+      "chargingTarget": 100,
+      "isChargerConnected": true,
+      "range": 174
+    },
+    "isLeftSteering": true,
+    "isLscSupported": true,
+    "lastFetched": "2022-06-22T14:24:23.982Z",
+    "lastUpdatedAt": "2022-06-22T13:58:52Z",
+    "range": 174,
+    "requiredServices": [
+      {
+        "dateTime": "2022-10-01T00:00:00.000Z",
+        "description": "Next service due by the specified date.",
+        "status": "OK",
+        "type": "BRAKE_FLUID"
+      },
+      {
+        "dateTime": "2023-05-01T00:00:00.000Z",
+        "description": "Next vehicle check due after the specified distance or date.",
+        "status": "OK",
+        "type": "VEHICLE_CHECK"
+      },
+      {
+        "dateTime": "2023-05-01T00:00:00.000Z",
+        "description": "Next state inspection due by the specified date.",
+        "status": "OK",
+        "type": "VEHICLE_TUV"
+      }
+    ],
+    "roofState": {
+      "roofState": "CLOSED",
+      "roofStateType": "SUN_ROOF"
+    },
+    "windowsState": {
+      "combinedState": "CLOSED",
+      "leftFront": "CLOSED",
+      "rightFront": "CLOSED"
+    }
+  }
+}

tests/components/bmw_connected_drive/fixtures/vehicles/I01/vehicles_v2_bmw_0.json

@@ -0,0 +1,47 @@
+[
+  {
+    "appVehicleType": "CONNECTED",
+    "attributes": {
+      "a4aType": "USB_ONLY",
+      "bodyType": "I01",
+      "brand": "BMW_I",
+      "color": 4284110934,
+      "countryOfOrigin": "CZ",
+      "driveTrain": "ELECTRIC_WITH_RANGE_EXTENDER",
+      "driverGuideInfo": {
+        "androidAppScheme": "com.bmwgroup.driversguide.row",
+        "androidStoreUrl": "https://play.google.com/store/apps/details?id=com.bmwgroup.driversguide.row",
+        "iosAppScheme": "bmwdriversguide:///open",
+        "iosStoreUrl": "https://apps.apple.com/de/app/id714042749?mt=8"
+      },
+      "headUnitType": "NBT",
+      "hmiVersion": "ID4",
+      "lastFetched": "2022-07-10T09:25:53.104Z",
+      "model": "i3 (+ REX)",
+      "softwareVersionCurrent": {
+        "iStep": 510,
+        "puStep": {
+          "month": 11,
+          "year": 21
+        },
+        "seriesCluster": "I001"
+      },
+      "softwareVersionExFactory": {
+        "iStep": 502,
+        "puStep": {
+          "month": 3,
+          "year": 15
+        },
+        "seriesCluster": "I001"
+      },
+      "year": 2015
+    },
+    "mappingInfo": {
+      "isAssociated": false,
+      "isLmmEnabled": false,
+      "isPrimaryUser": true,
+      "mappingStatus": "CONFIRMED"
+    },
+    "vin": "WBY00000000REXI01"
+  }
+]

tests/components/bmw_connected_drive/test_config_flow.py

@@ -12,15 +12,11 @@ from homeassistant.components.bmw_connected_drive.const import (
 )
 from homeassistant.const import CONF_USERNAME
 
-from . import FIXTURE_CONFIG_ENTRY, FIXTURE_USER_INPUT
+from . import FIXTURE_CONFIG_ENTRY, FIXTURE_REFRESH_TOKEN, FIXTURE_USER_INPUT
 
 from tests.common import MockConfigEntry
 
-FIXTURE_REFRESH_TOKEN = "SOME_REFRESH_TOKEN"
-FIXTURE_COMPLETE_ENTRY = {
-    **FIXTURE_USER_INPUT,
-    CONF_REFRESH_TOKEN: FIXTURE_REFRESH_TOKEN,
-}
+FIXTURE_COMPLETE_ENTRY = FIXTURE_CONFIG_ENTRY["data"]
 FIXTURE_IMPORT_ENTRY = {**FIXTURE_USER_INPUT, CONF_REFRESH_TOKEN: None}
 
 

tests/components/bmw_connected_drive/test_sensor.py

@@ -0,0 +1,52 @@
+"""Test BMW sensors."""
+import pytest
+
+from homeassistant.core import HomeAssistant
+from homeassistant.util.unit_system import (
+    IMPERIAL_SYSTEM as IMPERIAL,
+    METRIC_SYSTEM as METRIC,
+    UnitSystem,
+)
+
+from . import setup_mocked_integration
+
+
+@pytest.mark.parametrize(
+    "entity_id,unit_system,value,unit_of_measurement",
+    [
+        ("sensor.i3_rex_remaining_range_total", METRIC, "279", "km"),
+        ("sensor.i3_rex_remaining_range_total", IMPERIAL, "173.36", "mi"),
+        ("sensor.i3_rex_mileage", METRIC, "137009", "km"),
+        ("sensor.i3_rex_mileage", IMPERIAL, "85133.42", "mi"),
+        ("sensor.i3_rex_remaining_battery_percent", METRIC, "82", "%"),
+        ("sensor.i3_rex_remaining_battery_percent", IMPERIAL, "82", "%"),
+        ("sensor.i3_rex_remaining_range_electric", METRIC, "174", "km"),
+        ("sensor.i3_rex_remaining_range_electric", IMPERIAL, "108.12", "mi"),
+        ("sensor.i3_rex_remaining_fuel", METRIC, "6", "L"),
+        ("sensor.i3_rex_remaining_fuel", IMPERIAL, "1.59", "gal"),
+        ("sensor.i3_rex_remaining_range_fuel", METRIC, "105", "km"),
+        ("sensor.i3_rex_remaining_range_fuel", IMPERIAL, "65.24", "mi"),
+        ("sensor.i3_rex_remaining_fuel_percent", METRIC, "65", "%"),
+        ("sensor.i3_rex_remaining_fuel_percent", IMPERIAL, "65", "%"),
+    ],
+)
+async def test_unit_conversion(
+    hass: HomeAssistant,
+    entity_id: str,
+    unit_system: UnitSystem,
+    value: str,
+    unit_of_measurement: str,
+    bmw_fixture,
+) -> None:
+    """Test conversion between metric and imperial units for sensors."""
+
+    # Set unit system
+    hass.config.units = unit_system
+
+    # Setup component
+    assert await setup_mocked_integration(hass)
+
+    # Test
+    entity = hass.states.get(entity_id)
+    assert entity.state == value
+    assert entity.attributes.get("unit_of_measurement") == unit_of_measurement