fix: emit all relevant roles on bluetooth device state change#2903
Merged
Conversation
- Added ConnectStatus, ConnectStatusText, Battery, and BatteryIconPath roles to dataChanged signal. Log: emit all relevant roles on bluetooth device state change pms: BUG-343983
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEnsure that when a Bluetooth device’s state changes and the model emits dataChanged, all relevant display and status roles (name, connection, battery) are included so views update correctly. Sequence diagram for updated bluetooth device state change notificationsequenceDiagram
actor User
participant BluetoothDevice
participant BluetoothDeviceModel
participant BluetoothView
User->>BluetoothDevice: Change connection state
BluetoothDevice-->>BluetoothDeviceModel: notifyStateChanged
BluetoothDeviceModel->>BluetoothDeviceModel: updateData(device)
alt device moved to top
BluetoothDeviceModel->>BluetoothDeviceModel: moveToTop(deviceId)
BluetoothDeviceModel->>BluetoothDeviceModel: createIndex(0, 0)
BluetoothDeviceModel-->>BluetoothView: dataChanged(index, index, Name, ConnectStatus, ConnectStatusText, Battery, BatteryIconPath)
else device reordered but not moved to top
BluetoothDeviceModel->>BluetoothDeviceModel: createIndex(index, 0)
BluetoothDeviceModel-->>BluetoothView: dataChanged(index, index, Name, ConnectStatus, ConnectStatusText, Battery, BatteryIconPath)
end
BluetoothView->>BluetoothView: Update name, connect status, battery, battery icon
Class diagram for BluetoothDeviceModel dataChanged roles updateclassDiagram
class BluetoothDevice {
+QString id
+QString name
+bool isConnected
+int batteryLevel
}
class BluetoothDeviceModel {
+updateData(device)
+moveToTop(deviceId)
+reorderDevices()
+createIndex(row, column)
+dataChanged(indexStart, indexEnd, roles)
}
class BluetoothDeviceRoles {
<<enumeration>>
Name
ConnectStatus
ConnectStatusText
Battery
BatteryIconPath
}
BluetoothDeviceModel --> BluetoothDevice
BluetoothDeviceModel --> BluetoothDeviceRoles
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review这段代码是蓝牙设备模型中更新设备数据的函数。让我对修改后的代码进行分析：
static const QVector<int> DeviceUpdateRoles = {
Name,
ConnectStatus,
ConnectStatusText,
Battery,
BatteryIconPath
};
void BluetoothDeviceModel::updateData(BluetoothDevice *device)
{
if (!device) {
return;
}
const int index = findDevice(device->id());
if (index < 0) {
return;
}
const bool isConnected = device->isConnected();
const static QVector<int> updateRoles = {
Name,
ConnectStatus,
ConnectStatusText,
Battery,
BatteryIconPath
};
if (isConnected && index > 0) {
moveToTop(device->id());
const QModelIndex modelIndex = createIndex(0, 0);
emit dataChanged(modelIndex, modelIndex, updateRoles);
} else if (!isConnected) {
reorderDevices();
} else {
const QModelIndex modelIndex = createIndex(index, 0);
emit dataChanged(modelIndex, modelIndex, updateRoles);
}
}这些改进提高了代码的可维护性和健壮性，同时保持了原有的功能。 |
![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 list of roles
{Name, ConnectStatus, ConnectStatusText, Battery, BatteryIconPath}is duplicated; consider extracting it into a small helper or constant to avoid divergence if roles change later.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The list of roles `{Name, ConnectStatus, ConnectStatusText, Battery, BatteryIconPath}` is duplicated; consider extracting it into a small helper or constant to avoid divergence if roles change later.
## Individual Comments
### Comment 1
<location> `src/plugin-bluetooth/operation/bluetoothdevicemodel.cpp:56` </location>
<code_context>
moveToTop(device->id());
QModelIndex modelIndex = createIndex(0, 0);
- emit dataChanged(modelIndex, modelIndex, {Name});
+ emit dataChanged(modelIndex, modelIndex, {Name, ConnectStatus, ConnectStatusText, Battery, BatteryIconPath});
} else if (!isConnected) {
reorderDevices();
</code_context>
<issue_to_address>
**suggestion:** Avoid duplicating the same role list literal in multiple dataChanged() calls.
The same role set is now used in multiple branches. To avoid duplication and keep them in sync, factor the roles into a shared constexpr/`static const` container (e.g. `kDeviceStatusRoles`) or a helper that returns the list, so future changes only need to be made in one place.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| moveToTop(device->id()); | ||
| QModelIndex modelIndex = createIndex(0, 0); | ||
| emit dataChanged(modelIndex, modelIndex, {Name}); | ||
| emit dataChanged(modelIndex, modelIndex, {Name, ConnectStatus, ConnectStatusText, Battery, BatteryIconPath}); |
There was a problem hiding this comment.
suggestion: Avoid duplicating the same role list literal in multiple dataChanged() calls.
The same role set is now used in multiple branches. To avoid duplication and keep them in sync, factor the roles into a shared constexpr/static const container (e.g. kDeviceStatusRoles) or a helper that returns the list, so future changes only need to be made in one place.
18202781743
approved these changes
Dec 25, 2025
caixr23
approved these changes
Dec 25, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, caixr23, wyu71 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
|
/forcemerge |
|
This pr force merged! (status: blocked) |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Log: emit all relevant roles on bluetooth device state change
pms: BUG-343983
Summary by Sourcery
Bug Fixes: