Repository navigation
feat: support displaying LVDS monitor info without EDID - #962
GongHeng2017 wants to merge 3 commits into
Conversation
LVDS displays don't provide EDID data, causing device manager to fail to recognize them. Add getMonitorWithoutEdidFromXrandr() to parse xrandr --verbose output for connected displays without EDID, and setInfoFromLvds() to populate monitor info from xrandr data. Task: https://pms.uniontech.com/task-view-397927.html Story: https://pms.uniontech.com/story-view-42241.html
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: GongHeng2017 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 GuideThe PR adds an xrandr-based fallback monitor-generation path for connected displays without EDID, constructs display records from connector and mode data without replacing existing EDID-backed records, and adjusts monitor presentation for missing screen-size information. Sequence diagram for fallback monitor discovery without EDIDsequenceDiagram
participant Generator as DeviceGenerator
participant Xrandr as xrandr
participant Monitor as DeviceMonitor
participant Manager as DeviceManager
Generator->>Generator: getMonitorInfoFromHwinfo()
Generator->>Generator: getMonitorWithoutEdidFromXrandr()
Generator->>Xrandr: xrandr --verbose
Xrandr-->>Generator: connector and mode data
alt connected connector without EDID
Generator->>Monitor: setInfoFromLvds(connectorName, interfaceType, isPrimary, currentResolution, currentRate, supportResolutions)
Monitor-->>Generator: monitor information populated
Generator->>Manager: addMonitor(device)
else connector has EDID or is disconnected
Generator-->>Generator: skip connector
end
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 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="deepin-devicemanager/src/DeviceManager/DeviceMonitor.cpp" line_range="149" />
<code_context>
+
+ // 当前分辨率(先用原始格式计算比例,再转为显示格式)
+ m_CurrentResolution = currentResolution;
+ caculateScreenRatio();
+ m_CurrentResolution.replace("x", "×", Qt::CaseInsensitive);
+ if (!refreshRate.isEmpty()) {
</code_context>
<issue_to_address>
**Monitor discovery crashes**
When a connected output has no active mode, leaving `currentResolution` empty, `setInfoFromLvds()` passes the empty resolution to `caculateScreenRatio()`, which reaches `gcd(0, 0)` and divides by zero; device generation crashes instead of listing the display.
Skip ratio calculation or handle an empty resolution before calling `caculateScreenRatio()`.
</issue_to_address>
|
|
||
| // 当前分辨率(先用原始格式计算比例,再转为显示格式) | ||
| m_CurrentResolution = currentResolution; | ||
| caculateScreenRatio(); |
There was a problem hiding this comment.
🟠 High · Monitor discovery crashes
When a connected output has no active mode, leaving currentResolution empty, setInfoFromLvds() passes the empty resolution to caculateScreenRatio(), which reaches gcd(0, 0) and divides by zero; device generation crashes instead of listing the display.
Skip ratio calculation or handle an empty resolution before calling caculateScreenRatio().
Prompt for AI agents
In `deepin-devicemanager/src/DeviceManager/DeviceMonitor.cpp` at line 149:
**Monitor discovery crashes**
When a connected output has no active mode, leaving `currentResolution` empty, `setInfoFromLvds()` passes the empty resolution to `caculateScreenRatio()`, which reaches `gcd(0, 0)` and divides by zero; device generation crashes instead of listing the display.
Skip ratio calculation or handle an empty resolution before calling `caculateScreenRatio()`.
P2: In getMonitorWithoutEdidFromXrandr(), check existing monitors in DeviceManager for matching displayInput (connectorName) before calling addMonitor(), preventing duplicate entries under Wayland where xrandr cannot read EDID. P3: Replace waitForFinished(-1) with waitForFinished(5000) to avoid infinite blocking when xrandr hangs. Add displayInput() public getter to DeviceMonitor for the dedup check.
Update doxygen comment to remove LVDS-specific wording and reflect the general 'no EDID' purpose. Function logic unchanged.
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ❌评价: 存在逻辑缺陷 ❌ 不通过 潜在问题: 建议: ['在 setInfoFromXrandrWithoutEdid() 中增加 currentResolution 非空判断,仅当分辨率非空时才拼接刷新率', '将所有 QRegExp 替换为 QRegularExpression,使用 match() 和 captured() 替代 indexIn() 和 cap()', '将 QString::SkipEmptyParts 替换为 Qt::SkipEmptyParts'] 2. 代码质量 ✅评价: 代码结构清晰,注释完整 ✅ 通过 潜在问题: 建议: ['考虑将 getMonitorWithoutEdidFromXrandr() 拆分为 xrandr 输出解析和显示器创建两个子函数', "更新 DeviceMonitor.cpp 第138行注释为 '设置无 EDID 的已连接显示器信息'"] 3. 代码性能 ✅评价: 性能良好,资源使用合理 ✅ 通过 潜在问题: 建议: ['确认 generatorMonitorDevice() 在后台线程执行,避免阻塞主线程', '替换 QRegExp 为 QRegularExpression 以获得更好的正则匹配性能'] 4. 代码安全 🔒评价: 存在0个安全漏洞 ✅ 通过
安全漏洞详情: 建议: ['建议检查 process.waitForFinished(5000) 的返回值,虽然已有 output.isEmpty() 后续检查,但显式处理超时更健壮'] 💡 改进建议代码示例本报告由 AI 代码审查工具自动生成 |
背景
PMS Task #397927: 【智微智能】【HG3350】【IPC-830h】系统下设备管理器未识别到LVDS屏信息
Story #42241: 设备管理器-显示设备中支持显示LVDS显示器
问题
设备管理器-显示设备中当前从 EDID 读取厂商信息显示,LVDS 设备无 EDID,导致识别不到对应信息,设备管理器中无展示。
修复方案
通过 xrandr --verbose 读取所有已连接但缺少 EDID 的显示接口(不限于 LVDS,也包括 VGA/DVI/HDMI/DP/eDP 等),补充 hwinfo / xrandr(EDID) 未识别的显示器信息。
变更内容
DeviceMonitor::setInfoFromLvds(): 新增方法,设置无 EDID 显示器的基本信息(接口名、分辨率、刷新率、宽高比等)DeviceGenerator::getMonitorWithoutEdidFromXrandr(): 新增方法,执行xrandr --verbose解析所有已连接但无 EDID 的显示接口,补充分辨率/刷新率等信息DeviceGenerator::generatorMonitor()末尾调用,确保不覆盖已有 EDID 显示器流程基线
Summary by Sourcery
Display connected monitors without EDID data by supplementing existing monitor detection with xrandr information.
New Features:
Bug Fixes: