pick from v20 #598
Merged
pick from v20 #598
+6
−4
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
disable bluetooth failed due to name log: fix the disable bluetooth failed bug: https://pms.uniontech.com/bug-view-341783.html Change-Id: I4495dc67a1480ae3695c253b474e3dd45c11442a
fix from dmidecode get cpu num maybe error log: fix from dmidecode get cpu num maybe error bug: https://pms.uniontech.com/bug-view-343951.html
remove hci version attribute setting pick from: 1d58066 log: remove hci version attribute setting bug: https://pms.uniontech.com/bug-view-347359.html Change-Id: I10ee960525c16efa86869f13fbf24d4a06fcde81
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts CPU core/logical count selection logic to prefer dmidecode data only when /proc/cpuinfo appears inconsistent, and refines Bluetooth device attribute extraction from hciconfig by changing how certain fields are set. Sequence diagram for updated CPU device generation logicsequenceDiagram
participant DG as DeviceGenerator
participant Proc as ProcCpuinfo
participant DMI as Dmidecode
participant DM as DeviceManager
DG->>Proc: readCpuCoreAndLogicalCounts()
Proc-->>DG: coreNum, logicalNum
DG->>DMI: readDmiCpuCoreAndLogicalCounts()
DMI-->>DG: coreNum_dmi, logicalNum_dmi
DG->>DG: if coreNum_dmi > coreNum && coreNum_dmi <= 512
DG->>DG: if logicalNum != logicalNum_dmi
DG->>DG: coreNum = coreNum_dmi
DG->>DG: if logicalNum_dmi > logicalNum && logicalNum_dmi < 1024
DG->>DG: logicalNum = logicalNum_dmi
DG->>DM: setCpuNum(detectedCpuCount)
DM-->>DG: ack
Class diagram for updated DeviceGenerator and DeviceBluetoothclassDiagram
class DeviceGenerator {
+void generatorCpuDevice()
-int coreNum
-int logicalNum
-int coreNum_dmi
-int logicalNum_dmi
}
class DeviceManager {
+static DeviceManager instance()
+void setCpuNum(int cpuNum)
}
class DeviceBluetooth {
+void setInfoFromHciconfig(QMapQStringQString mapInfo)
-QString m_Name
-QString m_Alias
-QString m_Vendor
-QString m_Version
}
class AttributeHelper {
+void setAttribute(QMapQStringQString mapInfo, QString key, QString target)
+void setAttribute(QMapQStringQString mapInfo, QString key, QString target, bool strict)
}
DeviceGenerator --> DeviceManager : uses
DeviceBluetooth --> AttributeHelper : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review这段'll review the git diff changes and provide feedback on syntax, logic, code quality, performance, and security: Review of Changes1. DeviceBluetooth.cpp ChangesChanges Made:
Analysis:
Recommendations:
2. DeviceGenerator.cpp ChangesChanges Made:
Analysis:
Recommendations:
General Observations
Suggested ImprovementsFor DeviceBluetooth.cpp: // Use the raw name without any processing (third parameter false)
setAttribute(mapInfo, "Name", m_Name, false);For DeviceGenerator.cpp: // Due to offline policy: prefer dmidecode data when available and within valid range
if (coreNum_dmi > coreNum && coreNum_dmi <= 512) {
// Only update core count if logical counts differ between sources
if (logicalNum != logicalNum_dmi) {
coreNum = coreNum_dmi;
}
}
if (logicalNum_dmi > logicalNum && logicalNum_dmi < 1024) {
logicalNum = logicalNum_dmi;
}Alternatively, consider extracting the logic to a separate method: bool DeviceGenerator::shouldUpdateFromDmi(int dmiValue, int currentValue, int maxValue) {
return dmiValue > currentValue && dmiValue <= maxValue;
}
// Then use it like:
if (shouldUpdateFromDmi(coreNum_dmi, coreNum, 512)) {
if (logicalNum != logicalNum_dmi) {
coreNum = coreNum_dmi;
}
}
if (shouldUpdateFromDmi(logicalNum_dmi, logicalNum, 1024)) {
logicalNum = logicalNum_dmi;
}This improves readability and makes the code more maintainable. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
coreNum_dmi/logicalNum_dmithresholds (512, 1024) are currently hard-coded; consider extracting them into named constants or a small helper function so the offline policy is easier to understand and maintain.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `coreNum_dmi`/`logicalNum_dmi` thresholds (512, 1024) are currently hard-coded; consider extracting them into named constants or a small helper function so the offline policy is easier to understand and maintain.
## Individual Comments
### Comment 1
<location> `deepin-devicemanager/src/GenerateDevice/DeviceGenerator.cpp:225-227` </location>
<code_context>
logicalNum_dmi += dd4["Thread Count"].toInt();
}
}
+ if(coreNum_dmi > coreNum && coreNum_dmi <= 512) { //due to offline policy
+ // If obtaining data from /proc/cpuinfo is incorrect, refer to the content of dmidecode.
+ if (logicalNum != logicalNum_dmi)
+ coreNum = coreNum_dmi;
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Reconsider using `logicalNum != logicalNum_dmi` instead of a directional comparison to gate `coreNum` updates.
The previous behavior updated `coreNum` from DMI whenever `coreNum_dmi > coreNum && coreNum_dmi <= 512`, independent of logical CPU counts. Now the update is gated on `logicalNum != logicalNum_dmi`, which also triggers when `logicalNum_dmi < logicalNum`. In that case, you’d override `coreNum` from DMI while still trusting `/proc` for `logicalNum`, mixing sources asymmetrically. If the goal is to fix undercounting in `/proc/cpuinfo`, consider restricting this to `logicalNum_dmi > logicalNum && logicalNum_dmi < 1024` (aligned with the later logical update) or otherwise making the comparison direction explicit.
</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
+225
to
+227
| if(coreNum_dmi > coreNum && coreNum_dmi <= 512) { //due to offline policy | ||
| // If obtaining data from /proc/cpuinfo is incorrect, refer to the content of dmidecode. | ||
| if (logicalNum != logicalNum_dmi) |
There was a problem hiding this comment.
issue (bug_risk): Reconsider using logicalNum != logicalNum_dmi instead of a directional comparison to gate coreNum updates.
The previous behavior updated coreNum from DMI whenever coreNum_dmi > coreNum && coreNum_dmi <= 512, independent of logical CPU counts. Now the update is gated on logicalNum != logicalNum_dmi, which also triggers when logicalNum_dmi < logicalNum. In that case, you’d override coreNum from DMI while still trusting /proc for logicalNum, mixing sources asymmetrically. If the goal is to fix undercounting in /proc/cpuinfo, consider restricting this to logicalNum_dmi > logicalNum && logicalNum_dmi < 1024 (aligned with the later logical update) or otherwise making the comparison direction explicit.
lzwind
approved these changes
Jan 22, 2026
|
[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 |
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.
pick from v20
Summary by Sourcery
Adjust CPU and Bluetooth device information handling to improve accuracy and avoid incorrect attribute overrides.
Bug Fixes: