Conversation
- advance the counter by the real measured millis() delta and move the time reference before callbacks run, so neither check overshoot nor callback duration accumulates as drift (accuracy is now independent of count length and loop() load) - stop paused time from leaking into the count: start() resets the time reference only on a stopped-to-running transition - restart() no longer inherits the previous tick's remainder - define isCounterRunning() (was declared and documented, but failed at link time when called) - guard uninitialized interval/callback pointers in run() - add setCalibration(float): optional per-board correction of hardware millis() drift, with fractional milliseconds carried between ticks - sync keywords.txt with the public API
- README brought to current library standards: badges, feature list, quick start, per-mode usage snippets, full API reference table, examples/compatibility/contributing sections - add CONTRIBUTING.md, CHANGELOG.md (Keep a Changelog format), MIT LICENSE and CLAUDE.md - add examples/Basic (minimal count-down) and examples/Advanced (count-up, pause/resume over serial, state queries, re-programming the count time, calibration); clean up CountimerTest
- GitHub Actions: arduino-lint (strict, library-manager mode) plus compilation of all examples for arduino:avr:uno and esp32:esp32:esp32 - Claude Code project skills: verify-examples, check-api-sync, release - library.properties: bump version to 1.1.0, real description in paragraph and the includes field
PR #30 (master) added a non-const isCounterRunning() independently of the const version added on dev in 1d6daaa. Merging master into dev (b64b41c) kept both as non-conflicting additions, leaving a stray definition that doesn't match the const-qualified declaration in Countimer.h and breaks compilation. Keep the const version, which also correctly excludes completed counters.
The skill only documented installing arduino-cli on Windows. Add the official install script for Linux, noting that BINDIR must be set explicitly or the binary lands in ./bin instead of on PATH. Verified end-to-end in WSL (Debian): install, AVR core install, and compiling all three examples all succeed with the same output sizes as on Windows.
Generic ESP32 boards (esp32:esp32:esp32 FQBN) don't define LED_BUILTIN in the core, unlike AVR, causing the compile-sketches CI job to fail.
setCounter() gains overloads accepting a milliseconds (0-999) argument, and getCurrentMilliseconds()/getCurrentTimeWithMillis() read the sub-second remainder back. Existing overloads and getCurrentTime() are unchanged, so no existing call site needs to change. Closes #22 (sub-second count time), closes #15 (displaying milliseconds).
Resolves #17. reset() restores the counter to its initial time and leaves the timer stopped (not completed), ready to be started again with start() — filling the gap between restart() (reset + start) and stop() (reset + mark completed). Behaves uniformly across COUNT_UP / COUNT_DOWN / COUNT_NONE. restart() is now expressed as reset() + start(), removing the duplicated "return to start time" logic. Sync the public API across all four surfaces (header, cpp, README, keywords.txt) and add an 'E' key to the CountimerTest example.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR prepares the 1.1.0 release of Countimer by improving long-run timing accuracy (elapsed-delta based counting + corrected reference point), adding millisecond-level counter configuration/formatting and optional calibration, and bringing the project up to Arduino Library Manager/CI/documentation standards.
Changes:
- Refactors core timing logic to advance by measured
millis()deltas (with optional calibration) and fixesisCounterRunning()implementation. - Adds millisecond-precision APIs (
setCounter(..., milliseconds),getCurrentMilliseconds(),getCurrentTimeWithMillis()) and a newreset()control method. - Adds/updates release collateral: rewritten README, changelog, license, keywords/library metadata, examples, scripts, and GitHub Actions CI.
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Countimer.h | Adds new public APIs (ms precision, calibration, reset) and in-class member initialization. |
| src/Countimer.cpp | Implements drift fix via measured elapsed time + calibration; adds new API implementations and state semantics. |
| scripts/build-examples.sh | Adds a bash helper to compile examples with arduino-cli. |
| scripts/build-examples.ps1 | Adds a PowerShell helper to compile examples with arduino-cli. |
| README.md | Rewrites documentation to modern Arduino library standards and documents new APIs/semantics. |
| LICENSE | Adds MIT license file. |
| library.properties | Bumps version to 1.1.0 and improves metadata (paragraph/includes). |
| keywords.txt | Updates Arduino IDE keyword highlighting for new API and enums. |
| examples/CountimerTest/wokwi.toml | Adds Wokwi simulator config for CountimerTest example. |
| examples/CountimerTest/diagram.json | Adds Wokwi wiring diagram for CountimerTest example. |
| examples/CountimerTest/CountimerTest.ino | Updates example to use <Countimer.h>, LED portability, reset key, and sub-second demo. |
| examples/Basic/wokwi.toml | Adds Wokwi simulator config for Basic example. |
| examples/Basic/diagram.json | Adds Wokwi wiring diagram for Basic example. |
| examples/Basic/Basic.ino | Adds minimal countdown “Basic” example sketch. |
| examples/Advanced/wokwi.toml | Adds Wokwi simulator config for Advanced example. |
| examples/Advanced/diagram.json | Adds Wokwi wiring diagram for Advanced example. |
| examples/Advanced/Advanced.ino | Adds an “Advanced” interactive serial-controlled example sketch. |
| CONTRIBUTING.md | Adds contribution guidelines, API sync checklist, and build/verification instructions. |
| CLAUDE.md | Adds repository guidance documenting timing architecture and contribution notes. |
| CHANGELOG.md | Adds Keep-a-Changelog formatted changelog including 1.1.0 entry. |
| .gitignore | Ignores PlatformIO artifacts and example build outputs. |
| .github/workflows/ci.yml | Adds CI to run arduino-lint and compile examples for AVR Uno and ESP32. |
| .gitattributes | Excludes CI/scripts/docs/skill files from Arduino Library Manager release archives. |
| .claude/skills/verify-examples/SKILL.md | Adds documented Claude Code skill for compiling examples locally. |
| .claude/skills/release/SKILL.md | Adds documented Claude Code skill for release/tagging procedure. |
| .claude/skills/check-api-sync/SKILL.md | Adds documented Claude Code skill for API consistency auditing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| bool _isStopped = true; | ||
| char _formatted_time[10]; | ||
| CountType _countType; | ||
| char _formatted_time_ms[13]; |
Comment on lines
51
to
55
| void Countimer::setInterval(timer_callback callback, uint32_t interval) | ||
| { | ||
| _interval = interval; | ||
| _callback = callback; | ||
| } |
Comment on lines
+218
to
+222
| if (_currentCountTime + elapsed < _countTime) | ||
| { | ||
| // Add the real measured time, not the nominal interval, | ||
| // so the error does not accumulate over long counts. | ||
| _currentCountTime += elapsed; |
|
|
||
| void Countimer::run() | ||
| { | ||
| // timer is running only if is not completed or not stopped. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f62fdd34aa
ℹ️ 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 on lines
+113
to
+119
| if(_isStopped) | ||
| { | ||
| // Reset the reference point only on a real resume, so paused time | ||
| // is not counted. start() may be called on every loop() iteration. | ||
| _isStopped = false; | ||
| _previousMillis = millis(); | ||
| _calibrationRemainder = 0.0; |
There was a problem hiding this comment.
With the new guidance that callers may invoke start() on every loop() iteration, a timer that has just completed is also stopped because completion calls stop(). The next unconditional start() therefore enters this branch, then clears _isCounterCompleted, so the timer starts over and isCounterCompleted() is only observable during the completion callback; either make start() a no-op for completed timers or keep the documented loop pattern guarded by the completion state.
Useful? React with 👍 / 👎.
| void Countimer::countUp(uint32_t elapsed) | ||
| { | ||
| if (_currentCountTime < _countTime) | ||
| if (_currentCountTime + elapsed < _countTime) |
There was a problem hiding this comment.
For long count-up timers near the supported 999-hour maximum, if one run() call is delayed by a large measured delta (for example after sleep or a blocked loop), _currentCountTime + elapsed can overflow uint32_t before it is compared with _countTime. When that happens the condition remains true, the counter wraps back to a small value, and the completion callback is skipped instead of firing; compare elapsed to the remaining time (_countTime - _currentCountTime) before adding.
Useful? React with 👍 / 👎.
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
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.
Fixed
real measured
millis()delta instead of the nominal interval, and the time referencepoint is moved before user callbacks run. Previously, both the check overshoot
(the loop rarely hits the interval boundary exactly) and the callback execution time
(e.g.
Serial.printat 9600 baud) were silently lost on every tick, so the error grewlinearly with the count duration — easily tens of seconds per hour on a busy
loop().Accuracy is now independent of count length and
loop()load.start()now resets the time reference pointwhen resuming, so time spent paused is no longer counted. Calling
start()on everyloop()iteration (the documented usage pattern) remains safe — the reset only happenson an actual stopped-to-running transition.
restart()while running. The counter now restarts from a fresh time referencepoint instead of inheriting the previous tick's remainder.
isCounterRunning()missing implementation. The method was declared in the headerand documented in the README, but never defined — calling it failed at link time.
run()was called withoutsetInterval()configured.The interval and completion callback pointers were uninitialized; they now default
to
NULLand are guarded before invocation.Added
setCounter()now has overloads accepting amilliseconds(0-999) argument, so counts can be configured with sub-second precision (e.g. a 1.5 s
countdown).
(How to use 0.1 second #22)
getCurrentMilliseconds()andgetCurrentTimeWithMillis()(formattedHH:MM:SS.mmm) read it back. Existing overloads andgetCurrentTime()are unchanged.(How to show millisecond #15)
reset()— restore the counter to its initial time and leave the timer stopped(not completed), ready to be started again with
start(). Fills the gap betweenrestart()(reset + start) andstop()(reset + mark completed); behaves uniformlyacross
COUNT_UP/COUNT_DOWN/COUNT_NONE.restart()is now built on it.(Is it Possible to reset the timer without restarting it? #17)
setCalibration(float factor)— optional one-time, per-board correction of hardwaremillis()drift (e.g. ceramic resonator tolerance, up to ~0.5% on many Uno/Nanoclones). Measure
factor = real_elapsed_time / timer_indicated_timeagainst areference clock; default
1.0means no correction. Fractional milliseconds arecarried between ticks, so the correction itself introduces no rounding drift.
LICENSEfile (MIT).arduino-lint(strict, library-manager mode) plus examplecompilation for
arduino:avr:unoandesp32:esp32:esp32.keywords.txt:CountType(KEYWORD1) andCOUNT_NONE/COUNT_UP/COUNT_DOWN(LITERAL1) for Arduino IDE highlighting.
library.properties: real library description inparagraphand theincludesfield.examples/Basic— minimal count-down sketch for getting started.examples/Advanced— count-up with pause/resume over serial, state queries,re-programming the count time and calibration.
CONTRIBUTING.md— bug report / pull request guidelines, API sync checklist,build-and-test instructions and release steps.
.claude/skills/:verify-examples(bootstraparduino-cliand compile all examples locally),check-api-sync(audit public APIconsistency across the header, cpp, README and
keywords.txt) andrelease(release preconditions and tagging procedure).
Changed
Manager), feature list, quick start, per-mode usage snippets, a full API reference
table matching the header signatures, examples/compatibility/contributing sections.
The quick-start sketch now calls
start()once insetup()instead of conditionallyon every
loop()iteration.keywords.txtupdated withsetIntervalandsetCalibrationfor Arduino IDEhighlighting.
const,nullptrinstead ofNULL,snprintfinstead ofsprintfingetCurrentTime(), simplifiedgetCurrentSeconds()arithmetic, in-class member initialization (empty constructorand destructor removed), single include guard instead of guard +
#pragma once,removed redundant
Arduino.hinclude from the.cpp.<Countimer.h>, usesLED_BUILTINandconfigures it with
pinMode()insetup().time no longer exceeds the measured delta (up to one interval early), instead of one
full tick after reaching zero (up to one interval late). Either way the boundary error
is below one interval and does not accumulate.