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

Consider API breaking changes. #21

Closed
3 of 6 tasks
Cierpliwy opened this issue Oct 24, 2016 · 9 comments
Closed
3 of 6 tasks

Consider API breaking changes. #21

Cierpliwy opened this issue Oct 24, 2016 · 9 comments

Comments

@Cierpliwy
Copy link
Contributor

Cierpliwy commented Oct 24, 2016

Consider following API breaking changes before going 1.0:

  • Move all advertisement data related fields (including name and rssi) to advertisement object which will be included in Device if possible.
  • Change (error, T) => void callback types to (Result<T>) => void which will allow to apply better Flow type checking.
  • Change Device.uuid to Device.id as it's not UUID
  • Convert iOS library to Swift 3
  • Consider filtering out value updates from notification listener when read is performed on iOS. In that case behaviour will be in line with Android.
  • Change Base64 API to ArrayBuffer and do conversion on implementation side. This will allow to optimize it in further RN versions on iOS10+.
@Cierpliwy Cierpliwy added this to the Stable version 1.0 milestone Oct 24, 2016
@brycejacobs
Copy link

@Cierpliwy Any update on the base64 API? Is there any consideration to disabling the encoding / decoding?

We have a scenario with some embedded devices, running on a TI cc2650, that is fighting for bytes. The base64 encoding / decoding is a killer for when you want to send raw bytes across the wire.

@Cierpliwy
Copy link
Contributor Author

In our internal project we are currently doing Data -> Base64 -> Data which is definitely suboptimal and I would love to remove that step for library. RN team doesn't seem to work on it unfortunately:
facebook/react-native#1424.

Maybe I could check if it is difficult to add binary bridging between JSC and native modules, but to be honest I don't have so much time at my disposal.

As a side note: still when you are passing Base64 to API calls it is converted to binary form by native libraries used by react-native-ble-plx and then send to BLE radio. So you can definitely manipulate bytes and there is only Data -> Base64 -> Data inefficiency during bridging.

@brycejacobs
Copy link

Hm, maybe I read the source wrong.

I was under the assumption, that if I were to read a characteristic on a remote peripheral, that the library assumes the data received is in base64. Is this correct? If it's simply reading the data, then encoding to base64 for the trip back to js land then that should be fine, and may actually be the best way.

The scenario that I'm in, is where I have a device out in the wild that's broadcasting some advertisement data when not connected, that serves as service data, that contains some status values. I actually just booted back up the prototype, let me test my theory and get back to you before I waste any more time.

@Cierpliwy
Copy link
Contributor Author

Data is read from peripheral in binary form and converted to Base64 only when it needs to be sent to JS side, so everything should be fine.

@rclai
Copy link

rclai commented Jun 10, 2017

Yeah, you don't have to send base64 data to your TI cc2650.

@jchirschy
Copy link

Hi @Cierpliwy,

As I read this issue, could you confirm that data written to or read from a characteristic is not base64 encoded ? That means, peripheral doesn't need to encode or decode base64 ?

@Cierpliwy
Copy link
Contributor Author

@jchirschy That's correct

@brycejacobs
Copy link

brycejacobs commented Jun 27, 2017 via email

@Cierpliwy
Copy link
Contributor Author

Closing in favour of #299

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants