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

Replace noble with simpleble #44

Closed
wants to merge 9 commits into from

Conversation

Symbitic
Copy link
Contributor

This is a port to replace Noble with SimpleBLE.

Unlike Noble, SimpleBLE requires no special drivers on Windows and no root permissions on Linux.
Porting it to this project required rewriting a significant portion of the code. On the other hand, it simplifies type safety and is compatible with Deno and browsers.

Some other notes:

  • It requires Node 14 or later because of my SimpleBLE bindings. Might be able to lower it with SimpleBLE patches, but it really shouldn't be that much of an issue.
  • ESM only. This requires a modern version of Node anyway.

This port isn't finished yet. SimpleBLE is still missing a few things, like service filtering.
I thought I'd submit this so you can look at it first and see if it's something you are opposed to.

@thegecko
Copy link
Owner

Great, I'll try to find some time to test this out.

The bump in module format and node version would likely require a major bump in the version of this library. Out of interest, which NAPI version do you target with simpleble?

@Symbitic
Copy link
Contributor Author

Symbitic commented Nov 5, 2022

NAPI 7. My SimpleBLE bindings require bigint, so it requires a modern version of Node.

Since it requires using a version of Node that supports ESM anyway, I figured it may as well use modern standards that Node supports, like ESM, AbortController, DOMException and export interface BluetoothDevice + export class BluetoothDevice for type-safe events.

By removing Node-specific stuff like EventEmitter, it's also easier to make Deno and browser bindings for truly cross-runtime.

@thegecko
Copy link
Owner

thegecko commented Nov 6, 2022

@Symbitic I spent some time understanding the architecture here. You have a library - Node-SimpleBLE - which is a node/deno/bun library that wraps the SimpleBLE project.

This is similar to how I used to have the Node-WebUSB library set up to consume node-usb which in turn uses libusb.

Ultimately we realised that maintaining this chain can be quite complex, so the WebUSB library was merged into node-usb.

I think SimpleBLE looks like a great replacement for noble as it targets the same platforms and is maintained.
I'd be keen to follow the pattern we used in node-usb and consume it directly. This could be new work or potentially by merging your node-simpleble project into this one.

To ensure maximum compatibility, I'd like to keep the NAPI version as low as possible (although we may not be able to get lower than 6) and the Node.js target as low as possible. I'm not sure where the esm requirement comes in, but node-usb targets NAPI 6 and Node.js >v10.20.0 , so this should be possible.

I'd also be keen to ship the native binaries using prebuildify/prebuildify-cross to simplify installation.

Does this approach sounds sensible? If so, I'd be keen to help with the prebuildify setup, TypeScript work and API compatibility. I have lesser skills with c/c++/deno, so that could be your area of expertise?

@Symbitic
Copy link
Contributor Author

Symbitic commented Nov 6, 2022

@thegecko I have no problem with that. I'll experiment and try to see how low I can get the targeted Node.js version, then try to merge node-simpleble into this project.

