Skip to content

Add ssl option to the concord232 config flow - #179588

Draft
bharat wants to merge 17 commits into
home-assistant:devfrom
bharat:feat/concord232-config-flow-ssl
Draft

Add ssl option to the concord232 config flow#179588
bharat wants to merge 17 commits into
home-assistant:devfrom
bharat:feat/concord232-config-flow-ssl

Conversation

@bharat

@bharat bharat commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Proposed change

Adds an ssl option to the concord232 config flow, so the integration can connect to a concord232 server behind a TLS-terminating reverse proxy over HTTPS.

Stacked on #179587; only the last commit ("Add ssl option to the concord232 config flow") is new here. Entries created before this option default to http via data.get(CONF_SSL, False), covered by a test.

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 diff between library versions and ideally a link to the changelog/release notes is added to the PR description.

To help with the load of incoming pull requests:

Copilot AI balanced review requested due to automatic review settings August 19, 2026 18:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds config-entry support and optional HTTPS connectivity to Concord232.

Changes:

  • Adds config/options flows with YAML migration and SSL support.
  • Introduces shared coordinator-based polling and entities.
  • Updates metadata, translations, and tests.

Reviewed changes

Copilot reviewed 12 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
homeassistant/components/concord232/__init__.py Sets up entries and builds endpoint URLs.
homeassistant/components/concord232/alarm_control_panel.py Migrates the alarm panel to coordinator-backed setup.
homeassistant/components/concord232/binary_sensor.py Migrates zone sensors to coordinator entities.
homeassistant/components/concord232/config_flow.py Adds configuration, import, SSL, and options flows.
homeassistant/components/concord232/const.py Defines shared constants.
homeassistant/components/concord232/coordinator.py Adds shared polling and error handling.
homeassistant/components/concord232/manifest.json Enables config-flow metadata.
homeassistant/components/concord232/strings.json Adds flow and repair translations.
homeassistant/generated/config_flows.py Registers the generated config flow.
homeassistant/generated/integrations.json Updates generated integration metadata.
tests/components/concord232/conftest.py Adds shared entry and client fixtures.
tests/components/concord232/test_alarm_control_panel.py Tests coordinator-backed alarm behavior.
tests/components/concord232/test_binary_sensor.py Tests zone entities and polling.
tests/components/concord232/test_config_flow.py Tests flow, migration, SSL, and compatibility.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread homeassistant/components/concord232/alarm_control_panel.py Outdated
Comment thread homeassistant/components/concord232/manifest.json Outdated
Comment thread homeassistant/components/concord232/__init__.py Outdated
Copilot AI review requested due to automatic review settings August 19, 2026 18:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 12 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (3)

homeassistant/components/concord232/alarm_control_panel.py:121

  • Normalize an empty alarm code to None. The YAML import preserves an explicitly empty code, and this value is then treated as configured by _attr_code_arm_required and _validate_code, so codeless arming no longer works despite the option description promising it will.
        code: str | None = entry.options.get(CONF_CODE)

homeassistant/components/concord232/config_flow.py:127

  • Reload the entry when options are saved. Concord232Alarm snapshots CONF_CODE and CONF_MODE in its constructor, but plain OptionsFlow only updates storage, so the loaded entity continues using the old code and mode until a restart or manual reload; inherit from OptionsFlowWithReload instead.
class Concord232OptionsFlow(OptionsFlow):

homeassistant/components/concord232/config_flow.py:36

  • Validate the port with cv.port. Using int accepts values outside 1–65535, which are reported as a connection failure rather than rejected as invalid input; ness_alarm/config_flow.py:47 and aqualogic/config_flow.py:19 establish the config-flow convention.
        vol.Required(CONF_PORT, default=DEFAULT_PORT): int,

Copilot AI review requested due to automatic review settings August 19, 2026 19:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 12 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (4)

homeassistant/components/concord232/alarm_control_panel.py:129

  • Do not register the configured code as the entity's default code. AlarmControlPanelEntity substitutes this default whenever an action omits its code, so _validate_code receives the configured value and callers can arm or disarm without supplying the code despite this option being documented as required.
        code: str | None = entry.options.get(CONF_CODE)
        self._code = code
        self._alarm_control_panel_option_default_code = code
        # The panel protocol arms without a code; only require one when the
        # user configured a code to gate arming locally.
        self._attr_code_arm_required = code is not None

homeassistant/components/concord232/init.py:34

  • Reload the entry after options change. The alarm entity copies CONF_CODE and CONF_MODE only in its constructor, so saving the options flow updates storage but leaves the running entity on the old values until restart.
    entry.runtime_data = coordinator
    await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)

homeassistant/components/concord232/config_flow.py:110

  • Coordinate the two YAML platform imports before checking for an existing entry. Alarm and binary-sensor platforms are set up concurrently, so both import flows can pass this check before either creates an entry; without a unique ID this can create duplicate entries, and the binary-sensor import can also win while dropping the alarm-only name/code/mode. Use one serialized import path that merges both platform configurations and cover the combined YAML case.
        self._async_abort_entries_match(
            {CONF_HOST: data[CONF_HOST], CONF_PORT: data[CONF_PORT]}

homeassistant/components/concord232/manifest.json:5

  • Restore the omitted dependency-related checklist items in the PR description. The required PR template retains the unchecked “New or updated dependencies…” and dependency changelog/release-notes checkboxes even when they do not apply, but both are missing here.
  "config_flow": true,

Copilot AI review requested due to automatic review settings August 20, 2026 15:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 12 out of 14 changed files in this pull request and generated 1 comment.

Suppressed comments (5)

homeassistant/components/concord232/alarm_control_panel.py:179

  • Report a failed arm-away command instead of treating it as successful. Client.arm() returns False for a non-200 response, but this result is discarded and the service completes normally; check the result and raise a translated HomeAssistantError before refreshing.
        await self.hass.async_add_executor_job(self.coordinator.client.arm, "away")

homeassistant/components/concord232/alarm_control_panel.py:171

  • Report a failed arm-home command instead of treating it as successful. Both Client.arm() calls return False on a non-200 response, but the result is discarded and the service completes normally; check the result and raise a translated HomeAssistantError before refreshing.
            await self.hass.async_add_executor_job(
                self.coordinator.client.arm, "stay", "silent"
            )
        else:
            await self.hass.async_add_executor_job(self.coordinator.client.arm, "stay")

homeassistant/components/concord232/config_flow.py:129

  • Preserve the alarm YAML name when merging the second platform import. If the binary-sensor platform creates the entry first, this branch merges only the alarm options, so CONF_NAME is discarded and the migrated entry/panel is named from the host instead; update the entry title from import_data as part of this merge.
            if options:
                self.hass.config_entries.async_update_entry(
                    entry, options={**entry.options, **options}
                )
            return self.async_abort(reason="already_configured")

homeassistant/components/concord232/config_flow.py:90

  • Add a bounded timeout to the connection probe. The current concord232.Client performs requests.Session.get without a timeout, so a peer or TLS proxy that accepts a connection but never responds can leave this config flow pending indefinitely instead of returning cannot_connect; add request-timeout support in the client dependency and use it here.
            if await self.hass.async_add_executor_job(_try_connect, url):

homeassistant/components/concord232/coordinator.py:59

  • Ensure each coordinator poll has a request-level timeout. The current client’s HTTP calls have no timeout, so a server or reverse proxy that accepts but stops responding can leave this refresh stuck forever and prevent entities from becoming unavailable; add timeout support upstream and configure it for these calls.
            partitions = self.client.list_partitions()
            zones = self.client.list_zones()

Comment thread homeassistant/components/concord232/alarm_control_panel.py Outdated
Copilot AI review requested due to automatic review settings August 20, 2026 21:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 12 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (3)

homeassistant/components/concord232/config_flow.py:128

  • Preserve the alarm platform's imported name when merging into an entry created by the binary-sensor import. Because these imports can run in either order, a binary-sensor-first import leaves the title as the host and loses CONF_NAME; the alarm entity then gets a different name/entity ID depending on setup order. Update the title when CONF_NAME is present and cover this order in the merge test.
            if options:
                self.hass.config_entries.async_update_entry(
                    entry, options={**entry.options, **options}
                )

homeassistant/components/concord232/binary_sensor.py:90

  • Preserve legacy binary-sensor entity IDs during automatic YAML migration, or classify this as a breaking change. YAML sensors previously used the zone name directly (for example, binary_sensor.zone_1), while enabling entity names under the new named device prefixes IDs (the new tests expect binary_sensor.localhost_front_door), silently breaking existing automations even though the PR is marked as a non-breaking new feature. Add an entity-ID migration or retain legacy IDs for imported entries.
    _attr_has_entity_name = True

homeassistant/components/concord232/binary_sensor.py:132

  • Normalize the fork-specific zone state in the concord232 library instead of in the integration. Accepting two raw server response shapes here is protocol compatibility logic; exposing one stable shape from the dependency keeps the integration thin and avoids retaining raw-shape branching in every consumer. Update the dependency and simplify this property.
        # The original concord232 server reports zone state as a string; the
        # actively maintained fork reports a list of states. Accept both.
        state = zone["state"]
        states = state if isinstance(state, list) else [state]
        return states != ["Normal"]

Copilot AI review requested due to automatic review settings August 20, 2026 22:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 12 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (5)

homeassistant/components/concord232/config_flow.py:89

  • Handle malformed endpoint values as form errors. URL.build raises ValueError for inputs such as a pasted https://… URL (and potentially an invalid port), so the flow currently crashes instead of redisplaying cannot_connect.
            url = build_url(
                user_input[CONF_HOST], user_input[CONF_PORT], user_input[CONF_SSL]
            )

homeassistant/components/concord232/config_flow.py:129

  • Preserve the imported alarm name when merging the companion platform import. Because the alarm and binary-sensor imports race by design, a binary-sensor-first import creates the entry with the host title and this branch only merges code/mode, so CONF_NAME and the resulting alarm entity ID depend on startup order.
            if options:
                self.hass.config_entries.async_update_entry(
                    entry, options={**entry.options, **options}
                )
            return self.async_abort(reason="already_configured")

homeassistant/components/concord232/config_flow.py:133

  • Convert malformed imported endpoint values into the expected import abort. build_url can raise ValueError before _try_connect runs, which bypasses the cannot_connect repair path and fails YAML platform setup instead.
        url = build_url(data[CONF_HOST], data[CONF_PORT], data[CONF_SSL])
        if not await self.hass.async_add_executor_job(_try_connect, url):
            return self.async_abort(reason="cannot_connect")

homeassistant/components/concord232/config_flow.py:59

  • Add a bounded timeout to connection validation. The client’s list_partitions() uses requests.Session.get() without a timeout, so a black-holed host can leave this config flow waiting indefinitely and consume an executor worker; the dependency should expose a request timeout and this call should use it.

This issue also appears in the following locations of the same file:

  • line 87
  • line 125
  • line 131
def _try_connect(url: str) -> bool:
    """Return True when the Concord232 server answers."""
    try:
        concord232_client.Client(url).list_partitions()
    except requests.exceptions.RequestException:
        return False

homeassistant/components/concord232/coordinator.py:59

  • Use bounded request timeouts for coordinator polling as well. Both client methods call requests.Session.get() without a timeout, so an unresponsive endpoint can leave first setup or a later refresh stuck indefinitely rather than marking the entities unavailable.
            partitions = self.client.list_partitions()
            zones = self.client.list_zones()

Copilot AI review requested due to automatic review settings August 20, 2026 23:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 12 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (2)

homeassistant/components/concord232/config_flow.py:38

  • Validate the config-flow port with cv.port. The legacy schemas already use cv.port (binary_sensor.py:44 and alarm_control_panel.py:46), while a plain int accepts negative and out-of-range ports and defers them to URL/request handling instead of rejecting them in the form.
        vol.Required(CONF_PORT, default=DEFAULT_PORT): int,

homeassistant/components/concord232/init.py:24

  • Add a setup-path test for an entry with CONF_SSL set to True. The current SSL test only exercises config-flow validation, while setup coverage only uses missing/false SSL, so a regression that makes async_setup_entry ignore this value would still pass.
    url = build_url(
        entry.data[CONF_HOST], entry.data[CONF_PORT], entry.data.get(CONF_SSL, False)
    )

Copilot AI review requested due to automatic review settings August 20, 2026 23:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 12 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (1)

homeassistant/components/concord232/config_flow.py:128

  • Preserve CONF_NAME when the alarm YAML import merges into an entry created by the binary-sensor import. Because these platform imports can run in either order, the binary sensor can create a host-titled entry first; merging only code and mode then silently loses the configured alarm name and makes the migrated panel name depend on import order.
            if options:
                self.hass.config_entries.async_update_entry(
                    entry, options={**entry.options, **options}
                )

@bharat
bharat marked this pull request as ready for review August 21, 2026 00:00
@bharat
bharat marked this pull request as draft August 21, 2026 00:00
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.

2 participants