Basic bluetooth linking implementation by bburgess19 · Pull Request #1 · skiptools/skip-bluetooth

bburgess19 · 2024-08-30T17:42:48Z

Overview

This PR contains a limited but functional implementation Apple's CoreBluetooth of Bluetooth Low Energy (BLE) for Android.

Because CoreBluetooth's implementation is limited in many ways, i.e. not allowing certain advertisement data, this implementation also disables those features to emulate an experience as similar as possible.

Features

With these changes, it is possible to perform critical functions of BLE:

Peripheral

Configuring services and characteristics
Adding services to GATT database (CBPeripheralManager.add())
Advertising and stopping advertising
Receiving (un)subscription notifications
Receiving reads/writes and responding to them
Updating characteristic values (whose changes propagate to subscribed centrals)

Central

Scanning and stopping scanning for peripherals
- Some limited configuration for this is possible
Connecting to peripherals
Discovering services/characteristics
Reading/writing to characteristics
Subscribing to characteristics
Cancelling peripheral connections

Next Steps

Ancillary API Delegate Implementations

There are a number of API calls which aren't strictly necessary in BLE communication which aren't implemented. As of the time of this PR, these include:

CBPeripheralDelegate

func peripheral(_ peripheral: CBPeripheral, didReadRSSI RSSI: NSNumber, error: (any Error)?)
func peripheralDidDiscoverIncludedServicesFor(_ peripheral: CBPeripheral, didDiscoverIncludedServicesFor service: CBService, error: (any Error)?)
func peripheralDidDiscoverDescriptorsFor(_ peripheral: CBPeripheral, didDiscoverDescriptorsFor characteristic: CBCharacteristic, error: (any Error)?)
func peripheralDidUpdateValueFor(_ peripheral: CBPeripheral, didUpdateValueFor descriptor: CBDescriptor, error: (any Error)?)
func peripheralDidWriteValueFor(_ peripheral: CBPeripheral, didWriteValueFor descriptor: CBDescriptor, error: (any Error)?)
func peripheralIsReady(toSendWriteWithoutResponse peripheral: CBPeripheral)
func peripheral(_ peripheral: CBPeripheral, didOpen channel: CBL2CAPChannel?, error: (any Error)?)

CBPeripheralManagerDelegate

func peripheralManager(_ peripheral: CBPeripheralManager, willRestoreState dict: [String : Any])
func peripheralManager(_ peripheral: CBPeripheralManager, didAdd service: CBService, error: (any Error)?)

CBCentralManagerDelegate

func centralManager(_ central: CBCentralManager, willRestoreState dict: [String : Any])

There are a smattering of other fields and properties which are also unimplemented, but they are notated with @available(*, unavailable) which will signal compile-time failures.

Better advertisement cleanup logic

Apple's CoreBluetooth implementation ostensibly cleans up all GATT database values after both invoking CBPeripheralManager.stopAdvertising() and the instance is de-initialized, but since we cannot reliably hook into the finalize lifecycle in Skip, there is no guarantee the equivalent teardown function BluetoothGattServer.cleanup() function would be called if it was placed in there. There may be some clever solution here to guarantee the cleanup() function is called at a similar point in the lifecycle, but I am not aware of one.

Testing

Testing these APIs is quite challenging because their functionality requires hardware, and I am unfamiliar with testing to this end. This is a rich area for development going forward.

It's challenging to anticipate how vast the full implementation will be, but this is a version which compiles `CBCentralManager` and `CBPeripheralManager` with minimal implementations.

cla-bot · 2024-08-30T17:42:51Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Ben Burgess.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot · 2024-08-30T17:58:49Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Ben Burgess.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

aabewhite

Amazing! I left just a few comments

aabewhite · 2024-08-30T18:01:08Z

Sources/SkipBluetooth/CBCentralManager.swift

+        scanner?.stopScan(scanDelegate)
+    }
+
+#if !SKIP


You have a number of #if !SKIP blocks, but the whole thing is surrounded by #if SKIP, so they aren't functional. This pattern is repeated in other files. Is this your way of excluding things that aren't implemented? If so, why not use @available(*, unavailable) for these? Do the signatures use types that aren't implemented or similar?

Sometimes in those cases we change the unimplemented types to Any (i.e. func f(x: Any /* OriginalType /) and mark @available(, unavailable) to hopefully give a better error message on attempted use. But fine either way - just checking that I understand.

These are remnants from my initial implementation. I've replaced these sites with @available(*, unavailable) references instead. Sorry for the confusion here.

aabewhite · 2024-08-30T18:10:36Z

Sources/SkipBluetooth/CBPeer.swift

@@ -0,0 +1,12 @@
+#if SKIP
+import SkipFoundation


import Foundation

Changed all other references as well.

aabewhite · 2024-08-30T18:27:31Z

Sources/SkipBluetooth/Utility.swift

+/// This process is handled automatically by CoreBluetooth for Swift when you instantiate either
+/// `CBCentralManager` or `CBPeripheralManager`, but Android requires the request
+/// to occur within an activity.
+@Composable


Is this the only reason the package relies on SkipUI?

If so, I think it's worth moving this to documentation and dropping the dependency. What do you think?

Come to think of it, we should probably add a general permission check utility to SkipUI itself... but that's another topic.

Ah that dependency was vestigial! It's removed.

Actually, it does look like we need to depend on SkipUI, since that brings in the compose dependencies we need for androidx.activity.result.contract.ActivityResultContracts and other things that are being used. That seems to be the only thing stopping CI from passing (https://github.com/skiptools/skip-bluetooth/actions/runs/10640472849), so if you add back the SkipUI dependency, then we should get a clean CI run.

I'd rather not include the permission prompt util and not depend on SkipUI. What do you think?

Oh yeah, if askForBluetoothPermissions() is removed, then it looks like the androidx.activity.compose and other imports can be removed, and it can just depend on skip-foundation.

Removing that one function works for me. It's changed.

I think you just need to remove the import androidx.core… and import androidx.activity.compose… from CBCentralManager.swift, and it looks like the CI run should pass (see the failure at https://github.com/skiptools/skip-bluetooth/actions/runs/10641143687).

Ah I see. I wasn't checking the CI output -- fixed. Not sure what's causing the cla-bot failures though... Thank you for the help thus far!

marcprux · 2024-08-30T18:34:41Z

Sources/SkipBluetooth/CBATTRequest.swift

@@ -0,0 +1,42 @@
+import Foundation


Can you add the LGPL license header to this and the other files so they all have the same header?

Sure thing, completed.

marcprux · 2024-08-30T18:39:39Z

This looks great! We've added a couple of comments, but overall it seems like a really solid PR.

Also, the weird "cla-bot" errors are indicating that it wants you to add your username to https://github.com/skiptools/clabot-config

Once those are in place, we'll be ready to merge and we can cut a new release. Thanks!

cla-bot · 2024-08-30T23:30:25Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Ben Burgess.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot · 2024-08-30T23:35:19Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Ben Burgess.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot · 2024-08-30T23:40:23Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Ben Burgess.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot · 2024-08-31T01:09:52Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Ben Burgess.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot · 2024-08-31T01:38:16Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Ben Burgess.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot · 2024-08-31T01:47:11Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Ben Burgess.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

marcprux · 2024-08-31T02:07:08Z

CI passed, looks good! I'll go ahead and merge and create a tagged release.

FTR, not entirely sure what the cla-bot error is about (since you added your name there) – it looks like it might be happening from a lack of e-mail address, either configured in GitHub (https://github.com/settings/emails) or from your local git config (git config --global user.email email@example.com). In any case, it isn't a blocker for this, but it might be an ongoing annoyance if you make future PRs.

Thanks again for the contribution!

Ben Burgess added 12 commits August 8, 2024 23:25

feat: initial API scaffolding

4602d13

It's challenging to anticipate how vast the full implementation will be, but this is a version which compiles `CBCentralManager` and `CBPeripheralManager` with minimal implementations.

feat: refactor logic into separate files

d0a2d6c

feat: setup and teardown scanning connections

16a29f5

chore: add SkipUI as dependency

bc9182c

feat: setup and teardown advertisement connections

0f4cba0

feat: bluetooth permission request API

06c12ce

refactor: hasPermission to Utility class

e6adfaa

fix: naming collisions on API names

b0f5233

feat: customize advertisement packets

5fe399b

chore: update Package.swift

dc6d790

chore: update documentation

7e98b7f

feat: basic functional implementation complete

477ba36

chore: update documentation

b33883c

aabewhite reviewed Aug 30, 2024

View reviewed changes

marcprux reviewed Aug 30, 2024

View reviewed changes

bburgess19 added 4 commits August 30, 2024 19:27

fix: remove unnecessary SkipUI dependency

79e6c68

chore: add licensing headers

7a46861

refactor: redundant #if !SKIP blocks

dc1e876

chore: update documentation

9db4b8b

Update README.md

2e8efc4

Update README.md

58a2d8f

refactor: remove SkipUI utility logic

365e0ba

refactor: remove unused imports

3d91142

chore: update documentation

eb061d4

marcprux merged commit 3c1ff7b into skiptools:main Aug 31, 2024

Uh oh!

Conversation

bburgess19 commented Aug 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Features

Peripheral

Central

Next Steps

Ancillary API Delegate Implementations

CBPeripheralDelegate

CBPeripheralManagerDelegate

CBCentralManagerDelegate

Better advertisement cleanup logic

Testing

Uh oh!

cla-bot bot commented Aug 30, 2024

Uh oh!

cla-bot bot commented Aug 30, 2024

Uh oh!

aabewhite left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcprux commented Aug 30, 2024

Uh oh!

cla-bot bot commented Aug 30, 2024

Uh oh!

cla-bot bot commented Aug 30, 2024

Uh oh!

cla-bot bot commented Aug 30, 2024

Uh oh!

cla-bot bot commented Aug 31, 2024

Uh oh!

cla-bot bot commented Aug 31, 2024

Uh oh!

cla-bot bot commented Aug 31, 2024

Uh oh!

marcprux commented Aug 31, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bburgess19 commented Aug 30, 2024 •

edited

Loading