Merge #414

414: Implement the driver registration API in `doc/UnitTestOwnership.md`. r=jrvanwhy a=jrvanwhy As described in #355, I intend to remove the use of thread-local storage in `libtock_unittest`. That requires giving `fake::SyscallDriver` implementations a reference that allows them to schedule upcalls. This PR adds that reference, and updates the `fake::SyscallDrivers` to use it. Co-authored-by: Johnathan Van Why <[email protected]>
tock · Jun 27, 2022 · 632c2b2 · 632c2b2
2 parents 42b96d5 + 0465366
commit 632c2b2
Show file tree

Hide file tree

Showing 18 changed files with 405 additions and 317 deletions.
diff --git a/apis/buttons/src/tests.rs b/apis/buttons/src/tests.rs
@@ -1,11 +1,9 @@
 use core::cell::Cell;
 
 use libtock_platform::{share, ErrorCode, Syscalls, YieldNoWaitReturn};
-use libtock_unittest::{fake, upcall};
+use libtock_unittest::fake;
 
-use crate::{ButtonListener, DRIVER_NUM};
-
-use super::ButtonState;
+use crate::{ButtonListener, ButtonState};
 
 type Buttons = super::Buttons<fake::Syscalls>;
 
@@ -66,38 +64,39 @@ fn subscribe() {
         assert_eq!(state, ButtonState::Pressed);
         pressed_interrupt_count.set(true);
     });
+    assert_eq!(Buttons::enable_interrupts(0), Ok(()));
     share::scope(|subscribe| {
         assert_eq!(Buttons::register_listener(&listener, subscribe), Ok(()));
-        upcall::schedule(DRIVER_NUM, 0, (0, 1, 0)).expect("Unable to schedule upcall");
+        assert_eq!(driver.set_pressed(0, true), Ok(()));
         assert_eq!(fake::Syscalls::yield_no_wait(), YieldNoWaitReturn::Upcall);
     });
     assert!(pressed_interrupt_count.get());
 
     let pressed_interrupt_count: Cell<u32> = Cell::new(0);
     let expected_button_state: Cell<ButtonState> = Cell::new(ButtonState::Released);
     let listener = ButtonListener(|button, state| {
-        assert_eq!(button, 0);
+        assert_eq!(button, 1);
         assert_eq!(state, expected_button_state.get());
         pressed_interrupt_count.set(pressed_interrupt_count.get() + 1);
     });
     share::scope(|subscribe| {
-        assert_eq!(Buttons::enable_interrupts(0), Ok(()));
+        assert_eq!(Buttons::enable_interrupts(1), Ok(()));
         assert_eq!(Buttons::register_listener(&listener, subscribe), Ok(()));
         expected_button_state.set(ButtonState::Pressed);
-        assert_eq!(driver.set_pressed(0, true), Ok(()));
+        assert_eq!(driver.set_pressed(1, true), Ok(()));
         assert_eq!(fake::Syscalls::yield_no_wait(), YieldNoWaitReturn::Upcall);
-        assert_eq!(driver.set_pressed(0, true), Ok(()));
+        assert_eq!(driver.set_pressed(1, true), Ok(()));
         assert_eq!(fake::Syscalls::yield_no_wait(), YieldNoWaitReturn::NoUpcall);
         expected_button_state.set(ButtonState::Released);
-        assert_eq!(driver.set_pressed(0, false), Ok(()));
+        assert_eq!(driver.set_pressed(1, false), Ok(()));
         assert_eq!(fake::Syscalls::yield_no_wait(), YieldNoWaitReturn::Upcall);
-        assert_eq!(driver.set_pressed(0, false), Ok(()));
+        assert_eq!(driver.set_pressed(1, false), Ok(()));
         assert_eq!(fake::Syscalls::yield_no_wait(), YieldNoWaitReturn::NoUpcall);
 
-        assert_eq!(Buttons::disable_interrupts(0), Ok(()));
-        assert_eq!(driver.set_pressed(0, true), Ok(()));
+        assert_eq!(Buttons::disable_interrupts(1), Ok(()));
+        assert_eq!(driver.set_pressed(1, true), Ok(()));
         assert_eq!(fake::Syscalls::yield_no_wait(), YieldNoWaitReturn::NoUpcall);
-        assert_eq!(driver.set_pressed(0, false), Ok(()));
+        assert_eq!(driver.set_pressed(1, false), Ok(()));
         assert_eq!(fake::Syscalls::yield_no_wait(), YieldNoWaitReturn::NoUpcall);
     });
     assert_eq!(pressed_interrupt_count.get(), 2);

diff --git a/apis/gpio/src/tests.rs b/apis/gpio/src/tests.rs
@@ -1,12 +1,9 @@
 use core::cell::Cell;
 
 use libtock_platform::{share, ErrorCode, Syscalls, YieldNoWaitReturn};
-use libtock_unittest::{
-    fake::{self, GpioMode, InterruptEdge, PullMode},
-    upcall,
-};
+use libtock_unittest::fake::{self, GpioMode, InterruptEdge, PullMode};
 
-use crate::{GpioInterruptListener, GpioState, PullDown, PullNone, PullUp, DRIVER_NUM};
+use crate::{GpioInterruptListener, GpioState, PinInterruptEdge, PullDown, PullNone, PullUp};
 
 type Gpio = super::Gpio<fake::Syscalls>;
 
@@ -122,14 +119,15 @@ fn interrupts() {
         gpio_state.set(Some(state));
     });
 
+    assert_eq!(Gpio::enable_interrupts(0, PinInterruptEdge::Either), Ok(()));
     share::scope(|subscribe| {
         assert_eq!(Gpio::register_listener(&listener, subscribe), Ok(()));
-        assert_eq!(upcall::schedule(DRIVER_NUM, 0, (0, 0, 0)), Ok(()));
+        assert_eq!(driver.set_value(0, true), Ok(()));
         assert_eq!(fake::Syscalls::yield_no_wait(), YieldNoWaitReturn::Upcall);
-        assert_eq!(gpio_state.get(), Some(GpioState::Low));
+        assert_eq!(gpio_state.get(), Some(GpioState::High));
     });
 
-    assert_eq!(upcall::schedule(DRIVER_NUM, 0, (0, 0, 0)), Ok(()));
+    assert_eq!(driver.set_value(0, false), Ok(()));
     assert_eq!(fake::Syscalls::yield_no_wait(), YieldNoWaitReturn::NoUpcall);
 
     assert!(core::matches!(Gpio::get_pin(11), Err(ErrorCode::Invalid)));
@@ -285,13 +283,14 @@ fn subscribe() {
         gpio_state.set(Some(state));
     });
 
+    assert_eq!(Gpio::enable_interrupts(0, PinInterruptEdge::Either), Ok(()));
     share::scope(|subscribe| {
         assert_eq!(Gpio::register_listener(&listener, subscribe), Ok(()));
-        assert_eq!(upcall::schedule(DRIVER_NUM, 0, (0, 0, 0)), Ok(()));
+        assert_eq!(driver.set_value(0, true), Ok(()));
         assert_eq!(fake::Syscalls::yield_no_wait(), YieldNoWaitReturn::Upcall);
-        assert_eq!(gpio_state.get(), Some(GpioState::Low));
+        assert_eq!(gpio_state.get(), Some(GpioState::High));
     });
 
-    assert_eq!(upcall::schedule(DRIVER_NUM, 0, (0, 0, 0)), Ok(()));
+    assert_eq!(driver.set_value(0, false), Ok(()));
     assert_eq!(fake::Syscalls::yield_no_wait(), YieldNoWaitReturn::NoUpcall);
 }
diff --git a/syscalls_tests/src/allow_ro.rs b/syscalls_tests/src/allow_ro.rs
@@ -1,5 +1,5 @@
 use libtock_platform::{allow_ro, share, CommandReturn, ErrorCode, Syscalls};
-use libtock_unittest::{command_return, fake, RoAllowBuffer, SyscallLogEntry};
+use libtock_unittest::{command_return, fake, DriverInfo, RoAllowBuffer, SyscallLogEntry};
 use std::cell::Cell;
 use std::rc::Rc;
 use std::thread_local;
@@ -10,12 +10,8 @@ struct TestDriver {
 }
 
 impl fake::SyscallDriver for TestDriver {
-    fn id(&self) -> u32 {
-        42
-    }
-
-    fn num_upcalls(&self) -> u32 {
-        0
+    fn info(&self) -> DriverInfo {
+        DriverInfo::new(42)
     }
 
     fn command(&self, _command_num: u32, _argument0: u32, _argument1: u32) -> CommandReturn {

diff --git a/syscalls_tests/src/allow_rw.rs b/syscalls_tests/src/allow_rw.rs
@@ -1,5 +1,5 @@
 use libtock_platform::{allow_rw, share, CommandReturn, ErrorCode, Syscalls};
-use libtock_unittest::{command_return, fake, RwAllowBuffer, SyscallLogEntry};
+use libtock_unittest::{command_return, fake, DriverInfo, RwAllowBuffer, SyscallLogEntry};
 use std::cell::Cell;
 use std::rc::Rc;
 use std::thread_local;
@@ -10,12 +10,8 @@ struct TestDriver {
 }
 
 impl fake::SyscallDriver for TestDriver {
-    fn id(&self) -> u32 {
-        42
-    }
-
-    fn num_upcalls(&self) -> u32 {
-        0
+    fn info(&self) -> DriverInfo {
+        DriverInfo::new(42)
     }
 
     fn command(&self, _command_num: u32, _argument0: u32, _argument1: u32) -> CommandReturn {

diff --git a/syscalls_tests/src/subscribe_tests.rs b/syscalls_tests/src/subscribe_tests.rs
@@ -1,22 +1,22 @@
 use libtock_platform::{
     share, subscribe, CommandReturn, DefaultConfig, ErrorCode, Syscalls, YieldNoWaitReturn,
 };
-use libtock_unittest::{command_return, fake, upcall, SyscallLogEntry};
+use libtock_unittest::{command_return, fake, DriverInfo, DriverShareRef, SyscallLogEntry};
+use std::rc::Rc;
 
-// Fake driver that accepts an upcall. The unit test cases use this to make the
-// kernel willing to accept an upcall. The test cases invoke the upcall
-// themselves.
-//
-// TODO: Replace with a real driver once a driver that accepts an upcall exists.
-struct MockDriver;
+// Fake driver that accepts an upcall.
+#[derive(Default)]
+struct MockDriver {
+    share_ref: DriverShareRef,
+}
 
 impl fake::SyscallDriver for MockDriver {
-    fn id(&self) -> u32 {
-        1
+    fn info(&self) -> DriverInfo {
+        DriverInfo::new(1).upcall_count(1)
     }
 
-    fn num_upcalls(&self) -> u32 {
-        1
+    fn register(&self, share_ref: DriverShareRef) {
+        self.share_ref.replace(share_ref);
     }
 
     fn command(&self, _: u32, _: u32, _: u32) -> CommandReturn {
@@ -37,7 +37,7 @@ fn config() {
     }
 
     let kernel = fake::Kernel::new();
-    kernel.add_driver(&std::rc::Rc::new(MockDriver));
+    kernel.add_driver(&std::rc::Rc::new(MockDriver::default()));
     let called = core::cell::Cell::new(false);
     share::scope(|subscribe| {
         assert_eq!(
@@ -70,8 +70,9 @@ fn failed() {
 
 #[test]
 fn success() {
+    let driver = Rc::new(MockDriver::default());
     let kernel = fake::Kernel::new();
-    kernel.add_driver(&std::rc::Rc::new(MockDriver));
+    kernel.add_driver(&driver);
     let called = core::cell::Cell::new(None);
     share::scope(|subscribe| {
         assert_eq!(
@@ -85,7 +86,7 @@ fn success() {
                 subscribe_num: 0
             }]
         );
-        upcall::schedule(1, 0, (2, 3, 4)).unwrap();
+        driver.share_ref.schedule_upcall(0, (2, 3, 4)).unwrap();
         assert_eq!(fake::Syscalls::yield_no_wait(), YieldNoWaitReturn::Upcall);
         assert_eq!(called.get(), Some((2, 3, 4)));
         // Clear the syscall log.
@@ -99,7 +100,7 @@ fn success() {
             subscribe_num: 0
         }]
     );
-    upcall::schedule(1, 0, (2, 3, 4)).unwrap();
+    driver.share_ref.schedule_upcall(0, (2, 3, 4)).unwrap();
     assert_eq!(fake::Syscalls::yield_no_wait(), YieldNoWaitReturn::NoUpcall);
 }
 
@@ -115,15 +116,16 @@ fn unwinding_upcall() {
     }
 
     let exit = libtock_unittest::exit_test("subscribe_tests::unwinding_upcall", || {
+        let driver = Rc::new(MockDriver::default());
         let kernel = fake::Kernel::new();
-        kernel.add_driver(&std::rc::Rc::new(MockDriver));
+        kernel.add_driver(&driver);
         let upcall = BadUpcall;
         share::scope(|subscribe| {
             assert_eq!(
                 fake::Syscalls::subscribe::<_, _, DefaultConfig, 1, 0>(subscribe, &upcall),
                 Ok(())
             );
-            upcall::schedule(1, 0, (2, 3, 4)).unwrap();
+            driver.share_ref.schedule_upcall(0, (2, 3, 4)).unwrap();
             assert_eq!(fake::Syscalls::yield_no_wait(), YieldNoWaitReturn::Upcall);
         });
     });

diff --git a/unittest/src/driver_info.rs b/unittest/src/driver_info.rs
@@ -0,0 +1,45 @@
+/// Information that a `fake::SyscallDriver` provides to the `fake::Kernel`
+/// during registration. This may be expanded over time as new features are
+/// added to Tock.
+#[non_exhaustive]
+pub struct DriverInfo {
+    // All constructors of DriverInfo require the driver to specify
+    // `driver_num`.
+    pub(crate) driver_num: u32,
+
+    /// The maximum number of subscriptions to support. The maximum subscribe
+    /// number supported will be one less than `upcall_count`.
+    pub upcall_count: u32,
+}
+
+impl DriverInfo {
+    /// Creates a new `DriverInfo` with the given driver number. `upcall_count`
+    /// will be initialized to zero.
+    pub fn new(driver_num: u32) -> Self {
+        Self {
+            driver_num,
+            upcall_count: 0,
+        }
+    }
+
+    /// Sets `upcall_count` and returns `self`. Used similar to a builder.
+    ///
+    /// # Example
+    /// ```
+    /// use libtock_platform::CommandReturn;
+    /// use libtock_unittest::{DriverInfo, fake};
+    /// struct FooDriver;
+    /// impl fake::SyscallDriver for FooDriver {
+    ///     fn info(&self) -> DriverInfo {
+    ///         DriverInfo::new(3).upcall_count(2)
+    ///     }
+    ///     fn command(&self, _: u32, _: u32, _: u32) -> CommandReturn {
+    ///         unimplemented!("Example code");
+    ///     }
+    /// }
+    /// ```
+    pub fn upcall_count(mut self, upcall_count: u32) -> Self {
+        self.upcall_count = upcall_count;
+        self
+    }
+}
diff --git a/unittest/src/fake/alarm/mod.rs b/unittest/src/fake/alarm/mod.rs
@@ -7,28 +7,31 @@ use core::cell::Cell;
 use core::num::Wrapping;
 use libtock_platform::{CommandReturn, ErrorCode};
 
-use crate::upcall;
+use crate::{DriverInfo, DriverShareRef};
 
 pub struct Alarm {
     frequency_hz: u32,
     now: Cell<Wrapping<u32>>,
+    share_ref: DriverShareRef,
 }
 
 impl Alarm {
     pub fn new(frequency_hz: u32) -> std::rc::Rc<Alarm> {
         std::rc::Rc::new(Alarm {
             frequency_hz,
             now: Cell::new(Wrapping(0)),
+            share_ref: Default::default(),
         })
     }
 }
 
 impl crate::fake::SyscallDriver for Alarm {
-    fn id(&self) -> u32 {
-        DRIVER_NUM
+    fn info(&self) -> DriverInfo {
+        DriverInfo::new(DRIVER_NUM).upcall_count(1)
     }
-    fn num_upcalls(&self) -> u32 {
-        1
+
+    fn register(&self, share_ref: DriverShareRef) {
+        self.share_ref.replace(share_ref);
     }
 
     fn command(&self, command_number: u32, argument0: u32, _argument1: u32) -> CommandReturn {
@@ -41,8 +44,9 @@ impl crate::fake::SyscallDriver for Alarm {
                 // and waking immediately.
                 let relative = argument0;
                 let wake = self.now.get() + Wrapping(relative);
-                upcall::schedule(DRIVER_NUM, subscribe::CALLBACK, (wake.0, 0, 0))
-                    .expect("Unable to schedule upcall {}");
+                self.share_ref
+                    .schedule_upcall(subscribe::CALLBACK, (wake.0, 0, 0))
+                    .expect("schedule_upcall failed");
                 self.now.set(wake);
                 crate::command_return::success_u32(wake.0)
             }

diff --git a/unittest/src/fake/buttons/mod.rs b/unittest/src/fake/buttons/mod.rs
@@ -10,7 +10,7 @@
 use core::cell::Cell;
 use libtock_platform::{CommandReturn, ErrorCode};
 
-use crate::upcall;
+use crate::{DriverInfo, DriverShareRef};
 
 #[derive(Copy, Clone, Eq, PartialEq, Debug)]
 pub struct ButtonState {
@@ -20,6 +20,7 @@ pub struct ButtonState {
 
 pub struct Buttons<const NUM_BUTTONS: usize> {
     buttons: [Cell<ButtonState>; NUM_BUTTONS],
+    share_ref: DriverShareRef,
 }
 
 impl<const NUM_BUTTONS: usize> Buttons<NUM_BUTTONS> {
@@ -31,6 +32,7 @@ impl<const NUM_BUTTONS: usize> Buttons<NUM_BUTTONS> {
         });
         std::rc::Rc::new(Buttons {
             buttons: [OFF; NUM_BUTTONS],
+            share_ref: Default::default(),
         })
     }
 
@@ -46,7 +48,8 @@ impl<const NUM_BUTTONS: usize> Buttons<NUM_BUTTONS> {
                 if original_button_state.interrupt_enabled
                     && original_button_state.pressed != pressed
                 {
-                    upcall::schedule(DRIVER_NUM, 0, (button, pressed as u32, 0))
+                    self.share_ref
+                        .schedule_upcall(0, (button, pressed as u32, 0))
                         .expect("Unable to schedule upcall {}");
                 }
             })
@@ -59,11 +62,12 @@ impl<const NUM_BUTTONS: usize> Buttons<NUM_BUTTONS> {
 }
 
 impl<const NUM_BUTTONS: usize> crate::fake::SyscallDriver for Buttons<NUM_BUTTONS> {
-    fn id(&self) -> u32 {
-        DRIVER_NUM
+    fn info(&self) -> DriverInfo {
+        DriverInfo::new(DRIVER_NUM).upcall_count(1)
     }
-    fn num_upcalls(&self) -> u32 {
-        1
+
+    fn register(&self, share_ref: DriverShareRef) {
+        self.share_ref.replace(share_ref);
     }
 
     fn command(&self, command_number: u32, argument0: u32, _argument1: u32) -> CommandReturn {