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 <gedankenexperimenter@gmail.com>

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
+> Sets the length of time between reports to `interval` milliseconds.  The
-> two reports.
+> default is `1000`, so it will report once per second.
 
-## Overrideable methods
+### `.report(mean_cycle_time)`
 
-### `cycleTimeReport()`
+> Reports the average (mean) cycle time since the previous report.  This method
-
+> is called automatically, once per report interval (see above).  By default, it
-> Reports the average loop time. By default, it does so over `Serial`, every
+> does so over `Serial`.
-> time when the report period is up.
 >
 > 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
+> It takes no arguments, and returns nothing.
-> `CycleTimeReport.average_loop_time` above.
 
 ## Further reading
 

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();
+  // A counter storing the number of cycles since the last mean cycle time
-  return EventHandlerResult::OK;
+  // report was sent:
-}
+  static uint16_t elapsed_cycles = 0;
-
-EventHandlerResult CycleTimeReport::afterEachCycle() {
-  uint32_t loop_time = micros() - loop_start_time_;
 
-  if (average_loop_time)
+  // We only compute the mean cycle time once per report interval.
-    average_loop_time = (average_loop_time + loop_time) / 2;
+  if (Runtime.hasTimeExpired(last_report_millis_, report_interval_)) {
-  else
+    uint32_t elapsed_time    = micros() - last_report_micros_;
-    average_loop_time = loop_time;
+    uint32_t mean_cycle_time = elapsed_time / elapsed_cycles;
 
-  if (Runtime.hasTimeExpired(last_report_time_, uint16_t(1000))) {
+    report(mean_cycle_time);
-    cycleTimeReport();
 
-    average_loop_time = 0;
+    // Reset the cycle counter and timestamps.
-    last_report_time_ = Runtime.millisAtCycleStart();
+    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;

plugins/Kaleidoscope-CycleTimeReport/src/kaleidoscope/plugin/CycleTimeReport.h

@@ -17,7 +17,7 @@
 
 #pragma once
 
-#include <stdint.h>  // for uint32_t, uint16_t
+#include <stdint.h>  // 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_;
+  // Interval between reports, in milliseconds
-  static uint32_t loop_start_time_;
+  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;