cub-uanic / tmk_keyboard

keyboard controller firmware for Atmel AVR USB family
158 stars 157 forks source link

Modernise the LED interface. #37

Open squarefrog opened 8 years ago

squarefrog commented 8 years ago

There have been some massive improvements to tmk concerning hooks.

This makes it trivial to setup your own LED scheme directly from within your keymap. For example, I currently only use 4 layers, and rarely use caps, num or scroll lock. As a result I have the following in my keymap:

void show_layer_led(uint32_t layer)
{
    ergodox_led_all_off();

    switch (layer) {
        case 1 << 1:
            ergodox_right_led_1_on();
            break;
        case 1 << 2:
            ergodox_right_led_2_on();
            break;
        case 1 << 3:
            ergodox_right_led_3_on();
            break;
        case 1 << 4:
            ergodox_right_led_2_on();
            ergodox_right_led_3_on();
            break;
        default:
            break;
    }
}

void hook_layer_change(uint32_t layer_state)
{
    switch (layer_state) {
        case 1:
            show_layer_led(default_layer_state);
            break;
        default:
            show_layer_led(layer_state);
            break;
    }
}

void hook_default_layer_change(uint32_t layer_state)
{
    show_layer_led(default_layer_state);
}

Taking full advantage of this would help clean up matrix.c. I don't feel there is anything wrong with the current defaults in led.c, so these could be sane defaults in case users do not want to customise this.

marknsikora commented 8 years ago

Been waiting for this for quite some time. Stragenly the issue I'm following to track the hooks changes hasn't been updated. I'll be working on this tomorrow, it's something I already have coded up.

squarefrog commented 8 years ago

So far I'm really very impressed with it.

marknsikora commented 8 years ago

I've spent my afternoon playing with the new hook code and I noticed one problem. It's currently conflicting with the existing led code. On first boot the led is set by our old code. But if you change the layer then it uses the new code. And if you toggle numlock or capslock it goes back to the old code. So basically whatever state change was most recent decides which code is active.

I'm also don't think you need hooks for both layer commands. Based on what I read in the docs the layer_state will always have the default_layer_state bits flipped on

squarefrog commented 8 years ago

That would make sense. I can try toggling caps lock tomorrow. It would be easy for me to over look it as I don't use any of the lock keys.

I did find it necessary to set both although I could probably do something smarter by querying both values before my switch.