Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add weak attribute to led_set #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add weak attribute to led_set #4

wants to merge 1 commit into from

Conversation

p3lim
Copy link
Contributor

@p3lim p3lim commented Feb 14, 2016

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

@tmk
Copy link
Owner

tmk commented Apr 1, 2016

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
Copy link
Contributor Author

p3lim commented Apr 2, 2016

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

@tmk
Copy link
Owner

tmk commented Apr 2, 2016

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
Copy link
Owner

tmk commented Apr 2, 2016

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants