From be8e28503ce33dff219c8ad486513183ca438ec4 Mon Sep 17 00:00:00 2001 From: Dave T <17680170+davet2001@users.noreply.github.com> Date: Thu, 7 Apr 2022 23:01:29 +0100 Subject: [PATCH] Generic fix stream thumbnail (#69378) --- .../components/generic/config_flow.py | 115 ++++++++++++++---- homeassistant/components/generic/strings.json | 14 ++- .../components/generic/translations/en.json | 14 ++- tests/components/generic/test_config_flow.py | 63 +++++++++- 4 files changed, 173 insertions(+), 33 deletions(-) diff --git a/homeassistant/components/generic/config_flow.py b/homeassistant/components/generic/config_flow.py index ce8caa457ec..df8946ccbad 100644 --- a/homeassistant/components/generic/config_flow.py +++ b/homeassistant/components/generic/config_flow.py @@ -109,6 +109,20 @@ def build_schema( return vol.Schema(spec) +def build_schema_content_type(user_input: dict[str, Any] | MappingProxyType[str, Any]): + """Create schema for conditional 2nd page specifying stream content_type.""" + return vol.Schema( + { + vol.Required( + CONF_CONTENT_TYPE, + description={ + "suggested_value": user_input.get(CONF_CONTENT_TYPE, "image/jpeg") + }, + ): str, + } + ) + + def get_image_type(image): """Get the format of downloaded bytes that could be an image.""" fmt = None @@ -129,7 +143,7 @@ async def async_test_still(hass, info) -> tuple[dict[str, str], str | None]: """Verify that the still image is valid before we create an entity.""" fmt = None if not (url := info.get(CONF_STILL_IMAGE_URL)): - return {}, None + return {}, info.get(CONF_CONTENT_TYPE, "image/jpeg") if not isinstance(url, template_helper.Template) and url: url = cv.template(url) url.hass = hass @@ -228,6 +242,11 @@ class GenericIPCamConfigFlow(ConfigFlow, domain=DOMAIN): VERSION = 1 + def __init__(self): + """Initialize Generic ConfigFlow.""" + self.cached_user_input: dict[str, Any] = {} + self.cached_title = "" + @staticmethod def async_get_options_flow( config_entry: ConfigEntry, @@ -238,8 +257,8 @@ class GenericIPCamConfigFlow(ConfigFlow, domain=DOMAIN): def check_for_existing(self, options): """Check whether an existing entry is using the same URLs.""" return any( - entry.options[CONF_STILL_IMAGE_URL] == options[CONF_STILL_IMAGE_URL] - and entry.options[CONF_STREAM_SOURCE] == options[CONF_STREAM_SOURCE] + 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() ) @@ -264,10 +283,17 @@ class GenericIPCamConfigFlow(ConfigFlow, domain=DOMAIN): if not errors: user_input[CONF_CONTENT_TYPE] = still_format user_input[CONF_LIMIT_REFETCH_TO_URL_CHANGE] = False - await self.async_set_unique_id(self.flow_id) - return self.async_create_entry( - title=name, data={}, options=user_input - ) + if user_input.get(CONF_STILL_IMAGE_URL): + await self.async_set_unique_id(self.flow_id) + return self.async_create_entry( + title=name, data={}, options=user_input + ) + # If user didn't specify a still image URL, + # we can't (yet) autodetect it from the stream. + # Show a conditional 2nd page to ask them the content type. + self.cached_user_input = user_input + self.cached_title = name + return await self.async_step_content_type() else: user_input = DEFAULT_DATA.copy() @@ -277,6 +303,22 @@ class GenericIPCamConfigFlow(ConfigFlow, domain=DOMAIN): errors=errors, ) + async def async_step_content_type( + self, user_input: dict[str, Any] | None = None + ) -> FlowResult: + """Handle the user's choice for stream content_type.""" + if user_input is not None: + user_input = self.cached_user_input | user_input + await self.async_set_unique_id(self.flow_id) + return self.async_create_entry( + title=self.cached_title, data={}, options=user_input + ) + return self.async_show_form( + step_id="content_type", + data_schema=build_schema_content_type({}), + errors={}, + ) + async def async_step_import(self, import_config) -> FlowResult: """Handle config import from yaml.""" # abort if we've already got this one. @@ -316,6 +358,8 @@ class GenericOptionsFlowHandler(OptionsFlow): def __init__(self, config_entry: ConfigEntry) -> None: """Initialize Generic IP Camera options flow.""" self.config_entry = config_entry + self.cached_user_input: dict[str, Any] = {} + self.cached_title = "" async def async_step_init( self, user_input: dict[str, Any] | None = None @@ -324,29 +368,52 @@ class GenericOptionsFlowHandler(OptionsFlow): errors: dict[str, str] = {} if user_input is not None: - errors, still_format = await async_test_still(self.hass, user_input) + errors, still_format = await async_test_still( + self.hass, self.config_entry.options | user_input + ) errors = errors | await async_test_stream(self.hass, user_input) still_url = user_input.get(CONF_STILL_IMAGE_URL) stream_url = user_input.get(CONF_STREAM_SOURCE) if not errors: - return self.async_create_entry( - title=slug_url(still_url) or slug_url(stream_url) or DEFAULT_NAME, - data={ - CONF_AUTHENTICATION: user_input.get(CONF_AUTHENTICATION), - CONF_STREAM_SOURCE: user_input.get(CONF_STREAM_SOURCE), - CONF_PASSWORD: user_input.get(CONF_PASSWORD), - CONF_STILL_IMAGE_URL: user_input.get(CONF_STILL_IMAGE_URL), - CONF_CONTENT_TYPE: still_format, - CONF_USERNAME: user_input.get(CONF_USERNAME), - CONF_LIMIT_REFETCH_TO_URL_CHANGE: user_input[ - CONF_LIMIT_REFETCH_TO_URL_CHANGE - ], - CONF_FRAMERATE: user_input[CONF_FRAMERATE], - CONF_VERIFY_SSL: user_input[CONF_VERIFY_SSL], - }, - ) + title = slug_url(still_url) or slug_url(stream_url) or DEFAULT_NAME + data = { + CONF_AUTHENTICATION: user_input.get(CONF_AUTHENTICATION), + CONF_STREAM_SOURCE: user_input.get(CONF_STREAM_SOURCE), + CONF_PASSWORD: user_input.get(CONF_PASSWORD), + CONF_STILL_IMAGE_URL: user_input.get(CONF_STILL_IMAGE_URL), + CONF_CONTENT_TYPE: still_format + or self.config_entry.options.get(CONF_CONTENT_TYPE), + CONF_USERNAME: user_input.get(CONF_USERNAME), + CONF_LIMIT_REFETCH_TO_URL_CHANGE: user_input[ + CONF_LIMIT_REFETCH_TO_URL_CHANGE + ], + CONF_FRAMERATE: user_input[CONF_FRAMERATE], + CONF_VERIFY_SSL: user_input[CONF_VERIFY_SSL], + } + if still_url: + return self.async_create_entry( + title=title, + data=data, + ) + self.cached_title = title + self.cached_user_input = data + return await self.async_step_content_type() + return self.async_show_form( step_id="init", data_schema=build_schema(user_input or self.config_entry.options, True), errors=errors, ) + + async def async_step_content_type( + self, user_input: dict[str, Any] | None = None + ) -> FlowResult: + """Handle the user's choice for stream content_type.""" + if user_input is not None: + user_input = self.cached_user_input | user_input + return self.async_create_entry(title=self.cached_title, data=user_input) + return self.async_show_form( + step_id="content_type", + data_schema=build_schema_content_type(self.cached_user_input), + errors={}, + ) diff --git a/homeassistant/components/generic/strings.json b/homeassistant/components/generic/strings.json index eb1bfcc3c55..01b1fe48a82 100644 --- a/homeassistant/components/generic/strings.json +++ b/homeassistant/components/generic/strings.json @@ -30,11 +30,16 @@ "limit_refetch_to_url_change": "Limit refetch to url change", "password": "[%key:common::config_flow::data::password%]", "username": "[%key:common::config_flow::data::username%]", - "content_type": "Content Type", "framerate": "Frame Rate (Hz)", "verify_ssl": "[%key:common::config_flow::data::verify_ssl%]" } }, + "content_type": { + "description": "Specify the content type for the stream.", + "data": { + "content_type": "Content Type" + } + }, "confirm": { "description": "[%key:common::config_flow::description::confirm_setup%]" } @@ -51,10 +56,15 @@ "limit_refetch_to_url_change": "[%key:component::generic::config::step::user::data::limit_refetch_to_url_change%]", "password": "[%key:common::config_flow::data::password%]", "username": "[%key:common::config_flow::data::username%]", - "content_type": "[%key:component::generic::config::step::user::data::content_type%]", "framerate": "[%key:component::generic::config::step::user::data::framerate%]", "verify_ssl": "[%key:common::config_flow::data::verify_ssl%]" } + }, + "content_type": { + "description": "[%key:component::generic::config::step::content_type::description%]", + "data": { + "content_type": "[%key:component::generic::config::step::content_type::data::content_type%]" + } } }, "error": { diff --git a/homeassistant/components/generic/translations/en.json b/homeassistant/components/generic/translations/en.json index 5346c2b5106..478ea76b476 100644 --- a/homeassistant/components/generic/translations/en.json +++ b/homeassistant/components/generic/translations/en.json @@ -23,10 +23,15 @@ "confirm": { "description": "Do you want to start set up?" }, + "content_type": { + "data": { + "content_type": "Content Type" + }, + "description": "Specify the content type for the stream." + }, "user": { "data": { "authentication": "Authentication", - "content_type": "Content Type", "framerate": "Frame Rate (Hz)", "limit_refetch_to_url_change": "Limit refetch to url change", "password": "Password", @@ -57,10 +62,15 @@ "unknown": "Unexpected error" }, "step": { + "content_type": { + "data": { + "content_type": "Content Type" + }, + "description": "Specify the content type for the stream." + }, "init": { "data": { "authentication": "Authentication", - "content_type": "Content Type", "framerate": "Frame Rate (Hz)", "limit_refetch_to_url_change": "Limit refetch to url change", "password": "Password", diff --git a/tests/components/generic/test_config_flow.py b/tests/components/generic/test_config_flow.py index 7e6c55dafff..548b09cf0bb 100644 --- a/tests/components/generic/test_config_flow.py +++ b/tests/components/generic/test_config_flow.py @@ -10,6 +10,7 @@ import pytest import respx from homeassistant import config_entries, data_entry_flow, setup +from homeassistant.components.camera import async_get_image from homeassistant.components.generic.const import ( CONF_CONTENT_TYPE, CONF_FRAMERATE, @@ -191,7 +192,7 @@ async def test_form_rtsp_mode(hass, fakeimg_png, mock_av_open, user_flow): assert len(mock_setup.mock_calls) == 1 -async def test_form_only_stream(hass, mock_av_open): +async def test_form_only_stream(hass, mock_av_open, fakeimgbytes_jpg): """Test we complete ok if the user wants stream only.""" await setup.async_setup_component(hass, "persistent_notification", {}) result = await hass.config_entries.flow.async_init( @@ -204,21 +205,34 @@ async def test_form_only_stream(hass, mock_av_open): result["flow_id"], data, ) - assert result2["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY - assert result2["title"] == "127_0_0_1_testurl_2" - assert result2["options"] == { + assert result2["type"] == data_entry_flow.RESULT_TYPE_FORM + result3 = await hass.config_entries.flow.async_configure( + result2["flow_id"], + {CONF_CONTENT_TYPE: "image/jpeg"}, + ) + + assert result3["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result3["title"] == "127_0_0_1_testurl_2" + assert result3["options"] == { CONF_AUTHENTICATION: HTTP_BASIC_AUTHENTICATION, CONF_STREAM_SOURCE: "http://127.0.0.1/testurl/2", CONF_RTSP_TRANSPORT: "tcp", CONF_USERNAME: "fred_flintstone", CONF_PASSWORD: "bambam", CONF_LIMIT_REFETCH_TO_URL_CHANGE: False, - CONF_CONTENT_TYPE: None, + CONF_CONTENT_TYPE: "image/jpeg", CONF_FRAMERATE: 5, CONF_VERIFY_SSL: False, } await hass.async_block_till_done() + + with patch( + "homeassistant.components.generic.camera.GenericCamera.async_camera_image", + return_value=fakeimgbytes_jpg, + ): + image_obj = await async_get_image(hass, "camera.127_0_0_1_testurl_2") + assert image_obj.content == fakeimgbytes_jpg assert len(mock_setup.mock_calls) == 1 @@ -478,6 +492,45 @@ async def test_options_template_error(hass, fakeimgbytes_png, mock_av_open): assert result4["errors"] == {"still_image_url": "template_error"} +@respx.mock +async def test_options_only_stream(hass, fakeimgbytes_png, mock_av_open): + """Test the options flow without a still_image_url.""" + respx.get("http://127.0.0.1/testurl/2").respond(stream=fakeimgbytes_png) + data = TESTDATA.copy() + data.pop(CONF_STILL_IMAGE_URL) + + mock_entry = MockConfigEntry( + title="Test Camera", + domain=DOMAIN, + data={}, + options=data, + ) + with mock_av_open: + mock_entry.add_to_hass(hass) + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + + result = await hass.config_entries.options.async_init(mock_entry.entry_id) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "init" + + # try updating the config options + result2 = await hass.config_entries.options.async_configure( + result["flow_id"], + user_input=data, + ) + # Should be shown a 2nd form + assert result2["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result2["step_id"] == "content_type" + + result3 = await hass.config_entries.options.async_configure( + result2["flow_id"], + user_input={CONF_CONTENT_TYPE: "image/png"}, + ) + assert result3["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result3["data"][CONF_CONTENT_TYPE] == "image/png" + + # These below can be deleted after deprecation period is finished. @respx.mock async def test_import(hass, fakeimg_png, mock_av_open):