feat: Allow to acces and choose the Hardware Adapter to use #189
Conversation
Define a Information only interface for a Hardware Adapter. These info should also be available if other adapter-libs are added.
Enhance the "Adapter" interface with methods to get and define he HW Adapter to use
Adds the HW Adapter Index as option to the constructor like other "non Webbluetooth spec" options and set this.
In order to allow getting list of adapter we expose a "adapter" specific method. If you have a better idea just tell me, but for me this was the one that fits the "mostly compatible webbbluetooth interface" the best.
|
Is HardwareAdapter better than just Adapter? I know there's an internal adapter class, but since users don't see that, I'm wondering if the shorter Other than that, I like this idea. |
|
The naming is exactly one topic to discuss. You are right with "internal iuse of Adapter vs external use" - in fact I also thought that for library devs it should be clear and there Adapter is highly overlapping. But I'm very open to change. @thegecko WDYT? For your secondcomment: In fact the code shoud throw on "useHardwareAdapter" call as soon as "this.adapter" is set already. This happens in ... so it should already be as you proposed. |
There was a problem hiding this comment.
Thanks for this PR @Apollon77, I like the approach.
I've made a few minor comments, but agree with @Symbitic that calling the interface Adapter may be better. This may need you to merge the adapter interfaces.
| @@ -53,6 +53,24 @@ export class SimplebleAdapter extends EventEmitter implements BluetoothAdapter { | |||
| private descriptors = new Map<string, string[]>(); | |||
| private charEvents = new Map<string, (value: DataView) => void>(); | |||
|
|
|||
| getHardwareAdapters() { | |||
There was a problem hiding this comment.
| getHardwareAdapters() { | |
| public getHardwareAdapters() { |
Please make this explicitly public
| @@ -53,6 +53,24 @@ export class SimplebleAdapter extends EventEmitter implements BluetoothAdapter { | |||
| private descriptors = new Map<string, string[]>(); | |||
| private charEvents = new Map<string, (value: DataView) => void>(); | |||
|
|
|||
| getHardwareAdapters() { | |||
| return getAdapters().map( | |||
There was a problem hiding this comment.
I assume this is to reduce the exposed fields?
There was a problem hiding this comment.
Yes, because we do not want to expose the whole "adapter instance", but just the informative fields. I can add a comment
| identifier: string; | ||
| address: string; | ||
| active: boolean; | ||
| export interface Adapter extends HardwareAdapterDetails { |
There was a problem hiding this comment.
Is it worth merging these types?
There was a problem hiding this comment.
To have a clearly typed separateion between whats exposed as "information" and whats a full adapte rinstance I would leave it separated. Or what do you mean?
There was a problem hiding this comment.
If we change names I would rather rename HardwareAdapterDetails to just AdapterDetails ...
But yes this naming overlapping that we have "native BLE adapters" aka "Hardware level" and "BLE providers like Noble OR SimpleBLE" (at least structurewise) that are also called "Adapters is difficult.
I could imagine renaing Adapter -> "Provider" (or maybe anopther better word to remove this multi-use of "Adapter" for the SimpleBle/Noble level and keep Adapter for the "native level". WDYT?
| ); | ||
| } | ||
|
|
||
| useHardwareAdapter(adapterIndex: number) { |
There was a problem hiding this comment.
| useHardwareAdapter(adapterIndex: number) { | |
| public useHardwareAdapter(adapterIndex: number) { |
Please make this explicitly public
|
I will try to update here with all comments from above the next days. |
Any progress @Apollon77 ? |
|
did not found time yet, but your comment pushed it up in the queue gg |
This PR adds the option to query the available "Hardware BLE Adapters" (I did not wanted to say USB sticks because it could also be UART, so if you have a better name that do not conflict with "Adapter" already used just say :-) ) and define which one to use.
The Constructor of the WebBluetooth class is used to allow to specify the hadware adapter to use as optional option. If the option is not defined it will initialize the adapter HW with id 0 as before too laziely on first real use.
Additionally the method "getSimpleBleHardwareAdapters()" is exported and can be used. I added an example to show the usage. It outputs the following on my raspi
fixes #178