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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Generic fix stream thumbnail #69378
Generic fix stream thumbnail #69378
Conversation
vol.Optional( | ||
CONF_CONTENT_TYPE, | ||
description={ | ||
"suggested_value": user_input.get(CONF_CONTENT_TYPE, "image/jpeg") | ||
}, | ||
): str, |
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.
Why are we asking the user for input in that case? Do we know when only a stream is given already?
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.
The autodetect for content type does not work yet for streams.
This means if the user wants to configure only a stream (no still image) with a content type that is not the default jpeg, then we need to get that setting from the user.
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 make that a second step, and only show it if we know it's a content type that can be ambiguous ?
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.
We definitely can move it in a second step when we know the still image URL is empty.
@balloob Please let me know if this is what you had in mind and I'll add tests for the new code. I have tested manually ok. |
return self.async_show_form( | ||
step_id="user2", | ||
data_schema=build_schema2({}), | ||
errors={}, |
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.
Just call the step without user input, and let the step then generate the form if no input is given.
return self.async_show_form( | |
step_id="user2", | |
data_schema=build_schema2({}), | |
errors={}, | |
return await self.async_step_user2( |
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.
Ok done. 92bb1cb
Proposed change
The new generic camera config flow autodetects image type for still images. But it is not automatic for streams. So if a camera only had a
stream_url
and nostill_image_url
, the content_type would be undefined. This caused the thumbnail to fail for a stream #69128.This PR fixes this by reinstating the
content_type
field, and defaulting to the user value if the autodetect is skipped.It also enables the value to be read from YAML.
A small fix is made to the duplicate checking code which was triggering log errors.
Type of change
Additional information
Checklist
black --fast 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
..coveragerc
.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: