Use of Send prevents peripheral from being returned from function

[adzy2k6]

I am attempting to return Peripheral from a generic setup function, so that I can use this as a generic framework for some hardware I am testing. The Send trait prevents peripheral from being made into a trait object, which stops me from being able to return the peripheral from the function. This may be down to my inexperience with rust, but it does appear to be a weakness of the library.

[lyneca]

I'm having the same problem, trying to make a struct that contains a peripheral.

[lyneca]

I don't think it's Send that's the problem; I'm pretty sure that it's Clone that stops it from being made into a trait object. However, I'm a relative noob at rust - so I'm not 100% sure.

[LucaCerina]

I have noticed that there are two Peripheral structs in the Peripheral. One is api::Peripheral, the other is in bluez/adapter, but it is not directly accessible through the library.

This is definitely a issue in the library's quality

[bleggett]

Well, there aren't 2 Peripheral structs, there's a Peripheral struct (the one in bluez/adapter) and the Peripheral trait (the one under api::Peripheral)

@LucaCerina is correct, the former is not publicly accessible, the latter is.

With @lyneca's change I am able to use api::Peripheral as a trait object and e.g. store a Vector of Peripherals.

Peripherals are tied to the adapter instance, you can't create your own Peripheral structs by definition, and the list of Peripherals maintained by the CentralAdapter is constantly being updated, so exposing the trait and not the struct that implements it makes a certain amount of sense AFAICT.

(that being said the underlying implementation of ConnectedAdapter clone()s all the peripherals before returning them, which kinda undercuts that assumption)

I think a way to fix this might be to have the ConnectedAdapter peripheral helper methods return PeripheralDescriptors instead of references to concrete instances of Periperals discovered by the ConnectedAdapter, and move the public connect method out of the Peripheral trait, implementing it on the ConnectedAdapter instead, where it would merely take a BDAddr and connect to one of the private Peripheral instances maintained by ConnectedAdapter.

That way the internal collection of Peripheral structs would remain private to a ConnectedAdapter instance, which seems much cleaner (not an expert so this may be totally wrong)

[mwylde]

The idea behind the api::Peripheral trait is to eventually make the library agnostic to the underlying bluetooth implementation; ideally you'd be able to write the same code and target windows and other platforms. Therefore the linux/bluez specific stuff is hidden in the bluez package, while library consumers are only able to interact with the (hopefully) generic rumble::api stuff.

I think the only people interested in creating new peripheral structs would be somebody implementing rumble support for a new bluetooth stack, which hopefully would be contributed back to the library.

As far as how to manage and expose the underlying peripherals, that's something I've got back and forth on. The original design (around 22f734f23925e52e90927928072761963118d9b1) looked a lot like you proposed, with a struct that was just a static snapshot of the state of the peripheral (then called Device). However I found that somewhat hard to manage when actually writing applications; all actions had to be taken against the manager referencing the peripheral id.

I'm currently in the middle of a major rework to use async io, which should make a lot of this cleaner internally (io will be handled in a central place instead of having various threads in the manager and peripherals reading off of various sockets), and very happy to hear ideas about how to also make the external interface better.

[bleggett]

@mwylde Cool! Great to hear you're in the middle of a rework.

Ok, that's good to know that you started with peripheral snapshots+most functionality handled thru the ConnectedAdapter and then moved way from it, I started making some stabs at doing this in my fork and began to suspect as much.

It is slightly cumbersome to do everything through the connected adapter, but it models the underlying reality more closely (you do have to do everything through the connected adapter, regardless of the BT implementation you use, that's how BTLE Central mode works, and information collected about peripherals is always going to be a snapshot of sorts) and I'm not sure there's a non-leaky abstraction that will allow API consumers to write code as if that isn't the case.

My main complaint (which I'm sure you're keenly aware of) is that you have to include both rumble::bluez and rumble::api in any library consumer in order to do anything useful, and ideally I should be able to do everything I need to do by including rumble::api alone, as a start.

I've started trying to think about how that might look in a branch, if I come up with something that seems clean and reasonable I'll propose it.

[mwylde]

That's great! I'm excited to see what you come up with.