pick develop/eagle to master #563
Merged
Conversation
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
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefines monitor enumeration and ordering based on xrandr output, tightens filtering of non-battery power devices, normalizes temperature display units, and simplifies TOML-driven device creation by removing a fallback duplication path. Sequence diagram for updated monitor enumeration via xrandrsequenceDiagram
participant DeviceManager
participant ThreadExecXrandr
participant XrandrCLI as Xrandr_CLI
DeviceManager->>ThreadExecXrandr: getMonitorInfoFromXrandrVerbose()
activate ThreadExecXrandr
ThreadExecXrandr->>ThreadExecXrandr: loadXrandrVerboseInfo(lstMap, xrandr_verbose_cmd)
ThreadExecXrandr->>XrandrCLI: execute xrandr --verbose
XrandrCLI-->>ThreadExecXrandr: verbose_output
loop For_each_output_block
ThreadExecXrandr->>ThreadExecXrandr: parse mainInfo
ThreadExecXrandr->>ThreadExecXrandr: mainInfoList = mainInfo.split(" ")
alt has_port_token
ThreadExecXrandr->>ThreadExecXrandr: newMap[mainInfo] = mainInfo
ThreadExecXrandr->>ThreadExecXrandr: newMap[port] = mainInfoList[0]
else
ThreadExecXrandr->>ThreadExecXrandr: newMap[mainInfo] = mainInfo
end
ThreadExecXrandr->>ThreadExecXrandr: lstMap.append(newMap)
end
ThreadExecXrandr->>ThreadExecXrandr: sort lstMap by port ascending
ThreadExecXrandr-->>DeviceManager: ordered monitor list
deactivate ThreadExecXrandr
Class diagram for updated device manager and power handlingclassDiagram
class ThreadExecXrandr {
+void loadXrandrVerboseInfo(QList_QMap_string_string_ref lstMap, QString cmd)
+void getMonitorInfoFromXrandrVerbose()
}
class DevicePower {
+bool setInfoFromUpower(QMap_string_string_const_ref mapInfo)
+void loadBaseDeviceInfo()
-QString m_Temp
}
class DeviceManager {
+void tomlDeviceSet(DeviceType deviceType)
}
DevicePower ..> DeviceManager : reported_in_device_lists
DeviceManager ..> ThreadExecXrandr : uses_for_display_info
note for ThreadExecXrandr "loadXrandrVerboseInfo now extracts port from mainInfo and getMonitorInfoFromXrandrVerbose sorts monitors by port"
note for DevicePower "setInfoFromUpower now ignores line_power, empty, and unknown states; loadBaseDeviceInfo normalizes temperature to use ℃"
note for DeviceManager "tomlDeviceSet no longer creates fallback duplicate devices when not matched"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
lzwind
approved these changes
Nov 28, 2025
![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:
- In
ThreadExecXrandr::loadXrandrVerboseInfo, splitting(*it).trimmed()usingsplit(" ")can yield empty items when there are multiple spaces; consider usingsplit(QRegularExpression("\\s+"), Qt::SkipEmptyParts)(or theSkipEmptyPartsflag) to ensure theportvalue is parsed robustly. - The custom comparator in the
std::sortonlstMapreturnsfalsewhenever one of the maps lacks"port", which can violate strict weak ordering and lead to undefined behavior; make the comparator total (e.g., explicitly order entries with and without"port") instead of using a plainelse return false. - In
DevicePower::loadBaseDeviceInfo, callingm_Temp.replace(...)inside theaddBaseDeviceInfoargument both mutates the member and relies on a side effect for formatting; it would be clearer and safer to compute a local formatted temperature string and pass that intoaddBaseDeviceInfowithout modifyingm_Temp.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `ThreadExecXrandr::loadXrandrVerboseInfo`, splitting `(*it).trimmed()` using `split(" ")` can yield empty items when there are multiple spaces; consider using `split(QRegularExpression("\\s+"), Qt::SkipEmptyParts)` (or the `SkipEmptyParts` flag) to ensure the `port` value is parsed robustly.
- The custom comparator in the `std::sort` on `lstMap` returns `false` whenever one of the maps lacks `"port"`, which can violate strict weak ordering and lead to undefined behavior; make the comparator total (e.g., explicitly order entries with and without `"port"`) instead of using a plain `else return false`.
- In `DevicePower::loadBaseDeviceInfo`, calling `m_Temp.replace(...)` inside the `addBaseDeviceInfo` argument both mutates the member and relies on a side effect for formatting; it would be clearer and safer to compute a local formatted temperature string and pass that into `addBaseDeviceInfo` without modifying `m_Temp`.
## Individual Comments
### Comment 1
<location> `deepin-devicemanager/src/DeviceManager/DevicePower.cpp:210-212` </location>
<code_context>
addBaseDeviceInfo("SBDS Serial Number", m_SBDSSerialNumber);
addBaseDeviceInfo("SBDS Manufacture Date", m_SBDSManufactureDate);
addBaseDeviceInfo("SBDS Chemistry", m_SBDSChemistry);
- addBaseDeviceInfo("Temperature", m_Temp);
+ addBaseDeviceInfo(("Temperature"), m_Temp.replace("degrees C", "℃", Qt::CaseInsensitive));
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid mutating `m_Temp` as a side effect of formatting and remove redundant parentheses.
`m_Temp.replace(...)` mutates the member each time `loadBaseDeviceInfo()` runs, which can break callers that expect the original value. Instead, copy then format:
```cpp
QString temp = m_Temp;
temp.replace("degrees C", "℃", Qt::CaseInsensitive);
addBaseDeviceInfo("Temperature", temp);
```
You can also drop the extra parentheses around `"Temperature"` for readability.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
filter the invalid power device pick from: linuxdeepin@da94bed Log: filter the invalid power device Task: https://pms.uniontech.com/task-view-376787.html
fix the invalded device show incorrect Log: fix the invalded device show incorrect Bug: https://pms.uniontech.com/bug-view-256787.html Change-Id: I57c51ef8462b11a369a9f0276f039552a9ea2825
-- fix the monitor device incorrect order Log: fix issue Bug: https://pms.uniontech.com/bug-view-317457.html
fix the temperature of battery. pick from: linuxdeepin@07bb6c9 Log: fix the temperature of battery. Bug: https://pms.uniontech.com/task-view-377193.html
deepin pr auto review我来逐个分析这些代码变更：
- } else if ((deviceType != DT_Bios) && (deviceType != DT_Computer) && !fixSameOne) {
- fixSameOne = true; //标记为该项信息有用过 ，则不再增加了
- DeviceBaseInfo *device = createDevice(deviceType);
- tomlDeviceMapSet(deviceType, device, tomlMapLst[j]);
- tomlDeviceAdd(deviceType, device); //不存在 就加这段代码被删除了，看起来是为了简化逻辑。但这个改动可能会带来一些问题：
- if (mapInfo["Device"].contains("line_power", Qt::CaseInsensitive)) {
+ if (mapInfo["Device"].contains("line_power", Qt::CaseInsensitive) ||
+ mapInfo["state"].contains("empty", Qt::CaseInsensitive) ||
+ mapInfo["state"].contains("unknown", Qt::CaseInsensitive)) {这个改动增加了对电池状态的检查：
- addBaseDeviceInfo("Temperature", m_Temp);
+ addBaseDeviceInfo(("Temperature"), m_Temp.replace("degrees C", "℃", Qt::CaseInsensitive));这个改动改进了温度显示：
+ auto mainInfoList = (*it).trimmed().split(" ");
+ if (mainInfoList.size() > 0) {
+ newMap.insert("port", mainInfoList.at(0));
+ }新增了端口信息提取：
- if (Common::specialComType == Common::SpecialComputerType::NormalCom) {
- std::reverse(lstMap.begin(), lstMap.end());
- }
+ std::sort(lstMap.begin(), lstMap.end(), [](const QMap<QString, QString> &monintor1, const QMap<QString, QString> &monintor2) {
+ if (monintor1.contains("port") && monintor2.contains("port"))
+ return monintor1.value("port") < monintor2.value("port");
+ else
+ return false;
+ });显示器排序逻辑的改进：
总体建议：
|
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
|
/merge |
Contributor
Author
|
/forcemerge |
Contributor
|
This pr cannot be merged! (status: unstable) |
Contributor
|
This pr force merged! (status: unstable) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
pick develop/eagle to master
Summary by Sourcery
Sort monitor entries from xrandr verbose output by port name, adjust power device filtering for invalid or non-battery states, improve temperature display formatting, and simplify TOML-driven device creation logic.
New Features:
Bug Fixes:
Enhancements: