Remove C: Controller from HostResources<C>

[ivmarkov]

(Mentioned ~ 3 months ago.)

I guess the idea of HostResources is similar to embassy_net::StackResources which I think is a solid approach.

However - and because HostResources is generified by C - it simply cannot be stored in a static context, or in fact in any context which has a longer lifetime than the controller itself (as the controller most often than not sometimes is itself not : 'static, or if it can be made : 'static via a series of make_static calls to its parameters it means the BT stack runs forever).

I guess it is no coincidence that the examples have the HostResources allocated on-stack and not from a static cell - the later is not possible IMO.

Removing the controller from the host resources will have multiple benefits:

• Resources can then be allocated in a static or in general - in a context with a longer lifetime that the controller and then re-used (pooled) for multiple controller instantiations (that's what I need)
• Resources' constant will not take rodata space given that the QoS parameter is also removed. Currently it spoils the party, as it is neither all-0-bits, nor MaybeUninit and is the only such one in the struct

[jamessizeland]

My example project here allocates it in a static cell, which works okay where I can select the controller non-generically. https://github.com/jamessizeland/microbit-ble-gamepad/blob/main/src%2Fble%2Fgatt.rs#L49-L51

[ivmarkov]

My example project here allocates it in a static cell, which works okay where I can select the controller non-generically. https://github.com/jamessizeland/microbit-ble-gamepad/blob/main/src%2Fble%2Fgatt.rs#L49-L51

Sure but (a) the SoftDevice controller is 'static (b) your example runs forever.

Look at the esp32 controller.

It can be made 'static by calling mk_static on the wifi "init" and then the controller itself can be made static but that's exactly what I want to avoid, because I want to tear down the BT stack once it is done.

[ivmarkov]

The relevant PR that (I think) fixed a similar lifetime issue in embassy-net is this one. Bullet no (5) in the PR description specifically.

[lure23]

Would like to add one more benefit:

• easier for newcomers to adapt to their (likely single-platform) applications

[jamessizeland]

Yep I agree it would be worth making it able to be torn down.

[lulf]

The relevant PR that (I think) fixed a similar lifetime issue in embassy-net is this one. Bullet no (5) in the PR description specifically.

The above PR is actually the basis of the current design. However, I've not found a nice way to decouple the Controller trait from the resources, that still allows it to be allocated on the stack like for embassy-net. In contrast to embassy-net, which only needs to store a byte buffer for sockets that the drivers push/pull from, the bt-hci Controller defines all possible HCI commands that allows skipping serialization/deserialization.

Maybe we should just require HostResources to be static for now, and find a different way to make it possible to stack-allocate it.

Regarding PacketQos, I'd like to get rid of that entirely, as it's not as useful in practice as I originally intended.

[ivmarkov]

The above PR is actually the basis of the current design. However, I've not found a nice way to decouple the Controller trait from the resources, that still allows it to be allocated on the stack like for embassy-net.
In contrast to embassy-net, which only needs to store a byte buffer for sockets that the drivers push/pull from, the bt-hci Controller defines all possible HCI commands that allows skipping serialization/deserialization.

I'm not sure I follow how from this it follows that the Controller instance itself must be stored in the HostResources.
With that said, the code-base is complex and I might be missing something, so if you could elaborate?

Specifically, why is it that this field has to live inside BleHost and from there - inside HostResources as well? Can't it just be a standalone thing which is referenced by whoever needs it? As in server, etc?

Maybe we should just require HostResources to be static for now, and find a different way to make it possible to stack-allocate it.

Regarding PacketQos, I'd like to get rid of that entirely, as it's not as useful in practice as I originally intended.

I'm not sure I follow this either. Your examples do exactly that - stack allocate both the HostRespources instance and the Controller instance. However - and because HostResources contains the Controller, their lifetimes are bound (equal, if you wish). In other words:

• if HostResources is stack-allocated, the controller should as well, and they both have the same lifetime
• If HostResources has a static lifetime, so should the controller

And that's exactly the problem. We should strive to somehow make it possible for HostResources to outlive Controller I think.

[lulf]

The above PR is actually the basis of the current design. However, I've not found a nice way to decouple the Controller trait from the resources, that still allows it to be allocated on the stack like for embassy-net.
In contrast to embassy-net, which only needs to store a byte buffer for sockets that the drivers push/pull from, the bt-hci Controller defines all possible HCI commands that allows skipping serialization/deserialization.

I'm not sure I follow how from this it follows that the Controller instance itself must be stored in the HostResources. With that said, the code-base is complex and I might be missing something, so if you could elaborate?

So, if the Stack instance shall be Clone + Copy, the Host/Controller instance must be stored somewhere. As I think we've discussed before, there might be a need to have another 'Resources' thing that holds the Host/Controller separately from the other resources.

Specifically, why is it that this field has to live inside BleHost and from there - inside HostResources as well? Can't it just be a standalone thing which is referenced by whoever needs it? As in server, etc?

Yes, that would be possible, but isn't that a bit problematic, as you'd generally not want to risk users passing a different controller instance to each different method? Also it kind of convolutes the API a lot needing to pass the controller instance everywhere don't you think?

Maybe we should just require HostResources to be static for now, and find a different way to make it possible to stack-allocate it.
Regarding PacketQos, I'd like to get rid of that entirely, as it's not as useful in practice as I originally intended.

I'm not sure I follow this either. Your examples do exactly that - stack allocate both the HostRespources instance and the Controller instance. However - and because HostResources contains the Controller, their lifetimes are bound (equal, if you wish). In other words:

* if `HostResources` is stack-allocated, the controller should as well, and they _both_ have the same lifetime

* If `HostResources` has a static lifetime, so should the controller

And that's exactly the problem. We should strive to somehow make it possible for HostResources to outlive Controller I think.

Yes, I agree.

[lulf]

I think I have an approach that may work that I'm testing out that merges Stack and Builder:

HostResources<CONNS, CHANNELS, L2CAP_MTU, ADV_SETS>,
pub struct Stack<'d, C: Controller> {
    host: BleHost<'d, C>,
}

impl<'resources> Stack<'resources> {
    pub fn build<'stack>(&'stack self) -> (Peripheral<'stack, 'resources, C>, Central<'stack, 'resources, C>, Runner<'stack, 'resources, C>) {
        (Peripheral::new(self), Central::new(self), Runner::new(self))
    }
}

This way, I think it should be possible to fix this. HostResources no longer holds onto BleHost, it is stored in Stack. However, Stack is no longer Clone + Copy, I'm not really sure it needs to really.

It means an additional lifetime of the stack must be managed separately from the resources, but it should be contained to the Perpiheral, Central and Runner types.

[ivmarkov]

I think I have an approach that may work that I'm testing out that merges Stack and Builder:

HostResources<CONNS, CHANNELS, L2CAP_MTU, ADV_SETS>,
pub struct Stack<'d, C: Controller> {
    host: BleHost<'d, C>,
}

impl<'resources> Stack<'resources> {
    pub fn build<'stack>(&'stack self) -> (Peripheral<'stack, 'resources, C>, Central<'stack, 'resources, C>, Runner<'stack, 'resources, C>) {
        (Peripheral::new(self), Central::new(self), Runner::new(self))
    }
}

This way, I think it should be possible to fix this. HostResources no longer holds onto BleHost, it is stored in Stack. However, Stack is no longer Clone + Copy, I'm not really sure it needs to really.

It means an additional lifetime of the stack must be managed separately from the resources, but it should be contained to the Perpiheral, Central and Runner types.

That looks very promising!
(What I have not realized so far, is that the user might want multiple (peripheral,central,runner) triples active at a time, hence why you actually needed to fiddle with Clone (+ Copy) of the Stack.)

Is Stack (and BleHost) small enough in terms of memory footprint so that having it on the stack not to be an issue with this approach?

Specifically, why is it that this field has to live inside BleHost and from there - inside HostResources as well? Can't it just be a standalone thing which is referenced by whoever needs it? As in server, etc?

Yes, that would be possible, but isn't that a bit problematic, as you'd generally not want to risk users passing a different controller instance to each different method? Also it kind of convolutes the API a lot needing to pass the controller instance everywhere don't you think?

I never meant passing the C: Controller instance explicitly to methods of peripheral/central/runner, but rather - exactly what you just invented above. I.e. taking Controller out of HostResources and sticking it as a field into some other structure of the ones you have to deal with when using trouble.