From 041543a0b3cd4016a942667d2892bf1da3b6cd14 Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Wed, 1 Jun 2022 10:29:00 -0500 Subject: [PATCH 1/4] Deprecate public variable `CycleTimeReport::average_loop_time` Signed-off-by: Michael Richters --- .../src/kaleidoscope/plugin/CycleTimeReport.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/plugins/Kaleidoscope-CycleTimeReport/src/kaleidoscope/plugin/CycleTimeReport.h b/plugins/Kaleidoscope-CycleTimeReport/src/kaleidoscope/plugin/CycleTimeReport.h index 5b70dd4d..07a94c59 100644 --- a/plugins/Kaleidoscope-CycleTimeReport/src/kaleidoscope/plugin/CycleTimeReport.h +++ b/plugins/Kaleidoscope-CycleTimeReport/src/kaleidoscope/plugin/CycleTimeReport.h @@ -21,6 +21,15 @@ #include "kaleidoscope/event_handler_result.h" // for EventHandlerResult #include "kaleidoscope/plugin.h" // for Plugin +// ----------------------------------------------------------------------------- +// Deprecation warning messages +#include "kaleidoscope_internal/deprecations.h" // for DEPRECATED + +#define _DEPRECATED_MESSAGE_CYCLETIMEREPORT_AVG_TIME \ + "The `CycleTimeReport.average_loop_time` variable is deprecated. See\n" \ + "the current documentation for CycleTimeReport for details.\n" \ + "This variable will be removed after 2022-09-01." +// ----------------------------------------------------------------------------- namespace kaleidoscope { namespace plugin { @@ -32,7 +41,10 @@ class CycleTimeReport : public kaleidoscope::Plugin { EventHandlerResult beforeEachCycle(); EventHandlerResult afterEachCycle(); +#ifndef NDEPRECATED + DEPRECATED(CYCLETIMEREPORT_AVG_TIME) static uint32_t average_loop_time; +#endif private: static uint16_t last_report_time_; From 45c33c04f01b0e5877f9eeef292e242cd145f3a6 Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Wed, 1 Jun 2022 10:30:18 -0500 Subject: [PATCH 2/4] Remove unnecessary constructor definition Signed-off-by: Michael Richters --- .../src/kaleidoscope/plugin/CycleTimeReport.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/plugins/Kaleidoscope-CycleTimeReport/src/kaleidoscope/plugin/CycleTimeReport.h b/plugins/Kaleidoscope-CycleTimeReport/src/kaleidoscope/plugin/CycleTimeReport.h index 07a94c59..4646ba73 100644 --- a/plugins/Kaleidoscope-CycleTimeReport/src/kaleidoscope/plugin/CycleTimeReport.h +++ b/plugins/Kaleidoscope-CycleTimeReport/src/kaleidoscope/plugin/CycleTimeReport.h @@ -35,8 +35,6 @@ namespace kaleidoscope { namespace plugin { class CycleTimeReport : public kaleidoscope::Plugin { public: - CycleTimeReport() {} - EventHandlerResult onSetup(); EventHandlerResult beforeEachCycle(); EventHandlerResult afterEachCycle(); From f28a847329db238ea5ac3b370303cdee66d92073 Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Wed, 1 Jun 2022 10:31:08 -0500 Subject: [PATCH 3/4] Rewrite CycleTimeReport plugin The plugin was both more complex and less accurate than it could have been. For simplicity, it used a weighted average, with each cycle getting twice the weight of the previous one. As a result, the reported average really only took into account the last three or four cycles. On a keyboard with LEDs, some cycles take much longer than others because of relatively rare updates, so this could lead to misleading results, with the "average" cycle time usually being reported as lower than it really should have been, and occasionally much higher. This new version computes an evenly-weighted mean cycle time for each interval, and runs more efficiently, by dividing the total elapsed time by the number of cycles that has passed since the last report, rather than computing the time for each individual cycle. Signed-off-by: Michael Richters --- .../Kaleidoscope-CycleTimeReport/README.md | 22 ++++----- .../kaleidoscope/plugin/CycleTimeReport.cpp | 49 +++++++++---------- .../src/kaleidoscope/plugin/CycleTimeReport.h | 22 ++++++--- 3 files changed, 48 insertions(+), 45 deletions(-) diff --git a/plugins/Kaleidoscope-CycleTimeReport/README.md b/plugins/Kaleidoscope-CycleTimeReport/README.md index 2f3f0e50..4d2fc943 100644 --- a/plugins/Kaleidoscope-CycleTimeReport/README.md +++ b/plugins/Kaleidoscope-CycleTimeReport/README.md @@ -16,7 +16,7 @@ the box, without any further configuration: KALEIDOSCOPE_INIT_PLUGINS(CycleTimeReport); -void setup (void) { +void setup () { Kaleidoscope.serialPort().begin(9600); Kaleidoscope.setup (); } @@ -25,25 +25,23 @@ void setup (void) { ## Plugin methods The plugin provides a single object, `CycleTimeReport`, with the following -property. All times are in milliseconds. +methods: -### `.average_loop_time` +### `.setReportInterval(interval)` -> A read-only by contract value, the average time of main loop lengths between -> two reports. +> Sets the length of time between reports to `interval` milliseconds. The +> default is `1000`, so it will report once per second. -## Overrideable methods +### `.report(mean_cycle_time)` -### `cycleTimeReport()` - -> Reports the average loop time. By default, it does so over `Serial`, every -> time when the report period is up. +> Reports the average (mean) cycle time since the previous report. This method +> is called automatically, once per report interval (see above). By default, it +> does so over `Serial`. > > It can be overridden, to change how the report looks, or to make the report > toggleable, among other things. > -> It takes no arguments, and returns nothing, but has access to -> `CycleTimeReport.average_loop_time` above. +> It takes no arguments, and returns nothing. ## Further reading diff --git a/plugins/Kaleidoscope-CycleTimeReport/src/kaleidoscope/plugin/CycleTimeReport.cpp b/plugins/Kaleidoscope-CycleTimeReport/src/kaleidoscope/plugin/CycleTimeReport.cpp index 2e46812b..c4cc4b7d 100644 --- a/plugins/Kaleidoscope-CycleTimeReport/src/kaleidoscope/plugin/CycleTimeReport.cpp +++ b/plugins/Kaleidoscope-CycleTimeReport/src/kaleidoscope/plugin/CycleTimeReport.cpp @@ -26,43 +26,40 @@ namespace kaleidoscope { namespace plugin { -uint16_t CycleTimeReport::last_report_time_; -uint32_t CycleTimeReport::loop_start_time_; -uint32_t CycleTimeReport::average_loop_time; - -EventHandlerResult CycleTimeReport::onSetup() { - last_report_time_ = Runtime.millisAtCycleStart(); - return EventHandlerResult::OK; -} EventHandlerResult CycleTimeReport::beforeEachCycle() { - loop_start_time_ = micros(); - return EventHandlerResult::OK; -} - -EventHandlerResult CycleTimeReport::afterEachCycle() { - uint32_t loop_time = micros() - loop_start_time_; + // A counter storing the number of cycles since the last mean cycle time + // report was sent: + static uint16_t elapsed_cycles = 0; - if (average_loop_time) - average_loop_time = (average_loop_time + loop_time) / 2; - else - average_loop_time = loop_time; + // We only compute the mean cycle time once per report interval. + if (Runtime.hasTimeExpired(last_report_millis_, report_interval_)) { + uint32_t elapsed_time = micros() - last_report_micros_; + uint32_t mean_cycle_time = elapsed_time / elapsed_cycles; - if (Runtime.hasTimeExpired(last_report_time_, uint16_t(1000))) { - cycleTimeReport(); + report(mean_cycle_time); - average_loop_time = 0; - last_report_time_ = Runtime.millisAtCycleStart(); + // Reset the cycle counter and timestamps. + elapsed_cycles = 0; + last_report_millis_ += report_interval_; + last_report_micros_ = micros(); } + // Increment the cycle counter unconditionally. + ++elapsed_cycles; + return EventHandlerResult::OK; } +__attribute__((weak)) void CycleTimeReport::report(uint16_t mean_cycle_time) { + Focus.send(Focus.COMMENT, + F("mean cycle time:"), + mean_cycle_time, + F("µs"), + Focus.NEWLINE); +} + } // namespace plugin } // namespace kaleidoscope -__attribute__((weak)) void cycleTimeReport(void) { - Focus.send(Focus.COMMENT, F("average loop time:"), CycleTimeReport.average_loop_time, Focus.NEWLINE); -} - kaleidoscope::plugin::CycleTimeReport CycleTimeReport; diff --git a/plugins/Kaleidoscope-CycleTimeReport/src/kaleidoscope/plugin/CycleTimeReport.h b/plugins/Kaleidoscope-CycleTimeReport/src/kaleidoscope/plugin/CycleTimeReport.h index 4646ba73..34e09987 100644 --- a/plugins/Kaleidoscope-CycleTimeReport/src/kaleidoscope/plugin/CycleTimeReport.h +++ b/plugins/Kaleidoscope-CycleTimeReport/src/kaleidoscope/plugin/CycleTimeReport.h @@ -17,7 +17,7 @@ #pragma once -#include // for uint32_t, uint16_t +#include // for uint16_t, uint32_t #include "kaleidoscope/event_handler_result.h" // for EventHandlerResult #include "kaleidoscope/plugin.h" // for Plugin @@ -35,23 +35,31 @@ namespace kaleidoscope { namespace plugin { class CycleTimeReport : public kaleidoscope::Plugin { public: - EventHandlerResult onSetup(); EventHandlerResult beforeEachCycle(); - EventHandlerResult afterEachCycle(); #ifndef NDEPRECATED DEPRECATED(CYCLETIMEREPORT_AVG_TIME) static uint32_t average_loop_time; #endif + /// Set the length of time between reports (in milliseconds) + void setReportInterval(uint16_t interval) { + report_interval_ = interval; + } + + /// Report the given mean cycle time in microseconds + void report(uint16_t mean_cycle_time); + private: - static uint16_t last_report_time_; - static uint32_t loop_start_time_; + // Interval between reports, in milliseconds + uint16_t report_interval_ = 1000; + + // Timestamps recording when the last report was sent + uint16_t last_report_millis_ = 0; + uint32_t last_report_micros_ = 0; }; } // namespace plugin } // namespace kaleidoscope -void cycleTimeReport(void); - extern kaleidoscope::plugin::CycleTimeReport CycleTimeReport; From 9c5df517ae764dfc60ab6e7502781f4d66bc394a Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Wed, 1 Jun 2022 10:42:02 -0500 Subject: [PATCH 4/4] Update example sketch for CycleTimeReport plugin Signed-off-by: Michael Richters --- examples/Features/CycleTimeReport/CycleTimeReport.ino | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/examples/Features/CycleTimeReport/CycleTimeReport.ino b/examples/Features/CycleTimeReport/CycleTimeReport.ino index be45aebc..e8c9cc33 100644 --- a/examples/Features/CycleTimeReport/CycleTimeReport.ino +++ b/examples/Features/CycleTimeReport/CycleTimeReport.ino @@ -40,11 +40,20 @@ KEYMAPS( ) // clang-format on +// Override CycleTimeReport's reporting function: +void kaleidoscope::plugin::CycleTimeReport::report(uint16_t mean_cycle_time) { + Serial.print(F("average loop time = ")); + Serial.println(mean_cycle_time, DEC); +} + KALEIDOSCOPE_INIT_PLUGINS(CycleTimeReport); void setup() { Kaleidoscope.serialPort().begin(9600); Kaleidoscope.setup(); + + // Change the report interval to 2 seconds: + CycleTimeReport.setReportInterval(2000); } void loop() {