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.

Sign up for GitHub

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

Jump to bottom

fix(embassy-init): move out peripherals required for drivers earlier #337

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix(embassy-init): move out peripherals required for drivers earlier #337

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d2cbe4e
fix(embassy-init): move out peripherals required for drivers earlier
ROMemories Jul 1, 2024
9377054
fixup! fix(embassy-init): move out peripherals required for drivers e…
ROMemories Jul 2, 2024
ecc6870
fixup! fix(embassy-init): move out peripherals required for drivers e…
ROMemories Jul 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/riot-rs-embassy/src/arch/dummy/usb.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ use embassy_usb::driver::{
EndpointIn, EndpointInfo, EndpointOut, EndpointType, Event, Unsupported,
};

use crate::arch;

pub struct UsbDriver;

impl<'a> Driver<'a> for UsbDriver {
Expand Down Expand Up @@ -34,7 +32,9 @@ impl<'a> Driver<'a> for UsbDriver {
}
}

pub fn driver(_peripherals: &mut arch::OptionalPeripherals) -> UsbDriver {
crate::define_peripherals!(Peripherals {});
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This definition is what triggers the different macro error message. As the error message is clearer this way, this doesn't seem like an issue.


pub fn driver(_peripherals: Peripherals) -> UsbDriver {
unimplemented!();
}

Expand Down
9 changes: 4 additions & 5 deletions src/riot-rs-embassy/src/arch/nrf/usb.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ use embassy_nrf::{
},
};

use crate::arch;

