Skip to content

Conversation

@add-uos
Copy link
Contributor

@add-uos add-uos commented Jan 21, 2026

remove hci version attribute setting

log: remove hci version attribute setting
bug: https://pms.uniontech.com/bug-view-347359.html
Change-Id: I10ee960525c16efa86869f13fbf24d4a06fcde81

Summary by Sourcery

Bug Fixes:

  • Avoid potential issues caused by reading or storing the Bluetooth HCI version field from hciconfig output.

@sourcery-ai
Copy link

sourcery-ai bot commented Jan 21, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR modifies Bluetooth device info initialization by no longer setting the HCI Version attribute from hciconfig output, likely to avoid incorrect or obsolete version reporting while leaving all other attribute handling unchanged.

Sequence diagram for updated Bluetooth info initialization without HCI Version

sequenceDiagram
    participant DeviceBluetooth
    participant HciconfigMap

    DeviceBluetooth->>DeviceBluetooth: setInfoFromHciconfig(mapInfo)
    activate DeviceBluetooth
    DeviceBluetooth->>DeviceBluetooth: setAttribute(mapInfo, Name, m_Name, false)
    DeviceBluetooth->>DeviceBluetooth: setAttribute(mapInfo, Alias, m_Alias)
    DeviceBluetooth->>DeviceBluetooth: setAttribute(mapInfo, Manufacturer, m_Vendor)
    DeviceBluetooth->>DeviceBluetooth: getOtherMapInfo(mapInfo)
    deactivate DeviceBluetooth
Loading

Class diagram for DeviceBluetooth after removing HCI Version attribute setting

classDiagram
    class DeviceBluetooth {
        - QString m_Name
        - QString m_Alias
        - QString m_Vendor
        - QString m_Version
        + void setInfoFromHciconfig(QMapQStringQString mapInfo)
        - void setAttribute(QMapQStringQString mapInfo, QString key, QString attribute, bool overwrite)
        - void getOtherMapInfo(QMapQStringQString mapInfo)
    }
Loading

File-Level Changes

Change Details Files
Stop populating the Bluetooth HCI version attribute when parsing hciconfig output for device information.
  • Removed the call that mapped the "HCI Version" field from hciconfig data into the internal m_Version member using setAttribute
  • Kept all other attribute mappings (Name, Alias, Manufacturer, and additional info via getOtherMapInfo) intact
deepin-devicemanager/src/DeviceManager/DeviceBluetooth.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • If m_Version is no longer populated anywhere after removing this setter, consider removing the member or any dependent logic to avoid carrying dead state.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- If `m_Version` is no longer populated anywhere after removing this setter, consider removing the member or any dependent logic to avoid carrying dead state.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: add-uos, max-lvs

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

remove hci version attribute setting

log: remove hci version attribute setting
bug: https://pms.uniontech.com/bug-view-347359.html
Change-Id: I10ee960525c16efa86869f13fbf24d4a06fcde81
@deepin-ci-robot
Copy link

deepin pr auto review

针对你提供的 git diff 内容,我进行了仔细的审查。这段代码主要涉及 CMake 构建脚本的调试信息输出和 C++ 源代码中一行逻辑的删除。以下是关于语法逻辑、代码质量、代码性能和代码安全的详细审查意见及改进建议:

1. 文件:deepin-devicemanager/CMakeLists.txt

修改内容:
增加了 message 命令用于打印 OS_INFO_STR 变量的内容。

审查意见:

  • 代码质量

    • 问题:直接将 cat /etc/os-version 的原始输出打印到控制台,包含大量无关信息(如换行符),且在正式构建流程中产生视觉噪音。
    • 建议:如果这是为了临时调试版本号匹配问题,建议在调试完成后移除这些打印语句。如果必须保留日志,建议只打印提取后的关键变量(如 MAJOR_VERSION),或者使用 STATUS 级别的日志(message(STATUS ...))而不是默认的 SEND_ERROR/FATAL_ERROR 级别(虽然默认是普通输出,但显式指定级别更规范)。
    • 改进示例
      message(STATUS "Detected OS Major Version: ${MAJOR_VERSION}")
  • 代码逻辑

    • 问题:代码逻辑本身没有错误,能够正确读取文件并解析版本号。但是,依赖 cat 命令和正则解析 /etc/os-version 文件具有一定的脆弱性。如果文件格式变化,正则可能失效。
    • 建议:确保 /etc/os-version 是系统提供的标准且稳定的接口。如果是 Deepin 特有的,需确保该文件格式在后续版本中保持兼容。

2. 文件:deepin-devicemanager/src/DeviceManager/DeviceBluetooth.cpp

修改内容
删除了 setAttribute(mapInfo, "HCI Version", m_Version, true); 这一行代码。

审查意见:

  • 代码逻辑

    • 风险:删除了将 HCI Version(蓝牙控制器版本)赋值给 m_Version 的逻辑。这意味着 DeviceBluetooth 对象的 m_Version 成员变量将不再通过 setInfoFromHciconfig 方法从 mapInfo 中更新。
    • 潜在后果
      1. 如果 m_Version 在后续代码(如 UI 显示、设备信息查询)中被使用,它将保持未初始化状态或旧值,导致显示错误或逻辑判断错误。
      2. 如果 m_Version 仅用于此处且后续不再使用,删除是合理的,但需确认成员变量 m_Version 本身是否也应该被移除以减少冗余。
    • 建议
      • 请搜索项目中所有使用 m_Version 的地方,确认删除此赋值后不会导致空指针解引用、显示 "Unknown" 或逻辑错误。
      • 检查 getOtherMapInfo(mapInfo) 函数内部是否接管了 HCI Version 的处理。如果 getOtherMapInfo 负责处理剩余字段,请确保逻辑闭环。
  • 代码安全

    • 无直接影响:此修改主要涉及数据读取,不涉及内存操作或外部命令执行,因此没有直接的安全隐患。

总结与改进建议

  1. CMakeLists.txt

    • 移除调试代码:建议在合并代码前删除或注释掉 message("---------------------") 等调试输出,保持构建日志的整洁。
    • 健壮性:考虑添加对 execute_process 结果的检查,例如如果文件不存在或读取失败,设置一个默认版本号。
  2. DeviceBluetooth.cpp

    • 确认副作用:必须确认删除 HCI Version 的赋值是符合预期的业务逻辑变更(例如不再支持显示该版本),而不是误删。
    • 代码清理:如果 m_Version 确实不再需要,建议同时删除类中该成员变量的声明,以避免代码混淆和潜在的静态分析警告。

修改后的建议代码片段(CMake部分):

# 读取版本信息
execute_process(COMMAND cat /etc/os-version OUTPUT_VARIABLE OS_INFO_STR)
string(REGEX MATCHALL "MajorVersion=[0-9]+" MAJOR_STR "${OS_INFO_STR}")
string(REGEX MATCH "[0-9]+" MAJOR_VERSION "${MAJOR_STR}")

# 仅在调试模式下或需要时打印关键信息,避免污染输出
if(CMAKE_BUILD_TYPE MATCHES "Debug")
    message(STATUS "OS Info Raw: ${OS_INFO_STR}")
    message(STATUS "Extracted Major Version: ${MAJOR_VERSION}")
endif()

if (MAJOR_VERSION MATCHES "23")
    message(STATUS "Configuring for OS Build V23")
    add_definitions(-DOS_BUILD_V23)
endif()

@add-uos
Copy link
Contributor Author

add-uos commented Jan 21, 2026

流水线环境问题
image

@add-uos
Copy link
Contributor Author

add-uos commented Jan 21, 2026

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Jan 21, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit 1d58066 into linuxdeepin:develop/eagle Jan 21, 2026
16 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants