Skip to content

Let a Fronius power limit revert when Home Assistant stops - #181388

Open
farmio wants to merge 8 commits into
home-assistant:devfrom
farmio:fronius-modbus-revert-options
Open

Let a Fronius power limit revert when Home Assistant stops#181388
farmio wants to merge 8 commits into
home-assistant:devfrom
farmio:fronius-modbus-revert-options

Conversation

@farmio

@farmio farmio commented Sep 5, 2026

Copy link
Copy Markdown
Member

Breaking change

Proposed change

Adds a setting to the Fronius config flow: Revert the AC power limit if Home Assistant stops. It is offered when the integration is set up and in the reconfigure flow, next to the Modbus port; existing entries get it added by a migration.

A setpoint written over Modbus stays on the inverter until it is changed - including while Home Assistant is shut down or unreachable. The SunSpec Immediate Controls model (123) has a register for that case, WMaxLimPct_RvrtTms: the inverter drops the limit when it hasn't been sent it for that long and hands control back to its own priority list. With the setting on, the integration writes an hour into it and re-sends an active limit every 15 minutes, so the hour only ever runs out once Home Assistant has really stopped - a restart or a short outage has plenty of room to spare. The heartbeat schedules itself one beat at a time: it starts when a limit is put in force, restarts whenever the limit is written, and stops when the limit is switched off, so nothing is sent while there is nothing to keep alive.

Details worth knowing for the review:

  • The heartbeat is necessary. Fronius documents the timer as restarted by every received Modbus message, which would make the regular polling enough. Measured against a Gen24 (firmware 1.40.9-1), it is not: a limit read every five seconds still reverted after exactly its 60 second period. Only writing the limit again restarts it. The docstring records this so it doesn't get "simplified" away later.
  • AC power limit only. The storage model's equivalent register, InOutWRte_RvrtTms (124), is documented by Fronius as R / Not supported, so the battery setpoints cannot auto-revert. The option's description says so.
  • The heartbeat never takes control back. While no limit is in force there is no period counting down either, so it returns without touching the device - a limit the user released with the switch stays released.
  • A switch, not a period picker. The register accepts up to 8 hours, but a single well-chosen value is easier to explain and covers what the option is for: getting the inverter back to its own settings when nothing is sending the limit any more. That is an outage - and just as much a deliberate removal of the integration, which otherwise leaves the last limit in force with nothing left to take it back.

Also included: a device that exposes the controls model but no MPPT model got a second settings coordinator on every hourly re-scan, because the re-scan guard only looked at the MPPT coordinators. With the heartbeat that would have meant duplicate writes to the inverter, so the two coordinators are now tracked separately and each retried on its own.

That last part makes #181402 more likely to be hit: a settings coordinator can now come up on a re-scan while the readings coordinator is already there, and the platforms only learn about coordinators found after setup once that PR is in. Merging #181402 first and rebasing this one avoids the window - the two touch the same lines in _init_modbus_inverter.

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:

The inverter can drop an output power limit when it stops hearing from
whoever set it. Offering that as an option lets a limit survive a
restart but not an outage: Home Assistant sends the limit again every
15 minutes, so the inverter's hour only ever runs out once Home
Assistant is gone for good.

Fronius documents the timer as restarted by every received Modbus
message, which would make polling enough. Measured against a Gen24, it
is not - a limit read every five seconds still reverted on time, so the
limit has to be written again.

The storage model's timeout register is documented as not supported, so
this covers the AC power limit only.

Also stop a second settings coordinator from being created on every
re-scan for a device that exposes controls but no MPPT model.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

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.

🟡 Changes recommended

The heartbeat can alter default-off devices or reactivate released limits, and the rescan guard can prevent recovery from partial initialization failures.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds an optional Fronius AC power-limit timeout and heartbeat so limits revert after Home Assistant stops.

Changes:

  • Adds the auto-revert options flow and translations.
  • Configures and refreshes inverter fallback timers.
  • Prevents duplicate settings coordinators and expands tests.
File summaries
File Description
homeassistant/components/fronius/__init__.py Starts heartbeats and adjusts rescan guards.
homeassistant/components/fronius/config_flow.py Adds the options flow.
homeassistant/components/fronius/const.py Defines timeout and heartbeat constants.
homeassistant/components/fronius/coordinator.py Implements fallback configuration and heartbeat writes.
homeassistant/components/fronius/strings.json Adds option text.
tests/components/fronius/__init__.py Supports options in test setup.
tests/components/fronius/test_config_flow.py Tests option persistence.
tests/components/fronius/test_modbus.py Tests fallback, heartbeat, errors, and rescans.
Review details
  • Files reviewed: 8/8 changed files
  • Comments generated: 4
  • Review effort level: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread homeassistant/components/fronius/__init__.py Outdated
Comment thread homeassistant/components/fronius/coordinator.py
Comment thread homeassistant/components/fronius/coordinator.py Outdated
Comment thread homeassistant/components/fronius/config_flow.py Outdated
Deleting the config entry is the case where the fallback matters most:
it leaves the last limit in force with nothing left to take it back.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 5, 2026 16:38
@farmio
farmio marked this pull request as draft September 5, 2026 16:40

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.

🔵 Needs a closer look

The heartbeat can override device state, while the rescan guard can permanently suppress coordinator recovery.

Review details

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

homeassistant/components/fronius/coordinator.py:299

  • Preserve the device timeout until the user explicitly chooses this option. Treating a missing option as False makes upgraded entries push revert_seconds=0 at startup and on control writes, silently removing a timeout configured by another controller; distinguish an unset option from an explicitly disabled one.

homeassistant/components/fronius/init.py:344

  • Track the readings and settings coordinators independently. Returning when either coordinator exists means a transient MPPT refresh failure followed by successful settings setup permanently suppresses later MPPT retries, so those sensor entities never recover on hourly rescans.
        if inverter_info.solar_net_id in [
            coordinator.inverter_info.solar_net_id
            for coordinator in (
                *self.modbus_inverter_coordinators,
                *self.modbus_settings_coordinators,
            )

homeassistant/components/fronius/config_flow.py:216

  • Update the integration's quality-scale declaration for this new options flow. homeassistant/components/fronius/quality_scale.yaml:46-50 still marks docs-configuration-parameters exempt because the integration has no options; with this flow and the linked documentation PR, the rule should be marked done.
class FroniusOptionsFlow(OptionsFlowWithReload):

homeassistant/components/fronius/coordinator.py:333

  • Re-check that the device still has the limit enabled before reasserting it. This condition uses cached state, while fronius-modbus 0.2.0 refreshes inside set_power_limit() but then unconditionally writes enabled = 1; a limit released directly on the inverter since the previous poll is therefore taken back by the heartbeat.
        if (
            controls is None
            or not controls.enabled
            or (limit := controls.power_limit) is None
        ):
  • Files reviewed: 8/8 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

- Retry the two Modbus coordinators independently again. Folding them
  into one re-scan guard meant a device whose MPPT refresh failed once
  but whose controls came up never got its MPPT entities back.
- Leave a device alone that was never configured for a fallback here.
  Sending the limit with a zero period on every setup would drop a
  timeout another controller had set. Only the option being turned off
  takes a period back, and only while the device still holds one.
- Decide the heartbeat on a fresh read and write the registers
  directly. `set_power_limit` always enables the limit, so a limit
  released on the device between two polls was taken back by the next
  heartbeat - the opposite of what the switch promises.
- Mark docs-configuration-parameters done: the integration has options
  now, and the documentation PR describes them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 5, 2026 16:45

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.

🟡 Changes recommended

Heartbeat writes can race with user writes, and recovered settings coordinators do not create control entities.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 9/9 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread homeassistant/components/fronius/__init__.py
Comment thread homeassistant/components/fronius/coordinator.py Outdated
It belongs where the Modbus port already is: visible when the
integration is added, and changeable through the reconfigure flow. The
options flow is gone, and existing entries get the setting added by a
migration.

The heartbeat now schedules itself one beat at a time instead of
running on a fixed interval. It starts when a limit is put in force,
restarts whenever the limit is written - a write restarts the device's
period too - and stops when the limit is switched off, so nothing is
sent while there is nothing to keep alive.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 5, 2026 16: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.

🟡 Changes recommended

Heartbeat cleanup can survive unload, while unresolved write-serialization and recovered-entity discovery issues remain.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 10/10 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread homeassistant/components/fronius/coordinator.py
- Keep one heartbeat handle. The first refresh can schedule a beat
  before the heartbeat is started, whose handle was then dropped rather
  than cancelled - the orphan kept refreshing the limit after the entry
  was unloaded. Cancelling is now authoritative, and a beat that is
  under way while the entry unloads cannot schedule another one.
- Serialize the heartbeat against writes from entities. Its read and
  writes are not atomic, so a service call landing between them was
  undone by the rest of the beat.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 5, 2026 20:48

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.

🟡 Changes recommended

Timeout ownership, retry, and unload handling remain unsafe, while recovered settings entities still depend on unmerged PR #181402.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

homeassistant/components/fronius/config_flow.py:34

  • Update the Fronius quality-scale declaration for this configuration option. This Platinum integration still marks docs-configuration-parameters exempt at quality_scale.yaml:46-50 because it supposedly has no options, but the rule has no exceptions and the linked documentation PR covers this new field, so the rule should now be marked done.
    vol.Required(CONF_AUTO_REVERT, default=False): bool,

homeassistant/components/fronius/coordinator.py:336

  • Track and cancel or await the in-flight heartbeat task during unload. Once async_call_later fires, this handle is cleared and its remover only targets the already-fired timer (homeassistant/helpers/event.py:1525-1570); because _heartbeat_stopped is checked only after this await, unloading while the refresh is blocked can still write all three registers after the entry unloads.
        self._heartbeat_unsub = None
        await self._async_resend_power_limit()
        self._async_update_heartbeat()

homeassistant/components/fronius/coordinator.py:330

  • Only clear the timeout when this entry is known to have set it. Matching AUTO_REVERT_SECONDS does not establish ownership, so a migrated/default-off entry will erase an active one-hour timeout configured by another controller during startup, despite the option being disabled.
        if controls is not None and controls.revert_seconds == AUTO_REVERT_SECONDS:
            await self._async_resend_power_limit()
  • Files reviewed: 10/10 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread homeassistant/components/fronius/coordinator.py
farmio and others added 2 commits September 6, 2026 07:56
`auto_revert` said nothing about what falls back. Only the AC power
limit can - Fronius documents no timeout for the battery setpoints - so
the key says so.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It comes from the config entry, and changing it reloads the entry, so
there is nothing for a property to pick up.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Clearing it was a single attempt at startup whose failure was only
logged, so one refused write left the inverter armed to drop a limit
the user had just asked to keep. It is done from the refresh now, which
repeats it while the device still holds the period - and only ever
writes when it does.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 6, 2026 06: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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@farmio
farmio marked this pull request as ready for review September 6, 2026 07:22
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