From d396da59502ef67f624bb4d8927ff8697232f66c Mon Sep 17 00:00:00 2001
From: Jorge Aparicio <jorge@japaric.io>
Date: Thu, 27 Jul 2017 17:08:42 -0500
Subject: [PATCH] make task.$T.enabled optional

and move the logic that differentiates interrupts from exceptions from the crate
to the procedural macro logic
---
 examples/full-syntax.rs         |  4 +-
 examples/generics.rs            |  2 -
 examples/modules.rs             |  3 --
 examples/nested.rs              |  3 --
 examples/preemption.rs          |  1 -
 examples/two-tasks.rs           |  1 -
 macros/src/check.rs             | 73 +++++++++++++++++++++++++++++----
 macros/src/trans.rs             | 69 ++++++++++++++++---------------
 src/lib.rs                      | 22 ----------
 tests/cfail/critical-section.rs |  1 -
 tests/cfail/duplicated-task.rs  |  1 -
 tests/cfail/exception.rs        |  4 +-
 tests/cfail/interrupt.rs        | 10 ++---
 tests/cfail/lock.rs             |  3 --
 tests/cfail/token-outlive.rs    |  2 -
 tests/cfail/token-transfer.rs   |  1 -
 tests/cfail/wrong-threshold.rs  |  2 -
 17 files changed, 111 insertions(+), 91 deletions(-)

