Skip to content

Conversation

@add-uos
Copy link
Contributor

@add-uos add-uos commented Jan 22, 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

pick from: 6db8731

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

Unify special computer type enums under a consistent naming scheme and extend special-type handling across device monitoring and CPU logic.

New Features:

  • Introduce kSpecialType9 as a new special computer type with dedicated handling in monitor and CPU logic.

Enhancements:

  • Rename legacy special computer type enum values (including the former kCustomType) to a consistent kSpecialTypeN naming convention and update all call sites accordingly.
  • Extend monitor display and resolution handling paths to support the new kSpecialType9 alongside existing special computer types.
  • Align CPU frequency adjustment logic so that kSpecialType9 receives the same special-case treatment as other affected special computer types.
  • Refine board vendor key mapping and hardware platform detection to use the new enum names, keeping behavior consistent with previous mappings.

…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

pick from: linuxdeepin@6db8731

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

sourcery-ai bot commented Jan 22, 2026

Reviewer's Guide

Refactors the SpecialComputerType enum to a consistent kSpecialType* naming scheme, introduces kSpecialType8 and kSpecialType9, and updates CPU, monitor, disk, and startup logic to handle the new names and the new special type behavior.

Class diagram for Common::SpecialComputerType refactor and usages

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

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

    class DeviceMonitor {
        +void setInfoFromHwinfo(mapInfo)
        +TomlFixMethod setInfoFromTomlOneByOne(mapInfo)
        +QString getOverviewInfo()
        +void loadBaseDeviceInfo()
        +void loadOtherDeviceInfo()
        +bool setMainInfoFromXrandr(info, rate)
        -QString m_Name
        -QString m_Vendor
        -QString m_Model
        -QString m_DisplayInput
        -QString m_Interface
        -QString m_ScreenSize
        -QString m_CurrentResolution
        -QString m_AspectRatio
        -QString m_RefreshRate
    }

    class DeviceCpu {
        +void setCpuInfo(mapLscpu, mapCpuinfo, mapProduct)
        -QString m_Frequency
        -QString m_MaxFrequency
    }

    class HWGenerator {
        +void generatorDiskDevice()
    }

    class MainApplication {
        +int main(argc, argv)
    }

    Common o-- SpecialComputerType : defines
    DeviceMonitor ..> Common : uses_specialComType
    DeviceCpu ..> Common : uses_specialComType
    HWGenerator ..> Common : uses_specialComType
    MainApplication ..> Common : uses_specialComType
Loading

Flow diagram for specialComType-dependent behavior across components

flowchart TD
    A["Application start"] --> B["Read Common::specialComType"]

    B --> C{"specialComType in {Unknow, NormalCom, kSpecialType8}?"}
    C -- "yes" --> D["Common::isHwPlatform returns false"]
    C -- "no" --> E["Common::isHwPlatform returns true"]

    B --> F{"specialComType == kSpecialType2?"}
    F -- "yes" --> G["DeviceMonitor::setInfoFromTomlOneByOne sets m_IsTomlSet"]
    F -- "no" --> H["Skip TOML flag for monitor"]

    B --> I{"specialComType in {kSpecialType6, kSpecialType7, kSpecialType9}?"}
    I -- "yes" --> J["DeviceMonitor::getOverviewInfo shows size only"]
    I -- "no" --> K["DeviceMonitor::getOverviewInfo shows name and size"]

    B --> L{"specialComType in {kSpecialType5, kSpecialType6, kSpecialType7, kSpecialType9}?"}
    L -- "yes" --> M["DeviceMonitor::loadBaseDeviceInfo hides basic fields"]
    L -- "no" --> N["DeviceMonitor::loadBaseDeviceInfo shows name, vendor, type, input"]

    B --> O{"specialComType == kSpecialType4?"}
    O -- "yes" --> P["DeviceMonitor::loadOtherDeviceInfo parses refresh rate from CurrentResolution"]
    O -- "no" --> Q["Standard refresh rate handling"]

    B --> R{"specialComType in {kSpecialType1, kSpecialType5, kSpecialType6, kSpecialType7, kSpecialType9}?"}
    R -- "yes" --> S["DeviceMonitor::setMainInfoFromXrandr sets m_RefreshRate and custom m_CurrentResolution"]
    R -- "no" --> T["DeviceMonitor::setMainInfoFromXrandr sets m_CurrentResolution with rate suffix"]

    B --> U{"specialComType in {kSpecialType5, kSpecialType6, kSpecialType7, kSpecialType9}?"}
    U -- "yes" --> V["DeviceCpu::setCpuInfo adjusts m_Frequency and m_MaxFrequency"]
    U -- "no" --> W["DeviceCpu::setCpuInfo keeps default frequencies"]

    B --> X{"specialComType == kSpecialType2?"}
    X -- "yes" --> Y["HWGenerator::generatorDiskDevice sets disk Interface to UFS"]
    X -- "no" --> Z["HWGenerator uses detected disk Interface"]

    B --> AA{"specialComType == kSpecialType8?"}
    AA -- "yes" --> AB["main.cpp pre-caches GPU info via CommonTools::preGenerateGpuInfo"]
    AA -- "no" --> AC["No GPU pre-generation on startup"]

    D --> AD["End platform-specific behavior"]
    E --> AD
    S --> AD
    T --> AD
    V --> AD
    W --> AD
    Y --> AD
    Z --> AD
    AB --> AD
    AC --> AD
Loading

File-Level Changes

Change Details Files
Refactor SpecialComputerType enum to consistent kSpecialType* names and extend it with a new special type.
  • Rename legacy enum values PGUW/KLVV/KLVU/PGUV to kSpecialType1–4 for consistency with existing kSpecialType5–7 values.
  • Rename kCustomType to kSpecialType8 and add a new kSpecialType9 value to the enum.
  • Adjust Common::checkBoardVendorFlag switch cases to map new enum names to the existing board vendor keys.
  • Update Common::isHwPlatform logic so the non-hardware platform case now checks kSpecialType8 instead of kCustomType.
deepin-devicemanager/src/commonfunction.h
deepin-devicemanager/src/commonfunction.cpp
Update device monitor logic to use the renamed enum constants and support the new kSpecialType9 behavior.
  • Replace raw numeric comparisons of Common::specialComType (1,2,4,5,6,7) with comparisons against the new enum constants kSpecialType1–7.
  • Include kSpecialType7 and kSpecialType9 in the cases that hide base monitor fields like Name/Vendor/Type/Display Input.
  • Include kSpecialType9 in the cases where the overview string omits the monitor name and only shows screen size.
  • Extend refresh-rate and resolution handling so kSpecialType5/6/7/9 share the same specialized formatting rules as previously applied to some special types.
  • Minorly wrap some string literals in extra parentheses when calling addBaseDeviceInfo/addOtherDeviceInfo, without changing semantics.
deepin-devicemanager/src/DeviceManager/DeviceMonitor.cpp
Extend CPU frequency adjustment to the new special type and align with enum renames.
  • Add kSpecialType9 to the set of specialComType values that trigger the custom CPU frequency and max frequency string replacements.
  • Keep existing behavior for kSpecialType5–7 intact while using the new enum constants.
deepin-devicemanager/src/DeviceManager/DeviceCpu.cpp
Adjust hardware generation and startup paths to use the new enum constants.
  • Update HWGenerator disk device logic to check Common::kSpecialType2 instead of the hard-coded value 2 when setting UFS interface.
  • Update main() to use kSpecialType8 instead of kCustomType when deciding whether to pre-generate GPU information for special models.
deepin-devicemanager/src/GenerateDevice/HWGenerator.cpp
deepin-devicemanager/src/main.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 found 1 issue, and left some high level feedback:

  • There are several places (e.g., DeviceMonitor::loadBaseDeviceInfo/loadOtherDeviceInfo) where string literals were wrapped in extra parentheses ("Name" → ("Name")); these add no benefit and reduce consistency with surrounding code, so consider reverting to the simpler form.
  • Multiple conditions now repeat long chains of specialComType comparisons (e.g., in DeviceMonitor::getOverviewInfo, loadBaseDeviceInfo, setMainInfoFromXrandr, and DeviceCpu::setCpuInfo); consider introducing small helper predicates (e.g., isNoNameMonitorType(), isAdjustedRefreshRateType()) to centralize and simplify this logic.
  • Since the enum values were renamed to generic kSpecialTypeN identifiers while still encoding specific vendor/model semantics (PGUW, KLVV, etc.), it may be helpful to document or centralize the mapping between each kSpecialTypeN and its underlying meaning to keep the codebase understandable over time.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- There are several places (e.g., DeviceMonitor::loadBaseDeviceInfo/loadOtherDeviceInfo) where string literals were wrapped in extra parentheses ("Name" → ("Name")); these add no benefit and reduce consistency with surrounding code, so consider reverting to the simpler form.
- Multiple conditions now repeat long chains of `specialComType` comparisons (e.g., in DeviceMonitor::getOverviewInfo, loadBaseDeviceInfo, setMainInfoFromXrandr, and DeviceCpu::setCpuInfo); consider introducing small helper predicates (e.g., `isNoNameMonitorType()`, `isAdjustedRefreshRateType()`) to centralize and simplify this logic.
- Since the enum values were renamed to generic `kSpecialTypeN` identifiers while still encoding specific vendor/model semantics (PGUW, KLVV, etc.), it may be helpful to document or centralize the mapping between each `kSpecialTypeN` and its underlying meaning to keep the codebase understandable over time.

## Individual Comments

### Comment 1
<location> `deepin-devicemanager/src/commonfunction.cpp:172` </location>
<code_context>
 QString Common::checkBoardVendorFlag()
</code_context>

<issue_to_address>
**suggestion:** The mapping from `SpecialComputerType` to `boardVendorKey` doesn’t cover all enum values; verify that the default case is correct for the new types.

In `checkBoardVendorFlag`, `NormalCom`, `kSpecialType1..5`, and `kSpecialType8` are handled explicitly, but `kSpecialType6`, `kSpecialType7`, and `kSpecialType9` fall through to the default (empty key). Since `kSpecialType9` is used in device-specific branches elsewhere, consider adding explicit cases (or clearly documenting that these share the default behavior) to avoid unintended configuration differences.
</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.

boardVendorKey = "PGUX";
break;
case kCustomType:
case kSpecialType8:
Copy link

Choose a reason for hiding this comment

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

suggestion: The mapping from SpecialComputerType to boardVendorKey doesn’t cover all enum values; verify that the default case is correct for the new types.

In checkBoardVendorFlag, NormalCom, kSpecialType1..5, and kSpecialType8 are handled explicitly, but kSpecialType6, kSpecialType7, and kSpecialType9 fall through to the default (empty key). Since kSpecialType9 is used in device-specific branches elsewhere, consider adding explicit cases (or clearly documenting that these share the default behavior) to avoid unintended configuration differences.

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码主要进行了一次重构,将硬编码的数字类型标识符(如 PGUWKLVV 等)替换为更具语义化的枚举值(如 kSpecialType1kSpecialType2 等),并扩展了对新机型(kSpecialType9)的支持。以下是对这段代码的详细审查意见:

1. 语法与逻辑审查

  • 代码逻辑一致性

    • 代码逻辑总体上是一致的。在 commonfunction.h 中修改了枚举定义,并在所有使用 Common::specialComType 的地方进行了相应的替换。
    • 新增了 kSpecialType9,并在 DeviceCpu.cppDeviceMonitor.cpp 中添加了相应的处理逻辑,确保了新机型的兼容性。
  • 拼写错误

    • commonfunction.h 中,枚举的第一个值 Unknow 拼写有误,标准的英文拼写应为 Unknown。虽然这只是一个枚举值,但为了代码的专业性,建议修正。

