introduce OptionalPeripherals

[kaspar030]

I needed a way to spread peripheral initialization over multiple init() functions.
Passing &mut Peripherals around won't work as it's field cannot be moved out.
But steal()ing Peripherals in multiple places doesn't seem right.

So this PR augments peripherals_struct!() to also create a struct OptionalPeripherals {} that contains all fields of Peripherals, but wrapped in Option.

This is quite wasteful (1b per Peripheral, e.g., ~119b on nrf52840). I hope that this will be on stack only, or edit we'll optimize this into some kind of bit-field but with a similar interface, or even just compile it out.

edit this only exposes OptionalPeripherals for nrf52840 now. If I get positive feedback on the idea, I'll add re-exporting for the other crates. Marking as draft for now. now supports nrf, stm32 and rp. We have a similar PR to esp-rs.

[AdinAck]

Could you show an example of why this is useful?

[kaspar030]

Could you show an example of why this is useful?

Sure!

Basically I'm trying to provide high-level features ("net", "usb", "usb_ethernet", ...) that an application can select, together with target board features (e.g., "nrf52840dk", "rpi-pico"), and then minimize code duplication, having shared initialization functions.

  #[cfg(feature = "nrf52840dk")]                                                                                                                               
  mod nrf52840dk {                                                                                                                                             
      pub type UsbDriver = Driver<'static, peripherals::USBD, HardwareVbusDetect>;                                                                             
      pub fn init_usb(p: Peripherals) -> UsbDriver {                                                                                                           
          use embassy_nrf::peripherals;                                                                                                                        
          use embassy_nrf::usb::{vbus_detect::HardwareVbusDetect, Driver};                                                                                     
          pub fn usb_driver(p: &mut Peripherals) -> UsbDriver {                                                                                                
              let usbd = p.USBD; // cannot move out                                                                                                            
              use super::Irqs;                                                                                                                                 
              Driver::new(usbd, Irqs, HardwareVbusDetect::new(Irqs))                                                                                           
          }                                                                                                                                                    
      }                                                                                                                                                        
  }                                                                                                                                                            
                                                                                                                                                               
  #[cfg(feature = "rpi-pico")]                                                                                                                                 
  pub mod rpi_pico {                                                                                                                                           
      use embassy_rp::peripherals;                                                                                                                             
      use embassy_rp::usb::Driver;                                                                                                                             
      pub type UsbDriver = Driver<'static, peripherals::USB>;                                                                                                  
      pub fn usb_driver(p: &mut Peripherals) -> UsbDriver {                                                                                                    
          let usb = p.USB; // cannot move out                                                                                                                  
          Driver::new(usb, super::Irqs)                                                                                                                        
      }                                                                                                                                                        
  }                                                                                                                                                            
                                                                                                                                                               
  #[cfg(feature = "nrf52840dk")]                                                                                                                               
  use nrf52840dk as board;                                                                                                                                     
                                                                                                                                                               
  #[cfg(feature = "rpi-pico")]                                                                                                                                 
  use rpi_pico as board;                                                                                                                                       
                                                                                                                                                               
  fn init(p: Peripherals) {                                                                                                                                    
      let usb_driver = board::usb_driver(&mut p);                                                                                                              
  }

(pseudo code and mostly conceived example)

In this example, I'd like to move the initialization of usb_driver out of init() into functions (of modules, or elsewhere). With USBD (or USB) being fields of Peripheral, there's no way. If usb_driver() takes &mut Peripheral, the field cannot move out. If it consumes the peripheral, that cannot be used for any other initialization.

For this example, there's ways to work around this (e.g., by passing p.USBD to board::usb_driver()), but already there'd be two versions because of different naming of the USB peripheral field.

Down the line there'll be more variants, more peripheral types, more places to pull init_foo(p: Peripherals) from, ...

[adamgreig]

I recently published assign-resources which is another way to solve this problem: you can split Peripherals into multiple structs, e.g. you'd have one for fn usb_driver(), and you only write the name of the type in one place, so you can very easily #[cfg] multiple versions for different boards. In main(), you split up peripherals into the smaller structs, and pass each one to the relevant function, which then gets to own its peripherals.

For example:

#[cfg(feature = "nrf52840dk")]                                                                                                                               
assign_resources! {
    usb_driver: UsbResources {
        usb: USBD,
    }
}

#[cfg(feature = "rpi-pico")]                                                                                                                                 
assign_resources! {
    usb_driver: UsbResources {
        usb: USB,
    }
}