diff --git a/examples/full-syntax.rs b/examples/full-syntax.rs
index 918a2e6..184349b 100644
--- a/examples/full-syntax.rs
+++ b/examples/full-syntax.rs
@@ -37,7 +37,9 @@ app! {
         },
 
         TIM2: {
-            enabled: true,
+            // tasks are enabled, between `init` and `idle`, by default but they
+            // can start disabled if `false` is specified here
+            enabled: false,
             path: tim2,
             priority: 1,
             resources: [CO_OWNED],
diff --git a/examples/generics.rs b/examples/generics.rs
index 7c261d5..6c6e1d6 100644
--- a/examples/generics.rs
+++ b/examples/generics.rs
@@ -15,14 +15,12 @@ app! {
 
     tasks: {
         EXTI0: {
-            enabled: true,
             path: exti0,
             priority: 1,
             resources: [GPIOA, SPI1],
         },
 
         EXTI1: {
-            enabled: true,
             path: exti1,
             priority: 2,
             resources: [GPIOA, SPI1],
diff --git a/examples/modules.rs b/examples/modules.rs
index 313ebc4..6bdf8d9 100644
--- a/examples/modules.rs
+++ b/examples/modules.rs
@@ -31,14 +31,11 @@ app! {
     tasks: {
         SYS_TICK: {
             path: tasks::sys_tick,
-            priority: 1,
             resources: [CO_OWNED, ON, SHARED],
         },
 
         TIM2: {
-            enabled: true,
             path: tasks::tim2,
-            priority: 1,
             resources: [CO_OWNED],
         },
     },
diff --git a/examples/nested.rs b/examples/nested.rs
index 2cb23f9..92f90cb 100644
--- a/examples/nested.rs
+++ b/examples/nested.rs
@@ -24,21 +24,18 @@ app! {
 
     tasks: {
         EXTI0: {
-            enabled: true,
             path: exti0,
             priority: 1,
             resources: [LOW, HIGH],
         },
 
         EXTI1: {
-            enabled: true,
             path: exti1,
             priority: 2,
             resources: [LOW],
         },
 
         EXTI2: {
-            enabled: true,
             path: exti2,
             priority: 3,
             resources: [HIGH],
diff --git a/examples/preemption.rs b/examples/preemption.rs
index e117695..057e4fd 100644
--- a/examples/preemption.rs
+++ b/examples/preemption.rs
@@ -25,7 +25,6 @@ app! {
         },
 
         TIM2: {
-            enabled: true,
             path: tim2,
             priority: 1,
             resources: [COUNTER],
diff --git a/examples/two-tasks.rs b/examples/two-tasks.rs
index 7343137..42b91e4 100644
--- a/examples/two-tasks.rs
+++ b/examples/two-tasks.rs
@@ -33,7 +33,6 @@ app! {
             // For interrupts the `enabled` field must be specified. It
             // indicates if the interrupt will be enabled or disabled once
             // `idle` starts
-            enabled: true,
             path: tim2,
             priority: 1,
             resources: [COUNTER],
diff --git a/macros/src/check.rs b/macros/src/check.rs
index 571f235..e4a716b 100644
--- a/macros/src/check.rs
+++ b/macros/src/check.rs
@@ -16,8 +16,39 @@ pub struct App {
 
 pub type Tasks = HashMap<Ident, Task>;
 
+#[allow(non_camel_case_types)]
+pub enum Exception {
+    PENDSV,
+    SVCALL,
+    SYS_TICK,
+}
+
+impl Exception {
+    pub fn from(s: &str) -> Option<Self> {
+        Some(match s {
+            "PENDSV" => Exception::PENDSV,
+            "SVCALL" => Exception::SVCALL,
+            "SYS_TICK" => Exception::SYS_TICK,
+            _ => return None,
+        })
+    }
+
+    pub fn nr(&self) -> usize {
+        match *self {
+            Exception::PENDSV => 14,
+            Exception::SVCALL => 11,
+            Exception::SYS_TICK => 15,
+        }
+    }
+}
+
+pub enum Kind {
+    Exception(Exception),
+    Interrupt { enabled: bool },
+}
+
 pub struct Task {
-    pub enabled: Option<bool>,
+    pub kind: Kind,
     pub path: Option<Path>,
     pub priority: u8,
     pub resources: Idents,
@@ -31,8 +62,13 @@ pub fn app(app: check::App) -> Result<App> {
         resources: app.resources,
         tasks: app.tasks
             .into_iter()
-            .map(|(k, v)| (k, ::check::task(v)))
-            .collect(),
+            .map(|(k, v)| {
+                let v = ::check::task(k.as_ref(), v)
+                    .chain_err(|| format!("checking task `{}`", k))?;
+
+                Ok((k, v))
+            })
+            .collect::<Result<_>>()?,
     };
 
     ::check::resources(&app)
@@ -60,11 +96,34 @@ fn resources(app: &App) -> Result<()> {
     Ok(())
 }
 
-fn task(task: syntax::check::Task) -> Task {
-    Task {
-        enabled: task.enabled,
+fn task(name: &str, task: syntax::check::Task) -> Result<Task> {
+    let kind = match Exception::from(name) {
+        Some(e) => {
+            ensure!(
+                task.enabled.is_none(),
+                "`enabled` field is not valid for exceptions"
+            );
+
+            Kind::Exception(e)
+        }
+        None => {
+            if task.enabled == Some(true) {
+                bail!(
+                    "`enabled: true` is the default value; this line can be \
+                     omitted"
+                );
+            }
+
+            Kind::Interrupt {
+                enabled: task.enabled.unwrap_or(true),
+            }
+        }
+    };
+
+    Ok(Task {
+        kind,
         path: task.path,
         priority: task.priority.unwrap_or(1),
         resources: task.resources,
-    }
+    })
 }
diff --git a/macros/src/trans.rs b/macros/src/trans.rs
index 468d967..02b5e8c 100644
--- a/macros/src/trans.rs
+++ b/macros/src/trans.rs
@@ -2,7 +2,7 @@ use quote::{Ident, Tokens};
 use syn::{Lit, StrStyle};
 
 use analyze::{Ownership, Ownerships};
-use check::App;
+use check::{App, Kind};
 
 fn krate() -> Ident {
     Ident::from("rtfm")
@@ -236,44 +236,47 @@ fn init(app: &App, main: &mut Vec<Tokens>, root: &mut Vec<Tokens>) {
     let mut exceptions = vec![];
     let mut interrupts = vec![];
     for (name, task) in &app.tasks {
-        if let Some(enabled) = task.enabled {
-            // Interrupt. These can be enabled / disabled through the NVIC
-            if interrupts.is_empty() {
-                interrupts.push(quote! {
-                    let nvic = &*#device::NVIC.get();
+        match task.kind {
+            Kind::Exception(ref e) => {
+                if exceptions.is_empty() {
+                    exceptions.push(quote! {
+                        let scb = &*#device::SCB.get();
+                    });
+                }
+
+                let nr = e.nr();
+                let priority = task.priority;
+                exceptions.push(quote! {
+                    let prio_bits = #device::NVIC_PRIO_BITS;
+                    let hw = ((1 << prio_bits) - #priority) << (8 - prio_bits);
+                    scb.shpr[#nr - 4].write(hw);
                 });
             }
+            Kind::Interrupt { enabled } => {
+                // Interrupt. These can be enabled / disabled through the NVIC
+                if interrupts.is_empty() {
+                    interrupts.push(quote! {
+                        let nvic = &*#device::NVIC.get();
+                    });
+                }
 
-            let priority = task.priority;
-            interrupts.push(quote! {
-                let prio_bits = #device::NVIC_PRIO_BITS;
-                let hw = ((1 << prio_bits) - #priority) << (8 - prio_bits);
-                nvic.set_priority(#device::Interrupt::#name, hw);
-            });
-
-            if enabled {
-                interrupts.push(quote! {
-                    nvic.enable(#device::Interrupt::#name);
-                });
-            } else {
+                let priority = task.priority;
                 interrupts.push(quote! {
-                    nvic.disable(#device::Interrupt::#name);
+                    let prio_bits = #device::NVIC_PRIO_BITS;
+                    let hw = ((1 << prio_bits) - #priority) << (8 - prio_bits);
+                    nvic.set_priority(#device::Interrupt::#name, hw);
                 });
-            }
-        } else {
-            // Exception
-            if exceptions.is_empty() {
-                exceptions.push(quote! {
-                    let scb = &*#device::SCB.get();
-                });
-            }
 
-            let priority = task.priority;
-            exceptions.push(quote! {
-                let prio_bits = #device::NVIC_PRIO_BITS;
-                let hw = ((1 << prio_bits) - #priority) << (8 - prio_bits);
-                scb.shpr[#krate::Exception::#name.nr() - 4].write(hw);
-            });
+                if enabled {
+                    interrupts.push(quote! {
+                        nvic.enable(#device::Interrupt::#name);
+                    });
+                } else {
+                    interrupts.push(quote! {
+                        nvic.disable(#device::Interrupt::#name);
+                    });
+                }
+            }
         }
     }
 
diff --git a/src/lib.rs b/src/lib.rs
index 802d503..dc85659 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -140,25 +140,3 @@ where
     let nvic = unsafe { &*cortex_m::peripheral::NVIC.get() };
     nvic.set_pending(interrupt);
 }
-
-#[allow(non_camel_case_types)]
-#[doc(hidden)]
-pub enum Exception {
-    /// System service call via SWI instruction
-    SVCALL,
-    /// Pendable request for system service
-    PENDSV,
-    /// System tick timer
-    SYS_TICK,
-}
-
-impl Exception {
-    #[doc(hidden)]
-    pub fn nr(&self) -> usize {
-        match *self {
-            Exception::SVCALL => 11,
-            Exception::PENDSV => 14,
-            Exception::SYS_TICK => 15,
-        }
-    }
-}
diff --git a/tests/cfail/critical-section.rs b/tests/cfail/critical-section.rs
index 7e73fd3..728388e 100644
--- a/tests/cfail/critical-section.rs
+++ b/tests/cfail/critical-section.rs
@@ -21,7 +21,6 @@ app! {
 
     tasks: {
         EXTI0: {
-            enabled: true,
             path: exti0,
             priority: 1,
             resources: [ON],
diff --git a/tests/cfail/duplicated-task.rs b/tests/cfail/duplicated-task.rs
index a167576..d91f09b 100644
--- a/tests/cfail/duplicated-task.rs
+++ b/tests/cfail/duplicated-task.rs
@@ -9,7 +9,6 @@ use rtfm::app;
 
 app! {
     //~^ error proc macro panicked
-    //~| help parsing
     device: stm32f103xx,
 
     tasks: {
diff --git a/tests/cfail/exception.rs b/tests/cfail/exception.rs
index 1428bcc..065ccad 100644
--- a/tests/cfail/exception.rs
+++ b/tests/cfail/exception.rs
@@ -7,9 +7,7 @@ extern crate stm32f103xx;
 
 use rtfm::app;
 
-app! {
-    //~^ error no associated item named `SYS_TICK` found for type
-    //~| error no associated item named `SYS_TICK` found for type
+app! { //~ error proc macro panicked
     device: stm32f103xx,
 
     tasks: {
diff --git a/tests/cfail/interrupt.rs b/tests/cfail/interrupt.rs
index 2969d1f..c70528f 100644
--- a/tests/cfail/interrupt.rs
+++ b/tests/cfail/interrupt.rs
@@ -7,14 +7,14 @@ extern crate stm32f103xx;
 
 use rtfm::app;
 
-app! { //~ error no associated item named `EXTI0` found for type
+app! {
+    //~^ error no associated item named `EXTI33` found for type
+    //~| error no associated item named `EXTI33` found for type
     device: stm32f103xx,
 
     tasks: {
-        // ERROR `enabled` needs to be specified for interrupts
-        EXTI0: {
-            priority: 1,
-        },
+        // ERROR this interrupt doesn't exist
+        EXTI33: {},
     },
 }
 
diff --git a/tests/cfail/lock.rs b/tests/cfail/lock.rs
index 8e6da46..e0e37e0 100644
--- a/tests/cfail/lock.rs
+++ b/tests/cfail/lock.rs
@@ -18,21 +18,18 @@ app! {
 
     tasks: {
         EXTI0: {
-            enabled: true,
             path: exti0,
             priority: 1,
             resources: [MAX, ON],
         },
 
         EXTI1: {
-            enabled: true,
             path: exti1,
             priority: 2,
             resources: [ON],
         },
 
         EXTI2: {
-            enabled: true,
             path: exti2,
             priority: 16,
             resources: [MAX],
diff --git a/tests/cfail/token-outlive.rs b/tests/cfail/token-outlive.rs
index dc9112e..31231b7 100644
--- a/tests/cfail/token-outlive.rs
+++ b/tests/cfail/token-outlive.rs
@@ -17,14 +17,12 @@ app! {
 
     tasks: {
         EXTI0: {
-            enabled: true,
             path: exti0,
             priority: 1,
             resources: [STATE],
         },
 
         EXTI1: {
-            enabled: true,
             path: exti1,
             priority: 2,
             resources: [STATE],
diff --git a/tests/cfail/token-transfer.rs b/tests/cfail/token-transfer.rs
index b8bcc00..d10b82b 100644
--- a/tests/cfail/token-transfer.rs
+++ b/tests/cfail/token-transfer.rs
@@ -17,7 +17,6 @@ app! { //~ error bound `rtfm::Threshold: core::marker::Send` is not satisfied
 
     tasks: {
         EXTI0: {
-            enabled: true,
             path: exti0,
             priority: 1,
             resources: [TOKEN],
diff --git a/tests/cfail/wrong-threshold.rs b/tests/cfail/wrong-threshold.rs
index 57d6d30..c9459af 100644
--- a/tests/cfail/wrong-threshold.rs
+++ b/tests/cfail/wrong-threshold.rs
@@ -18,14 +18,12 @@ app! {
 
     tasks: {
         EXTI0: {
-            enabled: true,
             path: exti0,
             priority: 1,
             resources: [A, B],
         },
 
         EXTI1: {
-            enabled: true,
             path: exti1,
             priority: 2,
             resources: [A, B],
-- 
GitLab