Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions dxinput/keyboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,60 @@
package dxinput

import (
"errors"
"fmt"

. "github.com/linuxdeepin/dde-api/dxinput/common"
"github.com/linuxdeepin/dde-api/dxinput/kwayland"
"github.com/linuxdeepin/dde-api/dxinput/utils"
)

func SetKeyboardRepeat(enabled bool, delay, interval uint32) error {
return utils.SetKeyboardRepeat(enabled, delay, interval)
}

type Keyboard struct {
Id int32
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Name string
}

func NewKeyboard(id int32) (*Keyboard, error) {
infos := utils.ListDevice()
if infos == nil {
return nil, errors.New("no device")
}

info := infos.Get(id)

if info == nil {
return nil, fmt.Errorf("invalid device id: %v", id)
}
return NewKeyboardDevInfo(info)
}

func NewKeyboardDevInfo(dev *DeviceInfo) (*Keyboard, error) {
if dev == nil || dev.Type != DevTypeKeyboard {
return nil, fmt.Errorf("not a keyboard device(%d - %s)", dev.Id, dev.Name)
}

return &Keyboard{
Id: dev.Id,
Name: dev.Name,
}, nil
}

func (m *Keyboard) Enable(enabled bool) error {
if globalWayland {
return kwayland.Enable(fmt.Sprintf("%s%d", kwayland.SysNamePrefix, m.Id), enabled)
}

return enableDevice(m.Id, enabled)
}

func (m *Keyboard) IsEnabled() bool {
if globalWayland {
return kwayland.CanEnabled(fmt.Sprintf("%s%d", kwayland.SysNamePrefix, m.Id))
}

return isDeviceEnabled(m.Id)
}
49 changes: 49 additions & 0 deletions dxinput/utils/type.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ static int is_mouse_device(int deviceid);
static int is_touchpad_device(int deviceid);
static int is_touchscreen_device(int deviceid);
static int is_wacom_device(int deviceid);
static int is_keyboard_device(int deviceid);
static XIDeviceInfo* get_xdevice_by_id(int deviceid);

static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
Expand Down Expand Up @@ -64,6 +65,11 @@ query_device_type(int deviceid)
return TYPE_MOUSE;
}

if (is_keyboard_device(deviceid)) {
return TYPE_KEYBOARD;
}


return TYPE_UNKNOWN;
}

Expand Down Expand Up @@ -127,6 +133,49 @@ is_touchpad_device(int deviceid)
is_property_exist(deviceid, "libinput Tapping Enabled"));
}

static int
is_keyboard_device(int deviceid)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

{
Display *display;
int num_devices, i;

pthread_mutex_lock(&mutex);
// 打开 X11 显示
display = XOpenDisplay(NULL);
if (display == NULL) {
fprintf(stderr, "Open display failed at check prop exist\n");
pthread_mutex_unlock(&mutex);
return 0;
}

// 获取所有输入设备
XIDeviceInfo *devices = XIQueryDevice(display, deviceid, &num_devices);
if (devices == NULL || num_devices != 1) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

fprintf(stderr, "Error getting device information.\n");
pthread_mutex_unlock(&mutex);
XCloseDisplay(display);
return 0;
}

if(devices[0].use != XISlaveKeyboard)
{
fprintf(stderr, "Device is not keyboard.\n");
pthread_mutex_unlock(&mutex);
XIFreeDeviceInfo(devices);
XCloseDisplay(display);
return 0;
}

// 释放设备信息内存
XIFreeDeviceInfo(devices);

// 关闭 X11 显示
XCloseDisplay(display);
pthread_mutex_unlock(&mutex);

return 1;
}

// TODO: support libinput
static int
is_wacom_device(int deviceid)
Expand Down