From 18f29993c5efbd84dec7a0951d532635f58ab7b2 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Mon, 25 Sep 2023 17:08:59 -0700 Subject: [PATCH] Simplify fitbit unit system and conversions (#100825) * Simplify fitbit unit conversions * Use enum values in unit system schema * Use fitbit unit system enums --- homeassistant/components/fitbit/api.py | 48 ++++++++++-- homeassistant/components/fitbit/const.py | 69 ++++++----------- homeassistant/components/fitbit/sensor.py | 93 ++++++++++++----------- tests/components/fitbit/conftest.py | 9 +++ tests/components/fitbit/test_sensor.py | 24 +++++- 5 files changed, 143 insertions(+), 100 deletions(-) diff --git a/homeassistant/components/fitbit/api.py b/homeassistant/components/fitbit/api.py index 19f6965a4bb..1b58d26e286 100644 --- a/homeassistant/components/fitbit/api.py +++ b/homeassistant/components/fitbit/api.py @@ -6,7 +6,9 @@ from typing import Any from fitbit import Fitbit from homeassistant.core import HomeAssistant +from homeassistant.util.unit_system import METRIC_SYSTEM +from .const import FitbitUnitSystem from .model import FitbitDevice, FitbitProfile _LOGGER = logging.getLogger(__name__) @@ -19,11 +21,13 @@ class FitbitApi: self, hass: HomeAssistant, client: Fitbit, + unit_system: FitbitUnitSystem | None = None, ) -> None: """Initialize Fitbit auth.""" self._hass = hass self._profile: FitbitProfile | None = None self._client = client + self._unit_system = unit_system @property def client(self) -> Fitbit: @@ -32,14 +36,38 @@ class FitbitApi: def get_user_profile(self) -> FitbitProfile: """Return the user profile from the API.""" - response: dict[str, Any] = self._client.user_profile_get() - _LOGGER.debug("user_profile_get=%s", response) - profile = response["user"] - return FitbitProfile( - encoded_id=profile["encodedId"], - full_name=profile["fullName"], - locale=profile.get("locale"), - ) + if self._profile is None: + response: dict[str, Any] = self._client.user_profile_get() + _LOGGER.debug("user_profile_get=%s", response) + profile = response["user"] + self._profile = FitbitProfile( + encoded_id=profile["encodedId"], + full_name=profile["fullName"], + locale=profile.get("locale"), + ) + return self._profile + + def get_unit_system(self) -> FitbitUnitSystem: + """Get the unit system to use when fetching timeseries. + + This is used in a couple ways. The first is to determine the request + header to use when talking to the fitbit API which changes the + units returned by the API. The second is to tell Home Assistant the + units set in sensor values for the values returned by the API. + """ + if ( + self._unit_system is not None + and self._unit_system != FitbitUnitSystem.LEGACY_DEFAULT + ): + return self._unit_system + # Use units consistent with the account user profile or fallback to the + # home assistant unit settings. + profile = self.get_user_profile() + if profile.locale == FitbitUnitSystem.EN_GB: + return FitbitUnitSystem.EN_GB + if self._hass.config.units is METRIC_SYSTEM: + return FitbitUnitSystem.METRIC + return FitbitUnitSystem.EN_US def get_devices(self) -> list[FitbitDevice]: """Return available devices.""" @@ -58,6 +86,10 @@ class FitbitApi: def get_latest_time_series(self, resource_type: str) -> dict[str, Any]: """Return the most recent value from the time series for the specified resource type.""" + + # Set request header based on the configured unit system + self._client.system = self.get_unit_system() + response: dict[str, Any] = self._client.time_series(resource_type, period="7d") _LOGGER.debug("time_series(%s)=%s", resource_type, response) key = resource_type.replace("/", "-") diff --git a/homeassistant/components/fitbit/const.py b/homeassistant/components/fitbit/const.py index 045b58cfc5e..19734add07a 100644 --- a/homeassistant/components/fitbit/const.py +++ b/homeassistant/components/fitbit/const.py @@ -1,16 +1,10 @@ """Constants for the Fitbit platform.""" from __future__ import annotations +from enum import StrEnum from typing import Final -from homeassistant.const import ( - CONF_CLIENT_ID, - CONF_CLIENT_SECRET, - UnitOfLength, - UnitOfMass, - UnitOfTime, - UnitOfVolume, -) +from homeassistant.const import CONF_CLIENT_ID, CONF_CLIENT_SECRET DOMAIN: Final = "fitbit" @@ -43,46 +37,31 @@ DEFAULT_CONFIG: Final[dict[str, str]] = { } DEFAULT_CLOCK_FORMAT: Final = "24H" - -FITBIT_MEASUREMENTS: Final[dict[str, dict[str, str]]] = { - "en_US": { - ATTR_DURATION: UnitOfTime.MILLISECONDS, - ATTR_DISTANCE: UnitOfLength.MILES, - ATTR_ELEVATION: UnitOfLength.FEET, - ATTR_HEIGHT: UnitOfLength.INCHES, - ATTR_WEIGHT: UnitOfMass.POUNDS, - ATTR_BODY: UnitOfLength.INCHES, - ATTR_LIQUIDS: UnitOfVolume.FLUID_OUNCES, - ATTR_BLOOD_GLUCOSE: f"{UnitOfMass.MILLIGRAMS}/dL", - ATTR_BATTERY: "", - }, - "en_GB": { - ATTR_DURATION: UnitOfTime.MILLISECONDS, - ATTR_DISTANCE: UnitOfLength.KILOMETERS, - ATTR_ELEVATION: UnitOfLength.METERS, - ATTR_HEIGHT: UnitOfLength.CENTIMETERS, - ATTR_WEIGHT: UnitOfMass.STONES, - ATTR_BODY: UnitOfLength.CENTIMETERS, - ATTR_LIQUIDS: UnitOfVolume.MILLILITERS, - ATTR_BLOOD_GLUCOSE: "mmol/L", - ATTR_BATTERY: "", - }, - "metric": { - ATTR_DURATION: UnitOfTime.MILLISECONDS, - ATTR_DISTANCE: UnitOfLength.KILOMETERS, - ATTR_ELEVATION: UnitOfLength.METERS, - ATTR_HEIGHT: UnitOfLength.CENTIMETERS, - ATTR_WEIGHT: UnitOfMass.KILOGRAMS, - ATTR_BODY: UnitOfLength.CENTIMETERS, - ATTR_LIQUIDS: UnitOfVolume.MILLILITERS, - ATTR_BLOOD_GLUCOSE: "mmol/L", - ATTR_BATTERY: "", - }, -} - BATTERY_LEVELS: Final[dict[str, int]] = { "High": 100, "Medium": 50, "Low": 20, "Empty": 0, } + + +class FitbitUnitSystem(StrEnum): + """Fitbit unit system set when sending requests to the Fitbit API. + + This is used as a header to tell the Fitbit API which type of units to return. + https://dev.fitbit.com/build/reference/web-api/developer-guide/application-design/#Units + + Prefer to leave unset for newer configurations to use the Home Assistant default units. + """ + + LEGACY_DEFAULT = "default" + """When set, will use an appropriate default using a legacy algorithm.""" + + METRIC = "metric" + """Use metric units.""" + + EN_US = "en_US" + """Use United States units.""" + + EN_GB = "en_GB" + """Use United Kingdom units.""" diff --git a/homeassistant/components/fitbit/sensor.py b/homeassistant/components/fitbit/sensor.py index 3b1c831b116..653a4ee2508 100644 --- a/homeassistant/components/fitbit/sensor.py +++ b/homeassistant/components/fitbit/sensor.py @@ -29,6 +29,8 @@ from homeassistant.const import ( CONF_CLIENT_SECRET, CONF_UNIT_SYSTEM, PERCENTAGE, + UnitOfLength, + UnitOfMass, UnitOfTime, ) from homeassistant.core import HomeAssistant @@ -39,7 +41,6 @@ from homeassistant.helpers.json import save_json from homeassistant.helpers.network import NoURLAvailableError, get_url from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType from homeassistant.util.json import load_json_object -from homeassistant.util.unit_system import METRIC_SYSTEM from .api import FitbitApi from .const import ( @@ -56,9 +57,9 @@ from .const import ( FITBIT_AUTH_START, FITBIT_CONFIG_FILE, FITBIT_DEFAULT_RESOURCES, - FITBIT_MEASUREMENTS, + FitbitUnitSystem, ) -from .model import FitbitDevice, FitbitProfile +from .model import FitbitDevice _LOGGER: Final = logging.getLogger(__name__) @@ -97,12 +98,36 @@ def _clock_format_12h(result: dict[str, Any]) -> str: return f"{hours}:{minutes:02d} {setting}" +def _weight_unit(unit_system: FitbitUnitSystem) -> UnitOfMass: + """Determine the weight unit.""" + if unit_system == FitbitUnitSystem.EN_US: + return UnitOfMass.POUNDS + if unit_system == FitbitUnitSystem.EN_GB: + return UnitOfMass.STONES + return UnitOfMass.KILOGRAMS + + +def _distance_unit(unit_system: FitbitUnitSystem) -> UnitOfLength: + """Determine the distance unit.""" + if unit_system == FitbitUnitSystem.EN_US: + return UnitOfLength.MILES + return UnitOfLength.KILOMETERS + + +def _elevation_unit(unit_system: FitbitUnitSystem) -> UnitOfLength: + """Determine the elevation unit.""" + if unit_system == FitbitUnitSystem.EN_US: + return UnitOfLength.FEET + return UnitOfLength.METERS + + @dataclass class FitbitSensorEntityDescription(SensorEntityDescription): """Describes Fitbit sensor entity.""" unit_type: str | None = None value_fn: Callable[[dict[str, Any]], Any] = _default_value_fn + unit_fn: Callable[[FitbitUnitSystem], str | None] = lambda x: None FITBIT_RESOURCES_LIST: Final[tuple[FitbitSensorEntityDescription, ...]] = ( @@ -127,17 +152,17 @@ FITBIT_RESOURCES_LIST: Final[tuple[FitbitSensorEntityDescription, ...]] = ( FitbitSensorEntityDescription( key="activities/distance", name="Distance", - unit_type="distance", icon="mdi:map-marker", device_class=SensorDeviceClass.DISTANCE, value_fn=_distance_value_fn, + unit_fn=_distance_unit, ), FitbitSensorEntityDescription( key="activities/elevation", name="Elevation", - unit_type="elevation", icon="mdi:walk", device_class=SensorDeviceClass.DISTANCE, + unit_fn=_elevation_unit, ), FitbitSensorEntityDescription( key="activities/floors", @@ -201,17 +226,17 @@ FITBIT_RESOURCES_LIST: Final[tuple[FitbitSensorEntityDescription, ...]] = ( FitbitSensorEntityDescription( key="activities/tracker/distance", name="Tracker Distance", - unit_type="distance", icon="mdi:map-marker", device_class=SensorDeviceClass.DISTANCE, value_fn=_distance_value_fn, + unit_fn=_distance_unit, ), FitbitSensorEntityDescription( key="activities/tracker/elevation", name="Tracker Elevation", - unit_type="elevation", icon="mdi:walk", device_class=SensorDeviceClass.DISTANCE, + unit_fn=_elevation_unit, ), FitbitSensorEntityDescription( key="activities/tracker/floors", @@ -272,11 +297,11 @@ FITBIT_RESOURCES_LIST: Final[tuple[FitbitSensorEntityDescription, ...]] = ( FitbitSensorEntityDescription( key="body/weight", name="Weight", - unit_type="weight", icon="mdi:human", state_class=SensorStateClass.MEASUREMENT, device_class=SensorDeviceClass.WEIGHT, value_fn=_body_value_fn, + unit_fn=_weight_unit, ), FitbitSensorEntityDescription( key="sleep/awakeningsCount", @@ -360,8 +385,13 @@ PLATFORM_SCHEMA: Final = PARENT_PLATFORM_SCHEMA.extend( vol.Optional(CONF_CLOCK_FORMAT, default=DEFAULT_CLOCK_FORMAT): vol.In( ["12H", "24H"] ), - vol.Optional(CONF_UNIT_SYSTEM, default="default"): vol.In( - ["en_GB", "en_US", "metric", "default"] + vol.Optional(CONF_UNIT_SYSTEM, default=FitbitUnitSystem.LEGACY_DEFAULT): vol.In( + [ + FitbitUnitSystem.EN_GB, + FitbitUnitSystem.EN_US, + FitbitUnitSystem.METRIC, + FitbitUnitSystem.LEGACY_DEFAULT, + ] ), } ) @@ -487,17 +517,9 @@ def setup_platform( if int(time.time()) - cast(int, expires_at) > 3600: authd_client.client.refresh_token() - api = FitbitApi(hass, authd_client) + api = FitbitApi(hass, authd_client, config[CONF_UNIT_SYSTEM]) user_profile = api.get_user_profile() - if (unit_system := config[CONF_UNIT_SYSTEM]) == "default": - authd_client.system = user_profile.locale - if authd_client.system != "en_GB": - if hass.config.units is METRIC_SYSTEM: - authd_client.system = "metric" - else: - authd_client.system = "en_US" - else: - authd_client.system = unit_system + unit_system = api.get_unit_system() clock_format = config[CONF_CLOCK_FORMAT] monitored_resources = config[CONF_MONITORED_RESOURCES] @@ -508,11 +530,10 @@ def setup_platform( entities = [ FitbitSensor( api, - user_profile, + user_profile.encoded_id, config_path, description, - hass.config.units is METRIC_SYSTEM, - clock_format, + units=description.unit_fn(unit_system), ) for description in resource_list if description.key in monitored_resources @@ -523,11 +544,9 @@ def setup_platform( [ FitbitSensor( api, - user_profile, + user_profile.encoded_id, config_path, FITBIT_RESOURCE_BATTERY, - hass.config.units is METRIC_SYSTEM, - clock_format, device, ) for device in devices @@ -646,37 +665,25 @@ class FitbitSensor(SensorEntity): def __init__( self, api: FitbitApi, - user_profile: FitbitProfile, + user_profile_id: str, config_path: str, description: FitbitSensorEntityDescription, - is_metric: bool, - clock_format: str, device: FitbitDevice | None = None, + units: str | None = None, ) -> None: """Initialize the Fitbit sensor.""" self.entity_description = description self.api = api self.config_path = config_path - self.is_metric = is_metric - self.clock_format = clock_format self.device = device - self._attr_unique_id = f"{user_profile.encoded_id}_{description.key}" + self._attr_unique_id = f"{user_profile_id}_{description.key}" if device is not None: self._attr_name = f"{device.device_version} Battery" self._attr_unique_id = f"{self._attr_unique_id}_{device.id}" - if description.unit_type: - try: - measurement_system = FITBIT_MEASUREMENTS[self.api.client.system] - except KeyError: - if self.is_metric: - measurement_system = FITBIT_MEASUREMENTS["metric"] - else: - measurement_system = FITBIT_MEASUREMENTS["en_US"] - split_resource = description.key.rsplit("/", maxsplit=1)[-1] - unit_type = measurement_system[split_resource] - self._attr_native_unit_of_measurement = unit_type + if units is not None: + self._attr_native_unit_of_measurement = units @property def icon(self) -> str | None: diff --git a/tests/components/fitbit/conftest.py b/tests/components/fitbit/conftest.py index 291951a745a..7499a060933 100644 --- a/tests/components/fitbit/conftest.py +++ b/tests/components/fitbit/conftest.py @@ -66,14 +66,23 @@ def mock_monitored_resources() -> list[str] | None: return None +@pytest.fixture(name="configured_unit_system") +def mock_configured_unit_syststem() -> str | None: + """Fixture for the fitbit yaml config monitored_resources field.""" + return None + + @pytest.fixture(name="sensor_platform_config") def mock_sensor_platform_config( monitored_resources: list[str] | None, + configured_unit_system: str | None, ) -> dict[str, Any]: """Fixture for the fitbit sensor platform configuration data in configuration.yaml.""" config = {} if monitored_resources is not None: config["monitored_resources"] = monitored_resources + if configured_unit_system is not None: + config["unit_system"] = configured_unit_system return config diff --git a/tests/components/fitbit/test_sensor.py b/tests/components/fitbit/test_sensor.py index 7351f919380..636afeacf16 100644 --- a/tests/components/fitbit/test_sensor.py +++ b/tests/components/fitbit/test_sensor.py @@ -244,11 +244,27 @@ async def test_device_battery_level( @pytest.mark.parametrize( - ("monitored_resources", "profile_locale", "expected_unit"), + ( + "monitored_resources", + "profile_locale", + "configured_unit_system", + "expected_unit", + ), [ - (["body/weight"], "en_US", "kg"), - (["body/weight"], "en_GB", "st"), - (["body/weight"], "es_ES", "kg"), + # Defaults to home assistant unit system unless UK + (["body/weight"], "en_US", "default", "kg"), + (["body/weight"], "en_GB", "default", "st"), + (["body/weight"], "es_ES", "default", "kg"), + # Use the configured unit system from yaml + (["body/weight"], "en_US", "en_US", "lb"), + (["body/weight"], "en_GB", "en_US", "lb"), + (["body/weight"], "es_ES", "en_US", "lb"), + (["body/weight"], "en_US", "en_GB", "st"), + (["body/weight"], "en_GB", "en_GB", "st"), + (["body/weight"], "es_ES", "en_GB", "st"), + (["body/weight"], "en_US", "metric", "kg"), + (["body/weight"], "en_GB", "metric", "kg"), + (["body/weight"], "es_ES", "metric", "kg"), ], ) async def test_profile_local(