Skip to content

feat(inputdevice): Change gsettings to dconfig#886

Merged
mhduiy merged 1 commit intolinuxdeepin:masterfrom
mhduiy:dconfig/inputdevice
Sep 8, 2025
Merged

feat(inputdevice): Change gsettings to dconfig#886
mhduiy merged 1 commit intolinuxdeepin:masterfrom
mhduiy:dconfig/inputdevice

Conversation

@mhduiy
Copy link
Contributor

@mhduiy mhduiy commented Sep 4, 2025

Optimize dconfig Common

pms: TASK-374909

Summary by Sourcery

Migrate all input device configuration from GSettings to the new DConfig framework, replacing gio.Settings and gsettings dependencies with common DConfig across touchpad, mouse, wacom, keyboard, trackpoint and manager modules.

New Features:

  • Introduce DConfig support for touchpad, mouse, wacom, keyboard, and trackpoint services with initialization, typed getters, and change notifications
  • Add ListKeys and Reset methods plus typed GetValue* and ConnectValueChanged APIs to common/dconfig

Enhancements:

  • Replace gsprop and gio.Settings bindings with native Go types and DConfig in all input device modules
  • Unify DBus property write callbacks to persist settings via DConfig instead of gsettings
  • Remove go-gir/gio and gsettings dependencies from the project

@sourcery-ai
Copy link

sourcery-ai bot commented Sep 4, 2025

Reviewer's Guide

This pull request migrates all input device settings from GSettings (gio.Settings and gsprop) to a unified DConfig-based configuration system. It introduces a new dconfig package with extended API, updates each device (Touchpad, Wacom, Mouse, Keyboard, TrackPoint, and Manager) to initialize DConfig, load initial values, listen for remote changes, and persist property updates back to DConfig via DBus callbacks, while removing legacy GSettings bindings.

File-Level Changes

Change Details Files
Migrate settings from GSettings to DConfig across input modules
  • Removed gio.Settings and gsprop bindings in device structs
  • Initialized DConfig manager in each device's constructor
  • Loaded initial property values from DConfig with sensible fallbacks
  • Connected DConfig value-changed signals to update live state
inputdevices1/touchpad.go
inputdevices1/wacom.go
inputdevices1/mouse.go
inputdevices1/keyboard.go
inputdevices1/trackpoint.go
inputdevices1/manager.go
Implement DBus write-callbacks to persist changes to DConfig
  • Added Set*WriteCallbacks methods to each device
  • Bound each DBus PropertyWrite to call DConfig.SetValue
  • Ensured apply functions are invoked after save
inputdevices1/touchpad.go
inputdevices1/wacom.go
inputdevices1/mouse.go
inputdevices1/keyboard.go
inputdevices1/trackpoint.go
inputdevices1/inputdevices.go
Replace gsprop-based fields with native Go types
  • Converted gsprop.Bool/Int/Double to bool, int32/int64, float64, []string
  • Updated all getters/setters to use direct field access
inputdevices1/touchpad.go
inputdevices1/wacom.go
inputdevices1/mouse.go
inputdevices1/keyboard.go
inputdevices1/trackpoint.go
Extend and refactor the common dconfig API
  • Added ListKeys, Reset, GetValueInt64/32, GetValueUInt32, GetValueFloat64, GetValueStringList
  • Refactored ConnectConfigChanged and added ConnectValueChanged with signal loop init
common/dconfig/dconfig.go
Add JSON schema files for new DConfig defaults
  • Created misc/dsg-configs JSON templates for inputdevices, mouse, touchpad, trackpoint, wacom
misc/dsg-configs/org.deepin.dde.daemon.inputdevices.json
misc/dsg-configs/org.deepin.dde.daemon.mouse.json
misc/dsg-configs/org.deepin.dde.daemon.touchpad.json
misc/dsg-configs/org.deepin.dde.daemon.trackpoint.json
misc/dsg-configs/org.deepin.dde.daemon.wacom.json

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes - here's some feedback:

  • There’s a lot of repeated boilerplate for initializing DConfig, loading properties and wiring callbacks in each device; consider extracting a shared helper or base type to reduce duplication.
  • Panicking on DConfig init failures may be too heavy-handed—prefer returning an error or falling back gracefully instead of crashing the entire daemon.
  • The common/dconfig API has multiple conversion methods and two different change‐notification hooks; streamlining these (e.g. unifying ConnectConfigChanged/ConnectValueChanged and type conversions) would make the code easier to maintain.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There’s a lot of repeated boilerplate for initializing DConfig, loading properties and wiring callbacks in each device; consider extracting a shared helper or base type to reduce duplication.
- Panicking on DConfig init failures may be too heavy-handed—prefer returning an error or falling back gracefully instead of crashing the entire daemon.
- The common/dconfig API has multiple conversion methods and two different change‐notification hooks; streamlining these (e.g. unifying ConnectConfigChanged/ConnectValueChanged and type conversions) would make the code easier to maintain.

## Individual Comments

### Comment 1
<location> `inputdevices1/manager.go:265` </location>
<code_context>
+	managerServerObj := service.GetServerObject(m)
+
+	// WheelSpeed 写回调
+	err := managerServerObj.SetWriteCallback(m, "WheelSpeed", func(write *dbusutil.PropertyWrite) *dbus.Error {
+		err := m.dsgInputdevices.SetValue(dconfigKeyWheelSpeed, m.WheelSpeed)
+		if err != nil {
</code_context>

<issue_to_address>
WheelSpeed write callback uses property value from struct, not from DBus write.

The callback should use write.Value to update WheelSpeed, ensuring the value from DBus is correctly handled.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Sep 4, 2025

TAG Bot

New tag: 6.1.53
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #888

@mhduiy mhduiy force-pushed the dconfig/inputdevice branch 5 times, most recently from 4efe973 to 9fb85b0 Compare September 8, 2025 06:08
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fly602, mhduiy

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Optimize dconfig Common

pms: TASK-374909
@mhduiy mhduiy force-pushed the dconfig/inputdevice branch from 9fb85b0 to ef43751 Compare September 8, 2025 06:16
@deepin-ci-robot
Copy link

deepin pr auto review

根据提供的代码和diff,我将对代码进行审查,并提出改进意见:

1. 代码质量

  1. 代码结构

    • 新增的dcprop.go文件提供了类型安全的配置属性封装,这是一个良好的改进
    • 将gsettings迁移到dconfig的改动合理,提高了配置管理的统一性
  2. 命名规范

    • 常量命名统一,如dconfigKey*前缀的使用
    • 结构体和方法命名清晰,符合Go语言规范
  3. 错误处理

    • 大部分函数都有适当的错误处理
    • 建议在init*DConfig方法中增加更详细的错误日志,便于问题排查

2. 代码性能

  1. 并发安全

    • 使用sync.Once确保信号循环只初始化一次,避免重复创建
    • 配置变更时的回调处理有适当的互斥锁保护
  2. 资源管理

    • DBus连接和信号循环的初始化合理
    • 建议在destroy方法中确保所有资源正确释放

3. 代码安全

  1. 输入验证

    • 在SetValue方法中有类型检查,防止非法值
    • 建议增加对配置值的范围检查,如wheelSpeed的范围验证
  2. 权限控制

    • 配置项的权限设置合理,区分了public和private
    • 敏感配置如disableWhileTypingCmd被正确标记为private

4. 具体改进建议

  1. 键盘配置
// 建议增加配置值验证
func (kbd *Keyboard) validateLayout(layout string) bool {
    // 实现布局验证逻辑
    return true
}
  1. 鼠标配置
// 建议增加速度范围检查
func (m *Mouse) setWheelSpeed() {
    speed := m.WheelSpeed.Get()
    if speed < 1 || speed > 100 {
        logger.Warningf("Invalid wheel speed: %d, using default value", speed)
        speed = 50 // 默认值
    }
    // ... 其余逻辑
}
  1. 触控板配置
// 建议增加配置项联动检查
func (tpad *Touchpad) validateConfig() error {
    if tpad.PalmDetect.Get() && (tpad.PalmMinWidth.Get() <= 0 || tpad.PalmMinZ.Get() <= 0) {
        return errors.New("palm detect dimensions must be positive")
    }
    return nil
}
  1. Wacom配置
// 建议增加动作枚举验证
func (w *Wacom) validateAction(action int) bool {
    _, ok := actionMap[action]
    return ok
}
  1. 通用建议
    • 考虑添加配置项变更的撤销/重做功能
    • 增加配置项的版本控制,便于配置迁移
    • 添加配置项变更的审计日志

5. 测试建议

  1. 单元测试:

    • 为每个配置项的读写操作编写测试用例
    • 测试边界条件和异常情况
  2. 集成测试:

    • 测试配置变更对硬件的实际影响
    • 测试配置持久化和恢复功能

总体而言,这次代码重构提升了代码的可维护性和类型安全性,但在错误处理、输入验证和测试覆盖方面还有改进空间。建议按照上述建议进行优化,以提高代码的健壮性和可靠性。

@mhduiy mhduiy merged commit efc9b5e into linuxdeepin:master Sep 8, 2025
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants