pbio/drv/block_device: Drive ADC loop with fewer events.

We yielded and polled immediately after initiating the ADC transfer to give other awaitables a chance to start waiting on the transfer. This leads to many extra events and could theoretically introduce a race condition if the DMA operation completes before it gets into the right wait state.

Instead, higher level code can await new samples by waiting until the DMA completion timestamp has passed the time of interest, without inducing more events. While we do this, we can conveniently introduce a handle to wait until a specified future sample, which can be used if signals need to settle before measuring the ADC value.

Author: laurensvalk

lib/pbio/drv/adc/adc_stm32_hal.c

@@ -47,7 +47,7 @@ static uint32_t pbdrv_adc_last_error;
 
 PROCESS(pbdrv_adc_process, "ADC");
 
-pbio_error_t pbdrv_adc_await_new_samples(pbio_os_state_t *state) {
+pbio_error_t pbdrv_adc_await_new_samples(pbio_os_state_t *state, pbio_os_timer_t *timer, uint32_t future) {
     return PBIO_ERROR_NOT_SUPPORTED;
 }
 

lib/pbio/drv/adc/adc_stm32f0.c

@@ -66,7 +66,7 @@ void pbdrv_adc_init(void) {
     process_start(&pbdrv_adc_process);
 }
 
-pbio_error_t pbdrv_adc_await_new_samples(pbio_os_state_t *state) {
+pbio_error_t pbdrv_adc_await_new_samples(pbio_os_state_t *state, pbio_os_timer_t *timer, uint32_t future) {
     return PBIO_ERROR_NOT_SUPPORTED;
 }
 

lib/pbio/drv/block_device/block_device_ev3.c

@@ -23,6 +23,7 @@
 #include "../core.h"
 
 #include <pbdrv/block_device.h>
+#include <pbdrv/clock.h>
 #include <pbdrv/gpio.h>
 
 #include <tiam1808/edma.h>
@@ -103,26 +104,32 @@ static struct {
     uint8_t spi_cmd_buf_rx[SPI_CMD_BUF_SZ];
 } spi_dev;
 
+static uint32_t last_spi_dma_complete_time;
+
+static void spi_dma_complete(void) {
+    // Only complete once RX and TX complete.
+    if (spi_dev.status & SPI_STATUS_WAIT_ANY) {
+        return;
+    }
+    SPIIntDisable(SOC_SPI_0_REGS, SPI_DMA_REQUEST_ENA_INT);
+    pbio_os_request_poll();
+    last_spi_dma_complete_time = pbdrv_clock_get_ms();
+}
+
 /**
  * Tx transfer complete.
  */
 void pbdrv_block_device_ev3_spi_tx_complete(void) {
     spi_dev.status &= ~SPI_STATUS_WAIT_TX;
-    if (!(spi_dev.status & SPI_STATUS_WAIT_ANY)) {
-        SPIIntDisable(SOC_SPI_0_REGS, SPI_DMA_REQUEST_ENA_INT);
-        pbio_os_request_poll();
-    }
+    spi_dma_complete();
 }
 
 /**
  * Rx transfer complete.
  */
 void pbdrv_block_device_ev3_spi_rx_complete(void) {
     spi_dev.status &= ~SPI_STATUS_WAIT_RX;
-    if (!(spi_dev.status & SPI_STATUS_WAIT_ANY)) {
-        SPIIntDisable(SOC_SPI_0_REGS, SPI_DMA_REQUEST_ENA_INT);
-        pbio_os_request_poll();
-    }
+    spi_dma_complete();
 }
 
 /**
@@ -735,10 +742,10 @@ pbio_error_t pbdrv_adc_get_ch(uint8_t ch, uint16_t *value) {
     return PBIO_SUCCESS;
 }
 
-pbio_error_t pbdrv_adc_await_new_samples(pbio_os_state_t *state) {
+pbio_error_t pbdrv_adc_await_new_samples(pbio_os_state_t *state, pbio_os_timer_t *timer, uint32_t future) {
     PBIO_OS_ASYNC_BEGIN(state);
-    PBIO_OS_AWAIT_UNTIL(state, (spi_dev.status & SPI_STATUS_WAIT_ANY));
-    PBIO_OS_AWAIT_UNTIL(state, !(spi_dev.status & SPI_STATUS_WAIT_ANY));
+    pbio_os_timer_set(timer, 0);
+    PBIO_OS_AWAIT_UNTIL(state, pbio_util_time_has_passed(last_spi_dma_complete_time, timer->start + future));
     PBIO_OS_ASYNC_END(PBIO_SUCCESS);
 }
 
@@ -887,11 +894,7 @@ pbio_error_t ev3_spi_process_thread(pbio_os_state_t *state, void *context) {
             channel_data,
             PBDRV_CONFIG_ADC_EV3_ADC_NUM_CHANNELS + PBDRV_ADC_EV3_NUM_DELAY_SAMPLES);
 
-        // Allow event loop to run once so that processes that await new
-        // samples can begin awaiting completion of the transfer.
-        PBIO_OS_AWAIT_ONCE_AND_POLL(state);
-
-        // Await for actual transfer to complete.
+        // Await for transfer to complete.
         PBIO_OS_AWAIT_WHILE(state, (spi_dev.status & SPI_STATUS_WAIT_ANY));
 
         pbio_os_timer_set(&timer, ADC_SAMPLE_PERIOD);

lib/pbio/include/pbdrv/adc.h

@@ -35,8 +35,10 @@ pbio_error_t pbdrv_adc_get_ch(uint8_t ch, uint16_t *value);
  * Not implemented on all platforms.
  *
  * @param [in] state    Protothread state.
+ * @param [in] timer    Parent process timer, used to store time of calling this.
+ * @param [in] future   How far into the future the sample should be (ms).
  */
-pbio_error_t pbdrv_adc_await_new_samples(pbio_os_state_t *state);
+pbio_error_t pbdrv_adc_await_new_samples(pbio_os_state_t *state, pbio_os_timer_t *timer, uint32_t future);
 
 #else
 
@@ -45,7 +47,7 @@ static inline pbio_error_t pbdrv_adc_get_ch(uint8_t ch, uint16_t *value) {
     return PBIO_ERROR_NOT_SUPPORTED;
 }
 
-static inline pbio_error_t pbdrv_adc_await_new_samples(pbio_os_state_t *state) {
+static inline pbio_error_t pbdrv_adc_await_new_samples(pbio_os_state_t *state, pbio_os_timer_t *timer, uint32_t future) {
     return PBIO_ERROR_NOT_SUPPORTED;
 }
 

lib/pbio/src/port_dcm_ev3.c

@@ -374,14 +374,12 @@ pbio_error_t pbio_port_dcm_thread(pbio_os_state_t *state, pbio_os_timer_t *timer
             while (!pbdrv_gpio_input(&pins->p2)) {
                 // Reflected intensity.
                 pbdrv_gpio_out_high(&pins->p5);
-                PBIO_OS_AWAIT_MS(state, timer, 2);
-                PBIO_OS_AWAIT(state, &dcm->child, pbdrv_adc_await_new_samples(&dcm->child));
+                PBIO_OS_AWAIT(state, &dcm->child, pbdrv_adc_await_new_samples(&dcm->child, timer, 2));
                 dcm->nxt_rgba.r = pbio_port_dcm_get_mv(pins, 1);
 
                 // Ambient intensity.
                 pbdrv_gpio_out_low(&pins->p5);
-                PBIO_OS_AWAIT_MS(state, timer, 2);
-                PBIO_OS_AWAIT(state, &dcm->child, pbdrv_adc_await_new_samples(&dcm->child));
+                PBIO_OS_AWAIT(state, &dcm->child, pbdrv_adc_await_new_samples(&dcm->child, timer, 2));
                 dcm->nxt_rgba.a = pbio_port_dcm_get_mv(pins, 1);
             }
             continue;
@@ -412,23 +410,19 @@ pbio_error_t pbio_port_dcm_thread(pbio_os_state_t *state, pbio_os_timer_t *timer
             while (!pbdrv_gpio_input(&pins->p2)) {
 
                 pbdrv_gpio_out_low(&pins->p5);
-                PBIO_OS_AWAIT_MS(state, timer, 2);
-                PBIO_OS_AWAIT(state, &dcm->child, pbdrv_adc_await_new_samples(&dcm->child));
+                PBIO_OS_AWAIT(state, &dcm->child, pbdrv_adc_await_new_samples(&dcm->child, timer, 2));
                 dcm->nxt_rgba.a = pbio_port_dcm_get_mv(pins, 6);
 
                 pbdrv_gpio_out_high(&pins->p5);
-                PBIO_OS_AWAIT_MS(state, timer, 2);
-                PBIO_OS_AWAIT(state, &dcm->child, pbdrv_adc_await_new_samples(&dcm->child));
+                PBIO_OS_AWAIT(state, &dcm->child, pbdrv_adc_await_new_samples(&dcm->child, timer, 2));
                 dcm->nxt_rgba.r = pbio_port_dcm_get_mv(pins, 6);
 
                 pbdrv_gpio_out_low(&pins->p5);
-                PBIO_OS_AWAIT_MS(state, timer, 2);
-                PBIO_OS_AWAIT(state, &dcm->child, pbdrv_adc_await_new_samples(&dcm->child));
+                PBIO_OS_AWAIT(state, &dcm->child, pbdrv_adc_await_new_samples(&dcm->child, timer, 2));
                 dcm->nxt_rgba.g = pbio_port_dcm_get_mv(pins, 6);
 
                 pbdrv_gpio_out_high(&pins->p5);
-                PBIO_OS_AWAIT_MS(state, timer, 2);
-                PBIO_OS_AWAIT(state, &dcm->child, pbdrv_adc_await_new_samples(&dcm->child));
+                PBIO_OS_AWAIT(state, &dcm->child, pbdrv_adc_await_new_samples(&dcm->child, timer, 2));
                 dcm->nxt_rgba.b = pbio_port_dcm_get_mv(pins, 6);
             }
             pbdrv_gpio_out_low(&pins->p5);