USB_LED_CAPS_LOCK not working with LED anymore [Ergodox EZ]

patrickwelker commented 6 years ago

Maybe @ezuk can share a little light on this. I could imagine there might be some terminology changes that I'm not aware of. I searched the open/closed issues, but haven't found a thing.

@jackhumbert commented in 2016 confirming this works for all things OLKB. I'm 100% sure this ought to work for the EZ, too, since I used the code below for over about a year and it worked… but not anymore:

// Runs constantly in the background, in a loop.
void matrix_scan_user(void) {
  ergodox_board_led_off();
  ergodox_right_led_1_off();
  ergodox_right_led_2_off();
  ergodox_right_led_3_off();
  // This used to work 2017-09-06, but not anymore. Can't find a solution.
  if (host_keyboard_leds() & (1<<USB_LED_CAPS_LOCK)) {
    ergodox_right_led_1_on ();
  }
};

My board and led's work. Just tested that it's all red when going the reverse way:

if (host_keyboard_leds() & (1<<USB_LED_CAPS_LOCK)) {
  ergodox_right_led_3_on ();
} else {
  ergodox_right_led_1_on ();
}

drashna commented 6 years ago

Well, by default, they never did this (as far as I'm aware). But if you'd like to make it so the LEDs indicate the lock status, you can.

You want to use this: https://docs.qmk.fm/custom_quantum_functions.html

But specifically:

void matrix_init_user (void) {
    ergodox_board_led_off();
    ergodox_right_led_1_off();
    ergodox_right_led_2_off();
    ergodox_right_led_3_off();
}

void led_set_user(uint8_t usb_led) {
    if (usb_led & (1<<USB_LED_NUM_LOCK)) {
        ergodox_right_led_1_on();
    } else {
        ergodox_right_led_1_off();
    }
    if (usb_led & (1<<USB_LED_CAPS_LOCK)) {
        ergodox_right_led_2_on();
    } else {
        ergodox_right_led_2_off();
    }
    if (usb_led & (1<<USB_LED_SCROLL_LOCK)) {
        ergodox_right_led_3_on();
    } else {
        ergodox_right_led_3_off();
    }
}

// Runs whenever there is a layer state change.
uint32_t layer_state_set_user(uint32_t state) {
  uint8_t layer = biton32(state);
  switch (layer) {
      case 0:
        #ifdef RGBLIGHT_COLOR_LAYER_0
          rgblight_setrgb(RGBLIGHT_COLOR_LAYER_0);
        #else
        #ifdef RGBLIGHT_ENABLE
          rgblight_init();
        #endif
        #endif
        break;
      case 1:
        #ifdef RGBLIGHT_COLOR_LAYER_1
          rgblight_setrgb(RGBLIGHT_COLOR_LAYER_1);
        #endif
        break;
      case 2:
        #ifdef RGBLIGHT_COLOR_LAYER_2
          rgblight_setrgb(RGBLIGHT_COLOR_LAYER_2);
        #endif
        break;
      case 3:
        #ifdef RGBLIGHT_COLOR_LAYER_3
          rgblight_setrgb(RGBLIGHT_COLOR_LAYER_3);
        #endif
        break;
      case 4:
        #ifdef RGBLIGHT_COLOR_LAYER_4
          rgblight_setrgb(RGBLIGHT_COLOR_LAYER_4);
        #endif
        break;
      case 5:
        #ifdef RGBLIGHT_COLOR_LAYER_5
          rgblight_setrgb(RGBLIGHT_COLOR_LAYER_5);
        #endif
        break;
      case 6:
        #ifdef RGBLIGHT_COLOR_LAYER_6
          rgblight_setrgb(RGBLIGHT_COLOR_LAYER_6);
        #endif
        break;
      case 7:
        #ifdef RGBLIGHT_COLOR_LAYER_7
          rgblight_setrgb(RGBLIGHT_COLOR_LAYER_6);
        #endif
        break;
      default:
        break;
    }

  return state;
};

Replace the layer_state_set_user function with the above, otherwise it will change the LEDs.

Can confirm that this works. Even when caps are set elsewhere.

patrickwelker commented 6 years ago

Mh. I commented out my 'capslock if function' and tried adding your led_set_user at the bottom of my keymap.c (and since I have an older EZ without RGB LEDs ditched the layer_state_set_user), but it didn't work.

My best guess is that something in my configuration might interfere, although I can't put my finger on it. I copied and adapted a lot of Algernon's keymap. I try to understand everything, but as someone not speaking C it's really hard sometimes… even simple if-else statements can become a major obstacle if you don't know the basics.

drashna commented 6 years ago

Are you still having issues with this, or were you able to resolve it?

And I can confirm that caps lock works, actually... as I'm setting it in my keymap.

patrickwelker commented 6 years ago

@drashna Yup. I'll check it this weekend using your keymap or a simple one. If it works there I know for sure it's something in my keymap and I'll have to ~~debug~~ ignore the absence of the red light light.

drashna commented 6 years ago

hahaha, yeah, use a simple one.

Though my keymap does use the green LED for capslock (and shift and OSM'ed shift), I wouldn't call my simple, because I use the userspace stuff too.

patrickwelker commented 6 years ago

I setup a MWE to get rid of all the other potential culprits in my keymap. I tested the Caps Lock key with this minimal layout using @drashna's example ("TEST ONE" in the keymap) and with what I had before and what used to work ("TEST TWO" in the keymap).

Both tests where negative:

LED's don't work with Capslock
LED's work for (other) modifier keys
TEST ONE = used to work for me a few months ago, suddently stopped working one day (without me updating anything)
TEST TWO = works for Drashna

Possible Culprits

This might be an OS issue as it works for @drashna who is probably not running macOS.
Maybe it's a firmware issue and something changed since the EZ Shine got introduced. Could this be the case @jackhumbert?

Now that I finally did a test, I'm pretty clueless as to the source of this problem since I was sure it's because of my voluminous-hacked--together-firmware-without-knowing-C.

`test_capslock/keymap.c`

// Minimal working example to test the RGB lights for the Caps Lock key.
// Keyboard:  ErgoDox EZ [≠ ErgoDox EZ Shine]
// System: macOS 10.13.2
// Layout: Colemak-based keymap with KC_LOCKING_CAPS, KC_LNUM, KC_CAPS placed 'on the corners' to test them

#include QMK_KEYBOARD_H
#include "debug.h"
#include "action_layer.h"

#define COLEMAK 0 // default layer

const uint16_t PROGMEM keymaps[][MATRIX_ROWS][MATRIX_COLS] = {
/* LAYER 0: Basic layer
 *
 * ,--------------------------------------------------.           ,--------------------------------------------------.
 * |LOCKING-|   1  |   2  |   3  |   4  |   5  | CAPS |           |Clip- |   6  |   7  |   8  |   9  |   0  |   =    |
 * |   CAPS |   !  |   @  |   #  |   $  |   %  |      |           | Hist |   ^  |   &  |   *  |   (  |   )  |        |
 * |--------+------+------+------+------+-------------|           |------+------+------+------+------+------+--------|
 * | Esc    |   Q  |   W  |   F  |   P  |   G  |   [  |           |  ]   |   J  |   L  |   U  |   Y  | ;    |LaunchBa|
 * | Meh    |      |      |      |      |      | ~L1  |           | ~L1  |      |      |      |      |      |        |
 * |--------+------+------+------+------+------|      |           |      |------+------+------+------+------+--------|
 * | Tab    |   A  |   R  |   S  |   T  |   D  |------|           |------|   H  |   N  |   E  |   I  |   O  |        |
 * | Hyper  |      |      |      |      |      |      |           |      |      |      |      |      |      | '      |
 * |--------+------+------+------+------+------|      |           |      |------+------+------+------+------+--------|
 * | [OSM]  |   Z  |   X  |   C  |   V  |   B  |  `   |           |  \   |   K  |   M  |   ,  |   .  |   /= | [OSM]  |
 * | Shift  | Ctrl |      |      |      |      | Shift|           |Shift |      |      |      |      |      | Shift  |
 * `--------+------+------+------+------+-------------'           `-------------+------+------+------+------+--------'
 *   |NUMLOK| Undo | Cut  | Copy | Paste|                                       | LEFT | DOWN |  UP  | RIGHT|LCAP  |
 *   `----------------------------------'                                       `----------------------------------'
 *                                        ,-------------.       ,-------------.
 *                                        |  -   |      |       | -    |[OSM]|
 *                                        |  opt |  cmd |       | cmd  | opt |
 *                                 ,------|------|------|       |------+--------+------.
 *                                 |      |      |      |       | _    |        |      |
 *                                 |      |      | Ctrl |       | Ctrl |        |      |
 *                                 |  Back|  Del |------|       |------| Enter  |Space |
 *                                 | space|      |      |       | NUM- |        |      |
 *                                 |      |      | Esc  |       | LOCK |        |      |
 *                                 `--------------------'       `----------------------'
 */[COLEMAK] = KEYMAP(
        // left hand
        KC_LOCKING_CAPS,   KC_1,        KC_2,    KC_3,   KC_4,     KC_5,           KC_CAPS,
        MEH_T(KC_ESC),     KC_Q,        KC_W,    KC_F,   KC_P,     KC_G,           KC_LBRC,
        ALL_T(KC_TAB),     KC_A,        KC_R,    KC_S,   KC_T,     KC_D,
        OSM(MOD_LSFT),     CTL_T(KC_Z), KC_X,    KC_C,   KC_V,     KC_B,           SFT_T(KC_GRV),
        LCAG(KC_F16),      LGUI(KC_Z),  LGUI(KC_X),LGUI(KC_C), LGUI(KC_V),
                                                                   ALT_T(KC_MINS), OSM(MOD_LGUI),
                                                                                   OSM(MOD_LCTL),
                                                          KC_BSPC, KC_DELT,        KC_ESC,
        // right hand
        LGUI(KC_GRV),      KC_6,        KC_7,    KC_8,   KC_9,     KC_0,           KC_EQUAL,
        KC_RBRC,           KC_J,        KC_L,    KC_U,   KC_Y,     KC_SCLN,        LGUI(KC_SPACE),
                           KC_H,        KC_N,    KC_E,   KC_I,     KC_O,           KC_QUOT,
        SFT_T(KC_BSLS),    KC_K,        KC_M,    KC_COMM,KC_DOT,   CTL_T(KC_SLSH), OSM(MOD_RSFT),
                           KC_LEFT,     KC_DOWN, KC_UP,  KC_RIGHT, KC_LCAP,
        GUI_T(KC_MINS),    OSM(MOD_LALT),
        OSM(MOD_LCTL),
        KC_LNUM,           KC_ENT,      KC_SPC
  ),
};

const uint16_t PROGMEM fn_actions[] = {
};

const macro_t *action_get_macro(keyrecord_t *record, uint8_t id, uint8_t opt)
{
  // MACRODOWN only works in this function
  switch(id) {
    case 0:
      if (record->event.pressed) {
        register_code(KC_RSFT);
      } else {
        unregister_code(KC_RSFT);
      }
      break;
  }
  return MACRO_NONE;
};

///////////////////////////////////////////////////////////////////////////////////////////////
// TEST ONE: used Drashna's setup which works for him
// https://github.com/qmk/qmk_firmware/issues/2471#issuecomment-370165543
///////////////////////////////////////////////////////////////////////////////////////////////

// Runs just one time when the keyboard initializes.
void matrix_init_user(void) {
    ergodox_board_led_off();
    ergodox_right_led_1_off();
    ergodox_right_led_2_off();
    ergodox_right_led_3_off();
}

// Runs constantly in the background, in a loop.
void led_set_user(uint8_t usb_led) {
    if (usb_led & (1<<USB_LED_NUM_LOCK)) {
        ergodox_right_led_1_on();
    } else {
        ergodox_right_led_1_off();
    }
    if (usb_led & (1<<USB_LED_CAPS_LOCK)) {
        ergodox_right_led_2_on();
    } else {
        ergodox_right_led_2_off();
    }
    if (usb_led & (1<<USB_LED_SCROLL_LOCK)) {
        ergodox_right_led_3_on();
    } else {
        ergodox_right_led_3_off();
    }
};

///////////////////////////////////////////////////////////////////////////////////////////////
// TEST TWO: used a slimmer version of my old setup which suddendly stopped working for no reason
///////////////////////////////////////////////////////////////////////////////////////////////

// // Runs constantly in the background, in a loop.
// void matrix_scan_user(void) {
//   ergodox_board_led_off();
//   ergodox_right_led_1_off();
//   ergodox_right_led_2_off();
//   ergodox_right_led_3_off();
//   // This used to work 2017-09-06, but not anymore. Can't find a solution.
//   if (host_keyboard_leds() & (1<<USB_LED_CAPS_LOCK)) {
//     ergodox_right_led_1_on ();
//   }
//   // Light up red when shift is pressed/active
//   // https://github.com/qmk/qmk_firmware/blob/23faf9ec8119e211b1da236e9c869c230456aa52/keyboards/ergodox/keymaps/algernon/keymap.c#L850-L878
//   if (keyboard_report->mods & MOD_BIT(KC_LSFT) ||
//     ((get_oneshot_mods() & MOD_BIT(KC_LSFT)) && !has_oneshot_mods_timed_out()) ||
//       (keyboard_report->mods & MOD_BIT(KC_RSFT) ||
//     ((get_oneshot_mods() & MOD_BIT(KC_RSFT)) && !has_oneshot_mods_timed_out()))) {
//     ergodox_right_led_1_set (LED_BRIGHTNESS_HI);
//     ergodox_right_led_1_on ();
//   }
// };

drashna commented 6 years ago

Well, I tested this on my system, and it works fine. So this may be an OS specific issue.

Specifically, it looks like the status is pulled by using host_keyboard_leds(). Which pulls this status from The HID driver.

Unfortunately, I don't have a macOS device to test on, so I can't verify if it's the reason why.

That said, I'm checking the status in my keymap now, as well. In part because of this issue. And I can verify that it does light up properly. https://github.com/qmk/qmk_firmware/blob/master/layouts/community/ergodox/drashna/keymap.c#L336-L365

So, as for why it's not working here ... is beyond me.

patrickwelker commented 6 years ago

So odd… tested your latest code by adding it as TEST THREE, but not a thing to see (this time even the modifiers don't make the LED's turn on).

Don't hesitate to throw other things to test at me or forward person XYZ. I might clone the repo again this weekend and restore the QMK version from 1.5-2 years ago to see if it magically will work again with TEST TWO. At least that is the only thing to do that I can think of now.

///////////////////////////////////////////////////////////////////////////////////////////////
// TEST THREE (Drashna - https://github.com/qmk/qmk_firmware/issues/2471#issuecomment-380123904)
///////////////////////////////////////////////////////////////////////////////////////////////

//define layer change stuff for underglow indicator
bool skip_leds = false;
//define modifiers
#define MODS_SHIFT_MASK  (MOD_BIT(KC_LSHIFT)|MOD_BIT(KC_RSHIFT))
#define MODS_CTRL_MASK  (MOD_BIT(KC_LCTL)|MOD_BIT(KC_RCTRL))
#define MODS_ALT_MASK  (MOD_BIT(KC_LALT)|MOD_BIT(KC_RALT))
#define MODS_GUI_MASK  (MOD_BIT(KC_LGUI)|MOD_BIT(KC_RGUI))

bool process_record_keymap(uint16_t keycode, keyrecord_t *record) {
  return true;
}

void matrix_init_keymap(void) { // Runs boot tasks for keyboard
};

void matrix_scan_keymap(void) {  // runs frequently to update info
  uint8_t modifiders = get_mods();
  uint8_t led_usb_state = host_keyboard_leds();
  uint8_t one_shot = get_oneshot_mods();

  if (!skip_leds) {
    ergodox_board_led_off();
    ergodox_right_led_1_off();
    ergodox_right_led_2_off();
    ergodox_right_led_3_off();

    // Since we're not using the LEDs here for layer indication anymore,
    // then lets use them for modifier indicators.  Shame we don't have 4...
    // Also, no "else", since we want to know each, independently.
    if (modifiders & MODS_SHIFT_MASK || led_usb_state & (1<<USB_LED_CAPS_LOCK) || one_shot & MODS_SHIFT_MASK) {
      ergodox_right_led_2_on();
      ergodox_right_led_2_set( 50 );
    }
    if (modifiders & MODS_CTRL_MASK || one_shot & MODS_CTRL_MASK) {
      ergodox_right_led_1_on();
      ergodox_right_led_1_set( 10 );
    }
    if (modifiders & MODS_ALT_MASK || one_shot & MODS_ALT_MASK) {
      ergodox_right_led_3_on();
      ergodox_right_led_3_set( 10 );
    }

  }
};

coderkun commented 6 years ago

I experience the same issue as @patrickwelker. However I noticed that the led_set_user method is working fine for me but that xset led named "Scroll Lock" (which I use in Gajim) works only under X server but not under Wayland.

Maybe this information helps you to find the culprit on your system?

patrickwelker commented 6 years ago

Can I have a look at your keymap @coderkun ? (Das wäre recht hilfreich.)

coderkun commented 6 years ago

My current keymap is coderkun_neo2/keymap.c though I tested it with a slighly modified keymap (I am currently updating some layers and try to fix the LED issue). However I am ably to fix it by simply replacing the matrix_scan_user() function with the mentioned led_set_user() function.

“Fix” here means that the LED(s) work(s) properly when activating caps/num/scroll lock on the keyboard itself but using xset led works for me only when used with X server, not Wayland.

coderkun commented 6 years ago

@patrickwelker, for my keymap I was able to fix the LEDs with coderkun/qmk_firmware@39738d2. Hope it helps …

patrickwelker commented 6 years ago

Thanks @coderkun. Tested it again, but it's not working for me. Maybe because my EZ is older or it's the (mac)OS version or whatever. Nifty unicode layer by the way.

At this point it comes down to guess work. I also tried a minimal working example to no avail. Currently I use a separate shifted layer as a workaround. 😢

h-youhei commented 5 years ago

I have same issue.

https://github.com/qmk/qmk_firmware/releases/tag/0.6.85 works for me.

OS: Arch Linux Ergodox EZ: bought in 2016

DidierLoiseau commented 5 years ago

Same issue for me here on Ubuntu 18.10 – I will test it on Windows 10 tomorrow.

I am currently trying to upgrade my Ergodox EZ keymap (bepo_csa in the community keymaps) and I just observed that the LEDs do not work as they used to.

Taking the led_set_user() code from above (and removing my matrix_scan_user()), I observe that the 1st and 3rd LEDs (NumLk and ScrollLk) are always on, and the 2nd LED (CapsLk) is always off, no matter the actual OS state.

In the past, calling host_keyboard_leds() always worked, and could thus be used in matrix_scan_user() to update the LEDs.

I can also confirm that my old layout linked above still works as intended (i.e. 3rd LED reflects CapsLk state) as I still have its hex file. It's thus not something that changed in the OS.

Edit: note that with my old code, setting the 3rd LED in matrix_scan_user(), I can occasionally see it blink for maybe 100ms if I type CapsLk many times.

DidierLoiseau commented 5 years ago

Strangely enough, it works fine on Windows 10 with testing host_keyboard_leds() in matrix_scan_user(). This thus confirms the issue is not keymap-specific.

I think the next step would be to git bisect in order to find the commit that introduced the regression, but I don't think I will have time for this today.

DidierLoiseau commented 5 years ago

I had some time to do the git bisect, so here is the first bad commit for me: d6b7ca04f22e3f1e0f9ce4074f3902fddba338ad, the fix for #4471 implemented by @fauxpark and merged by @drashna. Reverting that commit on the latest master fixes the problem for me on Ubuntu 18.10.

This is a quite recent commit (2018-12-05 – release 0.6.198) so it is not the same issue as @patrickwelker, but it could be the same as @h-youhei who posted his comment 2 days after that commit. Maybe @patrickwelker's issue was actually the same as #4471, and is thus fixed now with that change?

Should I open a separate issue maybe to fix this new regression?

fauxpark commented 5 years ago

I can confirm that my fix solved the nonworking caps lock LED for me on my Satan and macOS (it was not limited to just the DZ60; it affected all boards running LUFA and was related to a recent improvement to the HID endpoint management).

I can't see how it could possibly have broken the EZ considering it too has a 32U4 and thus runs LUFA.

fauxpark commented 5 years ago

Well, I spun up a Ubuntu 18.10 VM and gave it my keyboard. On current QMK master, with KEYBOARD_SHARED_EP commented out in my rules.mk, the caps lock LED continues to work on Windows and macOS, as it should, but doesn't work on Ubuntu.

After some fiddling about getting Wireshark capturing USB packets, I saw that Ubuntu sends two SET_REPORT packets: one with 0x02 as expected (the caps lock LED is the "2" bit), and one with 0x0502. I have no idea where it's getting this extra byte from.

~I assume the two-packet thing is to somehow make sure the LEDs work regardless of whether the keyboard is expecting a report ID back.~ Evidently QMK is getting confused when it receives two SET_REPORT packets, one with the caps lock bit set, and the next with it unset (and this probably explains the occasional flickering that was mentioned).

EDIT: Ah. I know what the 0x05 is about. NKRO_ENABLE = yes in the base Satan rules.mk. Guess what the report ID for the NKRO endpoint is?

patrickwelker commented 5 years ago

Thanks for the rules.mk workaround @fauxpark , as soon as I added KEYBOARD_SHARED_EP = yes the caps lock LED lid up correctly on macOS.

Unfortunately this causes enough other problems with the keyboard not being recognized properly by some applications (c.f. Karabiner Elements). So I'm back using my shifted layer with a red LED as a replacement for now.

I'm on QMK Firmware 0.6.223. Without the above addition this classic way of toggling the caps lock LED does not work (for me):

void matrix_scan_user(void) {

    ergodox_board_led_off();
    ergodox_right_led_1_off();
    ergodox_right_led_2_off();
    ergodox_right_led_3_off();

    // Caps lock (red)
    if (host_keyboard_leds() & (1<<USB_LED_CAPS_LOCK)) {
        ergodox_right_led_3_on();
    }
};

Leszek111 commented 5 years ago

@patrickwelker thanks for mentioning Karabiner Elements, that's the culprit. With it the built-in keyboard caps-lock LED also doesn't work. If you quit Karabiner, both LED's work well. I'm using the function led_set_user() in my Ergodox layout defninition.

I'm on macOS Mojave, Karabiner Elements 12.1.0

fauxpark commented 5 years ago

@Leszek111 @patrickwelker apparently that is fixed in one of the recent betas: https://github.com/tekezo/Karabiner-Elements/issues/1502#issuecomment-432602231

DidierLoiseau commented 5 years ago

Ok so I added some debug in lufa.c to understand better what @fauxpark observed, like:

debug("received led_stats (before #3951): "); debug_hex16(keyboard_led_stats); debug("\n");

Here is what I get (added comments for clarity) before #3951, on commit 46cf8cc9:

# enabled caps lock
received led_stats (before #3951): 0003
received led_stats (before #3951): 0003
# disabled caps lock
received led_stats (before #3951): 0001
received led_stats (before #3951): 0001
# disabled num lock
received led_stats (before #3951): 0000
received led_stats (before #3951): 0000
# enabled num lock
received led_stats (before #3951): 0001
received led_stats (before #3951): 0001

So every report is duplicated, but correct. If I disable NKRO, they are not duplicated anymore.

Then I checked out 39bd760 which implemented #3951. With NKRO:

# disabled everything
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 00
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 05
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0000
# enable caps lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 02
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 05
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0002
# disable caps lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 00
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 05
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0000
# enable num lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 01
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0081
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 05
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0001
# enable caps lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 03
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 05
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0003
# disable caps lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 01
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0081
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 05
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0001
# disable num lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 00
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 05
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0000
# enable scroll lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 04
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 05
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0004
# enable caps lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 06
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 05
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0006
# disable caps lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 04
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 05
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0004
# disable scroll lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 00
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 05
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0000
# enable num lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 01
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0081
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 05
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0001
# enable scroll lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 05
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0080
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 05
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0005
# disable scroll lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 01
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0081
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 05
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0001

(note that I have to activate scroll lock on Ubuntu every time the keyboard reconnects to test this) So there is already something strange going on with the num lock and the 008x lines. It looks like we receive the led_stats twice, but the first time the led_stats are read as report_id. So when the value is Num Lock (01), it matches REPORT_ID_KEYBOARD and the next Endpoint_Read_8() returns 81 or 80.

So only the values for NKRO are read properly. If I disable it, only the 008x are still read, so the led state is not reflected properly anymore:

# initial state: nothing enabled
# enable num lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 01
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0081
# enable scroll lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 05
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0080
# disable scroll lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 01
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0081
# enable caps lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 03
# disable caps lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 01
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0081
# enable scroll lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 05
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0080
# enable caps lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 07
# disable scroll lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 03
# disable caps lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 01
received led_stats (#3951, with defined(SHARED_EP_ENABLE)): 0081
# disable num lock
received report_id (#3951, with defined(SHARED_EP_ENABLE)): 00

Concerning led status: num lock led is set for led_stats ending in 1 – as expected but it does not match num lock state.

To unset SHARED_EP_ENABLE, I put in my rules.mk the following:

MOUSEKEY_ENABLE  = no # Mouse keys(+4700)
EXTRAKEY_ENABLE  = no # Audio control and System control(+450)
NKRO_ENABLE      = no # USB Nkey Rollover - if this doesn't work, see here: https://github.com/tmk/tmk_keyboard/wiki/FAQ#nkro-doesnt-work
KEYBOARD_SHARED_EP = no

since all of these set SHARED_EP_ENABLE in common.mk. In that mode, the LEDs work perfectly fine, and I get the following output:

received led_stats (#3951, not defined(SHARED_EP_ENABLE)): 0003
received led_stats (#3951, not defined(SHARED_EP_ENABLE)): 0001
received led_stats (#3951, not defined(SHARED_EP_ENABLE)): 0005
received led_stats (#3951, not defined(SHARED_EP_ENABLE)): 0001
received led_stats (#3951, not defined(SHARED_EP_ENABLE)): 0000
received led_stats (#3951, not defined(SHARED_EP_ENABLE)): 0001

Nothing special – this is basically the same output as before #3951 without NKRO.

Now, with the latest master (9c2d7761), NKRO enabled:

# enable caps lock
received led_stats (master, no KEYBOARD_SHARED_EP): 0002
received led_stats (master, no KEYBOARD_SHARED_EP): 0005
# disable caps lock
received led_stats (master, no KEYBOARD_SHARED_EP): 0000
received led_stats (master, no KEYBOARD_SHARED_EP): 0005
# enable num lock
received led_stats (master, no KEYBOARD_SHARED_EP): 0001
received led_stats (master, no KEYBOARD_SHARED_EP): 0005
# enable caps lock
received led_stats (master, no KEYBOARD_SHARED_EP): 0003
received led_stats (master, no KEYBOARD_SHARED_EP): 0005
# disable caps lock
received led_stats (master, no KEYBOARD_SHARED_EP): 0001
received led_stats (master, no KEYBOARD_SHARED_EP): 0005
# disable num lock
received led_stats (master, no KEYBOARD_SHARED_EP): 0000
received led_stats (master, no KEYBOARD_SHARED_EP): 0005
…

Basically the NKRO report id (05) appears to be interpreted as led stats overriding the previously received value, and I get a nice fixed num lock + scrol lock status.

Without NKRO, everything works fine:

received led_stats (master, no KEYBOARD_SHARED_EP): 0000
received led_stats (master, no KEYBOARD_SHARED_EP): 0001
received led_stats (master, no KEYBOARD_SHARED_EP): 0003
received led_stats (master, no KEYBOARD_SHARED_EP): 0001
received led_stats (master, no KEYBOARD_SHARED_EP): 0005
received led_stats (master, no KEYBOARD_SHARED_EP): 0001

With KEYBOARD_SHARED_EP=yes (with or without NKRO) as well:

received report_id (master, with KEYBOARD_SHARED_EP): 01
received led_stats (master, with KEYBOARD_SHARED_EP): 0001
received report_id (master, with KEYBOARD_SHARED_EP): 01
received led_stats (master, with KEYBOARD_SHARED_EP): 0003
received report_id (master, with KEYBOARD_SHARED_EP): 01
received led_stats (master, with KEYBOARD_SHARED_EP): 0007
received report_id (master, with KEYBOARD_SHARED_EP): 01
received led_stats (master, with KEYBOARD_SHARED_EP): 0005
received report_id (master, with KEYBOARD_SHARED_EP): 01
received led_stats (master, with KEYBOARD_SHARED_EP): 0004
received report_id (master, with KEYBOARD_SHARED_EP): 01
received led_stats (master, with KEYBOARD_SHARED_EP): 0000

I will have to stop here, I hope this helps already a bit :slightly_smiling_face:

fauxpark commented 5 years ago

I'm thinking the 6KRO descriptor should just always have a report ID. That way we don't have to worry about how many bytes we're supposed to be reading.

patrickwelker commented 5 years ago

Thanks for the pointer to the latest Karabiner-Elements#1502 update @Leszek111. This means I can discard my shifted layer and make some room.

I'll still keep my eyes peeled for more Karabiner issues, c. f. one can't use the teensy_loader_cli with Karabiner-Elements running (which supposedly is the only way on Mojave to flash the board since GUI app is 32-bit).

qmk / qmk_firmware

USB_LED_CAPS_LOCK not working with LED anymore [Ergodox EZ] #2471

Possible Culprits

`test_capslock/keymap.c`