Skip to content

Get modbus unit(s) from HA - #1146

Open
WillCodeForCats wants to merge 15 commits into
mainfrom
ha-modbus-connection
Open

WillCodeForCats wants to merge 15 commits into
mainfrom
ha-modbus-connection

Conversation

@WillCodeForCats

@WillCodeForCats WillCodeForCats commented Oct 14, 2026 •

Copy link
Copy Markdown
Owner

This is a first attempt at using the new Modbus connections added in HA 2026.10 where the core modbus integration hands out units over connections it shares, and changes solaredge-modbus-multi to ask it for all of our Device IDs instead of building our own connection.

Unlike the core solaredge-modbus integration which treats each inverter as a separate device, we are gathering all inverters into a SharedUnits class and polling them together as a single system (our preferred method of working with leader/follower systems). Although this doesn't appear to be the design intent for async_get_unit(), our experience here is that leader/follower inverters should be treated as a single system, not like individual devices on a bus.

This requires HA 2026.10 and drops requirements for modbus-connection and tmodbus since we're now getting modbus from core.

Summary by CodeRabbit

  • Compatibility
    • Home Assistant 2026.10.0 or newer is now required.
  • Reliability
    • The integration now uses Home Assistant’s shared Modbus connection, allowing it to share connections with other integrations.
    • Setup reports a clear error when another integration uses the same host and port with incompatible connection settings.
    • Message spacing is restored after write operations to help maintain reliable communication.

@WillCodeForCats WillCodeForCats self-assigned this Oct 14, 2026
@WillCodeForCats WillCodeForCats added the enhancement New feature or request label Oct 14, 2026
@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: cbc1d49f-277f-49cd-872d-c51375a6e804
📥 Commits

Reviewing files that changed from the base of the PR and between 4e9b0a6 and fb6908a.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 4ecf839f-fd5a-4aa4-81a5-2c2b857942fc
📥 Commits

Reviewing files that changed from the base of the PR and between 0686a9c and 4e9b0a6.

📒 Files selected for processing: 16
  • .github/workflows/tests.yml
  • README.md
  • custom_components/solaredge_modbus_multi/__init__.py
  • custom_components/solaredge_modbus_multi/const.py
  • custom_components/solaredge_modbus_multi/diagnostics.py
  • custom_components/solaredge_modbus_multi/helpers.py
  • custom_components/solaredge_modbus_multi/hub.py
  • custom_components/solaredge_modbus_multi/manifest.json
  • custom_components/solaredge_modbus_multi/strings.json
  • custom_components/solaredge_modbus_multi/translations/en.json
  • hacs.json
  • requirements-test.txt
  • tests/test_diagnostics_device_keys.py
  • tests/test_helpers.py
  • tests/test_init_version_check.py
  • tests/test_shared_connection.py
💤 Files with no reviewable changes: 6
  • tests/test_init_version_check.py
  • custom_components/solaredge_modbus_multi/diagnostics.py
  • tests/test_helpers.py
  • custom_components/solaredge_modbus_multi/const.py
  • tests/test_diagnostics_device_keys.py
  • custom_components/solaredge_modbus_multi/helpers.py

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

📝 Walkthrough

Setup now obtains device units through Home Assistant’s shared Modbus connection and passes them to the hub. The integration removes package-version checks and tracking. The minimum Home Assistant version and test environment requirements are updated.

Changes: Shared Modbus connection

Layer / File(s) Summary
Remove package-version enforcement
custom_components/solaredge_modbus_multi/__init__.py, const.py, helpers.py, diagnostics.py, manifest.json, tests/test_diagnostics_device_keys.py, tests/test_helpers.py, tests/test_init_version_check.py
The integration removes package-version checks, stored version data, diagnostics fields, and the version parsing helper. The manifest declares the Modbus integration dependency, and tests for the removed version behavior are deleted.
Acquire and manage shared units
custom_components/solaredge_modbus_multi/__init__.py, hub.py, strings.json, translations/en.json, tests/test_shared_connection.py
Setup obtains a unit for each configured device ID, applies message spacing and timeout settings, and passes SharedUnits to the hub. Link-setting conflicts become translated config-entry errors. The hub manages unit lookup and disconnect behavior and restores message spacing after write-settle cycles.
Update platform and test requirements
README.md, hacs.json, .github/workflows/tests.yml, requirements-test.txt
The minimum Home Assistant version changes to 2026.10.0. The test workflow switches to Python 3.14, and test requirements are updated.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant ConfigEntry
  participant async_setup_entry
  participant HomeAssistantModbus
  participant SharedUnits
  participant SolarEdgeHub
  ConfigEntry->>async_setup_entry: Set up entry
  async_setup_entry->>HomeAssistantModbus: Request a unit for each device ID
  HomeAssistantModbus-->>async_setup_entry: Return units or raise a link-settings error
  async_setup_entry->>SharedUnits: Wrap units by device ID
  async_setup_entry->>SolarEdgeHub: Initialize hub with SharedUnits
Loading
Merge Risk: ⚪ Minimal · up to 4e9b0

The reviewed change has no established issue requiring a fix before merge; normal checks remain appropriate.

Security architecture risk: 🟡 Moderate · up to 4e9b0

Recovery from one device’s failure can now reset a connection used by other integrations. The effect is limited to consumers sharing that connection, and routine post-poll closure is disabled by default. Concurrent-operation safety and cleanup after interrupted setup remain unverified.

Retained concerns

  • Medium · security · inferred: Where other integrations share the transport, a configured device returning invalid model data or causing exhausted protocol/timeout failures can trigger connection-wide recovery resets. Previously these calls targeted an entry-owned connection; they now potentially disrupt other consumers’ telemetry or device-control traffic. Shared-link closure is documented, but interruption of concurrent requests and its duration remain unverified.
Security review details

Security Blast Radius

  • inferred — The evidenced propagation boundary is the shared transport: recovery initiated by this entry can close the link used by every unit on it, including other integrations where sharing occurs. The repository does not identify actual co-consumers or establish propagation to unrelated transports.

Security Findings and Attack Paths

  • inferred — An attacker controlling a configured device’s replies can return unsupported SunSpec model IDs or lengths. After initialization, that produces DeviceInvalid, then shared-link disconnect and DataUpdateFailed. This supports an availability concern across co-consumers, not a verified exploit, privilege gain, or demonstrated cancellation of another consumer’s active write.

Trust Boundaries and Controls

  • observed — Transport creation and lifecycle ownership are delegated to Home Assistant, but the integration retains transport-reset authority through acquired units. The wrapper exposes only its configured ID mapping; the external implementation must establish shared-transport provenance, ownership release, and safe coordination with other consumers.

Resilience and Maintainability Implications

  • observed — Read and write helpers retry connection, protocol, and timeout errors, and close_after_polling is disabled by default. The wrapper documents reopening on the next request. These mitigate transient resets but do not prove safe handling of another consumer’s active request or automatic release after partial setup failure.

Hardening Proposals

  • proposed — Make transport-reset authority explicitly consistent with Home Assistant’s shared-connection contract. Validate concurrent consumers, active reads and writes during reset, partial acquisition failure, interrupted first refresh, and unload/reload cleanup against the real implementation before relying on entry-local recovery policies.
Pre-merge checks: Passed 5
Passed Passed checks: 5 passed
Check name Status Explanation
Title check Passed The title clearly summarizes the main change: obtaining Modbus units from Home Assistant instead of creating a separate connection.
Description check Passed The description clearly explains the Home Assistant 2026.10 dependency, shared Modbus connections, the SharedUnits design, and removed dependencies. It is relevant and sufficiently detailed.
Docstring Coverage Passed Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 3 files. (7 skipped: 7 …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@WillCodeForCats WillCodeForCats added this to the Release v4.1.0 milestone Oct 14, 2026
WillCodeForCats added a commit that referenced this pull request Oct 14, 2026
This will go into a 4.1.0 release with #1146 which needs min HA to 2026.10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant