Improve scan interval for Airthings Corentium Home 2 #155694
Conversation
|
Hey there @vincegio, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds dynamic scan interval adjustment for the Airthings BLE integration based on device type. Specifically, it increases the polling interval to 30 minutes (1800 seconds) for Corentium Home 2 devices, which are radon-only monitors that don't need frequent polling, while keeping the default 5-minute interval for other device types.
- Adds logic to adjust the coordinator's update interval to 30 minutes when a Corentium Home 2 device is detected
- Introduces a new constant
RADON_SCAN_INTERVAL(1800 seconds) for radon-only devices - Adds comprehensive test coverage for the scan interval adjustment behavior
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| homeassistant/components/airthings_ble/const.py | Adds RADON_SCAN_INTERVAL constant for radon-specific devices |
| homeassistant/components/airthings_ble/coordinator.py | Implements dynamic scan interval adjustment based on device model type |
| tests/components/airthings_ble/init.py | Adds test fixture for Corentium Home 2 device |
| tests/components/airthings_ble/test_coordinator.py | Adds parametrized tests to verify scan interval adjustment for different device types |
|
|
||
| from . import CORENTIUM_HOME_2_DEVICE_INFO, WAVE_DEVICE_INFO, WAVE_ENHANCE_DEVICE_INFO | ||
|
|
||
| from tests.components.bluetooth import MockConfigEntry, generate_ble_device |
There was a problem hiding this comment.
Incorrect import path for MockConfigEntry. The tests.components.bluetooth module does not export MockConfigEntry in its all list. Import MockConfigEntry from tests.common instead, matching the pattern used in other test files in this integration.
| from tests.components.bluetooth import MockConfigEntry, generate_ble_device | |
| from tests.common import MockConfigEntry | |
| from tests.components.bluetooth import generate_ble_device |
| if ( | ||
| self.update_interval == timedelta(seconds=DEFAULT_SCAN_INTERVAL) | ||
| and data.model == AirthingsDeviceType.CORENTIUM_HOME_2 | ||
| ): | ||
| self.update_interval = timedelta(seconds=RADON_SCAN_INTERVAL) |
There was a problem hiding this comment.
The scan interval adjustment logic runs on every update but only changes the interval once. This check will execute unnecessarily on all subsequent updates after the initial adjustment. Consider using a flag or moving this logic to a one-time initialization location (e.g., after first successful data fetch) to avoid repeated checks.
There was a problem hiding this comment.
When we first fetch data, we don't know the type of device. As copilot says, we could add a flag, but isn't that a bit too much?
There was a problem hiding this comment.
I think ideally we store this in the entry data so we don't have to recalculate this. I think we can add it behind a boolean for now, but we should avoid checking this on every update
| if ( | ||
| self.update_interval == timedelta(seconds=DEFAULT_SCAN_INTERVAL) | ||
| and data.model == AirthingsDeviceType.CORENTIUM_HOME_2 | ||
| ): | ||
| self.update_interval = timedelta(seconds=RADON_SCAN_INTERVAL) |
There was a problem hiding this comment.
I think ideally we store this in the entry data so we don't have to recalculate this. I think we can add it behind a boolean for now, but we should avoid checking this on every update
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
There was a problem hiding this comment.
We require 100% test coverage in the config flow
======================================================================== tests coverage =========================================================================
________________________________________________________ coverage: platform linux, python 3.13.7-final-0 ________________________________________________________
Name Stmts Miss Cover Missing
-------------------------------------------------------------------------------------
homeassistant/components/airthings_ble/__init__.py 15 0 100%
homeassistant/components/airthings_ble/config_flow.py 120 1 99% 139
homeassistant/components/airthings_ble/const.py 9 0 100%
homeassistant/components/airthings_ble/coordinator.py 45 5 89% 67, 76-77, 95-96
homeassistant/components/airthings_ble/sensor.py 73 4 95% 185-188
-------------------------------------------------------------------------------------
TOTAL 262 10 96%
| assert result["type"] is FlowResultType.CREATE_ENTRY | ||
| assert result["title"] == "Airthings Wave Plus (2930123456)" | ||
| assert result["result"].unique_id == "cc:cc:cc:cc:cc:cc" | ||
| assert result["data"] == {"device_model": "2930"} |
There was a problem hiding this comment.
Please also use the constant in the tests
|
|
||
| inject_bluetooth_service_info(hass, service_info) | ||
|
|
||
| assert "device_model" not in entry.data |
|
|
||
| # Migration should have added device_model to entry data | ||
| assert "device_model" in entry.data | ||
| assert entry.data["device_model"] == CORENTIUM_HOME_2_DEVICE_INFO.model.value |
| self, user_input: dict[str, Any] | None = None | ||
| ) -> ConfigFlowResult: | ||
| """Confirm discovery.""" | ||
| assert self._discovered_device is not None |
There was a problem hiding this comment.
Is it okey to do this? This should really not be None in this step. Not sure how to rewrite it without this.
I previously had a if self._discovered_device is None, but wasn't sure how to actually test it.
|
I now refactored the tests to not touch runtime data |
Co-authored-by: Joostlek <[email protected]>
Breaking change
Proposed change
Adjusting the scan interval for the Airthings Corentium Home 2 to avoid a known firmware issue where the device may stop advertising over BLE when too many BLE connections occur. This appears to be triggered over time when scanning frequently. For me personally, the device has stopped advertising after 20–90 days when using a 5-minute scan interval. By increasing the scan interval to 30 minutes, the device continues advertising reliably for much longer.
Longer scan interval will also improve the battery life, which makes sense for this product since most users will place it in the basement or similar and rarely interact with it.
We can revisit this change later once more information about the firmware bug is available, or if user feedback indicates that they prefer more frequent updates at the expense of battery life.
Would be nice to get this in 2025.11.0
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: