Merge pull request #4644 from 0xc0170/fix_ticker_delta_negative

Ticker: add fire interrupt now function

Author: theotherjimmy

TESTS/mbed_hal/ticker/main.cpp

@@ -42,6 +42,7 @@ struct ticker_interface_stub_t {
     unsigned int disable_interrupt_call;
     unsigned int clear_interrupt_call;
     unsigned int set_interrupt_call;
+    unsigned int fire_interrupt_call;
 };
 
 static ticker_interface_stub_t interface_stub = { 0 };
@@ -75,6 +76,11 @@ static void ticker_interface_stub_set_interrupt(timestamp_t timestamp)
     interface_stub.interrupt_timestamp = timestamp; 
 }
 
+static void ticker_interface_stub_fire_interrupt()
+{
+    ++interface_stub.fire_interrupt_call;
+}
+
 static void reset_ticker_interface_stub()
 {
     interface_stub.interface.init = ticker_interface_stub_init;
@@ -84,6 +90,7 @@ static void reset_ticker_interface_stub()
     interface_stub.interface.clear_interrupt = 
         ticker_interface_stub_clear_interrupt;
     interface_stub.interface.set_interrupt =ticker_interface_stub_set_interrupt;
+    interface_stub.interface.fire_interrupt = ticker_interface_stub_fire_interrupt;
     interface_stub.initialized = false;
     interface_stub.interrupt_flag = false;
     interface_stub.timestamp = 0;
@@ -93,6 +100,7 @@ static void reset_ticker_interface_stub()
     interface_stub.disable_interrupt_call = 0;
     interface_stub.clear_interrupt_call = 0;
     interface_stub.set_interrupt_call = 0;
+    interface_stub.fire_interrupt_call = 0;
 }
 
 // stub of the event queue 
@@ -949,11 +957,7 @@ static void test_insert_event_us_underflow()
     );
 
     TEST_ASSERT_EQUAL_PTR(&event, queue_stub.head);
-    TEST_ASSERT_EQUAL_UINT32(
-        interface_stub.timestamp, 
-        interface_stub.interrupt_timestamp
-    );
-    TEST_ASSERT_EQUAL(1, interface_stub.set_interrupt_call);
+    TEST_ASSERT_EQUAL(1, interface_stub.fire_interrupt_call);
 
     TEST_ASSERT_EQUAL(0, interface_stub.disable_interrupt_call);
 }  
@@ -1982,6 +1986,58 @@ static void test_irq_handler_insert_non_immediate_in_irq()
     TEST_ASSERT_EQUAL(0, interface_stub.disable_interrupt_call);
 }
 
+static uint32_t ticker_interface_stub_read_interrupt_time()
+{
+    ++interface_stub.read_call;
+    // only if set interrupt call, to test the condition afterwards
+    if (interface_stub.set_interrupt_call) {
+        return interface_stub.interrupt_timestamp;
+    } else {
+        return interface_stub.timestamp;
+    }
+} 
+
+/**
+ * Test to insert an event that is already in the past, the fire_interrupt should
+ * be invoked, instead of set_interrupt
+ */
+static void test_set_interrupt_past_time()
+{
+    interface_stub.set_interrupt_call = 0;
+    interface_stub.fire_interrupt_call = 0;
+    interface_stub.timestamp = 0xFF;
+
+
+    // This tests fire now functinality when next_event_timestamp <= present
+    ticker_event_t event = { 0 };
+    const timestamp_t event_timestamp = interface_stub.timestamp;
+    const uint32_t id_last_event = 0xDEADDEAF;
+
+    ticker_insert_event(&ticker_stub, &event, event_timestamp, id_last_event);
+    TEST_ASSERT_EQUAL(0, interface_stub.set_interrupt_call);
+    TEST_ASSERT_EQUAL(1, interface_stub.fire_interrupt_call);
+}
+/**
+ * Test to insert an event that is being delayed, set_interrupt is set
+ * but then event is already in the past, thus fire_interrupt should be invoked right-away
+ */
+static void test_set_interrupt_past_time_with_delay()
+{
+    interface_stub.set_interrupt_call = 0;
+    interface_stub.fire_interrupt_call = 0;
+    interface_stub.timestamp = 0xFF;
+
+    // This tests fire now functionality when present time >= new_match_time
+    interface_stub.interface.read = ticker_interface_stub_read_interrupt_time;
+    ticker_event_t event = { 0 };
+    const timestamp_t event_timestamp = interface_stub.timestamp + 5;
+    const uint32_t id_last_event = 0xDEADDEAF;
+
+    ticker_insert_event(&ticker_stub, &event, event_timestamp, id_last_event);
+    TEST_ASSERT_EQUAL(1, interface_stub.set_interrupt_call);
+    TEST_ASSERT_EQUAL(1, interface_stub.fire_interrupt_call);
+}
+
 static const case_t cases[] = {
     MAKE_TEST_CASE("ticker initialization", test_ticker_initialization),
     MAKE_TEST_CASE(
@@ -2066,6 +2122,14 @@ static const case_t cases[] = {
     MAKE_TEST_CASE(
         "test_irq_handler_insert_non_immediate_in_irq", 
         test_irq_handler_insert_non_immediate_in_irq
+    ),
+    MAKE_TEST_CASE(
+        "test_set_interrupt_past_time", 
+        test_set_interrupt_past_time
+    ),
+    MAKE_TEST_CASE(
+        "test_set_interrupt_past_time_with_delay", 
+        test_set_interrupt_past_time_with_delay
     )
 };
 
@@ -2079,4 +2143,4 @@ int main()
 {
     Specification specification(greentea_test_setup, cases, greentea_test_teardown_handler);
     return !Harness::run(specification);
-}
+}

hal/lp_ticker_api.h

@@ -74,6 +74,12 @@ void lp_ticker_disable_interrupt(void);
  */
 void lp_ticker_clear_interrupt(void);
 
+/** Set pending interrupt that should be fired right away.
+ * 
+ * The ticker should be initialized prior calling this function.
+ */
+void lp_ticker_fire_interrupt(void);
+
 /**@}*/
 
 #ifdef __cplusplus

hal/mbed_lp_ticker_api.c

@@ -25,6 +25,7 @@ static const ticker_interface_t lp_interface = {
     .disable_interrupt = lp_ticker_disable_interrupt,
     .clear_interrupt = lp_ticker_clear_interrupt,
     .set_interrupt = lp_ticker_set_interrupt,
+    .fire_interrupt = lp_ticker_fire_interrupt,
 };
 
 static const ticker_data_t lp_data = {

hal/mbed_ticker_api.c

@@ -117,14 +117,23 @@ static void schedule_interrupt(const ticker_data_t *const ticker)
 
         // if the event at the head of the queue is in the past then schedule
         // it immediately.
-        if (next_event_timestamp < present) {
-            relative_timeout = 0;
+        if (next_event_timestamp <= present) {
+            ticker->interface->fire_interrupt();
+            return;
         } else if ((next_event_timestamp - present) < MBED_TICKER_INTERRUPT_TIMESTAMP_MAX_DELTA) {
             relative_timeout = next_event_timestamp - present;
         }
     } 
 
-    ticker->interface->set_interrupt(ticker->queue->present_time + relative_timeout);
+    us_timestamp_t new_match_time = ticker->queue->present_time + relative_timeout;
+    ticker->interface->set_interrupt(new_match_time);
+    // there could be a delay, reread the time, check if it was set in the past
+    // As result, if it is already in the past, we fire it immediately
+    update_present_time(ticker);
+    us_timestamp_t present = ticker->queue->present_time;
+    if (present >= new_match_time) {
+        ticker->interface->fire_interrupt();
+    }
 }
 
 void ticker_set_handler(const ticker_data_t *const ticker, ticker_event_handler handler)

hal/mbed_us_ticker_api.c

@@ -23,6 +23,7 @@ static const ticker_interface_t us_interface = {
     .disable_interrupt = us_ticker_disable_interrupt,
     .clear_interrupt = us_ticker_clear_interrupt,
     .set_interrupt = us_ticker_set_interrupt,
+    .fire_interrupt = us_ticker_fire_interrupt,
 };
 
 static const ticker_data_t us_data = {

hal/ticker_api.h

@@ -60,6 +60,7 @@ typedef struct {
     void (*disable_interrupt)(void);              /**< Disable interrupt function */
     void (*clear_interrupt)(void);                /**< Clear interrupt function */
     void (*set_interrupt)(timestamp_t timestamp); /**< Set interrupt function */
+    void (*fire_interrupt)(void);                 /**< Fire interrupt right-away */
 } ticker_interface_t;
 
 /** Ticker's event queue structure

hal/us_ticker_api.h

@@ -72,6 +72,12 @@ void us_ticker_disable_interrupt(void);
  */
 void us_ticker_clear_interrupt(void);
 
+/** Set pending interrupt that should be fired right away.
+ * 
+ * The ticker should be initialized prior calling this function.
+ */
+void us_ticker_fire_interrupt(void);
+
 /**@}*/
 
 #ifdef __cplusplus

targets/TARGET_ARM_SSG/TARGET_BEETLE/lp_ticker.c

@@ -115,27 +115,24 @@ uint32_t lp_ticker_read(void)
  */
 void lp_ticker_set_interrupt(timestamp_t timestamp)
 {
-    int32_t delta = 0;
-
     /* Verify if lp_ticker has been not Initialized */
     if (lp_ticker_initialized == 0)
         lp_ticker_init();
 
     /* Calculate the delta */
-    delta = (int32_t)(timestamp - lp_ticker_read());
-    /* Check if the event was in the past */
-    if (delta <= 0) {
-        /* This event was in the past */
-        DualTimer_SetInterrupt_1(DUALTIMER0, 0,
-                DUALTIMER_COUNT_32 | DUALTIMER_ONESHOT);
-        return;
-    }
+    uint32_t delta = timestamp - lp_ticker_read();
 
     /* Enable interrupt on SingleTimer1 */
     DualTimer_SetInterrupt_1(DUALTIMER0, delta,
             DUALTIMER_COUNT_32 | DUALTIMER_ONESHOT);
 }
 
+void lp_ticker_fire_interrupt(void)
+{
+    uint32_t lp_ticker_irqn = DualTimer_GetIRQn(DUALTIMER0);
+    NVIC_SetPendingIRQ((IRQn_Type)lp_ticker_irqn);
+}
+
 /**
  * Disable low power ticker interrupt
  */

targets/TARGET_ARM_SSG/TARGET_BEETLE/us_ticker.c

@@ -91,22 +91,19 @@ uint32_t us_ticker_read() {
 }
 
 void us_ticker_set_interrupt(timestamp_t timestamp) {
-    int32_t delta = 0;
-
     if (!us_ticker_inited)
         us_ticker_init();
-    delta = (int32_t)(timestamp - us_ticker_read());
-    /* Check if the event was in the past */
-    if (delta <= 0) {
-        /* This event was in the past */
-        Timer_SetInterrupt(TIMER0, 0);
-        return;
-    }
 
-    /* If the event was not in the past enable interrupt */
+    uint32_t delta = timestamp - us_ticker_read();
     Timer_SetInterrupt(TIMER0, delta);
 }
 
+void us_ticker_fire_interrupt(void)
+{
+    uint32_t us_ticker_irqn1 = Timer_GetIRQn(TIMER1);
+    NVIC_SetPendingIRQ((IRQn_Type)us_ticker_irqn1);
+}
+
 void us_ticker_disable_interrupt(void) {
     Timer_DisableInterrupt(TIMER0);
 }

targets/TARGET_ARM_SSG/TARGET_CM3DS_MPS2/us_ticker.c

@@ -108,22 +108,23 @@ uint32_t us_ticker_read()
 
 void us_ticker_set_interrupt(timestamp_t timestamp)
 {
-    int32_t delta = 0;
+    uint32_t delta = 0;
 
     if (!us_ticker_drv_data.inited) {
         us_ticker_init();
     }
 
-    delta = (int32_t)(timestamp - us_ticker_read());
+    delta = timestamp - us_ticker_read();
 
-    /* Check if the event was in the past */
-    if (delta <= 0) {
-        /* This event was in the past */
-        Timer_SetInterrupt(TIMER0, 0);
-    } else {
-        /* If the event was not in the past enable interrupt */
-        Timer_SetInterrupt(TIMER0, delta);
-    }
+    /* If the event was not in the past enable interrupt */
+    Timer_SetInterrupt(TIMER0, delta);
+
+}
+
+void us_ticker_fire_interrupt(void)
+{
+    uint32_t us_ticker_irqn1 = Timer_GetIRQn(TIMER1);
+    NVIC_SetPendingIRQ((IRQn_Type)us_ticker_irqn1);
 }
 
 void us_ticker_disable_interrupt(void)

targets/TARGET_ARM_SSG/TARGET_IOTSS/us_ticker.c

@@ -69,6 +69,11 @@ void us_ticker_set_interrupt(timestamp_t timestamp) {
     US_TICKER_TIMER1->TimerControl |= 0x80; //enable timer
 }
 
+void us_ticker_fire_interrupt(void)
+{
+    NVIC_SetPendingIRQ(US_TICKER_TIMER_IRQn);
+}
+
 void us_ticker_disable_interrupt(void) {
 
     US_TICKER_TIMER1->TimerControl &= 0xDF;

targets/TARGET_ARM_SSG/TARGET_MPS2/us_ticker.c

@@ -51,22 +51,23 @@ uint32_t return_value = 0;
 }
 
 void us_ticker_set_interrupt(timestamp_t timestamp) {
-int delta = 0;
     if (!us_ticker_inited)
         us_ticker_init();
-    delta = (int)(timestamp - us_ticker_read());
-    if (delta <= 0) {
-        // This event was in the past:
-        us_ticker_irq_handler();
-        return;
-    }
+
+     uint32_t delta = timestamp - us_ticker_read();
         // enable interrupt
     US_TICKER_TIMER1->TimerControl = 0x0; // disable timer
     US_TICKER_TIMER1->TimerControl = 0x62; // enable interrupt and set to 32 bit counter and set to periodic mode
     US_TICKER_TIMER1->TimerLoad = (delta)*25; //initialise the timer value
     US_TICKER_TIMER1->TimerControl |= 0x80; //enable timer
 }
 
+void us_ticker_fire_interrupt(void)
+{
+    NVIC_SetPendingIRQ(US_TICKER_TIMER_IRQn);
+}
+
+
 void us_ticker_disable_interrupt(void) {
 
     US_TICKER_TIMER1->TimerControl &= 0xDF;

targets/TARGET_Atmel/TARGET_SAM_CortexM4/lp_ticker.c

@@ -86,15 +86,10 @@ uint32_t lp_ticker_read()
 void lp_ticker_set_interrupt(timestamp_t timestamp)
 {
     uint32_t cur_time;
-    int32_t delta;
+    uint32_t delta;
 
     cur_time = lp_ticker_read();
-    delta = (int32_t)((uint32_t)timestamp - cur_time);
-    if (delta < 0) {
-        /* Event already occurred in past */
-        lp_ticker_irq_handler();
-        return;
-    }
+    delta = timestamp - cur_time;
 
     uint16_t interruptat=0;
 
@@ -120,6 +115,11 @@ void lp_ticker_set_interrupt(timestamp_t timestamp)
     tc_start(TICKER_COUNTER_lp, TICKER_COUNTER_CHANNEL2);
 }
 
+void lp_ticker_fire_interrupt(void)
+{
+    NVIC_SetPendingIRQ(TICKER_COUNTER_IRQn2);
+}
+
 void lp_ticker_disable_interrupt(void)
 {
     tc_stop(TICKER_COUNTER_lp, TICKER_COUNTER_CHANNEL2);

targets/TARGET_Atmel/TARGET_SAM_CortexM4/us_ticker.c

@@ -170,6 +170,11 @@ void us_ticker_set_interrupt(timestamp_t timestamp)
     tc_start(TICKER_COUNTER_uS, TICKER_COUNTER_CHANNEL1);
 }
 
+void us_ticker_fire_interrupt(void)
+{
+    NVIC_SetPendingIRQ(TICKER_COUNTER_IRQn1);
+}
+
 void us_ticker_disable_interrupt(void)
 {
     tc_stop(TICKER_COUNTER_uS, TICKER_COUNTER_CHANNEL1);