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

Promise handling rewrite break BLE device use #2495

Closed
halemmerich opened this issue Apr 23, 2024 · 3 comments
Closed

Promise handling rewrite break BLE device use #2495

halemmerich opened this issue Apr 23, 2024 · 3 comments

Comments

@halemmerich
Copy link
Contributor

2v21.114 on Bangle 2 breaks bthrm and bthrmlite for me. It seems to boil down to NRF.requestDevice() not returning an actual promise and then failing because of the missing then-function. The promise itself seems to be run correctly since it prints unhandled errors like Uncaught Error: Unhandled promise rejection: No device found matching filters.

You can check with:

print(NRF.requestDevice({filters:[]}));

It prints { } on 2v21.114 while it prints Promise: { } on 2v21.

@gfwilliams
Copy link
Member

Eurgh. Ok, thanks. This'll be because of #2454

Stuff like this in BLE is done with blePromise = jspromise_create(); and then to complete we do jspromise_resolve(blePromise, data); or similar.

But looking at this, jspromise_create just creates an empty object you've called box, so it's not a promise and nothing like .then could be used with it?

So I'm not sure how this can really work as-is... @d3nd3 any thoughts?

I guess maybe you assumed jspromise_create wasn't used outside so changed what it does? While we could just return the promise like jswrap_promise_constructor does, then the box is just not referenced at all and freed immediately which seems like it's not the plan?

@d3nd3
Copy link
Contributor

d3nd3 commented Apr 23, 2024

@gfwilliams I was aware of jspromise_create() usage outside, I thought its fine since I made the resolve/reject functions take a prombox instead of a promise, but if they are returning the promise to javascript usercode it has to be adjusted to return the child promise which it contains.

JsVar *promise = jsvLockAgainSafe(blePromise);
  jsble_central_getPrimaryServices(jswrap_ble_BluetoothRemoteGATTServer_getHandle(parent), uuid);
  return jsvObjectGetChildIfExists(blePromise, JS_HIDDEN_CHAR_STR"prom");

Can you validate the locks? Not my speciality.

Mb its good practise if we name the variables prombox to make it clear also. Box is just a naming for container. Also, are there any other functions which return a promise like this?

#2454

@gfwilliams
Copy link
Member

In the code snippet above, I'm not sure what's going on with JsVar *promise = jsvLockAgainSafe(blePromise); lock-wise, but in the actual PR as far as I could tell the locks seemed ok (and the tests do check for dangling locks, so the fact they pass is a good sign).

Also, are there any other functions which return a promise like this?

A few I think - it's any Espruino function that creates and returns something to the user - so off the top of my head, the bluetooth connect (maybe disconnect), write, startNotifications, then on Bangle we've got beep, buzz, pressure. etc.

Basically it's a pretty big bunch of changes. I'll try and reply properly in #2454

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

No branches or pull requests

3 participants