It already uses prebuild (see https://github.com/Symbitic/node-simpleble/releases/tag/v1.0.1), but it can't use prebuildify because (1) it uses CMake and (2) it has to build non-Node artifacts, like the libsimpleble.(so|dylib|dll) libraries that are used for Deno/Bun FFI. Porting to GYP might be possible, but it will be trickier.

Not sure they used prebuildify-cross specifically, but there are cmake.js projects that do cross builds for every platform. I'd already planned on adding that to simpleble once I got everything else ready.

If you can think of a way to make prebuildify work, I'd really appreciate the help.

Another thing I'd appreciate help with is the interfaces/classes. As I said above, using the EventMap + interface + class method allows type-safe events in fewer LOC and removes the need for TypedDispatcher, but it requires that everything except Bluetooth be in gatt.ts, which is 900+ lines (in counting). If you can find a way to split that across multiple files, I'd really appreciate it.

ESM-only isn't a strict requirement per se; it's just that all our lives will be better when we no longer need to worry about CJS. Node 12 and above support ESM; CJS is only needed if Node 10 is needed. I can add that if you think it's necessary.

No rush; until getting services before connecting (see OpenBluetoothToolbox/SimpleBLE#100) is done, service filtering isn't possible, so it isn't ready yet.

Thanks for replying and offering to merge my bindings. I think a single cross-platform, cross-runtime Bluetooth library that works anywhere and doesn't require any setup is going to make a big difference.

@thegecko
Copy link
Owner

Apologies on the silence here, I did try to get cmakejs working (I'm used to using node-gyp), but with no success. Will try to revisit this at the weekend.

@Symbitic
Copy link
Contributor Author

No problem. The branch I'm working on should build with cmakejs. I'm working on getting service filtering done for the weekend.

@thegecko
Copy link
Owner

I still failed to set up cmake-js, but also noticed that it isn't supported by prebuildify :(

I'm keen to ship binaries with the package, but the alternatives for cmake-js (or build agnostic) don't seem very popular.

Is there any compelling reason cmake-js is used rather than node-gyp? Does it make deno development easier?

@Symbitic
Copy link
Contributor Author

Symbitic commented Nov 27, 2022

It's needed for building SimpleBLE. Prebuildify doesn't support cmake-js, but prebuild-install does.
I use that for node-simpleble - just run npm install --save simpleble, and it should install without needing to build.

One problem for both cmake-js and node-gyp: SimpleBLE requires libdbus-1-dev and libsystemd0-dev on Linux. No problem when building in the Ubuntu x86_64 VM on GitHub, but for arm and aarch64, it's much trickier.

CMake supports cross-compiling; I'm experimenting with downloading the .deb files for dbus and systemd and extracting the library files for linking. I'll keep you posted.

UPDATE: I have aarch64 working with cross-compilation. I couldn't get prebuild Docker builds working in GitHub workflows because of dbus errors. Cross compiler says "libc++ does not work on multi-threaded ARM yet" so no armv6 yet.

@thegecko
Copy link
Owner

thegecko commented Dec 4, 2022

Update from my side, still slow progress, sorry.

I managed to get cmake-js building and have undertaken a small PoC around tidying up the bindings loading and exposing the native functions along with the types in a similar way to node-usb. It's all looking pretty neat.

I want to understand the restrictions mentioned regarding exposing this is a module for deno as I'd like to maintain compatibility if possible.

I will now likely focus on the typescript side of things and rewrite the existing webbluetooth API with async/await using the simpleble bindings.

@Symbitic
Copy link
Contributor Author

Symbitic commented Dec 5, 2022

Deno doesn't support NAPI. It has to use FFI. Also, Deno requires a filename extension (e.g. import x from "./x.ts").

The denoify tool takes care of the later (adds the extension to import x from "./x"). The later is why I made a SimpleBLE binding class. On Node, it wraps the NAPI bindings. On Deno, it wraps the FFI. It presents the same class in both cases, which allows both to be used.

I have API compatibility 90% taken care of and I'm working on adding the rest. I'm also replacing BigInt with External so it should work with older versions of NAPI.

@thegecko
Copy link
Owner

thegecko commented Dec 5, 2022

Deno doesn't support NAPI. It has to use FFI...

OK, I'd like to split this into a separate work thread and not change too much in one go.

I'll create a new v3 branch for us to collaborate on, feel free to continue your work here, too.

We can then open smaller PRs there to:

  • Bring in SimpleBLE
  • Rewrite /tidy the TS code
  • Release if ready
  • Add Deno/bun support
  • etc.

This ensures a manageable process to continue to drive things forwards.

@Symbitic
Copy link
Contributor Author

Hey @thegecko

Thought I'd give some updates.

I'm mostly waiting on SimpleBLE at this point. The Eddystone example won't work until service data and service-filtering is added. I made a PR for that, and I'll update this as soon as it's merged.

Long story short: new Bluetooth() won't work, but since that doesn't work in web browsers (throws "Illegal constructor") anyway, I don't think that's too much of an issue. A quick search of code on GitHub says that only 2 repos are using it at the moment.

I've still got several more things to take care of, but it's looking pretty good at the moment.

@thegecko
Copy link
Owner

thegecko commented Jan 19, 2023

Thought I'd give some updates.

Thanks.

From my side, I updated the dependencies used in #45 and have been working on just switching noble for simpleble in #46

This mostly works (CI w/ cmake, prebuilds, etc.) and there are ~14 TODOs left to fix on the branch.

Your SimpleBLE changes should bring this to a usable state and once merged, we can look at the ffi/deno stuff.

@thegecko
Copy link
Owner

new Bluetooth() won't work

This is designed to be a node - only constructor to allow the user to specify a selection UI and recreate the popup you get for the native browser version

@thegecko
Copy link
Owner

@thegecko
Copy link
Owner

I see that service advertisement data has landed in SimpleBLE v0.6.0.

I've updated the PR over in #46 to use this new release and expose the data.

It looks like the Web-bluetooth spec has changed since I wrote this library, though and exposing this data is now done differently. I'll need to fix this.

I also need to implement characteristic notifications/indicates

There's only a few items left to fix/implement in SimpleBLE before that branch can be merged and we'll have the initial version:

  • Event on adapter state changed (simpleble_adapter_set_callback_on_updated is wired to scan updates)
  • Service included services (getIncludedServices)
  • Missing characteristic capabilities (broadcast, authenticatedSignedWrites, reliableWrite, writableAuxiliaries)

Do you know if any of these are currently possible in SimpleBLE?

@Symbitic
Copy link
Contributor Author

Not sure what "Service included services" means. For the others, I'll take a look when I get home.

@thegecko
Copy link
Owner

Not sure what "Service included services" means.

Webbluetooth exposes a getIncludedServices call which is implemented in the noble adapter here:

https://github.com/thegecko/webbluetooth/blob/master/src/adapters/noble-adapter.ts#L231

For the others, I'll take a look when I get home.

Awesome!

@Symbitic
Copy link
Contributor Author

Sorry for taking so long to respond.

It looks like SimpleBLE doesn't have included services or any of the characteristic capabilities yet. I'll look into adding them next. For the first one, what are you looking for?

@Symbitic
Copy link
Contributor Author

For the first one, what are you looking for?

noble implements it here:

https://github.com/abandonware/noble/blob/6748de3e1837bbfc90353a503948128f316c0e77/lib/win/src/ble_manager.cc#L298

https://github.com/abandonware/noble/blob/6748de3e1837bbfc90353a503948128f316c0e77/lib/mac/src/ble_manager.mm#L174

That's for included services. I already know about that, and I'm working on adding it. I meant to ask what are you looking from adapter events? Detecting if the adapter has been powered off or has it's state changed?

@thegecko
Copy link
Owner

Detecting if the adapter has been powered off or has it's state changed?

Yes, if that's possible, but its not required for the spec.

@Symbitic
Copy link
Contributor Author

I submitted a PR to some more characteristic capabilities and included services. Some notes:

  • The included_services array is just an array of UUIDs, not objects. Getting that to work in the C (and Rust/Python) bindings safely would be more trouble than it's worth ATM. getIncludedServices might be in the WebBluetooth spec, but Chromium doesn't implement it yet.
  • I couldn't find reliableWrite or writableAuxiliaries in noble, but I did add all the other capabilities. can_write_command = write without response, can_write_authenticated = authenticatedSignedWrites, can_broadcast = broadcast, and has_extended_properties.
  • Adding state change detection would be hard because it's not consistent across all three platforms. Since this isn't required by the spec, I don't consider it a priority.

Anything else still missing?

@thegecko
Copy link
Owner

I submitted a PR to some more characteristic capabilities and included services. Some notes:

ref:OpenBluetoothToolbox/SimpleBLE#228

nice!

The included_services array is just an array of UUIDs

OK, lets see what we can do in the JS side when it lands

I couldn't find reliableWrite or writableAuxiliaries in noble, but I did add all the other capabilities

👍

Adding state change detection would be hard because it's not consistent across all three platforms

Perhaps we should be able to detect the error thrown when the adapter is missing or disabled and give a hint to the user?

Anything else still missing?

The last piece is the notification/indication events on characteristics I think. I haven't wired that up yet, so can't confirm it works. I don't think there is any more C/C++ work for now though

@kdewald
Copy link

kdewald commented Mar 19, 2023

Hello! I just found out this project exists! This is really cool!

I just started looking at the MR, there are a few issues that need to be fixed which might require some API changes, but we should be good to land this soon.

Once the switch is made to SimpleBLE, could you also add a small referral link? Thanks!

@Symbitic
Copy link
Contributor Author

Once the switch is made to SimpleBLE, could you also add a small referral link? Thanks!

I'd have no problem with that. I was already working on using SimpleBLE as bindings to create a webbluetooth library for Deno when it occurred to me it would do more good to make one that worked on Node as well. Since this project uses Noble (which requires custom drivers on Windows and sudo permissions on Linux) I decided to try porting it to SimpleBLE. Thanks again for a great cross-platform BLE library!

I just started looking at the MR, there are a few issues that need to be fixed which might require some API changes, but we should be good to land this soon.

I have no problem with that. If any changes to my PR are needed, just let me know.

@thegecko
Copy link
Owner

thegecko commented Mar 20, 2023

Adding state change detection would be hard ... Since this isn't required by the spec...

Unfortunately, this is used by getAvailability() and onavailabilitychanged in the spec.

@Symbitic
Copy link
Contributor Author

Very well. I'll see what I can do.

@thegecko
Copy link
Owner

Very well. I'll see what I can do.

That would be great, I don't think it should block our upgrade, though

@Symbitic Symbitic closed this Jun 3, 2023
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.

3 participants