leds: Fix the API

The leds API did not work in some cases. E.g. with the following sequence:
  leds_off(LEDS_ALL);
  leds_toggle(LEDS_GREEN);
  leds_off(LEDS_ALL);
the green LED was remaining on after the last call.

This was caused by the toggle feature made synonymous with the invert feature,
although it is unrelated. leds_toggle() is indeed supposed to toggle an LED,
while leds_invert() is supposed to change the active level of an LED. However,
all users of leds_invert() actually meant leds_toggle(), and the invert feature
does not make sense in this module because it is not handy due to successive
calls to leds_invert() changing the intended behavior, and hardware active
levels should be managed in leds_arch_set() (e.g. by XORing the passed value
with a hardware-specific constant before setting the output levels of the pins).

Consequently, this change:
 - removes the leds_invert() function,
 - makes leds_toggle() behave as expected relatively to leds_off() / leds_on(),
 - sanitizes the code in the leds module.

Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com>

core/dev/leds.c

@@ -34,54 +34,58 @@
 #include "sys/clock.h"
 #include "sys/energest.h"
 
-static unsigned char leds, invert;
+static unsigned char leds;
 /*---------------------------------------------------------------------------*/
 static void
-show_leds(unsigned char changed)
+show_leds(unsigned char new_leds)
 {
+  unsigned char changed;
+  changed = leds ^ new_leds;
+  leds = new_leds;
+
   if(changed & LEDS_GREEN) {
     /* Green did change */
-    if((invert ^ leds) & LEDS_GREEN) {
+    if(leds & LEDS_GREEN) {
       ENERGEST_ON(ENERGEST_TYPE_LED_GREEN);
     } else {
       ENERGEST_OFF(ENERGEST_TYPE_LED_GREEN);
     }
   }
   if(changed & LEDS_YELLOW) {
-    if((invert ^ leds) & LEDS_YELLOW) {
+    if(leds & LEDS_YELLOW) {
       ENERGEST_ON(ENERGEST_TYPE_LED_YELLOW);
     } else {
       ENERGEST_OFF(ENERGEST_TYPE_LED_YELLOW);
     }
   }
   if(changed & LEDS_RED) {
-    if((invert ^ leds) & LEDS_RED) {
+    if(leds & LEDS_RED) {
       ENERGEST_ON(ENERGEST_TYPE_LED_RED);
     } else {
       ENERGEST_OFF(ENERGEST_TYPE_LED_RED);
     }
   }
-  leds_arch_set(leds ^ invert);
+  leds_arch_set(leds);
 }
 /*---------------------------------------------------------------------------*/
 void
 leds_init(void)
 {
   leds_arch_init();
-  leds = invert = 0;
+  leds = 0;
 }
 /*---------------------------------------------------------------------------*/
 void
 leds_blink(void)
 {
-  /* Blink all leds. */
-  unsigned char inv;
-  inv = ~(leds ^ invert);
-  leds_invert(inv);
+  /* Blink all leds that were initially off. */
+  unsigned char blink;
+  blink = ~leds;
+  leds_toggle(blink);
 
   clock_delay(400);
 
-  leds_invert(inv);
+  leds_toggle(blink);
 }
 /*---------------------------------------------------------------------------*/
 unsigned char
@@ -92,31 +96,18 @@ leds_get(void) {
 void
 leds_on(unsigned char ledv)
 {
-  unsigned char changed;
-  changed = (~leds) & ledv;
-  leds |= ledv;
-  show_leds(changed);
+  show_leds(leds | ledv);
 }
 /*---------------------------------------------------------------------------*/
 void
 leds_off(unsigned char ledv)
 {
-  unsigned char changed;
-  changed = leds & ledv;
-  leds &= ~ledv;
-  show_leds(changed);
+  show_leds(leds & ~ledv);
 }
 /*---------------------------------------------------------------------------*/
 void
 leds_toggle(unsigned char ledv)
 {
-  leds_invert(ledv);
-}
-/*---------------------------------------------------------------------------*/
-/*   invert the invert register using the leds parameter */
-void
-leds_invert(unsigned char ledv) {
-  invert = invert ^ ledv;
-  show_leds(ledv);
+  show_leds(leds ^ ledv);
 }
 /*---------------------------------------------------------------------------*/

core/dev/leds.h

@@ -77,13 +77,12 @@ void leds_blink(void);
 #endif /* LEDS_CONF_ALL */
 
 /**
- * Returns the current status of all leds (respects invert)
+ * Returns the current status of all leds
  */
 unsigned char leds_get(void);
 void leds_on(unsigned char leds);
 void leds_off(unsigned char leds);
 void leds_toggle(unsigned char leds);
-void leds_invert(unsigned char leds);
 
 /**
  * Leds implementation

cpu/avr/minileds.c

@@ -58,8 +58,4 @@ leds_off(unsigned char leds)
 void
 leds_toggle(unsigned char leds)
 {
-  /*
-   * Synonym: void leds_invert(unsigned char leds);
-   */
-  asm(".global leds_invert\nleds_invert:\n");
 }

cpu/msp430/minileds.c

@@ -74,10 +74,5 @@ leds_off(unsigned char leds)
 void
 leds_toggle(unsigned char leds)
 {
-  /*
-   * Synonym: void leds_invert(unsigned char leds);
-   */
-  asm(".global leds_invert\nleds_invert:\n");
-
   LEDS_PxOUT ^= l2p[leds & LEDS_ALL];
 }

tools/sky/uip6-bridge/uip6-bridge-tap.c

@@ -64,7 +64,7 @@ tcpip_output(const uip_lladdr_t *a)
     /*    printf("pppp o %u tx %u rx %u\n", UIP_IP_BUF->proto,
 	   packetbuf_attr(PACKETBUF_ATTR_TRANSMIT_TIME),
 	   packetbuf_attr(PACKETBUF_ATTR_LISTEN_TIME));*/
-    leds_invert(LEDS_GREEN);
+    leds_toggle(LEDS_GREEN);
   }
   return 0;
 }
@@ -94,7 +94,7 @@ tcpip_input(void)
 	     packetbuf_attr(PACKETBUF_ATTR_TRANSMIT_TIME),
 	     packetbuf_attr(PACKETBUF_ATTR_LISTEN_TIME));*/
       slip_write(uip_buf, uip_len);
-      leds_invert(LEDS_RED);
+      leds_toggle(LEDS_RED);
       uip_len = 0;
     }
   }
@@ -112,7 +112,7 @@ slip_tcpip_input(void)
 static void
 slip_activity(void)
 {
-  leds_invert(LEDS_BLUE);
+  leds_toggle(LEDS_BLUE);
 }
 /*---------------------------------------------------------------------------*/
 PROCESS_THREAD(uip6_bridge, ev, data)

tools/stm32w/uip6_bridge/sicslow_ethernet.c

@@ -968,7 +968,7 @@ void mac_802154raw(const struct radio_driver *radio)
 #endif
 
   slip_write(uip_buf, len);
-  leds_invert(LEDS_RED);
+  leds_toggle(LEDS_RED);
 
   //rndis_send(raw_buf, sendlen, 1);
   //rndis_stat.rxok++;