Skip to content
Open
Show file tree
Hide file tree
Changes from 15 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
11 changes: 9 additions & 2 deletions homeassistant/components/airthings_ble/config_flow.py
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would assume these changes also have an effect to the test_config_flow.py

Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,12 @@ async def async_step_bluetooth_confirm(
):
return self.async_abort(reason="firmware_upgrade_required")

if self._discovered_device is None:
return self.async_abort(reason="no_devices_found")

return self.async_create_entry(
title=self.context["title_placeholders"]["name"], data={}
title=self.context["title_placeholders"]["name"],
data={"device_model": self._discovered_device.device.model.value},
)

self._set_confirm_only()
Expand Down Expand Up @@ -164,7 +168,10 @@ async def async_step_user(

self._discovered_device = discovery

return self.async_create_entry(title=discovery.name, data={})
return self.async_create_entry(
title=discovery.name,
data={"device_model": discovery.device.model.value},
)

current_addresses = self._async_current_ids(include_ignore=False)
devices: list[BluetoothServiceInfoBleak] = []
Expand Down
1 change: 1 addition & 0 deletions homeassistant/components/airthings_ble/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@
VOLUME_PICOCURIE = "pCi/L"

DEFAULT_SCAN_INTERVAL = 300
RADON_SCAN_INTERVAL = 1800

MAX_RETRIES_AFTER_STARTUP = 5
34 changes: 30 additions & 4 deletions homeassistant/components/airthings_ble/coordinator.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
from datetime import timedelta
import logging

from airthings_ble import AirthingsBluetoothDeviceData, AirthingsDevice
from airthings_ble import (
AirthingsBluetoothDeviceData,
AirthingsDevice,
AirthingsDeviceType,
)
from bleak.backends.device import BLEDevice
from bleak_retry_connector import close_stale_connections_by_address

Expand All @@ -16,7 +20,7 @@
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
from homeassistant.util.unit_system import METRIC_SYSTEM

from .const import DEFAULT_SCAN_INTERVAL, DOMAIN
from .const import DEFAULT_SCAN_INTERVAL, DOMAIN, RADON_SCAN_INTERVAL

_LOGGER = logging.getLogger(__name__)

Expand All @@ -34,12 +38,19 @@ def __init__(self, hass: HomeAssistant, entry: AirthingsBLEConfigEntry) -> None:
self.airthings = AirthingsBluetoothDeviceData(
_LOGGER, hass.config.units is METRIC_SYSTEM
)

device_model = entry.data.get("device_model")
if device_model == AirthingsDeviceType.CORENTIUM_HOME_2.value:
interval = RADON_SCAN_INTERVAL
else:
interval = DEFAULT_SCAN_INTERVAL

super().__init__(
hass,
_LOGGER,
config_entry=entry,
name=DOMAIN,
update_interval=timedelta(seconds=DEFAULT_SCAN_INTERVAL),
update_interval=timedelta(seconds=interval),
)

async def _async_setup(self) -> None:
Expand All @@ -58,11 +69,26 @@ async def _async_setup(self) -> None:
)
self.ble_device = ble_device

if "device_model" not in self.config_entry.data:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use a constant for device_model

try:
_LOGGER.debug("Fetching device info for migration")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
try:
_LOGGER.debug("Fetching device info for migration")
_LOGGER.debug("Fetching device info for migration")
try:

data = await self.airthings.update_device(self.ble_device)
except Exception as err:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be more specific here, ideally we should also update the one in async_update_data but that is outside of the scope

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted. Will try to handle this (+ the other one) in a separate PR.

raise ConfigEntryNotReady(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UpdateFailed

f"Unable to fetch data for migration: {err}"
) from err

self.hass.config_entries.async_update_entry(
self.config_entry,
data={**self.config_entry.data, "device_model": data.model.value},
)
if data.model == AirthingsDeviceType.CORENTIUM_HOME_2:
self.update_interval = timedelta(seconds=RADON_SCAN_INTERVAL)

async def _async_update_data(self) -> AirthingsDevice:
"""Get data from Airthings BLE."""
try:
data = await self.airthings.update_device(self.ble_device)
except Exception as err:
raise UpdateFailed(f"Unable to fetch data: {err}") from err

return data
18 changes: 18 additions & 0 deletions tests/components/airthings_ble/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,24 @@ def patch_airthings_device_update():
address="cc:cc:cc:cc:cc:cc",
)

CORENTIUM_HOME_2_DEVICE_INFO = AirthingsDevice(
manufacturer="Airthings AS",
hw_version="REV X",
sw_version="R-SUB-1.3.4-master+0",
model=AirthingsDeviceType.CORENTIUM_HOME_2,
name="Airthings Corentium Home 2",
identifier="123456",
sensors={
"connectivity_mode": "Bluetooth",
"battery": 90,
"temperature": 20.0,
"humidity": 55.0,
"radon_1day_avg": 45,
"radon_1day_level": "low",
},
address="cc:cc:cc:cc:cc:cc",
)

TEMPERATURE_V1 = MockEntity(
unique_id="Airthings Wave Plus 123456_temperature",
name="Airthings Wave Plus 123456 Temperature",
Expand Down
160 changes: 160 additions & 0 deletions tests/components/airthings_ble/test_coordinator.py
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would argue that we should still test this via test_sensor.py, and just use the freezer to make sure we only update when we expect it to

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored it now, and it's now storing the model name in the config flow. If not set, it will be set when setting up the device (e.g. already configured the device before this PR).

Also added more tests

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, we should still not create coordinators directly. instead, let's test the migration in test_init.py and the update interval via test_sensor.py

Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
"""Test the Airthings BLE coordinator."""

from datetime import timedelta
from unittest.mock import patch

import pytest

from homeassistant.components.airthings_ble.const import (
DEFAULT_SCAN_INTERVAL,
DOMAIN,
RADON_SCAN_INTERVAL,
)
from homeassistant.components.airthings_ble.coordinator import (
AirthingsBLEDataUpdateCoordinator,
)
from homeassistant.core import HomeAssistant

from . import CORENTIUM_HOME_2_DEVICE_INFO, WAVE_DEVICE_INFO, WAVE_ENHANCE_DEVICE_INFO

from tests.common import MockConfigEntry
from tests.components.bluetooth import generate_ble_device


@pytest.mark.parametrize(
("device_info", "expected_interval"),
[
(CORENTIUM_HOME_2_DEVICE_INFO, RADON_SCAN_INTERVAL),
(WAVE_DEVICE_INFO, DEFAULT_SCAN_INTERVAL),
(WAVE_ENHANCE_DEVICE_INFO, DEFAULT_SCAN_INTERVAL),
],
)
async def test_scan_interval_adjustment(
hass: HomeAssistant,
device_info,
expected_interval: int,
) -> None:
"""Test that scan interval is adjusted based on device type."""
coordinator = AirthingsBLEDataUpdateCoordinator(
hass=hass,
entry=MockConfigEntry(
domain=DOMAIN,
unique_id="cc:cc:cc:cc:cc:cc",
data={"device_model": device_info.model.value},
),
)
coordinator.ble_device = generate_ble_device("cc:cc:cc:cc:cc:cc", device_info.name)

assert coordinator.update_interval == timedelta(seconds=expected_interval)


async def test_migration_existing_entry_radon_device(
hass: HomeAssistant,
) -> None:
"""Test migration of existing config entry without device_model for radon device."""
entry = MockConfigEntry(
domain=DOMAIN,
unique_id="cc:cc:cc:cc:cc:cc",
data={},
)
entry.add_to_hass(hass)

coordinator = AirthingsBLEDataUpdateCoordinator(
hass=hass,
entry=entry,
)

assert coordinator.update_interval == timedelta(seconds=DEFAULT_SCAN_INTERVAL)
assert "device_model" not in entry.data

with (
patch(
"homeassistant.components.airthings_ble.coordinator.AirthingsBluetoothDeviceData.update_device",
return_value=CORENTIUM_HOME_2_DEVICE_INFO,
),
patch(
"homeassistant.components.bluetooth.async_ble_device_from_address",
return_value=generate_ble_device(
"cc:cc:cc:cc:cc:cc", CORENTIUM_HOME_2_DEVICE_INFO.name
),
),
):
await coordinator._async_setup()

assert "device_model" in entry.data
assert entry.data["device_model"] == CORENTIUM_HOME_2_DEVICE_INFO.model.value
assert coordinator.update_interval == timedelta(seconds=RADON_SCAN_INTERVAL)


async def test_migration_existing_entry_non_radon_device(
hass: HomeAssistant,
) -> None:
"""Test migration of existing config entry without device_model for non-radon device."""
entry = MockConfigEntry(
domain=DOMAIN,
unique_id="cc:cc:cc:cc:cc:cc",
data={},
)
entry.add_to_hass(hass)

coordinator = AirthingsBLEDataUpdateCoordinator(
hass=hass,
entry=entry,
)

assert coordinator.update_interval == timedelta(seconds=DEFAULT_SCAN_INTERVAL)
assert "device_model" not in entry.data

with (
patch(
"homeassistant.components.airthings_ble.coordinator.AirthingsBluetoothDeviceData.update_device",
return_value=WAVE_DEVICE_INFO,
),
patch(
"homeassistant.components.bluetooth.async_ble_device_from_address",
return_value=generate_ble_device(
"cc:cc:cc:cc:cc:cc", WAVE_DEVICE_INFO.name
),
),
):
await coordinator._async_setup()

assert "device_model" in entry.data
assert entry.data["device_model"] == WAVE_DEVICE_INFO.model.value
assert coordinator.update_interval == timedelta(seconds=DEFAULT_SCAN_INTERVAL)


async def test_no_migration_when_device_model_exists(
hass: HomeAssistant,
) -> None:
"""Test that migration does not run when device_model already exists."""
entry = MockConfigEntry(
domain=DOMAIN,
unique_id="cc:cc:cc:cc:cc:cc",
data={"device_model": WAVE_DEVICE_INFO.model.value},
)
entry.add_to_hass(hass)

coordinator = AirthingsBLEDataUpdateCoordinator(
hass=hass,
entry=entry,
)

assert coordinator.update_interval == timedelta(seconds=DEFAULT_SCAN_INTERVAL)

with (
patch(
"homeassistant.components.airthings_ble.coordinator.AirthingsBluetoothDeviceData.update_device",
) as mock_update,
patch(
"homeassistant.components.bluetooth.async_ble_device_from_address",
return_value=generate_ble_device(
"cc:cc:cc:cc:cc:cc", WAVE_DEVICE_INFO.name
),
),
):
await coordinator._async_setup()

# Migration should not have been called
mock_update.assert_not_called()
assert entry.data["device_model"] == WAVE_DEVICE_INFO.model.value
Loading
Loading