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

layer_state_set_user not being called? #24

Open
deBaer opened this issue Feb 3, 2021 · 5 comments
Open

layer_state_set_user not being called? #24

deBaer opened this issue Feb 3, 2021 · 5 comments

Comments

@deBaer
Copy link
Contributor

deBaer commented Feb 3, 2021

Hi, I want to emulate the original firmware's keypad behaviour and light the keypad LED when the NUMPAD layer is active.

Sadly, the code below in keymap.c doesn't work:

layer_state_t layer_state_set_user(layer_state_t state) {
  printf("layer_state_set_user running\n");
  switch (get_highest_layer(state)) {
    case NUMPAD:
      writePin(C3, 0);
      break;
    default:
      writePin(C3, 1);
      break;
  }
  return state;
}

The LED doesn't change on layer change, nor is there output in the console.

I even tried putting this into kint2pp.c, as it seems to be missing, but it also changes nothing:

layer_state_t layer_state_set_kb(layer_state_t state) {
    layer_state_set_user(state);
    return state;
}

Any idea what might be wrong here?

@deBaer
Copy link
Contributor Author

deBaer commented Feb 4, 2021

Oh, right, I also deactivated the LED being overwritten:

bool led_update_user(led_t led_state) {
  writePin(C4, !led_state.scroll_lock);
  writePin(C5, !led_state.num_lock);
  writePin(C1, !led_state.caps_lock);
  return false;
}

@stapelberg
Copy link
Contributor

This might be better answered in the QMK repository.

I agree that it seems like something is missing, but I don’t have the time right now to dig through how this is supposed to work in QMK.

@davmarksman
Copy link

davmarksman commented Mar 25, 2021

Hey, struggled on it too but got it working. You are right you have to deactivate the LED being overwritten:

This is what I did to get the leds working based on which layer I was on:

layer_state_t layer_state_set_user(layer_state_t state) {
    // interestingly had to set the one I wanted on to 0 and the rest to 1 (for off)
    switch (get_highest_layer(state)) {
      case SYM:
          // For symbol layer set compose led on
          writePin(LED_COMPOSE_PIN, 0);     
          writePin(LED_NUM_LOCK_PIN, 1);     
          writePin(LED_SCROLL_LOCK_PIN, 1);
          break;
      case L1:
          // For Layer 1 set numlock led on
          writePin(LED_COMPOSE_PIN, 1);     
          writePin(LED_NUM_LOCK_PIN, 0);     
          writePin(LED_SCROLL_LOCK_PIN, 1);
          break;      
      case NAV:
          // For Layer Nav set scroll lock led on
          writePin(LED_COMPOSE_PIN, 1);     
          writePin(LED_NUM_LOCK_PIN, 1);     
          writePin(LED_SCROLL_LOCK_PIN, 0);
          break; 
      default:
          // set off
          writePin(LED_COMPOSE_PIN, 1);
          writePin(LED_SCROLL_LOCK_PIN, 1);
          writePin(LED_NUM_LOCK_PIN, 1);    
    }
  return state;
}
       
bool led_update_user(led_t led_state) {
    // you need to implement this method and return false otherwise it will overwrite what happened in layer_state_set_user

    // we want caps lock to keep working as is:
    writePin(LED_CAPS_LOCK_PIN, !led_state.caps_lock);

    return false;
}

@stapelberg
Copy link
Contributor

stapelberg commented Apr 1, 2021

https://github.com/qmk/qmk_firmware/blob/ac0ba832c73dd2130f66ba1f3781ccb16c150a89/tmk_core/common/action_layer.c#L99 is the dispatch logic using weak symbols: if layer_state_set_kb is not overriden, it calls layer_state_set_user.

LEDs seem to be updated after processing all actions in the keyboard task: https://github.com/qmk/qmk_firmware/blob/ac0ba832c73dd2130f66ba1f3781ccb16c150a89/tmk_core/common/keyboard.c#L500

So it should be possible to introduce a state variable that you modify from your layer_state_set_user and then handle LEDs accordingly in your led_update_user. This seems a little cleaner to me than modifying LEDs from multiple places :)

@davmarksman
Copy link

davmarksman commented Apr 1, 2021

Thanks for the feedback.

Experiment with that and it doesn't work for one shot OSL(kc) (I'm guessing led_update_user doesn't get called once the OSL gets cleared). It was fine for toggle layer TO(kc) and tap and hold layer LT(layer, kc)

Also tried layer_state_is from the doc's but same issue.

// works with TO(kc) and LT(layer,kc) but not OSL(kc)
bool led_update_user(led_t led_state) {
    writePin(LED_COMPOSE_PIN, !layer_state_is(GAME)); // set compose led when on game layer
    writePin(LED_SCROLL_LOCK_PIN, !layer_state_is(NAV)); // set scroll lock led when on nav layer
    writePin(LED_NUM_LOCK_PIN, !layer_state_is(L1));   

    // we want caps lock to keep working as is:
    writePin(LED_CAPS_LOCK_PIN, !led_state.caps_lock);

    return false;
}

Great work on the controller btw. Loving using it 👍

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

No branches or pull requests

3 participants