finished documentation and such about timer

especially relating to the unexpected behavior i was getting last night

firmware/lib/timer.h

@@ -33,19 +33,64 @@ uint32_t timer__get_milliseconds(void);
 /**                               functions/timer__get_milliseconds/description
  * Return the number of milliseconds since the timer was initialized (mod 2^32)
  *
+ *     ---------------------------------------------------------------------
+ *      number     highest value        in          in        in        in
+ *      of bits    (milliseconds)     seconds     minutes    hours     days
+ *     ---------  ----------------  -----------  ---------  --------  ------
+ *           8                255          0.3        0.0       0.0     0.0
+ *          16              65535         65.5        1.1       0.0     0.0
+ *          32         4294967295    4294967.3    71582.8    1193.0    49.7
+ *     ---------------------------------------------------------------------
+ *
+ *
  * Usage notes:
  *
- * - It's unnecessary to keep 32-bit (or even 16-bit) resolution when storing
- *   the value returned by `timer__get_milliseconds()` if you don't need it.
- *   Casting to a smaller unsigned value should be safe (as long as you cast
- *   *all* the values you plan to compare with each other to the same type!).
+ * - It's unnecessary to keep 32-bit resolution when storing the value returned
+ *   by `timer__get_milliseconds()` if you don't need it.  Use variables of the
+ *   smallest type that can (*always*) hold the amount of time you'll be
+ *   dealing with.
  *
  * - Use `end_time - start_time` for determining time difference.  Since the
  *   returned values are unsigned (and you should be storing them in unsigned
  *   variables as well) this will work across overflows, for up to the maximum
  *   amount of milliseconds representable by the type you're using.  (See [this
  *   answer] (http://stackoverflow.com/a/50632) on <http://stackoverflow.com/>
- *   if you're curious as to why.)
+ *   if you're curious as to why this workes across overflows.)
+ *
+ *
+ * Warnings:
+ *
+ * - Do not cast the return value of `timer__get_milliseconds()` directly.
+ *   Instead, store the return value in a smaller variable
+ *
+ *       uint8_t start_time = timer__get_milliseconds()
+ *
+ *   or cast the expression as a whole
+ *
+ *       (uint8_t)( timer__get_milliseconds() - start_time )
+ *
+ *   Casting directly within the end condition check of a `while` or `for` loop
+ *   does not produce the desired behavior.  I don't know assembly well enough
+ *   to figure out what it's *actually* doing, but the code generated is longer
+ *   than when casting as shown, and it doesn't appear to be throwing away all
+ *   but the least significant 8 bits before calculation, as I'd expect.
+ *   Casting directly when assigning to a variable doesn't appear to do
+ *   anything, and casting directly outside end condition checks seems to work
+ *   at least sometimes, but those casts may as well be avoided for
+ *   consistency.
+ *
+ * - Be careful when subtracting (or doing anything, really, with multiple)
+ *   times, to make sure that if the values are not of the same type, the
+ *   expression is cast to the smallest type you're using.  If `start_time` is
+ *   a `uint8_t` value,
+ *
+ *       (uint8_t)( timer__get_milliseconds() - start_time )
+ *
+ *   has a very different meaning than
+ *
+ *       timer__get_milliseconds() - start_time
+ *
+ *   except within the first 255 milliseconds of the timer being initialized.
  */
 
 

firmware/main.c

@@ -82,13 +82,6 @@ int main(void) {
     time_scan_started  // on the first iteration, scan immediately
         = timer__get_milliseconds() - OPT__DEBOUNCE_TIME;
 
-    // TODO
-    // - test the scan rate, to make sure it's what we want
-    // - write a nice note about not casting `timer__get_milliseconds()` inside
-    //   while loops... or maybe write a `timer__diff_with_now()` or something
-    //   function
-    // - look at the generated asm for these things, maybe, to see what's going
-    // on
     for(;;) {
         temp = is_pressed;
         is_pressed = was_pressed;
@@ -97,7 +90,7 @@ int main(void) {
         // delay if necessary, then rescan
         // - add 1 to `OPT__DEBOUNCE_TIME` in case `time_scan_started` caught
         //   the tail end of the millisecond it recorded
-        while( timer__get_milliseconds() - time_scan_started
+        while( (uint8_t)(timer__get_milliseconds()-time_scan_started)
                < OPT__DEBOUNCE_TIME + 1 );
         time_scan_started = timer__get_milliseconds();
         kb__update_matrix(*is_pressed);