Merge pull request #49 from SteelPh0enix/critical-section-cleanup

ROS#160: Moved `critical-section` to Aerugo, removed from HAL

Cargo.toml

@@ -49,6 +49,7 @@ aerugo-hal = { version = "0.1.0", path = "aerugo-hal" }
 aerugo-samv71-hal = { version = "0.1.0", path = "arch/cortex-m/aerugo-samv71-hal", optional = true }
 aerugo-x86 = { version = "0.1.0", path = "arch/x86/aerugo-x86", optional = true }
 aerugo-x86-hal = { version = "0.1.0", path = "arch/x86/aerugo-x86-hal", optional = true }
+critical-section = "1.1.2"
 env-parser = { version = "1.0.0", path = "utils/env-parser" }
 heapless = "0.7"
 samv71-hal = { version = "0.1.0", path = "arch/cortex-m/samv71-hal", optional = true }
@@ -60,7 +61,7 @@ assert_cmd = "2.0"
 [features]
 default = ["log"]
 use-aerugo-cortex-m = ["aerugo-cortex-m", "aerugo-samv71-hal"]
-use-aerugo-x86 = ["aerugo-x86", "aerugo-x86-hal"]
+use-aerugo-x86 = ["aerugo-x86", "aerugo-x86-hal", "critical-section/std"]
 test-aerugo-cortex-m = ["use-aerugo-x86"]
 rt = ["aerugo-samv71-hal?/rt"]
 log = ["aerugo-cortex-m?/log", "aerugo-x86?/log"]

aerugo-hal/Cargo.toml

@@ -10,5 +10,4 @@ license.workspace = true
 description = "Hardware Abstraction Layer for Aerugo system"
 
 [dependencies]
-critical-section = "1.1.2"
 fugit = "0.3.6"

aerugo-hal/src/config.rs

@@ -6,15 +6,12 @@ use crate::time;
 pub struct SystemHardwareConfig {
     /// Timeout for the watchdog.
     pub watchdog_timeout: time::MillisDurationU32,
-    /// If true, all interrupts will be disabled until `AERUGO.start()` is called.
-    pub disable_interrupts_during_setup: bool,
 }
 
 impl Default for SystemHardwareConfig {
     fn default() -> Self {
         SystemHardwareConfig {
             watchdog_timeout: time::MillisDurationU32::secs(3),
-            disable_interrupts_during_setup: true,
         }
     }
 }

aerugo-hal/src/lib.rs

@@ -11,8 +11,6 @@ HAL (Hardware Abstract Layer) for Aerugo system.
 mod config;
 
 pub use config::SystemHardwareConfig;
-pub use critical_section;
-pub use critical_section::CriticalSection;
 pub use fugit as time;
 
 /// Constant representing system timer frequency.
@@ -42,25 +40,4 @@ pub trait AerugoHal {
 
     /// Feeds the system watchdog.
     fn feed_watchdog();
-
-    /// Enters critical section
-    fn enter_critical();
-
-    /// Exits critical section
-    fn exit_critical();
-
-    /// Executes closure `f` in an interrupt-free context.
-    ///
-    /// # Generic Parameters
-    /// * `F` - Closure type.
-    /// * `R` - Closure return type.
-    ///
-    /// # Parameters
-    /// * `f` - Closure to execute.
-    ///
-    /// # Return
-    /// Closure result.
-    fn execute_critical<F, R>(f: F) -> R
-    where
-        F: FnOnce(CriticalSection) -> R;
 }

arch/cortex-m/aerugo-cortex-m/Cargo.toml

@@ -10,7 +10,6 @@ license.workspace = true
 description = "Cortex-M specific implementation for Aerugo"
 
 [dependencies]
-cortex-m = "0.7.7"
 rtt-target = "0.4.0"
 
 [features]

arch/cortex-m/aerugo-cortex-m/src/lib.rs

@@ -8,8 +8,6 @@ Cortex-M specific implementation for Aerugo.
 
 #[cfg(feature = "log")]
 mod logger;
-mod mutex;
 
 #[cfg(feature = "log")]
 pub use self::logger::{init_log, log, logln};
-pub use self::mutex::Mutex;

arch/cortex-m/aerugo-samv71-hal/Cargo.toml

@@ -11,6 +11,7 @@ description = "Crate with AerugoHal implementation for SAMV71"
 
 [dependencies]
 aerugo-hal = { version = "0.1.0", path = "../../../aerugo-hal" }
+critical-section = "1.1.2"
 samv71-hal = { version = "0.1.0", path = "../samv71-hal" }
 
 [features]

