Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove use_metric_units parameter #584

Merged
merged 2 commits into from
Nov 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions bimmer_connected/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -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. All returned values are metric units (km, l)."""

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:
Expand Down Expand Up @@ -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."""
Expand Down
3 changes: 1 addition & 2 deletions bimmer_connected/api/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down Expand Up @@ -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",
}
45 changes: 10 additions & 35 deletions bimmer_connected/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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)
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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()}")()
Expand All @@ -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)

Expand All @@ -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)

Expand All @@ -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)

Expand Down Expand Up @@ -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."""
Expand Down
4 changes: 2 additions & 2 deletions bimmer_connected/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
29 changes: 17 additions & 12 deletions bimmer_connected/tests/test_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
64 changes: 0 additions & 64 deletions bimmer_connected/tests/test_vehicle_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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
Expand Down
13 changes: 6 additions & 7 deletions bimmer_connected/vehicle/fuel_and_battery.py
Original file line number Diff line number Diff line change
Expand Up @@ -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", {})
Expand All @@ -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"),
),
)
Expand Down Expand Up @@ -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 = {}
Expand All @@ -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"
Expand Down
Loading