fix(cpu): format CPU frequency range with two decimals - #723
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideStandardize CPU frequency display formatting by switching QString::arg(double) calls to fixed two-decimal formatting for GHz and MHz, ensuring consistent frequency ranges in the UI without altering logic or thresholds. 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:
- Since the GHz and MHz formatting logic is now duplicated in multiple branches, consider extracting a small helper (e.g.
formatFrequencyGHz(double)/formatFrequencyMHz(double)) to keep the formatting rules centralized and easier to adjust later. - Using two decimal places for MHz values (e.g.
800.00 MHz) may be unnecessarily verbose compared to the original output; consider using a different precision for MHz (or trimming trailing zeros) while still keeping GHz at two decimals for consistency within ranges.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Since the GHz and MHz formatting logic is now duplicated in multiple branches, consider extracting a small helper (e.g. `formatFrequencyGHz(double)` / `formatFrequencyMHz(double)`) to keep the formatting rules centralized and easier to adjust later.
- Using two decimal places for MHz values (e.g. `800.00 MHz`) may be unnecessarily verbose compared to the original output; consider using a different precision for MHz (or trimming trailing zeros) while still keeping GHz at two decimals for consistency within ranges.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
a8a8c9a to
af278e2
Compare
Use fixed 2-decimal format for CPU GHz/MHz frequency display strings; sync related unit-test assertions to the new format. 将CPU频率(GHz/MHz)显示统一格式化为定点保留两位小数;同步更新相关单测断言。 Log: 统一CPU频率显示为保留两位小数,避免频率范围两端小数位数不一致 PMS: BUG-372847 Influence: 修复兆芯等机型上设备管理器CPU频率范围两端小数位数显示不一致的问题。
af278e2 to
109cf2d
Compare
deepin pr auto review★ 总体评分:100分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // 当前代码已为最佳实践,无需额外改进,以下为确认其正确性的上下文参考
void DeviceCpu::setInfoFromLscpu(const QMap<QString, QString> &mapInfo)
{
// ...
if (fabs(minHz - maxHz) < 0.001) {
m_FrequencyIsRange = false;
m_Frequency = maxHz > 1 ? QString("%1 GHz").arg(maxHz, 0, 'f', 2) : QString("%1 MHz").arg(maxHz * 1000, 0, 'f', 2);
} else {
m_Frequency = QString("%1-%2 GHz").arg(minHz, 0, 'f', 2).arg(maxHz, 0, 'f', 2);
}
// ...
} |
|
[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 |
|
/forcemerge |
根因分析
DeviceCpu::setInfoFromLscpu(deepin-devicemanager/src/DeviceManager/DeviceCpu.cpp:306/308)用QString::arg(double)的默认'g'(6 位有效数字)格式显示由 MHz 换算得到的 GHz 值。兆芯 7000 等机型 lscpu 的CPU min/max MHz为非整数值,换算后两端按"有效数字"输出,小数位数不一致(如1.59997-2.8 GHz),即 bug 所述"频率信息显示不统一"。m_Frequency经概览表"频率"列(:453)与详情"Max Speed"(:406)两处展示,故界面可见。关键证据:
DeviceCpu.cpp:308QString("%1-%2 GHz").arg(minHz).arg(maxHz)—— 默认'g'按有效数字输出,非整数 MHz 时两端小数位数不同。:453/:406——m_Frequency写入概览表"频率"列与详情"Max Speed",即用户所见。'g'6="1.59997" vs'f'2="1.60";2.8 → "2.8" vs "2.80"。修复方案
将三处
arg(double)改为arg(double, 0, 'f', 2)(定点两位小数),频率范围统一为1.60-2.80 GHz,直接消除小数位数不一致的根因。覆盖单值 GHz / 单值 MHz / 范围三种取值,不改变分支逻辑与阈值,改动最小。同步更新既有单测UT_DeviceCpu_setInfoFromLscpu_002/_003两条m_Frequency断言为新格式(0.80-4.20 GHz/800.00 MHz)。改动安全评估
中风险:仅改输出格式与对应单测断言,函数签名不变,无生产调用方回归;历史提交
fe4b2f678a(修复"最大频率不显示单位")的单位后缀被完整保留,不回滚。UT_DeviceCpu_setInfoFromLscpu_002/_003(tests/src/DeviceManager/ut_devicecpu.cpp:266/296)的两条断言已同步更新为新格式,与修复后的生产行为一致;其余单测断言(4085.639 MHz、8300 MHz)走未被改动的分支,无需调整。