-
- Notifications
You must be signed in to change notification settings - Fork 36.1k
Netatmo NOCamera on/off fix #158741
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?
Netatmo NOCamera on/off fix #158741
Conversation
| Hey there @cgtobi, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
| "NACamera-off", | ||
| "NOCamera-off", | ||
| "NACamera-disconnection", | ||
| "NOCamera-disconnection", |
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.
Can we cover also this event type in a test? The already supported event "NACamera-disconnection" is also not tested it seems, can that too be covered?
| "NACamera-on", | ||
| "NOCamera-on", | ||
| WEBHOOK_NACAMERA_CONNECTION, | ||
| "NOCamera-connection", |
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.
Can we add a variant of this test for outdoor camera?
Maybe the event type can be parametrized to avoid duplicating the test?
core/tests/components/netatmo/test_camera.py
Lines 428 to 486 in 2beb551
| async def test_camera_reconnect_webhook( | |
| hass: HomeAssistant, config_entry: MockConfigEntry | |
| ) -> None: | |
| """Test webhook event on camera reconnect.""" | |
| fake_post_hits = 0 | |
| async def fake_post(*args: Any, **kwargs: Any): | |
| """Fake error during requesting backend data.""" | |
| nonlocal fake_post_hits | |
| fake_post_hits += 1 | |
| return await fake_post_request(hass, *args, **kwargs) | |
| with ( | |
| patch( | |
| "homeassistant.components.netatmo.api.AsyncConfigEntryNetatmoAuth" | |
| ) as mock_auth, | |
| patch("homeassistant.components.netatmo.data_handler.PLATFORMS", ["camera"]), | |
| patch( | |
| "homeassistant.components.netatmo.async_get_config_entry_implementation", | |
| ), | |
| patch( | |
| "homeassistant.components.netatmo.webhook_generate_url", | |
| ) as mock_webhook, | |
| ): | |
| mock_auth.return_value.async_post_api_request.side_effect = fake_post | |
| mock_auth.return_value.async_addwebhook.side_effect = AsyncMock() | |
| mock_auth.return_value.async_dropwebhook.side_effect = AsyncMock() | |
| mock_webhook.return_value = "https://example.com" | |
| assert await hass.config_entries.async_setup(config_entry.entry_id) | |
| await hass.async_block_till_done() | |
| webhook_id = config_entry.data[CONF_WEBHOOK_ID] | |
| # Fake webhook activation | |
| response = { | |
| "push_type": "webhook_activation", | |
| } | |
| await simulate_webhook(hass, webhook_id, response) | |
| await hass.async_block_till_done() | |
| assert fake_post_hits == 8 | |
| calls = fake_post_hits | |
| # Fake camera reconnect | |
| response = { | |
| "push_type": "NACamera-connection", | |
| } | |
| await simulate_webhook(hass, webhook_id, response) | |
| await hass.async_block_till_done() | |
| async_fire_time_changed( | |
| hass, | |
| dt_util.utcnow() + timedelta(seconds=60), | |
| ) | |
| await hass.async_block_till_done() | |
| assert fake_post_hits >= calls | |
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 we need to parametrize rather the camera type (NOC/NAC) and camera_id + entity name too. But I see feasible at first glance.
Proposed change
Add handling of on/off webhooks for NOCamera (Netatmo Outdoor Camera). NOCamera light mode changes
are already there so this should be there too. Earlier added test are now updated to assert the right state changes.
Type of change
Additional information
Note: As the on/off will be there the next step would be to refactor. E.g. use event-type as more generic as push-type attribute of the webhook.
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: