duncanthrax / roccat-vulcan

Linux RGB LED effect support for the Roccat Vulcan 100/120 Keyboard
GNU General Public License v3.0
79 stars 20 forks source link

Pause/Stop/Next/Prev keys stop working after init #6

Open SpiritCroc opened 5 years ago

SpiritCroc commented 5 years ago

After sending the init reports, more precisely the 0x150001 sequence, the keyboard changes what data is sent on the .2 device.

On one hand, additional data is sent for each key press and release, which I suppose is used for effects in the swarm app, since we can get feedback from FN-presses and alike here too, while we don't get a normal key press from that. On the other hand, the media keys (play-pause, stop, previous song, next song) completely change their data sent (possibly for easier key remapping for the original driver?).

To work around that, I wrote this small linux kernel driver which listens for the new data and reports the normal media keys: https://github.com/SpiritCroc/linux/commits/roccat-vulcan

So I wanted to ask you for your opinion, if you maybe have a different/better solution or whether you think if this is worth trying to get upstream?

duncanthrax commented 5 years ago

Hey, this is pretty cool, thanks for investigating! Of course, extending your workaround driver to a full-blown one that exposes LED support to userspace would be even better. That would also remove the crutch of having to read keypresses via evdev.

But I'm not sure if upstream would like drivers that send binary magic to hardware ... but then I'm also not sure if they like "workaround" drivers either. I'd say, for the time being try and get this upstream so we have no real problems left and we can take it from there.

I'm planning to add some topology recording to the program, so we can (manually) read in topology (rows, cols, neighbors etc.) for the different models. This would enable some more nice effects.

SpiritCroc commented 5 years ago

Looking at the linux kernel, the old roccat driver suite seems to do it by having some kernel drivers exposing basic operations from the kernel to the userspace driver as well.

The custom effects are probably better done from userspace so that they can easily be extended, and I don't think that those would have a good change getting upstream either way. I can imagine that we might be able to propagate the keypress information for fx usage to userspace - I'm not sure how to best do that right now though.

For sending the init sequence - that would indeed be sending some binary magic to the keyboard, so I'd rather not include it in the linux driver either. Since the led mapping depends on the init sequence, I'll guess that will have to stay out then as well. On the other hand, some might even want to use the keyboard without the extra fx stuff, but only use the inbuilt profiles, so sending the init sequence should be optional at least. Since there is no issue I'm aware of when doing this from the userspace driver, I guess we can keep that part as is. The init sequence will only be needed when using the userspace driver either way.

Before I try to get it upstream, I'll do some more testing (an earlier version make my system hang after some time), maybe you (and others who might read this) can test this as well, so we have some feedback from more devices/layouts (and I could add a Tested-by tag to the patch).

For the topology, I assume there is some way to detect it automatically for the official driver, but building it manually might be useful still if we might be able to use it later for automatic selection.

SpiritCroc commented 5 years ago

FYI, I extended my kernel driver to also react to profile keys: it now sends F13-F16 (edit: will probably change it to fn_f1 - fn_f4, seeing that keycodes for those seem to exist) for those, updated driver is here

duncanthrax commented 5 years ago

Thanks, I compiled the last version just 2 hours ago. Works fine so far. Observations:

Index 0: ROCCAT ROCCAT Vulcan 100 AIMO Main Keyboard Index 1: ROCCAT ROCCAT Vulcan 100 AIMO Extra Keyboard Index 2: ROCCAT ROCCAT Vulcan 100 AIMO Misc Device Index 3: ROCCAT ROCCAT Vulcan 100 AIMO LED Device

I realize this is probably part of the existing driver for Roccat Hardware, but maybe you can patch that in too?

duncanthrax commented 5 years ago

One more thing: Using the driver (both old and new code), my CAPSLOCK and LEFTMETA key don't work, both before and after sending the LED init sequence.

Edit: With "don't work", I mean I don't get any event on any evdev device for both keys when using evtest tool.

Edit2: Forget it, the keyboard went in "Game mode" where these keys are blackholed by the hardware. But I'm not sure how I ended up in that mode.

SpiritCroc commented 5 years ago

Thanks for the feedback!

About the missing device, don't know yet why this is happening, do you happen to know what it was used for?

For the device naming, it looks like the "Mouse", "Consumer Control" and "System Control" suffixes were originally added by the generic driver, and I think I'll be able to change them to usable names when using my driver like you requested.

I'm glad to hear the other issue was game mode, I'd have no idea what was happening otherwise, haha

SpiritCroc commented 5 years ago

Updated driver now reports suggested input names, and maps profile buttons to FN_F1 - FN_F4. Unfortunately, those keys aren't supported by xorg because the keycode is > 255 from what I've read though.

duncanthrax commented 5 years ago

Thanks, works perfectly here! I assume the FN key does not send any scancode also when LED init sequence is sent? Because supporting that key would fill the last gap ...

Meanwhile I have added some topology recording functions. I have to finish the neighbor one, then we can have proper ANSI support. I've seen you already opened and closed a PR?

SpiritCroc commented 5 years ago

I closed the old PR that fixed backslash for me but probably broke it on ISO layout. The new PR makes -b ansi work for me for static color use (using my work-in-progress pipe patch) , I probably can update it with the neighbour data when the reading function is done.

From what I've seen, the FN key is handled by the keyboard itself to change keycodes sent for other keys like f5-f8, or do some internal profile switching like with f1-f4. As soon as we send the init sequence with the userspace driver though, additional packages are sent for every key on the keyboard for press and release (capture data typically 0x0300fb + 2 more bytes that identify the key), including the fn key - probably used for typing effects by the original driver. This means we can grab and report fn key presses in case the init sequence was sent earlier. I'm not sure yet whether reporting it is a good idea, since the fn key actually changes the hardware keyboard behaviour, but it should be easy to extend my current driver to do so.

duncanthrax commented 5 years ago

OK, I have commited the simple topo recording support. To go the full mile, you have to first record the available keys with -t keys. The resulting hex salad goes to rv_ev2rv in evdev.c. Then recompile and run those in any order while specifying -b ansi:

You can send me the console output for each of those, I'll cut it up and insert in the appropriate place. I was too lazy to make nicely formatted output ...

The column recording will be a bit ambiguous .. Contrary to common sense, the F12 key must be in a col with PRINT, INSERT, DEL ... otherwise it would be its own column. :/

duncanthrax commented 5 years ago

Re wrt FN key - would be cool to report it as KEY_FN 0x1d0 - the ev code is there so why not use it. I would then extend the ev2rv map to include keys up to KEY_MAX.

SpiritCroc commented 5 years ago

Updated kernel driver to add FN key, and change FN_F1 - FN_F4 detection data to fix key release.

Recorded keys: https://gist.github.com/SpiritCroc/27a9cb8aa841b8a260fcdf05927d6610

duncanthrax commented 5 years ago

Great, latest code supports three times the number of ev codes just to support FN key. ANSI tables updated too, I also added the FN key code where applicable.