Cleanup code

controller/src/logic.rs

@@ -6,7 +6,7 @@ use heapless::Deque;
 use i2c2_target::{I2C2, MESSAGE_SIZE};
 
 struct State {
-    _i2c: I2C2,
+    i2c: I2C2,
     motors: Tb6612fng<'static>,
     encoders: Encoders<'static>,
     max_motor_percentage: u8,
@@ -17,7 +17,7 @@ struct State {
 impl State {
     fn new(i2c2: I2C2, motors: Tb6612fng<'static>, encoders: Encoders<'static>) -> Self {
         Self {
-            _i2c: i2c2,
+            i2c: i2c2,
             motors,
             encoders,
             max_motor_percentage: 100,
@@ -38,10 +38,12 @@ pub async fn start_i2c_target(
     motors.standby_enter();
     unsafe {
         STATE = Some(State::new(i2c2, motors, encoders));
+        STATE.as_mut().unwrap().i2c.start(CALLBACK);
     }
-    I2C2::set_callback(handle_command);
 }
 
+const CALLBACK: i2c2_target::Callback = &handle_command;
+
 const WHO_AM_I: u8 = 0x0f;
 const MAX_MOTOR_PERCENTAGE: u8 = 0x11;
 const MOTOR_SPEED: u8 = 0x30;
@@ -49,10 +51,7 @@ const ENCODER_TICKS: u8 = 0x32;
 
 fn handle_command(command: &[u8], response: &mut Deque<u8, MESSAGE_SIZE>) {
     #[allow(static_mut_refs)]
-    let Some(state) = (unsafe { &mut STATE }) else {
-        defmt::trace!("state not installed yet");
-        return;
-    };
+    let state = unsafe { STATE.as_mut().unwrap() };
     defmt::trace!("Processing command {:?}", command);
     match command {
         [WHO_AM_I, ..] => {

i2c2-target/src/lib.rs

@@ -9,7 +9,7 @@ use embassy_stm32::{
 };
 use heapless::{Deque, Vec};
 
-static mut STATE: State = State::default();
+static mut STATE: Option<State> = None;
 
 pub const MESSAGE_SIZE: usize = 8;
 
@@ -22,7 +22,7 @@ enum Operation {
     WriteInProgress,
 }
 
-type Callback = fn(&[u8], &mut Deque<u8, MESSAGE_SIZE>);
+pub type Callback = &'static dyn Fn(&[u8], &mut Deque<u8, MESSAGE_SIZE>);
 
 struct State {
     // The operation in progress
@@ -32,16 +32,16 @@ struct State {
     // Data from the target to the controller
     outgoing: Deque<u8, MESSAGE_SIZE>,
     // Callback for exchanging data
-    callback: Option<Callback>,
+    callback: Callback,
 }
 
 impl State {
-    pub const fn default() -> Self {
+    pub fn new(callback: Callback) -> Self {
         Self {
             operation: Operation::Idle,
             incoming: Vec::new(),
             outgoing: Deque::new(),
-            callback: None,
+            callback,
         }
     }
 
@@ -87,16 +87,12 @@ impl State {
 
     pub fn callback(&mut self) {
         self.outgoing.clear();
-        if let Some(callback) = &mut self.callback {
-            defmt::trace!("calling callback with {:?}", &self.incoming[..]);
-            callback(&self.incoming, &mut self.outgoing);
-            defmt::trace!(
-                "after callback, ready to send {} bytes",
-                self.outgoing.len()
-            );
-        } else {
-            defmt::error!("no callback installed");
-        }
+        defmt::trace!("calling callback with {:?}", &self.incoming[..]);
+        (self.callback)(&self.incoming, &mut self.outgoing);
+        defmt::trace!(
+            "after callback, ready to send {} bytes",
+            self.outgoing.len()
+        );
         self.incoming.clear();
     }
 }
@@ -108,6 +104,7 @@ pub struct I2C2 {
 }
 
 impl I2C2 {
+    /// Create the device without enabling the peripheral yet.
     pub fn new(i2c2: peripherals::I2C2, pb10: PB10, pb11: PB11, addr: u16, prio: Priority) -> Self {
         // Set pins in AF mode
         let af_num = SclPin::af_num(&pb10);
@@ -140,9 +137,6 @@ impl I2C2 {
             w.set_ack(true);
         });
         interrupt::I2C2_EV.set_priority(prio);
-        unsafe {
-            interrupt::I2C2_EV.enable();
-        }
         Self {
             _pb10: pb10,
             _pb11: pb11,
@@ -150,12 +144,12 @@ impl I2C2 {
         }
     }
 
-    /// Install a callback which will be called whenever an I²C
-    /// request arrives. This callback will decide which bytes are
-    /// sent back if a read request follows. The read operation will
-    /// be completed by zeroes as needed.
-    pub fn set_callback(callback: Callback) {
-        unsafe { STATE.callback = Some(callback) };
+    /// Start answering I²C requests using the provided callback.
+    pub fn start(&mut self, callback: Callback) {
+        unsafe {
+            STATE = Some(State::new(callback));
+            interrupt::I2C2_EV.enable();
+        }
     }
 }
 
@@ -168,17 +162,18 @@ unsafe fn I2C2_EV() {
         pac::I2C2.cr1().read().0,
         pac::I2C2.cr2().read().0
     );
+    let state = unsafe { STATE.as_mut().unwrap() };
     if sr1.rxne() {
         defmt::trace!("I2C2_EV RXNE");
         let b = pac::I2C2.dr().read().0 as u8;
         defmt::trace!("received byte {:#04x}", &b);
-        if unsafe { STATE.incoming.push(b) }.is_err() {
+        if state.incoming.push(b).is_err() {
             defmt::error!("I2C byte has overfilled the buffer");
         }
     }
     if sr1.txe() {
         defmt::trace!("I2C2_EV TXE");
-        let b = match unsafe { STATE.outgoing.pop_front() } {
+        let b = match state.outgoing.pop_front() {
             Some(b) => Some(b),
             None => {
                 if sr1.af() {
@@ -198,7 +193,7 @@ unsafe fn I2C2_EV() {
     if sr1.stopf() {
         defmt::trace!(
             "I2C2_EV STOPF while state is {:?}",
-            defmt::Debug2Format(unsafe { &STATE.operation })
+            defmt::Debug2Format(&state.operation)
         );
         // Clear stopf by writing to cr1
         pac::I2C2.cr1().modify(|_w| ());
@@ -206,21 +201,19 @@ unsafe fn I2C2_EV() {
         // by reading and writing dr
         pac::I2C2.dr().modify(|_w| ());
         // If receiving, indicate the end of data
-        unsafe {
-            STATE.set_idle();
-        }
+        state.set_idle();
     }
     if sr1.addr() {
         defmt::trace!(
             "I2C2_EV addr while state is {:?}",
-            defmt::Debug2Format(unsafe { &STATE.operation })
+            defmt::Debug2Format(&state.operation)
         );
         // Addr event is cleared by reading sr2
         let sr2 = pac::I2C2.sr2().read();
         if sr2.tra() {
-            unsafe { STATE.set_read_in_progress() };
+            state.set_read_in_progress();
         } else {
-            unsafe { STATE.set_write_in_progress() };
+            state.set_write_in_progress();
         }
     }
 }