Skip to content
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

Add config flow stream preview to generic camera #122563

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
e27e1d9
Add config flow stream preview
davet2001 Jul 22, 2024
1d3f3a2
Update tests
davet2001 Jul 24, 2024
a46af3a
Code review: Move init_camera_prefs to shared function
davet2001 Jul 30, 2024
2d92191
Code review: Remove schema double check
davet2001 Jul 31, 2024
647f4dd
Combine test and preview code for stream
davet2001 Aug 7, 2024
c7a0173
Code review: Switch stream preview error to specific Exception
davet2001 Sep 7, 2024
16e4bc2
Update camera __init__.py from dev
davet2001 Oct 7, 2024
ce0e531
Test image preview timeout
davet2001 Oct 8, 2024
6e20d4b
Test the camera preview websocket command
davet2001 Oct 8, 2024
e73efac
Increase test coverage
davet2001 Oct 8, 2024
e3e5e4e
Fixes following #130672
davet2001 Nov 17, 2024
c9d4c90
Code review: no need to split out prefs init
davet2001 Nov 23, 2024
b240194
Code review: remove unnecessary check. Simplify.
davet2001 Nov 23, 2024
3efa681
Code review: include domain in preview stream name
davet2001 Nov 23, 2024
aa3aca5
Code review: apply suggestions
davet2001 Nov 23, 2024
c9329a3
Code review: remove unused WS parameters
davet2001 Nov 23, 2024
8004ec5
Code review: reorder and simplify WS function
davet2001 Nov 23, 2024
6a5e41e
Code review: remove Options flow code until later PR
davet2001 Nov 23, 2024
7ebd328
Code review: correct attribute naming style
davet2001 Nov 23, 2024
928766f
Fix websocket test
davet2001 Nov 24, 2024
8a90c65
Increase test coverage
davet2001 Nov 25, 2024
c0972de
Set unused parameters to optional
davet2001 Nov 25, 2024
88b6996
Ditch Stream wrapper. Use built in timeout for streams.
davet2001 Dec 2, 2024
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
5 changes: 2 additions & 3 deletions homeassistant/components/generic/camera.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,9 @@ def __init__(
self._stream_source = device_info.get(CONF_STREAM_SOURCE)
if self._stream_source:
self._stream_source = Template(self._stream_source, hass)
self._limit_refetch = device_info[CONF_LIMIT_REFETCH_TO_URL_CHANGE]
self._attr_frame_interval = 1 / device_info[CONF_FRAMERATE]
if self._stream_source:
self._attr_supported_features = CameraEntityFeature.STREAM
self._limit_refetch = device_info.get(CONF_LIMIT_REFETCH_TO_URL_CHANGE, False)
self._attr_frame_interval = 1 / device_info[CONF_FRAMERATE]
self.content_type = device_info[CONF_CONTENT_TYPE]
self.verify_ssl = device_info[CONF_VERIFY_SSL]
if device_info.get(CONF_RTSP_TRANSPORT):
Expand Down
172 changes: 130 additions & 42 deletions homeassistant/components/generic/config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import asyncio
from collections.abc import Mapping
import contextlib
from datetime import datetime
from datetime import datetime, timedelta
from errno import EHOSTUNREACH, EIO
import io
import logging
Expand All @@ -17,18 +17,21 @@
import voluptuous as vol
import yarl

from homeassistant.components import websocket_api
from homeassistant.components.camera import (
CAMERA_IMAGE_TIMEOUT,
DOMAIN as CAMERA_DOMAIN,
DynamicStreamSettings,
_async_get_image,
)
from homeassistant.components.http import HomeAssistantView
from homeassistant.components.http.view import HomeAssistantView
from homeassistant.components.stream import (
CONF_RTSP_TRANSPORT,
CONF_USE_WALLCLOCK_AS_TIMESTAMPS,
HLS_PROVIDER,
RTSP_TRANSPORTS,
SOURCE_TIMEOUT,
Stream,
create_stream,
)
from homeassistant.config_entries import (
Expand All @@ -49,7 +52,9 @@
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import HomeAssistantError, TemplateError
from homeassistant.helpers import config_validation as cv, template as template_helper
from homeassistant.helpers.entity_platform import EntityPlatform
from homeassistant.helpers.httpx_client import get_async_client
from homeassistant.setup import async_prepare_setup_platform
from homeassistant.util import slugify

from .camera import GenericCamera, generate_auth
Expand Down Expand Up @@ -79,6 +84,15 @@
IMAGE_PREVIEWS_ACTIVE = "previews"


class InvalidStreamException(HomeAssistantError):
"""Error to indicate an invalid stream."""

def __init__(self, error: str, details: str | None = None) -> None:
"""Initialize the error."""
super().__init__(error)
self.details = details


def build_schema(
user_input: Mapping[str, Any],
is_options_flow: bool = False,
Expand Down Expand Up @@ -231,12 +245,16 @@ def slug(
return None


async def async_test_stream(
async def async_test_and_preview_stream(
hass: HomeAssistant, info: Mapping[str, Any]
) -> dict[str, str]:
"""Verify that the stream is valid before we create an entity."""
) -> Stream | None:
"""Verify that the stream is valid before we create an entity.

Returns the stream object if valid. Raises InvalidStreamException if not.
The stream object is used to preview the video in the UI.
"""
if not (stream_source := info.get(CONF_STREAM_SOURCE)):
return {}
return None
# Import from stream.worker as stream cannot reexport from worker
# without forcing the av dependency on default_config
# pylint: disable-next=import-outside-toplevel
Expand All @@ -248,7 +266,7 @@ async def async_test_stream(
stream_source = stream_source.async_render(parse_result=False)
except TemplateError as err:
_LOGGER.warning("Problem rendering template %s: %s", stream_source, err)
return {CONF_STREAM_SOURCE: "template_error"}
raise InvalidStreamException("template_error") from err
stream_options: dict[str, str | bool | float] = {}
if rtsp_transport := info.get(CONF_RTSP_TRANSPORT):
stream_options[CONF_RTSP_TRANSPORT] = rtsp_transport
Expand All @@ -257,10 +275,10 @@ async def async_test_stream(

try:
url = yarl.URL(stream_source)
except ValueError:
return {CONF_STREAM_SOURCE: "malformed_url"}
except ValueError as err:
raise InvalidStreamException("malformed_url") from err
if not url.is_absolute():
return {CONF_STREAM_SOURCE: "relative_url"}
raise InvalidStreamException("relative_url")
if not url.user and not url.password:
username = info.get(CONF_USERNAME)
password = info.get(CONF_PASSWORD)
Expand All @@ -273,29 +291,28 @@ async def async_test_stream(
stream_source,
stream_options,
DynamicStreamSettings(),
"test_stream",
f"{DOMAIN}.test_stream",
)
hls_provider = stream.add_provider(HLS_PROVIDER)
Copy link
Contributor

@allenporter allenporter Nov 8, 2024

Choose a reason for hiding this comment

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

this can be moved out of the try (i realize it was inside before)

Maybe the PreviewStream creation should also be moved out too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but create_stream can trigger a HomeAssistantError if stream is not set up.

This was added in #121308

await stream.start()
if not await hls_provider.part_recv(timeout=SOURCE_TIMEOUT):
hass.async_create_task(stream.stop())
return {CONF_STREAM_SOURCE: "timeout"}
await stream.stop()
except StreamWorkerError as err:
Copy link
Contributor

Choose a reason for hiding this comment

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

Does all of this error handling work? I'm not sure that many of these exceptions are actually raised on create_stream given this creates a stream without starting it. Also, stream creation happens in the background inside of _run_worker. My impression is that it only returns an available bit here without providing much other feedback.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These were added at the time of migrating to the UI.

In that case, perhaps stream.start() should move within the try?

The aim at the time was to get a larger number of meaningful error messages to the user, to avoid a generic 'invalid settings' message.

But I'm all for consolidating/rationalising if that's sensible here; maybe that can be a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I very much like the goal of giving helpful error messages and I think it is a good idea. However, I'm just wondering if its even possible for these errors to be thrown. I'm not sure it is possible for a StreamWorkerError, PermissionError, OSError etc to get raised here. My impression is none of these exceptions can be raised directly here at the moment.

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 you are right. I just tried a few error scenarios (invalid domain, URL without a camera stream, wrong password). All of them result in specific messages in the error log/console, but give a general timeout on the UI.

So we can probably take them out. But that would be more changes than I planned here. Can it be done in a separate PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure.

return {CONF_STREAM_SOURCE: "unknown_with_details", "error_details": str(err)}
except PermissionError:
return {CONF_STREAM_SOURCE: "stream_not_permitted"}
raise InvalidStreamException("unknown_with_details", str(err)) from err
except PermissionError as err:
raise InvalidStreamException("stream_not_permitted") from err
except OSError as err:
if err.errno == EHOSTUNREACH:
return {CONF_STREAM_SOURCE: "stream_no_route_to_host"}
raise InvalidStreamException("stream_no_route_to_host") from err
if err.errno == EIO: # input/output error
return {CONF_STREAM_SOURCE: "stream_io_error"}
raise InvalidStreamException("stream_io_error") from err
raise
except HomeAssistantError as err:
if "Stream integration is not set up" in str(err):
return {CONF_STREAM_SOURCE: "stream_not_set_up"}
raise InvalidStreamException("stream_not_set_up") from err
raise
return {}
await stream.start()
if not await hls_provider.part_recv(timeout=SOURCE_TIMEOUT):
hass.async_create_task(stream.stop())
raise InvalidStreamException("timeout")
return stream


def register_preview(hass: HomeAssistant) -> None:
Expand All @@ -316,6 +333,7 @@ class GenericIPCamConfigFlow(ConfigFlow, domain=DOMAIN):
def __init__(self) -> None:
"""Initialize Generic ConfigFlow."""
self.preview_cam: dict[str, Any] = {}
self.preview_stream: Stream | None = None
self.user_input: dict[str, Any] = {}
self.title = ""

Expand All @@ -326,14 +344,6 @@ def async_get_options_flow(
"""Get the options flow for this handler."""
return GenericOptionsFlowHandler()

def check_for_existing(self, options: dict[str, Any]) -> bool:
"""Check whether an existing entry is using the same URLs."""
return any(
entry.options.get(CONF_STILL_IMAGE_URL) == options.get(CONF_STILL_IMAGE_URL)
and entry.options.get(CONF_STREAM_SOURCE) == options.get(CONF_STREAM_SOURCE)
for entry in self._async_current_entries()
)

async def async_step_user(
self, user_input: dict[str, Any] | None = None
) -> ConfigFlowResult:
Expand All @@ -349,10 +359,17 @@ async def async_step_user(
errors["base"] = "no_still_image_or_stream_url"
else:
errors, still_format = await async_test_still(hass, user_input)
errors = errors | await async_test_stream(hass, user_input)
try:
self.preview_stream = await async_test_and_preview_stream(
hass, user_input
)
except InvalidStreamException as err:
errors[CONF_STREAM_SOURCE] = str(err)
if err.details:
errors["error_details"] = err.details
self.preview_stream = None
if not errors:
user_input[CONF_CONTENT_TYPE] = still_format
user_input[CONF_LIMIT_REFETCH_TO_URL_CHANGE] = False
still_url = user_input.get(CONF_STILL_IMAGE_URL)
stream_url = user_input.get(CONF_STREAM_SOURCE)
name = (
Expand All @@ -365,14 +382,9 @@ async def async_step_user(
user_input[CONF_CONTENT_TYPE] = "image/jpeg"
self.user_input = user_input
self.title = name

if still_url is None:
return self.async_create_entry(
title=self.title, data={}, options=self.user_input
)
# temporary preview for user to check the image
self.preview_cam = user_input
return await self.async_step_user_confirm_still()
return await self.async_step_user_confirm()
if "error_details" in errors:
description_placeholders["error"] = errors.pop("error_details")
elif self.user_input:
Expand All @@ -386,11 +398,14 @@ async def async_step_user(
errors=errors,
)

async def async_step_user_confirm_still(
async def async_step_user_confirm(
self, user_input: dict[str, Any] | None = None
) -> ConfigFlowResult:
"""Handle user clicking confirm after still preview."""
if user_input:
if ha_stream := self.preview_stream:
# Kill off the temp stream we created.
await ha_stream.stop()
if not user_input.get(CONF_CONFIRMED_OK):
return await self.async_step_user()
return self.async_create_entry(
Expand All @@ -399,16 +414,22 @@ async def async_step_user_confirm_still(
register_preview(self.hass)
preview_url = f"/api/generic/preview_flow_image/{self.flow_id}?t={datetime.now().isoformat()}"
return self.async_show_form(
step_id="user_confirm_still",
step_id="user_confirm",
data_schema=vol.Schema(
{
vol.Required(CONF_CONFIRMED_OK, default=False): bool,
}
),
description_placeholders={"preview_url": preview_url},
errors=None,
preview="generic_camera",
)

@staticmethod
async def async_setup_preview(hass: HomeAssistant) -> None:
"""Set up preview WS API."""
websocket_api.async_register_command(hass, ws_start_preview)


class GenericOptionsFlowHandler(OptionsFlow):
"""Handle Generic IP Camera options."""
Expand All @@ -423,13 +444,21 @@ async def async_step_init(
) -> ConfigFlowResult:
"""Manage Generic IP Camera options."""
errors: dict[str, str] = {}
description_placeholders = {}
hass = self.hass

if user_input is not None:
errors, still_format = await async_test_still(
hass, self.config_entry.options | user_input
)
errors = errors | await async_test_stream(hass, user_input)
try:
await async_test_and_preview_stream(hass, user_input)
except InvalidStreamException as err:
errors[CONF_STREAM_SOURCE] = str(err)
if err.details:
errors["error_details"] = err.details
# Stream preview during options flow not yet implemented

still_url = user_input.get(CONF_STILL_IMAGE_URL)
if not errors:
if still_url is None:
Expand All @@ -449,13 +478,16 @@ async def async_step_init(
# temporary preview for user to check the image
self.preview_cam = data
return await self.async_step_confirm_still()
if "error_details" in errors:
description_placeholders["error"] = errors.pop("error_details")
return self.async_show_form(
step_id="init",
data_schema=build_schema(
user_input or self.config_entry.options,
True,
self.show_advanced_options,
),
description_placeholders=description_placeholders,
errors=errors,
)

Expand Down Expand Up @@ -518,3 +550,59 @@ async def get(self, request: web.Request, flow_id: str) -> web.Response:
CAMERA_IMAGE_TIMEOUT,
)
return web.Response(body=image.content, content_type=image.content_type)


@websocket_api.websocket_command(
{
vol.Required("type"): "generic_camera/start_preview",
vol.Required("flow_id"): str,
vol.Optional("flow_type"): vol.Any("config_flow"),
vol.Optional("user_input"): dict,
}
)
@websocket_api.async_response
async def ws_start_preview(
hass: HomeAssistant,
connection: websocket_api.ActiveConnection,
msg: dict[str, Any],
) -> None:
"""Generate websocket handler for the camera still/stream preview."""
_LOGGER.debug("Generating websocket handler for generic camera preview")

flow_id = msg["flow_id"]
flow = cast(
GenericIPCamConfigFlow,
hass.config_entries.flow._progress.get(flow_id), # noqa: SLF001
)
user_input = flow.preview_cam

# Create an EntityPlatform, needed for name translations
platform = await async_prepare_setup_platform(hass, {}, CAMERA_DOMAIN, DOMAIN)
entity_platform = EntityPlatform(
hass=hass,
logger=_LOGGER,
domain=CAMERA_DOMAIN,
platform_name=DOMAIN,
platform=platform,
scan_interval=timedelta(seconds=3600),
entity_namespace=None,
)
await entity_platform.async_load_translations()

ha_still_url = None
ha_stream_url = None

if user_input.get(CONF_STILL_IMAGE_URL):
ha_still_url = f"/api/generic/preview_flow_image/{msg['flow_id']}?t={datetime.now().isoformat()}"
_LOGGER.debug("Got preview still URL: %s", ha_still_url)

if ha_stream := flow.preview_stream:
ha_stream_url = ha_stream.endpoint_url(HLS_PROVIDER)
_LOGGER.debug("Got preview stream URL: %s", ha_stream_url)

connection.send_message(
websocket_api.event_message(
msg["id"],
{"attributes": {"still_url": ha_still_url, "stream_url": ha_stream_url}},
)
)
2 changes: 1 addition & 1 deletion homeassistant/components/generic/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"name": "Generic Camera",
"codeowners": ["@davet2001"],
"config_flow": true,
"dependencies": ["http"],
"dependencies": ["http", "stream"],
"documentation": "https://www.home-assistant.io/integrations/generic",
"integration_type": "device",
"iot_class": "local_push",
Expand Down
Loading