-
Notifications
You must be signed in to change notification settings - Fork 40
pick develop/eagle to master #570
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
-- Code Logic error, "sizeof(logicalName::toStdString().c_str())" always return 8, not the length of logicalName. -- So, When the length more than 8, the net card will can not find. Log: fix issue Bug: https://pms.uniontech.com/bug-view-326565.html
-- Function parameter should be passed by const reference.
-- A destructor but is not marked 'override'. -- When dealing with inheritance, deleting a derived class object through a base class pointer without a virtual destructor will lead to problems.
-- Change the right way to get memory. Log: fix issue Bug: https://pms.uniontech.com/bug-view-328705.html
Implemented information masking for certain display configurations on special hardware models. pick from: linuxdeepin@71f38f8 Log: Add display info filtering for special devices Bug: https://pms.uniontech.com/bug-view-328971.html Change-Id: Ie2826605b25bdb1ef1c4956bd6856af95284b108
update monitor of subtitle for specific device models pick from: linuxdeepin@26ac1a4 Log: update monitor of subtitle for specific device models Bug: https://pms.uniontech.com/bug-view-328971.html Change-Id: Ica04bfa281acf49fde7fcdbbd8b84bf0d357afbe
update filter for some unkown netcard pick from: linuxdeepin@3165c94 Log: update filter for some unkown netcard Bug: https://pms.uniontech.com/bug-view-328939.html Change-Id: I31e2da109343b3646f497389fa8735c9b6bb3c7b
Enhanced model identification for external adapter-based NICs (including Type-C to RJ45 converters) to ensure reliable device model retrieval. Log: Fix NIC model detection failure Bug: https://pms.uniontech.com/bug-view-329931.html Change-Id: I25a5b0485228f8c431866608754601519c22b612
-- The scope of the variable can be reduced. -- Uninitialized struct member: resoulution -- Local varible pathList shadows outer variable
Reviewer's GuideRefines GPU memory parsing to read a new kernel debug path and handle generic size units, fixes Wake-on-LAN ioctl interface name handling, adjusts monitor page/table behavior for special devices, improves type safety and const-correctness across several classes, enhances network device naming and Wi‑Fi detection, and adds minor UI translation updates. Updated class diagram for driver control, device info, and UI typesclassDiagram
class DBusDriverInterface {
+slotProcessChange(value: qint32, detail: const QString &): void
+slotProcessEnd(success: bool, msg: const QString &): void
+slotDownloadProgressChanged(msg: const QStringList &): void
}
class CmdTool {
+parseOemTomlInfo(filename: const QString &): bool
}
class GetInfoPool
class CmdTask {
+CmdTask(key: const QString &, file: const QString &, info: const QString &, parent: GetInfoPool *)
+run(): void
+~CmdTask()
-m_Key: QString
-m_File: QString
-m_Info: QString
-m_Parent: GetInfoPool *
}
CmdTask ..> GetInfoPool : parent
class DriverScanner {
+setDriverList(lstInfo: const QList<DriverInfo *> &): void
-m_ListDriverInfo: QList<DriverInfo *>
}
class LoadInfoThread {
+LoadInfoThread()
+~LoadInfoThread()
}
LoadInfoThread --|> QThread
class DeviceWidget {
+DeviceWidget(parent: QWidget *)
+~DeviceWidget()
}
DeviceWidget --|> DWidget
class MainWindow {
+slotSetPage(page: const QString &): void
}
class DeviceNetwork {
+setInfoFromHwinfo(mapInfo: const QMap<QString, QString> &): bool
-m_Name: QString
}
class DeviceMonitor {
+subTitle(): QString
-m_Name: QString
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `deepin-devicemanager-server/customgpuinfo/main.cpp:83-90` </location>
<code_context>
+ double value = memInfoMatch.captured(1).toDouble();
+ QString unit = memInfoMatch.captured(2).toUpper();
+
+ if (unit == "MB") {
+ totalValue = QString("%1GB").arg(value / 1024.0, 0, 'f', 2);
+ } else if (unit == "GB") {
+ totalValue = QString("%1GB").arg(value, 0, 'f', 2);
+ } else if (unit == "KB") {
+ totalValue = QString("%1GB").arg(value / (1024.0 * 1024.0), 0, 'f', 2);
+ } else if (unit == "B") {
+ totalValue = QString("%1GB").arg(value / (1024.0 * 1024.0 * 1024.0), 0, 'f', 2);
}
</code_context>
<issue_to_address>
**question (bug_risk):** Recheck the unit conversion and formatting semantics against previous behavior
Previously we only formatted as GB (integer) when `memSize >= 1 GiB` (with `memSize` in KiB); otherwise we showed MB. The new code always emits a fractional GB value, even for small sizes.
If callers of `kGraphicsMemory` rely on MB for <1 GB or on integer values, this is a behavioral change that could cause UI/logic issues. Either keep the old MB/GB cutoff behavior or confirm all consumers can handle fractional GB in all cases.
Also verify whether `/sys/kernel/debug/gc/total_mem` reports binary units (KiB/MiB/GiB) or SI (KB/MB/GB), and ensure the conversion (1024 vs 1000) matches that.
</issue_to_address>
### Comment 2
<location> `deepin-devicemanager/src/DeviceManager/DeviceMonitor.cpp:381-382` </location>
<code_context>
QString DeviceMonitor::subTitle()
{
// qCDebug(appLog) << "Getting monitor subtitle";
+ if (Common::specialComType >= 1) {
+ m_Name.clear();
+ }
return m_Name;
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Clearing `m_Name` on every `subTitle()` call may introduce subtle state issues
This change makes `subTitle()` mutate object state whenever `specialComType >= 1`, which can unexpectedly erase `m_Name` set elsewhere (e.g., on initialization or monitor changes) and complicate reasoning about the class. If the goal is just to hide the subtitle, prefer returning an empty string without modifying the member:
```cpp
if (Common::specialComType >= 1)
return QString();
return m_Name;
```
That keeps `m_Name` as the stored value while the UI logic decides when to show it.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
-- unused parameter -- implicit conversion turns floating-point number into integer: 'double' to 'int' -- 9 enumeration values not handled in switch: 'ST_CAN_UPDATE', 'ST_WAITING', 'ST_DRIVER_NOT_BACKUP'... -- Function parameter 'status' should be passed by const reference pick from: linuxdeepin@61ad67e
…ation - Add isHwPlatform() method to Common class for platform type checking - Implement getMonitorNameFromEdid() in DeviceMonitor for EDID parsing - Add monitor name validation logic in setInfoFromXradr() method - Skip monitor name validation on non-hardware platforms - Enhance monitor device detection accuracy by comparing EDID monitor names Log: fix issue Bug: https://pms.uniontech.com/bug-view-330145.html
- Use case-insensitive contains matching instead of exact string comparison - Support interface name variations like 'ps/2', 'bluetooth low energy', etc. - Refactor hardcoded interface checks to use static list and loop - Fix device availability detection for input devices with non-standard interface names pick from: linuxdeepin@b8c68ae Log: Improve input device Bus interface detection Bug: https://pms.uniontech.com/bug-view-331181.html Change-Id: Ifbc7c40f44f45e146f07fe101ed3fc960e945459
-- Add logic code to append usb audio. pick from: linuxdeepin@76729e9 Log: fix issue Bug: https://pms.uniontech.com/bug-view-331293.html
deepin pr auto review我来对这次代码变更进行审查:
总体评价:
这次变更整体上是积极的,提高了代码的健壮性和可维护性。 |
|
[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 |
|
/forcemerge |
|
This pr force merged! (status: unstable) |
pick develop/eagle to master
Summary by Sourcery
Update device manager GPU, network, monitor handling and modernize several interfaces and threading helpers for better safety and compatibility.
Bug Fixes:
Enhancements: