Skip to content

Conversation

@HarlemSquirrel
Copy link
Contributor

@HarlemSquirrel HarlemSquirrel commented Oct 19, 2025

Proposed change

Add config flow for Vivotek integration

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

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:

Copilot AI review requested due to automatic review settings October 19, 2025 00:20
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds config flow support to the existing Vivotek integration, enabling users to configure Vivotek IP cameras through the Home Assistant UI instead of only through YAML configuration. The implementation includes both initial setup and reconfiguration capabilities.

Key changes:

  • Adds config flow implementation with user setup and reconfiguration steps
  • Updates the integration to support both YAML and config entry setup methods
  • Extracts constants to a separate module for better organization

Reviewed Changes

Copilot reviewed 6 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
homeassistant/components/vivotek/config_flow.py New config flow implementation with user and reconfigure steps
homeassistant/components/vivotek/const.py New constants file extracting domain-specific configuration keys
homeassistant/components/vivotek/strings.json New localization strings for config flow UI
homeassistant/components/vivotek/manifest.json Enables config flow support in manifest
homeassistant/components/vivotek/camera.py Updates camera platform to support config entries and refactors existing code
homeassistant/components/vivotek/__init__.py New entry point for config entry setup and unloading

@home-assistant home-assistant bot marked this pull request as draft October 21, 2025 12:39
@home-assistant
Copy link

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

@HarlemSquirrel HarlemSquirrel marked this pull request as ready for review October 30, 2025 03:38
Copy link
Contributor

@erwindouna erwindouna left a comment

Choose a reason for hiding this comment

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

Making good progress, @HarlemSquirrel! I've left some points. :)

Comment on lines 124 to 143
if "ip_address" in import_data:
user_input[CONF_IP_ADDRESS] = import_data["ip_address"]
if "name" in import_data:
user_input[CONF_NAME] = import_data["name"]
if "username" in import_data:
user_input[CONF_USERNAME] = import_data["username"]
if "password" in import_data:
user_input[CONF_PASSWORD] = import_data["password"]
if "authentication" in import_data:
user_input[CONF_AUTHENTICATION] = import_data["authentication"]
if "ssl" in import_data:
user_input[CONF_SSL] = import_data["ssl"]
if "verify_ssl" in import_data:
user_input[CONF_VERIFY_SSL] = import_data["verify_ssl"]
if "framerate" in import_data:
user_input[CONF_FRAMERATE] = import_data["framerate"]
if "security_level" in import_data:
user_input[CONF_SECURITY_LEVEL] = import_data["security_level"]
if "stream_path" in import_data:
user_input[CONF_STREAM_PATH] = import_data["stream_path"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be made to switch-case?

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 don't think so because we need all of these not just one of them

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need so many safe-guards? Aren't there better solutions?

Copy link
Member

Choose a reason for hiding this comment

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

I think it's good to note that the validation of the existing YAML still stands, so we already know what we could expect. So yes, I think we can improve this

sec_lvl=data[CONF_SECURITY_LEVEL], # type: ignore[literal-required]
)
mac = await hass.async_add_executor_job(cam_client.get_mac)
assert len(mac) > 0
Copy link
Member

Choose a reason for hiding this comment

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

Why do we have this assert? What happens when this is the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was using it as a way to fail faster but can remove it

"doc_url": "https://www.home-assistant.io/integrations/vivotek/"
}

CONF_SCHEMA = vol.Schema(
Copy link
Member

Choose a reason for hiding this comment

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

Also note, we have support for section() now. Check telegram_bot for an example. It might help splitting easy stuff from complex stuff.

I would also consider that everything that is not part of the connection details (like host, auth) can be an options flow as it would just be configuration.

Comment on lines 124 to 143
if "ip_address" in import_data:
user_input[CONF_IP_ADDRESS] = import_data["ip_address"]
if "name" in import_data:
user_input[CONF_NAME] = import_data["name"]
if "username" in import_data:
user_input[CONF_USERNAME] = import_data["username"]
if "password" in import_data:
user_input[CONF_PASSWORD] = import_data["password"]
if "authentication" in import_data:
user_input[CONF_AUTHENTICATION] = import_data["authentication"]
if "ssl" in import_data:
user_input[CONF_SSL] = import_data["ssl"]
if "verify_ssl" in import_data:
user_input[CONF_VERIFY_SSL] = import_data["verify_ssl"]
if "framerate" in import_data:
user_input[CONF_FRAMERATE] = import_data["framerate"]
if "security_level" in import_data:
user_input[CONF_SECURITY_LEVEL] = import_data["security_level"]
if "stream_path" in import_data:
user_input[CONF_STREAM_PATH] = import_data["stream_path"]
Copy link
Member

Choose a reason for hiding this comment

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

I think it's good to note that the validation of the existing YAML still stands, so we already know what we could expect. So yes, I think we can improve this

@home-assistant home-assistant bot marked this pull request as draft November 17, 2025 14:20
@HarlemSquirrel HarlemSquirrel marked this pull request as ready for review November 19, 2025 03:46
Copilot finished reviewing on behalf of HarlemSquirrel November 19, 2025 03:47
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 11 out of 13 changed files in this pull request and generated 2 comments.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants