chore: extend flmx mode of device#557
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom Nov 28, 2025
Merged
Conversation
extend flmx mode of device Log: extend flmx mode of device Bug: https://pms.uniontech.com/task-view-368603.html
Reviewer's GuideExtends special device mode handling (via Common::specialComType) for storage and memory views, hides vendor info in that mode, unifies several platform-specific generators to HWGenerator, and removes obsolete monitor debugging scaffolding, with corresponding translation updates. Sequence diagram for PageMultiInfo.updateInfo with specialComType handlingsequenceDiagram
actor User
participant PageMultiInfo
participant Common
participant QLabel
participant DeviceTable
participant DeviceDetail
User->>PageMultiInfo: updateInfo(lst)
PageMultiInfo->>PageMultiInfo: getTableListInfo(lst, m_deviceList, m_menuControlList)
PageMultiInfo->>DeviceTable: setVisible(true)
PageMultiInfo->>DeviceTable: setFixedHeight(TABLE_HEIGHT)
PageMultiInfo->>DeviceTable: updateTable(m_deviceList, m_menuControlList)
PageMultiInfo->>Common: read specialComType
alt specialComType >= 1
PageMultiInfo->>QLabel: text()
alt text is Storage or Memory
PageMultiInfo->>DeviceTable: setVisible(false)
PageMultiInfo->>DeviceTable: setFixedHeight(0)
end
end
PageMultiInfo->>DeviceDetail: showDeviceInfo(lst)
PageMultiInfo-->>User: UI updated
Class diagram for updated DeviceFactory generator selectionclassDiagram
class DeviceFactory {
+getDeviceGenerator() DeviceGenerator*
}
class DeviceGenerator {
<<interface>>
}
class HWGenerator {
+generatorMonitorDevice() void
}
class X86Generator {
}
class MipsGenerator {
}
class ArmGenerator {
}
DeviceFactory ..> DeviceGenerator : creates
DeviceGenerator <|-- HWGenerator
DeviceGenerator <|-- X86Generator
DeviceGenerator <|-- MipsGenerator
DeviceGenerator <|-- ArmGenerator
class Common {
+specialComType int
}
DeviceFactory ..> Common : reads type
%% PGUV, PGUW, PGUX and other types now map to HWGenerator instead of
%% platform specific generators that were removed in this PR.
Class diagram for PageMultiInfo and special mode table handlingclassDiagram
class PageMultiInfo {
-mp_Table DeviceTable*
-mp_Label QLabel*
-mp_Detail DeviceDetail*
-m_deviceList QList~DeviceBaseInfo*~
-m_menuControlList QList~MenuControlInfo~
+updateInfo(lst QList~DeviceBaseInfo*~) void
+resizeEvent(e QResizeEvent*) void
}
class DeviceTable {
+setVisible(visible bool) void
+setFixedHeight(height int) void
+updateTable(deviceList QList~DeviceBaseInfo*~, menuControlList QList~MenuControlInfo~, limitRows bool, rowCount int) void
+updateTable(deviceList QList~DeviceBaseInfo*~, menuControlList QList~MenuControlInfo~) void
}
class DeviceDetail {
+showDeviceInfo(lst QList~DeviceBaseInfo*~) void
}
class QLabel {
+text() QString
}
class Common {
+specialComType int
}
PageMultiInfo o--> DeviceTable : mp_Table
PageMultiInfo o--> DeviceDetail : mp_Detail
PageMultiInfo o--> QLabel : mp_Label
PageMultiInfo ..> Common : checks specialComType
%% In specialComType >= 1 and when label text is Storage or Memory,
%% PageMultiInfo hides mp_Table and sets its height to 0.
Class diagram for DeviceMemory and DeviceStorage base info loading in special modeclassDiagram
class DeviceInfo {
+addBaseDeviceInfo(key QString, value QString) void
}
class DeviceMemory {
-m_Name QString
-m_Vendor QString
-m_Size QString
-m_Type QString
-m_Speed QString
+loadBaseDeviceInfo() void
}
class DeviceStorage {
-m_Name QString
-m_Vendor QString
-m_MediaType QString
-m_Size QString
-m_Version QString
+loadBaseDeviceInfo() void
+translateStr(value QString) QString
}
class Common {
+specialComType int
}
DeviceInfo <|-- DeviceMemory
DeviceInfo <|-- DeviceStorage
DeviceMemory ..> Common : guards Vendor field
DeviceStorage ..> Common : guards Vendor field
%% loadBaseDeviceInfo now only adds Vendor when specialComType <= 0.
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review我来对这次代码变更进行详细审查：
改进建议：
总的来说，这次代码修改质量较高，提高了代码的复用性和可维护性，同时保持了良好的性能和安全性。 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The repeated checks
if (Common::specialComType ... && mp_Label->text() == tr("Storage")/tr("Memory"))inPageMultiInfomake the logic fragile and translation-dependent; consider introducing a dedicated enum or type flag on the page/device instead of comparing translated label text in multiple places. - In
DeviceFactory::getDeviceGenerator, the log messages still mentionPanguVGenerator/PanguXGeneratoreven thoughHWGeneratoris now created; update the debug output to reflect the actual generator being instantiated to avoid confusion during debugging. - The conditional hiding of memory/storage vendor information via
Common::specialComTypeis scattered across multiple classes; consider encapsulating this policy in a single helper (e.g., a function onCommonor a utility) to avoid repeating the same condition and to keep behavior consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The repeated checks `if (Common::specialComType ... && mp_Label->text() == tr("Storage")/tr("Memory"))` in `PageMultiInfo` make the logic fragile and translation-dependent; consider introducing a dedicated enum or type flag on the page/device instead of comparing translated label text in multiple places.
- In `DeviceFactory::getDeviceGenerator`, the log messages still mention `PanguVGenerator`/`PanguXGenerator` even though `HWGenerator` is now created; update the debug output to reflect the actual generator being instantiated to avoid confusion during debugging.
- The conditional hiding of memory/storage vendor information via `Common::specialComType` is scattered across multiple classes; consider encapsulating this policy in a single helper (e.g., a function on `Common` or a utility) to avoid repeating the same condition and to keep behavior consistent.
## Individual Comments
### Comment 1
<location> `deepin-devicemanager/src/Page/PageMultiInfo.cpp:102-103` </location>
<code_context>
+ mp_Table->setVisible(true);
+ mp_Table->setFixedHeight(TABLE_HEIGHT);
mp_Table->updateTable(m_deviceList, m_menuControlList);
+ if (Common::specialComType >= 1) {
+ if (mp_Label->text() == tr("Storage") || mp_Label->text() == tr("Memory")) {
+ mp_Table->setVisible(false);
+ mp_Table->setFixedHeight(0);
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Avoid using translated label text to drive logic; consider a more robust device-type check.
This logic depends on `mp_Label->text() == tr("Storage")/tr("Memory")` to control table visibility. Using UI text for behavior is brittle and can break with translation or wording changes. Prefer an explicit device type signal (e.g., enum or internal ID on the page) so you’re not relying on localized strings or duplicating these comparisons elsewhere.
Suggested implementation:
```cpp
// 更新表格
mp_Table->setVisible(true);
mp_Table->setFixedHeight(TABLE_HEIGHT);
mp_Table->updateTable(m_deviceList, m_menuControlList);
if (Common::specialComType >= 1) {
// 使用设备类型判断是否隐藏表格，避免依赖翻译文本
if (shouldHideTableForDeviceType()) {
mp_Table->setVisible(false);
mp_Table->setFixedHeight(0);
}
}
```
To fully remove the dependency on localized label text, you’ll need to:
1. **Add an explicit device type to the page (if not present already)**
- In the corresponding header file (likely `PageMultiInfo.h`), introduce an enum describing device types, for example:
```cpp
enum class DeviceType {
Unknown,
Storage,
Memory,
// ... other types
};
```
- Add a member to `PageMultiInfo`:
```cpp
DeviceType m_deviceType = DeviceType::Unknown;
```
2. **Provide a helper method used above**
- In `PageMultiInfo.h` declare:
```cpp
bool shouldHideTableForDeviceType() const;
```
- In `PageMultiInfo.cpp` implement:
```cpp
bool PageMultiInfo::shouldHideTableForDeviceType() const
{
switch (m_deviceType) {
case DeviceType::Storage:
case DeviceType::Memory:
return true;
default:
return false;
}
}
```
3. **Initialize `m_deviceType` based on non-UI data**
- Wherever this page is constructed or updated (likely in the constructor or a `setDeviceInfo`/`updatePage`-style method that receives `lst` or device metadata), set `m_deviceType` from a stable identifier (e.g., a field in `lst`, `m_deviceList`, or `m_menuControlList`, or a higher-level page ID), *not* from `mp_Label->text()` or any translated string:
```cpp
// Example sketch – adapt to your real model:
if (deviceCategoryFromModel == DeviceCategory::Storage)
m_deviceType = DeviceType::Storage;
else if (deviceCategoryFromModel == DeviceCategory::Memory)
m_deviceType = DeviceType::Memory;
else
m_deviceType = DeviceType::Unknown;
```
4. **(Optional) Remove any remaining logic that uses `mp_Label->text()` as a behavior driver**
- Search the codebase for usages like `mp_Label->text() == tr("Storage")` or `tr("Memory")` checks and replace them with `m_deviceType` / `DeviceType`-based checks, so behavior is no longer coupled to UI strings.
</issue_to_address>
### Comment 2
<location> `deepin-devicemanager/src/GenerateDevice/DeviceFactory.cpp:45` </location>
<code_context>
generator = new HWGenerator();
} else if (type == "PGUV" || type == "PGUW") {
qCDebug(appLog) << "DeviceFactory::getDeviceGenerator create PanguVGenerator";
</code_context>
<issue_to_address>
**issue:** Debug log messages no longer match the actual generator type being instantiated.
For `type == "PGUV" || type == "PGUW"` (and `PGUX`), the log still says `create PanguVGenerator` / `create PanguXGenerator` while the code now instantiates `HWGenerator`. This mismatch can confuse field debugging; please update the messages (or centralize logging in a helper) so they reflect the actual generator class being created.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+102
to
+103
|
|
||
| // 更新详细信息 |
There was a problem hiding this comment.
suggestion (bug_risk): Avoid using translated label text to drive logic; consider a more robust device-type check.
This logic depends on mp_Label->text() == tr("Storage")/tr("Memory") to control table visibility. Using UI text for behavior is brittle and can break with translation or wording changes. Prefer an explicit device type signal (e.g., enum or internal ID on the page) so you’re not relying on localized strings or duplicating these comparisons elsewhere.
Suggested implementation:
// 更新表格
mp_Table->setVisible(true);
mp_Table->setFixedHeight(TABLE_HEIGHT);
mp_Table->updateTable(m_deviceList, m_menuControlList);
if (Common::specialComType >= 1) {
// 使用设备类型判断是否隐藏表格，避免依赖翻译文本
if (shouldHideTableForDeviceType()) {
mp_Table->setVisible(false);
mp_Table->setFixedHeight(0);
}
}
To fully remove the dependency on localized label text, you’ll need to:
-
Add an explicit device type to the page (if not present already)
- In the corresponding header file (likely
PageMultiInfo.h), introduce an enum describing device types, for example:enum class DeviceType { Unknown, Storage, Memory, // ... other types };
- Add a member to
PageMultiInfo:DeviceType m_deviceType = DeviceType::Unknown;
- In the corresponding header file (likely
-
Provide a helper method used above
- In
PageMultiInfo.hdeclare:bool shouldHideTableForDeviceType() const;
- In
PageMultiInfo.cppimplement:bool PageMultiInfo::shouldHideTableForDeviceType() const { switch (m_deviceType) { case DeviceType::Storage: case DeviceType::Memory: return true; default: return false; } }
- In
-
Initialize
m_deviceTypebased on non-UI data- Wherever this page is constructed or updated (likely in the constructor or a
setDeviceInfo/updatePage-style method that receiveslstor device metadata), setm_deviceTypefrom a stable identifier (e.g., a field inlst,m_deviceList, orm_menuControlList, or a higher-level page ID), not frommp_Label->text()or any translated string:// Example sketch – adapt to your real model: if (deviceCategoryFromModel == DeviceCategory::Storage) m_deviceType = DeviceType::Storage; else if (deviceCategoryFromModel == DeviceCategory::Memory) m_deviceType = DeviceType::Memory; else m_deviceType = DeviceType::Unknown;
- Wherever this page is constructed or updated (likely in the constructor or a
-
(Optional) Remove any remaining logic that uses
mp_Label->text()as a behavior driver- Search the codebase for usages like
mp_Label->text() == tr("Storage")ortr("Memory")checks and replace them withm_deviceType/DeviceType-based checks, so behavior is no longer coupled to UI strings.
- Search the codebase for usages like
| @@ -48,13 +45,13 @@ DeviceGenerator *DeviceFactory::getDeviceGenerator() | |||
| generator = new HWGenerator(); | |||
| } else if (type == "PGUV" || type == "PGUW") { | |||
There was a problem hiding this comment.
issue: Debug log messages no longer match the actual generator type being instantiated.
For type == "PGUV" || type == "PGUW" (and PGUX), the log still says create PanguVGenerator / create PanguXGenerator while the code now instantiates HWGenerator. This mismatch can confuse field debugging; please update the messages (or centralize logging in a helper) so they reflect the actual generator class being created.
lzwind
approved these changes
Nov 28, 2025
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
Author
|
/forcemerge |
Contributor
|
This pr cannot be merged! (status: unstable) |
Contributor
|
This pr force merged! (status: unstable) |
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.
extend flmx mode of device
Log: extend flmx mode of device
Bug: https://pms.uniontech.com/task-view-368603.html
Summary by Sourcery
Adjust device information display and generator selection for special device types and clean up unused platform-specific generators.
Bug Fixes:
Enhancements: