Repository: bimmerconnected/bimmer_connected (python)

Issue:
CLI requires lat/lon in params for charging
@rikroe I ran into a problem with 16.1 CLI version, when trying to start/stop charging:

`bimmerconnected charge USERNAME PASSWORD rest_of_world VIN start`

`Traceback (most recent call last):
  File "C:\Users\jeter\AppData\Local\Programs\Python\Python310\lib\runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Users\jeter\AppData\Local\Programs\Python\Python310\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "C:\Users\jeter\AppData\Local\Programs\Python\Python310\Scripts\bimmerconnected.exe\__main__.py", line 7, in <module>
  File "C:\Users\jeter\AppData\Local\Programs\Python\Python310\lib\site-packages\bimmer_connected\cli.py", line 334, in main
    if args.lat and args.lng:
AttributeError: 'Namespace' object has no attribute 'lat'`

I guess you need something like: 
`if args.lat.exists()` in line 334 in cli.py?

_Originally posted by @jeterke in https://github.com/bimmerconnected/bimmer_connected/discussions/627#discussioncomment-10214331_

Issue after refactoring CLI. Testing/parsing `lat` and `lon` only needs to be done for specific parameters.

Pull request description:
CLI: Set observer position only for vehicle finder
## Proposed change
<!--
  Describe the big picture of your changes here to communicate to the
  maintainers why we should accept this pull request. If it fixes a bug
  or resolves a feature request, be sure to link to that issue in the
  additional information section.
-->
Set observer position from positional arguments only for vehicle finder.

## Type of change
- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (which adds functionality to this library)
- [ ] Breaking change (fix/feature causing existing functionality to break)
- [ ] Code quality improvements to existing code or addition of tests

## Additional information
<!--
  Details are important, and help maintainers processing your PR.
  Please be sure to fill out additional details, if applicable.
-->

- This PR fixes or closes issue: fixes #644 
- This PR is related to issue: 

## Checklist
<!--
  Put an `x` in the boxes that apply. You can also fill these out after
  creating the PR. If you're unsure about any of them, don't hesitate to ask.
  We're here to help! This is simply a reminder of what we are going to look
  for before merging your code.
-->

- [ ] The code change is tested and works locally.
- [ ] Tests have been added to verify that the new code works.


Patch:
diff --git a/bimmer_connected/account.py b/bimmer_connected/account.py
index 490c0e8..15b4881 100644
--- a/bimmer_connected/account.py
+++ b/bimmer_connected/account.py
@@ -154,7 +154,7 @@ class MyBMWAccount:
 
     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 3b7b94b..a23d3b8 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 f9506b2..b0b78b6 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()


Test patch:
diff --git a/bimmer_connected/tests/test_account.py b/bimmer_connected/tests/test_account.py
index 347c97e..5d27fa2 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 9897a94..d310aa3 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"):