plan 0010: local timezone offset + LED day/night brightness - #147
Open
TheAngryRaven wants to merge 1 commit into
Open
plan 0010: local timezone offset + LED day/night brightness#147TheAngryRaven wants to merge 1 commit into
TheAngryRaven wants to merge 1 commit into
Conversation
Give the device a local wall clock so the NeoPixel strip can dim after
dark on the driver's schedule: "7am" has to mean their 7am, not
Greenwich's. A US Central driver at 07:30 local is at 12:30 UTC, which a
naive UTC gate calls the middle of the night.
New pure unit local_time.{h,cpp}: a fixed signed minute offset applied to
a 4-digit-year DateTime with correct rollover both ways across month,
year and leap-day boundaries, plus isNight(), which tests the window
[nightStart, dayStart) modulo the day so the ordinary wrapped case
(19:00 -> 07:00) needs no special casing at the call site. Equal bounds
mean an empty window, which is how the swap is disabled without a
separate flag. Minutes rather than hours because India is +5:30 and
Newfoundland -3:30. An out-of-band offset is ignored, not clamped: a
corrupt setting must not be able to walk the calendar. 30 host tests.
The consumer is npxEffectiveBrightness() feeding npxPushFrame(), which
was already the single global-brightness choke point, so the invariant
that no channel exceeds the cap is untouched. Two rules there: no time
lock means DAY (timeValid can be ~12.5 min out from a cold start, and a
strip that comes up dark reads as dead hardware), and a night cap of 0
blanks the frame without cutting the 5 V rail -- only led_brightness 0
does that.
No DST, deliberately. A fixed offset walks the boundary an hour twice a
year, beneath the resolution of a dim-after-dark gate; rule tables are a
standing correctness liability and tzdata is ~100 KB on a sealed device.
Logged data is unchanged and still UTC. DOVEX row timestamps are Unix
epoch ms, and the header datetime, log filenames and generated course
names all stay UTC. A log is routinely viewed somewhere other than where
it was recorded, so timezone presentation belongs to the viewing app.
Nothing in the logging pipeline may call into local_time.
Also fixes a latent cliff the four new keys would have gone over: the
settings file and JSON document were both 512 bytes, every read path
caps at sizeof(settingsFileBuffer) - 1, and the 18-key file was already
436 B. At 543 B the file parses as IncompleteInput, EVERY key read
fails, SETTINGS_SETUP reads that as corruption and regenerates the file
(losing the BLE name, PIN and pairing), ensureDefaultSettings grows it
back over the cap, and it loops every boot. The document was a second
wall -- a <512> doc returns NoMemory at 22 string pairs, measured
against the pinned ArduinoJson 6.21.5. Both raised to 1024, and
setSettingInner() now refuses any write whose document overflowed() or
whose measureJson() exceeds the buffer, turning a silent permanent brick
into one loud refused write with the old file left intact.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F5A5R2ec4hBDiYZX6YjPqa
Coverage — host-testable units📂 Overall coverage
📄 File coverage
|
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
2 participants
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
Gives the device a local wall clock so the NeoPixel strip can dim after dark on the driver's schedule — "7am" has to mean their 7am, not Greenwich's.
Nothing in the firmware derived local time before this;
gpsData.houris raw UTC out of the PVT callback. The failure mode without an offset isn't subtle: a US Central driver at 07:30 local is at 12:30 UTC, which a naive UTC gate calls the middle of the night and dims the strip in broad daylight.New pure unit
local_time.{h,cpp}— a fixed signed minute offset applied to a 4-digit-yearDateTime, with correct date rollover both directions across month, year and leap-day boundaries. PlusisNight(), which tests the window[nightStart, dayStart)modulo the day, so the ordinary wrapped case (19:00 → 07:00) needs no special casing at the call site. Equal bounds = empty window = swap disabled, no separate enable flag to keep in sync.Three details worth flagging:
GpsData.yearstays 2-digit for the existing filename/header format strings; the unit takes 4 digits and callers add 2000. The leap rule (gps_time::isLeapYear, reused rather than re-derived) needs the century.The consumer is
npxEffectiveBrightness()feedingnpxPushFrame(). That was already the single global-brightness choke point (led_frame::applyCap), so the whole feature is a change of argument at one call site and the "no channel ever exceeds the cap" invariant is untouched.Two traps handled explicitly:
gpsData.timeValidneedsfullyResolved, up to ~12.5 min from a cold start. Rendering night brightness in that window would mean a strip that comes up dark and reads as broken hardware.led_brightness0 does that. Power-cycling the boost converter at 19:00 mid-session is not a brightness change.Deliberately out of scope
No DST. A fixed offset walks the boundary an hour twice a year, which is beneath the resolution of a dim-after-dark gate. Rule tables are a standing correctness liability (legislatures keep moving the dates) and tzdata is ~100 KB shipped to a sealed device.
local_timeis where rules would go if that changes.Local time never touches saved data. DOVEX row timestamps are Unix epoch ms, and the header
datetime, log filenames and generated course names all stay UTC. A log is routinely viewed somewhere other than where it was recorded, so the conversion belongs on the presentation side where the reader's preference is known. Nothing in the logging pipeline may call intolocal_time— the header comment says so.Also fixes: the settings file was one key away from a boot loop
Adding four keys turned up a latent cliff.
settings.inohad a 512-byte file buffer and aStaticJsonDocument<512>, every read path caps atsizeof(settingsFileBuffer) - 1(511), and the existing 18-key file already serialized to 436 bytes. The four keys here put it at 543.Past the cap the failure is silent and self-inflicting:
read()truncates at 511 bytes, mid-token.deserializeJsonreturnsIncompleteInput, so everygetSetting()fails — not just the new keys.SETTINGS_SETUP()reads that as corruption, quarantines toSETTINGS.json.bad, regenerates fresh defaults — new random BLE name, new PIN, new device name; the user's pairing is gone.ensureDefaultSettings()adds the keys back one read-modify-write at a time until the file crosses 511 again.The document was a second independent wall: a
<512>doc returnsNoMemoryat 22 string pairs (measured against the pinned ArduinoJson 6.21.5, not estimated).Both raised to 1024 — they must stay equal, the invariant being that the buffer can always hold what the document serializes — and
setSettingInner()now refuses any write whose documentoverflowed()or whosemeasureJson()exceeds the buffer. That turns a silent permanent brick into one loudly refused write with the previous file intact. Costs ~1 KB RAM.Adding a settings key is no longer free — check the serialized size.
Type of change
Not breaking: the log format, filenames and BLE protocol are byte-for-byte unchanged, and the four new settings keys auto-populate with defaults that reproduce the previous behaviour (
utc_offset_min0 = UTC).How it was verified
-Wall -Wextra -Wpedantic -Werrorclean. 30 of those are newlocal_timecases: real-world zones (+5:30, −3:30, +12:45), rollover in both directions across month/year/leap-day boundaries, an exhaustive all-1440-minutes sweep proving the night window is half-open so no minute is both day and night, out-of-band offsets ignored, and no-2000s-assumption cases (1999→2000, 2100 non-leap).clang-tidyclean — not run locally (no toolchain in this environment); CI covers it.compile-sketchis the first real check.BETA, so no new warnings.NoMemorywall was found.SETTINGS.jsonstill reads its settings after the buffer bump.Checklist
CHANGELOG.mdupdated under[Unreleased](Added: the feature; Changed: the settings buffer fix)ARCHITECTURE.md/CLAUDE.mdupdated — new subsystem 17, thelocal_timepure unit in both file maps, settings table + key constants, and two new ARCHITECTURE design-decision sections (Local time is presentation-only, The settings file has a hard size ceiling)tests/local_time_test.cppdocs/plans/0010-local-timezone-led-day-night.mdrecords the rationale, including the cliffNotes for review
settingUtcOffsetMinis extern'd inneopixel.heven though it's conceptually device-wide, because the LED strip is its only consumer today. Flagged in a comment to move it if a second one appears.SETTINGS.jsonshape, following the SensorEgg DOVEX-column precedent) but only read by aBIRDSEYE_ENABLE_NEOPIXELbuild.Generated by Claude Code