Add visual image preview during generic camera config flow (#71269)

* Add visual preview during setup of generic camera

* Code review: standardize preview  url

* Fix slug test

* Refactor to use HomeAssistantView

* Code review: simplify

* Update manifest

* Don't illegally access protected member

* Increase test coverage

* Prevent browser caching of preview images.

* Code review:move incrementor to ?t=X + simplify

* Discard old flow preview data

* Increase test coverage

* Code review: rename variables for clarity

* Add timeout for image previews

* Fix preview timeout tests

* Simplify: store cam image preview in config_flow

* Call step method to transition between flow steps

* Only store user_input in flow, not CameraObject

* Fix problem where test wouldn't run in isolation.

* Simplify test

* Don't move directly to another step's form

* Remove unused constant

* Simplify test

Co-authored-by: Dave T <davet2001@users.noreply.github.com>

homeassistant/components/generic/config_flow.py

@@ -3,17 +3,21 @@ from __future__ import annotations
 
 from collections.abc import Mapping
 import contextlib
+from datetime import datetime
 from errno import EHOSTUNREACH, EIO
 import io
 import logging
 from typing import Any
 
 import PIL
+from aiohttp import web
 from async_timeout import timeout
 from httpx import HTTPStatusError, RequestError, TimeoutException
 import voluptuous as vol
 import yarl
 
+from homeassistant.components.camera import CAMERA_IMAGE_TIMEOUT, _async_get_image
+from homeassistant.components.http.view import HomeAssistantView
 from homeassistant.components.stream import (
     CONF_RTSP_TRANSPORT,
     CONF_USE_WALLCLOCK_AS_TIMESTAMPS,
@@ -33,14 +37,15 @@ from homeassistant.const import (
     HTTP_DIGEST_AUTHENTICATION,
 )
 from homeassistant.core import HomeAssistant
-from homeassistant.data_entry_flow import FlowResult
+from homeassistant.data_entry_flow import FlowResult, UnknownFlow
 from homeassistant.exceptions import TemplateError
 from homeassistant.helpers import config_validation as cv, template as template_helper
 from homeassistant.helpers.httpx_client import get_async_client
 from homeassistant.util import slugify
 
-from .camera import generate_auth
+from .camera import GenericCamera, generate_auth
 from .const import (
+    CONF_CONFIRMED_OK,
     CONF_CONTENT_TYPE,
     CONF_FRAMERATE,
     CONF_LIMIT_REFETCH_TO_URL_CHANGE,
@@ -62,6 +67,7 @@ DEFAULT_DATA = {
 }
 
 SUPPORTED_IMAGE_TYPES = {"png", "jpeg", "gif", "svg+xml", "webp"}
+IMAGE_PREVIEWS_ACTIVE = "previews"
 
 
 def build_schema(
@@ -190,6 +196,7 @@ def slug(
     hass: HomeAssistant, template: str | template_helper.Template | None
 ) -> str | None:
     """Convert a camera url into a string suitable for a camera name."""
+    url = ""
     if not template:
         return None
     if not isinstance(template, template_helper.Template):
@@ -197,10 +204,8 @@ def slug(
     try:
         url = template.async_render(parse_result=False)
         return slugify(yarl.URL(url).host)
-    except TemplateError as err:
+    except (ValueError, TemplateError, TypeError) as err:
-        _LOGGER.error("Syntax error in '%s': %s", template.template, err)
+        _LOGGER.error("Syntax error in '%s': %s", template, err)
-    except (ValueError, TypeError) as err:
-        _LOGGER.error("Syntax error in '%s': %s", url, err)
     return None
 
 
@@ -261,6 +266,16 @@ async def async_test_stream(
     return {}
 
 
+def register_preview(hass: HomeAssistant):
+    """Set up previews for camera feeds during config flow."""
+    hass.data.setdefault(DOMAIN, {})
+
+    if not hass.data[DOMAIN].get(IMAGE_PREVIEWS_ACTIVE):
+        _LOGGER.debug("Registering camera image preview handler")
+        hass.http.register_view(CameraImagePreview(hass))
+    hass.data[DOMAIN][IMAGE_PREVIEWS_ACTIVE] = True
+
+
 class GenericIPCamConfigFlow(ConfigFlow, domain=DOMAIN):
     """Config flow for generic IP camera."""
 
@@ -268,8 +283,8 @@ class GenericIPCamConfigFlow(ConfigFlow, domain=DOMAIN):
 
     def __init__(self) -> None:
         """Initialize Generic ConfigFlow."""
-        self.cached_user_input: dict[str, Any] = {}
+        self.user_input: dict[str, Any] = {}
-        self.cached_title = ""
+        self.title = ""
 
     @staticmethod
     def async_get_options_flow(
@@ -314,19 +329,45 @@ class GenericIPCamConfigFlow(ConfigFlow, domain=DOMAIN):
                         # The automatically generated still image that stream generates
                         # is always jpeg
                         user_input[CONF_CONTENT_TYPE] = "image/jpeg"
+                    self.user_input = user_input
+                    self.title = name
 
-                    return self.async_create_entry(
+                    # temporary preview for user to check the image
-                        title=name, data={}, options=user_input
+                    self.context["preview_cam"] = user_input
-                    )
+                    return await self.async_step_user_confirm_still()
+        elif self.user_input:
+            user_input = self.user_input
         else:
             user_input = DEFAULT_DATA.copy()
-
         return self.async_show_form(
             step_id="user",
             data_schema=build_schema(user_input),
             errors=errors,
         )
 
+    async def async_step_user_confirm_still(
+        self, user_input: dict[str, Any] | None = None
+    ) -> FlowResult:
+        """Handle user clicking confirm after still preview."""
+        if user_input:
+            if not user_input.get(CONF_CONFIRMED_OK):
+                return await self.async_step_user()
+            return self.async_create_entry(
+                title=self.title, data={}, options=self.user_input
+            )
+        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",
+            data_schema=vol.Schema(
+                {
+                    vol.Required(CONF_CONFIRMED_OK, default=False): bool,
+                }
+            ),
+            description_placeholders={"preview_url": preview_url},
+            errors=None,
+        )
+
     async def async_step_import(self, import_config: dict[str, Any]) -> FlowResult:
         """Handle config import from yaml."""
         # abort if we've already got this one.
@@ -410,3 +451,33 @@ class GenericOptionsFlowHandler(OptionsFlow):
             ),
             errors=errors,
         )
+
+
+class CameraImagePreview(HomeAssistantView):
+    """Camera view to temporarily serve an image."""
+
+    url = "/api/generic/preview_flow_image/{flow_id}"
+    name = "api:generic:preview_flow_image"
+    requires_auth = False
+
+    def __init__(self, hass: HomeAssistant) -> None:
+        """Initialise."""
+        self.hass = hass
+
+    async def get(self, request: web.Request, flow_id: str) -> web.Response:
+        """Start a GET request."""
+        _LOGGER.debug("processing GET request for flow_id=%s", flow_id)
+        try:
+            flow: FlowResult = self.hass.config_entries.flow.async_get(flow_id)
+        except UnknownFlow as exc:
+            raise web.HTTPNotFound() from exc
+        user_input = flow["context"]["preview_cam"]
+        camera = GenericCamera(self.hass, user_input, flow_id, "preview")
+        if not camera.is_on:
+            _LOGGER.debug("Camera is off")
+            raise web.HTTPServiceUnavailable()
+        image = await _async_get_image(
+            camera,
+            CAMERA_IMAGE_TIMEOUT,
+        )
+        return web.Response(body=image.content, content_type=image.content_type)

homeassistant/components/generic/const.py

@@ -2,6 +2,7 @@
 
 DOMAIN = "generic"
 DEFAULT_NAME = "Generic Camera"
+CONF_CONFIRMED_OK = "confirmed_ok"
 CONF_CONTENT_TYPE = "content_type"
 CONF_LIMIT_REFETCH_TO_URL_CHANGE = "limit_refetch_to_url_change"
 CONF_STILL_IMAGE_URL = "still_image_url"

homeassistant/components/generic/manifest.json

@@ -3,6 +3,7 @@
   "name": "Generic Camera",
   "config_flow": true,
   "requirements": ["ha-av==10.0.0b5", "pillow==9.2.0"],
+  "dependencies": ["http"],
   "documentation": "https://www.home-assistant.io/integrations/generic",
   "codeowners": ["@davet2001"],
   "iot_class": "local_push"

homeassistant/components/generic/strings.json

@@ -40,8 +40,12 @@
           "content_type": "Content Type"
         }
       },
-      "confirm": {
+      "user_confirm_still": {
-        "description": "[%key:common::config_flow::description::confirm_setup%]"
+        "title": "Preview",
+        "description": "![Camera Still Image Preview]({preview_url})",
+        "data": {
+          "confirmed_ok": "This image looks good."
+        }
       }
     }
   },

homeassistant/components/generic/translations/en.json

@@ -23,9 +23,6 @@
             "unknown": "Unexpected error"
         },
         "step": {
-            "confirm": {
-                "description": "Do you want to start set up?"
-            },
             "content_type": {
                 "data": {
                     "content_type": "Content Type"
@@ -45,6 +42,13 @@
                     "verify_ssl": "Verify SSL certificate"
                 },
                 "description": "Enter the settings to connect to the camera."
+            },
+            "user_confirm_still": {
+                "data": {
+                    "confirmed_ok": "This image looks good."
+                },
+                "description": "![Camera Still Image Preview]({preview_url})",
+                "title": "Preview"
             }
         }
     },

tests/components/generic/conftest.py

@@ -78,7 +78,6 @@ def mock_create_stream():
 @pytest.fixture
 async def user_flow(hass):
     """Initiate a user flow."""
-
     result = await hass.config_entries.flow.async_init(
         DOMAIN, context={"source": config_entries.SOURCE_USER}
     )

tests/components/generic/test_camera.py

@@ -20,6 +20,7 @@ from homeassistant.components.generic.const import (
     CONF_STREAM_SOURCE,
     DOMAIN,
 )
+from homeassistant.components.stream.const import CONF_RTSP_TRANSPORT
 from homeassistant.components.websocket_api.const import TYPE_RESULT
 from homeassistant.config_entries import SOURCE_IMPORT
 from homeassistant.const import CONF_PASSWORD, CONF_USERNAME, CONF_VERIFY_SSL
@@ -209,6 +210,7 @@ async def test_stream_source(hass, hass_client, hass_ws_client, fakeimgbytes_png
             CONF_VERIFY_SSL: False,
             CONF_USERNAME: "barney",
             CONF_PASSWORD: "betty",
+            CONF_RTSP_TRANSPORT: "http",
         },
     )
     mock_entry.add_to_hass(hass)

tests/components/generic/test_config_flow.py

@@ -1,8 +1,9 @@
 """Test The generic (IP Camera) config flow."""
 
 import errno
+from http import HTTPStatus
 import os.path
-from unittest.mock import AsyncMock, patch
+from unittest.mock import AsyncMock, PropertyMock, patch
 
 import httpx
 import pytest
@@ -12,6 +13,7 @@ from homeassistant import config_entries, data_entry_flow
 from homeassistant.components.camera import async_get_image
 from homeassistant.components.generic.config_flow import slug
 from homeassistant.components.generic.const import (
+    CONF_CONFIRMED_OK,
     CONF_CONTENT_TYPE,
     CONF_FRAMERATE,
     CONF_LIMIT_REFETCH_TO_URL_CHANGE,
@@ -58,16 +60,30 @@ TESTDATA_YAML = {
 
 
 @respx.mock
-async def test_form(hass, fakeimg_png, user_flow, mock_create_stream):
+async def test_form(hass, fakeimgbytes_png, hass_client, user_flow, mock_create_stream):
     """Test the form with a normal set of settings."""
 
+    respx.get("http://127.0.0.1/testurl/1").respond(stream=fakeimgbytes_png)
     with mock_create_stream as mock_setup, patch(
         "homeassistant.components.generic.async_setup_entry", return_value=True
     ) as mock_setup_entry:
-        result2 = await hass.config_entries.flow.async_configure(
+        result1 = await hass.config_entries.flow.async_configure(
             user_flow["flow_id"],
             TESTDATA,
         )
+        assert result1["type"] == data_entry_flow.FlowResultType.FORM
+        assert result1["step_id"] == "user_confirm_still"
+        client = await hass_client()
+        preview_id = result1["flow_id"]
+        # Check the preview image works.
+        resp = await client.get(f"/api/generic/preview_flow_image/{preview_id}?t=1")
+        assert resp.status == HTTPStatus.OK
+        assert await resp.read() == fakeimgbytes_png
+        result2 = await hass.config_entries.flow.async_configure(
+            result1["flow_id"],
+            user_input={CONF_CONFIRMED_OK: True},
+        )
+        await hass.async_block_till_done()
     assert result2["type"] == data_entry_flow.FlowResultType.CREATE_ENTRY
     assert result2["title"] == "127_0_0_1"
     assert result2["options"] == {
@@ -83,6 +99,9 @@ async def test_form(hass, fakeimg_png, user_flow, mock_create_stream):
     }
 
     await hass.async_block_till_done()
+    # Check that the preview image is disabled after.
+    resp = await client.get(f"/api/generic/preview_flow_image/{preview_id}")
+    assert resp.status == HTTPStatus.NOT_FOUND
     assert len(mock_setup.mock_calls) == 1
     assert len(mock_setup_entry.mock_calls) == 1
 
@@ -99,11 +118,17 @@ async def test_form_only_stillimage(hass, fakeimg_png, user_flow):
     data = TESTDATA.copy()
     data.pop(CONF_STREAM_SOURCE)
     with patch("homeassistant.components.generic.async_setup_entry", return_value=True):
-        result2 = await hass.config_entries.flow.async_configure(
+        result1 = await hass.config_entries.flow.async_configure(
             user_flow["flow_id"],
             data,
         )
         await hass.async_block_till_done()
+        assert result1["type"] == data_entry_flow.FlowResultType.FORM
+        assert result1["step_id"] == "user_confirm_still"
+        result2 = await hass.config_entries.flow.async_configure(
+            result1["flow_id"],
+            user_input={CONF_CONFIRMED_OK: True},
+        )
     assert result2["type"] == data_entry_flow.FlowResultType.CREATE_ENTRY
     assert result2["title"] == "127_0_0_1"
     assert result2["options"] == {
@@ -120,16 +145,65 @@ async def test_form_only_stillimage(hass, fakeimg_png, user_flow):
     assert respx.calls.call_count == 1
 
 
+@respx.mock
+async def test_form_reject_still_preview(
+    hass, fakeimgbytes_png, mock_create_stream, user_flow
+):
+    """Test we go back to the config screen if the user rejects the still preview."""
+    respx.get("http://127.0.0.1/testurl/1").respond(stream=fakeimgbytes_png)
+    with mock_create_stream:
+        result1 = await hass.config_entries.flow.async_configure(
+            user_flow["flow_id"],
+            TESTDATA,
+        )
+    assert result1["type"] == data_entry_flow.RESULT_TYPE_FORM
+    assert result1["step_id"] == "user_confirm_still"
+    result2 = await hass.config_entries.flow.async_configure(
+        result1["flow_id"],
+        user_input={CONF_CONFIRMED_OK: False},
+    )
+    assert result2["type"] == data_entry_flow.RESULT_TYPE_FORM
+    assert result2["step_id"] == "user"
+
+
+@respx.mock
+async def test_form_still_preview_cam_off(
+    hass, fakeimg_png, mock_create_stream, user_flow, hass_client
+):
+    """Test camera errors are triggered during preview."""
+    with patch(
+        "homeassistant.components.generic.camera.GenericCamera.is_on",
+        new_callable=PropertyMock(return_value=False),
+    ), mock_create_stream:
+        result1 = await hass.config_entries.flow.async_configure(
+            user_flow["flow_id"],
+            TESTDATA,
+        )
+        assert result1["type"] == data_entry_flow.RESULT_TYPE_FORM
+        assert result1["step_id"] == "user_confirm_still"
+        preview_id = result1["flow_id"]
+        # Try to view the image, should be unavailable.
+        client = await hass_client()
+        resp = await client.get(f"/api/generic/preview_flow_image/{preview_id}?t=1")
+    assert resp.status == HTTPStatus.SERVICE_UNAVAILABLE
+
+
 @respx.mock
 async def test_form_only_stillimage_gif(hass, fakeimg_gif, user_flow):
     """Test we complete ok if the user wants a gif."""
     data = TESTDATA.copy()
     data.pop(CONF_STREAM_SOURCE)
     with patch("homeassistant.components.generic.async_setup_entry", return_value=True):
-        result2 = await hass.config_entries.flow.async_configure(
+        result1 = await hass.config_entries.flow.async_configure(
             user_flow["flow_id"],
             data,
         )
+        assert result1["type"] == data_entry_flow.RESULT_TYPE_FORM
+        assert result1["step_id"] == "user_confirm_still"
+        result2 = await hass.config_entries.flow.async_configure(
+            result1["flow_id"],
+            user_input={CONF_CONFIRMED_OK: True},
+        )
         await hass.async_block_till_done()
     assert result2["type"] == data_entry_flow.FlowResultType.CREATE_ENTRY
     assert result2["options"][CONF_CONTENT_TYPE] == "image/gif"
@@ -143,11 +217,17 @@ async def test_form_only_svg_whitespace(hass, fakeimgbytes_svg, user_flow):
     data = TESTDATA.copy()
     data.pop(CONF_STREAM_SOURCE)
     with patch("homeassistant.components.generic.async_setup_entry", return_value=True):
-        result2 = await hass.config_entries.flow.async_configure(
+        result1 = await hass.config_entries.flow.async_configure(
             user_flow["flow_id"],
             data,
         )
-        await hass.async_block_till_done()
+        assert result1["type"] == data_entry_flow.FlowResultType.FORM
+        assert result1["step_id"] == "user_confirm_still"
+        result2 = await hass.config_entries.flow.async_configure(
+            result1["flow_id"],
+            user_input={CONF_CONFIRMED_OK: True},
+        )
+    await hass.async_block_till_done()
     assert result2["type"] == data_entry_flow.FlowResultType.CREATE_ENTRY
 
 
@@ -170,10 +250,16 @@ async def test_form_only_still_sample(hass, user_flow, image_file):
     data = TESTDATA.copy()
     data.pop(CONF_STREAM_SOURCE)
     with patch("homeassistant.components.generic.async_setup_entry", return_value=True):
-        result2 = await hass.config_entries.flow.async_configure(
+        result1 = await hass.config_entries.flow.async_configure(
             user_flow["flow_id"],
             data,
         )
+        assert result1["type"] == data_entry_flow.RESULT_TYPE_FORM
+        assert result1["step_id"] == "user_confirm_still"
+        result2 = await hass.config_entries.flow.async_configure(
+            result1["flow_id"],
+            user_input={CONF_CONFIRMED_OK: True},
+        )
         await hass.async_block_till_done()
     assert result2["type"] == data_entry_flow.FlowResultType.CREATE_ENTRY
 
@@ -186,31 +272,31 @@ async def test_form_only_still_sample(hass, user_flow, image_file):
         (
             "http://localhost:812{{3}}/static/icons/favicon-apple-180x180.png",
             "http://localhost:8123/static/icons/favicon-apple-180x180.png",
-            data_entry_flow.FlowResultType.CREATE_ENTRY,
+            "user_confirm_still",
             None,
         ),
         (
             "{% if 1 %}https://bla{% else %}https://yo{% endif %}",
             "https://bla/",
-            data_entry_flow.FlowResultType.CREATE_ENTRY,
+            "user_confirm_still",
             None,
         ),
         (
             "http://{{example.org",
             "http://example.org",
-            data_entry_flow.FlowResultType.FORM,
+            "user",
             {"still_image_url": "template_error"},
         ),
         (
             "invalid1://invalid:4\\1",
             "invalid1://invalid:4%5c1",
-            data_entry_flow.FlowResultType.FORM,
+            "user",
             {"still_image_url": "malformed_url"},
         ),
         (
             "relative/urls/are/not/allowed.jpg",
             "relative/urls/are/not/allowed.jpg",
-            data_entry_flow.FlowResultType.FORM,
+            "user",
             {"still_image_url": "relative_url"},
         ),
     ],
@@ -229,7 +315,7 @@ async def test_still_template(
             data,
         )
         await hass.async_block_till_done()
-    assert result2["type"] == expected_result
+    assert result2["step_id"] == expected_result
     assert result2.get("errors") == expected_errors
 
 
@@ -242,10 +328,15 @@ async def test_form_rtsp_mode(hass, fakeimg_png, user_flow, mock_create_stream):
     with mock_create_stream as mock_setup, patch(
         "homeassistant.components.generic.async_setup_entry", return_value=True
     ):
-        result2 = await hass.config_entries.flow.async_configure(
+        result1 = await hass.config_entries.flow.async_configure(
             user_flow["flow_id"], data
         )
-    assert "errors" not in result2, f"errors={result2['errors']}"
+        assert result1["type"] == data_entry_flow.RESULT_TYPE_FORM
+        assert result1["step_id"] == "user_confirm_still"
+        result2 = await hass.config_entries.flow.async_configure(
+            result1["flow_id"],
+            user_input={CONF_CONFIRMED_OK: True},
+        )
     assert result2["type"] == data_entry_flow.FlowResultType.CREATE_ENTRY
     assert result2["title"] == "127_0_0_1"
     assert result2["options"] == {
@@ -265,21 +356,23 @@ async def test_form_rtsp_mode(hass, fakeimg_png, user_flow, mock_create_stream):
     assert len(mock_setup.mock_calls) == 1
 
 
-async def test_form_only_stream(hass, fakeimgbytes_jpg, mock_create_stream):
+async def test_form_only_stream(hass, fakeimgbytes_jpg, user_flow, mock_create_stream):
     """Test we complete ok if the user wants stream only."""
-    result = await hass.config_entries.flow.async_init(
-        DOMAIN, context={"source": config_entries.SOURCE_USER}
-    )
     data = TESTDATA.copy()
     data.pop(CONF_STILL_IMAGE_URL)
     data[CONF_STREAM_SOURCE] = "rtsp://user:pass@127.0.0.1/testurl/2"
     with mock_create_stream as mock_setup:
-        result3 = await hass.config_entries.flow.async_configure(
+        result1 = await hass.config_entries.flow.async_configure(
-            result["flow_id"],
+            user_flow["flow_id"],
             data,
         )
+        assert result1["type"] == data_entry_flow.RESULT_TYPE_FORM
+        assert result1["step_id"] == "user_confirm_still"
+        result3 = await hass.config_entries.flow.async_configure(
+            result1["flow_id"],
+            user_input={CONF_CONFIRMED_OK: True},
+        )
         await hass.async_block_till_done()
-
     assert result3["type"] == data_entry_flow.FlowResultType.CREATE_ENTRY
     assert result3["title"] == "127_0_0_1"
     assert result3["options"] == {