Expose LEDs via USB

[mondalaci]

This issue contains a specification for exposing LED states (both of the LED display and the per-key LEDs) via getter and setter USB commands, which is useful for displaying custom information and animations.

First up, let's add the UsbVariable_LedOverride struct and expose it via the getter/setter interface of right/src/usb_commands/usb_command_{get,set}_variable.[ch]. The following byte fields should be featured:

• letters (the LEDs of the 3 letter keymap abbreviation)
• layers (the 3 layer LEDs)
• capsLock (the caps lock icon)
• agent (the Agent icon)
• adaptiveMode (the triangle icon of the adaptive mode)
• keys (per-key LEDs)

By default, these fields are set to 0, meaning that the UHK firmware is in control of all the LEDs. By setting a field to 1, the relevant LEDs won't be controlled by the UHK firmware anymore, and the user will be able to override their value.

Let's add the GetLedValues USB command. The input parameters of this command are slotId and ledIndex, and based on these parameters, GenericHidOutBuffer should be filled up with the relevant LED values taken from LedDriverValues.

Let's also add the SetLedValues USB command. The first argument is the slotId byte. Subsequent arguments are byte-pairs of ledId (within the slot) and the related ledValue to be set. A ledId of 255 value should be ignored, and it marks the end of the slot's LED settings, meaning that a new slot id and led index, value pairs follow. This way, bytes with 255 value should be used for padding the unused part of GenericHidOutBuffer.

I also have a good idea regarding the host-side API, so please open a new Agent issue before starting to implementing it, even if your language of preference is not TypeScript or JavaScript because I'd like to keep libraries of different languages consistent.

[dancek]

I see where you're coming from, ie. it would be great to have full control over each led and their individual brightness over the same API. I agree on that. This seems like a good spec to me. I'm willing to work on an implementation, though I'll note I don't have backlit switches.

There are a couple of things that should be considered before settling on this spec. One is a change I'm suggesting, the others are just questions that probably have easy answers.

Message splitting

A single interrupt message is limited to at most 64 bytes and the minimum interval between messages is 1ms. The command type and slotId take two bytes, so a single message can carry at most 31 pairs of ledId + ledValue.

The character display has 3*14 = 42 segments. The left half has 31 keys, the right one 33, totaling 64. In practice, you always need two messages to set the character display and three messages to set key backlight leds. My experience in frontend development and communication protocol design says this is bad. I'm a newbie with USB so I'm not sure what could go wrong, but my instinct says it's best to avoid split messages and millisecond delays in the middle of a logical change.

My proposed solution is to remove the ledIndex/ledId parameters. I.e. the GetLedValues with slotId = LETTERS would return all the letter segments in a single message. Of course, the key leds will still not fit a single message. But it would be logical to have separate slots for the left half and the right half.

Other ideas to overcome this issue (that I don't consider as worthwhile) are

• separate endpoints for setting leds on/off with single bits and brightness per led
• bulk transfer

Kernel driver

The caps lock LED is usually driven by a kernel driver, with a Set_Report call on the keyboard hid device. Should these calls be left unhandled when the user has overridden the capslock led control? Perhaps some other similar questions arise, eg. should the Agent take over control of the Agent LED?

Partial overrides

The proposed spec seems to suggest each individual LED has its own override flag. However, overriding individual LEDs in the character display doesn't make much sense to me. I'd only treat the mod/fn/mouse/capslock/agent/adaptivemode LEDs individually, and the character display and backlights as groups. But maybe there's a good argument for individual LEDs?

Returning to non-overridden mode

As the overrides can be set to off, the firmware should know both the firmware-specified value and the user-specified value for each led. Otherwise setting an LED override off would leave it in an inconsistent state. Am I right?

[mondalaci]

Can you solder? If so, we can send you a backlight upgrade kit, and you'll be able to upgrade your UHK 60 v1.

There are a number of ways to implement USB data transfer and each has its pros and cons. As far as I know, the current implementation has no match when it comes to the lack of driver installation and the general lack of issues, so I don't plan to move away from 64 byte USB interrupt communication packages.

I don't foresee the LED display getting changed often via this USB API, and even if it gets changed, it will only take 2 ms. I much rather expect folks to play with per-key backlighting, and RGB LEDs will take far more bandwidth for fancy animations given that one RGB LED contains 3 separate LEDs. Based on the above, I don't think it's worth introducing a dedicated slot for the LED display.

I don't suggest to make LEDs individually overridable as I don't think it'd make much practical sense. Treat every one of the 6 fields of the struct I suggested as boolean values.

When the override mode is disabled, the firmware should overwrite LedDriverValues according to the normal operation of the UHK. When the override mode is enabled, the relevant LEDs should not be overridden by the firmware, only via SetLedValues. We should simply branch based on the override values, so the user can enable or disable the override any time and the UHK will work naturally.

[dancek]

Can you solder? If so, we can send you a backlight upgrade kit, and you'll be able to upgrade your UHK 60 v1.

I can, but I'm not sure I'm very eager to. Let's discuss this later when we have something else working.

There are a number of ways to implement USB data transfer and each has its pros and cons. As far as I know, the current implementation has no match when it comes to the lack of driver installation and the general lack of issues, so I don't plan to move away from 64 byte USB interrupt communication packages.

I agree about staying with interrupt communication, but I'd still like to nitpick a bit.

For basic HID, that's true. But for interrupt messages we already need custom software. Yes, it can be userspace software given something like libusb (which actually just uses OS USB drivers under the hood). AFAICT Agent uses node-hid which uses hidapi which uses libusb which uses the USB drivers provided by the OS. So by using anything that libusb supports we're not really adding any dependencies.

I don't foresee the LED display getting changed often via this USB API, and even if it gets changed, it will only take 2 ms. I much rather expect folks to play with per-key backlighting, and RGB LEDs will take far more bandwidth for fancy animations given that one RGB LED contains 3 separate LEDs. Based on the above, I don't think it's worth introducing a dedicated slot for the LED display.

I'm already running scrolling texts on the led display so updating once every 100ms is IMHO reasonable. Of course 2ms is not that much, but considering the same bandwidth is used for everything here, hogging it doesn't sound like a good idea.

I'm not sure what you mean by slot. I thought the slots would be the same as the six fields of UsbVariable_LedOverride?

Also, I hadn't considered RGB LEDs at all. That's a lot of bandwidth if every key has three 8-bit brightness indicators. So if there are 3 LEDs per key * 64 keys = 192 backlight leds and you can set 31 in a single interrupt packet, just setting backlight on/off takes 7 interrupt packets.

I'm still of the opinion that logical state changes should fit in a single packet. I'll try to think of a protocol and suggest that later.

[mondalaci]

I don't think many will use scrolling text with such a fast speed.

Regarding slots, see https:/UltimateHackingKeyboard/firmware/blob/master/right/src/slot.h

What do you mean by "logical state changes"?

[dancek]

By logical state changes I mean what happens "at once". It could be setting the backlight color for the whole keyboard. It could be setting a specific text in the display. It could be a single animation frame in backlight animation. These can mostly fit in a single message, and I'll write a protocol proposal soon.

[mondalaci]

I'm sorry, but I really don't want to implement additional protocol commands, and I'd rather keep the protocol as simple as possible. I consider implementing an arbitrary set of specialized commands premature optimization at this point.

[dancek]

SetLedValues protocol proposal

First, some rationale. I consider splitting messages undesirable if it is not necessary. At the same time, a protocol should be easy to understand. It should also fit use cases as well as possible. If it's necessary to make some use cases difficult, they should be the rare ones.

Expected use cases

• Setting single LEDs to a specific value
• Setting the LED display to a monochromatic configuration; some LEDs are off, some are at a user-defined value.
• Setting the backlight fully to a specific color (ie. all RGB LEDs are set at once).
• "Rendering" a backlight animation frame.
• Anyfhing I'm missing?

The general idea

Now, consider backlight animations which are the most difficult and bandwidth intensive. Usually an animation would not require every RGB LED to have a different value. Rather, you could have different kinds of waves going around the keyboard. Typically you'd have maybe up to a dozen different colors for the 64 keys, but often just a few. Therefore we can hugely optimize by setting multiple keys to the same color at once rather than each LED separately. Also, note that keys are the logical single unit, not the three individual LEDs in the RGB LED package. Almost every time the LED value is changed, all color components are changed.

A SetLedValues payload

First, we define an enum called Target with the following values:

• Target_Display
• Target_Layers
• Target_CapsLock
• Target_Agent
• Target_AdaptiveMode
• Target_Backlight

Now, the interrupt message for SetLedValues would be:

Offset	Size (bytes)	Description
0	1	constant: UsbCommandId_SetLedValues
1	1	Target
2	1	n (command count, 1..5)
3..	11 * n	array of SetLedCommand

And the structure of SetLedCommand is

Offset	Size (bytes)	Description
0	1	LED value (R / single)
1	1	LED value (G)
2	1	LED value (B)
3	8	A bitmask indicating which LEDs are to be set. The indices are different for different targets.

On the implementation of GetLedValues

The values for the RGB LEDs do not fit in a single message, no matter what. However, we can fit one color channel (64 bytes) in a single message, so requesting them a channel at a time would make sense. Ie. add three enum values Target_Backlight_[RGB] that are for GetLedValues only.

Comments

Voila, we have a simple format (or "protocol"; sorry for the confusion) which we can use to set all the LEDs of a single type (Target) at once, for up to 5 different colors. If more than five colors are needed, multiple interrupt messages are needed. However, I dare say this protocol will require fewer messages for all practical purposes than one with (ledIndex, value) pairs.

For purposes other than setting the RGB LEDs this protocol wastes a lot of bytes, but that doesn't matter because a single USB interrupt message is used anyway.

[dancek]

Actually, it might be better to move Target into the SetLedCommand struct and be able to run 5 commands against multiple targets with one message.

Anyway, looking forward for comments on the general concept.

[mondalaci]

I can see the value of your suggested protocol, assuming that USB bandwidth is scarce, but it's plentiful. I2C bandwidth is scarce.

USB bandwidth is 1,000 * 64 = 64,000 bytes per sec. I2C bandwidth is 100,000 / 8 = 12,500 bytes per sec. The UHK essentially protocol-translates SetLedValues from USB to I2C, and the latter is the bottleneck. The USB protocol isn't used for bandwidth-intensive purposes other than LEDs, so optimizing it merely adds to its complexity.

[dancek]

That just means we might need a better backlight protocol over I2C (or a faster I2C). If the I2C communication sends two bytes per LED and is used to set 33 RGB LEDs = 99 LEDs, it takes 2B*99 / 12,500 B/s ~= 16ms to complete. That will look weird to the human eye.

[mondalaci]

The I2C protocol of the LED drivers is fixed in silicon, and the baud rate cannot be raised by much given the parasitic capacitance of the I2C bus. These are hardware constraints. I think the baud rate is sufficient for reasonable smooth animations.

[dancek]

Oh, you mean the I2C for IS31FL3731, I thought we're talking about the communication between the right and the left half (I hadn't really looked at the hardware side of the UHK before). The chip seems to support 8 frames in memory that can be switched to or run as an animation. The best animation system would be built around this, but that sounds like a lot of work.

[dancek]

WRT the (ledIndex, value) approach, are you sure there will be no issues when sending 7 interrupt messages at once? Could the device or a USB driver drop messages if there already are other unhandled interrupt messages?

[mondalaci]

We'll be using 3 different ISSI ICs in our current and upcoming keyboards and modules, and most of them don't support animations, so we can't rely on the animations feature.

Interrupt communication has always been reliable, and no dropped messages have been noticed.

[benedekkupper]

How's the progress on this? I'm a bit late to the party, but it's definitely worth pointing out that there is already a standardized HID usage definition for RGB LED control on things such as keyboards. Check HID usage page 59 Lighting and Illumination.

[mondalaci]

There's no progress on this issue yet. If there is any progress, we'll update this issue.

Thanks for pointing to the HID page, but I doubt any keyboard uses it, and it'd complicate our USB implementation.

[dancek]

How's the progress on this? I'm a bit late to the party, but it's definitely worth pointing out that there is already a standardized HID usage definition for RGB LED control on things such as keyboards. Check HID usage page 59 Lighting and Illumination.

Good to know! As you can guess from the discussion I lost motivation to work on this. It shouldn't be very difficult to implement but I don't have LEDs on my hardware and don't really care about them, and the way LED control is going to be implemented isn't very good for my use case (animating the segment display). My old closed PR might give hints on how to implement the proper LED control discussed here.

[Psychopompous]

If there is any progress, we'll update this issue.

It's been more than a year, so I hope it's okay to ask if there's some update on this topic, we also have multiple other LED color related tickets that occasionally get updated.

I recently got a v2 keyboard ... and I thought the RGB lighting was one of the main new features. But we're mostly stuck with the default coloring, unless one wants to start playing around with other firmwares.

I would love to be able to dynamically change colors via the command line, for example to indicate the end of long running processes, or have a visual notification of a meeting about to start. And after work some relaxing animated sea of color, pulsing to the sound of the ambient music. 🚥

[mondalaci]

@Psychopompous I'm as enthusiastic as you about the mentioned possibilities, but we plan to implement per-key LED color settings via Agent first. Chances are we'll also be able to implement this feature in 2023.

[jlsjonas]

I would love to be able to dynamically change colors via the command line, for example to indicate the end of long running processes, or have a visual notification of a meeting about to start. And after work some relaxing animated sea of color, pulsing to the sound of the ambient music.

That's pretty much what I anticipated doing once I finally have enough time to unbox & set up the v2 (v1 works fine, but doesn't allow for the above)

we plan to implement per-key LED color settings via Agent first.

🤔 I must say that it kinda looks implied that this option is already present, glad I didn't open my v2 yet...