Conversation
不删除触控屏绑定配置 Log: 修复触控屏绑定问题 PMS: BUG-339549 Influence: 触控屏
Reviewer's guide (collapsed on small PRs)Reviewer's GuidePreserves touchscreen-to-display binding configuration across device unplug/reattach events by removing cleanup logic that deleted mappings when a touchscreen or its bound output was not currently present. Sequence diagram for touchscreen change handling after fixsequenceDiagram
participant TouchEventSource
participant Manager
participant MonitorRepo as MonitorRepository
TouchEventSource->>Manager: handleTouchscreenChanged()
Manager->>MonitorRepo: getConnectedMonitors()
MonitorRepo-->>Manager: monitors
alt one_touchscreen_and_one_monitor
Manager->>Manager: associateTouch(monitor, touchUUID, true)
else multiple_or_zero_devices
loop for_each_touchscreen_in_Touchscreens
Manager->>Manager: lookup mapping in touchscreenMap
alt mapping_exists
alt bound_output_exists_in_monitors
Manager->>Manager: associateTouch(monitor, touchUUID, false)
else bound_output_missing
Manager->>Manager: keep mapping in touchscreenMap
end
else mapping_missing
Manager->>Manager: no association change
end
end
end
Manager-->>TouchEventSource: return
Updated class diagram for Manager touchscreen mapping behaviorclassDiagram
class ManagerBeforePR {
map~string_string~ touchscreenMap
map~string_TouchInfo~ TouchMap
Touchscreen[] Touchscreens
+updateTouchscreenMap(outputName string, touchUUID string, auto bool) error
+associateTouch(monitor *Monitor, touchUUID string, auto bool) error
+handleTouchscreenChanged()
-removeTouchscreenMap(touchUUID string)
}
class ManagerAfterPR {
map~string_string~ touchscreenMap
map~string_TouchInfo~ TouchMap
Touchscreen[] Touchscreens
+updateTouchscreenMap(outputName string, touchUUID string, auto bool) error
+associateTouch(monitor *Monitor, touchUUID string, auto bool) error
+handleTouchscreenChanged()
}
ManagerBeforePR <|-- ManagerAfterPR
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 left some high level feedback:
- With the removal of
removeTouchscreenMapand the logic that pruned entries for unplugged touchscreens,touchscreenMapandTouchMapcan now retain stale mappings indefinitely; consider adding a separate, intentional cleanup path (e.g., on explicit reset or after a long period of disuse) so these maps don’t grow unbounded with obsolete UUIDs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- With the removal of `removeTouchscreenMap` and the logic that pruned entries for unplugged touchscreens, `touchscreenMap` and `TouchMap` can now retain stale mappings indefinitely; consider adding a separate, intentional cleanup path (e.g., on explicit reset or after a long period of disuse) so these maps don’t grow unbounded with obsolete UUIDs.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
deepin pr auto review这段代码的 diff 展示了对触摸屏映射管理逻辑的修改,主要涉及删除了 以下是对代码变更的审查意见,涵盖逻辑、质量、性能和安全四个方面: 1. 逻辑与功能
2. 代码质量
3. 代码性能
4. 代码安全
总结与改进建议这次改动主要是删除了处理触摸屏移除时的清理逻辑。 主要改进建议:
结论:代码改动在语法和结构上是正确的,且提升了代码整洁度。但需关注删除清理逻辑后可能带来的"脏数据"残留问题,建议通过防御性编程确保残留的旧配置不会导致运行时错误。 |
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
不删除触控屏绑定配置
Log: 修复触控屏绑定问题
PMS: BUG-339549
Influence: 触控屏
Summary by Sourcery
Bug Fixes: