Fix clock colon blink regularity and AM/PM redraw heap churn #335

2026-07-15T00:47:08Z

Bullpuph123 commented

2026-07-15 00:47:08 +00:00

(Migrated from github.com)

Split from #330 (part 2 of 3), rebased onto dev as requested.

Summary

Smooth, drift-free colon blink (NORMAL clock): the colon was driven by seconds parity from GlobalTime, so any skipped/duplicated second made the blink irregular. It's now driven by a millis()-based 500ms blinker with parity-correct catch-up (if draw is delayed by multiple periods, visibility only toggles on an odd number of elapsed periods). NIXIE/custom clocks keep the image-based 1s colon.
AM/PM redrawn only on change: displayAmPm() was called every second, and each call reloads TTF fonts (DSEG7↔DSEG14) plus per-loop String allocations — over days of uptime this fragments the heap and eventually corrupts glyph rendering (stray "PM" artifacts observed after long uptime). It's now redrawn only when the value changes or on force, with the old value cleared first.
Bounds/ownership guards: displayDigit() validates the display index; displaySeconds() only draws on the status orb and validates the seconds range — defense against a corrupted index painting tick marks on the wrong panel.
CLOCK_UPDATE_DELAY/CLOCK_DRAW_DELAY defaults changed from OneSecond to OneHundredMilliseconds so the 500ms colon blinker actually gets a chance to run; all per-second work is still internally gated on second changes, so this doesn't add redraw load.

Test plan

Builds with pio run -e infoorbs-v0_3
Equivalent change soak-tested on hardware for several days on the main-based branch from #330 (colon blinks evenly, no AM/PM artifacts)
Verify NIXIE/custom clock colon still alternates each second

🤖 Generated with Claude Code

Split from #330 (part 2 of 3), rebased onto dev as requested. ## Summary - **Smooth, drift-free colon blink (NORMAL clock)**: the colon was driven by seconds parity from GlobalTime, so any skipped/duplicated second made the blink irregular. It's now driven by a millis()-based 500ms blinker with parity-correct catch-up (if draw is delayed by multiple periods, visibility only toggles on an odd number of elapsed periods). NIXIE/custom clocks keep the image-based 1s colon. - **AM/PM redrawn only on change**: `displayAmPm()` was called every second, and each call reloads TTF fonts (DSEG7↔DSEG14) plus per-loop String allocations — over days of uptime this fragments the heap and eventually corrupts glyph rendering (stray "PM" artifacts observed after long uptime). It's now redrawn only when the value changes or on force, with the old value cleared first. - **Bounds/ownership guards**: `displayDigit()` validates the display index; `displaySeconds()` only draws on the status orb and validates the seconds range — defense against a corrupted index painting tick marks on the wrong panel. - `CLOCK_UPDATE_DELAY`/`CLOCK_DRAW_DELAY` defaults changed from `OneSecond` to `OneHundredMilliseconds` so the 500ms colon blinker actually gets a chance to run; all per-second work is still internally gated on second changes, so this doesn't add redraw load. ## Test plan - [x] Builds with `pio run -e infoorbs-v0_3` - [x] Equivalent change soak-tested on hardware for several days on the main-based branch from #330 (colon blinks evenly, no AM/PM artifacts) - [x] Verify NIXIE/custom clock colon still alternates each second 🤖 Generated with [Claude Code](https://claude.com/claude-code)

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2026-07-15 23:37:38 +00:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull request overview

This PR improves long-uptime stability and visual consistency of the firmware ClockWidget, focusing on a drift-free colon blink for the NORMAL clock type and reducing heap churn by avoiding per-second AM/PM redraws.

Changes:

Implement a millis()-driven 500ms colon blinker for NORMAL clocks (with catch-up parity handling).
Redraw AM/PM only when it changes (or on forced redraw), avoiding repeated font reload cycles.
Add defensive guards so colon/seconds tick rendering is constrained to the middle/status orb and avoids invalid indices/ranges.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
firmware/src/widgets/clockwidget/ClockWidget.h	Adds status-orb ownership constant, colon blink state, and changes default draw/update cadence to 100ms.
firmware/src/widgets/clockwidget/ClockWidget.cpp	Implements 500ms colon blink for NORMAL, gates AM/PM redraw to changes, and adds bounds/ownership guards for digit/seconds rendering.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## Pull request overview This PR improves long-uptime stability and visual consistency of the firmware `ClockWidget`, focusing on a drift-free colon blink for the NORMAL clock type and reducing heap churn by avoiding per-second AM/PM redraws. **Changes:** - Implement a millis()-driven 500ms colon blinker for NORMAL clocks (with catch-up parity handling). - Redraw AM/PM only when it changes (or on forced redraw), avoiding repeated font reload cycles. - Add defensive guards so colon/seconds tick rendering is constrained to the middle/status orb and avoids invalid indices/ranges. ### Reviewed changes Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment. | File | Description | | ---- | ----------- | | firmware/src/widgets/clockwidget/ClockWidget.h | Adds status-orb ownership constant, colon blink state, and changes default draw/update cadence to 100ms. | | firmware/src/widgets/clockwidget/ClockWidget.cpp | Implements 500ms colon blink for NORMAL, gates AM/PM redraw to changes, and adds bounds/ownership guards for digit/seconds rendering. | --- 💡 <a href="/brettdottech/info-orbs/new/dev?filename=.github/instructions/*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.

firmware/src/widgets/clockwidget/ClockWidget.cpp

					
				@ -97,0 +131,4 @@

				        }

				    }

				    if (m_secondSingle != m_lastSecondSingle || force) {

In the NORMAL-clock colon blink logic, a forced redraw (force == true) can still run the catch-up path (elapsed >= m_colonBlinkInterval) and toggle m_colonVisible based on time elapsed since setup(). This can make the colon immediately start in the “off” state right after a screen clear / widget switch, which is unintuitive for a forced redraw.

In the NORMAL-clock colon blink logic, a forced redraw (`force == true`) can still run the catch-up path (`elapsed >= m_colonBlinkInterval`) and toggle `m_colonVisible` based on time elapsed since `setup()`. This can make the colon immediately start in the “off” state right after a screen clear / widget switch, which is unintuitive for a forced redraw.

ericthelin (Migrated from github.com) reviewed

2026-07-15 23:40:49 +00:00

ericthelin (Migrated from github.com) left a comment

I would like to see the constant renamed then I will approve this

firmware/src/widgets/clockwidget/ClockWidget.h

					
				@ -89,8 +89,13 @@ public:

				    String getName() override;

I don't love the variable name SCREEN_STATUS because "status" is not really what is being displayed

Bullpuph123 (Migrated from github.com) reviewed

2026-07-16 02:29:28 +00:00

firmware/src/widgets/clockwidget/ClockWidget.h

					
				@ -89,8 +89,13 @@ public:

				    String getName() override;

Good point — renamed to SCREEN_MIDDLE in 2b44a84, matching the existing BUTTON_MIDDLE naming convention.

Good point — renamed to SCREEN_MIDDLE in 2b44a84, matching the existing BUTTON_MIDDLE naming convention.

Bullpuph123 (Migrated from github.com) reviewed

2026-07-16 02:29:32 +00:00

firmware/src/widgets/clockwidget/ClockWidget.cpp

					
				@ -97,0 +131,4 @@

				        }

				    }

				    if (m_secondSingle != m_lastSecondSingle || force) {

Fixed in 2b44a84: forced redraws now restart the blink phase with the colon visible (m_colonVisible = true, m_colonBlinkPrev = now) instead of running the catch-up path, so the colon can no longer reappear in the "off" state after a screen clear or widget switch.

Fixed in 2b44a84: forced redraws now restart the blink phase with the colon visible (m_colonVisible = true, m_colonBlinkPrev = now) instead of running the catch-up path, so the colon can no longer reappear in the "off" state after a screen clear or widget switch.

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin Bullpuph123/fix/clock-colon-ampm:Bullpuph123/fix/clock-colon-ampm

git switch Bullpuph123/fix/clock-colon-ampm

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch dev

git merge --no-ff Bullpuph123/fix/clock-colon-ampm

git switch Bullpuph123/fix/clock-colon-ampm

git rebase dev

git switch dev

git merge --ff-only Bullpuph123/fix/clock-colon-ampm

git switch Bullpuph123/fix/clock-colon-ampm

git rebase dev

git switch dev

git merge --no-ff Bullpuph123/fix/clock-colon-ampm

git switch dev

git merge --squash Bullpuph123/fix/clock-colon-ampm