tmk / tmk_core

TMK keyboard library
161 stars 59 forks source link

Add weak attribute to led_set #4

Open p3lim opened 8 years ago

p3lim commented 8 years ago

This makes LEDs optional, lots of boards don't have any.

tmk commented 8 years ago

It makes sense. I like to change LED api it self later but this would be useful until then.

Only adding the attribute 'weak' is enough for this pupose? don't you need default implementation somewhere?

p3lim commented 8 years ago

It's enough, in my tests it built without errors/warnings.

tmk commented 8 years ago

I just tested, it can compiled as you said but it stops working or run away when pressing Capslock. From my understanding with weak attribute symbols are compiled as 0(EDIT: when no implementation is given). So if you don't have any implementation of led_set() the weak reference 0 is used when it is called. In the end it jumps into address 0 and go away.

tmk commented 8 years ago

I guess we have two solutions;

  1. give default implementation In common/led.c:
__attribute__((weak))        
void led_set(uint8_t usb_led)
{
}

or use alias like: __attribute__((weak, alias("_led_set"))) ?

  1. check before calling it We have to check the reference everywhere led_set() is called.
diff --git a/tmk_core/common/keyboard.c b/tmk_core/common/keyboard.c
index eb7b096..bfca3e7 100644
--- a/tmk_core/common/keyboard.c
+++ b/tmk_core/common/keyboard.c
@@ -173,5 +173,5 @@ MATRIX_LOOP_END:
 void keyboard_set_leds(uint8_t leds)
 {
     if (debug_keyboard) { debug("keyboard_set_led: "); debug_hex8(leds); debug("\n"); }
-    led_set(leds);
+    if (led_set) led_set(leds);
 }

http://www.cs.virginia.edu/~wh5a/blog/The%20weak%20attribute%20of%20gcc.html