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

Allow for configuration of transport and PHY on Android #90

Merged
merged 5 commits into from
Apr 28, 2021

Conversation

twyatt
Copy link
Member

@twyatt twyatt commented Apr 17, 2021

Enables for configuration of transport and PHY of bluetooth connections (Android only).

For more details on the parameters and what they mean, the following links have very helpful information:


Closes #61

@twyatt twyatt marked this pull request as ready for review April 17, 2021 19:44
@twyatt twyatt requested review from a team, sdonn3 and QuantumRand April 17, 2021 19:44
@twyatt twyatt changed the title Allow configuration of transport and PHY on Android Allow for configuration of transport and PHY on Android Apr 17, 2021
@twyatt twyatt requested a review from cedrickcooke April 20, 2021 06:38
Copy link
Contributor

@sdonn3 sdonn3 left a comment

Choose a reason for hiding this comment

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

appreciate the links giving context, super neat

Copy link
Contributor

@cedrickcooke cedrickcooke left a comment

Choose a reason for hiding this comment

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

Not blocking the PR on the comment, just something to consider.

Comment on lines 45 to 82
/** Preferred transport for GATT connections to remote dual-mode devices. */
public enum class Transport {

/** No preference of physical transport for GATT connections to remote dual-mode devices. */
Auto,

/** Prefer BR/EDR transport for GATT connections to remote dual-mode devices. */
BreDr,

/** Prefer LE transport for GATT connections to remote dual-mode devices. */
Le,
}

/** Preferred Physical Layer (PHY) for connections to remote LE devices. */
public enum class Phy {

/** Bluetooth LE 1M PHY. */
Le1M,

/**
* Bluetooth LE 2M PHY.
*
* Per [Exploring Bluetooth 5 – Going the Distance](https://www.bluetooth.com/blog/exploring-bluetooth-5-going-the-distance/#mcetoc_1d7vdh6b25):
* "The new LE 2M PHY allows the physical layer to operate at 2 Ms/s and thus enables higher data rates than LE 1M
* and Bluetooth 4."
*/
Le2M,

/**
* Bluetooth LE Coded PHY.
*
* Per [Exploring Bluetooth 5 – Going the Distance](https://www.bluetooth.com/blog/exploring-bluetooth-5-going-the-distance/#mcetoc_1d7vdh6b26):
* "The LE Coded PHY allows range to be quadrupled (approximately), compared to Bluetooth® 4 and this has been
* accomplished without increasing the transmission power required."
*/
LeCoded,
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Considering this is a bluetooth concept and not necessarily an android concept, should these be moved to commonMain?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. Though I still need to investigate what transports Core Bluetooth (e.g. iOS) and JavaScript support, or if it is even configurable on those platforms.

The closest thing to transport configuration I can find in Core Bluetooth is: https://developer.apple.com/documentation/corebluetooth/cbconnectperipheraloptionenabletransportbridgingkey

But I'm not sure if it is the same as Android's concept of choosing a preferred transport (BR/EDR vs. LE).

As for JavaScript, from what I can tell, it is not configurable at all.

So, if they are truly only configurable on the Android target, then I'll probably keep them in the Android source set.

For PHY, I can't find anything about being able to configure it via Core Bluetooth API, but it does look like perhaps the peripheral can request a different PHY and Core Bluetooth will handle it. 🤷


I've considered introducing an @ExperimentalApi annotation to help mark APIs that are still being investigated and are likely to change, but being pre-1.0.0, pretty much the entire API is experimental / subject to change. 🤷 🙃


@davidtaylor-juul do you happen to know what Core Bluetooth supports as being configurable (in re: to transport and PHY)?

Copy link
Contributor

Choose a reason for hiding this comment

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

There have been some releases in the past (mostly Betas IIRC) that do expose some options but in the current (iOS 14) the negotiation for PHY etc is handled by the framework and it will attempt to auto select whatever option is "best" (aka fastest).

Generally Core Bluetooth does not expose API for configuration at that level, apart from the backwards compatibility options already mentioned. I guess they consider that layer to be owned by the OS, so it's likely to remain this way in keeping with their philosophy.

You can select higher layer modes like L2CAP, but nothing below that sort of level is twiddle-able.

@twyatt twyatt enabled auto-merge (squash) April 28, 2021 04:53
@twyatt twyatt merged commit 151e54d into main Apr 28, 2021
@twyatt twyatt deleted the twyatt/transport+phy branch April 28, 2021 05:00
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.

Set transport flag to BluetoothDevice.TRANSPORT_LE by default
5 participants