feat(CapabilityMap): add generic hidraw button driver#563
feat(CapabilityMap): add generic hidraw button driver#563honjow wants to merge 3 commits intoShadowBlip:mainfrom
Conversation
pastaq
left a comment
pastaq
left a comment
There was a problem hiding this comment.
Much needed feature, thanks for working on this. I want to discuss a few things, so before you change anything below lets get to a consensus.
There was a problem hiding this comment.
This file name doesn't match our current convention. gpd_type5 would be the next logical pattern. TBS, perhaps it would be good to have more descriptive names that include the family, capability map version, and subsystem.
i.e. gpd_type1 would become v1_gpd_evdev1 and this could be v2_gpd_hid1
@ShadowApex thoughts?
There was a problem hiding this comment.
For sorting, I would prefer something like gpd_v1_evdev1, gpd_v2_hid1, etc. But yes, I agree with having more descriptive filenames.
There was a problem hiding this comment.
So I'll rename it to gpd_v2_hid1? @pastaq
| let mut mappings = Vec::new(); | ||
|
|
||
| for mapping in capability_map.mapping.iter() { | ||
| for source in mapping.source_events.iter() { |
There was a problem hiding this comment.
Can we call this source_event? Its a little ambiguous since we also have source_device elsewhere.
| } | ||
|
|
||
| if mapping.byte_index >= report.len() { | ||
| continue; |
There was a problem hiding this comment.
This is be very spammy, but we should add a warn here so people know why their mapping is failing.
There was a problem hiding this comment.
Makes sense, will add a warn log here.
src/input/event/hidraw/translator.rs
Outdated
| Self { mappings, state } | ||
| } | ||
|
|
||
| pub fn has_mappings(&self) -> bool { |
There was a problem hiding this comment.
I don't know why we need both this and has_hid_mappings? It already filters entries to self.mappings based on hidraw events, so its more performant to just check if self.mappings is empty in both cases since the work is done. In evdev we use has_translation. Perhaps we should rename that one to has_evdev_translation to disambiguate, and call this one has_hid_translation and also remove the below one?
There was a problem hiding this comment.
Yeah, no reason to keep both. Will rename to has_hid_translation to match the evdev pattern and drop the static one.
src/input/source/hidraw.rs
Outdated
| match driver_type { | ||
| DriverType::Unknown => Err("No driver for hidraw interface found".into()), | ||
| DriverType::Unknown => { | ||
| if let Some(cap_map) = Self::load_hidraw_capability_map(&conf) { |
There was a problem hiding this comment.
Instead of checking this and running an iter() again when there are mappings, we could just load the GenericHidrawButtons and have it return an error if there are no translations in the map.
There was a problem hiding this comment.
Right, that simplifies things. Will just pass the capability map to GenericHidrawButtons and let it handle the validation.
3 participants
Summary
Add a generic hidraw button driver that uses capability map YAML configs to define button mappings, following the same pattern as existing evdev capability maps. This removes the need to write per-device Rust driver modules for simple HID button devices.
Includes GPD Win 5 HID button support (new firmware) as the first use case. Supersedes #558, which implements the same functionality with a dedicated driver.
Approach
The existing evdev source already uses a declarative approach — devices like MSI Claw and GPD define button mappings in YAML capability maps without any device-specific Rust code. This PR extends that pattern to hidraw by:
HidrawConfigwith flexible detection modes (non-zero byte, exact value match, specific bit check)HidrawEventTranslatorto translate raw HID reports based on the capability mapDriverType::Unknownbut a hidraw capability map is presentAdding support for a new simple HID button device now only requires a YAML capability map file — no Rust code or recompilation needed.
Test plan