fixed probable timing bug in timer functions; documentation

doc/references.md

@@ -208,6 +208,10 @@
   (http://www.avrfreaks.net/wiki/index.php/Documentation:AVR_GCC/AVR_GCC_Tool_Collection)  
   Nice picture, and short descriptions of the relevent command line tools.
 
+* [A Very Simple Arduino Task Manager]
+  (http://bleaklow.com/2010/07/20/a_very_simple_arduino_task_manager.html)  
+  A nice little write up on cooperative multitasking with an AVR.
+
 
 ## Protocol Stuff
 

firmware/lib/timer.h

@@ -139,7 +139,7 @@ void timer___tick_cycles (void);
  *
  *       timer__get_milliseconds() - start_time
  *
- *   except within the first 255 milliseconds of the timer being initialized.
+ *   except within the first 2^8 milliseconds of the timer being initialized.
  */
 
 // === timer__schedule_cycles() ===
@@ -155,12 +155,11 @@ void timer___tick_cycles (void);
  * - failure: [other]
  *
  * Usage notes:
- * - See the documentation for `timer__schedule_milliseconds()`
- * - This function should only be preferable to
- *   `timer__schedule_milliseconds()` (performance wise) due to the cycles
- *   counter having a lower real time resolution (and therefore slightly lower
- *   bookkeeping overhead).  Functions scheduled with either will be run with
- *   interrupts enabled.  Use whichever function makes more sense logically.
+ * - If possible, prefer this function to `timer__schedule_milliseconds()`: it
+ *   has slightly lower overhead (since scan cycles are going to be longer than
+ *   1 millisecond); and since functions scheduled here are not executed inside
+ *   an interrupt vector, you need not worry about any of them messing with
+ *   access to a shared resource.
  */
 
 // === timer__schedule_milliseconds() ===
@@ -176,11 +175,20 @@ void timer___tick_cycles (void);
  * - failure: [other]
  *
  * Usage notes:
+ * - Functions will be run with interrupts enabled, so you need not worry more
+ *   than usual about how long your function takes.
  * - If a function needs a longer wait time than is possible with a 16-bit
  *   millisecond resolution counter, it can repeatedly schedule itself to run
  *   in, say, 1 minute (= 1000*60 milliseconds), increment a counter each time,
  *   and then only execute its body code after, say, 5 calls (for a 5 minute
  *   delay).
+ *
+ * Warnings:
+ * - Be *very* careful when using this to schedule functions that share
+ *   resources: functions scheduled here are still called from within an
+ *   interrupt vector, and the interrupt vector may have interrupted something
+ *   else in the middle of an access to that resource.  You must pay full
+ *   attention to all the issues this could possibly cause.
  */
 
 // ----------------------------------------------------------------------------

firmware/lib/timer/device/atmega32u4.c

@@ -99,8 +99,8 @@ uint8_t timer__schedule_milliseconds( uint16_t milliseconds,
 // ----------------------------------------------------------------------------
 
 void timer___tick_cycles (void) {
-        _cycles++;
-        event_list__tick(_cycles__scheduled);
+    _cycles++;
+    event_list__tick(_cycles__scheduled);
 }
 
 // ----------------------------------------------------------------------------

firmware/lib/timer/event-list.h

@@ -20,7 +20,8 @@
  *   the completion of a scan cycle.
  *
  * - To "tick" (as a verb) is to denote the passage of a "tick" of time by
- *   performing the actions corresponding thereto.
+ *   performing the actions corresponding thereto (i.e. incrementing the
+ *   counter and running any scheduled events).
  *
  * - All functions declared here should be safe for use within interrupt
  *   routines, as long as you pay attention to the warnings below.

firmware/lib/timer/event-list/atmega32u4.c

@@ -6,12 +6,16 @@
 
 /**                                                                 description
  * Implements the event-list interface defined in "../event-list.h" for the
- * ATMega32U4 (though, the code should be the same for anything in the AVR
- * family)
+ * ATMega32U4
+ *
+ * Notes:
+ * - The code should be the same for anything in the AVR family.  It's the
+ *   `<util/atomic>` macros that make this non-universal.
  */
 
 
 #include <stdint.h>
+#include <stdlib.h>
 #include <util/atomic.h>
 #include "../../../../firmware/lib/data-types/list.h"
 
@@ -46,18 +50,35 @@ uint8_t event_list__append( list__list_t * list,
 void event_list__tick(list__list_t * list) {
     if (!list) return;
 
+    event_t * next;
+    event_t * run = NULL;  // for keeping track of events to run
+
+    // go through the list
+    // - keep track of the events that need to be run this "tick"
+    // - note that every other event is one "tick" closer to being run
     ATOMIC_BLOCK( ATOMIC_RESTORESTATE ) {
         for (event_t * event = list->head; event;) {
             if (event->ticks == 0) {
-                NONATOMIC_BLOCK( NONATOMIC_RESTORESTATE ) {
-                    (*event->function)();
-                }
-                event = list__pop_node_next(list, event);
+                next = event->_private.next;
+                list__pop_node(list, event);
+                event->_private.next = run;
+                run = event;
+                event = next;
             } else {
                 event->ticks--;
                 event = event->_private.next;
             }
         }
     }
+
+    // run all the scheduled events, with interrupts enabled
+    NONATOMIC_BLOCK( NONATOMIC_RESTORESTATE ) {
+        while (run) {
+            next = run->_private.next;
+            (*run->function)();
+            free(run);
+            run = next;
+        }
+    }
 }