Skip to content

Commit

Permalink
CLI: Set observer position only for vehicle finder (#645)
Browse files Browse the repository at this point in the history
* CLI: Set observer position only for vehicle finder

* Enhance GPSPosition error handling

* Fix tests
  • Loading branch information
rikroe authored Aug 26, 2024
1 parent a7e91d4 commit e30f346
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 15 deletions.
2 changes: 1 addition & 1 deletion bimmer_connected/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions bimmer_connected/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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):
Expand Down
20 changes: 15 additions & 5 deletions bimmer_connected/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
12 changes: 6 additions & 6 deletions bimmer_connected/tests/test_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion bimmer_connected/tests/test_vehicle.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"):
Expand Down

0 comments on commit e30f346

Please sign in to comment.