arch/cortex-m/aerugo-samv71-hal/src/hal.rs

@@ -1,7 +1,6 @@
 //! System HAL implementation for Cortex-M SAMV71 target.
 
-use aerugo_hal::critical_section;
-use aerugo_hal::{AerugoHal, CriticalSection, Instant, SystemHardwareConfig};
+use aerugo_hal::{AerugoHal, Instant, SystemHardwareConfig};
 
 use crate::cortex_m;
 use crate::error::HalError;
@@ -47,7 +46,7 @@ impl Hal {
     /// [`Some(UserPeripherals)`] if called for the first time after HAL initialization,
     /// [`None`] otherwise.
     pub fn create_user_peripherals() -> Option<UserPeripherals> {
-        Hal::execute_critical(|_| {
+        critical_section::with(|_| {
             if let Some(system_peripherals) = unsafe { &mut HAL_SYSTEM_PERIPHERALS } {
                 let mcu_peripherals = unsafe { pac::Peripherals::steal() };
                 let core_peripherals = unsafe { pac::CorePeripherals::steal() };
@@ -134,7 +133,7 @@ impl AerugoHal for Hal {
     /// # Return
     /// `()` on success, [`HalError`] if HAL was already initialized.
     fn configure_hardware(config: SystemHardwareConfig) -> Result<(), HalError> {
-        let result = Hal::execute_critical(|_| {
+        critical_section::with(|_| {
             Hal::initialize()?;
 
             // SAFETY: Immutable access to system peripherals is safe, as we're in critical section
@@ -184,13 +183,7 @@ impl AerugoHal for Hal {
             peripherals.timer.trigger_all_channels();
 
             Ok(())
-        });
-
-        if config.disable_interrupts_during_setup {
-            Hal::enter_critical();
-        }
-
-        result
+        })
     }
 
     fn get_system_time() -> Instant {
@@ -234,26 +227,6 @@ impl AerugoHal for Hal {
 
         peripherals.watchdog.feed();
     }
-
-    /// Enters critical section by disabling global interrupts.
-    fn enter_critical() {
-        cortex_m::interrupt::disable();
-    }
-
-    /// Exits critical section by enabling global interrupts.
-    ///
-    /// # Safety
-    /// <div class="warning">This function should never be called from scope-bound critical sections (like the one created with <code>AerugoHal::execute_critical</code>)</div>
-    fn exit_critical() {
-        unsafe { cortex_m::interrupt::enable() };
-    }
-
-    fn execute_critical<F, R>(f: F) -> R
-    where
-        F: FnOnce(CriticalSection) -> R,
-    {
-        critical_section::with(f)
-    }
 }
 
 /// Type representing all TC0 channels in Waveform mode.

arch/x86/aerugo-x86-hal/src/hal.rs

@@ -4,7 +4,7 @@ use std::convert::TryInto;
 use std::time::SystemTime;
 
 use aerugo_hal::Instant;
-use aerugo_hal::{AerugoHal, CriticalSection, SystemHardwareConfig};
+use aerugo_hal::{AerugoHal, SystemHardwareConfig};
 use once_cell::sync::Lazy;
 
 use crate::error::HalError;
@@ -45,20 +45,4 @@ impl AerugoHal for Hal {
     fn feed_watchdog() {
         // There is no watchdog for x86 target.
     }
-
-    fn enter_critical() {
-        // There is no critical section implementation for x86 target.
-    }
-
-    fn exit_critical() {
-        // There is no critical section implementation for x86 target.
-    }
-
-    fn execute_critical<F, R>(f: F) -> R
-    where
-        F: FnOnce(CriticalSection) -> R,
-    {
-        // There is no critical section implementation for x86 target.
-        f(unsafe { CriticalSection::new() })
-    }
 }

arch/x86/aerugo-x86/src/lib.rs

@@ -7,8 +7,6 @@ x86 specific implementation for Aerugo.
 
 #[cfg(feature = "log")]
 mod logger;
-mod mutex;
 
 #[cfg(feature = "log")]
 pub use self::logger::{init_log, log, logln};
-pub use self::mutex::Mutex;

src/aerugo.rs

@@ -5,7 +5,8 @@
 //!
 //! This module also contains singleton instances of all system parts.
 
-use aerugo_hal::{AerugoHal, CriticalSection, SystemHardwareConfig};
+use aerugo_hal::{AerugoHal, SystemHardwareConfig};
+use critical_section::CriticalSection;
 use env_parser::read_env;
 
 use crate::api::{InitApi, InitError, RuntimeApi, RuntimeError, SystemApi};
@@ -95,7 +96,6 @@ impl Aerugo {
                 .set_system_start()
                 .expect("Failed to set system start time");
         }
-        Hal::exit_critical();
         EXECUTOR.run_scheduler()
     }
 }
@@ -746,19 +746,11 @@ impl RuntimeApi for Aerugo {
         todo!()
     }
 
-    fn enter_critical() {
-        Hal::enter_critical();
-    }
-
-    fn exit_critical() {
-        Hal::exit_critical();
-    }
-
     fn execute_critical<F, R>(f: F) -> R
     where
         F: FnOnce(CriticalSection) -> R,
     {
-        Hal::execute_critical(f)
+        critical_section::with(f)
     }
 }
 

src/api/runtime_api.rs

@@ -2,7 +2,7 @@
 //!
 //! This API can be used by the user in tasklet functions to interact with the system.
 
-use aerugo_hal::CriticalSection;
+use critical_section::CriticalSection;
 
 use crate::api::RuntimeError;
 use crate::event::EventId;
@@ -62,16 +62,6 @@ pub trait RuntimeApi {
     /// Execution statistics for this tasklet.
     fn get_execution_statistics(&'static self, task_id: TaskletId) -> ExecutionStats;
 
-    /// Enters critical section
-    fn enter_critical()
-    where
-        Self: Sized;
-
-    /// Exits critical section
-    fn exit_critical()
-    where
-        Self: Sized;
-
     /// Executes closure `f` in an interrupt-free context.
     ///
     /// # Generic Parameters

src/boolean_condition.rs

@@ -11,9 +11,9 @@ pub use self::boolean_condition_storage::BooleanConditionStorage;
 
 use crate::aerugo::{Aerugo, AERUGO};
 use crate::api::{InitError, SystemApi};
-use crate::arch::Mutex;
 use crate::data_provider::DataProvider;
 use crate::internal_list::InternalList;
+use crate::mutex::Mutex;
 use crate::tasklet::TaskletPtr;
 
 /// List of tasklets registered to a condition

src/event/event_set.rs

@@ -4,10 +4,10 @@ use heapless::spsc::Queue;
 
 use crate::aerugo::AERUGO;
 use crate::api::{RuntimeError, SystemApi};
-use crate::arch::Mutex;
 use crate::data_provider::DataProvider;
 use crate::event::EventId;
 use crate::event_manager::EventManager;
+use crate::mutex::Mutex;
 use crate::tasklet::TaskletPtr;
 use crate::utils::max;
 

src/executor.rs

@@ -9,7 +9,7 @@ use heapless::binary_heap::{BinaryHeap, Max};
 
 use crate::aerugo::{Aerugo, AERUGO};
 use crate::api::{RuntimeApi, RuntimeError, SystemApi};
-use crate::arch::Mutex;
+use crate::mutex::Mutex;
 use crate::tasklet::{TaskletPtr, TaskletStatus};
 
 /// Type for the tasklet execution queue

src/lib.rs

@@ -20,6 +20,7 @@ mod execution_monitoring;
 mod executor;
 mod internal_list;
 mod message_queue;
+mod mutex;
 mod stubs;
 mod tasklet;
 mod time_source;
@@ -34,13 +35,16 @@ pub use self::event::{EventEnabler, EventId};
 pub use self::message_queue::{MessageQueueHandle, MessageQueueStorage};
 pub use self::tasklet::{TaskletConfig, TaskletStorage};
 pub use aerugo_hal::{time, Duration, Instant, SystemHardwareConfig};
+pub use mutex::Mutex;
 
 #[cfg(feature = "use-aerugo-cortex-m")]
+#[cfg(feature = "log")]
 pub(crate) use aerugo_cortex_m as arch;
 #[cfg(feature = "use-aerugo-cortex-m")]
 pub use aerugo_samv71_hal as hal;
 
 #[cfg(feature = "use-aerugo-x86")]
+#[cfg(feature = "log")]
 pub(crate) use aerugo_x86 as arch;
 #[cfg(feature = "use-aerugo-x86")]
 pub use aerugo_x86_hal as hal;

src/message_queue.rs

@@ -10,9 +10,9 @@ pub(crate) use self::message_queue_storage::QueueData;
 
 use crate::aerugo::{Aerugo, AERUGO};
 use crate::api::{InitError, RuntimeError, SystemApi};
-use crate::arch::Mutex;
 use crate::data_provider::DataProvider;
 use crate::internal_list::InternalList;
+use crate::mutex::Mutex;
 use crate::tasklet::TaskletPtr;
 
 /// List of tasklets registered to a queue