Comms: Add BluetoothLE Link #13340
Comms: Add BluetoothLE Link #13340
Conversation
HTRamsey
force-pushed
the
dev-comms-ble
branch
from
201d480 to
26b515b
Compare
|
@Davidsastresas Should there be a toggle between Bluetooth and BLE or just scan whatever is available? |
There was a problem hiding this comment.
Pull Request Overview
This PR adds Bluetooth Low Energy (BLE) support to the existing Bluetooth communication link functionality. The changes extend the current classic Bluetooth implementation to support both classic Bluetooth and BLE modes through a unified interface.
Key changes:
- Added BLE mode selection to BluetoothConfiguration with enum for ModeClassic and ModeLowEnergy
- Implemented BLE controller and service management in BluetoothWorker alongside existing classic socket handling
- Enhanced device discovery to filter devices based on selected mode (classic vs BLE)
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/UI/AppSettings/BluetoothSettings.qml | Minor QML formatting fixes and brace corrections |
| src/Comms/LinkConfiguration.h | Added logging category declaration for LinkConfiguration |
| src/Comms/LinkConfiguration.cc | Implemented logging category and updated debug statements |
| src/Comms/CMakeLists.txt | Added formatting whitespace after Bluetooth source files |
| src/Comms/BluetoothLink.h | Major refactor to support both classic and BLE modes with new properties and methods |
| src/Comms/BluetoothLink.cc | Complete implementation of BLE functionality alongside existing classic Bluetooth support |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Thanks! To be honest I have very little experience with these devices, so I don't know if I am the best to answer that. |
HTRamsey
force-pushed
the
dev-comms-ble
branch
from
26b515b to
8378bb9
Compare
|
@DonLakeFlyer if I just get something usable down would you make any changes/fixes to the QML? |
|
Sure, no problem |
HTRamsey
force-pushed
the
dev-comms-ble
branch
from
8378bb9 to
eb6baa8
Compare
HTRamsey
force-pushed
the
dev-comms-ble
branch
2 times, most recently
from
c8ab8d5 to
d3aff7d
Compare
HTRamsey
force-pushed
the
dev-comms-ble
branch
from
d3aff7d to
808399d
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (modelData !== "") | ||
| subEditConfig.setDevice(modelData) | ||
| QGCLabel { | ||
| text: (paired, subEditConfig.getPairingStatus(subEditConfig.address)) |
There was a problem hiding this comment.
The text binding syntax is incorrect. The comma operator in QML text binding creates a tuple which won't display properly. Should be: text: subEditConfig.getPairingStatus(subEditConfig.address)
| text: (paired, subEditConfig.getPairingStatus(subEditConfig.address)) | |
| text: subEditConfig.getPairingStatus(subEditConfig.address) |
Copilot uses AI. Check for mistakes.
| readonly property bool isPaired: (paired === paired) && | ||
| subEditConfig.mode === BluetoothConfiguration.BluetoothMode.ModeClassic && |
There was a problem hiding this comment.
The condition (paired === paired) is always true and serves no purpose. This appears to be an attempt to create a dependency on the paired property for re-evaluation, but this doesn't work as intended. Consider using a proper dependency mechanism or remove this redundant check.
| readonly property bool isPaired: (paired === paired) && | |
| subEditConfig.mode === BluetoothConfiguration.BluetoothMode.ModeClassic && | |
| readonly property bool isPaired: subEditConfig.mode === BluetoothConfiguration.BluetoothMode.ModeClassic && |
Copilot uses AI. Check for mistakes.
| if (_mode == BluetoothMode::ModeLowEnergy) { | ||
| const QList<QBluetoothUuid> serviceUuids = info.serviceUuids(); | ||
| if (!serviceUuids.isEmpty()) { | ||
| _serviceUuid = QBluetoothUuid(); |
There was a problem hiding this comment.
Setting _serviceUuid to a null UUID and then immediately checking if it's null in the condition below will always be true. This logic appears incorrect - the service UUID should only be reset if no known services are found, not before checking for them.
Copilot uses AI. Check for mistakes.
3 participants
Add Bluetooth Low Energy Comm Link.
Closes #12884
TODO: Add RSSI?