diff --git a/homeassistant/components/fitbit/api.py b/homeassistant/components/fitbit/api.py index e5ae88c5420..7a273e3ba18 100644 --- a/homeassistant/components/fitbit/api.py +++ b/homeassistant/components/fitbit/api.py @@ -1,22 +1,30 @@ """API for fitbit bound to Home Assistant OAuth.""" from abc import ABC, abstractmethod -from collections.abc import Callable +from collections.abc import Awaitable, Callable import logging from typing import Any, cast from fitbit import Fitbit from fitbit.exceptions import HTTPException, HTTPUnauthorized +from fitbit_web_api import ApiClient, Configuration, DevicesApi +from fitbit_web_api.exceptions import ( + ApiException, + OpenApiException, + UnauthorizedException, +) +from fitbit_web_api.models.device import Device from requests.exceptions import ConnectionError as RequestsConnectionError from homeassistant.const import CONF_ACCESS_TOKEN from homeassistant.core import HomeAssistant from homeassistant.helpers import config_entry_oauth2_flow +from homeassistant.helpers.aiohttp_client import async_get_clientsession from homeassistant.util.unit_system import METRIC_SYSTEM from .const import FitbitUnitSystem from .exceptions import FitbitApiException, FitbitAuthException -from .model import FitbitDevice, FitbitProfile +from .model import FitbitProfile _LOGGER = logging.getLogger(__name__) @@ -58,6 +66,14 @@ class FitbitApi(ABC): expires_at=float(token[CONF_EXPIRES_AT]), ) + async def _async_get_fitbit_web_api(self) -> ApiClient: + """Create and return an ApiClient configured with the current access token.""" + token = await self.async_get_access_token() + configuration = Configuration() + configuration.pool_manager = async_get_clientsession(self._hass) + configuration.access_token = token[CONF_ACCESS_TOKEN] + return ApiClient(configuration) + async def async_get_user_profile(self) -> FitbitProfile: """Return the user profile from the API.""" if self._profile is None: @@ -94,21 +110,13 @@ class FitbitApi(ABC): return FitbitUnitSystem.METRIC return FitbitUnitSystem.EN_US - async def async_get_devices(self) -> list[FitbitDevice]: - """Return available devices.""" - client = await self._async_get_client() - devices: list[dict[str, str]] = await self._run(client.get_devices) + async def async_get_devices(self) -> list[Device]: + """Return available devices using fitbit-web-api.""" + client = await self._async_get_fitbit_web_api() + devices_api = DevicesApi(client) + devices: list[Device] = await self._run_async(devices_api.get_devices) _LOGGER.debug("get_devices=%s", devices) - return [ - FitbitDevice( - id=device["id"], - device_version=device["deviceVersion"], - battery_level=int(device["batteryLevel"]), - battery=device["battery"], - type=device["type"], - ) - for device in devices - ] + return devices async def async_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.""" @@ -140,6 +148,20 @@ class FitbitApi(ABC): _LOGGER.debug("Error from fitbit API: %s", err) raise FitbitApiException("Error from fitbit API") from err + async def _run_async[_T](self, func: Callable[[], Awaitable[_T]]) -> _T: + """Run client command.""" + try: + return await func() + except UnauthorizedException as err: + _LOGGER.debug("Unauthorized error from fitbit API: %s", err) + raise FitbitAuthException("Authentication error from fitbit API") from err + except ApiException as err: + _LOGGER.debug("Error from fitbit API: %s", err) + raise FitbitApiException("Error from fitbit API") from err + except OpenApiException as err: + _LOGGER.debug("Error communicating with fitbit API: %s", err) + raise FitbitApiException("Communication error from fitbit API") from err + class OAuthFitbitApi(FitbitApi): """Provide fitbit authentication tied to an OAuth2 based config entry.""" diff --git a/homeassistant/components/fitbit/coordinator.py b/homeassistant/components/fitbit/coordinator.py index 867723419dd..20bd4ef5fc4 100644 --- a/homeassistant/components/fitbit/coordinator.py +++ b/homeassistant/components/fitbit/coordinator.py @@ -6,6 +6,8 @@ import datetime import logging from typing import Final +from fitbit_web_api.models.device import Device + from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant from homeassistant.exceptions import ConfigEntryAuthFailed @@ -13,7 +15,6 @@ from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, Upda from .api import FitbitApi from .exceptions import FitbitApiException, FitbitAuthException -from .model import FitbitDevice _LOGGER = logging.getLogger(__name__) @@ -23,7 +24,7 @@ TIMEOUT = 10 type FitbitConfigEntry = ConfigEntry[FitbitData] -class FitbitDeviceCoordinator(DataUpdateCoordinator[dict[str, FitbitDevice]]): +class FitbitDeviceCoordinator(DataUpdateCoordinator[dict[str, Device]]): """Coordinator for fetching fitbit devices from the API.""" config_entry: FitbitConfigEntry @@ -41,7 +42,7 @@ class FitbitDeviceCoordinator(DataUpdateCoordinator[dict[str, FitbitDevice]]): ) self._api = api - async def _async_update_data(self) -> dict[str, FitbitDevice]: + async def _async_update_data(self) -> dict[str, Device]: """Fetch data from API endpoint.""" async with asyncio.timeout(TIMEOUT): try: @@ -50,7 +51,7 @@ class FitbitDeviceCoordinator(DataUpdateCoordinator[dict[str, FitbitDevice]]): raise ConfigEntryAuthFailed(err) from err except FitbitApiException as err: raise UpdateFailed(err) from err - return {device.id: device for device in devices} + return {device.id: device for device in devices if device.id is not None} @dataclass diff --git a/homeassistant/components/fitbit/manifest.json b/homeassistant/components/fitbit/manifest.json index 7739c7237f0..443f4c0b2fc 100644 --- a/homeassistant/components/fitbit/manifest.json +++ b/homeassistant/components/fitbit/manifest.json @@ -6,6 +6,6 @@ "dependencies": ["application_credentials", "http"], "documentation": "https://www.home-assistant.io/integrations/fitbit", "iot_class": "cloud_polling", - "loggers": ["fitbit"], - "requirements": ["fitbit==0.3.1"] + "loggers": ["fitbit", "fitbit_web_api"], + "requirements": ["fitbit==0.3.1", "fitbit-web-api==2.13.5"] } diff --git a/homeassistant/components/fitbit/model.py b/homeassistant/components/fitbit/model.py index cd8ece163a4..c1752616b2f 100644 --- a/homeassistant/components/fitbit/model.py +++ b/homeassistant/components/fitbit/model.py @@ -21,26 +21,6 @@ class FitbitProfile: """The locale defined in the user's Fitbit account settings.""" -@dataclass -class FitbitDevice: - """Device from the Fitbit API response.""" - - id: str - """The device ID.""" - - device_version: str - """The product name of the device.""" - - battery_level: int - """The battery level as a percentage.""" - - battery: str - """Returns the battery level of the device.""" - - type: str - """The type of the device such as TRACKER or SCALE.""" - - @dataclass class FitbitConfig: """Information from the fitbit ConfigEntry data.""" diff --git a/homeassistant/components/fitbit/sensor.py b/homeassistant/components/fitbit/sensor.py index f5c8a81ca26..9cee1d4f952 100644 --- a/homeassistant/components/fitbit/sensor.py +++ b/homeassistant/components/fitbit/sensor.py @@ -8,6 +8,8 @@ import datetime import logging from typing import Any, Final, cast +from fitbit_web_api.models.device import Device + from homeassistant.components.sensor import ( SensorDeviceClass, SensorEntity, @@ -32,7 +34,7 @@ from .api import FitbitApi from .const import ATTRIBUTION, BATTERY_LEVELS, DOMAIN, FitbitScope, FitbitUnitSystem from .coordinator import FitbitConfigEntry, FitbitDeviceCoordinator from .exceptions import FitbitApiException, FitbitAuthException -from .model import FitbitDevice, config_from_entry_data +from .model import config_from_entry_data _LOGGER: Final = logging.getLogger(__name__) @@ -657,7 +659,7 @@ class FitbitBatterySensor(CoordinatorEntity[FitbitDeviceCoordinator], SensorEnti coordinator: FitbitDeviceCoordinator, user_profile_id: str, description: FitbitSensorEntityDescription, - device: FitbitDevice, + device: Device, enable_default_override: bool, ) -> None: """Initialize the Fitbit sensor.""" @@ -677,7 +679,9 @@ class FitbitBatterySensor(CoordinatorEntity[FitbitDeviceCoordinator], SensorEnti @property def icon(self) -> str | None: """Icon to use in the frontend, if any.""" - if battery_level := BATTERY_LEVELS.get(self.device.battery): + if self.device.battery is not None and ( + battery_level := BATTERY_LEVELS.get(self.device.battery) + ): return icon_for_battery_level(battery_level=battery_level) return self.entity_description.icon @@ -697,7 +701,7 @@ class FitbitBatterySensor(CoordinatorEntity[FitbitDeviceCoordinator], SensorEnti @callback def _handle_coordinator_update(self) -> None: """Handle updated data from the coordinator.""" - self.device = self.coordinator.data[self.device.id] + self.device = self.coordinator.data[cast(str, self.device.id)] self._attr_native_value = self.device.battery self.async_write_ha_state() @@ -715,7 +719,7 @@ class FitbitBatteryLevelSensor( coordinator: FitbitDeviceCoordinator, user_profile_id: str, description: FitbitSensorEntityDescription, - device: FitbitDevice, + device: Device, ) -> None: """Initialize the Fitbit sensor.""" super().__init__(coordinator) @@ -736,6 +740,6 @@ class FitbitBatteryLevelSensor( @callback def _handle_coordinator_update(self) -> None: """Handle updated data from the coordinator.""" - self.device = self.coordinator.data[self.device.id] + self.device = self.coordinator.data[cast(str, self.device.id)] self._attr_native_value = self.device.battery_level self.async_write_ha_state() diff --git a/requirements_all.txt b/requirements_all.txt index efb6b94f206..0a1c777d155 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -958,6 +958,9 @@ fing_agent_api==1.0.3 # homeassistant.components.fints fints==3.1.0 +# homeassistant.components.fitbit +fitbit-web-api==2.13.5 + # homeassistant.components.fitbit fitbit==0.3.1 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 1426f9f92f0..74fb2fff9b9 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -846,6 +846,9 @@ fing_agent_api==1.0.3 # homeassistant.components.fints fints==3.1.0 +# homeassistant.components.fitbit +fitbit-web-api==2.13.5 + # homeassistant.components.fitbit fitbit==0.3.1 diff --git a/tests/components/fitbit/conftest.py b/tests/components/fitbit/conftest.py index 8a408748f16..3f38a9614fb 100644 --- a/tests/components/fitbit/conftest.py +++ b/tests/components/fitbit/conftest.py @@ -20,6 +20,7 @@ from homeassistant.core import HomeAssistant from homeassistant.setup import async_setup_component from tests.common import MockConfigEntry +from tests.test_util.aiohttp import AiohttpClientMocker CLIENT_ID = "1234" CLIENT_SECRET = "5678" @@ -206,12 +207,13 @@ def mock_device_response() -> list[dict[str, Any]]: @pytest.fixture(autouse=True) -def mock_devices(requests_mock: Mocker, devices_response: dict[str, Any]) -> None: +def mock_devices( + aioclient_mock: AiohttpClientMocker, devices_response: dict[str, Any] +) -> None: """Fixture to setup fake device responses.""" - requests_mock.register_uri( - "GET", + aioclient_mock.get( DEVICES_API_URL, - status_code=HTTPStatus.OK, + status=HTTPStatus.OK, json=devices_response, ) diff --git a/tests/components/fitbit/test_init.py b/tests/components/fitbit/test_init.py index a4794a63162..277965848c4 100644 --- a/tests/components/fitbit/test_init.py +++ b/tests/components/fitbit/test_init.py @@ -4,7 +4,6 @@ from collections.abc import Awaitable, Callable from http import HTTPStatus import pytest -from requests_mock.mocker import Mocker from homeassistant.components.fitbit.const import ( CONF_CLIENT_ID, @@ -90,14 +89,18 @@ async def test_token_refresh_success( assert await integration_setup() assert config_entry.state is ConfigEntryState.LOADED - # Verify token request - assert len(aioclient_mock.mock_calls) == 1 + # Verify token request and that the device API is called with new token + assert len(aioclient_mock.mock_calls) == 2 assert aioclient_mock.mock_calls[0][2] == { CONF_CLIENT_ID: CLIENT_ID, CONF_CLIENT_SECRET: CLIENT_SECRET, "grant_type": "refresh_token", "refresh_token": FAKE_REFRESH_TOKEN, } + assert str(aioclient_mock.mock_calls[1][1]) == DEVICES_API_URL + assert aioclient_mock.mock_calls[1][3].get("Authorization") == ( + "Bearer server-access-token" + ) # Verify updated token assert ( @@ -144,15 +147,15 @@ async def test_device_update_coordinator_failure( integration_setup: Callable[[], Awaitable[bool]], config_entry: MockConfigEntry, setup_credentials: None, - requests_mock: Mocker, + aioclient_mock: AiohttpClientMocker, ) -> None: """Test case where the device update coordinator fails on the first request.""" assert config_entry.state is ConfigEntryState.NOT_LOADED - requests_mock.register_uri( - "GET", + aioclient_mock.clear_requests() + aioclient_mock.get( DEVICES_API_URL, - status_code=HTTPStatus.INTERNAL_SERVER_ERROR, + status=HTTPStatus.INTERNAL_SERVER_ERROR, ) assert not await integration_setup() @@ -164,15 +167,15 @@ async def test_device_update_coordinator_reauth( integration_setup: Callable[[], Awaitable[bool]], config_entry: MockConfigEntry, setup_credentials: None, - requests_mock: Mocker, + aioclient_mock: AiohttpClientMocker, ) -> None: """Test case where the device update coordinator fails on the first request.""" assert config_entry.state is ConfigEntryState.NOT_LOADED - requests_mock.register_uri( - "GET", + aioclient_mock.clear_requests() + aioclient_mock.get( DEVICES_API_URL, - status_code=HTTPStatus.UNAUTHORIZED, + status=HTTPStatus.UNAUTHORIZED, json={ "errors": [{"errorType": "invalid_grant"}], }, diff --git a/tests/components/fitbit/test_sensor.py b/tests/components/fitbit/test_sensor.py index cee9835f89f..e11d295f746 100644 --- a/tests/components/fitbit/test_sensor.py +++ b/tests/components/fitbit/test_sensor.py @@ -29,6 +29,7 @@ from .conftest import ( ) from tests.common import MockConfigEntry +from tests.test_util.aiohttp import AiohttpClientMocker DEVICE_RESPONSE_CHARGE_2 = { "battery": "Medium", @@ -736,31 +737,13 @@ async def test_device_battery_level_update_failed( hass: HomeAssistant, setup_credentials: None, integration_setup: Callable[[], Awaitable[bool]], - requests_mock: Mocker, + aioclient_mock: AiohttpClientMocker, ) -> None: """Test API failure for a battery level sensor for devices.""" - - requests_mock.register_uri( - "GET", + aioclient_mock.clear_requests() + aioclient_mock.get( DEVICES_API_URL, - [ - { - "status_code": HTTPStatus.OK, - "json": [DEVICE_RESPONSE_CHARGE_2], - }, - # Fail when requesting an update - { - "status_code": HTTPStatus.INTERNAL_SERVER_ERROR, - "json": { - "errors": [ - { - "errorType": "request", - "message": "An error occurred", - } - ] - }, - }, - ], + json=[DEVICE_RESPONSE_CHARGE_2], ) assert await integration_setup() @@ -770,6 +753,19 @@ async def test_device_battery_level_update_failed( assert state.state == "Medium" # Request an update for the entity which will fail + aioclient_mock.clear_requests() + aioclient_mock.get( + DEVICES_API_URL, + status=HTTPStatus.INTERNAL_SERVER_ERROR, + json={ + "errors": [ + { + "errorType": "request", + "message": "An error occurred", + } + ] + }, + ) await async_update_entity(hass, "sensor.charge_2_battery") await hass.async_block_till_done() @@ -791,28 +787,15 @@ async def test_device_battery_level_reauth_required( setup_credentials: None, integration_setup: Callable[[], Awaitable[bool]], config_entry: MockConfigEntry, - requests_mock: Mocker, + aioclient_mock: AiohttpClientMocker, ) -> None: """Test API failure requires reauth.""" - requests_mock.register_uri( - "GET", + aioclient_mock.clear_requests() + aioclient_mock.get( DEVICES_API_URL, - [ - { - "status_code": HTTPStatus.OK, - "json": [DEVICE_RESPONSE_CHARGE_2], - }, - # Fail when requesting an update - { - "status_code": HTTPStatus.UNAUTHORIZED, - "json": { - "errors": [{"errorType": "invalid_grant"}], - }, - }, - ], + json=[DEVICE_RESPONSE_CHARGE_2], ) - assert await integration_setup() state = hass.states.get("sensor.charge_2_battery") @@ -820,6 +803,14 @@ async def test_device_battery_level_reauth_required( assert state.state == "Medium" # Request an update for the entity which will fail + aioclient_mock.clear_requests() + aioclient_mock.get( + DEVICES_API_URL, + status=HTTPStatus.UNAUTHORIZED, + json={ + "errors": [{"errorType": "invalid_grant"}], + }, + ) await async_update_entity(hass, "sensor.charge_2_battery") await hass.async_block_till_done()