Skip to content

Conversation

@add-uos
Copy link
Contributor

@add-uos add-uos commented Jan 19, 2026

…ng convention

  • Rename kCustomType to kSpecialType8
  • Add new kSpecialType9 enum value
  • Update all references throughout codebase to use new enum names
  • Add support for kSpecialType9 in CPU frequency and monitor display logic

log: rename special computer type enums to consistent naming convention
task: https://pms.uniontech.com/task-view-385813.html
Change-Id: Id3da3d2b9ca530992ecddff69beb0da7a8fffcb6

Summary by Sourcery

Standardize special computer type enum names and extend handling for a new special type across device info, CPU, and monitor logic.

New Features:

  • Introduce kSpecialType9 special computer type and handle it in CPU frequency adjustment and display overview formatting.

Enhancements:

  • Rename legacy special computer type enum values to a consistent kSpecialType* naming scheme and update all call sites accordingly.
  • Refine special computer type checks in device monitor, CPU, disk generation, and main startup logic to use the new enum names.

…ng convention

- Rename kCustomType to kSpecialType8
- Add new kSpecialType9 enum value
- Update all references throughout codebase to use new enum names
- Add support for kSpecialType9 in CPU frequency and monitor display logic

log: rename special computer type enums to consistent naming convention
task: https://pms.uniontech.com/task-view-385813.html
Change-Id: Id3da3d2b9ca530992ecddff69beb0da7a8fffcb6
@deepin-ci-robot
Copy link

deepin pr auto review

这段代码主要进行了对特殊计算机类型枚举值的重构,将原本语义不明确的命名(如PGUW、KLVV等)统一替换为具有通用性的命名规范(kSpecialType1-9),并扩展了对新类型(kSpecialType9)的支持。以下是对代码的详细审查和改进意见:

1. 语法逻辑

现状分析:
代码逻辑上主要是一致性的替换和条件判断的扩展。例如,在 DeviceCpu.cppDeviceMonitor.cpp 中,增加了对 kSpecialType9 的判断分支,确保新类型的设备能应用已有的特殊处理逻辑(如频率修正、显示格式化等)。

改进意见:

  1. 条件判断的冗余
    DeviceMonitor.cpp 中,存在大量重复的条件判断组合,例如 kSpecialType5 || kSpecialType6 || kSpecialType7 || kSpecialType9

    • 建议:如果这些类型具有相同的处理逻辑,建议在 Common 类中增加一个辅助判断函数,例如 bool Common::isSpecialDisplayType(),将这些类型封装起来。这样可以减少代码重复,提高可维护性。
  2. Magic Number 替换
    代码中已经将 Common::specialComType == 2 替换为 Common::specialComType == Common::kSpecialType2,这是很好的改进。请确保全库范围内没有遗漏其他直接使用数字比较的地方。

2. 代码质量

现状分析:
通过将枚举值从特定代号(PGUW, KLVV)改为通用序号(kSpecialType1...),代码的可读性在某种程度上降低了(无法直接通过枚举名知道具体代表哪款机器),但提高了通用性和扩展性。

改进意见:

  1. 枚举值的语义化
    虽然使用了 kSpecialType 前缀,但数字后缀依然缺乏语义。如果可能,建议在注释中详细说明每个 Type 对应的具体硬件特征或机型代号,或者保留旧的枚举名作为别名或注释,以便后续维护者查阅。

    • 示例
      enum SpecialComputerType {
          Unknow = -1,
          NormalCom,
          kSpecialType1, // 对应旧版 PGUW
          kSpecialType2, // 对应旧版 KLVV
          ...
      };
  2. 字符串硬编码
    commonfunction.cppcheckBoardVendorFlag 中,依然存在硬编码的字符串(如 "PGUW", "KLVV")。虽然这是为了与硬件标识匹配,但建议将这些字符串定义为常量,并与枚举值对应,防止拼写错误。

3. 代码性能

现状分析:
修改主要涉及逻辑判断和字符串操作,没有引入明显的性能瓶颈。QString::replaceQString::number 等操作在初始化阶段调用,对运行时性能影响极小。

改进意见:

  1. 字符串操作优化
    DeviceCpu.cpp 中:
    m_Frequency = m_Frequency.replace("2.189", "2.188");
    m_MaxFrequency = m_MaxFrequency.replace("2189", "2188");
    这种硬编码的字符串替换看起来是针对特定硬件的“修正补丁”。如果性能敏感,可以考虑使用 QStringRef 或仅在必要时执行,但鉴于这可能在初始化时执行一次,当前性能影响可忽略。

4. 代码安全

现状分析:
代码中使用了大量的条件分支来判断 specialComType。如果 specialComType 未被正确初始化或越界,可能导致逻辑错误。

改进意见:

  1. 枚举越界检查
    在使用 specialComType 进行数组索引或复杂逻辑判断前,建议增加断言或边界检查,确保其值在合法范围内。

    • 示例
      Q_ASSERT(Common::specialComType >= Common::Unknow && Common::specialComType <= Common::kSpecialType9);
  2. 类型安全
    确保 specialComTypeSpecialComputerType 枚举类型,而不是 int。在头文件 commonfunction.h 中,枚举定义是正确的,但在使用时(如 main.cpp)要避免隐式转换。

总结

这次改动主要是为了规范化命名和扩展对新硬件类型的支持,整体方向是正确的。主要的改进点在于:

  1. 消除重复的条件判断,提取公共逻辑函数。
  2. 增加注释以弥补枚举值语义的缺失。
  3. 确保字符串常量的定义统一,避免硬编码。
  4. 增加必要的边界检查以提高健壮性。

代码示例(提取公共逻辑):

// commonfunction.h
class Common {
public:
    // ... 其他代码 ...
    static bool isSpecialDisplayType() {
        int type = specialComType;
        return type == kSpecialType5 || type == kSpecialType6 || 
               type == kSpecialType7 || type == kSpecialType9;
    }
};

// DeviceMonitor.cpp
// 使用前
if (Common::specialComType == Common::kSpecialType5 || Common::specialComType == Common::kSpecialType6 ...)

// 使用后
if (Common::isSpecialDisplayType()) {
    // ...
}

@sourcery-ai
Copy link

sourcery-ai bot commented Jan 19, 2026

Reviewer's Guide

Refactors the SpecialComputerType enum to a consistent kSpecialTypeN naming scheme, replaces all legacy enum usages with the new names, and extends display/CPU logic to treat the new kSpecialType8 and kSpecialType9 variants appropriately.

Updated class diagram for SpecialComputerType enum and its usages

classDiagram
    class Common {
        +SpecialComputerType specialComType
        +QString checkBoardVendorFlag()
        +bool isHwPlatform()
    }

    class SpecialComputerType {
        <<enumeration>>
        Unknow = -1
        NormalCom
        kSpecialType1
        kSpecialType2
        kSpecialType3
        kSpecialType4
        kSpecialType5
        kSpecialType6
        kSpecialType7
        kSpecialType8
        kSpecialType9
    }

    class DeviceMonitor {
        +void setInfoFromHwinfo(QMap_QString_QString mapInfo)
        +TomlFixMethod setInfoFromTomlOneByOne(QMap_QString_QString mapInfo)
        +QString getOverviewInfo()
        +void loadBaseDeviceInfo()
        +void loadOtherDeviceInfo()
        +bool setMainInfoFromXrandr(QString info, QString rateInfo)
    }

    class DeviceCpu {
        +void setCpuInfo(QMap_QString_QString mapLscpu, QMap_QString_QString mapCpuinfo)
    }

    class HWGenerator {
        +void generatorDiskDevice()
    }

    class AppMain {
        +int main(int argc, char argv)
    }

    Common ..> SpecialComputerType : uses
    DeviceMonitor ..> Common : reads_specialComType
    DeviceCpu ..> Common : reads_specialComType
    HWGenerator ..> Common : reads_specialComType
    AppMain ..> Common : reads_specialComType
Loading

Flow diagram for CPU frequency and disk interface logic with SpecialComputerType

flowchart TD
    subgraph CpuFrequencyAdjustment
        A[Start setCpuInfo] --> B[Read Common.specialComType]
        B --> C{Is specialComType in
kSpecialType5,
kSpecialType6,
kSpecialType7,
kSpecialType9?}
        C -- Yes --> D[Replace 2.189 with 2.188 in m_Frequency]
        D --> E[Replace 2189 with 2188 in m_MaxFrequency]
        C -- No --> F[Leave CPU frequencies unchanged]
        E --> G[Continue other CPU info processing]
        F --> G[Continue other CPU info processing]
        G --> H[End setCpuInfo]
    end

    subgraph DiskInterfaceOverride
        I[Start generatorDiskDevice] --> J[Read Common.specialComType]
        J --> K{Is specialComType kSpecialType2?}
        K -- Yes --> L[Set Interface field to UFS]
        K -- No --> M[Keep detected Interface]
        L --> N[Continue disk generation]
        M --> N[Continue disk generation]
        N --> O[End generatorDiskDevice]
    end

    subgraph GpuPreGeneration
        P[Start main] --> Q[Read Common.specialComType]
        Q --> R{Is specialComType kSpecialType8
and no PCI graphics card?}
        R -- Yes --> S[Call preGenerateGpuInfo]
        R -- No --> T[Skip pre generation]
        S --> U[Continue normal startup]
        T --> U[Continue normal startup]
        U --> V[End main startup phase]
    end
Loading

File-Level Changes

Change Details Files
Rename SpecialComputerType enum values to a consistent kSpecialTypeN scheme and introduce kSpecialType8 and kSpecialType9.
  • Replace PGUW/KLVV/KLVU/PGUV with kSpecialType1–kSpecialType4 in the SpecialComputerType enum definition
  • Rename kCustomType to kSpecialType8 and add a new kSpecialType9 enum value
  • Update checkBoardVendorFlag switch cases to use the new enum names and keep existing vendor key strings
  • Update isHwPlatform to treat kSpecialType8 as non-HW platform (same behavior as old kCustomType)
deepin-devicemanager/src/commonfunction.h
deepin-devicemanager/src/commonfunction.cpp
Update all call sites to use the new SpecialComputerType enum names instead of raw integers or old identifiers.
  • Replace literal numeric comparisons (e.g., 1,2,4,5,6,7) with the corresponding Common::kSpecialTypeN constants
  • Replace usages of Common::kCustomType with Common::kSpecialType8 in main and other logic
  • Adjust HWGenerator disk generation logic to test against Common::kSpecialType2 instead of magic value 2
deepin-devicemanager/src/DeviceManager/DeviceMonitor.cpp
deepin-devicemanager/src/GenerateDevice/HWGenerator.cpp
deepin-devicemanager/src/main.cpp
Extend CPU and monitor handling logic to support the new kSpecialType9 variant alongside existing special types.
  • Include Common::kSpecialType9 in DeviceMonitor conditions that control subtitle formatting, base info display suppression, refresh rate handling, and resolution formatting
  • Include Common::kSpecialType9 in DeviceCpu frequency-adjustment condition so its CPU frequency strings are normalized like types 5–7
deepin-devicemanager/src/DeviceManager/DeviceMonitor.cpp
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 - I've left some high level feedback:

  • There are several places now with long chains of specialComType == kSpecialTypeX checks (e.g., in DeviceMonitor and DeviceCpu); consider introducing small helper functions (like isMonitorSpecialType() / isFrequencyAdjustedType()) or a set-based check to centralize this logic and reduce the chance of missing types in the future.
  • Renaming the enum values from domain-specific names (e.g., PGUW, KLVV) to generic kSpecialTypeX reduces readability; if the semantic names are still relevant, consider either keeping them as aliases or adding comments/documentation on what each kSpecialTypeX represents.
  • You’ve added handling for kSpecialType9 in multiple locations, but it’s not included in checkBoardVendorFlag() or other central dispatch points; double-check whether it should participate in those mappings as well so the behavior for the new type is consistently defined.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- There are several places now with long chains of `specialComType == kSpecialTypeX` checks (e.g., in `DeviceMonitor` and `DeviceCpu`); consider introducing small helper functions (like `isMonitorSpecialType()` / `isFrequencyAdjustedType()`) or a set-based check to centralize this logic and reduce the chance of missing types in the future.
- Renaming the enum values from domain-specific names (e.g., `PGUW`, `KLVV`) to generic `kSpecialTypeX` reduces readability; if the semantic names are still relevant, consider either keeping them as aliases or adding comments/documentation on what each `kSpecialTypeX` represents.
- You’ve added handling for `kSpecialType9` in multiple locations, but it’s not included in `checkBoardVendorFlag()` or other central dispatch points; double-check whether it should participate in those mappings as well so the behavior for the new type is consistently defined.

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.

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: add-uos, 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

@add-uos
Copy link
Contributor Author

add-uos commented Jan 19, 2026

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Jan 19, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit 6db8731 into linuxdeepin:develop/eagle Jan 19, 2026
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