2. 代码质量审查

  • 命名规范改进

    • 优点:将 PGUW, KLVV 等缩写替换为 kSpecialType1 等通用命名是一个好的方向。这消除了"魔法数字"(Magic Numbers)和晦涩的缩写,使得代码对于新加入的开发者来说更易读。
    • 缺点:虽然 kSpecialType1 比数字好,但它仍然丢失了具体的业务含义(例如 PGUW 可能代表某个特定的主板厂商或机型系列)。如果这些类型在代码注释或文档中有详细说明,尚可接受;否则,这种命名方式可能会导致后续维护时难以区分 Type 1 和 Type 2 的具体区别。
    • 建议:如果可能,枚举值应尽量描述其代表的实体特征,例如 kVendorPGUWkModelKLVV,或者保持 kSpecialTypeX 的命名,但务必在枚举定义处添加详细的注释,说明每种类型对应的硬件特征。
  • 冗余的括号

    • DeviceMonitor.cpploadBaseDeviceInfoloadOtherDeviceInfo 方法中,出现了 addBaseDeviceInfo(("Name"), m_Name); 这样的写法。
    • 字符串字面量外层的额外括号 ("Name") 是完全多余的,虽然编译器不会报错,但这降低了代码的可读性,显得不够整洁。建议删除多余的括号,改为 addBaseDeviceInfo("Name", m_Name);

3. 代码性能审查

  • 性能影响
    • 本次改动主要涉及枚举值的比较和字符串的替换。
    • 枚举值的比较(如 Common::specialComType == Common::kSpecialType9)是整数比较,性能极高,没有问题。
    • 字符串操作(如 replace)在原有逻辑中就存在,本次改动未增加额外的性能负担。

4. 代码安全审查

  • 全局静态变量的使用

    • 代码中大量使用了 Common::specialComType。这通常是一个静态全局变量或单例成员。
    • 潜在风险:如果这个变量在多线程环境下被写入或读取,且没有适当的锁保护(例如互斥锁),可能会导致数据竞争。
    • 建议:检查 Common::specialComType 的初始化时机和后续访问。如果它只在程序启动时设置一次,后续只读,则是安全的。如果在运行时可能发生变化,则需要考虑线程安全措施。
  • 硬编码字符串替换

    • DeviceCpu.cpp 中:
      m_Frequency = m_Frequency.replace("2.189", "2.188");
    • 这种针对特定频率值的硬编码替换显得非常脆弱。如果硬件批次更新导致频率变为 2.190 或 2.187,这段代码就会失效。
    • 建议:这种修正逻辑通常是为了修复BIOS或硬件报告的已知错误。建议将这种特定的修正规则封装在更通用的配置文件(如之前的 TOML)中,或者至少添加详细的注释说明为什么要修正这个特定的值,以及它对应的硬件 Bug ID 或厂商说明。

总结与改进建议代码示例

建议修改 commonfunction.h 中的枚举定义,增加注释:

enum SpecialComputerType {
    Unknown = -1,  // 修正拼写
    NormalCom,     // 普通机型
    kSpecialType1, // 对应原 PGUW (厂商A)
    kSpecialType2, // 对应原 KLVV (厂商B - UFS接口)
    kSpecialType3, // 对应原 KLVU
    kSpecialType4, // 对应原 PGUV
    kSpecialType5, // 对应原 PGUX
    kSpecialType6,
    kSpecialType7,
    kSpecialType8, // 对应原 kCustomType
    kSpecialType9  // 新增机型
};

建议修改 DeviceMonitor.cpp 中的冗余括号:

// 修改前
addBaseDeviceInfo(("Name"), m_Name);

// 修改后
addBaseDeviceInfo("Name", m_Name);

关于 DeviceCpu.cpp 中的硬编码替换:
虽然不改也能运行,但建议添加注释解释原因:

// 修正特定机型(KLVV/PGUW等)BIOS报告频率偏差的问题: 2.189GHz 实际应为 2.188GHz
if (Common::specialComType == Common::kSpecialType2 || ...) {
    m_Frequency = m_Frequency.replace("2.189", "2.188");
    // ...
}

总体而言,这次提交提高了代码的可读性和可维护性,但在命名语义化和硬编码处理方面还有进一步优化的空间。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: add-uos, lzwind

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

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