Skip to content

Add Modbus Connection integration - #175407

Merged
balloob merged 18 commits into
devfrom
modbus_connection-integration
Jul 5, 2026
Merged

Add Modbus Connection integration#175407
balloob merged 18 commits into
devfrom
modbus_connection-integration

Conversation

@balloob

@balloob balloob commented Jul 2, 2026

Copy link
Copy Markdown
Member

Breaking change

Proposed change

Implement home-assistant/architecture#1418

A new Modbus Connection integration whose only job is to own Modbus connection config entries (one per physical link) and publish a live, backend-neutral connection that consumer integrations borrow units from.

  • config flow: Network (TCP / RTU-over-TCP) or Serial (RTU, including network serial proxies). A duplicate link aborts before the connection is opened; otherwise the connection is opened to validate it
  • runtime_data holds the live ModbusConnection; the entry owns close() and reloads on connection loss
  • async_get_unit(hass, connection_entry_id, unit_id): the only consumer touchpoint. Returns a backend-neutral ModbusUnit, or raises ConnectionNotReady (a ConfigEntryNotReady, so consumers get setup-retry)

Integration relies on modbus-connection, a new Python package to abstract modbus connections. Source at https://github.com/home-assistant-libs/modbus-connection

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:

Layer 2 of the shared-Modbus-connection design: a new integration whose only job
is to own Modbus connection config entries (one per physical link) and publish a
live, backend-neutral connection that consumer integrations borrow units from.

- config flow: Network (TCP / RTU-over-TCP) or Serial (RTU, including network
  serial proxies). A duplicate link aborts before the connection is opened;
  otherwise the connection is opened to validate it
- runtime_data holds the live ModbusConnection (built via the tmodbus-backed
  modbus_connection.tmodbus connect functions); the entry owns close() and
  reloads on connection loss
- async_get_unit(hass, entry_id, unit_id): the only consumer touchpoint -
  returns a backend-neutral ModbusUnit, or raises ConnectionNotReady (a
  ConfigEntryNotReady, so consumers get setup-retry, and a ModbusError)
- quality_scale.yaml at bronze with strict typing (unique-config-entry exempt:
  Modbus endpoints have no hardware unique ID; entity/action rules exempt: the
  integration has no entities)
- tests: setup/unload, connect-failure retry, reload-on-loss, the config flow
  (network/serial happy paths, cannot_connect, cannot_open_serial_port, and a
  parametrized duplicate check) and the async_get_unit accessor

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016huRs96kGdQbNQopXseC4H
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

Check requirements

Checked at commit ff72927.

⚠️ Some checks require attention — see the details below.

Package Old New No Advisories Not Yanked Repo Public CI Upload Release Pipeline Security PR Link Async Safe
modbus-connection 3.2.0 ☑️
📦 modbus-connection: 3.2.0
  • No Advisories: ✅ No active advisories reported by PyPI for version 3.2.0.
  • Not Yanked: ✅ Version 3.2.0 is a live (non-yanked) release.
  • Repo Public: ✅ https://github.com/home-assistant-libs/modbus-connection is publicly accessible.
  • CI Upload: ✅ Trusted Publisher attestation found (GitHub).
  • Release Pipeline: ✅ OIDC via Trusted Publisher attestation (GitHub); automated CI upload verified by PyPI.
  • Security: ☑️ Baseline scan found nothing obvious in pyproject.toml, src/modbus_connection/__init__.py, src/modbus_connection/pymodbus/__init__.py, src/modbus_connection/tmodbus/__init__.py, src/modbus_connection/mock.py. This is not a security review — only the cheap checks were run.
  • PR Link: ❌ PR description must link to the source repository at https://github.com/home-assistant-libs/modbus-connection. A PyPI page link is not sufficient.
  • Async Safe: ✅ No blocking calls found. All I/O is async; TLS context loading for connect_tls is correctly offloaded via asyncio.to_thread.

Generated by Check requirements (AW) · 180.6 AIC · ⌖ 25.8 AIC · ⊞ 29.9K ·

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

This PR adds a new foundational modbus_connection integration (implementing architecture discussion #1418). Its sole responsibility is to own one config entry per physical Modbus link and expose a live, backend-neutral ModbusConnection that consumer integrations borrow units from via a single accessor. The integration is a thin wrapper over the new modbus-connection[tmodbus] library; connection/protocol logic lives in that library, keeping the integration itself minimal.

Changes:

  • New config flow with a menu choosing Network (TCP / RTU-over-TCP) or Serial (RTU), validated by actually opening the connection, and deduplicated by connection parameters.
  • async_setup_entry/async_unload_entry store the live connection in runtime_data, schedule an entry reload on connection loss, and close the connection on unload.
  • Public async_get_unit(hass, entry_id, unit_id) accessor plus a ConnectionNotReady (a ConfigEntryNotReady/ModbusError) for consumers; bronze quality scale, strict typing, and tests for setup/unload/config flow.

Reviewed changes

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

Show a summary per file
File Description
homeassistant/components/modbus_connection/init.py Setup/unload, _async_open, connection-lost reload, and the async_get_unit accessor.
homeassistant/components/modbus_connection/config_flow.py Menu + network/serial steps, param-based dedupe, and connect-to-validate.
homeassistant/components/modbus_connection/exceptions.py ConnectionNotReady combining ConfigEntryNotReady and library ModbusError.
homeassistant/components/modbus_connection/const.py Domain, transport, and serial default constants.
homeassistant/components/modbus_connection/manifest.json New integration manifest (hub, config_flow, requirement).
homeassistant/components/modbus_connection/strings.json Config-flow and exception strings.
homeassistant/components/modbus_connection/quality_scale.yaml Bronze quality-scale rule statuses.
.strict-typing Enables strict typing for the new integration.
requirements_all.txt Adds the modbus-connection[tmodbus]==3.2.0 dependency.
tests/components/modbus_connection/* Tests and fixtures for setup, config flow, and async_get_unit.

Comment thread homeassistant/components/modbus_connection/manifest.json
Comment thread homeassistant/components/modbus_connection/__init__.py Outdated
Comment thread homeassistant/components/modbus_connection/__init__.py
balloob and others added 3 commits July 3, 2026 07:46
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016huRs96kGdQbNQopXseC4H
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016huRs96kGdQbNQopXseC4H
async_setup_entry only caught ModbusConnectionError, while the config flow's
validation catches the broader ModbusError. A serial open failing with a
different ModbusError would propagate instead of raising ConfigEntryNotReady.
Widen the caught type to match and cover serial in the retry test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016huRs96kGdQbNQopXseC4H
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown

Check requirements

Checked at commit d46bc39.

⚠️ Some checks require attention — see the details below.

Package Old New No Advisories Not Yanked Repo Public CI Upload Release Pipeline Security PR Link Async Safe
modbus-connection 3.3.0 ☑️
📦 modbus-connection: 3.3.0
  • No Advisories: ✅ No active advisories reported by PyPI for version 3.3.0.
  • Not Yanked: ✅ Version 3.3.0 is a live (non-yanked) release.
  • Repo Public: ✅ https://github.com/home-assistant-libs/modbus-connection is publicly accessible.
  • CI Upload: ✅ Trusted Publisher attestation found (GitHub).
  • Release Pipeline: ✅ OIDC via Trusted Publisher attestation (GitHub); automated CI upload verified by PyPI.
  • Security: ☑️ Baseline scan found nothing obvious in pyproject.toml, init.py, _protocol.py, _tls.py, _types.py, _callbacks.py, exceptions.py, mock.py, model/, pymodbus/init.py, tmodbus/init.py. This is not a security review — only the cheap checks were run.
  • PR Link: ❌ PR description must link to the source repository at https://github.com/home-assistant-libs/modbus-connection. A PyPI page link is not sufficient.
  • Async Safe: ✅ No blocking calls found in the full source tree; TLS context setup in _tls.py is correctly offloaded to a thread via asyncio.to_thread() in both backends.

Generated by Check requirements (AW) · 290.9 AIC · ⌖ 39.1 AIC · ⊞ 29.9K ·

The integration has no conditions or triggers.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016huRs96kGdQbNQopXseC4H
Copilot AI review requested due to automatic review settings July 3, 2026 12:10
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016huRs96kGdQbNQopXseC4H

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 15 out of 17 changed files in this pull request and generated 2 comments.

Comment thread homeassistant/components/modbus_connection/__init__.py Outdated
Comment thread tests/components/modbus_connection/test_init.py Outdated
Copilot AI review requested due to automatic review settings July 3, 2026 12:18
@balloob
balloob marked this pull request as ready for review July 3, 2026 12:18
Guard against a connection_entry_id that resolves to a loaded config entry from
another integration: check the entry domain before touching runtime_data, so it
raises ConnectionNotReady instead of crashing on a foreign runtime_data.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016huRs96kGdQbNQopXseC4H

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 15 out of 17 changed files in this pull request and generated 2 comments.

Comment on lines +3 to +15
from collections.abc import Generator
from unittest.mock import AsyncMock, patch

from modbus_connection.mock import MockModbusConnection
import pytest

from homeassistant.components.modbus_connection.const import CONNECTION_TCP, DOMAIN
from homeassistant.const import CONF_HOST, CONF_PORT, CONF_TYPE
from homeassistant.core import HomeAssistant

from tests.common import MockConfigEntry


Comment thread homeassistant/components/modbus_connection/strings.json Outdated
Copilot AI review requested due to automatic review settings July 3, 2026 12:26
Reject an out-of-range TCP port (must be 1-65535) and a non-positive baud rate
instead of only coercing to int.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016huRs96kGdQbNQopXseC4H

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

Comment thread homeassistant/components/modbus_connection/config_flow.py Outdated
Copilot AI review requested due to automatic review settings July 3, 2026 19:57

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

Comment thread homeassistant/components/modbus_connection/config_flow.py Outdated
balloob and others added 2 commits July 3, 2026 16:14
Use a SelectSelector so parity renders as None/Even/Odd instead of the raw
N/E/O codes, drop the option list from the field description, and keep the
default at None (the most common).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016huRs96kGdQbNQopXseC4H
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 4, 2026 12:00
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016huRs96kGdQbNQopXseC4H

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

Comment thread homeassistant/components/modbus_connection/config_flow.py Outdated
Copilot AI review requested due to automatic review settings July 4, 2026 12:07

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

Comment thread homeassistant/components/modbus_connection/config_flow.py Outdated
balloob and others added 2 commits July 4, 2026 15:11
Replace the manual entry-iteration dedupe with the idiomatic
_async_abort_entries_match (a serial link is matched by device path, a TCP link
by host and port). This is the hassfest-recognized pattern, so unique-config-entry
is now done rather than exempt.

Selector translation option keys must be lowercase, so the uppercase N/E/O parity
codes failed hassfest. Use explicit option labels (None/Even/Odd) instead and keep
the stored N/E/O values.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016huRs96kGdQbNQopXseC4H
Selector translation keys must be lowercase, so store parity as lowercase n/e/o
with a translation_key (None/Even/Odd) and uppercase it in _async_open before
passing it to the connection.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016huRs96kGdQbNQopXseC4H
Copilot AI review requested due to automatic review settings July 4, 2026 20:37

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

Uppercase the selected parity in the config flow so the entry data holds the
N/E/O code the connection expects, keeping _async_open a plain pass-through.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016huRs96kGdQbNQopXseC4H
Comment on lines +91 to +94
entry = cast(
"ModbusConnectionConfigEntry | None",
hass.config_entries.async_get_entry(connection_entry_id),
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: cast before validation

@bluetoothbot

Copy link
Copy Markdown

PR Review — Add Modbus Connection integration

Clean, well-scoped connection-owner integration; prior Copilot findings are largely addressed, but confirm test fixtures resolve and the RTU-over-TCP claim.

Strengths worth calling out:

  • Every earlier review comment appears resolved in this revision: hassfest-generated files (config_flows.py, integrations.json, CODEOWNERS) are included; async_get_unit now rejects unknown/wrong-domain entry IDs before touching runtime_data; TCP port and baud rate now have real vol.Range bounds; setup and config-flow now both catch the broader ModbusError; and the hand-rolled dedupe was replaced with _async_abort_entries_match.
  • The accessor contract is tight and well-documented: @callback async_get_unit distinguishes programming errors (ValueError) from transient not-ready (ConnectionNotReady, a ConfigEntryNotReady so consumers inherit setup-retry).
  • Reload-on-connection-loss via async_on_unload(connection.on_connection_lost(...)) is a clean lifecycle, and the config flow validates by actually opening the link then closing it.

What needs attention before merge:

  • Test fixtures mock_modbus_connection / mock_modbus_unit are referenced but not defined in the shown conftest.py — confirm the library auto-registers them as a pytest plugin, else collection fails.
  • Description promises RTU-over-TCP but the TCP step exposes no framing selector — reconcile code or wording.
  • Confirm ConnectionNotReady's ConfigEntryNotReady+ModbusError co-inheritance constructs without a TypeError.

🟡 Important

1. Referenced `mock_modbus_connection` / `mock_modbus_unit` fixtures are not defined here
tests/components/modbus_connection/conftest.py:18-34

The mock_connect fixture takes mock_modbus_connection as an argument, and test_config_flow.py / test_init.py reference mock_modbus_connection (and per the prior Copilot comment, mock_modbus_unit). Neither fixture is defined in this conftest.py, and no pytest_plugins is registered.

Why it matters: if these fixtures are not resolvable, pytest fails at collection with fixture 'mock_modbus_connection' not found, and the whole test module errors out — the opposite of the "local tests pass" checkbox.

This is only safe if the modbus-connection library ships them as an auto-registered pytest plugin (a pytest11 entry point exposing mock_modbus_connection/mock_modbus_unit). I could not verify that — the library isn't installed in this environment.

Please confirm the library registers these fixtures via a pytest11 entry point; if not, define them in this conftest.py (constructing MockModbusConnection / MockModbusUnit from modbus_connection.mock).

def mock_connect(
    mock_modbus_connection: MockModbusConnection,
) -> Generator[AsyncMock]:

🟢 Suggestions

1. RTU-over-TCP promised in the description has no way to be selected
homeassistant/components/modbus_connection/config_flow.py:93-100

The PR description and the serial menu string say the network path covers "TCP / RTU-over-TCP", but the TCP step only collects host + port and always calls connect_tcp(host, port=...) with no framing/protocol selector. There's no user-visible way to choose RTU-over-TCP framing.

Why it matters: RTU-over-TCP and plain Modbus-TCP use different framing on the wire; a device that only speaks RTU-over-TCP would fail to communicate with a plain TCP connection. If connect_tcp auto-detects framing this is a non-issue — but that isn't evident from the diff.

Either add a framing option to STEP_MODBUS_TCP (passed through _async_open), or adjust the description/strings so they don't promise RTU-over-TCP support that the flow can't express. Unverified: I could not inspect the library to confirm how connect_tcp handles framing.

    return await connect_tcp(data[CONF_HOST], port=data[CONF_PORT])

Checklist

  • hassfest-generated files updated (config_flows, integrations.json, CODEOWNERS)
  • Input validation at config-flow boundaries (port/baud ranges)
  • async_get_unit guards unknown/wrong-domain entry IDs
  • Test fixtures resolve at collection — warning #1
  • PR description matches implemented transports — suggestion #1
  • Exception construction is safe
  • Config entry unloading closes the owned connection

To rebase specific severity levels, mention me: @bluetoothbot rebase critical (fixes 🔴 only), @bluetoothbot rebase important (fixes 🔴 + 🟡), or just @bluetoothbot rebase for all.


Automated review by Kōan (Claude) HEAD=2e3e7b7 3 min 31s

@bluetoothbot bluetoothbot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking issues found.

  • Referenced mock_modbus_connection / mock_modbus_unit fixtures are not defined here

@balloob
balloob merged commit 0e1c190 into dev Jul 5, 2026
49 checks passed
@balloob
balloob deleted the modbus_connection-integration branch July 5, 2026 10:48
bradleyseanf pushed a commit to bradleyseanf/core that referenced this pull request Jul 5, 2026
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 6, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants