-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Comms: Add BluetoothLE Link #13340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Comms: Add BluetoothLE Link #13340
Conversation
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
src/Comms/BluetoothLink.cc
Outdated
// dev.setRssi(info.rssi()); | ||
// } | ||
// if (updatedFields & QBluetoothDeviceInfo::Field::ManufacturerData) { | ||
// const QMultiHash<quint16, QByteArray> data = info.manufacturerData(); | ||
// for (quint16 id : info.manufacturerIds()) { | ||
// dev.setManufacturerData(id, data.value(id)); | ||
// } | ||
// } | ||
// if (updatedFields & QBluetoothDeviceInfo::Field::ServiceData) { | ||
// const QMultiHash<QBluetoothUuid, QByteArray> data = info.serviceData(); | ||
// for (QBluetoothUuid uuid : info.serviceIds()) { | ||
// dev.setServiceData(uuid, data.value(uuid)); | ||
// } | ||
// } | ||
// if (updatedFields & QBluetoothDeviceInfo::Field::All) { | ||
// dev = info; | ||
// } | ||
// break; | ||
// } | ||
// } | ||
// } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Large block of commented-out code should be removed rather than left in the codebase. If this functionality is needed in the future, it can be retrieved from version control history.
// dev.setRssi(info.rssi()); | |
// } | |
// if (updatedFields & QBluetoothDeviceInfo::Field::ManufacturerData) { | |
// const QMultiHash<quint16, QByteArray> data = info.manufacturerData(); | |
// for (quint16 id : info.manufacturerIds()) { | |
// dev.setManufacturerData(id, data.value(id)); | |
// } | |
// } | |
// if (updatedFields & QBluetoothDeviceInfo::Field::ServiceData) { | |
// const QMultiHash<QBluetoothUuid, QByteArray> data = info.serviceData(); | |
// for (QBluetoothUuid uuid : info.serviceIds()) { | |
// dev.setServiceData(uuid, data.value(uuid)); | |
// } | |
// } | |
// if (updatedFields & QBluetoothDeviceInfo::Field::All) { | |
// dev = info; | |
// } | |
// break; | |
// } | |
// } | |
// } |
Copilot uses AI. Check for mistakes.
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. |
26b515b
to
8378bb9
Compare
@DonLakeFlyer if I just get something usable down would you make any changes/fixes to the QML? |
Sure, no problem |
8378bb9
to
eb6baa8
Compare
Add Bluetooth Low Energy Comm Link.
Closes #12884
TODO: Add RSSI?