From 300e1afacb60972a679de6e9c42aabe86a431cc2 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Thu, 25 Oct 2018 22:53:08 +0200 Subject: [PATCH 1/2] IdleLEDs: Use an `end_time` instead of calculating it each cycle Signed-off-by: Gergely Nagy --- src/kaleidoscope/plugin/IdleLEDs.cpp | 8 +++----- src/kaleidoscope/plugin/IdleLEDs.h | 4 ++-- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/kaleidoscope/plugin/IdleLEDs.cpp b/src/kaleidoscope/plugin/IdleLEDs.cpp index fe9f9c30..8ef92d9a 100644 --- a/src/kaleidoscope/plugin/IdleLEDs.cpp +++ b/src/kaleidoscope/plugin/IdleLEDs.cpp @@ -22,13 +22,11 @@ namespace kaleidoscope { namespace plugin { uint16_t IdleLEDs::idle_time_limit = 600; // 10 minutes -uint32_t IdleLEDs::last_keypress_time_; +uint32_t IdleLEDs::end_time_; EventHandlerResult IdleLEDs::beforeEachCycle() { - uint32_t idle_limit = idle_time_limit * 1000; - if (!::LEDControl.paused && - Kaleidoscope.millisAtCycleStart() - last_keypress_time_ >= idle_limit) { + (Kaleidoscope.millisAtCycleStart() >= end_time_)) { ::LEDControl.set_all_leds_to(CRGB(0, 0, 0)); ::LEDControl.syncLeds(); @@ -45,7 +43,7 @@ EventHandlerResult IdleLEDs::onKeyswitchEvent(Key &mapped_key, byte row, byte co ::LEDControl.refreshAll(); } - last_keypress_time_ = Kaleidoscope.millisAtCycleStart(); + end_time_ = Kaleidoscope.millisAtCycleStart() + idle_time_limit * 1000; return EventHandlerResult::OK; } diff --git a/src/kaleidoscope/plugin/IdleLEDs.h b/src/kaleidoscope/plugin/IdleLEDs.h index edfa2826..e6e307a4 100644 --- a/src/kaleidoscope/plugin/IdleLEDs.h +++ b/src/kaleidoscope/plugin/IdleLEDs.h @@ -29,14 +29,14 @@ class IdleLEDs: public kaleidoscope::Plugin { static uint16_t idle_time_limit; EventHandlerResult onSetup() { - last_keypress_time_ = millis(); + end_time_ = millis() + idle_time_limit * 1000; return EventHandlerResult::OK; } EventHandlerResult beforeEachCycle(); EventHandlerResult onKeyswitchEvent(Key &mapped_key, byte row, byte col, uint8_t key_state); private: - static uint32_t last_keypress_time_; + static uint32_t end_time_; }; } } From 31d64a2f3d4a2007c8bb674f8d9078c9d73b1c46 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Fri, 26 Oct 2018 00:00:16 +0200 Subject: [PATCH 2/2] IdleLEDs: Explicitly cast idle_time_limit to uint32_t When calculating `end_time`, explicitly cast `idle_time_limit` to 32 bits, otherwise the `idle_time_limit * 1000` operation will be done on 16 bits, which would overflow at about 65 seconds. With the cast, the operation will use all 32 bits, and we avoid the overflow. Many thanks to @nevd for the report and the help in debugging & testing the fix. Signed-off-by: Gergely Nagy --- src/kaleidoscope/plugin/IdleLEDs.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kaleidoscope/plugin/IdleLEDs.cpp b/src/kaleidoscope/plugin/IdleLEDs.cpp index 8ef92d9a..00918e60 100644 --- a/src/kaleidoscope/plugin/IdleLEDs.cpp +++ b/src/kaleidoscope/plugin/IdleLEDs.cpp @@ -43,7 +43,7 @@ EventHandlerResult IdleLEDs::onKeyswitchEvent(Key &mapped_key, byte row, byte co ::LEDControl.refreshAll(); } - end_time_ = Kaleidoscope.millisAtCycleStart() + idle_time_limit * 1000; + end_time_ = Kaleidoscope.millisAtCycleStart() + (uint32_t)idle_time_limit * 1000; return EventHandlerResult::OK; }