From e30f346645a09ee63b3f43c3f161282c028e260b Mon Sep 17 00:00:00 2001 From: Richard Kroegel <42204099+rikroe@users.noreply.github.com> Date: Mon, 26 Aug 2024 19:47:29 +0200 Subject: [PATCH] CLI: Set observer position only for vehicle finder (#645) * CLI: Set observer position only for vehicle finder * Enhance GPSPosition error handling * Fix tests --- bimmer_connected/account.py | 2 +- bimmer_connected/cli.py | 3 +-- bimmer_connected/models.py | 20 +++++++++++++++----- bimmer_connected/tests/test_account.py | 12 ++++++------ bimmer_connected/tests/test_vehicle.py | 8 +++++++- 5 files changed, 30 insertions(+), 15 deletions(-) diff --git a/bimmer_connected/account.py b/bimmer_connected/account.py index 490c0e83..15b48815 100644 --- a/bimmer_connected/account.py +++ b/bimmer_connected/account.py @@ -154,7 +154,7 @@ def get_vehicle(self, vin: str) -> Optional[MyBMWVehicle]: def set_observer_position(self, latitude: float, longitude: float) -> None: """Set the position of the observer for all vehicles.""" - self.config.observer_position = GPSPosition(latitude=latitude, longitude=longitude) + self.config.observer_position = GPSPosition.init_nonempty(latitude=latitude, longitude=longitude) def set_refresh_token( self, refresh_token: str, gcid: Optional[str] = None, access_token: Optional[str] = None diff --git a/bimmer_connected/cli.py b/bimmer_connected/cli.py index 3b7b94bf..a23d3b81 100644 --- a/bimmer_connected/cli.py +++ b/bimmer_connected/cli.py @@ -204,6 +204,7 @@ async def horn(account: MyBMWAccount, args) -> None: async def vehicle_finder(account: MyBMWAccount, args) -> None: """Trigger the vehicle finder to locate it.""" + account.set_observer_position(args.lat, args.lng) await account.get_vehicles() vehicle = get_vehicle_or_return(account, args.vin) status = await vehicle.remote_services.trigger_remote_vehicle_finder() @@ -331,8 +332,6 @@ def main(): logging.getLogger("asyncio").setLevel(logging.WARNING) 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) if args.oauth_store.exists(): with contextlib.suppress(json.JSONDecodeError): diff --git a/bimmer_connected/models.py b/bimmer_connected/models.py index f9506b2e..b0b78b68 100644 --- a/bimmer_connected/models.py +++ b/bimmer_connected/models.py @@ -64,17 +64,27 @@ class GPSPosition: longitude: Optional[float] def __post_init__(self): - if len([v for v in self.__dict__.values() if v is None]) not in [0, len(self.__dataclass_fields__)]: - raise TypeError("Either none or all arguments must be 'None'.") + non_null_values = [k for k, v in self.__dict__.items() if v is None] + if len(non_null_values) not in [0, len(self.__dataclass_fields__)]: + raise TypeError(f"{type(self).__name__} requires either none or both arguments set") for field_name in self.__dataclass_fields__: value = getattr(self, field_name) if value is not None and not isinstance(value, (float, int)): - raise TypeError(f"'{field_name}' not of type '{Optional[Union[float, int]]}'") + raise TypeError(f"{type(self).__name__} '{field_name}' not of type '{Optional[Union[float, int]]}'") if field_name == "latitude" and not (-90 <= value <= 90): - raise ValueError(f"'latitude' must be between -90 and 90, but got '{value}'") + raise ValueError(f"{type(self).__name__} 'latitude' must be between -90 and 90, but got '{value}'") elif field_name == "longitude" and not (-180 <= value <= 180): - raise ValueError(f"'longitude' must be between -180 and 180, but got '{value}'") + raise ValueError(f"{type(self).__name__} 'longitude' must be between -180 and 180, but got '{value}'") + # Force conversion to float + setattr(self, field_name, float(value)) + + @classmethod + def init_nonempty(cls, latitude: Optional[float], longitude: Optional[float]): + """Initialize GPSPosition but do not allow empty latitude/longitude.""" + if latitude is None or longitude is None: + raise ValueError(f"{cls.__name__} requires both 'latitude' and 'longitude' set") + return cls(latitude, longitude) def __iter__(self): yield from self.__dict__.values() diff --git a/bimmer_connected/tests/test_account.py b/bimmer_connected/tests/test_account.py index 347c97e5..5d27fa23 100644 --- a/bimmer_connected/tests/test_account.py +++ b/bimmer_connected/tests/test_account.py @@ -326,14 +326,14 @@ async def test_set_observer_invalid_values(bmw_fixture: respx.Router): """Test set_observer_position with invalid arguments.""" account = await prepare_account_with_vehicles() - with pytest.raises(TypeError): - account.set_observer_position(None, 2.0) + with pytest.raises(ValueError, match="requires both 'latitude' and 'longitude' set"): + account.set_observer_position(1, None) - with pytest.raises(TypeError): - account.set_observer_position(1.0, None) + with pytest.raises(ValueError, match="requires both 'latitude' and 'longitude' set"): + account.set_observer_position(None, None) - with pytest.raises(TypeError): - account.set_observer_position(1.0, "16.0") + account.set_observer_position(1, 2) + assert account.config.observer_position == GPSPosition(1.0, 2.0) @pytest.mark.asyncio diff --git a/bimmer_connected/tests/test_vehicle.py b/bimmer_connected/tests/test_vehicle.py index 9897a94c..d310aa3c 100644 --- a/bimmer_connected/tests/test_vehicle.py +++ b/bimmer_connected/tests/test_vehicle.py @@ -284,7 +284,13 @@ def test_gpsposition(): assert pos != "(1, 2)" assert pos[0] == 1 - with pytest.raises(TypeError, match="Either none or all arguments must be 'None'."): + with pytest.raises(TypeError, match="GPSPosition requires either none or both arguments set"): + GPSPosition(1, None) + + with pytest.raises(TypeError, match="GPSPosition requires either none or both arguments set"): + GPSPosition(None, 2) + + with pytest.raises(TypeError, match="GPSPosition requires either none or both arguments set"): GPSPosition(1, None) with pytest.raises(TypeError, match="'longitude' not of type"):