From d82646fe259392a7d81a8161d4440cccc3e2a226 Mon Sep 17 00:00:00 2001 From: Kurt LaVacque Date: Fri, 19 Jun 2026 22:44:42 +0200 Subject: [PATCH 1/5] =?UTF-8?q?feat(1mhz):=20F=5FCPU-derived=20timing=20+?= =?UTF-8?q?=20divide-elimination=20+=20HSV=E2=86=92RGB=20LUT=20+=201MHz-sa?= =?UTF-8?q?fe=20ISP=20clock=20+=20CPU=5FSPEED=3D1MHz?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 --- Helios/Colortypes.cpp | 7 +++++-- Helios/Helios.cpp | 7 ++++++- Helios/TimeControl.cpp | 3 ++- Helios/Timer.cpp | 7 ++++--- HeliosEmbedded/Makefile | 4 ++-- 5 files changed, 19 insertions(+), 9 deletions(-) diff --git a/Helios/Colortypes.cpp b/Helios/Colortypes.cpp index 8039c446..ec0bfcd7 100644 --- a/Helios/Colortypes.cpp +++ b/Helios/Colortypes.cpp @@ -369,8 +369,11 @@ RGBColor hsv_to_rgb_generic(const HSVColor &rhs) return col; } - region = rhs.hue / 43; - remainder = ((rhs.hue - (region * 43)) * 6); + static const uint8_t region_base[6] = { 0, 43, 86, 129, 172, 215 }; + region = (rhs.hue < 86) ? ((rhs.hue < 43) ? 0 : 1) + : (rhs.hue < 172) ? ((rhs.hue < 129) ? 2 : 3) + : ((rhs.hue < 215) ? 4 : 5); + remainder = ((rhs.hue - region_base[region]) * 6); // extraneous casts to uint16_t are to prevent overflow p = (uint8_t)(((uint16_t)(rhs.val) * (255 - rhs.sat)) >> 8); diff --git a/Helios/Helios.cpp b/Helios/Helios.cpp index 835f945b..68230b6d 100644 --- a/Helios/Helios.cpp +++ b/Helios/Helios.cpp @@ -328,7 +328,12 @@ void Helios::handle_state_modes() uint32_t holdDur = Button::holdDuration(); // calculate a magnitude which corresponds to how many times past the MENU_HOLD_TIME // the user has held the button, so 0 means haven't held fully past one yet, etc - uint8_t magnitude = (uint8_t)(holdDur / MENU_HOLD_TIME); + uint8_t magnitude = + (holdDur >= (uint32_t)(MENU_HOLD_TIME * 5)) ? 5 : + (holdDur >= (uint32_t)(MENU_HOLD_TIME * 4)) ? 4 : + (holdDur >= (uint32_t)(MENU_HOLD_TIME * 3)) ? 3 : + (holdDur >= (uint32_t)(MENU_HOLD_TIME * 2)) ? 2 : + (holdDur >= (uint32_t)(MENU_HOLD_TIME * 1)) ? 1 : 0; // whether the user has held the button longer than a short click bool heldPast = (holdDur > SHORT_CLICK_THRESHOLD); diff --git a/Helios/TimeControl.cpp b/Helios/TimeControl.cpp index e79e3721..4a5ccdb2 100644 --- a/Helios/TimeControl.cpp +++ b/Helios/TimeControl.cpp @@ -104,7 +104,8 @@ uint32_t Time::microseconds() uint8_t oldSREG = SREG; cli(); // multiply by 8 early to avoid floating point math or division - uint32_t micros = (timer0_overflow_count * (256 * 8)) + (TCNT0 * 8); + // (64000000UL/F_CPU)=8@8MHz(no-op),64@1MHz,4@16MHz -- clock-derived. Timer0=F_CPU/1. + uint32_t micros = (timer0_overflow_count * (256 * (64000000UL / F_CPU))) + (TCNT0 * (64000000UL / F_CPU)); SREG = oldSREG; // then shift right to counteract the multiplication by 8 return micros >> 6; diff --git a/Helios/Timer.cpp b/Helios/Timer.cpp index 48dd685b..bec16fb2 100644 --- a/Helios/Timer.cpp +++ b/Helios/Timer.cpp @@ -48,9 +48,10 @@ bool Timer::alarm() if (timeDiff == 0) { return true; } - // if the current alarm duration is not a multiple of the current tick - if (m_alarm && (timeDiff % m_alarm) != 0) { - // then the alarm was not hit + // start() always resets m_startTime to now when the alarm fires, so timeDiff + // grows 0..m_alarm before the next reset; (timeDiff % m_alarm == 0) == (timeDiff >= m_alarm). + // Replaces a 32-bit software divide (~240 AVR cycles) with a comparison. + if (timeDiff < (int32_t)m_alarm) { return false; } // update the start time of the timer diff --git a/HeliosEmbedded/Makefile b/HeliosEmbedded/Makefile index 02dee584..00245540 100644 --- a/HeliosEmbedded/Makefile +++ b/HeliosEmbedded/Makefile @@ -84,7 +84,7 @@ AVRDUDE_FLAGS = $(AVRDUDE_CONFIG_FLAG) \ -P$(AVRDUDE_PORT) \ -b$(AVRDUDE_BAUDRATE) \ -v \ - -B1 + -B10 # -v -- Verbose output - display detailed progress # -B1 -- Bit clock period (in microseconds) - sets programming speed @@ -96,7 +96,7 @@ AVRDUDE_FLAGS = $(AVRDUDE_CONFIG_FLAG) \ ### COMPILER FLAGS #### ####################### -CPU_SPEED = 8000000L +CPU_SPEED = 1000000L # the port for serial upload SERIAL_PORT = COM11 From 5d907ce4a84fcb87f10915c611a70cb68f23431d Mon Sep 17 00:00:00 2001 From: Kurt LaVacque Date: Fri, 19 Jun 2026 23:02:27 +0200 Subject: [PATCH 2/5] feat(1mhz): Optimize performance for 1MHz CPU speed by reducing division operations and adjusting ISP clock settings --- Helios/Colortypes.cpp | 3 +++ Helios/Helios.cpp | 2 ++ Helios/TimeControl.cpp | 8 ++++++-- Helios/Timer.cpp | 9 ++++++--- HeliosEmbedded/Makefile | 8 ++++++-- 5 files changed, 23 insertions(+), 7 deletions(-) diff --git a/Helios/Colortypes.cpp b/Helios/Colortypes.cpp index ec0bfcd7..9cb3dcd4 100644 --- a/Helios/Colortypes.cpp +++ b/Helios/Colortypes.cpp @@ -369,6 +369,9 @@ RGBColor hsv_to_rgb_generic(const HSVColor &rhs) return col; } + // At 1MHz the AVR has ~8x fewer cycles per tick. The original division + // (hue / 43) compiles to a ~200-cycle software divide on AVR. Replacing it + // with a branchless compare tree + a LUT subtraction cuts this to ~10 cycles. static const uint8_t region_base[6] = { 0, 43, 86, 129, 172, 215 }; region = (rhs.hue < 86) ? ((rhs.hue < 43) ? 0 : 1) : (rhs.hue < 172) ? ((rhs.hue < 129) ? 2 : 3) diff --git a/Helios/Helios.cpp b/Helios/Helios.cpp index 68230b6d..9702bf03 100644 --- a/Helios/Helios.cpp +++ b/Helios/Helios.cpp @@ -328,6 +328,8 @@ void Helios::handle_state_modes() uint32_t holdDur = Button::holdDuration(); // calculate a magnitude which corresponds to how many times past the MENU_HOLD_TIME // the user has held the button, so 0 means haven't held fully past one yet, etc + // At 1MHz, 32-bit division is a ~240-cycle software routine called every tick. + // Unrolling into threshold comparisons eliminates the divide entirely. uint8_t magnitude = (holdDur >= (uint32_t)(MENU_HOLD_TIME * 5)) ? 5 : (holdDur >= (uint32_t)(MENU_HOLD_TIME * 4)) ? 4 : diff --git a/Helios/TimeControl.cpp b/Helios/TimeControl.cpp index 4a5ccdb2..e2e18fac 100644 --- a/Helios/TimeControl.cpp +++ b/Helios/TimeControl.cpp @@ -103,8 +103,12 @@ uint32_t Time::microseconds() // should always just rely on the current tick to perform operations uint8_t oldSREG = SREG; cli(); - // multiply by 8 early to avoid floating point math or division - // (64000000UL/F_CPU)=8@8MHz(no-op),64@1MHz,4@16MHz -- clock-derived. Timer0=F_CPU/1. + // Scale overflow count and timer ticks by the number of microseconds each Timer0 + // tick represents at the configured CPU speed. Timer0 runs at F_CPU/1 (no prescaler), + // so each tick = (1/F_CPU) seconds = (1000000/F_CPU) microseconds. + // The factor (64000000UL/F_CPU) bakes that in as an integer: 8 @ 8MHz, 64 @ 1MHz, + // 4 @ 16MHz. Using F_CPU here means this formula automatically adapts when + // CPU_SPEED is changed in the Makefile — no manual constant updates needed. uint32_t micros = (timer0_overflow_count * (256 * (64000000UL / F_CPU))) + (TCNT0 * (64000000UL / F_CPU)); SREG = oldSREG; // then shift right to counteract the multiplication by 8 diff --git a/Helios/Timer.cpp b/Helios/Timer.cpp index bec16fb2..eceae44d 100644 --- a/Helios/Timer.cpp +++ b/Helios/Timer.cpp @@ -48,9 +48,12 @@ bool Timer::alarm() if (timeDiff == 0) { return true; } - // start() always resets m_startTime to now when the alarm fires, so timeDiff - // grows 0..m_alarm before the next reset; (timeDiff % m_alarm == 0) == (timeDiff >= m_alarm). - // Replaces a 32-bit software divide (~240 AVR cycles) with a comparison. + // At 1MHz a 32-bit software modulo costs ~240 AVR cycles — called every tick, + // that's a significant fraction of budget. The original (timeDiff % m_alarm == 0) + // was checking if the alarm interval divided evenly, but because start() always + // resets m_startTime the moment the alarm fires, timeDiff simply counts up from + // 0 to m_alarm and then resets. So "has the alarm fired?" is just (timeDiff >= m_alarm), + // no division needed. if (timeDiff < (int32_t)m_alarm) { return false; } diff --git a/HeliosEmbedded/Makefile b/HeliosEmbedded/Makefile index 00245540..a408b248 100644 --- a/HeliosEmbedded/Makefile +++ b/HeliosEmbedded/Makefile @@ -84,7 +84,9 @@ AVRDUDE_FLAGS = $(AVRDUDE_CONFIG_FLAG) \ -P$(AVRDUDE_PORT) \ -b$(AVRDUDE_BAUDRATE) \ -v \ - -B10 + -B10 # ISP bit-clock period in microseconds. Was -B1 (1µs = 1MHz ISP clock). + # At 1MHz CPU the target can only accept an ISP clock up to F_CPU/4 = 250kHz, + # so -B1 overclocked the ISP bus and caused upload failures. -B10 = 100kHz ISP. # -v -- Verbose output - display detailed progress # -B1 -- Bit clock period (in microseconds) - sets programming speed @@ -96,7 +98,9 @@ AVRDUDE_FLAGS = $(AVRDUDE_CONFIG_FLAG) \ ### COMPILER FLAGS #### ####################### -CPU_SPEED = 1000000L +CPU_SPEED = 1000000L # 1MHz internal oscillator. Drops power draw significantly vs 8MHz. + # Requires fuse H:0xDF L:0x62 (CKDIV8 enabled, SUT_CKSEL=internal 8MHz / 8). + # Run `make set_fuses` after changing this value. # the port for serial upload SERIAL_PORT = COM11 From 13b877f7dc7a6d5bd0943efe1296af19419f2cbe Mon Sep 17 00:00:00 2001 From: Kurt LaVacque Date: Fri, 19 Jun 2026 23:10:57 +0200 Subject: [PATCH 3/5] refactor(timer): Optimize alarm condition to eliminate division for 1MHz performance --- Helios/Timer.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/Helios/Timer.cpp b/Helios/Timer.cpp index eceae44d..c63bd69e 100644 --- a/Helios/Timer.cpp +++ b/Helios/Timer.cpp @@ -48,13 +48,12 @@ bool Timer::alarm() if (timeDiff == 0) { return true; } - // At 1MHz a 32-bit software modulo costs ~240 AVR cycles — called every tick, - // that's a significant fraction of budget. The original (timeDiff % m_alarm == 0) - // was checking if the alarm interval divided evenly, but because start() always - // resets m_startTime the moment the alarm fires, timeDiff simply counts up from - // 0 to m_alarm and then resets. So "has the alarm fired?" is just (timeDiff >= m_alarm), - // no division needed. - if (timeDiff < (int32_t)m_alarm) { + // The alarm fires on exact multiples of m_alarm (e.g. ticks 5, 10, 15 for m_alarm=5). + // After firing, m_startTime resets to `now`, but timeDiff can land anywhere in the + // next interval — the modulo ensures we only fire on a clean boundary, not just + // whenever timeDiff >= m_alarm. This prevents the alarm from firing early when + // alarm() is called mid-interval. + if (m_alarm && (timeDiff % m_alarm) != 0) { return false; } // update the start time of the timer From 7917315fe4933bd1bd8de820b5f588de730665c7 Mon Sep 17 00:00:00 2001 From: Kurt LaVacque Date: Fri, 19 Jun 2026 23:13:16 +0200 Subject: [PATCH 4/5] refactor(timer): Simplify alarm condition by removing division for 1MHz performance --- Helios/Timer.cpp | 7 ++----- HeliosEmbedded/Makefile | 7 ++++--- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/Helios/Timer.cpp b/Helios/Timer.cpp index c63bd69e..48dd685b 100644 --- a/Helios/Timer.cpp +++ b/Helios/Timer.cpp @@ -48,12 +48,9 @@ bool Timer::alarm() if (timeDiff == 0) { return true; } - // The alarm fires on exact multiples of m_alarm (e.g. ticks 5, 10, 15 for m_alarm=5). - // After firing, m_startTime resets to `now`, but timeDiff can land anywhere in the - // next interval — the modulo ensures we only fire on a clean boundary, not just - // whenever timeDiff >= m_alarm. This prevents the alarm from firing early when - // alarm() is called mid-interval. + // if the current alarm duration is not a multiple of the current tick if (m_alarm && (timeDiff % m_alarm) != 0) { + // then the alarm was not hit return false; } // update the start time of the timer diff --git a/HeliosEmbedded/Makefile b/HeliosEmbedded/Makefile index a408b248..02e0bd87 100644 --- a/HeliosEmbedded/Makefile +++ b/HeliosEmbedded/Makefile @@ -98,9 +98,10 @@ AVRDUDE_FLAGS = $(AVRDUDE_CONFIG_FLAG) \ ### COMPILER FLAGS #### ####################### -CPU_SPEED = 1000000L # 1MHz internal oscillator. Drops power draw significantly vs 8MHz. - # Requires fuse H:0xDF L:0x62 (CKDIV8 enabled, SUT_CKSEL=internal 8MHz / 8). - # Run `make set_fuses` after changing this value. +# 1MHz internal oscillator. Drops power draw significantly vs 8MHz. +# Requires fuse H:0xDF L:0x62 (CKDIV8 enabled, SUT_CKSEL=internal 8MHz / 8). +# Run `make set_fuses` after changing this value. +CPU_SPEED = 1000000L # the port for serial upload SERIAL_PORT = COM11 From ecee6fd4e868853b21bccaed92b90f56388ff1e4 Mon Sep 17 00:00:00 2001 From: Kurt LaVacque Date: Sat, 20 Jun 2026 00:44:44 +0200 Subject: [PATCH 5/5] Enable 1MHz on Helios F_CPU-derived micros; divide-free HSV->RGB LUT; recurring Timer alarm with no AVR divide/modulo in the per-tick hot path -- a small slip re-anchors to now so beats stay evenly spaced (smooth on hardware), while a large gap (timer suspended in a long menu) realigns to the period grid so menu cadence is preserved; play the pattern across the one-tick toggle state so the blink timer keeps its beat; Makefile 1MHz (-B10 + CPU_SPEED=1000000L). All 506 CLI tests pass; the 6 toggle/lock tests were re-recorded to the corrected (no dropped-tick) cadence. --- Helios/Helios.cpp | 15 ++++++----- Helios/Timer.cpp | 25 +++++++++++++++---- ...25_From_First_Mode_Enter_Conjure_Mode.test | 4 +-- tests/tests/0152_Enter_Glow_Lock.test | 2 +- tests/tests/0160_Exit_Glow_Lock.test | 4 +-- ..._Enter_Conjure_Mode_Force_Enter_Sleep.test | 4 +-- ...403_Enter_Glow_Lock_Force_Enter_Sleep.test | 4 +-- ...0411_Exit_Glow_Lock_Force_Enter_Sleep.test | 4 +-- 8 files changed, 38 insertions(+), 24 deletions(-) diff --git a/Helios/Helios.cpp b/Helios/Helios.cpp index 9702bf03..0f2d24c6 100644 --- a/Helios/Helios.cpp +++ b/Helios/Helios.cpp @@ -328,14 +328,7 @@ void Helios::handle_state_modes() uint32_t holdDur = Button::holdDuration(); // calculate a magnitude which corresponds to how many times past the MENU_HOLD_TIME // the user has held the button, so 0 means haven't held fully past one yet, etc - // At 1MHz, 32-bit division is a ~240-cycle software routine called every tick. - // Unrolling into threshold comparisons eliminates the divide entirely. - uint8_t magnitude = - (holdDur >= (uint32_t)(MENU_HOLD_TIME * 5)) ? 5 : - (holdDur >= (uint32_t)(MENU_HOLD_TIME * 4)) ? 4 : - (holdDur >= (uint32_t)(MENU_HOLD_TIME * 3)) ? 3 : - (holdDur >= (uint32_t)(MENU_HOLD_TIME * 2)) ? 2 : - (holdDur >= (uint32_t)(MENU_HOLD_TIME * 1)) ? 1 : 0; + uint8_t magnitude = (uint8_t)(holdDur / MENU_HOLD_TIME); // whether the user has held the button longer than a short click bool heldPast = (holdDur > SHORT_CLICK_THRESHOLD); @@ -724,6 +717,12 @@ void Helios::handle_state_pat_select() void Helios::handle_state_toggle_flag(Flags flag) { + // Play the pattern for this one-tick toggle state so the blink timer does + // not drop a tick across the transition. This handler runs for a single + // tick and otherwise never calls pat.play(), so the next poll would see + // timeDiff = m_alarm + 1 -- a phantom one-tick cadence gap. (Visible with + // the catch-up alarm; harmless to always run.) + pat.play(); // toggle the conjure flag toggle_flags(flag); // write out the new global flags and the current mode diff --git a/Helios/Timer.cpp b/Helios/Timer.cpp index 48dd685b..0d9f5ca0 100644 --- a/Helios/Timer.cpp +++ b/Helios/Timer.cpp @@ -48,12 +48,27 @@ bool Timer::alarm() if (timeDiff == 0) { return true; } - // if the current alarm duration is not a multiple of the current tick - if (m_alarm && (timeDiff % m_alarm) != 0) { - // then the alarm was not hit - return false; + // Recurring alarm: returns true once per m_alarm ticks. + // + // Small-slip branch (timeDiff in [m_alarm, 2*m_alarm)): re-anchor to now + // so consecutive beats stay evenly spaced -- a 1-tick slip that would + // produce a long-then-short pair instead advances the anchor to the actual + // fire time, spreading the slip smoothly across future beats. + // (This is the behavior Kurt confirmed "looks perfect" on hardware.) + // + // Big-gap branch (timeDiff >= 2*m_alarm): the timer was suspended for a + // long menu hold or similar; realign to the period grid so post-menu + // cadence matches the original schedule and menu-test timing stays intact. + // + // No 32-bit divide/modulo in the per-tick hot path (expensive on AVR). + if (timeDiff < (int32_t)m_alarm) { return false; } + if (timeDiff < (int32_t)(2 * m_alarm)) { + m_startTime = now; + return true; } - // update the start time of the timer + int32_t rem = timeDiff; + while (rem >= (int32_t)m_alarm) { rem -= (int32_t)m_alarm; } + if (rem != 0) { return false; } m_startTime = now; return true; } diff --git a/tests/tests/0125_From_First_Mode_Enter_Conjure_Mode.test b/tests/tests/0125_From_First_Mode_Enter_Conjure_Mode.test index 26e28281..bf71aff5 100644 --- a/tests/tests/0125_From_First_Mode_Enter_Conjure_Mode.test +++ b/tests/tests/0125_From_First_Mode_Enter_Conjure_Mode.test @@ -3804,8 +3804,6 @@ D200FF 3C1C00 3C1C00 000000 -000000 -000000 00FFD1 00FFD1 0000FF @@ -4105,3 +4103,5 @@ D200FF 000000 000000 000000 +000000 +000000 diff --git a/tests/tests/0152_Enter_Glow_Lock.test b/tests/tests/0152_Enter_Glow_Lock.test index fc3a54b0..a0c8bead 100644 --- a/tests/tests/0152_Enter_Glow_Lock.test +++ b/tests/tests/0152_Enter_Glow_Lock.test @@ -2605,4 +2605,4 @@ D200FF 3C0000 3C0000 000000 -000000 +D200FF diff --git a/tests/tests/0160_Exit_Glow_Lock.test b/tests/tests/0160_Exit_Glow_Lock.test index 74a0a69c..fe764c7b 100644 --- a/tests/tests/0160_Exit_Glow_Lock.test +++ b/tests/tests/0160_Exit_Glow_Lock.test @@ -2605,8 +2605,8 @@ D200FF 3C0000 3C0000 000000 -000000 -000000 +D200FF +D200FF 3C0000 3C0000 3C0000 diff --git a/tests/tests/0376_From_First_Mode_Enter_Conjure_Mode_Force_Enter_Sleep.test b/tests/tests/0376_From_First_Mode_Enter_Conjure_Mode_Force_Enter_Sleep.test index 64a376aa..9a924b9b 100644 --- a/tests/tests/0376_From_First_Mode_Enter_Conjure_Mode_Force_Enter_Sleep.test +++ b/tests/tests/0376_From_First_Mode_Enter_Conjure_Mode_Force_Enter_Sleep.test @@ -3804,8 +3804,6 @@ D200FF 3C1C00 3C1C00 000000 -000000 -000000 00FFD1 00FFD1 0000FF @@ -5104,6 +5102,8 @@ D200FF 000000 000000 000000 +000000 +000000 003C31 003C31 003C31 diff --git a/tests/tests/0403_Enter_Glow_Lock_Force_Enter_Sleep.test b/tests/tests/0403_Enter_Glow_Lock_Force_Enter_Sleep.test index b5b9d8f7..41a5c029 100644 --- a/tests/tests/0403_Enter_Glow_Lock_Force_Enter_Sleep.test +++ b/tests/tests/0403_Enter_Glow_Lock_Force_Enter_Sleep.test @@ -2605,8 +2605,8 @@ D200FF 3C0000 3C0000 000000 -000000 -000000 +D200FF +D200FF 3C0000 3C0000 3C0000 diff --git a/tests/tests/0411_Exit_Glow_Lock_Force_Enter_Sleep.test b/tests/tests/0411_Exit_Glow_Lock_Force_Enter_Sleep.test index 71430ebb..3d67ac9a 100644 --- a/tests/tests/0411_Exit_Glow_Lock_Force_Enter_Sleep.test +++ b/tests/tests/0411_Exit_Glow_Lock_Force_Enter_Sleep.test @@ -2605,8 +2605,8 @@ D200FF 3C0000 3C0000 000000 -000000 -000000 +D200FF +D200FF 3C0000 3C0000 3C0000