DPL: Refactor state machine to determine battery and dc bus usage

[SW-Niko]

• fixes Lastkorrektur berechnet Werte verspätet und "Use battery at night even if only partially charged" lässt sich unter bestimmten Umständen nicht deaktivieren #2334
• refactoring of getBatteryPower() from 2 states -> 4 states
• rename getBatterPower() -> getBatteryState()
• Use the 4 states in the loop() to control the battery provided inverter

State machine for the battery
Conditions: we use 'Below Stop Threshold', 'Above Start Threshold', 'Solar-Passthrough', 'Use Battery at night',
Night/Day' and 'From which direction did we enter the stop-start zone' to determine the state.

• STOP: we must stop the inverter, because the battery is below the stop threshold
• NO_DISCHARGE: we can use the inverter, but we do not allow to discharge the battery,
• DISCHARGE_ALLOWED: we can use the inverter and we allow discharging of the battery
• DISCHARGE_NIGHT: we can use the inverter and we allow discharging of a partial charged battery at night.

[github-actions]

Build Artifacts

Firmware built from this pull request's code:

• opendtu-onbattery-generic_esp32_4mb_no_ota.zip
• opendtu-onbattery-generic_esp32_8mb.zip
• opendtu-onbattery-generic_esp32s3.zip
• opendtu-onbattery-generic_esp32s3_usb.zip

Notice

• These artifacts are ZIP files containing the factory update binary as well as the OTA update binary.
  Extract the binaries from the ZIP files first. Do not use the ZIP files themselves to perform an update.
• These links point to artifacts of the latest successful build run.
• The linked artifacts were built from 71d9323.

[coderabbitai]

Warning

Rate limit exceeded

@AndreasBoehm has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 5 minutes and 18 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 18 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 88541131-b0fd-4fb8-be91-d0471c29818d

📥 Commits

Reviewing files that changed from the base of the PR and between 8d1ccf2 and c6049ef.

📒 Files selected for processing (2)

• include/PowerLimiter.h
• src/PowerLimiter.cpp

Walkthrough

Replaces two private booleans with a private enum class BatteryState and two lifecycle flags; implements a battery-state machine (STOP, NO_DISCHARGE, DISCHARGE_ALLOWED, DISCHARGE_NIGHT) that drives discharge decisions and updates related logic paths in the power-limiter loop and APIs.

Changes

Cohort / File(s)	Summary
Header State Definition include/PowerLimiter.h	Replaced _batteryDischargeEnabled and _nighttimeDischarging with _batteryState (enum class BatteryState) and added _fromStart, _oneStopPerNightDone flags for state-machine lifecycle tracking.
State Machine Logic & Control Flow src/PowerLimiter.cpp	Introduced getBatteryState() and computes _batteryState in the DPL loop; removed _batteryDischargeEnabled assignments; updated loop(), getPowerLimiterState(), calcPowerBusUsage(), getBatteryDischargeLimit() and logging/UI mappings to use _batteryState (including early return when STOP).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• fix: full solar passthrough configuration fault #2299: Modifies PowerLimiter discharge decision logic and passthrough/stop interactions—closely related to this state-machine rework.

Poem

🐰 I hopped into the code tonight,
Swapped booleans for a clearer light.
STOP to NIGHT, the states now sing,
Hops and checks keep battery spring. ⚡🌙

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description check	✅ Passed	PR description clearly describes refactoring getBatteryPower() into a 4-state machine with specific state definitions and conditions.
Linked Issues check	✅ Passed	PR addresses both objectives from #2334: refactored state machine logic and proper handling of 'Use battery at night' option disabling.
Out of Scope Changes check	✅ Passed	All changes are scoped to PowerLimiter header and implementation files with focus on battery state management and related logic.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0dc7b6c and 5218d11.

📒 Files selected for processing (12)

• include/PowerLimiter.h (2 hunks)
• include/RuntimeData.h (1 hunks)
• src/PowerLimiter.cpp (8 hunks)
• src/RuntimeData.cpp (1 hunks)
• src/WebApi_firmware.cpp (2 hunks)
• src/WebApi_maintenance.cpp (2 hunks)
• src/WebApi_sysstatus.cpp (2 hunks)
• src/main.cpp (2 hunks)
• webapp/src/components/FirmwareInfo.vue (1 hunks)
• webapp/src/locales/de.json (1 hunks)
• webapp/src/locales/en.json (1 hunks)
• webapp/src/types/SystemStatus.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

webapp/src/**/*.{js,jsx,ts,tsx,vue}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

webapp/src/**/*.{js,jsx,ts,tsx,vue}: Webapp source must pass ESLint (yarn lint)
Webapp source must be Prettier-formatted (yarn prettier --check src/)

Files:

• webapp/src/components/FirmwareInfo.vue
• webapp/src/types/SystemStatus.ts

{src,include,lib/Hoymiles,lib/MqttSubscribeParser,lib/TimeoutHelper,lib/ResetReason}/**/*.{c,cc,cpp,cxx,h,hpp,hxx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

C/C++ code must pass cpplint with the specified filters

Files:

• src/WebApi_maintenance.cpp
• src/WebApi_sysstatus.cpp
• src/PowerLimiter.cpp
• src/RuntimeData.cpp
• src/main.cpp
• include/RuntimeData.h
• src/WebApi_firmware.cpp
• include/PowerLimiter.h

🧠 Learnings (2)

📓 Common learnings

Learnt from: SW-Niko
Repo: hoylabs/OpenDTU-OnBattery PR: 2262
File: src/main.cpp:149-159
Timestamp: 2025-10-17T08:29:40.205Z
Learning: In the OpenDTU-OnBattery project (src/main.cpp), RuntimeData must be initialized after SolarCharger, PowerMeter, PowerLimiter, GridCharger, and Battery components have been initialized, but before the main loop. RuntimeData acts as a persistence service that these components access during their runtime operations (not during initialization).

📚 Learning: 2025-10-17T08:29:40.205Z

Learnt from: SW-Niko
Repo: hoylabs/OpenDTU-OnBattery PR: 2262
File: src/main.cpp:149-159
Timestamp: 2025-10-17T08:29:40.205Z
Learning: In the OpenDTU-OnBattery project (src/main.cpp), RuntimeData must be initialized after SolarCharger, PowerMeter, PowerLimiter, GridCharger, and Battery components have been initialized, but before the main loop. RuntimeData acts as a persistence service that these components access during their runtime operations (not during initialization).

Applied to files:

• src/PowerLimiter.cpp
• src/RuntimeData.cpp
• src/main.cpp
• include/RuntimeData.h
• src/WebApi_firmware.cpp
• include/PowerLimiter.h

🧬 Code graph analysis (2)

include/RuntimeData.h (1)

src/RuntimeData.cpp (16)

• init (42-49)
• init (42-42)
• read (137-172)
• read (137-137)
• write (70-131)
• write (70-70)
• getWriteCount (178-182)
• getWriteCount (178-178)
• getWriteEpochTime (188-192)
• getWriteEpochTime (188-188)
• getWriteCountAndTimeString (200-216)
• getWriteCountAndTimeString (200-200)
• loop (55-63)
• loop (55-55)
• getWriteTrigger (222-239)
• getWriteTrigger (222-222)

include/PowerLimiter.h (1)

src/PowerLimiter.cpp (4)

• serializeRTD (979-984)
• serializeRTD (979-979)
• deserializeRTD (986-991)
• deserializeRTD (986-986)

🪛 Clang (14.0.6)

src/RuntimeData.cpp

[error] 20-20: 'Utils.h' file not found

(clang-diagnostic-error)

include/RuntimeData.h

[error] 4-4: 'ArduinoJson.h' file not found

(clang-diagnostic-error)

🔇 Additional comments (4)

include/RuntimeData.h (1)

34-43: Solid separation of fast-path atomics vs. guarded shared state.

Nice touch keeping the immediate flags (_writeNow, _readOK, _writeOK) atomic while the more complex state continues to sit behind the mutex.

webapp/src/locales/de.json (1)

270-288: Translation addition looks consistent.

The new RuntimeSaveCount string matches the neighbouring phrasing and keeps the count/time wording clear.

webapp/src/components/FirmwareInfo.vue (1)

86-89: UI wiring lines up perfectly.

Thanks for threading the new runtime_savecount straight through the table — nothing else to flag here.

src/WebApi_firmware.cpp (1)

52-54: Good call persisting after OTA.

Hooking the runtime write right after the OTA response will keep the new state machine data on disk without waiting for the nightly loop.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/PowerLimiter.cpp (1)

356-358: Consider extracting the nested ternary for readability.

The nested ternary chain works correctly but is dense. A helper or switch could improve clarity.

🔎 Optional: Extract to a helper function

// Add as a static helper or member function:
static const char* batteryStateToString(BatteryState state) {
    switch (state) {
        case BatteryState::DISCHARGE_ALLOWED:
        case BatteryState::DISCHARGE_NIGHT:
            return "allowed";
        case BatteryState::NO_DISCHARGE:
            return "restricted";
        case BatteryState::STOP:
        default:
            return "stopped";
    }
}

Then use: DTU_LOGD("battery discharge %s, ...", batteryStateToString(_batteryState), ...);

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5218d11 and b82d344.

📒 Files selected for processing (1)

• src/PowerLimiter.cpp

🧰 Additional context used

📓 Path-based instructions (1)

{src,include,lib/Hoymiles,lib/MqttSubscribeParser,lib/TimeoutHelper,lib/ResetReason}/**/*.{c,cc,cpp,cxx,h,hpp,hxx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

C/C++ code must pass cpplint with the specified filters

Files:

• src/PowerLimiter.cpp

🧠 Learnings (2)

📓 Common learnings

Learnt from: SW-Niko
Repo: hoylabs/OpenDTU-OnBattery PR: 2262
File: src/main.cpp:149-159
Timestamp: 2025-10-17T08:29:40.205Z
Learning: In the OpenDTU-OnBattery project (src/main.cpp), RuntimeData must be initialized after SolarCharger, PowerMeter, PowerLimiter, GridCharger, and Battery components have been initialized, but before the main loop. RuntimeData acts as a persistence service that these components access during their runtime operations (not during initialization).

📚 Learning: 2025-10-17T08:29:40.205Z

Learnt from: SW-Niko
Repo: hoylabs/OpenDTU-OnBattery PR: 2262
File: src/main.cpp:149-159
Timestamp: 2025-10-17T08:29:40.205Z
Learning: In the OpenDTU-OnBattery project (src/main.cpp), RuntimeData must be initialized after SolarCharger, PowerMeter, PowerLimiter, GridCharger, and Battery components have been initialized, but before the main loop. RuntimeData acts as a persistence service that these components access during their runtime operations (not during initialization).

Applied to files:

• src/PowerLimiter.cpp

🔇 Additional comments (4)

src/PowerLimiter.cpp (4)

240-300: State machine logic looks correct and well-documented.

The state machine implementation handles the various conditions (stop/start thresholds, solar passthrough, night usage) with appropriate state transitions. The _oneStopPerNightDone flag correctly prevents oscillation between STOP and DISCHARGE_NIGHT states during the same night.

One observation: Line 296 returns STOP during the day when BatteryAlwaysUseAtNight is enabled but solarPassThroughEnabled is false. This means daytime behavior differs based on solar passthrough config even when the "use at night" feature is what's enabled. Verify this is the intended behavior.

---

549-550: LGTM!

The UI state determination correctly reflects the new BatteryState enum values.

---

737-740: LGTM!

Early exit for STOP state correctly prevents battery usage when below the stop threshold.

---

822-822: LGTM!

Correctly returns zero discharge limit for both STOP and NO_DISCHARGE states.

[coderabbitai]

🧹 Nitpick comments (1)

src/PowerLimiter.cpp (1)

738-741: Minor: Log message may not always be accurate.

The STOP state can also occur when usesBatteryPoweredInverter() returns false (line 261), not just when the battery is below the stop threshold. Consider a more general message:

     if (_batteryState == BatteryState::STOP) {
-        DTU_LOGD("DC power bus usage blocked by battery below the stop threshold");
+        DTU_LOGD("DC power bus usage blocked (battery state: STOP)");
         return 0;
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/PowerLimiter.cpp` around lines 738 - 741, The log message at the check
for _batteryState == BatteryState::STOP is too specific; update the DTU_LOGD
call inside the PowerLimiter::... (where _batteryState is compared to
BatteryState::STOP) to use a more general message that covers both battery below
the stop threshold and the case where usesBatteryPoweredInverter() is false
(e.g., "DC power bus usage blocked: battery STOP state or inverter not used");
keep the conditional and return as-is but change only the log text to accurately
reflect both causes.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/PowerLimiter.cpp`:
- Around line 738-741: The log message at the check for _batteryState ==
BatteryState::STOP is too specific; update the DTU_LOGD call inside the
PowerLimiter::... (where _batteryState is compared to BatteryState::STOP) to use
a more general message that covers both battery below the stop threshold and the
case where usesBatteryPoweredInverter() is false (e.g., "DC power bus usage
blocked: battery STOP state or inverter not used"); keep the conditional and
return as-is but change only the log text to accurately reflect both causes.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 39c2dc52-0e9d-496b-8de6-09d918edd5a2

📥 Commits

Reviewing files that changed from the base of the PR and between b82d344 and 65d4a78.

📒 Files selected for processing (2)

• include/PowerLimiter.h
• src/PowerLimiter.cpp

[SW-Niko]

Hallo @AndreasBoehm ,
ich habe den PR aktualisiert. Dabei habe ich die Abhängigkeit #2262 entfernt.
Das macht die Integration einfacher.

[AndreasBoehm]

Thanks a lot for this massive improvement, i only left one comment that should help us in the future to easier understand how the state machine works.

[AndreasBoehm]

Sorry, ich hab was übersehen