kinx-project / kint

kinT keyboard controller (Kinesis controller replacement)
Other
325 stars 41 forks source link

layer_state_set_user not being called? #24

Open deBaer opened 3 years ago

deBaer commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 👍