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

Returns DataView instead of ArrayBuffer on readValue #172

Closed
beaufortfrancois opened this issue Oct 7, 2015 · 14 comments
Closed

Returns DataView instead of ArrayBuffer on readValue #172

beaufortfrancois opened this issue Oct 7, 2015 · 14 comments

Comments

@beaufortfrancois
Copy link
Member

I've realized that all the arrayBuffer I receive from readValue are always wrapped around a DataView and I was wondering if we could simply return a DataView instead of an ArrayBuffer then.

Instead of

characteristic.readValue().then(buffer => {
  let data = new DataView(buffer);
  let foo = data.getUint8(0);
  ...

We would have something simpler like

characteristic.readValue().then(data => {
  let foo = data.getUint8(0);
  // We can always access ArrayBuffer if we need to with data.buffer
  ...
@jyasskin
Copy link
Member

https://heycam.github.io/webidl/#idl-buffer-source-types says "When designing APIs that create and return a buffer, it is recommended to use the ArrayBuffer type rather than Uint8Array." However, I agree that the DataView is more convenient for most uses of Characteristic values, since they tend to be heterogenous.

@heycam, is that bit of WebIDL intended to discourage returning DataViews too? Is this something we should bring up on public-script-coord or just change here? Thanks.

@heycam
Copy link

heycam commented Oct 12, 2015

The suggestion there is to dissuade people from using Uint8Array as a "generic buffer of data" type, when ArrayBuffer is more appropriate for that. If you want you can provide a DataView, but is it possible that for a given characteristic it might be more convenient for the author to have a different typed array view of the buffer, and so you should leave the choice of whether to wrap the buffer in a DataView or one of the other types up to them?

@jyasskin
Copy link
Member

It's definitely possible that some characteristics will be easier to use as a UInt8Array, in which case the author would need to write new UInt8Array(view.buffer) explicitly. Some characteristics may also store an array of 16- or 32-bit integers, but in those cases authors will need to use DataView over UInt16Array because they'll need to select the byte ordering of the remote data, rather than their platform's ordering. (Worse, most platforms are little-endian, which matches the common Bluetooth ordering, so if authors use UInt16Array, they'll get the right answers on their development machines but break on big-endian implementations ... if any of those exist.)

I don't have data on this, but I expect the balance of convenience points toward returning DataView.

@heycam
Copy link

heycam commented Oct 12, 2015

OK. If you want to return DataView then I think it's fine.

@jyasskin
Copy link
Member

Thanks!

@beaufortfrancois
Copy link
Member Author

@beaufortfrancois happy ;)

@beaufortfrancois
Copy link
Member Author

@g-ortuno
Copy link
Contributor

Could we get this in the spec first before we implement it? We don't want to introduce discrepancies between the browser and the spec specially now that other browsers are implementing.

@jyasskin
Copy link
Member

It'll be mailed today.

jyasskin added a commit to jyasskin/web-bluetooth-1 that referenced this issue Jan 19, 2016
This makes it easier to access most Bluetooth values, since they encode
little-endian values of different sizes, rather than a uniform sequence of
platform-endian values.
jyasskin added a commit that referenced this issue Jan 19, 2016
Fix #172: Return DataViews instead of ArrayBuffers.
@thegecko
Copy link
Contributor

I'm aware this specification targets web browsers, but in situations where the specification may be implemented in different environments, using more specific data structures can make conversion and usage difficult, .e.g. node:

https://github.com/thegecko/bleat/blob/web-bluetooth/dist/bleat.noble.js

@g-ortuno
Copy link
Contributor

How would returning a DataView make conversion more difficult? Converting from an ArrayBuffer to DataView is fairly easy:

var buffer = new ArrayBuffer(2);
var dataView = new DataView(buffer);

Similarly from DataView to ArrayBuffer:

var buffer = dataView.buffer;

What am I missing?

@thegecko
Copy link
Contributor

OK, I spent a bit of time investigating this and node supports DataViews. If I come across other JS engines which supports ArrayBuffers but not DataViews, I'll comment here. My concern was simply that using more specific constructs in the target (web) environment could make implementing this specification in other JS environments more difficult. Thanks!

@jyasskin
Copy link
Member

DataView is part of Javascript, so I wouldn't expect any other JS engines to support ArrayBuffer but not it. However, if you notice us depending on any part of HTML that standalone JS engines don't support, do let us know, and we'll try to work something out.

@thegecko
Copy link
Contributor

OK, no problem

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

5 participants