fix(bluetooth): align audio device icon with taskbar - #3407
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR fixes a Bluetooth audio device icon mismatch between the control center and taskbar and restores the duplicate-connection guard for audio devices by aligning icon mappings with the device type values used in connection logic. Sequence diagram for BluetoothWorker::connectDevice audio device duplicate-connection guardsequenceDiagram
actor User
participant ControlCenter
participant BluetoothWorker
participant BluetoothAdapter
participant BluetoothDevice
User ->> ControlCenter: requestConnect(deviceId)
ControlCenter ->> BluetoothWorker: connectDevice(deviceId, adapterPath)
BluetoothWorker ->> BluetoothAdapter: deviceById(deviceId)
BluetoothAdapter -->> BluetoothWorker: BluetoothDevice
alt [device exists and is audio headset/phones]
BluetoothWorker ->> BluetoothDevice: deviceType()
BluetoothWorker ->> BluetoothDevice: state()
note over BluetoothWorker,BluetoothDevice: deviceType == bluetooth_pheadset or bluetooth_headset
BluetoothWorker -->> ControlCenter: return (skip duplicate connect)
else [other device types or states]
BluetoothWorker ->> BluetoothWorker: connectDeviceInternal
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider adding a brief comment near
deviceType()usage inBluetoothWorker::connectDeviceto clarify that it returns the mapped icon/device type (e.g.,bluetooth_pheadset) rather than the originalaudio-*type, to prevent future confusion or regressions. - The string literals for device types/icons (e.g.,
bluetooth_pheadset,bluetooth_headset) are repeated in multiple places; centralizing them as named constants or an enum-like structure would reduce the risk of mismatches in future changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider adding a brief comment near `deviceType()` usage in `BluetoothWorker::connectDevice` to clarify that it returns the mapped icon/device type (e.g., `bluetooth_pheadset`) rather than the original `audio-*` type, to prevent future confusion or regressions.
- The string literals for device types/icons (e.g., `bluetooth_pheadset`, `bluetooth_headset`) are repeated in multiple places; centralizing them as named constants or an enum-like structure would reduce the risk of mismatches in future changes.
## Individual Comments
### Comment 1
<location path="src/plugin-bluetooth/operation/bluetoothdevice.h" line_range="26" />
<code_context>
{"audio-card","bluetooth_pheadset"},
{"audio-headset","bluetooth_pheadset"},
- {"audio-headphones","bluetooth_headset"},
+ {"audio-headphones","bluetooth_pheadset"},
{"network-wireless","bluetooth_lan"},
{"camera-video","bluetooth_vidicon"},
</code_context>
<issue_to_address>
**suggestion:** Consider centralizing type-to-icon mapping and avoiding string duplication.
This mapping couples logical device types (e.g. `"audio-headphones"`) to icon IDs (e.g. `"bluetooth_pheadset"`), while other parts of the code branch on hard-coded type strings. Repeating these literals for both behavior and icons increases the risk of mismatches (as seen in `connectDevice`). Consider centralizing device-type definitions (e.g. constants or an enum) and keeping icon IDs separate, so behavioral logic never relies on icon names.
Suggested implementation:
```c
{BluetoothDevice::Type::InputTablet, BluetoothDevice::Icon::Touchpad},
{BluetoothDevice::Type::AudioCard, BluetoothDevice::Icon::PersonalHeadset},
{BluetoothDevice::Type::AudioHeadset, BluetoothDevice::Icon::PersonalHeadset},
{BluetoothDevice::Type::AudioHeadphones, BluetoothDevice::Icon::PersonalHeadset},
{BluetoothDevice::Type::NetworkWireless, BluetoothDevice::Icon::Lan},
{BluetoothDevice::Type::CameraVideo, BluetoothDevice::Icon::Vidicon},
{BluetoothDevice::Type::Printer, BluetoothDevice::Icon::Print},
```
To make this compile and fully implement the centralization:
1. **Introduce device type identifiers** in `bluetoothdevice.h`:
- Add an enum (or enum class) for logical device types:
```cpp
struct BluetoothDevice {
enum class Type {
InputTablet,
AudioCard,
AudioHeadset,
AudioHeadphones,
NetworkWireless,
CameraVideo,
Printer,
// ...existing types
};
enum class Icon {
Touchpad,
PersonalHeadset,
Lan,
Vidicon,
Print,
// ...existing icons
};
// Optionally: helper conversion functions to/from QString/QByteArray
static Type typeFromString(const QString &typeId);
static Icon iconFromString(const QString &iconId);
static const char *toString(Type type);
static const char *toString(Icon icon);
};
```
- Ensure this `BluetoothDevice` type (or equivalent) is declared in a header that can be used both by behavioral logic (e.g. `connectDevice`) and icon mapping.
2. **Adapt the mapping container type**:
- The initializer you edited likely belongs to an array or map of string pairs. After switching to `BluetoothDevice::Type` and `BluetoothDevice::Icon`, update the declaration accordingly, e.g.:
```cpp
struct DeviceTypeIconMapping {
BluetoothDevice::Type type;
BluetoothDevice::Icon icon;
};
static const DeviceTypeIconMapping kDeviceTypeIconMap[] = {
{BluetoothDevice::Type::InputTablet, BluetoothDevice::Icon::Touchpad},
{BluetoothDevice::Type::AudioCard, BluetoothDevice::Icon::PersonalHeadset},
{BluetoothDevice::Type::AudioHeadset, BluetoothDevice::Icon::PersonalHeadset},
{BluetoothDevice::Type::AudioHeadphones, BluetoothDevice::Icon::PersonalHeadset},
{BluetoothDevice::Type::NetworkWireless, BluetoothDevice::Icon::Lan},
{BluetoothDevice::Type::CameraVideo, BluetoothDevice::Icon::Vidicon},
{BluetoothDevice::Type::Printer, BluetoothDevice::Icon::Print},
// ...
};
```
- If the existing mapping is a `QHash<QString, QString>` or similar, either:
* change it to use `Type`/`Icon` directly, or
* keep that structure but build it from the strongly-typed map (converting via the helper functions in `BluetoothDevice`).
3. **Update behavioral logic to use `BluetoothDevice::Type` instead of raw strings**:
- Anywhere in the codebase where you currently branch on raw type strings, e.g.:
```cpp
if (deviceType == "audio-headphones") { ... }
```
replace that with:
```cpp
if (deviceType == BluetoothDevice::Type::AudioHeadphones) { ... }
```
- Ensure that conversion from the underlying DBus / BlueZ / configuration strings to `BluetoothDevice::Type` happens in one place (e.g. a factory or parser), so the rest of the code deals only in the enum.
4. **Keep icon IDs decoupled from logic**:
- Where you actually need the icon string (e.g. for UI), use a function that maps `BluetoothDevice::Type` to an icon:
```cpp
QString BluetoothDevice::iconNameFor(BluetoothDevice::Type type) {
// use kDeviceTypeIconMap to look up the Icon enum and then toString(Icon)
}
```
- Ensure no behavioral logic branches on icon ID strings; only the UI/rendering layer should consume those strings.
These additional steps will fully centralize the type-to-icon mapping and prevent behavioral logic from relying on duplicated string literals or icon IDs.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| {"audio-card","bluetooth_pheadset"}, | ||
| {"audio-headset","bluetooth_pheadset"}, | ||
| {"audio-headphones","bluetooth_headset"}, | ||
| {"audio-headphones","bluetooth_pheadset"}, |
There was a problem hiding this comment.
suggestion: Consider centralizing type-to-icon mapping and avoiding string duplication.
This mapping couples logical device types (e.g. "audio-headphones") to icon IDs (e.g. "bluetooth_pheadset"), while other parts of the code branch on hard-coded type strings. Repeating these literals for both behavior and icons increases the risk of mismatches (as seen in connectDevice). Consider centralizing device-type definitions (e.g. constants or an enum) and keeping icon IDs separate, so behavioral logic never relies on icon names.
Suggested implementation:
{BluetoothDevice::Type::InputTablet, BluetoothDevice::Icon::Touchpad},
{BluetoothDevice::Type::AudioCard, BluetoothDevice::Icon::PersonalHeadset},
{BluetoothDevice::Type::AudioHeadset, BluetoothDevice::Icon::PersonalHeadset},
{BluetoothDevice::Type::AudioHeadphones, BluetoothDevice::Icon::PersonalHeadset},
{BluetoothDevice::Type::NetworkWireless, BluetoothDevice::Icon::Lan},
{BluetoothDevice::Type::CameraVideo, BluetoothDevice::Icon::Vidicon},
{BluetoothDevice::Type::Printer, BluetoothDevice::Icon::Print},To make this compile and fully implement the centralization:
-
Introduce device type identifiers in
bluetoothdevice.h:- Add an enum (or enum class) for logical device types:
struct BluetoothDevice { enum class Type { InputTablet, AudioCard, AudioHeadset, AudioHeadphones, NetworkWireless, CameraVideo, Printer, // ...existing types }; enum class Icon { Touchpad, PersonalHeadset, Lan, Vidicon, Print, // ...existing icons }; // Optionally: helper conversion functions to/from QString/QByteArray static Type typeFromString(const QString &typeId); static Icon iconFromString(const QString &iconId); static const char *toString(Type type); static const char *toString(Icon icon); };
- Ensure this
BluetoothDevicetype (or equivalent) is declared in a header that can be used both by behavioral logic (e.g.connectDevice) and icon mapping.
- Add an enum (or enum class) for logical device types:
-
Adapt the mapping container type:
- The initializer you edited likely belongs to an array or map of string pairs. After switching to
BluetoothDevice::TypeandBluetoothDevice::Icon, update the declaration accordingly, e.g.:struct DeviceTypeIconMapping { BluetoothDevice::Type type; BluetoothDevice::Icon icon; }; static const DeviceTypeIconMapping kDeviceTypeIconMap[] = { {BluetoothDevice::Type::InputTablet, BluetoothDevice::Icon::Touchpad}, {BluetoothDevice::Type::AudioCard, BluetoothDevice::Icon::PersonalHeadset}, {BluetoothDevice::Type::AudioHeadset, BluetoothDevice::Icon::PersonalHeadset}, {BluetoothDevice::Type::AudioHeadphones, BluetoothDevice::Icon::PersonalHeadset}, {BluetoothDevice::Type::NetworkWireless, BluetoothDevice::Icon::Lan}, {BluetoothDevice::Type::CameraVideo, BluetoothDevice::Icon::Vidicon}, {BluetoothDevice::Type::Printer, BluetoothDevice::Icon::Print}, // ... };
- If the existing mapping is a
QHash<QString, QString>or similar, either:- change it to use
Type/Icondirectly, or - keep that structure but build it from the strongly-typed map (converting via the helper functions in
BluetoothDevice).
- change it to use
- The initializer you edited likely belongs to an array or map of string pairs. After switching to
-
Update behavioral logic to use
BluetoothDevice::Typeinstead of raw strings:- Anywhere in the codebase where you currently branch on raw type strings, e.g.:
replace that with:
if (deviceType == "audio-headphones") { ... }
if (deviceType == BluetoothDevice::Type::AudioHeadphones) { ... } - Ensure that conversion from the underlying DBus / BlueZ / configuration strings to
BluetoothDevice::Typehappens in one place (e.g. a factory or parser), so the rest of the code deals only in the enum.
- Anywhere in the codebase where you currently branch on raw type strings, e.g.:
-
Keep icon IDs decoupled from logic:
- Where you actually need the icon string (e.g. for UI), use a function that maps
BluetoothDevice::Typeto an icon:QString BluetoothDevice::iconNameFor(BluetoothDevice::Type type) { // use kDeviceTypeIconMap to look up the Icon enum and then toString(Icon) }
- Ensure no behavioral logic branches on icon ID strings; only the UI/rendering layer should consume those strings.
- Where you actually need the icon string (e.g. for UI), use a function that maps
These additional steps will fully centralize the type-to-icon mapping and prevent behavioral logic from relying on duplicated string literals or icon IDs.
f247000 to
8aa9ce2
Compare
|
TAG Bot New tag: 6.1.104 |
8aa9ce2 to
1f33faf
Compare
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The hard-coded device type strings in
connectDeviceanddeviceType2Iconmake the logic brittle; consider centralizing these values as named constants or an enum so future changes to type names or mappings remain consistent. - Since
deviceType()returns mapped values, it would help to clearly document this behavior wheredeviceType()is defined or used so future readers don’t mistakenly compare against raw device types again.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The hard-coded device type strings in `connectDevice` and `deviceType2Icon` make the logic brittle; consider centralizing these values as named constants or an enum so future changes to type names or mappings remain consistent.
- Since `deviceType()` returns mapped values, it would help to clearly document this behavior where `deviceType()` is defined or used so future readers don’t mistakenly compare against raw device types again.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const BluetoothDevice *device = adapter->deviceById(deviceId); | ||
| if (device | ||
| && (device->deviceType() == "audio-headset" || device->deviceType() == "audio-headphones") | ||
| && (device->deviceType() == "bluetooth_pheadset" || device->deviceType() == "bluetooth_headset") |
There was a problem hiding this comment.
device type 不是"audio-headphones"这种么,
There was a problem hiding this comment.
void BluetoothDevice::setDeviceType(const QString &deviceType)
{
m_deviceType = deviceType2Icon.contains(deviceType) ? deviceType2Icon[deviceType] : "bluetooth_other";
}
原代码在设置时就做了转换,所以再获取时是"bluetooth_pheadset"这种
…kbar 1. Map audio-headphones icon to bluetooth_pheadset in deviceType2Icon, unifying audio-card/audio-headset/audio-headphones rendering 2. Fix BluetoothWorker::connectDevice audio device type comparison: deviceType() returns the icon name mapped by setDeviceType(), not the raw BlueZ type, so compare against bluetooth_pheadset instead of the raw audio-headset/audio-headphones which never matched 3. Update SPDX copyright year to 2026 PMS: BUG-275675 fix: 统一控制中心与任务栏的蓝牙音频设备图标 1. 将 deviceType2Icon 中 audio-headphones 的图标映射从 bluetooth_headset 改为 bluetooth_pheadset,与 audio-card/audio-headset 统一 2. 修复 BluetoothWorker::connectDevice 中音频设备类型判断:deviceType() 返回的是经 setDeviceType 转换后的图标名而非原始 BlueZ 类型,因此应比较 bluetooth_pheadset,原比较 audio-headset/audio-headphones 永不命中 3. 更新 SPDX 版权年份至 2026 PMS: BUG-275675 Change-Id: Ic64995399a61dce9e3a6ac7d5d445ea90fdf8eb0
1f33faf to
4d8dff7
Compare
deepin pr auto review★ 总体评分:100分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // 当前代码已为最佳实践,无需额外改进,此处展示当前最终形态以供校验
// src/plugin-bluetooth/operation/bluetoothdevice.h
static const QMap<QString,QString> deviceType2Icon {
// ...(其他映射省略)
{"audio-card","bluetooth_pheadset"},
{"audio-headset","bluetooth_pheadset"},
{"audio-headphones","bluetooth_pheadset"},
// ...
};
// src/plugin-bluetooth/operation/bluetoothworker.cpp
void BluetoothWorker::connectDevice(const QString &deviceId, const QString adapt)
{
// ...(前文省略)
const BluetoothDevice *device = adapter->deviceById(deviceId);
// deviceType() returns the icon name mapped by setDeviceType() (bluetoothdevice.cpp:87),
// not the raw BlueZ type. audio-card/audio-headset/audio-headphones all map to
// bluetooth_pheadset, so compare against that to skip duplicate audio-device connects.
if (device
&& device->deviceType() == "bluetooth_pheadset"
&& device->state() == BluetoothDevice::StateAvailable) {
return;
}
// ...(后文省略)
} |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, caixr23 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 |
修复 BUG-275675:连接蓝牙音箱时任务栏与控制中心设备图标不一致
改动内容(2 文件 2 行)
bluetoothdevice.h: 将audio-headphones的图标映射从bluetooth_headset改为bluetooth_pheadset,与audio-card和audio-headset保持一致,消除控制中心与任务栏蓝牙列表的图标不一致。bluetoothworker.cpp: 修复BluetoothWorker::connectDevice中音频设备类型判断错误。deviceType()返回的是映射后的值(如bluetooth_pheadset),原代码比较原始值audio-headset/audio-headphones永远不匹配,导致音频设备防重复连接优化从未生效。改为比较映射后的bluetooth_pheadset/bluetooth_headset。PMS Bug
https://pms.uniontech.com/bug-view-275675.html
代码审核
已通过 multica 代码审核(标准-代码审核bot),审核报告确认改动正确、安全、低风险。
跨仓库遗留
dde-tray-loader 的
deviceType2Icon同样缺少audio-headset/audio-headphones条目,导致任务栏侧仍回退bluetooth_other。此为另一仓库的后续跟进项,不影响本 patch。Log: 修复连接蓝牙音箱时控制中心与任务栏设备图标不一致问题,统一音频设备图标映射并修复音频设备连接判断逻辑
Bug: https://pms.uniontech.com/bug-view-275675.html
Summary by Sourcery
Align Bluetooth audio device handling so taskbar and control center use consistent icons and prevent redundant connections for mapped audio device types.
Bug Fixes: