Skip to content

Conversation

@farkasdi
Copy link
Contributor

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

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

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.

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:
  • Link to developer documentation pull request:
  • Link to frontend pull request:

Checklist

  • I understand the code I am submitting and can explain how it works.
  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.
  • Any generated code has been carefully reviewed for correctness and compliance with project standards.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

@home-assistant
Copy link

Hey there @cgtobi, mind taking a look at this pull request as it has been labeled with an integration (netatmo) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of netatmo can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign netatmo Removes the current integration label and assignees on the pull request, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.
"NACamera-off",
"NOCamera-off",
"NACamera-disconnection",
"NOCamera-disconnection",
Copy link
Contributor

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",
Copy link
Contributor

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?

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

Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment