feat: implement keyboard device management#134
Merged
Conversation
- Added Keyboard struct and associated methods for creating and managing keyboard devices. - Introduced error handling for device creation and validation. - Implemented functions to enable/disable keyboards and check their status. - Added utility function to identify keyboard devices in the X11 environment. Log: This enhancement improves keyboard device handling within the dxinput package. pms: BUG-315763
deepin pr auto review代码审查意见：
|
Reviewer's GuideThis PR extends the dxinput package with a dedicated Keyboard type—complete with creation, validation, enable/disable, and status-check methods—and enhances the underlying C utilities to accurately detect keyboard devices in an X11 environment. Sequence Diagram for NewKeyboard function logicsequenceDiagram
participant Caller
participant NK as "dxinput.NewKeyboard()"
participant Utils as "utils"
participant DevInfoList as "DeviceInfoList"
participant NKDI as "dxinput.NewKeyboardDevInfo()"
participant KeyboardStruct as "Keyboard (struct)"
Caller->>NK: NewKeyboard(id)
NK->>Utils: ListDevice()
Utils-->>NK: infos (*DeviceInfoList)
alt infos == nil
NK-->>Caller: (nil, error "no device")
end
NK->>DevInfoList: infos.Get(id)
DevInfoList-->>NK: info (*DeviceInfo)
alt info == nil
NK-->>Caller: (nil, error "invalid device id")
end
NK->>NKDI: NewKeyboardDevInfo(info)
alt info == nil OR info.Type != DevTypeKeyboard
NKDI-->>NK: (nil, error "not a keyboard device")
else Valid DeviceInfo
NKDI-->>KeyboardStruct: &Keyboard{Id: info.Id, Name: info.Name}
KeyboardStruct-->>NKDI: keyboardInstance
NKDI-->>NK: (keyboardInstance, nil)
end
NK-->>Caller: (*Keyboard, error)
Sequence Diagram for Keyboard.Enable() method logicsequenceDiagram
participant Caller
participant K as "Keyboard instance"
participant kwayland_mod as "kwayland module"
participant dxinput_utils_go as "utils.enableDevice (Go)"
Caller->>K: Enable(enabled)
alt globalWayland is true
K->>kwayland_mod: Enable(m.Id_formatted_for_Wayland, enabled)
kwayland_mod-->>K: error_or_nil
else globalWayland is false
K->>dxinput_utils_go: enableDevice(m.Id, enabled)
dxinput_utils_go-->>K: error_or_nil
end
K-->>Caller: error_or_nil
Sequence Diagram for C function is_keyboard_device()sequenceDiagram
participant CallingCFunction as "Calling C Code (e.g., query_device_type)"
participant is_keyboard_device_c as "is_keyboard_device() (C function)"
participant X11Server as "X11 Server"
CallingCFunction->>is_keyboard_device_c: is_keyboard_device(deviceid)
is_keyboard_device_c->>X11Server: XOpenDisplay(NULL)
X11Server-->>is_keyboard_device_c: display*
alt display == NULL
is_keyboard_device_c-->>CallingCFunction: 0 (false)
end
is_keyboard_device_c->>X11Server: XIQueryDevice(display, deviceid, &num_devices)
X11Server-->>is_keyboard_device_c: devices* (XIDeviceInfo)
alt devices == NULL OR num_devices != 1
is_keyboard_device_c->>X11Server: XIFreeDeviceInfo(devices) %% Potentially if devices not null
is_keyboard_device_c->>X11Server: XCloseDisplay(display)
is_keyboard_device_c-->>CallingCFunction: 0 (false)
end
is_keyboard_device_c->>is_keyboard_device_c: Check devices[0].use == XISlaveKeyboard
alt devices[0].use == XISlaveKeyboard
is_keyboard_device_c->>X11Server: XIFreeDeviceInfo(devices)
is_keyboard_device_c->>X11Server: XCloseDisplay(display)
is_keyboard_device_c-->>CallingCFunction: 1 (true)
else devices[0].use != XISlaveKeyboard
is_keyboard_device_c->>X11Server: XIFreeDeviceInfo(devices)
is_keyboard_device_c->>X11Server: XCloseDisplay(display)
is_keyboard_device_c-->>CallingCFunction: 0 (false)
end
Class Diagram for the new Keyboard struct and related componentsclassDiagram
class Keyboard {
+Id: int32
+Name: string
+Enable(enabled bool) error
+IsEnabled() bool
}
class dxinputPackage {
<<Go Package>>
+NewKeyboard(id int32) (*Keyboard, error)
+NewKeyboardDevInfo(dev *DeviceInfo) (*Keyboard, error)
}
dxinputPackage --o Keyboard : creates
class DeviceInfo {
+Id: int32
+Name: string
+Type: DevType
}
class utilsPackage {
<<Go Package>>
+ListDevice() *DeviceInfoList
+enableDevice(id int32, enabled bool) error
+isDeviceEnabled(id int32) bool
}
class kwaylandPackage {
<<Go Package>>
+Enable(deviceId string, enabled bool) error
+CanEnabled(deviceId string) bool
}
dxinputPackage ..> utilsPackage : uses ListDevice()
dxinputPackage ..> DeviceInfo : uses for NewKeyboardDevInfo()
Keyboard ..> utilsPackage : uses enableDevice(), isDeviceEnabled() (X11 path)
Keyboard ..> kwaylandPackage : uses Enable(), CanEnabled() (Wayland path)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @fly602 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } | ||
|
|
||
| type Keyboard struct { | ||
| Id int32 |
There was a problem hiding this comment.
suggestion: Use 'ID' instead of 'Id' for Go naming consistency
Consider renaming the Id field to ID and updating all references for consistency with Go conventions.
Suggested implementation:
type Keyboard struct {
ID int32
Name string
}func NewKeyboard(id int32) (*Keyboard, error) {
infos := utils.ListDevice()
if infos == nil {
return nil, errors.New("no device")If there are any other references to the Id field of Keyboard elsewhere in this file (or in other files), they should also be updated to use ID for consistency.
|
|
||
| // 获取所有输入设备 | ||
| XIDeviceInfo *devices = XIQueryDevice(display, deviceid, &num_devices); | ||
| if (devices == NULL || num_devices != 1) { |
There was a problem hiding this comment.
suggestion (bug_risk): Free devices on error path to avoid memory leak
Call XIFreeDeviceInfo(devices) before returning when num_devices != 1 to prevent a memory leak.
| } | ||
|
|
||
| static int | ||
| is_keyboard_device(int deviceid) |
There was a problem hiding this comment.
issue (complexity): Consider replacing the is_keyboard_device() implementation with a call to get_xdevice_by_id() to simplify resource management and error handling.
// Replace the current implementation of is_keyboard_device() with a call to
// get_xdevice_by_id(), which already handles the X11 connection, locking,
// error‐checking and cleanup for you.
static int
is_keyboard_device(int deviceid)
{
XIDeviceInfo *info = get_xdevice_by_id(deviceid);
if (!info)
return 0;
int is_kbd = (info->use == XISlaveKeyboard);
XIFreeDeviceInfo(info);
return is_kbd;
}By doing this you:
- Eliminate per-call XOpenDisplay()/XCloseDisplay() boilerplate.
- Reuse the shared mutex and error handling in get_xdevice_by_id().
- Keep all existing functionality (checking
use == XISlaveKeyboard).
caixr23
approved these changes
Jun 3, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: caixr23, fly602 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Log: This enhancement improves keyboard device handling within the dxinput package.
pms: BUG-315763
Summary by Sourcery
Implement keyboard device management in the dxinput package, including device creation, validation, enable/disable controls, and X11 detection.
New Features: