Remove use_metric_units parameter (#584)

* Remove use_metric_units parameter

* Fix description

---------

Co-authored-by: rikroe <[email protected]>

bimmer_connected/account.py

@@ -48,18 +48,25 @@ class MyBMWAccount:
     observer_position: InitVar[GPSPosition] = None
     """Optional. Required for getting a position on older cars."""
 
-    use_metric_units: InitVar[bool] = True
-    """Optional. Use metric units (km, l) by default. Use imperial units (mi, gal) if False."""
+    use_metric_units: InitVar[Optional[bool]] = None
+    """Deprecated. All returned values are metric units (km, l)."""
 
     vehicles: List[MyBMWVehicle] = field(default_factory=list, init=False)
 
     def __post_init__(self, password, log_responses, observer_position, use_metric_units):
+        """Initialize the account."""
+
+        if use_metric_units is not None:
+            _LOGGER.warning(
+                "The use_metric_units parameter is deprecated and will be removed in a future release. "
+                "All values will be returned in metric units, as the parameter has no effect on the API."
+            )
+
         if self.config is None:
             self.config = MyBMWClientConfiguration(
                 MyBMWAuthentication(self.username, password, self.region),
                 log_responses=log_responses,
                 observer_position=observer_position,
-                use_metric_units=use_metric_units,
             )
 
     async def _init_vehicles(self) -> None:
@@ -185,10 +192,6 @@ def set_refresh_token(self, refresh_token: str, gcid: Optional[str] = None) -> N
         self.config.authentication.refresh_token = refresh_token
         self.config.authentication.gcid = gcid
 
-    def set_use_metric_units(self, use_metric_units: bool) -> None:
-        """Change between using metric units (km, l) if True or imperial units (mi, gal) if False."""
-        self.config.use_metric_units = use_metric_units
-
     @staticmethod
     def get_stored_responses() -> List[AnonymizedResponse]:
         """Return responses stored if log_responses was set to True."""

bimmer_connected/api/client.py

@@ -25,7 +25,6 @@ class MyBMWClientConfiguration:
     authentication: MyBMWAuthentication
     log_responses: Optional[bool] = False
     observer_position: Optional[GPSPosition] = None
-    use_metric_units: Optional[bool] = True
 
     def set_log_responses(self, log_responses: bool) -> None:
         """Set if responses are logged and clear response store."""
@@ -89,6 +88,6 @@ def generate_default_header(self, brand: Optional[CarBrands] = None) -> Dict[str
                 region=self.config.authentication.region.value,
             ),
             **get_correlation_id(),
-            "bmw-units-preferences": "d=KM;v=L" if self.config.use_metric_units else "d=MI;v=G",
+            "bmw-units-preferences": "d=KM;v=L",
             "24-hour-format": "true",
         }

bimmer_connected/cli.py

@@ -130,9 +130,7 @@ async def get_status(args) -> None:
         for handler in logging.root.handlers[:]:
             logging.root.removeHandler(handler)
 
-    account = MyBMWAccount(
-        args.username, args.password, get_region_from_name(args.region), use_metric_units=(not args.imperial)
-    )
+    account = MyBMWAccount(args.username, args.password, get_region_from_name(args.region))
     if args.lat and args.lng:
         account.set_observer_position(args.lat, args.lng)
     await account.get_vehicles()
@@ -168,7 +166,6 @@ async def fingerprint(args) -> None:
         args.password,
         get_region_from_name(args.region),
         log_responses=True,
-        use_metric_units=(not args.imperial),
     )
     if args.lat and args.lng:
         account.set_observer_position(args.lat, args.lng)
@@ -180,9 +177,7 @@ async def fingerprint(args) -> None:
 
 async def light_flash(args) -> None:
     """Trigger the vehicle to flash its lights."""
-    account = MyBMWAccount(
-        args.username, args.password, get_region_from_name(args.region), use_metric_units=(not args.imperial)
-    )
+    account = MyBMWAccount(args.username, args.password, get_region_from_name(args.region))
     await account.get_vehicles()
     vehicle = get_vehicle_or_return(account, args.vin)
     status = await vehicle.remote_services.trigger_remote_light_flash()
