Skip to content
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

Added: BLE Serial Port Profile #103

Closed
wants to merge 18 commits into from

Conversation

Witty-Wizard
Copy link

@seeul8er, I have created this Pull Request for review purposes, this is still a work in progress, as stated in #97, I managed to get data from Betaflight Configurator. I will do full testing before submitting the final PR

@Witty-Wizard Witty-Wizard marked this pull request as ready for review March 14, 2025 17:20
@Witty-Wizard Witty-Wizard marked this pull request as draft March 14, 2025 17:21
@Witty-Wizard
Copy link
Author

The read_process_serial_link is trying to do a lot at once, its a bit misleading as well. it might be better if this function could be refactored to a cleaner version, with separate handlers for different protocols.

@Witty-Wizard
Copy link
Author

Update

image
Got data form BLE to UART.
It uses queue for sending data, similar to ESP NOW queue for this reason creating a global queue that can handle both BLE and ESP NOW queues

Refactor : Formated Headers

Refactor : Formated esp32 control module

Refactor : Formated esp32 control module

Refactor: Cleaned up some more files
@Witty-Wizard
Copy link
Author

Update

This PR is done for review, the BLE write task was assigned a higher priority causing issues with large packets, it works now with betaflight over BLE

@Witty-Wizard Witty-Wizard marked this pull request as ready for review March 16, 2025 18:31
@Witty-Wizard
Copy link
Author

Oops, Looks like I need to change the SDK Configuration files.

@Witty-Wizard
Copy link
Author

Got it working

Need to do something about the latency, i suspect queues are making it slow
https://github.com/user-attachments/assets/b6ee252b-7219-4c5a-919d-517c71f35593

@Witty-Wizard
Copy link
Author

https://github.com/Witty-Wizard/ESP32-DroneBridge/blob/f982386756cade35aebef7eec1fbe5ea8d9b8bd2/main/db_ble.h#L43
I am using this struct for sending data between tasks, but looks like the buffer is too big making the communication a little slow

@Witty-Wizard
Copy link
Author

@seeul8er, please test this on your end as well.
I am also facing an issue where, the data communication just stops, I think the packets are not fragmented leading to the configurator to act weird. If you have any idea on how to fix that it will be great.

I am also looking at the possibility of flashing the flight controller over Bluetooth, for that I have created 2 new characteristics for reading and writing commands but this would require changes in the configurator as well. I will talk with the Betalight developers as they are also working on Bluetooth flashing.

* Added: BLE Serial Port Profile

* Added: bluetooth control module (as much as i understood)

* Added: Sending data from BLE callback to uart

* Fixed: Error on Wi-Fi Connection

* Added: task fro getting data from queue

* Added: Global queues for BLE task (Need to do some testing, and then refractor the code)

* Refactor: Cleaned up code with comments and documentation

Refactor : Formated Headers

Refactor : Formated esp32 control module

Refactor : Formated esp32 control module

Refactor: Cleaned up some more files

* Refactor: Cleaned esp now files

* Removed: .vscode directory (When did i push that lol)

* Added: NimBLE Config to sdkconfig files

* Refactor: Cleaned up db_serial files

* Fixed: BLE Task not yielding to scheduler

* Added: Notify and read characterisitc for betaflight

* Added: Global Queues (need to shift esp now queues)

* Added: Command Characteristics (Might be useful later for flashing purposes)

* Added: Pushing data pointer to queue to save transfer time
@Witty-Wizard
Copy link
Author

@seeul8er, how to update the sdkconfig files, BLE is not enabled in those. I am new to the ESP-IDF thing.

@seeul8er
Copy link
Contributor

I will check today and tomorrow 👍

@Witty-Wizard
Copy link
Author

@seeul8er, are u planning to merge https://github.com/DroneBridge/ESP32/tree/v2.0dev into main, if yes then please do so that I can rebase my changes on top of it, this will be much easier and quicker

@seeul8er
Copy link
Contributor

Hi, yes I do. Sorry for the delay I had a lot of other things to do in the last days. I hope I can integrate it in the coming two days.

@seeul8er
Copy link
Contributor

@Witty-Wizard merged into master

@Witty-Wizard
Copy link
Author

@Witty-Wizard merged into master

great, I will create a new PR, that would be easier than resolving the conflicts.

@seeul8er
Copy link
Contributor

seeul8er commented Mar 30, 2025

@Witty-Wizard For the new request it would be nice if you could focus it on the BLE addition. In the previous one there were a lot of editorial changes and formatting changes. This makes it hard for me to follow and I am not sure I want half the project reformatted. Even when some of these made good sense 😊

@Witty-Wizard
Copy link
Author

Witty-Wizard commented Mar 30, 2025

@seeul8er, Yes I had the same thoughts, I still need to have a global queue or i can implement a queue specifically for BLE and then maybe later on we can have a global queue that is common for both BLE and ESPNOW.

In a later PR I would still suggest some format changes, as the code readability is not that good. I can see that as the project progressed some changes could have been handled in a better and more readable way. NVM for now I'll just make changes for the BLE part

@Witty-Wizard
Copy link
Author

@seeul8er, you can check #108

@seeul8er
Copy link
Contributor

@Witty-Wizard For now we can do separate Queues. We can check in a later step to fuse them

@Witty-Wizard
Copy link
Author

@Witty-Wizard For now we can do separate Queues. We can check in a later step to fuse them

Great, that's what I have done

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.

2 participants