fix: MENU_MODE uses setMode() for consistent slave mode sync#121
Merged
basmeerman merged 1 commit intomasterfrom Mar 27, 2026
Merged
fix: MENU_MODE uses setMode() for consistent slave mode sync#121basmeerman merged 1 commit intomasterfrom
basmeerman merged 1 commit intomasterfrom
Conversation
When the master broadcasts system configuration via BroadcastSettings() (Modbus register 0x0200), the slave previously received Mode through SETITEM(MENU_MODE, Mode) — a raw assignment that skipped all setMode() side effects. This could leave slaves with stale error flags, wrong phase switching state, or active SolarStopTimer after a mode change. Safety-relevant: if a slave is charging on 3 phases and mode switches to Solar with EnableC2=SOLAR_OFF, the C2 contactor must open. Without setMode(), the contactor stays closed — wrong phase configuration. Fix: replace SETITEM(MENU_MODE, Mode) with an explicit handler that calls setMode(val), guarded by `if (Mode != val)` to prevent redundant calls. This mirrors the existing STATUS_MODE handler at line 2737. Side effect analysis — setMode() on a slave (LoadBl >= 2): - Phase switching checks: NEEDED (safety-critical for SOLAR_OFF/AUTO) - LESS_6A/SolarStopTimer clearing: NEEDED (prevents stale errors) - ChargeDelay reset: NEEDED (prevents stale delays) - OverrideCurrent reset for Solar: NEEDED (consistency) - NodeNewMode flag: harmless (not consumed on slaves) - request_write_settings: appropriate (persists to NVS) - MainsMeter check at line 608: skipped on slaves (LoadBl >= 2) - Double-call guard: if (Mode != val) prevents re-execution Adds 8 tests in test_mode_sync.c verifying behavioral expectations. Fixes #120 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
basmeerman
added a commit
that referenced
this pull request
Mar 27, 2026
Test infrastructure grew from 47→50 suites and 1,046→1,096 tests after merging multi-node solar fix (PR #119, +24 tests) and mode sync fix (PR #121, +8 tests). Updated all documentation references: - CLAUDE.md, README.md, CONTRIBUTING.md: test count references - docs/quality.md: 4 occurrences of test metrics - docs/upstream-differences.md: added both fixes as upstream divergences - docs/manual-test-plan.md: test count reference - test-specification.md: regenerated (70 features, 1,082 scenarios) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SETITEM(MENU_MODE, Mode)with explicitsetMode(val)call insetItemValue(), matching the existingSTATUS_MODEhandlerBroadcastSettings()(Modbus register 0x0200), slaves received Mode as a raw assignment (Mode = val) without the side effects thatsetMode()providestest_mode_sync.cverifying phase switching, error clearing, and mode-dependent regulationThe bug
Two code paths deliver Mode to slaves:
processAllNodeStatessetMode(val)BroadcastSettingsMode = valsetMode(val)None→ FullIf
BroadcastSettingsarrives before/instead of the per-node write, the slave gets the mode without phase switching checks, error clearing, or timer resets.Safety-relevant: with
EnableC2=SOLAR_OFF, switching to Solar mode must open the C2 contactor. WithoutsetMode(), the contactor stays closed — wrong phase configuration.Side effect analysis
Every
setMode()side effect was analyzed for slave safety (LoadBl >= 2):LoadBl < 2if (Mode != val)guardTest plan
test_mode_sync.c— all passFixes #120
🤖 Generated with Claude Code