Refactor vehicle init (#592)

* Update tests

* Refactor getting vehicle state

* Fix typing for py3.8

* Fix rebase issue

* Update to latest black formatting

bimmer_connected/__init__.py

@@ -8,7 +8,6 @@
 This library is not affiliated with or endorsed by BMW Group.
 """
 
-
 from importlib.metadata import version
 
 __version__ = version("bimmer_connected")

bimmer_connected/account.py

@@ -11,10 +11,7 @@
 from bimmer_connected.api.regions import Regions
 from bimmer_connected.const import (
     ATTR_ATTRIBUTES,
-    ATTR_CAPABILITIES,
-    VEHICLE_CHARGING_DETAILS_URL,
     VEHICLE_PROFILE_URL,
-    VEHICLE_STATE_URL,
     VEHICLES_URL,
     CarBrands,
 )
@@ -94,77 +91,38 @@ async def _init_vehicles(self) -> None:
                         **{"vin": vehicle_profile["vin"]}
                     )
 
-                    self.add_vehicle(vehicle_base, None, None, fetched_at)
+                    await self.add_vehicle(vehicle_base, fetched_at)
 
     async def get_vehicles(self, force_init: bool = False) -> None:
         """Retrieve vehicle data from BMW servers."""
         _LOGGER.debug("Getting vehicle list")
 
-        fetched_at = datetime.datetime.now(datetime.timezone.utc)
-
         if len(self.vehicles) == 0 or force_init:
             await self._init_vehicles()
 
-        async with MyBMWClient(self.config) as client:
-            error_count = 0
-            for vehicle in self.vehicles:
-                # Get the detailed vehicle state
-                try:
-                    state_response = await client.get(
-                        VEHICLE_STATE_URL,
-                        params={
-                            "apptimezone": self.utcdiff,
-                            "appDateTime": int(fetched_at.timestamp() * 1000),
-                        },
-                        headers={
-                            **client.generate_default_header(vehicle.brand),
-                            "bmw-vin": vehicle.vin,
-                        },
-                    )
-                    vehicle_state = state_response.json()
-
-                    # Get detailed charging settings if supported by vehicle
-                    charging_settings = None
-                    if vehicle_state[ATTR_CAPABILITIES].get("isChargingPlanSupported", False) or vehicle_state[
-                        ATTR_CAPABILITIES
-                    ].get("isChargingSettingsEnabled", False):
-                        charging_settings_response = await client.get(
-                            VEHICLE_CHARGING_DETAILS_URL,
-                            params={
-                                "fields": "charging-profile",
-                                "has_charging_settings_capabilities": vehicle_state[ATTR_CAPABILITIES][
-                                    "isChargingSettingsEnabled"
-                                ],
-                            },
-                            headers={
-                                **client.generate_default_header(vehicle.brand),
-                                "bmw-current-date": fetched_at.isoformat(),
-                                "bmw-vin": vehicle.vin,
-                            },
-                        )
-                        charging_settings = charging_settings_response.json()
-
-                    self.add_vehicle(vehicle.data, vehicle_state, charging_settings, fetched_at)
-                except (MyBMWAPIError, json.JSONDecodeError) as ex:
-                    # We don't want to fail completely if one vehicle fails, but we want to know about it
-                    error_count += 1
-
-                    # If it's a MyBMWQuotaError or MyBMWAuthError, we want to raise it
-                    if isinstance(ex, (MyBMWQuotaError, MyBMWAuthError)):
-                        raise ex
-
-                    # Always log the error
-                    _LOGGER.error("Unable to get details for vehicle %s - (%s) %s", vehicle.vin, type(ex).__name__, ex)
-
-                    # If all vehicles fail, we want to raise an exception
-                    if error_count == len(self.vehicles):
-                        raise ex
-
-    def add_vehicle(
+        error_count = 0
+        for vehicle in self.vehicles:
+            # Get the detailed vehicle state
+            try:
+                await vehicle.get_vehicle_state()
+            except (MyBMWAPIError, json.JSONDecodeError) as ex:
+                # We don't want to fail completely if one vehicle fails, but we want to know about it
+                error_count += 1
+
+                # If it's a MyBMWQuotaError or MyBMWAuthError, we want to raise it
+                if isinstance(ex, (MyBMWQuotaError, MyBMWAuthError)):
+                    raise ex
+
+                # Always log the error
+                _LOGGER.error("Unable to get details for vehicle %s - (%s) %s", vehicle.vin, type(ex).__name__, ex)
+
+                # If all vehicles fail, we want to raise an exception
+                if error_count == len(self.vehicles):
+                    raise ex
+
+    async def add_vehicle(
         self,
         vehicle_base: dict,
-        vehicle_state: Optional[dict],
-        charging_settings: Optional[dict],
         fetched_at: Optional[datetime.datetime] = None,
     ) -> None:
         """Add or update a vehicle from the API responses."""
@@ -173,9 +131,9 @@ def add_vehicle(
 
         # If vehicle already exists, just update it's state
         if existing_vehicle:
-            existing_vehicle.update_state(vehicle_base, vehicle_state, charging_settings, fetched_at)
+            await existing_vehicle.get_vehicle_state()
         else:
-            self.vehicles.append(MyBMWVehicle(self, vehicle_base, vehicle_state, charging_settings, fetched_at))
+            self.vehicles.append(MyBMWVehicle(self, vehicle_base, fetched_at))
 
     def get_vehicle(self, vin: str) -> Optional[MyBMWVehicle]:
         """Get vehicle with given VIN.

bimmer_connected/api/regions.py

@@ -1,4 +1,5 @@
 """Get the right url for the different countries."""
+
 from base64 import b64decode
 from typing import List
 

bimmer_connected/const.py

@@ -1,4 +1,5 @@
 """URLs for different services and error code mapping."""
+
 from enum import Enum
 
 

bimmer_connected/tests/conftest.py

@@ -1,4 +1,5 @@
 """Fixtures for BMW tests."""
+
 from collections import deque
 from typing import Deque, Generator, Optional
 

bimmer_connected/tests/test_api.py

@@ -1,4 +1,5 @@
 """Tests for API that are not covered by other tests."""
+
 import json
 
 import httpx

bimmer_connected/tests/test_remote_services.py

@@ -1,4 +1,5 @@
 """Test for remote_services."""
+
 from unittest import mock
 from uuid import uuid4
 
@@ -247,7 +248,7 @@ async def test_vehicles_without_enabled_services(bmw_fixture: respx.Router):
     # Errors on combustion engines
     vehicle = account.get_vehicle(VIN_F31)
 
-    vehicle.update_state(vehicle.data, {"capabilities": {}})
+    vehicle.update_state({"capabilities": {}})
 
     for service in ALL_SERVICES.values():
         with pytest.raises(ValueError):
@@ -268,7 +269,7 @@ async def test_trigger_charge_start_stop_warnings(caplog, bmw_fixture: respx.Rou
         "chargingStatus": "INVALID",
         "isChargerConnected": False,
     }
-    vehicle.update_state(vehicle.data, {"state": {"electricChargingState": fixture_not_connected}})
+    vehicle.update_state({"state": {"electricChargingState": fixture_not_connected}})
 
     result = await vehicle.remote_services.trigger_charge_start()
     assert result.state == ExecutionState.IGNORED
@@ -285,7 +286,7 @@ async def test_trigger_charge_start_stop_warnings(caplog, bmw_fixture: respx.Rou
         "chargingStatus": "WAITING_FOR_CHARGING",
         "isChargerConnected": True,
     }
-    vehicle.update_state(vehicle.data, {"state": {"electricChargingState": fixture_connected_not_charging}})
+    vehicle.update_state({"state": {"electricChargingState": fixture_connected_not_charging}})
 
     result = await vehicle.remote_services.trigger_charge_stop()
     assert result.state == ExecutionState.IGNORED

bimmer_connected/tests/test_utils.py

@@ -1,4 +1,5 @@
 """Tests for utils."""
+
 import datetime
 import json
 
@@ -31,6 +32,7 @@ async def test_drive_train(bmw_fixture: respx.Router):
         "has_combustion_drivetrain",
         "has_electric_drivetrain",
         "is_charging_plan_supported",
+        "is_charging_settings_supported",
         "is_lsc_enabled",
         "is_remote_charge_start_enabled",
         "is_remote_charge_stop_enabled",

bimmer_connected/tests/test_vehicle.py

@@ -1,4 +1,5 @@
 """Tests for MyBMWVehicle."""
+
 import pytest
 import respx
 

bimmer_connected/utils.py

@@ -1,6 +1,5 @@
 """General utils and base classes used in the library."""
 
-
 import datetime
 import inspect
 import json

bimmer_connected/vehicle/fuel_and_battery.py

@@ -1,6 +1,5 @@
 """Generals models used for bimmer_connected."""
 
-
 import datetime
 import logging
 from dataclasses import dataclass

bimmer_connected/vehicle/vehicle.py

@@ -1,15 +1,18 @@
 """Models state and remote services of one vehicle."""
+
 import datetime
 import logging
-from typing import TYPE_CHECKING, Dict, List, Optional, Tuple, Type
+from typing import TYPE_CHECKING, Dict, List, Optional, Tuple, Type, Union
 
 from bimmer_connected.api.client import MyBMWClient
 from bimmer_connected.const import (
     ATTR_ATTRIBUTES,
     ATTR_CAPABILITIES,
     ATTR_CHARGING_SETTINGS,
     ATTR_STATE,
+    VEHICLE_CHARGING_DETAILS_URL,
     VEHICLE_IMAGE_URL,
+    VEHICLE_STATE_URL,
     CarBrands,
 )
 from bimmer_connected.models import StrEnum, ValueWithUnit
@@ -72,13 +75,11 @@ def __init__(
         self,
         account: "MyBMWAccount",
         vehicle_base: dict,
-        vehicle_state: Optional[dict] = None,
-        charging_settings: Optional[dict] = None,
         fetched_at: Optional[datetime.datetime] = None,
     ) -> None:
         """Initialize a MyBMWVehicle."""
         self.account = account
-        self.data = self.combine_data(account, vehicle_base, vehicle_state, charging_settings, fetched_at)
+        self.data = {}
         self.remote_services = RemoteServices(self)
         self.fuel_and_battery: FuelAndBattery = FuelAndBattery(account_timezone=account.timezone)
         self.vehicle_location: VehicleLocation = VehicleLocation(account_region=account.region)
@@ -90,17 +91,59 @@ def __init__(
         self.charging_profile: Optional[ChargingProfile] = None
         self.tires: Optional[Tires] = None
 
-        self.update_state(vehicle_base, vehicle_state, charging_settings, fetched_at)
+        self.data = self.combine_data(vehicle_base, fetched_at=fetched_at)
+
+    async def get_vehicle_state(self) -> None:
+        """Retrieve vehicle data from BMW servers."""
+        _LOGGER.debug("Getting vehicle list")
+
+        fetched_at = datetime.datetime.now(datetime.timezone.utc)
+
+        async with MyBMWClient(self.account.config) as client:
+            # Get state details
+            state_response = await client.get(
+                VEHICLE_STATE_URL,
+                params={
+                    "apptimezone": self.account.utcdiff,
+                    "appDateTime": int(fetched_at.timestamp() * 1000),
+                },
+                headers={
+                    **client.generate_default_header(self.brand),
+                    "bmw-vin": self.vin,
+                },
+            )
+            vehicle_state: Dict = state_response.json()
+
+            # If vehicle has not been initialized with capabilities from state, do it once
+            if not self.data.get(ATTR_CAPABILITIES):
+                self.data = self.combine_data(vehicle_state, fetched_at=fetched_at)
+
+            # Get detailed charging settings if supported by vehicle
+            charging_settings = {ATTR_CHARGING_SETTINGS: None}
+            if self.is_charging_plan_supported or self.is_charging_settings_supported:
+                charging_settings_response = await client.get(
+                    VEHICLE_CHARGING_DETAILS_URL,
+                    params={
+                        "fields": "charging-profile",
+                        "has_charging_settings_capabilities": self.is_charging_settings_supported,
+                    },
+                    headers={
+                        **client.generate_default_header(self.brand),
+                        "bmw-current-date": fetched_at.isoformat(),
+                        "bmw-vin": self.vin,
+                    },
+                )
+                charging_settings = {ATTR_CHARGING_SETTINGS: charging_settings_response.json()}
+
+            self.update_state([vehicle_state, charging_settings], fetched_at)
 
     def update_state(
         self,
-        vehicle_base: dict,
-        vehicle_state: Optional[dict] = None,
-        charging_settings: Optional[dict] = None,
+        data: Union[Dict, List[Dict]],
         fetched_at: Optional[datetime.datetime] = None,
     ) -> None:
         """Update the state of a vehicle."""
-        vehicle_data = self.combine_data(self.account, vehicle_base, vehicle_state, charging_settings, fetched_at)
+        vehicle_data = self.combine_data(data, fetched_at)
         self.data = vehicle_data
 
         update_entities: List[Tuple[Type["VehicleDataBase"], str]] = [
@@ -124,22 +167,26 @@ def update_state(
             except (KeyError, TypeError) as ex:
                 _LOGGER.warning("Unable to update %s - (%s) %s", vehicle_attribute, type(ex).__name__, ex)
 
-    @staticmethod
-    def combine_data(
-        account: "MyBMWAccount",
-        vehicle_base: dict,
-        vehicle_state: Optional[dict],
-        charging_settings: Optional[dict],
-        fetched_at: Optional[datetime.datetime] = None,
-    ) -> Dict:
+    def combine_data(self, data: Union[Dict, List[Dict]], fetched_at: Optional[datetime.datetime] = None) -> Dict:
         """Combine API responses and additional information to a single dictionary."""
-        return {
-            **vehicle_base,
-            **(vehicle_state or {}),
-            ATTR_CHARGING_SETTINGS: charging_settings or {},
-            "fetched_at": fetched_at or datetime.datetime.now(datetime.timezone.utc),
+        if isinstance(data, dict):
+            data = [data]
+
+        vehicle_data = {
+            ATTR_ATTRIBUTES: {},
+            ATTR_CAPABILITIES: {},
+            ATTR_STATE: {},
+            ATTR_CHARGING_SETTINGS: {},
+            **self.data,
         }
 
+        for entry in data:
+            vehicle_data.update(entry)
+
+        vehicle_data["fetched_at"] = fetched_at or datetime.datetime.now(datetime.timezone.utc)
+
+        return vehicle_data
+
     # # # # # # # # # # # # # # #
     # Generic attributes
     # # # # # # # # # # # # # # #
@@ -209,6 +256,11 @@ def is_charging_plan_supported(self) -> bool:
         """Return True if charging profile is available and can be set via API."""
         return self.data[ATTR_CAPABILITIES].get("isChargingPlanSupported", False)
 
+    @property
+    def is_charging_settings_supported(self) -> bool:
+        """Return True if charging settings can be set via API."""
+        return self.data[ATTR_CAPABILITIES].get("isChargingSettingsEnabled", False)
+
     @property
     def is_vehicle_tracking_enabled(self) -> bool:
         """Return True if vehicle finder is enabled in vehicle."""