Fix(cpu): Improve CPU header fallback and dynamic PageMultiInfo heade…#670
Merged
deepin-bot[bot] merged 1 commit intoMay 28, 2026
Conversation
|
Warning
详情 {
"dmidecode": {
"deepin-devicemanager/src/GenerateDevice/DeviceGenerator.cpp": [
" if (!baseCpuInfo.contains(\"model name\") && dmidecode.contains(\"Version\")) {",
" baseCpuInfo.insert(\"model name\", dmidecode[\"Version\"]); // \u9002\u914d\u7279\u6b8a\u673a\u578bCPU",
" if (!baseCpuInfo.contains(\"vendor_id\") && dmidecode.contains(\"Manufacturer\")) {",
" baseCpuInfo.insert(\"vendor_id\", dmidecode[\"Manufacturer\"]); // \u9002\u914d\u7279\u6b8a\u673a\u578bCPU"
]
}
} |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR enhances CPU device header generation by falling back to dmidecode fields when CPU model/vendor are missing, and adjusts the PageMultiInfo header area to size dynamically based on its content instead of using a hard-coded height. Sequence diagram for CPU header generation with dmidecode fallbacksequenceDiagram
participant DeviceGenerator
participant lsCpu
participant dmidecode
participant calAndSetCpuHeaderInfo
DeviceGenerator->>lsCpu: size()
alt [lsCpu.size() > 0]
DeviceGenerator->>lsCpu: at(0)
DeviceGenerator->>DeviceGenerator: baseCpuInfo = lsCpu.at(0)
alt [!baseCpuInfo.contains(model name) && dmidecode.contains(Version)]
DeviceGenerator->>dmidecode: [read Version]
DeviceGenerator->>DeviceGenerator: baseCpuInfo.insert(model name, dmidecode[Version])
end
alt [!baseCpuInfo.contains(vendor_id) && dmidecode.contains(Manufacturer)]
DeviceGenerator->>dmidecode: [read Manufacturer]
DeviceGenerator->>DeviceGenerator: baseCpuInfo.insert(vendor_id, dmidecode[Manufacturer])
end
DeviceGenerator->>calAndSetCpuHeaderInfo: calAndSetCpuHeaderInfo(baseCpuInfo, coreNum, logicalNum)
else [lsCpu.size() == 0]
DeviceGenerator-->>DeviceGenerator: [do not call calAndSetCpuHeaderInfo]
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:
- In
generatorCpuDevice, you only fall back to dmidecode values when themodel name/vendor_idkeys are missing; consider also handling the case where the keys exist but contain empty strings so that truly missing values are correctly replaced. - The dynamic
defaultHeightinPageMultiInfo::updateInfois calculated only fromheaderInfo.at(0).size(), but multipleHeaderInfoTableWidgets are added; consider basing the height on the maximum row count among all headers or on the widgets’sizeHint()to better handle multiple CPUs. - The
+ 2adjustment in thedefaultHeightcalculation (headerInfo.at(0).size() * ROW_HEIGHT + 2) is a magic number; consider deriving this from actual margins or constants so that the intent and future adjustments are clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `generatorCpuDevice`, you only fall back to dmidecode values when the `model name`/`vendor_id` keys are missing; consider also handling the case where the keys exist but contain empty strings so that truly missing values are correctly replaced.
- The dynamic `defaultHeight` in `PageMultiInfo::updateInfo` is calculated only from `headerInfo.at(0).size()`, but multiple `HeaderInfoTableWidget`s are added; consider basing the height on the maximum row count among all headers or on the widgets’ `sizeHint()` to better handle multiple CPUs.
- The `+ 2` adjustment in the `defaultHeight` calculation (`headerInfo.at(0).size() * ROW_HEIGHT + 2`) is a magic number; consider deriving this from actual margins or constants so that the intent and future adjustments are clearer.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…r sizing --Use dmidecode Version and Manufacturer as fallback values for missing CPU model name and vendor_id in DeviceGenerator::generatorCpuDevice --Remove hard-coded PageMultiInfo header widget height and compute the height dynamically from row count, improving layout handling for varying header data Log: fix issue Bug: https://pms.uniontech.com/bug-view-363473.html
GongHeng2017
force-pushed
the
202605281613-dev-eagle-fix
branch
from
fa7679a to
4c3487e
Compare
|
Warning
详情 {
"dmidecode": {
"deepin-devicemanager/src/GenerateDevice/DeviceGenerator.cpp": [
" if (!baseCpuInfo.contains(kModelName) && dmidecode.contains(kDmiVersion)) {",
" baseCpuInfo.insert(kModelName, dmidecode[kDmiVersion]); // \u9002\u914d\u7279\u6b8a\u673a\u578bCPU",
" if (!baseCpuInfo.contains(kVendorId) && dmidecode.contains(kDmiManufacturer)) {",
" baseCpuInfo.insert(kVendorId, dmidecode[kDmiManufacturer]); // \u9002\u914d\u7279\u6b8a\u673a\u578bCPU"
]
}
} |
deepin pr auto review你好！我是CodeGeeX，你的智能编程助手。我已仔细审查了你提供的Git Diff代码。这次修改主要涉及两个部分：一是针对特殊机型CPU信息的兼容性适配，二是CPU头部信息展示控件的高度自适应调整。 总体来说，代码的意图清晰，提取字符串常量避免了“魔法字符串”，高度自适应也比写死固定值更合理。但在语法逻辑、代码质量和性能安全方面，还有一些可以改进和优化的空间。以下是详细的审查意见： 一、 语法与逻辑1.
// 修改建议：使用引用，避免深拷贝
if (!lsCpu.isEmpty()) {
QMap<QString, QString> baseCpuInfo = lsCpu.at(0); // 此处仍会拷贝，因为需要insert修改
// 如果不想拷贝，且不想改变原 lsCpu，可以采用以下方式：
// 方式A：直接修改 calAndSetCpuHeaderInfo 接口，让它接收补充信息
// 方式B：如果 lsCpu 允许修改，直接用引用：QMap<QString, QString> &baseCpuInfo = lsCpu[0];
// 当前逻辑下，如果坚持使用拷贝，代码逻辑没问题，但需知晓这里有拷贝开销。
// 优化写法：先判断再拷贝，如果没有缺失，就不需要拷贝和修改
bool needPatch = (!baseCpuInfo.contains(kModelName) && dmidecode.contains(kDmiVersion)) ||
(!baseCpuInfo.contains(kVendorId) && dmidecode.contains(kDmiManufacturer));
QMap<QString, QString> patchedCpuInfo = lsCpu.at(0); // 按需拷贝
if (needPatch) {
if (!patchedCpuInfo.contains(kModelName) && dmidecode.contains(kDmiVersion)) {
patchedCpuInfo.insert(kModelName, dmidecode[kDmiVersion]);
}
if (!patchedCpuInfo.contains(kVendorId) && dmidecode.contains(kDmiManufacturer)) {
patchedCpuInfo.insert(kVendorId, dmidecode[kDmiManufacturer]);
}
}
calAndSetCpuHeaderInfo(patchedCpuInfo, coreNum, logicalNum);
}2. 容器判空逻辑的严谨性
// 修改前
if (lsCpu.size() > 0)
// 修改后
if (!lsCpu.isEmpty())
// 修改前
if (headerInfo.size() > 0 && headerInfo.at(0).size() > 0)
// 修改后
if (!headerInfo.isEmpty() && !headerInfo.at(0).isEmpty())二、 代码质量1. 全局常量的定义规范
// 推荐做法1：如果在 .cpp 内部使用，放入匿名命名空间或加 static
namespace {
const QString kModelName { "model name" };
const QString kVendorId { "vendor_id" };
const QString kDmiVersion { "Version" };
const QString kDmiManufacturer { "Manufacturer" };
}
// 推荐做法2：如果使用 C++17 或更高版本，且字符串不需要隐式共享机制，使用 constexpr (Qt 6 支持，Qt 5 部分支持)
// 或者使用 QStringLiteral 减少运行时内存分配
static const QString kModelName = QStringLiteral("model name");2.
三、 代码性能1. UI 控件的高度计算与布局性能
// 动态获取行高的示例
QFontMetrics fm(this->font());
int lineHeight = fm.height() + some_padding;
defaultHeight = headerInfo.at(0).size() * lineHeight + kAdjustHeight;四、 代码安全1.
if (!baseCpuInfo.contains(kModelName) && dmidecode.contains(kDmiVersion)) {
QString version = dmidecode[kDmiVersion].trimmed(); // 清除首尾空白符
if (!version.isEmpty()) {
baseCpuInfo.insert(kModelName, version);
}
}总结与重构建议综合以上意见，针对 // 常量定义建议放入匿名命名空间或加 static，并使用 QStringLiteral 优化
namespace {
const QString kModelName = QStringLiteral("model name");
const QString kVendorId = QStringLiteral("vendor_id");
const QString kDmiVersion = QStringLiteral("Version");
const QString kDmiManufacturer = QStringLiteral("Manufacturer");
}
void DeviceGenerator::generatorCpuDevice()
{
// ... 前置代码 ...
// 计算并设置CPU头部信息(当前没有多个物理CPU的环境，所以只能编码单物理CPU的逻辑)
if (!lsCpu.isEmpty()) {
QMap<QString, QString> baseCpuInfo = lsCpu.at(0);
bool needPatch = false;
// 检查是否需要补丁，并对 dmidecode 的数据进行清洗
if (!baseCpuInfo.contains(kModelName) && dmidecode.contains(kDmiVersion)) {
QString version = dmidecode[kDmiVersion].trimmed();
if (!version.isEmpty()) {
baseCpuInfo.insert(kModelName, version);
needPatch = true;
}
}
if (!baseCpuInfo.contains(kVendorId) && dmidecode.contains(kDmiManufacturer)) {
QString manufacturer = dmidecode[kDmiManufacturer].trimmed();
if (!manufacturer.isEmpty()) {
baseCpuInfo.insert(kVendorId, manufacturer);
needPatch = true;
}
}
calAndSetCpuHeaderInfo(baseCpuInfo, coreNum, logicalNum);
}
}对于 |
max-lvs
approved these changes
May 28, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: GongHeng2017, max-lvs 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 |
Contributor
Author
|
/forcemerge |
Contributor
|
This pr force merged! (status: unstable) |
deepin-bot
Bot
merged commit 0008d44
into
linuxdeepin:develop/eagle
21 of 22 checks passed
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.
…r sizing
--Use dmidecode Version and Manufacturer as fallback values for
missing CPU model name and vendor_id in DeviceGenerator::generatorCpuDevice
--Remove hard-coded PageMultiInfo header widget height and compute the height dynamically from row count, improving layout handling for varying header data
Log: fix issue
Bug: https://pms.uniontech.com/bug-view-363473.html
Summary by Sourcery
Improve CPU header information generation and make the multi-info page header height adapt to its content.
Bug Fixes:
Enhancements: