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

Simple BLE #46

Merged
merged 49 commits into from May 30, 2023
Merged

Simple BLE #46

merged 49 commits into from May 30, 2023

Conversation

thegecko
Copy link
Owner

@thegecko thegecko commented Dec 29, 2022

Switch backend to use SimpleBLE

@thegecko thegecko changed the title Simple ble Simple BLE Dec 29, 2022
@RReverser
Copy link

RReverser commented Mar 11, 2023

Just tried this branch on Windows and it was so much easier to get going with it than the published version. Can't wait for it to become upstream :)

@RReverser
Copy link

Can't wait for it to become upstream :)

FWIW I fried the device board in an unrelated accident since writing this comment, so looks like I can wait after all 😅

@Symbitic
Copy link
Contributor

Sorry, forgot to push. It's in my simpleble-port branch.

@thegecko
Copy link
Owner Author

Erk, was hoping for a drop-in fix with some wiring, but it looks like the entire cpp interface has changed :/

@Symbitic
Copy link
Contributor

Yeah, sorry. Objects turned out to be necessary because the only way to get callbacks to work was with function references.

If you don't want to change your code, I can make whatever changes you want to my branch so you can merge it. I changed it so that new Bluetooth works by making it work closer to yours (adapters.ts exports an array of adapters which the constructor in Bluetooth uses).

@thegecko
Copy link
Owner Author

thegecko commented May 7, 2023

Sorry, forgot to push. It's in my simpleble-port branch.

Thanks, I have grabbed your latest bindings and updated these on this branch:

https://github.com/thegecko/webbluetooth/tree/simple-ble-update

Although everything compiles, I'm getting this error when a device is found:

#
# Fatal error in HandleScope::HandleScope
# Entering the V8 API without proper locking in place
#

I seem to remember having this issue before when using the found callback so the adapter previously used the simpleble_adapter_scan_get_results_handle instead.

Your new API no longer exposes this method, so I'll see if I can fix the issue...

@thegecko
Copy link
Owner Author

thegecko commented May 7, 2023

UPDATE: Just found the peripherals property, d'oh!

@thegecko
Copy link
Owner Author

thegecko commented May 7, 2023

OK, so I have your latest updates in to this branch and working as well as before. I much prefer this interface, very clean 👍

Unfortunately I still see errors for the found, indicate and notify callbacks when triggered:

#
# Fatal error in HandleScope::HandleScope
# Entering the V8 API without proper locking in place
#

I'm still using polling to get round the found issue, but have no workarounds for indicate/notify.

BTW, I had to fix a couple of issues in your code:

  • Info is zero-based: f3b0650
  • UUIDs need the termination character removed before passing to TypeScript: 2f25429

Any other ideas for fixing these callback issues?

@Symbitic
Copy link
Contributor

Symbitic commented May 7, 2023

I'll take a look. I'm on vacation, so I don't have access to a Windows laptop. I'll definitely keep working on this, though. Can you try building my branch and see if it has the same error? That will help narrow it down. If you still do, I'll send you some debug statements to add to peripheral.cpp to identify where the problem is.

@thegecko
Copy link
Owner Author

thegecko commented May 8, 2023

I'll take a look. I'm on vacation, so I don't have access to a Windows laptop. I'll definitely keep working on this, though. Can you try building my branch and see if it has the same error? That will help narrow it down. If you still do, I'll send you some debug statements to add to peripheral.cpp to identify where the problem is.

My Windows machine doesn't have bluetooth so all my dev/testing is done on macOS.

I tried your branch. I can build it, but can't work out how to run anything due to the module system

> deno run examples/deno/scan.ts  
error: Module not found "file:///.../webbluetooth/mod.ts".
    at file:///.../webbluetooth/examples/deno/scan.ts:3:26
> node examples/selector.js                             
file:///.../webbluetooth/examples/selector.js:86
const bluetooth = new Bluetooth({ deviceFound });
      ^

SyntaxError: Identifier 'bluetooth' has already been declared
    at ESMLoader.moduleStrategy (node:internal/modules/esm/translators:115:18)
    at ESMLoader.moduleProvider (node:internal/modules/esm/loader:289:14)
    at async link (node:internal/modules/esm/module_job:70:21)
> ts-node --esm examples/scan.ts                        
Error [ERR_UNKNOWN_BUILTIN_MODULE]: No such built-in module: node:readline/promises
    at new NodeError (node:internal/errors:371:5)
    at ESMLoader.builtinStrategy (node:internal/modules/esm/translators:258:11)
    at ESMLoader.moduleProvider (node:internal/modules/esm/loader:289:14) {
  code: 'ERR_UNKNOWN_BUILTIN_MODULE'

As the lib folder between our branches are the same, I would expect to see the same issues if I got this running.

@Symbitic
Copy link
Contributor

The Deno port is not working at all ATM in my branch (I decided to wait until I got the Node stuff finished). The selector.js example in my branch isn't working because the scan in Bluetooth class doesn't use new Promise so selectFn won't work.

Which version of Node.js are you using? node:readline/promises was added in v12 and became stable in v14. I suppose I could remove that and use the older version of readline if needed.

I am getting that V8 error when using your branch. Looking into it. This would all have been so much easier if ThreadSafeFunction just worked.

@Symbitic
Copy link
Contributor

I think I finally got it! The problem with ThreadSafeFunction was that it would block the program from exiting. I managed to solve that with HandleScope and Unref. There can sometimes be a delay (<1s) but all callbacks, including setCallbackOnScanStart and setCallbackOnConnected worked for me.

Should be much safer now that it's using TSFN.

@thegecko
Copy link
Owner Author

thegecko commented May 27, 2023

Nice work! I tried your update earlier and saw callbacks working. I'll update the lib and tests on this branch.

@thegecko thegecko marked this pull request as ready for review May 27, 2023 19:40
@thegecko
Copy link
Owner Author

Thanks to the awesome efforts from @Symbitic , this is ready to be merged. I'll leave it open for a few days for comment.

@thegecko thegecko merged commit 4a56b50 into master May 30, 2023
6 checks passed
@thegecko thegecko deleted the simple-ble branch May 30, 2023 17:58
@Symbitic
Copy link
Contributor

I think this looks great! Good job!

I'll close my PR and start a new fork to start adding things incrementally, like removing Node-specific things like EventEmitter so I can start on Deno compatibility later. For now, the important thing is: webbluetooth no longer requires sudo on Linux and installing custom drivers on Windows! 🥳

@thegecko
Copy link
Owner Author

thegecko commented Jun 1, 2023

webbluetooth@3.0.2 has just been released containing the new SimpleBLE backend.

I've tested this on MacOS (the only system I have access to), would love to hear how this works for others!

@Symbitic
Copy link
Contributor

Symbitic commented Jun 2, 2023

Tested on Linux. Looks good to me.

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

Successfully merging this pull request may close these issues.

None yet

3 participants