From ba3f63a1aa96e3fda8f862c77a6c06803eaf10dd Mon Sep 17 00:00:00 2001 From: rikroe Date: Fri, 24 Nov 2023 23:40:12 +0100 Subject: [PATCH 1/2] Remove use_metric_units parameter --- bimmer_connected/account.py | 17 +++-- bimmer_connected/api/client.py | 3 +- bimmer_connected/cli.py | 45 +++---------- bimmer_connected/tests/conftest.py | 4 +- bimmer_connected/tests/test_account.py | 29 +++++---- bimmer_connected/tests/test_vehicle_status.py | 64 ------------------- bimmer_connected/vehicle/fuel_and_battery.py | 13 ++-- bimmer_connected/vehicle/reports.py | 7 +- bimmer_connected/vehicle/vehicle.py | 3 +- 9 files changed, 49 insertions(+), 136 deletions(-) diff --git a/bimmer_connected/account.py b/bimmer_connected/account.py index f9486254..da26af86 100644 --- a/bimmer_connected/account.py +++ b/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. Use metric units (km, l) by default. Use imperial units (mi, gal) if False.""" 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.""" diff --git a/bimmer_connected/api/client.py b/bimmer_connected/api/client.py index 30d2d211..5c1fa24c 100644 --- a/bimmer_connected/api/client.py +++ b/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", } diff --git a/bimmer_connected/cli.py b/bimmer_connected/cli.py index ca324750..810e4fd3 100644 --- a/bimmer_connected/cli.py +++ b/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.""" diff --git a/bimmer_connected/tests/conftest.py b/bimmer_connected/tests/conftest.py index 49c5329f..035ad900 100644 --- a/bimmer_connected/tests/conftest.py +++ b/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 diff --git a/bimmer_connected/tests/test_account.py b/bimmer_connected/tests/test_account.py index 7bcfa0ac..d4a39c02 100644 --- a/bimmer_connected/tests/test_account.py +++ b/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 diff --git a/bimmer_connected/tests/test_vehicle_status.py b/bimmer_connected/tests/test_vehicle_status.py index 65406a3d..3f9ebbc3 100644 --- a/bimmer_connected/tests/test_vehicle_status.py +++ b/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 diff --git a/bimmer_connected/vehicle/fuel_and_battery.py b/bimmer_connected/vehicle/fuel_and_battery.py index 5532fce3..7ecbcede 100644 --- a/bimmer_connected/vehicle/fuel_and_battery.py +++ b/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" diff --git a/bimmer_connected/vehicle/reports.py b/bimmer_connected/vehicle/reports.py index 0d6a174b..2eed86fa 100644 --- a/bimmer_connected/vehicle/reports.py +++ b/bimmer_connected/vehicle/reports.py @@ -37,11 +37,10 @@ def from_api_entry( status: str, dateTime: Optional[str] = None, mileage: Optional[int] = None, - is_metric: bool = True, **kwargs, ): """Parse a condition based service entry from the API format to `ConditionBasedService`.""" - due_distance = ValueWithUnit(mileage, "km" if is_metric else "mi") if mileage else ValueWithUnit(None, None) + due_distance = ValueWithUnit(mileage, "km") if mileage else ValueWithUnit(None, None) due_date = parse_datetime(dateTime) if dateTime else None return cls(type, ConditionBasedServiceStatus(status), due_date, due_distance) @@ -63,9 +62,7 @@ def _parse_vehicle_data(cls, vehicle_data: Dict) -> Optional[Dict]: if ATTR_STATE in vehicle_data and "requiredServices" in vehicle_data[ATTR_STATE]: messages = vehicle_data[ATTR_STATE]["requiredServices"] - retval["messages"] = [ - ConditionBasedService.from_api_entry(**m, is_metric=vehicle_data["is_metric"]) for m in messages - ] + retval["messages"] = [ConditionBasedService.from_api_entry(**m) for m in messages] retval["is_service_required"] = any((m.state != ConditionBasedServiceStatus.OK) for m in retval["messages"]) return retval diff --git a/bimmer_connected/vehicle/vehicle.py b/bimmer_connected/vehicle/vehicle.py index e6b6ac78..6a783f25 100644 --- a/bimmer_connected/vehicle/vehicle.py +++ b/bimmer_connected/vehicle/vehicle.py @@ -137,7 +137,6 @@ def combine_data( **vehicle_base, **(vehicle_state or {}), ATTR_CHARGING_SETTINGS: charging_settings or {}, - "is_metric": account.config.use_metric_units, "fetched_at": fetched_at or datetime.datetime.now(datetime.timezone.utc), } @@ -168,7 +167,7 @@ def drive_train(self) -> DriveTrainType: @property def mileage(self) -> ValueWithUnit: """Get the mileage of the vehicle.""" - return ValueWithUnit(self.data[ATTR_STATE].get("currentMileage", 0), "km" if self.data["is_metric"] else "mi") + return ValueWithUnit(self.data[ATTR_STATE].get("currentMileage", 0), "km") @property def timestamp(self) -> Optional[datetime.datetime]: From b3483459622ce105b908ebb14fa64417e8fe19b0 Mon Sep 17 00:00:00 2001 From: rikroe Date: Fri, 24 Nov 2023 23:44:31 +0100 Subject: [PATCH 2/2] Fix description --- bimmer_connected/account.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bimmer_connected/account.py b/bimmer_connected/account.py index da26af86..0f3d2da6 100644 --- a/bimmer_connected/account.py +++ b/bimmer_connected/account.py @@ -49,7 +49,7 @@ class MyBMWAccount: """Optional. Required for getting a position on older cars.""" use_metric_units: InitVar[Optional[bool]] = None - """Deprecated. Use metric units (km, l) by default. Use imperial units (mi, gal) if False.""" + """Deprecated. All returned values are metric units (km, l).""" vehicles: List[MyBMWVehicle] = field(default_factory=list, init=False)