Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
Reviewer's GuideAdds D-Bus and DConfig support for a touchpad anti-misclick preference, then synchronizes the /proc/uos/touchpad_expand_switch kernel node with that preference and the global touchpad state across startup and runtime changes, without introducing security-loader or Polkit changes. Sequence diagram for touchpad expand synchronizationsequenceDiagram
participant Client
participant Touchpad
participant DConfig
participant Proc as KernelProcNode
Client->>Touchpad: SetTouchpadExpandEnable(enabled)
Touchpad->>DConfig: SetValue(touchpadExpandEnabled, enabled)
Touchpad->>Proc: WriteFile(enable or disable)
Proc-->>Touchpad: Updated switch state
Touchpad-->>Client: ExpandEnable property changed
DConfig->>Touchpad: ValueChanged(touchpadEnabled or touchpadExpandEnabled)
Touchpad->>DConfig: Value(touchpadExpandEnabled)
Touchpad->>Proc: WriteFile(globalEnabled && preference)
Touchpad-->>Client: ExpandEnable or ExpandIsExist changed
Flow diagram for startup touchpad expand state restorationflowchart TD
A[Touchpad initialization] --> B{Expand switch exists?}
B -- No --> C[Set ExpandIsExist false]
B -- Yes --> D[Read touchpadEnabled and touchpadExpandEnabled]
D --> E[Compute globalEnabled && preference]
E --> F[Write enable or disable to /proc/uos/touchpad_expand_switch]
F --> G[Publish ExpandIsExist and ExpandEnable]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="system/inputdevices1/touchpad.go" line_range="96-98" />
<code_context>
+ return dbusutil.ToError(err)
+ }
+ if err := t.syncTouchpadExpand(enabled); err != nil {
+ logger.Warning("failed to sync touchpad expand:", err)
+ }
+ return nil
+}
</code_context>
<issue_to_address>
**issue (bug_risk):** SetTouchpadEnable returns success after the global udev operation succeeds even when syncTouchpadExpand fails, so callers receive no error while the anti-mistouch kernel node remains in the wrong state.
**Triggers:** When /proc/uos/touchpad_expand_switch exists but cannot be written.
**Suggested fix:** Return the synchronization error from SetTouchpadEnable, or explicitly document and expose the operation as best-effort rather than reporting success.
```suggestion
if err := t.syncTouchpadExpand(enabled); err != nil {
return dbusutil.ToError(err)
}
```
</issue_to_address>
### Comment 2
<location path="system/inputdevices1/touchpad.go" line_range="153-154" />
<code_context>
+// syncTouchpadExpand 根据全局触控板状态和 DConfig 偏好同步防误触开关。
+// 该函数只写 proc 节点,不更新 DConfig。
+func (t *Touchpad) syncTouchpadExpand(globalEnabled bool) error {
+ if err := touchpadExpandExist(touchpadExpandSwitchFile); err != nil {
+ t.setPropExpandIsExist(false)
+ logger.Info("/proc/uos/touchpad_expand_switch not exist.")
+ return nil
+ }
+ t.setPropExpandIsExist(true)
+
+ expandEnabled, err := getDsgConf(_dsettingsTouchpadExpandEnabledKey)
+ if err != nil {
+ logger.Warning(err)
+ // DConfig 默认值为 true,读取失败时按默认值恢复实际状态。
+ expandEnabled = true
+ }
+ return t.setTouchpadExpandEnable(globalEnabled && expandEnabled, false)
+}
+
+func (t *Touchpad) setTouchpadExpandEnable(enabled bool, updateDsg bool) error {
+ logger.Infof("setTouchpadExpandEnable: %v", enabled)
+ if err := touchpadExpandExist(touchpadExpandSwitchFile); err != nil {
+ return err
+ }
+ t.setPropExpandIsExist(true)
</code_context>
<issue_to_address>
**issue (bug_risk):** setTouchpadExpandEnable returns before clearing ExpandIsExist when the proc node disappears after initialization, leaving the exported property permanently true even though the node no longer exists.
**Triggers:** When the proc node is removed or becomes inaccessible after a previous successful existence check.
**Suggested fix:** Set ExpandIsExist to false before returning the existence error, or centralize existence-state updates in the caller.
```suggestion
if err := touchpadExpandExist(touchpadExpandSwitchFile); err != nil {
t.setPropExpandIsExist(false)
return err
```
</issue_to_address>1. Add SetTouchpadExpandEnable and expand state properties. 2. Keep the global touchpad switch controlled by udev rules. 3. Sync the expand proc node with the global and user settings. Log: Add support for the touchpad anti-mistouch switch Influence: 1. Verify SetTouchpadExpandEnable writes /proc/uos/touchpad_expand_switch. 2. Disable the global touchpad and verify expand is disabled. 3. Re-enable the global touchpad and verify expand preference is restored. feat: 支持触控板防误触开关 1. 新增 SetTouchpadExpandEnable 方法及防误触状态属性。 2. 全局触控板开关继续保持 udev 规则控制方案。 3. 根据全局开关和用户配置同步防误触内核节点。 Log: 支持触控板防误触功能开关 Influence: 1. 验证 SetTouchpadExpandEnable 能正确写入防误触节点。 2. 关闭全局触控板,验证防误触开关联动关闭。 3. 重新开启全局触控板,验证用户防误触配置恢复。 PMS: TASK-395683
67f8eed to
c75b01c
Compare
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: [] 2. 代码质量 ✅评价: 良好 ✅ 通过 潜在问题:
建议: ['dconfig JSON 中 name 字段 "touchpad_Expand Enabled" 大小写不一致(Expand 首字母大写),建议统一为 "touchpad Expand Enabled" 或 "touchpad_expand_enabled"。'] 3. 代码性能 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: [] 4. 代码安全 🔒评价: 优秀 ✅ 通过
安全漏洞详情: 建议: [] 💡 改进建议代码示例// 暂无代码示例本报告由 AI 代码审查工具自动生成 |
变更内容
验证
PMS: TASK-395683
Summary by Sourcery
Support the touchpad anti-mistouch expansion switch while preserving global touchpad control through udev rules.
New Features:
Bug Fixes:
Enhancements: