From f28a847329db238ea5ac3b370303cdee66d92073 Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Wed, 1 Jun 2022 10:31:08 -0500 Subject: [PATCH] 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;