Skip to content

fix(timer): keep pump running when extension wraps past midnight - #180

Merged
stritti merged 3 commits into
mainfrom
fix/temp-circ
Aug 17, 2026
Merged

stritti merged 3 commits into
mainfrom
fix/temp-circ

Conversation

@stritti

@stritti stritti commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Problem

The temperature-based pump runtime extension ("Zeitverlängerung") is silently lost whenever the extended end time wraps past midnight while the base timer stays within a single day.

Example: base timer 16:00–20:00, pool water above threshold → extension of 720 min → effective end 04:00 next day. Instead of running until 04:00, the pump turned OFF at 20:00 and the extension was reset to 0 on every loop during the base window.

Root cause

checkPoolPumpTimer() Step 2 keyed the midnight-crossing check on the base timer only (crossesMidnight). For a same-day base timer whose temperature extension pushed the end past midnight, the normalized end (_activeEndMinutes % 1440) is earlier in the day than the base window, so nowMinutes < normalizedEnd was false during the base window. The extension was recomputed and immediately reset to 0 every loop; at the base end the pump turned off.

Latent defect present since the feature was introduced (b16a9d0) and still in origin/main.

Fix

Key the wrap detection on the base timer or the extended end (_activeEndMinutes >= 1440) and bound the same-day case with the base start time:

  • wrap window: now >= baseStart || now <= normalizedEnd
  • same-day window: now >= baseStart && now < normalizedEnd

Tests

Added native regression tests with a controllable mock clock (setMockTime in stubs.cpp, new TestTime.hpp):

  • wrap-past-midnight extension keeps pump ON (the regression)
  • same-day extension still works
  • midnight-crossing base timer without extension
  • midnight-crossing base timer with extension

Verified RED/GREEN: the new tests fail (3 assertions) against origin/main and pass with the fix. Full suite: 111 suites, 202 assertions, 0 failures.

Related device-side findings (not addressed by this fix)

The reported symptom can also be caused by device state:

  • Firmware v4.2.1 (latest release) resets the temperature extension on every setMode call, including same-mode web settings saves and MQTT thermostat commands. Already fixed in main (dd91530); devices should run a build from main.
  • The extension is only computed when poolTemp > 0 and not NaN — a disconnected/filter-mismatched pool sensor silently disables it.
  • NVS keys set_circth / set_circfa / set_circmx are not zeroed on save (both v4.2.1 and main guard with hasArg).

@github-actions

Copy link
Copy Markdown
Contributor

Native Test Coverage

Metric Value
Line Coverage 44.3%
Branch Coverage 73.1%
Lines Hit/Total 333/751
Branches Hit/Total 163/223

Report from native unit tests (ASan + gcov).

@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

⚠️MegaLinter analysis: Success with warnings

Descriptor Linter Files Fixed Errors Max errors Warnings Elapsed time
✅ ACTION actionlint 8 0 0 0.7s
✅ BASH bash-exec 1 0 0 0.67s
✅ BASH shellcheck 1 0 0 0.61s
✅ BASH shfmt 1 0 0 0.01s
✅ C clang-format 1 0 0 0.04s
✅ C cppcheck 1 0 0 0.02s
✅ C cpplint 1 0 0 0.33s
✅ CPP clang-format 82 0 0 0.82s
✅ CPP cppcheck 82 0 0 6.14s
✅ CPP cpplint 82 0 0 7.29s
✅ EDITORCONFIG editorconfig-checker 261 0 0 0.55s
✅ JSON jsonlint 6 0 0 0.14s
✅ JSON v8r 6 0 0 3.89s
⚠️ MARKDOWN markdownlint 103 3 0 4.14s
✅ YAML yamllint 25 0 0 0.95s

Detailed Issues

⚠️ MARKDOWN / markdownlint - 3 errors
.opencode/skills/web-ui/SKILL.md:34 error MD028/no-blanks-blockquote Blank line inside blockquote
docs/superpowers/plans/2026-08-10-olimex-c6-local-ui-implementation.md:106:401 error MD013/line-length Line length [Expected: 400; Actual: 452]
docs/superpowers/plans/2026-08-16-norvi-button-calibration.md:7:401 error MD013/line-length Line length [Expected: 400; Actual: 412]

Notices

⚠️ Your configuration references items that have been removed from MegaLinter and are ignored: MAKEFILE, MAKEFILE_CHECKMAKE, MARKDOWN_MARKDOWN_LINK_CHECK. See Removed linters to find their replacements.

See detailed reports in MegaLinter artifacts

Your project could benefit from a custom flavor, which would allow you to run only the linters you need, and thus improve runtime performances. (Skip this info by defining FLAVOR_SUGGESTIONS: false)

  • Documentation: Custom Flavors
  • Command: npx mega-linter-runner@10.0.0 --custom-flavor-setup --custom-flavor-linters ACTION_ACTIONLINT,BASH_EXEC,BASH_SHELLCHECK,BASH_SHFMT,C_CPPCHECK,C_CPPLINT,C_CLANG_FORMAT,CPP_CPPCHECK,CPP_CPPLINT,CPP_CLANG_FORMAT,EDITORCONFIG_EDITORCONFIG_CHECKER,JSON_JSONLINT,JSON_V8R,MARKDOWN_MARKDOWNLINT,YAML_YAMLLINT

MegaLinter is provided by OX Security
Show us your support by starring ⭐ the repository

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 97c3becc88

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Rule.hpp Outdated
Comment thread test/native/tests/test_rules.cpp Outdated
@github-actions

Copy link
Copy Markdown
Contributor

Native Test Coverage

Metric Value
Line Coverage 44.5%
Branch Coverage 73.2%
Lines Hit/Total 335/752
Branches Hit/Total 156/213

Report from native unit tests (ASan + gcov).

The temperature-based runtime extension was lost whenever the extended
end time wrapped past midnight while the base timer stayed within a
single day (e.g. base 16:00-20:00 extended to 04:00 next day).

Step 2 keyed the midnight-crossing check on the base timer only. For a
same-day base timer whose extension wrapped past midnight, the
normalized end (% 1440) is earlier than the base window, so
"nowMinutes < normalizedEnd" was false during the base window: the
extension was reset on every loop and the pump turned off at the base
end.

Key the wrap detection on the base timer OR the extended end (>= 1440)
and bound the same-day case with the base start time. Regression tests
cover wrap-past-midnight, same-day, and midnight-crossing base timers.
Anchor the temperature extension to an absolute expiry time computed from
the current cycle's base start, instead of comparing minutes since midnight.

With the maximum runtime (1440 min) the extended end equals the next base
start, so the normalized wrap predicate "now >= start || now <= end" was
true at every minute of the day: the extension never expired and the pump
stayed on even after the water cooled. Minutes alone cannot distinguish a
wrap-past-midnight end from the next cycle's start.

Step 1 now stores the absolute expiry (cycle start + runtime) in
_activeEndTime, anchored to the previous day for the post-midnight part of
crossing timers; Step 2 simply checks now < _activeEndTime. Extends the
mock wall clock with a day offset so tests express real next-day times.
Each check overwrote the shared rc variable, so a failing assertion was
masked when a later check in the same suite passed. test_main.cpp exits
on the suite counter (g_testsFailed), not the assertion counter, so CI
could pass despite failed assertions.

Track suiteFailed (sticky OR of all per-check results) in the five
PumpTimer suites and report the suite as failed when any check failed.
Verified: a mid-suite failure now yields exit code 1.
@github-actions

Copy link
Copy Markdown
Contributor

Native Test Coverage

Metric Value
Line Coverage 44.5%
Branch Coverage 73.2%
Lines Hit/Total 335/752
Branches Hit/Total 156/213

Report from native unit tests (ASan + gcov).

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 69680d7f9b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Rule.hpp
@stritti
stritti merged commit 2a8f919 into main Aug 17, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant