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

Support bluetooth connections #156

Merged
merged 10 commits into from
Feb 9, 2024
Merged

Support bluetooth connections #156

merged 10 commits into from
Feb 9, 2024

Conversation

jcapona
Copy link
Contributor

@jcapona jcapona commented Sep 26, 2023

Status Ticket/Issue
Hold Ticket

Main changes

Support bluetooth connections by adding a GATT server with a service and several characteristics to interact with the frontend app.

Screenshots (feature, test output, profiling, dev tools etc)

[insert screenshots here]

Other notes (e.g. implementation quirks, edge cases, questions / issues)

Manual testing tips

  • Download deb
  • Make sure that the unstable apt repository is installed. You can install it by running:
sudo apt update
sudo apt install pi-top-os-unstable-apt-source
# Run again to update source
sudo apt update
  • Install deb artifact by running sudo apt install ./<file>. It should automatically fetch the new dependencies from the unstable repository.
  • Restart the further-link and bluetooth services
sudo systemctl daemon-reload
sudo systemctl restart further-link
sudo systemctl restart bluetooth
  • Install miniscreen app using the deb file from this PR.
  • Enable pairing mode from Overview page of miniscreen app
  • Use this frontend branch for testing.

Tag anyone who definitely needs to review or help

further_link/util/bluetooth/messages.py Fixed Show fixed Hide fixed
further_link/__main__.py Fixed Show fixed Hide fixed
further_link/util/bluetooth/device.py Fixed Show fixed Hide fixed
tests/e2e/test_bluetooth_run_lib.py Fixed Show fixed Hide fixed
tests/e2e/test_bluetooth_run_lib.py Fixed Show fixed Hide fixed
further_link/util/bluetooth/device.py Fixed Show fixed Hide fixed
@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

Attention: 171 lines in your changes are missing coverage. Please review.

Comparison is base (0d6e230) 72.83% compared to head (fd182a9) 73.52%.

Files Patch % Lines
further_link/endpoint/run.py 72.82% 25 Missing ⚠️
further_link/util/bluetooth/pairing.py 0.00% 25 Missing ⚠️
further_link/util/bluetooth/encryption.py 0.00% 24 Missing ⚠️
further_link/util/state.py 54.54% 20 Missing ⚠️
further_link/util/bluetooth/service.py 88.15% 18 Missing ⚠️
further_link/util/bluetooth/utils.py 37.93% 18 Missing ⚠️
...er_link/util/bluetooth/messages/chunked_message.py 84.74% 9 Missing ⚠️
further_link/util/bluetooth/messages/format.py 87.69% 8 Missing ⚠️
further_link/util/bluetooth/server.py 82.50% 7 Missing ⚠️
further_link/util/ipc.py 58.82% 7 Missing ⚠️
... and 3 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #156      +/-   ##
==========================================
+ Coverage   72.83%   73.52%   +0.68%     
==========================================
  Files          25       38      +13     
  Lines        1119     1760     +641     
==========================================
+ Hits          815     1294     +479     
- Misses        304      466     +162     
Flag Coverage Δ
unittests 73.52% <74.88%> (+0.68%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

further_link/__main__.py Outdated Show resolved Hide resolved
further_link/__main__.py Outdated Show resolved Hide resolved
further_link/__main__.py Outdated Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved

# remove trailing commas
message_str = re.sub(",[ \t\r\n]+}", "}", message_str)
message_str = re.sub(",[ \t\r\n]+]", "]", message_str)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we shouldn't really need this - the clients should send valid json

Copy link
Contributor

@angusjfw angusjfw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When sending as well as as chunking outgoing messages we probably need to handle buffering additional messages that are being sent while the previous one is still going out. An alternative would be to support simultaneous messages which are tagged by a message id.

Once we have finalised the chunking protocol we should add unit tests for it - we'll need to implement it in javascript and python so we want it to be pretty stable.

@angusjfw
Copy link
Contributor

We discussed going with a message id approach rather than buffering. We'll drop support for unchunked messages. So chunked messages have the format:
[2-bytes id][3-bytes end chunk index][3-bytes chunk index][data]

receiving logic is like:

  • if there's no open message with the id create a new one
  • if there is an open message with the id use that one
  • append the chunk (for now if it's not the next expected index that's an error, could start skipping these messages)
  • if the chunk index is the end chunk index close and handle the message

@jcapona jcapona force-pushed the SWE-186-bluetooth branch 2 times, most recently from a8c2c00 to ce0da00 Compare September 26, 2023 19:14
@angusjfw
Copy link
Contributor

run steps:

  • install deb
  • sudo pip3 install dbus-next bless
sudo apt install -y bluez

# Run bluetooth service in experimental mode
sudo sed -i '/^ExecStart=/ExecStart=/usr/libexec/bluetooth/bluetoothd --experimental' /lib/systemd/system/bluetooth.service
sudo sed -i '/^ExecStart=/ExecStart=/usr/libexec/bluetooth/bluetoothd --experimental' /etc/systemd/system/bluetooth.service
sudo systemctl daemon-reload
sudo systemctl restart bluetooth```

@jcapona jcapona force-pushed the SWE-186-bluetooth branch 2 times, most recently from 20dcd75 to dcedfe4 Compare November 20, 2023 19:15
@angusjfw angusjfw marked this pull request as ready for review February 1, 2024 18:10
@jcapona jcapona force-pushed the SWE-186-bluetooth branch 4 times, most recently from 49ae29e to 5db75ff Compare February 6, 2024 14:32
tests/e2e/test_bluetooth_run.py Fixed Show fixed Hide fixed
tests/e2e/test_bluetooth_run.py Fixed Show fixed Hide fixed
tests/e2e/test_bluetooth_run.py Fixed Show fixed Hide fixed
tests/e2e/test_bluetooth_run.py Fixed Show fixed Hide fixed
@jcapona jcapona force-pushed the SWE-186-bluetooth branch 3 times, most recently from 6756fe8 to ab20fb2 Compare February 6, 2024 14:47
@jcapona jcapona mentioned this pull request Feb 6, 2024
@jcapona jcapona merged commit d07779d into master Feb 9, 2024
10 checks passed
@jcapona jcapona deleted the SWE-186-bluetooth branch February 9, 2024 17:43
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