fix(input): detect PS/2 wakeup by interface, not name only - #735
Closed
tianming-1996 wants to merge 1 commit into
Closed
fix(input): detect PS/2 wakeup by interface, not name only#735tianming-1996 wants to merge 1 commit into
tianming-1996 wants to merge 1 commit into
Conversation
Align wakeupPath() PS/2 check with isWakeupMachine() (name||interface). Fix PageMultiInfo PS/2 guard to also match the interface field. Extend setWakeupMachine() with interfaceType and fix caller args. 将 wakeupPath() 的 PS/2 判定与 isWakeupMachine() 对齐（name||interface）。 修正 PageMultiInfo 的 PS/2 条件，同时匹配接口字段。 为 setWakeupMachine() 增加 interfaceType 形参并修正调用点参数。 Log: 修复内置PS/2键盘因名称不含PS/2导致唤起电脑菜单缺失的问题 PMS: BUG-294489 Influence: PS/2键盘唤起电脑菜单可正常显示与切换，其他设备唤起行为不变
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: tianming-1996 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 guide (collapsed on small PRs)Reviewer's GuideAlign PS/2 wakeup detection across DeviceInput, DBusWakeupInterface, and UI layers by using both device name and interface type, and extend setWakeupMachine to accept interfaceType so PS/2 keyboard devices without 'PS/2' in their name can properly control wake capability. Sequence diagram for PS2 wakeup detection using name and interfaceTypesequenceDiagram
actor User
participant PageSingleInfo
participant DeviceInput
participant DBusWakeupInterface
participant InputService
User ->> PageSingleInfo: slotWakeupMachine()
PageSingleInfo ->> DeviceInput: wakeupID()
PageSingleInfo ->> DeviceInput: sysPath()
PageSingleInfo ->> DeviceInput: name()
PageSingleInfo ->> DeviceInput: hardwareClass()
PageSingleInfo ->> DeviceInput: getInterface()
PageSingleInfo ->> DBusWakeupInterface: setWakeupMachine(wakeupID, sysPath, wakeup, name, hardwareClass, interfaceType)
alt [name or interfaceType contains PS/2]
DBusWakeupInterface ->> InputService: setWakeupMachine using /proc/acpi/wakeup
else [non PS/2 device]
DBusWakeupInterface ->> InputService: setWakeupMachine using /sys/.../power/wakeup
end
DeviceInput ->> DeviceInput: wakeupPath()
alt [m_Name contains PS/2 or m_Interface contains PS/2]
DeviceInput --> DeviceInput: return /proc/acpi/wakeup
else [other interfaces]
DeviceInput --> DeviceInput: return /sys/.../power/wakeup
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 left some high level feedback:
- The PS/2 detection logic (
name.contains("PS/2") || interface.contains("PS/2")) is now duplicated in several places; consider extracting a small helper (e.g.isPs2Device(name, interface)) to avoid future inconsistency between call sites. - In
PageMultiInfo::getTableListInfo, the PS/2 check usesinfo->name()while other wakeup-related logic usesDeviceInputfields; aligning this to useinput->name()would make the PS/2 detection more consistent with the rest of the input handling.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The PS/2 detection logic (`name.contains("PS/2") || interface.contains("PS/2")`) is now duplicated in several places; consider extracting a small helper (e.g. `isPs2Device(name, interface)`) to avoid future inconsistency between call sites.
- In `PageMultiInfo::getTableListInfo`, the PS/2 check uses `info->name()` while other wakeup-related logic uses `DeviceInput` fields; aligning this to use `input->name()` would make the PS/2 detection more consistent with the rest of the input handling.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★ 总体评分：95分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 --- a/deepin-devicemanager/src/DeviceManager/DeviceInput.h
+++ b/deepin-devicemanager/src/DeviceManager/DeviceInput.h
@@ -12,6 +12,11 @@
#include <QMap>
+namespace DeviceHelper {
+// 统一提取 PS/2 设备判断逻辑，避免多处理重复代码
+inline bool isPS2Device(const QString &name, const QString &interfaceType) {
+ return name.contains("PS/2") || interfaceType.contains("PS/2");
+}
+}
+
class DeviceInput : public DeviceBaseInfo
{--- a/deepin-devicemanager/src/DeviceManager/DeviceInput.cpp
+++ b/deepin-devicemanager/src/DeviceManager/DeviceInput.cpp
@@ -508,7 +508,7 @@ QString DeviceInput::wakeupPath()
return "";
}
- if (m_Name.contains("PS/2") || m_Interface.contains("PS/2")) {
+ if (DeviceHelper::isPS2Device(m_Name, m_Interface)) {
return "/proc/acpi/wakeup";
} else {
return QString("/sys") + m_SysPath.left(index) + QString("/power/wakeup");--- a/deepin-devicemanager/src/WakeupControl/DBusWakeupInterface.cpp
+++ b/deepin-devicemanager/src/WakeupControl/DBusWakeupInterface.cpp
@@ -39,7 +39,7 @@ bool DBusWakeupInterface::setWakeupMachine(const QString &unique_id,
const QString &interfaceType)
{
if (nullptr != mp_InputIface && mp_InputIface->isValid()) {
- QStringList pathList = path.split("/", QString::SkipEmptyParts);
+ QStringList pathList = path.split("/", Qt::SkipEmptyParts);
if (pathList.size() < 3)
return false;
- if (name.contains("PS/2") || interfaceType.contains("PS/2")) {
+ if (DeviceHelper::isPS2Device(name, interfaceType)) {
// ps2设备无法通过/sys/devices/platform/i8042/serio1/power/wakeup控制，只能通过acpi的接口进行控制
QDBusInterface interface(INPUT_SERVICE_NAME, INPUT_WAKEUP_SERVICE_PATH, INPUT_WAKEUP_PROPERTIES_INTERFACE, QDBusConnection::systemBus()); |
Contributor
Author
|
Closed per maintainer's decision (majiuping): proceeding with PR #726 (minimal fix). PMS 294489. |
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
2 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.
根因分析
DeviceInput::wakeupPath()（src/DeviceManager/DeviceInput.cpp:508）判断 PS/2 时仅m_Name.contains("PS/2")，与同文件isWakeupMachine()（第 484 行m_Name.contains("PS/2") || m_Interface.contains("PS/2")）不一致。L420 等机型的内置 PS/2 键盘 hwinfo 名称为"AT Translated Set 2 keyboard"（不含 "PS/2"），而m_Interface = "PS/2"，导致wakeupPath()走错分支返回 sysfs/sys<syspath>/power/wakeup（PS/2 设备无此节点）→canWakeupMachine()打开失败返回 false → 右键菜单"允许唤起电脑"被置灰。关键证据：
DeviceInput.cpp:508name-only 判定 vs:484name||interface 判定（同文件不一致即缺陷）。DeviceInput.cpp:469-471canWakeupMachine()经wakeupPath()打开失败返回 false（能力门禁）。98b67c8e（Bug 336943）已把姊妹函数isInputWakeupMachine()的 PS/2 判定改为按interfaceType判定并加hardwareClass区分 PS2M/PS2K，但未同步修正wakeupPath()、setWakeupMachine()、PageMultiInfo三处同源 name-only 判定，启用 PS/2 键盘唤醒菜单的意图被抵消。修复方案
对齐三处同源 PS/2 name-only 判定，统一为
name.contains("PS/2") || interface.contains("PS/2")：DeviceInput.cpp:508（主根因）wakeupPath()加|| m_Interface.contains("PS/2")，与isWakeupMachine():484对齐 →canWakeupMachine()返回 true，菜单显示。PageMultiInfo.cpp:328加|| input->getInterface().contains("PS/2")→ 表格页 menuControl 正确追加 PS/2 设备的 hardwareClass/interface，唤醒状态查询不再恒为未勾选。DBusWakeupInterface::setWakeupMachine()扩展签名增加interfaceType形参，name.contains("PS/2")→name.contains("PS/2") || interfaceType.contains("PS/2")；同步 4 个调用点（DeviceInput.cpp:440/445、TextBrowser.cpp:114、PageSingleInfo.cpp:359）。其中PageSingleInfo.cpp:359原误将getInterface()传入name形参（历史遗留），一并纠正为name()+getInterface()。该签名扩展与98b67c8e对isInputWakeupMachine的改造同模式，name形参仍用于判定故无新增 unused-param。改动安全评估
风险等级：中（由 #3 的函数签名变更带来；#1/#2 为低风险单行加条件）。#3 经评估风险可控：有
98b67c8e的成熟先例、4 处调用点全在仓内已同步、name仅用于本次判定的contains检查、不变 DBus 协议（服务端ControlInterface::setWakeupMachine签名不变）、不撤销任何历史修复（相反补全了98b67c8e想做漏做的setWakeupMachine侧）。PMS: https://pms.uniontech.com/bug-view-294489.html
Summary by Sourcery
Detect PS/2 wakeup devices by interface as well as name so PS/2 input wakeup controls work consistently.
Bug Fixes:
Enhancements: