From 09441b496c6b40be151e4e655572cb37518e7225 Mon Sep 17 00:00:00 2001
From: "R. Steve McKown" <rsmckown@gmail.com>
Date: Sat, 10 Dec 2011 01:31:35 -0700
Subject: [PATCH] These changes save 685 code words

---
 btn.c  | 33 +++++++++++++++++++++++++++++++++
 btn.h  | 28 +++++-----------------------
 isr.c  | 15 +++++++++++++++
 isr.h  | 14 ++------------
 main.c | 29 +++++++++++++----------------
 rgb.h  |  2 +-
 task.c | 16 +++++++++-------
 task.h |  4 ++--
 8 files changed, 80 insertions(+), 61 deletions(-)

diff --git a/btn.c b/btn.c
index 84a73aa..2fefcbe 100644
--- a/btn.c
+++ b/btn.c
@@ -25,6 +25,39 @@ void btn_init()
   GIE = 1;             /* enable global interrupts */
 }
 
+void btn_pben()
+{
+  ndi();
+  IOCBP0 = 1; IOCBN0 = 1; IOCBF0 = 0;
+  nei();
+}
+
+/* Disable the pushbutton from user code */
+void btn_pbdis()
+{
+  ndi();
+  IOCBP0 = 0; IOCBN0 = 0; IOCBF0 = 0;
+  nei();
+}
+
+/* Enable the rocker switch from user code */
+void btn_rsen()
+{
+  ndi();
+  IOCBP4 = 1; IOCBN4 = 1; IOCBF4 = 0;
+  IOCBP5 = 1; IOCBN5 = 1; IOCBF5 = 0;
+  nei();
+}
+
+/* Disable the rocker switch from user code */
+void btn_rsdis()
+{
+  ndi();
+  IOCBP4 = 0; IOCBN4 = 0; IOCBF4 = 0;
+  IOCBP5 = 0; IOCBN5 = 0; IOCBF5 = 0;
+  nei();
+}
+
 void btn_isr()
 {
   if (IOCIF) {
diff --git a/btn.h b/btn.h
index e0ce0ae..093298b 100644
--- a/btn.h
+++ b/btn.h
@@ -30,37 +30,19 @@ enum {
 #define btn_pb() (RB0)
 
 /* Read the current state of the rocker switch */
-#define btn_rs() ((RB4 + ((unsigned char)RB5 << 1)) % 3)
+#define btn_rs() (RB4 + ((unsigned char)RB5 << 1))
 
 /* Enable the pushbutton from user code */
-#define btn_pben() do { \
-    ndi(); \
-    IOCBP0 = 1; IOCBN0 = 1; IOCBF0 = 0; \
-    nei(); \
-  } while (0)
+void btn_pben();
 
 /* Disable the pushbutton from user code */
-#define btn_pbdis() do { \
-    ndi(); \
-    IOCBP0 = 0; IOCBN0 = 0; IOCBF0 = 0; \
-    nei(); \
-  } while (0)
+void btn_pbdis();
 
 /* Enable the rocker switch from user code */
-#define btn_rsen() do { \
-    ndi(); \
-    IOCBP4 = 1; IOCBN4 = 1; IOCBF4 = 0; \
-    IOCBP5 = 1; IOCBN5 = 1; IOCBF5 = 0; \
-    nei(); \
-  } while (0)
+void btn_rsen();
 
 /* Disable the rocker switch from user code */
-#define btn_rsdis() do { \
-    ndi(); \
-    IOCBP4 = 0; IOCBN4 = 0; IOCBF4 = 0; \
-    IOCBP5 = 0; IOCBN5 = 0; IOCBF5 = 0; \
-    nei(); \
-  } while (0)
+void btn_rsdis();
 
 /* Initialize the button module */
 void btn_init();
diff --git a/isr.c b/isr.c
index a1b9a49..8db018f 100644
--- a/isr.c
+++ b/isr.c
@@ -13,6 +13,21 @@
 bit _isr_gie; /* Used to store the state of GIE for nested ndi()/nei() */
 unsigned char _isr_di; /* Count of nested ndi() */
 
+void ndi()
+{
+  if (_isr_di++ == 0) {
+    _isr_gie = GIE;
+    di();
+  }
+}
+
+/* Nested enable interrupts inline function.  Should be OK even in ISR. */
+void nei()
+{
+  if (--_isr_di == 0 && _isr_gie)
+    ei();
+}
+
 void interrupt isr()
 {
   tmr_isr();
diff --git a/isr.h b/isr.h
index 4b84774..220b381 100644
--- a/isr.h
+++ b/isr.h
@@ -13,20 +13,10 @@ extern bit _isr_gie;
 extern unsigned char _isr_di;
 
 /* Nested disable interrupts inline function.  Should be OK even in ISR. */
-#define ndi() \
-  do { \
-    if (_isr_di++ == 0) { \
-      _isr_gie = GIE; \
-      di(); \
-    } \
-  } while (0)
+void ndi();
 
 /* Nested enable interrupts inline function.  Should be OK even in ISR. */
-#define nei() \
-  do { \
-    if (--_isr_di == 0 && _isr_gie) \
-      ei(); \
-  } while (0)
+void nei();
 
 void interrupt isr();
 
diff --git a/main.c b/main.c
index ac75428..1df9445 100644
--- a/main.c
+++ b/main.c
@@ -37,16 +37,8 @@
 
 #define AUTO_OFF_COUNT          549316UL  /*  5 hrs in 32.768 ms units */
 #define AUTO_ON_COUNT           2087402UL /* 19 hrs in 32.768 ms units */
-#define rand_u8()               (rand() & 0xff)
-#define rand_u16()              ((rand() << 8) + rand_u8())
-#define rand_incolor_steps(s)   (min_incolor_steps[s & 3] + \
-                                    (rand() % range_incolor_steps[s & 3]))
-#define rand_fade_steps(s)      (min_fade_steps[s & 3] + \
-                                    (rand() % range_fade_steps[s & 3]))
-#define leds_set(r,g,b,w)       rgb_set((r).value >> 7, \
-                                    (g).value >> 7, \
-                                    (b).value >> 7, \
-                                    0)
+#define leds_set(r,g,b,w)       rgb_set((r).value >> 7, (g).value >> 7, \
+                                    (b).value >> 7, 0)
 #define dbgpin_init()           do { \
                                     /* Set RA2 as output low */ \
                                     RA2 = 0; \
@@ -62,11 +54,14 @@ typedef struct {
   signed char remainder;
 } led_t;
 
-/* The index of all step arrays is the speed variable */
+/* The index of all step arrays is the speed variable.  min values must be
+ * at least one.  range values are bit-ANDed with rand() to generate a range, so
+ * ensure they are one less than a power of 2 and at least 1.
+ */
 const static unsigned min_incolor_steps[4] =   {   320,  32, 32,  1 };
-const static unsigned range_incolor_steps[4] = { 32768, 128, 32,  8 };
-const static int min_fade_steps[4] =           {    64,  32, 32,  1 };
-const static int range_fade_steps[4] =         {   416, 128, 32,  8 };
+const static unsigned range_incolor_steps[4] = { 32767, 127, 31,  7 };
+const static int min_fade_steps[4] =           {    64,  32, 32,  8 };
+const static int range_fade_steps[4] =         {   511, 127, 31,  7 };
 
 led_t red;
 led_t grn;
@@ -93,7 +88,8 @@ void start_fade()
   } while (newr == 0 && newg == 0 && newb == 0 && neww == 0);
 
   /* Random # of steps to reach the new color */
-  fade_steps = rand_fade_steps(speed);
+  fade_steps = min_fade_steps[speed & 3] +
+      (rand() & range_fade_steps[speed & 3]);
 
   /* Compute increment per fade step, and remainder, for each led */
   red.increment = (newr - red.value) / fade_steps;
@@ -178,7 +174,8 @@ void fade_task()
     blu.value += blu.remainder;
     wht.value += wht.remainder;
     tmr_stop(TMR_FADE);
-    tmr_start(TMR_INCOLOR, rand_incolor_steps(speed));
+    tmr_start(TMR_INCOLOR, min_incolor_steps[speed & 3] +
+          (rand() & range_incolor_steps[speed & 3]));
   }
   leds_set(red, grn, blu, wht);
 }
diff --git a/rgb.h b/rgb.h
index d24b41b..05a061c 100644
--- a/rgb.h
+++ b/rgb.h
@@ -14,7 +14,7 @@
 void rgb_init();
 
 /* Turn on the rgb.  Outputs are zero, or last values set by rgb_set(). */
-#define rgb_on() do { TMR2ON = 1; } while (0)
+#define rgb_on() (TMR2ON = 1)
 
 /* Turn off the rgb, first setting outputs to zero. */
 void rgb_off();
diff --git a/task.c b/task.c
index bce2b6d..d704d60 100644
--- a/task.c
+++ b/task.c
@@ -36,22 +36,24 @@ task_id_t task_get(unsigned char block)
     nei();
     if (ids) {
       for (unsigned char i = 0; t == -1 && i < TASK_COUNT; i++) {
-        if (bit_get(ids, _task_bitno))
+        if (bit_get(ids, _task_bitno)) {
           t = _task_bitno;
+          ndi();
+          bit_clr(_task_ids, t);
+          nei();
+        }
         if (++_task_bitno == TASK_COUNT)
           _task_bitno = 0;
       }
     }
-#if 0 /* Not until we have a crystal and can wake from sleep via tmr module */
+#if 0
+    /* Something like this when we have a crystal.  But watch for the race of
+     * going to sleep when a task is posted by an ISR.
+     */
     else
       SLEEP();
 #endif
   } while (t == -1 && block == 1);
-  if (t >= 0) {
-    ndi();
-    bit_clr(_task_ids, t);
-    nei();
-  }
   return t;
 }
 
diff --git a/task.h b/task.h
index b53640f..d08a363 100644
--- a/task.h
+++ b/task.h
@@ -41,10 +41,10 @@ extern unsigned long _task_ids;
 extern task_id_t _task_bitno;
 
 /* Post task t.  No need for ndi() since bit_set is a single instruction. */
-#define task_post(t) do { bit_set(_task_ids, t); } while (0)
+#define task_post(t) (bit_set(_task_ids, t))
 
 /* Initialize the task subsystem */
-#define task_init() do { _task_ids = 0; _task_bitno = 0; } while (0)
+#define task_init() (_task_ids = 0)
 
 /* Returns non-zero if one or more tasks are posted.  Does not block. */
 bit task_check();
-- 
2.39.2