Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions deepin-devicemanager/src/DeviceManager/DeviceImage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ DeviceImage::DeviceImage()
qCDebug(appLog) << "DeviceImage constructor initialized.";
m_CanEnable = true;
m_CanUninstall = true;
m_forcedDisplay = true;
}

void DeviceImage::setInfoFromLshw(const QMap<QString, QString> &mapInfo)
Expand Down
31 changes: 26 additions & 5 deletions deepin-devicemanager/src/DeviceManager/DeviceMonitor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
, m_CurrentResolution("")
, m_SerialNumber("")
, m_ProductionWeek("")
, m_RefreshRate("")
, m_Width(0)
, m_Height(0)
, m_IsTomlSet(false)
Expand Down Expand Up @@ -338,9 +339,13 @@
qCDebug(appLog) << "Getting monitor overview information";
QString ov;

ov = QString("%1(%2)").arg(m_Name).arg(m_ScreenSize);

Check warning on line 342 in deepin-devicemanager/src/DeviceManager/DeviceMonitor.cpp

View workflow job for this annotation

GitHub Actions / cppcheck

Variable 'ov' is reassigned a value before the old one has been used.
Copy link

Choose a reason for hiding this comment

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

issue (complexity): Consider refactoring the new monitor-handling logic to remove redundant assignments, replace magic specialComType numbers with named enums, and centralize resolution/refresh-rate formatting into small helpers.

You can keep all the new behavior but significantly reduce complexity and duplication with a few small refactors.

1. Simplify getOverviewInfo

The current logic reassigns ov unnecessarily and repeats the same format string:

QString ov;

ov = QString("%1(%2)").arg(m_Name).arg(m_ScreenSize);
if (Common::specialComType == 6) {
    ov = QString("(%1)").arg(m_ScreenSize);
} else {
    ov = QString("%1(%2)").arg(m_Name).arg(m_ScreenSize);
}

You can remove the redundant assignment and use a conditional expression (or a tiny helper) instead:

const QString DeviceMonitor::getOverviewInfo()
{
    qCDebug(appLog) << "Getting monitor overview information";

    const QString ov = (Common::specialComType == 6)
        ? QString("(%1)").arg(m_ScreenSize)
        : QString("%1(%2)").arg(m_Name).arg(m_ScreenSize);

    qCDebug(appLog) << "Monitor overview:" << ov;
    return ov;
}

2. Replace magic specialComType numbers with named values

You already have several magic numbers (1, 4, 5, 6). You can centralize their meaning without changing behavior by introducing an enum (or enum class) and using it where you branch:

namespace Common {
    enum SpecialComType {
        Normal              = 0,
        RateWithDecimals    = 1, // was 1
        ParseResolutionRate = 4, // was 4
        NoRateInResolution  = 5, // was 5
        NameLessOverview    = 6  // was 6
    };
}

Then update conditions locally for clarity:

if (Common::specialComType == Common::NameLessOverview) { ... }
if (Common::specialComType == Common::ParseResolutionRate) { ... }
// etc.

This keeps all branching exactly as-is but makes the intent much clearer and reduces cognitive load.

3. Localize resolution/refresh-rate formatting

Right now, resolution and refresh rate formatting is scattered:

  • setMainInfoFromXrandr:
    • specialComType == 1: tweak curRate string with a ".00" inserted.
    • specialComType == 1 || 6: set m_RefreshRate.
    • specialComType == 5 || 6: set m_CurrentResolution without @.
    • Else: "Size @Rate" format with a space.
  • loadOtherDeviceInfo:
    • specialComType == 4: parse m_CurrentResolution to extract refresh rate.

You can centralize this into compact helpers that preserve behavior but remove duplication and condition scattering.

3.1 Extract a formatCurRate helper

This removes the duplicated curRate expression with/without ".00":

static QString formatCurRate(const QString &rawRate, int specialComType)
{
    QRegularExpression rateStart("[a-zA-Z]");
    QRegularExpressionMatch rateMatch = rateStart.match(rawRate);
    int pos = rateMatch.capturedStart();

    if (pos <= 0 || rawRate.size() <= pos || Common::boardVendorType().isEmpty())
        return rawRate;

    const QString base = QString::number(ceil(rawRate.left(pos).toDouble()));
    const QString suffix = rawRate.right(rawRate.size() - pos);

    if (specialComType == Common::RateWithDecimals) { // was 1
        return base + ".00" + suffix;
    }
    return base + suffix;
}

Use it in setMainInfoFromXrandr:

QString curRate = formatCurRate(rate, Common::specialComType);

if (Common::specialComType == Common::RateWithDecimals ||
    Common::specialComType == Common::NameLessOverview) {
    m_RefreshRate = curRate;
}

if (Common::specialComType == Common::NoRateInResolution ||
    Common::specialComType == Common::NameLessOverview) {
    m_CurrentResolution = QT_REGEXP_CAPTURE(reScreenSize, 1, info);
} else {
    m_CurrentResolution = QString("%1 @%2")
        .arg(QT_REGEXP_CAPTURE(reScreenSize, 1, info))
        .arg(curRate);
}

Behavior is unchanged; logic is easier to follow and the curRate formatting is in one place.

3.2 Extract refresh-rate parsing from m_CurrentResolution

The parsing for specialComType == 4 can also be centralized to avoid repeating rules elsewhere:

static QString extractRefreshRateFromResolution(const QString &resolution)
{
    if (!resolution.contains('@'))
        return {};

    const QStringList parts = resolution.split('@', QT_SKIP_EMPTY_PARTS);
    if (parts.size() != 2)
        return {};

    return parts.at(1).trimmed();
}

Then loadOtherDeviceInfo becomes:

if (m_CurrentResolution != "@Hz") {
    addOtherDeviceInfo("Current Resolution", m_CurrentResolution);
    addOtherDeviceInfo("Display Ratio", m_AspectRatio);

    if (Common::specialComType == Common::ParseResolutionRate) {
        m_RefreshRate = extractRefreshRateFromResolution(m_CurrentResolution);
    }
}

This centralizes the @ parsing logic; if other specialComType values later need similar parsing, it’s now reusable.


All of the above keeps your feature behavior intact, but:

  • eliminates redundant assignments and duplicate string formatting,
  • replaces magic numbers with named constants,
  • and localizes resolution/refresh-rate formatting and parsing into small, focused helpers.

if (Common::specialComType == 6) {
ov = QString("(%1)").arg(m_ScreenSize);
} else {
ov = QString("%1(%2)").arg(m_Name).arg(m_ScreenSize);
}
qCDebug(appLog) << "Monitor overview:" << ov;

return ov;
}

Expand All @@ -354,7 +359,8 @@
{
qCDebug(appLog) << "Loading base device info for monitor";
// 添加基本信息
addBaseDeviceInfo("Name", m_Name);
if (Common::specialComType != 6)
addBaseDeviceInfo("Name", m_Name);
addBaseDeviceInfo("Vendor", m_Vendor);
addBaseDeviceInfo("Type", m_Model);
addBaseDeviceInfo("Display Input", m_DisplayInput);
Expand All @@ -369,6 +375,14 @@
if (m_CurrentResolution != "@Hz") {
addOtherDeviceInfo("Current Resolution", m_CurrentResolution);
addOtherDeviceInfo("Display Ratio", m_AspectRatio);
if (Common::specialComType == 4) {
if (m_CurrentResolution.contains("@")) {
QStringList refreshList = m_CurrentResolution.split('@', QT_SKIP_EMPTY_PARTS);
if (refreshList.size() == 2) {
m_RefreshRate = refreshList.at(1).trimmed();
}
}
}
}
addOtherDeviceInfo("Primary Monitor", m_MainScreen);
addOtherDeviceInfo("Size", m_ScreenSize);
Expand Down Expand Up @@ -427,12 +441,19 @@
int pos = rateMatch.capturedStart();
if (pos > 0 && curRate.size() > pos && !Common::boardVendorType().isEmpty()) {
qCDebug(appLog) << "Adjusting rate for board vendor type";
curRate = QString::number(ceil(curRate.left(pos).toDouble())) + curRate.right(curRate.size() - pos);
if (Common::specialComType == 1) {
curRate = QString::number(ceil(curRate.left(pos).toDouble())) + ".00" + curRate.right(curRate.size() - pos);
} else {
curRate = QString::number(ceil(curRate.left(pos).toDouble())) + curRate.right(curRate.size() - pos);
}
}
if (Common::specialComType == 1 || Common::specialComType == 6) {
m_RefreshRate = QString("%1").arg(curRate);
Comment on lines +450 to +451
Copy link

Choose a reason for hiding this comment

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

issue (bug_risk): m_RefreshRate may retain stale values when no rate is detected

m_RefreshRate is only updated when reRate.indexIn(rate) matches and the specialComType condition holds. When no rate is found (or the condition fails), it retains the previous value, which may be wrong for the current monitor. Please clear or reset m_RefreshRate when no valid rate is detected so consumers can distinguish “no rate” from a stale value.

}
if (Common::specialComType == 5) {
if (Common::specialComType == 5 || Common::specialComType == 6) {
m_CurrentResolution = QString("%1").arg(QT_REGEXP_CAPTURE(reScreenSize, 1, info));
} else {
m_CurrentResolution = QString("%1@%2").arg(QT_REGEXP_CAPTURE(reScreenSize, 1, info)).arg(curRate);
m_CurrentResolution = QString("%1 @%2").arg(QT_REGEXP_CAPTURE(reScreenSize, 1, info)).arg(curRate);
}
} else {
qCDebug(appLog) << "Rate is empty, setting current resolution without rate";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1021,7 +1021,7 @@ void DeviceGenerator::getBlueToothInfoFromHwinfo()
continue;
}

if ((*it)["Model"].contains("bluetooth", Qt::CaseInsensitive) || (*it)["Hardware Class"] == "bluetooth" || (*it)["Driver"] == "btusb" || (*it)["Device"] == "BCM20702A0") {
if ((*it)["Model"].contains("bluetooth", Qt::CaseInsensitive) || (*it)["Hardware Class"] == "bluetooth" || (*it)["Driver"] == "btusb" || (*it)["Device"] == "BCM20702A0" || (*it)["Device"] == "SCM2625 BT 5.2 network adapter") {
// qCDebug(appLog) << "Found bluetooth device from hwinfo";
// 判断重复设备数据
QString unique_id = uniqueID(*it);
Expand Down
2 changes: 0 additions & 2 deletions deepin-devicemanager/src/GenerateDevice/HWGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -310,8 +310,6 @@ void HWGenerator::generatorDiskDevice()
tempMap["Name"] = "nouse";
// 应HW的要求,将描述固定为 Universal Flash Storage
tempMap["Description"] = "Universal Flash Storage";
// 应H的要求,添加interface UFS 3.1
tempMap["Interface"] = "UFS 3.1";

// 读取interface版本
QProcess process;
Expand Down