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

Bluetooth #3131

Open
wants to merge 170 commits into
base: master
from

Conversation

@EvanBacon
Copy link
Contributor

commented Jan 6, 2019

Why

https://expo.canny.io/feature-requests/p/bluetooth-api

How

Not very committed to this API at the moment. iOS only right now.

Currently the listener system is very incomplete and needs some kind of tagging system to get updates regarding input events.

  • startScanAsync()
  • stopScanAsync()
  • connectAsync()
  • disconnectAsync()
  • discoverServicesAsync()
  • readAsync()
  • writeAsync()
  • updateAsync()
  • readRSSIAsync()
  • getPeripheralsAsync()
  • getCentralAsync()
  • isScanningAsync()
  • addListener()
  • removeSubscription()
  • removeAllListeners()

Test Plan

Added a screen to NCL that lets you poll for devices, connect/disconnect to any device, finally read or write to the device.

@EvanBacon EvanBacon self-assigned this Jan 6, 2019

@EvanBacon

This comment has been minimized.

Copy link
Contributor Author

commented Jan 6, 2019

Currently you can do an operation like:

Bluetooth.addListener(({event, data}) => {
  if (event === Bluetooth.Events.CENTRAL_DID_CONNECT_PERIPHERAL_EVENT) {
    // Peripheral connected
  } else if (event === Bluetooth.Events.CENTRAL_DID_FAIL_TO_CONNECT_PERIPHERAL_EVENT) {
    // Peripheral failed (should throw error?)
  }
})
Bluetooth.connectAsync(...);

But ideally you would do

try {
  const data = await Bluetooth.connectAsync(...);
} catch (error) {
}

@EvanBacon EvanBacon requested a review from ide Jan 6, 2019

packages/expo-bluetooth/src/Bluetooth.js Outdated Show resolved Hide resolved
packages/expo-bluetooth/src/Bluetooth.js Outdated Show resolved Hide resolved
packages/expo-bluetooth/src/Bluetooth.js Outdated Show resolved Hide resolved
packages/expo-bluetooth/src/Bluetooth.js Outdated Show resolved Hide resolved
packages/expo-bluetooth/src/Bluetooth.js Outdated Show resolved Hide resolved
packages/expo-bluetooth/ios/EXBluetooth/EXBluetooth.m Outdated Show resolved Hide resolved
packages/expo-bluetooth/ios/EXBluetooth/EXBluetooth.m Outdated Show resolved Hide resolved
CBCharacteristic *characteristic = [self characteristicFromUUID:characteristicUUID service:service prop:characteristicProperties];

if (characteristicProperties == CBCharacteristicPropertyNotify && !characteristic) {
characteristic = [self characteristicFromUUID:characteristicUUID service:service prop:CBCharacteristicPropertyIndicate];

This comment has been minimized.

Copy link
@ide

ide Jan 8, 2019

Member

Not sure we want to apply this logic -- I think it might be better to have the programmer specify Indicate | Notify themselves. What is this fallback supposed to do?

packages/expo-bluetooth/ios/EXBluetooth/EXBluetooth.m Outdated Show resolved Hide resolved
return nil;
}

- (CBCharacteristic *)characteristicFromUUID:(CBUUID *)UUID service:(CBService *)service prop:(CBCharacteristicProperties)prop

This comment has been minimized.

Copy link
@ide

ide Jan 8, 2019

Member

Write out:

Suggested change
- (CBCharacteristic *)characteristicFromUUID:(CBUUID *)UUID service:(CBService *)service prop:(CBCharacteristicProperties)prop
- (CBCharacteristic *)characteristicFromUUID:(CBUUID *)UUID service:(CBService *)service properties:(CBCharacteristicProperties)properties

I think it makes sense to document (or rename) whether this method is supposed to return a characteristic with any of the props or all of the props.

@watadarkstar

This comment has been minimized.

Copy link

commented Jan 8, 2019

Great to see this as a PR! Looking forward to seeing this progress! 🎉


@interface EXBluetooth (JSON)

+ (NSDictionary *)CBPeripheral_NativeToJSON:(CBPeripheral *)input;

This comment has been minimized.

Copy link
@ide

ide Jan 9, 2019

Member
Suggested change
+ (NSDictionary *)CBPeripheral_NativeToJSON:(CBPeripheral *)input;
+ (nullable NSDictionary *)JSONFromCBPeripheral:(nullable CBPeripheral *)input;

matches Cocoa conventions more closely

{
switch (input) {
case CBPeripheralStateDisconnected:
return @"disconnected";

This comment has been minimized.

Copy link
@ide

ide Jan 9, 2019

Member

On the JS/TS side we'll want to access these values via a TS enum (enum BluetoothPeripheralState { Disconnected = 'disconnected', ... }). I think it's OK here not to define a corresponding enum in Obj-C with items like EXPeripheralStateDisconnected but add the TS enum and a comment saying these values correspond to the enum.

{
NSMutableArray *output = [NSMutableArray new];
for (CBService *value in input) {
NSDictionary *serializedValue = [[self class] CBService_NativeToJSON:value];

This comment has been minimized.

Copy link
@ide

ide Jan 9, 2019

Member

[self class] works but I think it's harder to read than just writing out the class name and unless we expect EXBluetooth to be subclassed, writing out EXBluetooth is more straightforward

Suggested change
NSDictionary *serializedValue = [[self class] CBService_NativeToJSON:value];
NSDictionary *serializedValue = [EXBluetooth CBService_NativeToJSON:value];

+ (NSDictionary *)CBCentralManager_NativeToJSON:(CBCentralManager *)input
{
if (!input) return nil;

This comment has been minimized.

Copy link
@ide

ide Jan 9, 2019

Member

Shouldn't need this since the central manager is always non-nil when passed in

{
return @{
@"data": [input base64EncodedStringWithOptions:0],
@"type": @"ArrayBuffer"

This comment has been minimized.

Copy link
@ide

ide Jan 9, 2019

Member

This "type" field doesn't seem to help a lot since nothing else has a "type" field -- this is the only type. IMO we should remove this method, inline [input base64EncodedStringWithOptions:0], and on the JS side know that we need to convert the service data to ArrayBuffers.


+ (NSDictionary *)CBDescriptor_NativeToJSON:(CBDescriptor *)input;

+ (NSMutableArray *)CBPeripheralList_NativeToJSON:(NSArray<CBPeripheral *> *)input;

This comment has been minimized.

Copy link
@ide

ide Jan 9, 2019

Member

Some of the conversion methods use List while others use Array -> pick one unless this discrepancy is intentional

NSMutableArray *output = [NSMutableArray new];
for (id value in input) {
if ([value isKindOfClass:[NSString class]]) {
// TODO: Bacon: Maybe add support for generating CBUUID from Data (ArrayBuffer)

This comment has been minimized.

Copy link
@ide

ide Jan 9, 2019

Member

Probably should stick to strings, we're doing BLE I/O here, using strings vs. ArrayBuffers is not a meaningful performance comparison and in terms of an API IMO strings are more natural to work with


+ (NSMutableArray<CBUUID *> *)CBUUIDList_JSONToNative:(NSArray *)input
{
if (!input) return nil;

This comment has been minimized.

Copy link
@ide

ide Jan 9, 2019

Member

Should check that we don't get NSNull passed in as well..

EvanBacon added some commits Jan 9, 2019

Updated demo
* Invalidating peripherals when the central state is destroyed
* Added connection timeout to `connectAsync`
* Made `observeStateAsync` call the callback when it's added
* Removed `throw new Error('Unhandled transactionId');` in the case that bluetooth is turned off externally.
Added expo-bluetooth-utils
* updated demo to read in data
@OmniAsetty

This comment has been minimized.

Copy link

commented May 3, 2019

Thank you so much for working on this! Is this feature getting added in Expo v33? We are going to start working on Bluetooth app in June first week. Do you think this feature will be ready by then?

@zocheyado

This comment has been minimized.

Copy link

commented Jun 11, 2019

Is this feature going include bluetooth serial or just bluetooth low energy? Planning a future project but I suspect this will only be low energy.

@bbarthec bbarthec removed their request for review Jul 15, 2019

@josecarlosns

This comment has been minimized.

Copy link

commented Jul 18, 2019

I am working on a project now and the abscence of bluetooth support is the ONLY reason keeping me from using Expo to build my apps, for bluetooth support is essential in my app.

Also, will this feature support bluetooth serial? I hope it does.

@ide

This comment has been minimized.

Copy link
Member

commented Jul 19, 2019

This module is only for a subset of BLE, namely scanning for peripherals. Broadcasting as a peripheral is out of scope for this module. Additionally this module will be part of the bare workflow due to extra permissions needed to use Bluetooth. The native Bluetooth libraries are inconsistent/broken enough that doing a quality job on a subset of the BLE APIs is a big investment.

We plan not to work on Bluetooth serial due to the fact that it is complex to maintain despite the fact that few apps need it, and on iOS you need to enroll in MFi.

To use Expo with Bluetooth serial you would use the bare workflow and then write your own Bluetooth serial module.

@EvanBacon EvanBacon requested a review from sjchmiela as a code owner Aug 8, 2019

EvanBacon added a commit that referenced this pull request Aug 8, 2019

EvanBacon added a commit that referenced this pull request Aug 8, 2019

[bluetooth] Create a module stub for expo-bluetooth (#5250)
* [WIP] Added expo-bluetooth from #3131

* ignore expo-bluetooth in the client

* Created expo-bluetooth stub

* Removed apps and pods

* Update Project.xml

* Update UMDefines.h

* Added build files

* Update Bluetooth-test.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
10 participants
You can’t perform that action at this time.