@@ -191,9 +186,7 @@ async def light_flash(args) -> None:
 
 async def horn(args) -> None:
     """Trigger the vehicle to horn."""
-    account = MyBMWAccount(
-        args.username, args.password, get_region_from_name(args.region), use_metric_units=(not args.imperial)
-    )
+    account = MyBMWAccount(args.username, args.password, get_region_from_name(args.region))
     await account.get_vehicles()
     vehicle = get_vehicle_or_return(account, args.vin)
     status = await vehicle.remote_services.trigger_remote_horn()
@@ -202,9 +195,7 @@ async def horn(args) -> None:
 
 async def vehicle_finder(args) -> None:
     """Trigger the vehicle finder to locate it."""
-    account = MyBMWAccount(
-        args.username, args.password, get_region_from_name(args.region), use_metric_units=(not args.imperial)
-    )
+    account = MyBMWAccount(args.username, args.password, get_region_from_name(args.region))
     account.set_observer_position(args.lat, args.lng)
     await account.get_vehicles()
     vehicle = get_vehicle_or_return(account, args.vin)
@@ -217,9 +208,7 @@ async def chargingsettings(args) -> None:
     """Trigger a change to charging settings."""
     if not args.target_soc and not args.ac_limit:
         raise ValueError("At least one of 'charging-target' and 'ac-limit' has to be provided.")
-    account = MyBMWAccount(
-        args.username, args.password, get_region_from_name(args.region), use_metric_units=(not args.imperial)
-    )
+    account = MyBMWAccount(args.username, args.password, get_region_from_name(args.region))
     await account.get_vehicles()
     vehicle = get_vehicle_or_return(account, args.vin)
     status = await vehicle.remote_services.trigger_charging_settings_update(
@@ -232,9 +221,7 @@ async def chargingprofile(args) -> None:
     """Trigger a change to charging profile."""
     if not args.charging_mode and not args.precondition_climate:
         raise ValueError("At least one of 'charging-mode' and 'precondition-climate' has to be provided.")
-    account = MyBMWAccount(
-        args.username, args.password, get_region_from_name(args.region), use_metric_units=(not args.imperial)
-    )
+    account = MyBMWAccount(args.username, args.password, get_region_from_name(args.region))
     await account.get_vehicles()
     vehicle = get_vehicle_or_return(account, args.vin)
     status = await vehicle.remote_services.trigger_charging_profile_update(
@@ -245,9 +232,7 @@ async def chargingprofile(args) -> None:
 
 async def charge(args) -> None:
     """Trigger a vehicle to start or stop charging."""
-    account = MyBMWAccount(
-        args.username, args.password, get_region_from_name(args.region), use_metric_units=(not args.imperial)
-    )
+    account = MyBMWAccount(args.username, args.password, get_region_from_name(args.region))
     await account.get_vehicles()
     vehicle = get_vehicle_or_return(account, args.vin)
     status = await getattr(vehicle.remote_services, f"trigger_charge_{args.action.lower()}")()
@@ -256,9 +241,7 @@ async def charge(args) -> None:
 
 async def image(args) -> None:
     """Download a rendered image of the vehicle."""
-    account = MyBMWAccount(
-        args.username, args.password, get_region_from_name(args.region), use_metric_units=(not args.imperial)
-    )
+    account = MyBMWAccount(args.username, args.password, get_region_from_name(args.region))
     await account.get_vehicles()
     vehicle = get_vehicle_or_return(account, args.vin)
 
@@ -274,9 +257,7 @@ async def image(args) -> None:
 
 async def send_poi(args) -> None:
     """Send Point Of Interest to car."""
-    account = MyBMWAccount(
-        args.username, args.password, get_region_from_name(args.region), use_metric_units=(not args.imperial)
-    )
+    account = MyBMWAccount(args.username, args.password, get_region_from_name(args.region))
     await account.get_vehicles()
     vehicle = get_vehicle_or_return(account, args.vin)
 
@@ -294,9 +275,7 @@ async def send_poi(args) -> None:
 
 async def send_poi_from_address(args) -> None:
     """Create Point of Interest from OSM Nominatim and send to car."""
-    account = MyBMWAccount(
-        args.username, args.password, get_region_from_name(args.region), use_metric_units=(not args.imperial)
-    )
+    account = MyBMWAccount(args.username, args.password, get_region_from_name(args.region))
     await account.get_vehicles()
     vehicle = get_vehicle_or_return(account, args.vin)
 
@@ -333,10 +312,6 @@ def _add_default_arguments(parser: argparse.ArgumentParser):
     parser.add_argument("password", help="Connected Drive password")
     parser.add_argument("region", choices=valid_regions(), help="Region of the Connected Drive account")
 
-    parser.add_argument(
-        "-i", "--imperial", default=False, help="(optional) Use imperial instead of metric units", action="store_true"
-    )
-
 
 def _add_position_arguments(parser: argparse.ArgumentParser):
     """Add the lat and lng attributes to the parser."""

bimmer_connected/tests/conftest.py

@@ -41,8 +41,8 @@ def bmw_log_all_responses(monkeypatch: pytest.MonkeyPatch):
     monkeypatch.setattr("bimmer_connected.account.RESPONSE_STORE", temp_store)
 
 
-async def prepare_account_with_vehicles(region: Optional[Regions] = None, metric: bool = True):
+async def prepare_account_with_vehicles(region: Optional[Regions] = None):
     """Initialize account and get vehicles."""
-    account = MyBMWAccount(TEST_USERNAME, TEST_PASSWORD, region or TEST_REGION, use_metric_units=metric)
+    account = MyBMWAccount(TEST_USERNAME, TEST_PASSWORD, region or TEST_REGION)
     await account.get_vehicles()
     return account

bimmer_connected/tests/test_account.py

@@ -332,23 +332,28 @@ async def test_set_observer_invalid_values(bmw_fixture: respx.Router):
 
 
 @pytest.mark.asyncio
-async def test_set_use_metric_units():
-    """Test set_observer_position with no arguments."""
-    account = MyBMWAccount(TEST_USERNAME, TEST_PASSWORD, TEST_REGION)
-    assert account.config.use_metric_units is True
+async def test_set_use_metric_units(caplog):
+    """Test (deprecated) use_metrics_units flag."""
 
+    # Default
+    account = MyBMWAccount(TEST_USERNAME, TEST_PASSWORD, TEST_REGION)
+    assert len(caplog.records) == 0
     metric_client = MyBMWClient(account.config)
     assert metric_client.generate_default_header()["bmw-units-preferences"] == "d=KM;v=L"
 
-    account.set_use_metric_units(False)
-    assert account.config.use_metric_units is False
-    imperial_client = MyBMWClient(account.config)
-    assert imperial_client.generate_default_header()["bmw-units-preferences"] == "d=MI;v=G"
+    # Set to true
+    caplog.clear()
+    account = MyBMWAccount(TEST_USERNAME, TEST_PASSWORD, TEST_REGION, use_metric_units=True)
+    assert len(caplog.records) == 1
+    metric_client = MyBMWClient(account.config)
+    assert metric_client.generate_default_header()["bmw-units-preferences"] == "d=KM;v=L"
 
-    imperial_account = MyBMWAccount(TEST_USERNAME, TEST_PASSWORD, TEST_REGION, use_metric_units=False)
-    assert imperial_account.config.use_metric_units is False
-    imperial_client = MyBMWClient(imperial_account.config)
-    assert imperial_client.generate_default_header()["bmw-units-preferences"] == "d=MI;v=G"
+    # Set to false
+    caplog.clear()
+    account = MyBMWAccount(TEST_USERNAME, TEST_PASSWORD, TEST_REGION, use_metric_units=True)
+    assert len(caplog.records) == 1
+    metric_client = MyBMWClient(account.config)
+    assert metric_client.generate_default_header()["bmw-units-preferences"] == "d=KM;v=L"
 
 
 @pytest.mark.asyncio

bimmer_connected/tests/test_vehicle_status.py

@@ -105,7 +105,6 @@ async def test_range_combustion_no_info(caplog, bmw_fixture: respx.Router):
 @pytest.mark.asyncio
 async def test_range_combustion(caplog, bmw_fixture: respx.Router):
     """Test if the parsing of mileage and range is working."""
-    # Metric units
     status = (await prepare_account_with_vehicles()).get_vehicle(VIN_G20).fuel_and_battery
 
     assert (40, "L") == status.remaining_fuel
@@ -119,25 +118,10 @@ async def test_range_combustion(caplog, bmw_fixture: respx.Router):
 
     assert len(get_deprecation_warning_count(caplog)) == 0
 
-    # Imperial units
-    status = (await prepare_account_with_vehicles(metric=False)).get_vehicle(VIN_G20).fuel_and_battery
-
-    assert (40, "gal") == status.remaining_fuel
-    assert (629, "mi") == status.remaining_range_fuel
-    assert status.remaining_fuel_percent == 80
-
-    assert status.remaining_battery_percent is None
-    assert status.remaining_range_electric == (None, None)
-
-    assert (629, "mi") == status.remaining_range_total
-
-    assert len(get_deprecation_warning_count(caplog)) == 0
-
 
 @pytest.mark.asyncio
 async def test_range_phev(caplog, bmw_fixture: respx.Router):
     """Test if the parsing of mileage and range is working."""
-    # Metric units
     status = (await prepare_account_with_vehicles()).get_vehicle(VIN_G01).fuel_and_battery
 
     assert (40, "L") == status.remaining_fuel
@@ -153,27 +137,10 @@ async def test_range_phev(caplog, bmw_fixture: respx.Router):
 
     assert len(get_deprecation_warning_count(caplog)) == 0
 
-    # Imperial units
-    status = (await prepare_account_with_vehicles(metric=False)).get_vehicle(VIN_G01).fuel_and_battery
-
-    assert (40, "gal") == status.remaining_fuel
-    assert (436, "mi") == status.remaining_range_fuel
-    assert 80 == status.remaining_fuel_percent
-
-    assert 80 == status.remaining_battery_percent
-    assert (40, "mi") == status.remaining_range_electric
-
-    assert (476, "mi") == status.remaining_range_total
-
-    assert status.remaining_range_fuel[0] + status.remaining_range_electric[0] == status.remaining_range_total[0]
-
-    assert len(get_deprecation_warning_count(caplog)) == 0
-
 
 @pytest.mark.asyncio
 async def test_range_rex(caplog, bmw_fixture: respx.Router):
     """Test if the parsing of mileage and range is working."""
-    # Metric units
     status = (await prepare_account_with_vehicles()).get_vehicle(VIN_I01_REX).fuel_and_battery
 
     assert (6, "L") == status.remaining_fuel
@@ -189,27 +156,10 @@ async def test_range_rex(caplog, bmw_fixture: respx.Router):
 
     assert len(get_deprecation_warning_count(caplog)) == 0
 
-    # Imperial units
-    status = (await prepare_account_with_vehicles(metric=False)).get_vehicle(VIN_I01_REX).fuel_and_battery
-
-    assert (6, "gal") == status.remaining_fuel
-    assert (105, "mi") == status.remaining_range_fuel
-    assert status.remaining_fuel_percent is None
-
-    assert 82 == status.remaining_battery_percent
-    assert (174, "mi") == status.remaining_range_electric
-
-    assert (279, "mi") == status.remaining_range_total
-
-    assert status.remaining_range_fuel[0] + status.remaining_range_electric[0] == status.remaining_range_total[0]
-
-    assert len(get_deprecation_warning_count(caplog)) == 0
-
 
 @pytest.mark.asyncio
 async def test_range_electric(caplog, bmw_fixture: respx.Router):
     """Test if the parsing of mileage and range is working."""
-    # Metric units
     status = (await prepare_account_with_vehicles()).get_vehicle(VIN_I20).fuel_and_battery
 
     assert status.remaining_fuel == (None, None)
@@ -223,20 +173,6 @@ async def test_range_electric(caplog, bmw_fixture: respx.Router):
 
     assert len(get_deprecation_warning_count(caplog)) == 0
 
-    # Imperial units
-    status = (await prepare_account_with_vehicles(metric=False)).get_vehicle(VIN_I20).fuel_and_battery
-
-    assert status.remaining_fuel == (None, None)
-    assert status.remaining_range_fuel == (None, None)
-    assert status.remaining_fuel_percent is None
-
-    assert 70 == status.remaining_battery_percent
-    assert (340, "mi") == status.remaining_range_electric
-
-    assert (340, "mi") == status.remaining_range_total
-
-    assert len(get_deprecation_warning_count(caplog)) == 0
-
 
 @time_machine.travel("2021-11-28 21:28:59 +0000", tick=False)
 @pytest.mark.asyncio

bimmer_connected/vehicle/fuel_and_battery.py

@@ -101,7 +101,7 @@ def _parse_vehicle_data(cls, vehicle_data: Dict) -> Optional[Dict]:
         state = vehicle_data["state"]
 
         if drivetrain in COMBUSTION_ENGINE_DRIVE_TRAINS:
-            retval.update(cls._parse_fuel_data(state.get("combustionFuelLevel", {}), vehicle_data["is_metric"]))
+            retval.update(cls._parse_fuel_data(state.get("combustionFuelLevel", {})))
 
         if drivetrain in HV_BATTERY_DRIVE_TRAINS:
             electric_data = state.get("electricChargingState", {})
@@ -110,7 +110,6 @@ def _parse_vehicle_data(cls, vehicle_data: Dict) -> Optional[Dict]:
                     cls._parse_electric_data(
                         electric_data,
                         vehicle_data["fetched_at"],
-                        vehicle_data["is_metric"],
                         state.get("chargingProfile", {}).get("reductionOfChargeCurrent"),
                     ),
                 )
@@ -142,20 +141,20 @@ def _parse_vehicle_data(cls, vehicle_data: Dict) -> Optional[Dict]:
         return retval
 
     @staticmethod
-    def _parse_fuel_data(fuel_data: Dict, is_metric: bool) -> Dict:
+    def _parse_fuel_data(fuel_data: Dict) -> Dict:
         """Parse fuel data."""
         retval = {}
         if "remainingFuelLiters" in fuel_data:
-            retval["remaining_fuel"] = ValueWithUnit(fuel_data["remainingFuelLiters"], "L" if is_metric else "gal")
+            retval["remaining_fuel"] = ValueWithUnit(fuel_data["remainingFuelLiters"], "L")
         if "remainingFuelPercent" in fuel_data:
             retval["remaining_fuel_percent"] = fuel_data["remainingFuelPercent"]
         if "range" in fuel_data:
-            retval["remaining_range_fuel"] = ValueWithUnit(fuel_data["range"], "km" if is_metric else "mi")
+            retval["remaining_range_fuel"] = ValueWithUnit(fuel_data["range"], "km")
         return retval
 
     @staticmethod
     def _parse_electric_data(
-        electric_data: Dict, fetched_at: datetime.datetime, is_metric: bool, charging_window: Optional[Dict] = None
+        electric_data: Dict, fetched_at: datetime.datetime, charging_window: Optional[Dict] = None
     ) -> Dict:
         """Parse electric data."""
         retval = {}
@@ -164,7 +163,7 @@ def _parse_electric_data(
         if "chargingLevelPercent" in electric_data:
             retval["remaining_battery_percent"] = int(electric_data["chargingLevelPercent"])
         if "range" in electric_data:
-            retval["remaining_range_electric"] = ValueWithUnit(electric_data["range"], "km" if is_metric else "mi")
+            retval["remaining_range_electric"] = ValueWithUnit(electric_data["range"], "km")
         if "chargingStatus" in electric_data:
             retval["charging_status"] = ChargingState(
                 electric_data["chargingStatus"] if electric_data["chargingStatus"] != "INVALID" else "NOT_CHARGING"