pub fn make_usb_driver(r: UsbResources) -> UsbDriver {
    // it's always called `r.usb` in `UsbResources` and is an owned peripheral, not a reference
    Driver::new(r.usb, Irqs, #[cfg(feature = "nrf52840dk")] HardwareVbusDetect::new(Irqs))
}

fn init(p: Peripherals) {
    let r = split_resources!(p);
    // pass `make_usb_driver()` all the USB related resources (pins, peripherals, DMA channels, etc)
    let usb_driver = make_usb_driver(r.usb_driver);
}

[kaspar030]

I recently published assign-resources which is another way to solve this problem: you can split Peripherals into multiple structs

This is very useful, and it solves the example I gave pretty elegantly. But I think I aimed too low. :)

Even with assign-resources, there's a single place in the sources that owns Peripherals and needs to be able to do split_resources() on it. I think that works well for applications that want/need to take this kind of control.
I'm trying to get to the point where the application can be much simpler, and sth under the hood takes care of Peripherals, supporting multiple boards. The application might bring their own board(s) but not want to deal with "default" initializations (like, usb_networking + stack). Maybe there's build system level configuration on what sensors are connected to which pins.

IMO using the type system & borrow checker to provide compile time "allocation" for the resources doesn't scale.

I wish I could create this OptionalPeripherals outside of embassy (in our code), but I'm not sure Rust's macros have enough information. And, doing it in peripherals_struct!() is really easy.

[kaspar030]

Just so this doesn't get cleaned: we might still want to use this, and are still searching for alternatives or better justification. :)

[kaspar030]

Some update:

We've been using OptionalPeripherals in our fork of embassy for a while now. We're pairing it with a variant of assign_resources!() (we call it riot_rs::define_peripherals!()).

In combination, it allows writing applications that don't fully control Peripherals, which is kinda the thing that Ariel OS (turning into an opinionated distribution of embassy) adds on top of embassy.

I'm realizing that we don't really have a succinct example showcasing this. Probably our hid test application comes closest.

The general idea is that an (application or library) crate can do sth like this:

use riot_rs::...;

riot_rs::define_peripherals!(Buttons {
    btn1: P0_11,
    btn2: P0_12,
    btn3: P0_24,
    btn4: P0_25,
});

#[riot_rs::task(autostart, peripherals)]
async fn button_user(buttons: Buttons) {
  // this task now owns the peripherals in `Buttons`
...
}

This should look quite familiar to a "regular" embassy application. Here, the ariel-os crates drive MCU setup & startup (controlling main() and Peripherals), take care (using OptionalPeripherals) of extracting Buttons in a safe (works or panics) way, and of spawning the task.
Potentially there are other tasks which use other Peripherals, started elsewhere in the dependency tree, with ownership tracked at runtime using OptionalPeripherals. Like, enabling a feature on the riot-rs crate also enables the network stack.

We'd love to upstream this so we might eventually not use a fork of embassy. And we don't see a way to implement this in our code, the peripherals_struct!() is just the right place. 😄

Personally I think the name needs bike shedding. Apart from that, I hope embassy finds this useful enough to merge, or unintrusive enough to merge because we want to use it.

edit updated links to the new project name and location (Ariel OS).

[Dirbaio]

I'm sorry, but I don't think this is something we should merge.

• Solutions like assign_resources! already offer ways of splitting initialization between multiple methods, without potential "unwrap of None" panics.
• I'm not a fan of having two ways of doing the same thing, with subtle tradeoffs between them.

so I'm going to close this. Thanks anyway for the PR!

[kaspar030]

Hm, this is very unfortunate, we depend on this quit a bit in Ariel OS, and we'd love to not have to use a fork at some point.

What the runtime tracking allows us to do, and what no other solution that we know of does, is that it solves what we've dubbed the "partial move problem" in code that is independent at the type level.

assign_resources!() allows splitting initialization between multiple methods from one method that knows all the types, as the compiler can reason about the partial moves. Only that one method can move fields out of Peripherals.

OptionalPeripherals allows runtime tracking of the actually moved out peripherals, which allows us to pass such a struct to arbitrary methods without knowing the exact peripherals those methods would take out.

I think assign_resources!() actually solves an orthogonal problem (bundling some Peripherals fields and partially moving them out of Peripherals, whereas we need to fully move/take them out, elsewhere).

(In Ariel, we're trying to mitigate the "unwrap of None" panics by having some defined "initialization phase", but we're not too happy about that either. )

We'd love to handle this in our code base, but don't see how. Suggestions welcome. :)

@Dirbaio, is this sufficiently different to make you reconsider?

Or, can we maybe ease taking this change in somehow?

Some options I see:

0. rename the struct (we're not happy with it, we're converging to TrackedPeripherals)

1. use some macro to create the wrapped struct (I found https://github.com/roignpar/optfield). hide its use (and the necessary reexports) behind a feature.
   This would add the optional dependency, but reduce code impact to a handful of feature-gated lines

2. Find (must exist, else build) a macro that exports the structure of Peripherals (create a macro that creates a macro that creates Peripherals which embassy exports, which we can use in our code to wrap it in optfield)