fix(inputdevices): refactor natural scroll event logging for accurate…#1105
Merged
Merged
Conversation
Reviewer's GuideRefactors natural scroll event logging so mouse and touchpad states are reported via separate, renamed fields, and events are emitted exactly once from startup and dconfig change paths, with added touchpad hotplug handling and removal of unused helpers. Sequence diagram for refactored natural scroll event loggingsequenceDiagram
participant Manager
participant Mouse
participant Touchpad
participant EventLog
alt [startup]
Manager->>Manager: init
Manager->>Mouse: initMouseDConfig
Manager->>Touchpad: initTouchpadDConfig
Manager->>EventLog: LogMouseNaturalScroll(m.mouse.NaturalScroll.Get())
Manager->>EventLog: LogTouchpadNaturalScroll(m.tpad.NaturalScroll.Get(), m.tpad.Exist)
end
alt [mouse natural scroll dconfig change]
Mouse->>Mouse: initMouseDConfig
Mouse->>Mouse: enableNaturalScroll()
Mouse->>EventLog: LogMouseNaturalScroll(m.NaturalScroll.Get())
end
alt [touchpad natural scroll dconfig change]
Touchpad->>Touchpad: initTouchpadDConfig
Touchpad->>Touchpad: enableNaturalScroll()
Touchpad->>EventLog: LogTouchpadNaturalScroll(tpad.NaturalScroll.Get(), true)
end
alt [touchpad hotplug]
Touchpad->>Touchpad: handleDeviceChanged()
alt [touchpad plugged in]
Touchpad->>EventLog: LogTouchpadNaturalScroll(tpad.NaturalScroll.Get(), true)
else [touchpad unplugged]
Touchpad->>EventLog: LogTouchpadNaturalScroll(false, false)
end
end
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 - I've found 1 issue, and left some high level feedback:
- The new
LogTouchpadNaturalScroll(enabled bool, hasTouchpad bool)API is a bit confusing sinceenabledis ignored whenhasTouchpadis false; consider either deriving the empty string internally fromExist(and droppinghasTouchpad), or documenting/changing the signature so callers don’t pass a meaninglessenabledvalue on unplug.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `LogTouchpadNaturalScroll(enabled bool, hasTouchpad bool)` API is a bit confusing since `enabled` is ignored when `hasTouchpad` is false; consider either deriving the empty string internally from `Exist` (and dropping `hasTouchpad`), or documenting/changing the signature so callers don’t pass a meaningless `enabled` value on unplug.
## Individual Comments
### Comment 1
<location path="inputdevices1/event_log.go" line_range="70-74" />
<code_context>
return "false"
}
-
-// LogOnStartup logs the current state on startup with a delay
-// Both touchpad and mouse natural scroll states are logged in one combined event
-func LogOnStartup(touchpadNaturalScroll, mouseNaturalScroll bool) {
- // Delay logging to ensure the event log system is ready
- time.AfterFunc(5*time.Second, func() {
- LogNaturalScroll(touchpadNaturalScroll, mouseNaturalScroll)
- })
</code_context>
<issue_to_address>
**issue (bug_risk):** Removing the delayed startup logging might reintroduce a race with the event log initialization.
The previous `LogOnStartup` delayed logging by 5s so the event log system was ready before startup events were written; the new code logs immediately. Please confirm whether `eventlog.WriteEventLog` is now guaranteed to be safe at startup, and if not, keep a small delay (at this call site) rather than removing it entirely to avoid losing events.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
… reporting
Rename event fields and restructure logging triggers to ensure
each event is reported exactly once from the correct source:
- Rename mouse_natural_scroll -> mouse_scroll_native_on
- Rename touchpad_natural_scroll -> touchpad_scroll_native_on
- Report mouse and touchpad separately on startup instead of combined
- Move logging from enableNaturalScroll() to dconfig change callbacks
to avoid duplicate events during device hotplug/init
- Add touchpad hotplug detection: report real value on plug-in,
empty value on unplug, no event otherwise
- Remove LogOnStartup and LogNaturalScroll helper functions
修改自然滚动事件埋点字段名及上报逻辑，确保事件准确上报：
- 重命名字段 mouse_natural_scroll -> mouse_scroll_native_on
- 重命名字段 touchpad_natural_scroll -> touchpad_scroll_native_on
- 开机时鼠标和触摸板分别独立上报，不再合并
- 将埋点从 enableNaturalScroll() 移至 dconfig 值变更回调，
避免设备热插拔和初始化时重复上报
- 新增触摸板热插拔检测：插入时上报真实值，拔出时上报空值，
其他情况不触发
- 移除 LogOnStartup 和 LogNaturalScroll 辅助函数
Test:
1. systemctl --user restart org.dde.session.Daemon1
-> 2 events: mouse_scroll_native_on ("true"/"false") +
touchpad_scroll_native_on ("true"/"false")
2. Toggle touchpad natural scroll in control center
-> 1 event: touchpad_scroll_native_on only
3. Toggle mouse natural scroll in control center
-> 1 event: mouse_scroll_native_on only
4. Unbind touchpad via sysfs (simulates unplug)
-> 1 event: touchpad_scroll_native_on ("")
5. Bind touchpad via sysfs (simulates plug-in)
-> 1 event: touchpad_scroll_native_on ("true"/"false")
PMS: BUG-361165、BUG-361159、BUG-361147、BUG-361141
Ivy233
force-pushed
the
fix/event-natural-scroll-log
branch
from
9cb24a0 to
06483c6
Compare
deepin pr auto review你好！我是你的智能编程助手 CodeGeeX。我已仔细审查了你提供的 Git Diff。本次代码变更主要重构了输入设备（触控板和鼠标）自然滚动状态的事件日志记录逻辑，将原本的合并日志拆分为独立记录，并增加了对触控板热插拔场景的处理。 以下是我从语法逻辑、代码质量、代码性能和代码安全四个维度提出的详细审查意见和改进建议： 1. 语法与逻辑
2. 代码质量
3. 代码性能
4. 代码安全
综合改进建议代码示例针对上述部分建议，我对 event_log.go 改进： // LogTouchpadNaturalScroll logs touchpad natural scroll state
// When hasTouchpad is false, the enabled parameter is ignored and an empty value is reported to indicate absence.
func LogTouchpadNaturalScroll(enabled bool, hasTouchpad bool) {
value := ""
if hasTouchpad {
value = boolToString(enabled)
}
data := &eventlog.EventLogData{
Tid: EventTidNaturalScroll,
Target: "natural_scroll",
Message: map[string]string{
// 建议确认下游是否能接受空字符串，若不能，可考虑 "null" 或 "not_exist"
"touchpad_scroll_native_on": value,
},
}
if eventlog.WriteEventLog(data) {
logger.Debug("EventLog: touchpad natural scroll:", value)
}
}touchpad.go 改进（增加防抖与并发提醒）： func (tpad *Touchpad) handleDeviceChanged() {
// TODO: 如果此函数可能在多个 Goroutine 并发调用，需要加锁保护 tpad 的状态
// tpad.mu.Lock()
// defer tpad.mu.Unlock()
oldExist := tpad.Exist
tpad.updateDXTpads()
tpad.init()
if oldExist != tpad.Exist {
if tpad.Exist {
// Touchpad plugged in: report real value
LogTouchpadNaturalScroll(tpad.NaturalScroll.Get(), true)
} else {
// Touchpad unplugged: report empty value (false is ignored)
LogTouchpadNaturalScroll(false, false)
}
}
}总结：本次代码变更整体思路清晰，重构方向正确，降低了模块间的耦合，并完善了热插拔场景的日志覆盖。只需稍微注意启动时序（延迟日志移除的影响）和空字符串的下游兼容性即可。 |
mhduiy
approved these changes
May 14, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Ivy233, mhduiy 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.
… reporting
Rename event fields and restructure logging triggers to ensure each event is reported exactly once from the correct source:
修改自然滚动事件埋点字段名及上报逻辑，确保事件准确上报：
Test:
PMS: BUG-361165、BUG-361159、BUG-361147、BUG-361141
Summary by Sourcery
Refine natural scroll telemetry to log mouse and touchpad states separately and accurately across startup, configuration changes, and touchpad hotplug events.
New Features:
Bug Fixes:
Enhancements: