feat(Hardware Support): Razer Tartarus Pro#518
Conversation
2c1c8ab to
7c8446e
Compare
7c8446e to
18a2cee
Compare
pastaq
left a comment
There was a problem hiding this comment.
Sorry for taking so long to review. Overall I am impressed with the direction this is going. I do have a few concerns about a couple of the approaches, but we can discuss them below in each of the issues I raised.
18a2cee to
7dc77a8
Compare
7dc77a8 to
dad1e02
Compare
ec8d9eb to
556b36c
Compare
2aff43f to
d0dc460
Compare
There was a problem hiding this comment.
This file seems odd to me. The mappings are all 1-1. If the device is already populating native events (used for detection) with those values they will route to any target device that supports them. Can you explain what you were trying to accomplish here?
There was a problem hiding this comment.
This file is more of a template to aid users in customizing keys bound to the secondary actuation should they use it as function of analog mode.
How this is intended to work is that during analog mode there are no HID scancodes to speak of, we have to create them in the driver based on what keydepth is seen during sampling and send that to the composite device. As per the analog mode description this driver supports primary and secondary actuation levels. The key associated with primary actuation takes its value from the scancode that is normally emitted from basic mode but we still need a unique value signify a secondary actuation, the Tartarus itself doesn't have much to offer on that front.
Take for example Tartarus key '01'; In basic mode this is mapped to ANSI numeric row 1 and that carries over to the primary actuation. When representing the second actuation, I picked the "unused" 'b' character as that's sequentially the first gap in the USB scancode list that the Tartarus emits, 'a' being mapped to Tartarus key '12' in firmware. Codes that are and aren't emitted from firmware are documented in razer_tartarus_pro/event.rs
Following on, 'b' in effect is used to signal to the composite device that this is "the secondary actuation of key '01'", not a literal 'b'. Though for the purposes of presenting an output having 'b' pulling double duty as the default value suffices. The profile exists to allow remapping as 'b' from the source (the output of the analog mode handler) has a fixed definition to the composite device which is compiled in, but may not be the key the user actually wants emitted when they press it. Changing the target representation provides this customization. This logic extends to the other keys, 'g' for key '02' and so on.
This seemed the most straight forward way to implement this functionality given the existing capability types, albeit with a very convenient amount of characters to map on the Tartarus. The profile being a 1-1 mapping is a consequence of really being a template, if the users are happy with the default mapping they can go without.
| pub const VID: u16 = 0x1532; | ||
| pub const PID: u16 = 0x0244; | ||
|
|
||
| const ENDPOINT_1: i32 = 0; |
There was a problem hiding this comment.
Do these need to be i32 for a specific reason? u8 seems more appropriate at a glance.
There was a problem hiding this comment.
It compares against hidapi::DeviceInfo::interface_number which demands i32. Could always cast it but that may be complicating it. Originally in the match (line 203) these were direct values, but naming them makes it easier to read IMO. Happy to alter as required.
|
This it's starting to look pretty close to being ready. I'll take another look at it soon (the comments from today were pending for a while and I didn't realize I hadn't posted them yet) Prior to moving this from WIP to ready, can you squash the commits into a single commit that follows our semantic versioning format? Something like: |
d0dc460 to
6438ad8
Compare
6438ad8 to
d8c7815
Compare
…er Tararus Pro - Supports analog mode functions: primary/secondary actuation & retrigger - Configuration via device YAML, refer documentation - Introduce num_enum 0.7.6 crate
d8c7815 to
dc286fb
Compare
This PR implements support for the Razer Tartarus Pro gaming keypad. To date I've not found anything which allows usage of the analog mode specific to this model under Linux. This aims to bridge that gap.
The need for a driver exists because analog mode generates vendor defined HID reports that contain the positional value of each key instead of events. These must be processed and mapped to input events and this seemed like a natural fit for InputPlumber to handle.
I think this device is the first of its kind for InputPlumber and assuming the project wishes to extend support to these peripherals, how to represent them from a profile perspective in my mind is the major piece of work to go.
The driver has two modes, basic and analog. Basic mode simply uses the Tartarus Pro in its power-up configuration as a regular keyboard/mouse hybrid and is configured as any other device. Analog mode requires commanding the Tartarus Pro. This can occur manually before starting InputPlumber or if using OpenRazer >=3.12.2 you can use
driver_mode = Truein your config file to the same effect.Falsewill leave the device in basic mode.This implementation takes full control of the hidraw (x3) and evdev nodes (x4) but passes through the last endpoint allowing lighting control software e.g. OpenRGB or Polychromatic to manage the RGB functions whilst InputPlumber is running.
The analog event handler is implemented using simple linear regressions on a small buffer* to drive a per-key state machine and supports the following:
* The buffer depth of 5 was based on protocol analysis of fast key transitions from the device
** Quantization and implementation quirks label the minimum at 1.4mm but the Tartarus Pro marketing spec stands.
*** AKA Rapid Trigger from other vendors
Analog mode usage:
Analog mode parameters currently reside at
50-razer_tartarus_pro.yamlin the analogkeys mapping. There are 5 arrays of 20 values, each value left-to-right represents the numbered key on the Tartarus Pro in order, putting aside 0 indexing. All 20 keys have individual analog settings and these will apply to any loaded key binding profiles. Values are sanitised with a specific exception in place for zeroes. Analog parameters cannot be changed at runtime, a restart of InputPlumber is required to take effect.secondary_rebind.yamlLimits to this PR:
Other topics influencing implementation, not necessarily impacting the path out of WIP:
Like a lot of products in this class it is possible to acquire the serial number, firmware revision and other such data from the device registers. This could be used to uniquely assign profiles to a specific device or implement quirks for a given firmware. Should this be incorporated?
The Tartarus Pro is a left-handed device but someone, somewhere will somehow find a way to use 2 of them. Should we embrace this or defend against it?
Assuming this type of device can be included, I propose the following to navigate out of WIP:
or refactor to only manage analog modefeat: add mouse wheel support #514merged to support scroll wheel remappingReturn device to basic mode either on demand or if InputPlumber closesInputPlumber assumes device state is managed externally, no implementation required.Blockers:
#582 - As this impacts external applications managing device state