Windows - Mute Volume Key

So I just received one of my Model 01s today (hurray!) and naturally started working with it a bit. One thing I noticed is that the Volume Mute key (default Fn + N) does not seem to work on my desktop. I’m running Windows 10.

I did a fair bit of research, but some of the USB HID stuff is over my head. I figured out (hopefully correctly) that I need to send the “Consumer_Mute” key code (0xE2) instead of the default “Key_Mute” key code for Windows to be happy with it.

To make sure I was doing the remapping properly and to verify the hardware was functioning properly, I remapped Fn+N to do some other stuff, and that all worked correctly… so I think it’s just a matter of putting the right key code in there.

Anyone have any great advice? :slight_smile: Appreciate whatever you can come up with.

I have to add that the keyboard is just fantastic - this isn’t even that big of a deal, but I figured I would bring it up since others may have the same question.

Edit - As additional information, I have a generic Logitech keyboard that I’ve been using for a long time that is able to send the mute command successfully. The Model 01 will send Volume Up/Down with no problem… just getting that pesky mute to work…!

1 Like

Its a known bug. I tried key_mute and consumer_mute and neither work.

https://github.com/keyboardio/Kaleidoscope/issues/153

1 Like

Ah, okay. Thanks a lot!

If someone can figure out what the correct keycode for Mute, as generated by that Logitech keyboard, is, we can probably get this sorted out.

1 Like

When I used an online key code detector, it detected the mute key as 173,
volume down as 174, and volume up as 175. Not sure if that’s helpful…

I’m a little adventurous, so I decided to download this software that said it could monitor USB traffic. It seems to work pretty well, though I really have no idea what I’m looking at (well, a little idea, but not much). It’s pretty neat, but over my head as far as the technical stuff that is going on.

So, according to this software, the Logitech keyboard I’m using appears as two separate devices; one is a “Logitech USB Input Device” and one is a “USB Input Device.” The Logitech USB Input Device only sends packets for “normal” keys, not any of the special extra ones like volume up/down, etc. The USB Input Device sends packets for the extra stuff.

Here’s the Logitech packet for “mute”

Logitech packet for “volume down”

Logitech packet for “volume up”

Now the Model 01… sending “mute” does not register at all.

Model 01 packet for “volume down”

Model 01 packet for “volume up”

Now, is this helpful? Maybe, maybe not… but I feel like I’m learning something though, so I guess that’s a positive outcome regardless. :slight_smile:

3 Likes

Okay… so I was able to solve this one. Although it’s probably more of a “hey, this works, but it isn’t actually the right way to fix it…”

The fact that the Model 01 wasn’t even sending a packet that my computer could understand made me wonder if it had something to do with the fact that HID_CONSUMER_MUTE is HID Type OOC (On Off Controls), vs. the fact that HID_CONSUMER_VOLUME_INCREMENT and DECREMENT are HID Type RTCs. So I went into the key_defs_consumerctl.h file and changed the definition of Consumer_Mute to the following:

#define Consumer_Mute (Key) { HID_CONSUMER_MUTE, KEY_FLAGS | SYNTHETIC|IS_CONSUMER | HID_TYPE_RTC }

After reflashing, it worked perfectly. Why does it work? No idea…but at least the keyboard mutes properly now. Unfortunately, I feel like there is probably an underlying issue with handling HID Type OOCs (On Off Controls) that I’m just avoiding by doing this hack.

1 Like

I believe I’ve traced it to the root of the error…some snippets of code below for convenience.

key_defs.h:
#define IS_INTERNAL B00001000
#define HID_TYPE_OOC B00001100

key_defs_consumerctl.h:
#define Consumer_Mute (Key) { HID_CONSUMER_MUTE, KEY_FLAGS | SYNTHETIC|IS_CONSUMER | HID_TYPE_OOC }

key_events.cpp:
static bool handleSyntheticKeyswitchEvent(Key mappedKey, uint8_t keyState) {
if (mappedKey.flags & RESERVED)
return false;

  if (!(mappedKey.flags & SYNTHETIC))
    return false;

  if (mappedKey.flags & IS_INTERNAL) {
    return false;
  } else if (mappedKey.flags & IS_CONSUMER) {
    if (keyIsPressed(keyState)) {
    } else if (keyWasPressed(keyState)) {
      kaleidoscope::hid::pressConsumerControl(mappedKey);
      kaleidoscope::hid::releaseConsumerControl(mappedKey);
    }
  } else if (mappedKey.flags & IS_SYSCTL) {
    if (keyIsPressed(keyState)) {
    } else if (keyWasPressed(keyState)) {
      kaleidoscope::hid::pressSystemControl(mappedKey);
      kaleidoscope::hid::releaseSystemControl(mappedKey);
    }
  } else if (mappedKey.flags & SWITCH_TO_KEYMAP) {
    // Should not happen, handled elsewhere.
  }

  return true;
}

Because the flag HID_TYPE_OOC is B00001100, it overlaps with the IS_INTERNAL flag. When the compare (mappedKey.flags & IS_INTERNAL) is done in handleSyntheticKeyswitchEvent(), it returns false before doing anything else because the bit is high in that position.

I verified this by commenting out that part of the if statement in key_events.cpp, and the mute button started working again (I changed the key_defs_consumerctl.h definition of Consumer_Mute back to having the HID_TYPE_OOC flag).

Hopefully that makes sense…and is actually correct.

P.S. I’m bad at putting code in the forums and making it look nice.

6 Likes

That’s a very nice catch, thank you!

We should probably explore if we can drop the INTERNAL flag some way…

Nice job. How did you figure it out?

Basically just combing through the code via brute force… a lot of ctrl+F. :slight_smile:

@algernon

Here’s 1 suggestion… it seems like IS_INTERNAL is only used in one key definition:

Kaleidoscope-LEDControl.h
#define Key_LEDEffectNext (Key) { 0, KEY_FLAGS | SYNTHETIC | IS_INTERNAL | LED_TOGGLE }

That doesn’t mean that it won’t change in the future. But, I would assume that in the cases where a key carries the IS_INTERNAL flag, it would not carry the IS_CONSUMER or IS_SYSCTL flags. Perhaps a simple fix to this issue would be to reorder the if statements in handleSyntheticKeyswitchEvent as such:

key_events.cpp:

static bool handleSyntheticKeyswitchEvent(Key mappedKey, uint8_t keyState) {
  if (mappedKey.flags & RESERVED)
    return false;

  if (!(mappedKey.flags & SYNTHETIC))
    return false;

  if (mappedKey.flags & IS_CONSUMER) {
    if (keyIsPressed(keyState)) {
    } else if (keyWasPressed(keyState)) {
      kaleidoscope::hid::pressConsumerControl(mappedKey);
      kaleidoscope::hid::releaseConsumerControl(mappedKey);
    }
  } else if (mappedKey.flags & IS_SYSCTL) {
    if (keyIsPressed(keyState)) {
    } else if (keyWasPressed(keyState)) {
      kaleidoscope::hid::pressSystemControl(mappedKey);
      kaleidoscope::hid::releaseSystemControl(mappedKey);
    }
  } else if (mappedKey.flags & SWITCH_TO_KEYMAP) {
    // Should not happen, handled elsewhere.
  } else if (mappedKey.flags & IS_INTERNAL) { //relocated from the top of this if chain
    return false;
  }

  return true;
}

This simple change would resolve this issue completely. I tested the fix on my board, and it seems to be fine (mute works and the LED key works). I’m not sure if there’s a nuance of the software that I’m missing, though, where one would need to test for IS_INTERNAL prior to IS_CONSUMER or IS_SYSCTL. Recognize that this bug prevents any HID types with the 4th bit set (NARY, OOC, and SEL) from being sent to the computer.

3 Likes

I think this would work. Another thing that may be worth trying is to remove the IS_INTERNAL check from handleSyntheticKeyswitchEvent, and change the event handler in LEDControl so it doesn’t require a full byte for essentially a single key.

In other words, we should try to kill IS_INTERNAL… But perhaps we should do that in a separate step, and get the easier fix in first.

Can you open an issue (or a PR) on GitHub? No worries if not, I’ll do it in that case.

Small clarification here: I believe that the quick fix is to remove the IS_INTERNAL check from handleSyntheticKeyswitchEvent. It is there only because at the dawn of time, LEDControl lived in the same repo as the rest of Kaleidoscope, and was coupled much tighter. It’s a safety check so that if one has a Key_LEDEffectNext, but does not have &LEDControl (or any LED effect) in their Kaleidoscope.use(), we wouldn’t do something silly. I think this is such a corner case that we need not care about. If one has LEDControl enabled, then its event handler will catch the keycode, and it will never reach handleSyntheticKeyswitchEvent, making the check there pointless.

The real fix, one that would mean least complexity would be to add something like LEDNEXT to Kaleidoscope-Ranges, and make LEDControl use that instead. This would avoid any conflicts of bit flags, and wouldn’t reserve a full byte for essentially one key. It introduces a dependency on Kaleidoscope-Ranges in a core plugin, mind you. But that’s… not such a terrible thing, I hope.

1 Like

I fully support us pulling IS_INTERNAL.

I kind of wonder if LED Next should just be a macro.

1 Like

Core dependency on Kaleidoscope-Ranges, core dependency on Kaleidoscope-Macros - not much difference? The Ranges solution seems slightly cleaner to me, or at least more in line with how other plugins’ “special” keys are handled.

understood. i’m sleepdepped. and thinking about this from my phone :wink:

What I have wanted for quite some time is a clean way to define keymap keys as custom functions called when events happen on that key. Sort of like macros but better isolated. I feel like that will let us do a lot of stuff in a cleaner way.

I’ll try to figure out how to submit a PR later tonight… I’ll submit my reorder change until we can figure out another option for IS_INTERNAL.

Edit: Hopefully did it correctly. I’ve never used GitHub before…but I think it’s correct. :blush:

1 Like

That would be an even easier solution than my earlier proposal of adding it to Kaleidoscope-Ranges.

Should we just wait till this gets fixed and merged? Then recompile?

On a different note, I’m an idiot and I’m sometimes too lazy to look at that wonderful laminated guide, so… volume decrement is “n”, mute is “m” (even though it doesn’t work yet), and volume increment is “<”. Mute as M is much easier to remember.

Will we just need to recompile and flash or do we need to change the value from Key_Mute to Consumer_Mute?