#[cfg(context = "nrf52")]
bind_interrupts!(struct Irqs {
USBD => usb::InterruptHandler<peripherals::USBD>;
Expand All @@ -23,7 +21,8 @@ bind_interrupts!(struct Irqs {

pub type UsbDriver = Driver<'static, peripherals::USBD, HardwareVbusDetect>;

pub fn driver(peripherals: &mut arch::OptionalPeripherals) -> UsbDriver {
let usbd = peripherals.USBD.take().unwrap();
Driver::new(usbd, Irqs, HardwareVbusDetect::new(Irqs))
crate::define_peripherals!(Peripherals { usbd: USBD });

pub fn driver(peripherals: Peripherals) -> UsbDriver {
Driver::new(peripherals.usbd, Irqs, HardwareVbusDetect::new(Irqs))
}
9 changes: 4 additions & 5 deletions src/riot-rs-embassy/src/arch/rp2040/usb.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,14 @@ use embassy_rp::{
usb::{Driver, InterruptHandler},
};

use crate::arch;

bind_interrupts!(struct Irqs {
USBCTRL_IRQ => InterruptHandler<peripherals::USB>;
});

pub type UsbDriver = Driver<'static, peripherals::USB>;

pub fn driver(peripherals: &mut arch::OptionalPeripherals) -> UsbDriver {
let usb = peripherals.USB.take().unwrap();
Driver::new(usb, Irqs)
crate::define_peripherals!(Peripherals { usb: USB });

pub fn driver(peripherals: Peripherals) -> UsbDriver {
Driver::new(peripherals.usb, Irqs)
}
21 changes: 15 additions & 6 deletions src/riot-rs-embassy/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,23 @@

#[embassy_executor::task]
async fn init_task(mut peripherals: arch::OptionalPeripherals) {
use crate::define_peripherals::TakePeripherals;

Check warning on line 146 in src/riot-rs-embassy/src/lib.rs

View workflow job for this annotation

GitHub Actions / lint

unused import: `crate::define_peripherals::TakePeripherals` 

warning: unused import: `crate::define_peripherals::TakePeripherals`
   --> src/riot-rs-embassy/src/lib.rs:146:9
    |
146 |     use crate::define_peripherals::TakePeripherals;
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(unused_imports)]` on by default
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style: move imports to top of file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather keep this import closer to its usage locations, given that its usage is non-obvious.


println!("riot-rs-embassy::init_task()");

#[cfg(feature = "hwrng")]
arch::hwrng::construct_rng(&mut peripherals);
// Clock startup and entropy collection may lend themselves to parallelization, provided that
// doesn't impact runtime RAM or flash use.

// Move out the peripherals required for drivers, so that tasks cannot mistakenly take them.
#[cfg(feature = "usb")]
let usb_peripherals: arch::usb::Peripherals = (&mut peripherals).take_peripherals();
#[cfg(feature = "wifi-cyw43")]
let wifi_cyw43_peripherals: wifi::cyw43::Peripherals = (&mut peripherals).take_peripherals();
#[cfg(feature = "wifi-esp")]
let esp_wifi_peripherals: wifi::esp_wifi::Peripherals = (&mut peripherals).take_peripherals();

#[cfg(all(context = "nrf", feature = "usb"))]
{
// nrf52840
Expand All @@ -162,6 +172,8 @@

let spawner = Spawner::for_current_executor().await;

// Tasks have to be started before driver initializations so that the tasks are able to
// configure the drivers using hooks.
for task in EMBASSY_TASKS {
task(spawner, &mut peripherals);
}
Expand All @@ -170,7 +182,7 @@
let mut usb_builder = {
let usb_config = usb::config();

let usb_driver = arch::usb::driver(&mut peripherals);
let usb_driver = arch::usb::driver(usb_peripherals);

// Create embassy-usb DeviceBuilder using the driver and config.
let builder = usb::UsbBuilder::new(
Expand Down Expand Up @@ -226,13 +238,10 @@
}

#[cfg(feature = "wifi-cyw43")]
let (device, control) = {
let (net_device, control) = wifi::cyw43::device(&mut peripherals, &spawner).await;
(net_device, control)
};
let (device, control) = wifi::cyw43::init(wifi_cyw43_peripherals, &spawner).await;

#[cfg(feature = "wifi-esp")]
let device = wifi::esp_wifi::init(&mut peripherals, spawner);
let device = wifi::esp_wifi::init(esp_wifi_peripherals, spawner);

#[cfg(feature = "net")]
{
Expand Down
28 changes: 13 additions & 15 deletions src/riot-rs-embassy/src/wifi/cyw43.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ use embassy_rp::{

use riot_rs_debug::println;

use self::rpi_pico_w::{Cyw43Periphs, CywSpi, Irqs};
use crate::{arch::OptionalPeripherals, make_static};
use self::rpi_pico_w::{CywSpi, Irqs};
use crate::make_static;

pub use rpi_pico_w::Cyw43Periphs as Peripherals;

pub type NetworkDevice = cyw43::NetDriver<'static>;

Expand All @@ -34,14 +36,10 @@ async fn wifi_cyw43_task(runner: Runner<'static, Output<'static>, CywSpi>) -> !
runner.run().await
}

pub async fn device<'a, 'b: 'a>(
mut p: &'a mut OptionalPeripherals,
pub async fn init<'a>(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I renamed this function for consistency with other driver modules; this is not a breaking change as this module is private.

periperals: Peripherals,
spawner: &crate::Spawner,
) -> (embassy_net_driver_channel::Device<'b, 1514>, Control<'b>) {
use crate::define_peripherals::TakePeripherals;

let pins: Cyw43Periphs = p.take_peripherals();

) -> (embassy_net_driver_channel::Device<'a, 1514>, Control<'a>) {
let fw = include_bytes!("cyw43/firmware/43439A0.bin");
let clm = include_bytes!("cyw43/firmware/43439A0_clm.bin");

Expand All @@ -52,17 +50,17 @@ pub async fn device<'a, 'b: 'a>(
//let fw = unsafe { core::slice::from_raw_parts(0x10100000 as *const u8, 230321) };
//let clm = unsafe { core::slice::from_raw_parts(0x10140000 as *const u8, 4752) };

let pwr = Output::new(pins.pwr, Level::Low);
let cs = Output::new(pins.cs, Level::High);
let mut pio = Pio::new(pins.pio, Irqs);
let pwr = Output::new(periperals.pwr, Level::Low);
let cs = Output::new(periperals.cs, Level::High);
let mut pio = Pio::new(periperals.pio, Irqs);
let spi = CywSpi::new(
&mut pio.common,
pio.sm0,
pio.irq0,
cs,
pins.dio,
pins.clk,
pins.dma,
periperals.dio,
periperals.clk,
periperals.dma,
);

let state = make_static!(cyw43::State::new());
Expand Down
16 changes: 10 additions & 6 deletions src/riot-rs-embassy/src/wifi/esp_wifi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use esp_wifi::{wifi::WifiStaDevice, EspWifiInitialization};
use esp_hal::peripherals;
use esp_wifi::{
wifi::{WifiController, WifiDevice, WifiStaDevice},
EspWifiInitialization,
};
use once_cell::sync::OnceCell;

use crate::{arch::OptionalPeripherals, Spawner};

use esp_wifi::wifi::{WifiController, WifiDevice};
use crate::Spawner;

pub type NetworkDevice = WifiDevice<'static, WifiStaDevice>;

Expand All @@ -14,8 +16,10 @@ pub type NetworkDevice = WifiDevice<'static, WifiStaDevice>;
// sure.
pub static WIFI_INIT: OnceCell<EspWifiInitialization> = OnceCell::new();

pub fn init(peripherals: &mut OptionalPeripherals, spawner: Spawner) -> NetworkDevice {
let wifi = peripherals.WIFI.take().unwrap();
crate::define_peripherals!(Peripherals { wifi: WIFI });

pub fn init(peripherals: Peripherals, spawner: Spawner) -> NetworkDevice {
let wifi = peripherals.wifi;
let init = WIFI_INIT.get().unwrap();
let (device, controller) = esp_wifi::wifi::new_with_mode(init, wifi, WifiStaDevice).unwrap();

Expand Down
11 changes: 9 additions & 2 deletions src/riot-rs-macros/tests/ui/task/incorrect_fn_peripheral_param_type.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
error[E0599]: no method named `take_peripherals` found for mutable reference `&mut OptionalPeripherals` in the current scope
error[E0308]: mismatched types
--> tests/ui/task/incorrect_fn_peripheral_param_type.rs:7:1
|
7 | #[riot_rs::task(autostart, peripherals)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ method not found in `&mut OptionalPeripherals`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `Bar`, found `Peripherals`
8 | async fn main(_foo: Bar) {}
| ---- arguments to this function are incorrect
|
note: function defined here
--> tests/ui/task/incorrect_fn_peripheral_param_type.rs:8:10
|
8 | async fn main(_foo: Bar) {}
| ^^^^ ---------
= note: this error originates in the attribute macro `riot_rs::task` (in Nightly builds, run with -Z macro-backtrace for more info)
Loading