-
-
Notifications
You must be signed in to change notification settings - Fork 35.8k
Improve scan interval for Airthings Corentium Home 2 #155694
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
base: dev
Are you sure you want to change the base?
Changes from 5 commits
e024fa7
3564eb7
7133275
4afac76
cc6e552
9573a82
c0f2bb3
011c0a7
2bac611
8fa4fac
bd76df9
2d70980
f115848
bdd741c
04b786e
64c041a
3b1a873
46ea184
7db8444
d59ac29
af07f3d
c0627d6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,5 +7,6 @@ | |
| VOLUME_PICOCURIE = "pCi/L" | ||
|
|
||
| DEFAULT_SCAN_INTERVAL = 300 | ||
| RADON_SCAN_INTERVAL = 1800 | ||
|
|
||
| MAX_RETRIES_AFTER_STARTUP = 5 | ||
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| """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", | ||
| ), | ||
| ) | ||
| coordinator.ble_device = generate_ble_device("cc:cc:cc:cc:cc:cc", device_info.name) | ||
|
|
||
| assert coordinator.update_interval == timedelta(seconds=DEFAULT_SCAN_INTERVAL) | ||
|
|
||
| with patch( | ||
| "homeassistant.components.airthings_ble.coordinator.AirthingsBluetoothDeviceData.update_device", | ||
| return_value=device_info, | ||
| ): | ||
| await coordinator.async_refresh() | ||
|
|
||
| assert coordinator.update_interval == timedelta(seconds=expected_interval) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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