Skip to content

Conversation

@GongHeng2017
Copy link
Contributor

@GongHeng2017 GongHeng2017 commented Dec 10, 2025

-- Add max boost clock item on special cpu type.

Log: add feature
Task: https://pms.uniontech.com/task-view-384577.html

Summary by Sourcery

Add support for special CPU types and display their maximum boost clock in the device manager UI.

New Features:

  • Read a configurable special CPU type from DConfig to distinguish custom CPU handling.
  • Display a Max Boost Clock field for designated special CPU types using dmidecode data in the CPU information table.

Enhancements:

  • Consolidate DConfig key validation logic for specialComType and TomlFilesName and extend it to specialCpuType.

-- Add max boost clock item on special cpu type.

Log: add feature
Task: https://pms.uniontech.com/task-view-384577.html
@deepin-ci-robot
Copy link

deepin pr auto review

我来对这段代码进行审查:

  1. 代码逻辑审查:
  • 添加了特殊CPU类型的支持,通过DConfig配置文件来控制
  • 在DeviceCpu类中增加了最大加速频率的显示功能
  • 代码整体逻辑清晰,新增功能与现有代码集成良好
  1. 代码质量建议:
  • 在commonfunction.h中,枚举类型命名可以更规范:
enum SpecialCpuType {
    kUnknownCpuType = 0,  // "Unknow"拼写错误,应为"Unknown"
    kSpecialCpuType1      // 建议使用更具描述性的名称,如kTurboBoostCpuType
};
  • 在DeviceCpu.cpp中,条件判断可以提取为常量:
static const bool IS_SPECIAL_CPU_TYPE = (Common::curCpuType == Common::kSpecialCpuType1);
  1. 性能优化建议:
  • 多次使用Common::curCpuType == Common::kSpecialCpuType判断,可以将其缓存为局部变量
  • 在loadTableHeader和loadTableData中的重复判断可以合并
  1. 安全性建议:
  • DConfig值的类型转换缺少错误处理:
if (dconfig->keyList().contains("specialCpuType")) {
    bool ok;
    int value = dconfig->value("specialCpuType").toInt(&ok);
    if (ok) {
        Common::curCpuType = static_cast<Common::SpecialCpuType>(value);
    }
}
  1. 其他建议:
  • JSON配置文件中的description有拼写错误:"Unknow"应为"Unknown"
  • 建议为新增的配置项添加默认值处理
  • 考虑添加配置值的有效性检查,防止非法值导致程序异常
  1. 代码风格建议:
  • 建议统一使用驼峰命名法
  • 注释风格保持一致
  • 建议为新增的公共接口添加详细的文档说明

总体来说,代码实现合理,但需要在细节处理上进一步完善,特别是在错误处理和命名规范方面。

@sourcery-ai
Copy link

sourcery-ai bot commented Dec 10, 2025

Reviewer's Guide

Adds support for a configurable special CPU type so that, for a specific CPU type, the device manager reads and displays a Max Boost Clock value obtained from dmidecode, wiring the configuration through DConfig into the CPU view model and table rendering.

Sequence diagram for configuring specialCpuType and displaying Max Boost Clock

sequenceDiagram
    actor User
    participant AppMain
    participant DConfig
    participant Common
    participant DeviceCpu

    User ->> AppMain: startApplication()
    AppMain ->> DConfig: create(org.deepin.devicemanager)
    AppMain ->> DConfig: isValid()
    alt DConfig valid
        AppMain ->> DConfig: keyList()
        AppMain ->> DConfig: value(specialCpuType)
        AppMain ->> Common: set curCpuType(static_cast SpecialCpuType)
    end

    AppMain ->> DeviceCpu: createAndInit()
    AppMain ->> DeviceCpu: setInfoFromDmidecode(mapInfo)
    DeviceCpu ->> Common: read curCpuType
    alt curCpuType == kSpecialCpuType1
        DeviceCpu ->> DeviceCpu: setAttribute(mapInfo, Max Speed, m_MaxBoostClock)
    end

    AppMain ->> DeviceCpu: loadTableHeader()
    DeviceCpu ->> Common: read curCpuType
    alt curCpuType == kSpecialCpuType1
        DeviceCpu ->> DeviceCpu: append Max Boost Clock to m_TableHeader
    end

    AppMain ->> DeviceCpu: loadTableData()
    DeviceCpu ->> Common: read curCpuType
    alt curCpuType == kSpecialCpuType1
        DeviceCpu ->> DeviceCpu: append m_MaxBoostClock to m_TableData
    end
Loading

Class diagram for Common and DeviceCpu CPU type and Max Boost Clock support

classDiagram
    class Common {
        <<utility>>
        +static int specialComType
        +static SpecialCpuType curCpuType
        +static QString getArch()
        +static QString getArchStore()
        +static QByteArray executeClientCmd(QString cmd, QStringList args, QString workPath, int msecsWaiting)
    }

    class SpecialType {
        <<enumeration>>
        kSpecialType1
        kSpecialType2
        kSpecialType3
        kSpecialType4
        kSpecialType5
        kSpecialType6
        kSpecialType7
        kCustomType
    }

    class SpecialCpuType {
        <<enumeration>>
        kUnknowCpuType = 0
        kSpecialCpuType1
    }

    class DeviceCpu {
        +void setInfoFromDmidecode(QMap~QString, QString~ mapInfo)
        +void loadTableHeader()
        +void loadTableData()
        -QString m_Name
        -QString m_Vendor
        -QString m_Frequency
        -QString m_Architecture
        -bool m_FrequencyIsCur
        -QMap~int, QString~ m_trNumber
        -QString m_MaxBoostClock
    }

    class QString
    class QMap

    Common --> SpecialCpuType : uses
    Common --> SpecialType : uses
    DeviceCpu --> Common : reads curCpuType
    DeviceCpu --> QString : uses
    DeviceCpu --> QMap : uses
Loading

File-Level Changes

Change Details Files
Wire special CPU type configuration from DConfig into global state for conditional behavior.
  • Refactor DConfig initialization to check validity once before accessing keys.
  • Read optional "specialComType" and "TomlFilesName" keys only if present.
  • Introduce reading of a new "specialCpuType" integer key and store it as Common::curCpuType using the SpecialCpuType enum.
deepin-devicemanager/src/main.cpp
Introduce special-CPU-specific handling to capture and display a Max Boost Clock value from dmidecode.
  • Extend Common with a SpecialCpuType enum and a curCpuType static field, initialized to kUnknowCpuType.
  • Add a m_MaxBoostClock member on DeviceCpu for storing the maximum boost frequency.
  • When curCpuType is kSpecialCpuType1, read "Max Speed" from dmidecode into m_MaxBoostClock in setInfoFromDmidecode.
  • Conditionally append a "Max Boost Clock" column header and corresponding data cell when curCpuType is kSpecialCpuType1, ensuring table shape stays consistent for other CPU types.
deepin-devicemanager/src/commonfunction.h
deepin-devicemanager/src/commonfunction.cpp
deepin-devicemanager/src/DeviceManager/DeviceCpu.h
deepin-devicemanager/src/DeviceManager/DeviceCpu.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes - here's some feedback:

  • When reading specialCpuType from DConfig, consider validating or clamping the integer value before static_cast to Common::SpecialCpuType so that unexpected config values don’t silently map to an unintended enum case.
  • In main.cpp the DConfig key lookup calls dconfig->keyList() three times; caching the key list (or using a helper to query keys) would make the logic cheaper and easier to extend as more keys are added.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- When reading `specialCpuType` from DConfig, consider validating or clamping the integer value before `static_cast` to `Common::SpecialCpuType` so that unexpected config values don’t silently map to an unintended enum case.
- In `main.cpp` the DConfig key lookup calls `dconfig->keyList()` three times; caching the key list (or using a helper to query keys) would make the logic cheaper and easier to extend as more keys are added.

## Individual Comments

### Comment 1
<location> `deepin-devicemanager/src/commonfunction.h:32` </location>
<code_context>
     };
+
+    enum SpecialCpuType {
+        kUnknowCpuType = 0,
+        kSpecialCpuType1
+    };
</code_context>

<issue_to_address>
**issue (typo):** Fix the spelling of `kUnknowCpuType` to `kUnknownCpuType` for clarity.

If you rename this enum value, make sure all references are updated consistently, including the initialization in `commonfunction.cpp`.

Suggested implementation:

```c
    enum SpecialCpuType {
        kUnknownCpuType = 0,
        kSpecialCpuType1
    };

```

You should also:
1. Search in `deepin-devicemanager/src` (especially `commonfunction.cpp`) for all occurrences of `kUnknowCpuType` and rename them to `kUnknownCpuType`.
2. Rebuild to ensure there are no remaining references to the old identifier.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

};

enum SpecialCpuType {
kUnknowCpuType = 0,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (typo): Fix the spelling of kUnknowCpuType to kUnknownCpuType for clarity.

If you rename this enum value, make sure all references are updated consistently, including the initialization in commonfunction.cpp.

Suggested implementation:

    enum SpecialCpuType {
        kUnknownCpuType = 0,
        kSpecialCpuType1
    };

You should also:

  1. Search in deepin-devicemanager/src (especially commonfunction.cpp) for all occurrences of kUnknowCpuType and rename them to kUnknownCpuType.
  2. Rebuild to ensure there are no remaining references to the old identifier.

@deepin-ci-robot
Copy link

[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.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@GongHeng2017
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Dec 10, 2025

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit 6924401 into linuxdeepin:develop/eagle Dec 10, 2025
16 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants