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

Missing docs for startNotification result. #1005

Open
fudom opened this issue Mar 8, 2024 · 1 comment
Open

Missing docs for startNotification result. #1005

fudom opened this issue Mar 8, 2024 · 1 comment

Comments

@fudom
Copy link
Contributor

fudom commented Mar 8, 2024

There is a lack of information in the startNotification docs.

It seems that the received result is an array [dataBuffer, nextSequenceNumber] (type: [ArrayBuffer, number]). There is an example with data[0]. But you don't see, why using the first index here. After digging into the code, I found that the second index is a next sequence number - whatever that is.

private PluginResult createSequentialResult(byte[] data) {
List<PluginResult> resultList = new ArrayList<PluginResult>(2);
PluginResult dataResult = new PluginResult(PluginResult.Status.OK, data);
PluginResult sequenceResult = new PluginResult(PluginResult.Status.OK, this.getNextSequenceNumber());
resultList.add(dataResult);
resultList.add(sequenceResult);
return new PluginResult(PluginResult.Status.OK, resultList);
}

I'm not sure what the result is on iOS. And is sequence number required? What is the use case? But at least, this should be visible in the docs.

In the types.d.ts is only the ArrayBuffer. Is this correct? I'm confused. Update: It seems to be correct. Not sure why the [dataBuffer, nextSequenceNumber]. Tested with withPromises.startNotification.

startNotification(
device_id: string,
service_uuid: string,
characteristic_uuid: string,
success: (rawData: ArrayBuffer) => any,
failure?: (error: string | BLEError) => any
): Promise<void>;

Btw. avoid any type. In this case, why should callback return any? Just use void. I would also be consistent on variable naming. The convention is JS is camelCase.

@peitschie
Copy link
Collaborator

@fudom the array there gets flattened out by the callback when returning to javascript. I.e., the success callback on the javascript side is effectively a function like function success(dataResult, sequenceResult).

Having said that, it looks like I broke the sequenceResult in the callback, and agree the typing needs to be updated to reflect this.

Just as an aside, the use of any in the callback return types makes it easier to write async callbacks (which are effectively functions that return promises). Some of these are slightly legacy features, but changing it to a stricter void return types would be a breaking change for some codebases (some of the ones I maintain would definitely require changes).

In general, I work very hard to ensure no breaking changes 🙂 .

I'll clarify the types and documentation around this. Thanks for noticing!

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

2 participants