Fixed soem ADC bugs and optimized things a bit

TinyFrame/TF_Integration.c

@@ -7,7 +7,6 @@
 #include "platform.h"
 #include "task_main.h"
 
-#include "utils/hexdump.h"
 #include "USB/usbd_cdc_if.h"
 #include "TinyFrame.h"
 
@@ -20,13 +19,13 @@ void TF_WriteImpl(TinyFrame *tf, const uint8_t *buff, uint32_t len)
 #define CHUNK 64 // same as TF_SENDBUF_LEN, so we should always have only one run of the loop
     int32_t total = (int32_t) len;
     while (total > 0) {
-        int32_t mxStatus = osSemaphoreWait(semVcomTxReadyHandle, 250);
+        const int32_t mxStatus = osSemaphoreWait(semVcomTxReadyHandle, 100);
         if (mxStatus != osOK) {
             TF_Error("Tx stalled");
             return;
         }
 
-        uint16_t chunksize = (uint16_t) MIN(total, CHUNK);
+        const uint16_t chunksize = (uint16_t) MIN(total, CHUNK);
         assert_param(USBD_OK == CDC_Transmit_FS((uint8_t *) buff, chunksize));
 
         buff += chunksize;
@@ -38,11 +37,10 @@ void TF_WriteImpl(TinyFrame *tf, const uint8_t *buff, uint32_t len)
 bool TF_ClaimTx(TinyFrame *tf)
 {
     (void) tf;
-    assert_param(osThreadGetId() != tskMainHandle);
+//    assert_param(osThreadGetId() != tskMainHandle);
     assert_param(!inIRQ());
 
     assert_param(osOK == osMutexWait(mutTinyFrameTxHandle, 5000));
-
     return true;
 }
 

tasks/task_msg.c

@@ -11,7 +11,6 @@ volatile uint32_t msgQueHighWaterMark = 0;
 static bool que_safe_post(struct rx_sched_combined_que_item *slot)
 {
     uint32_t count = 0;
-    assert_param(slot != NULL);
 
     if (inIRQ()) {
         BaseType_t xHigherPriorityTaskWoken = pdFALSE;
@@ -80,12 +79,9 @@ void TaskMsgJob(const void *argument)
         xQueueReceive(queMsgJobHandle, &slot, osWaitForever);
 
         if (slot.is_job) {
-            assert_param(slot.job.cb != NULL);
             slot.job.cb(&slot.job);
         }
         else {
-            assert_param(slot.msg.len > 0 && slot.msg.len <= MSG_QUE_SLOT_SIZE); // check the len is within bounds
-
             #if CDC_LOOPBACK_TEST
                 TF_WriteImpl(comm, slot.msg.data, slot.msg.len);
             #else

units/adc/_adc_api.c

@@ -1,29 +0,0 @@
-//
-// Created by MightyPork on 2018/02/03.
-//
-
-#include "platform.h"
-#include "unit_base.h"
-#include "unit_adc.h"
-
-#define ADC_INTERNAL
-#include "_adc_internal.h"
-
-error_t UU_ADC_AbortCapture(Unit *unit)
-{
-    CHECK_TYPE(unit, &UNIT_ADC);
-    struct priv *priv = unit->data;
-
-    enum uadc_opmode old_opmode = priv->opmode;
-
-    priv->auto_rearm = false;
-    UADC_SwitchMode(unit, ADC_OPMODE_IDLE);
-
-    if (old_opmode == ADC_OPMODE_BLCAP ||
-        old_opmode == ADC_OPMODE_STREAM ||
-        old_opmode == ADC_OPMODE_TRIGD) {
-        UADC_ReportEndOfStream(unit);
-    }
-
-    return E_SUCCESS;
-}

units/adc/_adc_core.c

@@ -21,6 +21,8 @@ static void UADC_JobSendBlockChunk(Job *job)
     const bool close = (bool) (job->data3 & 0x80);
     const bool tc = (bool) (job->data3 & 0x01);
 
+//    assert_param(count <= priv->buf_itemcount);
+
     TF_TYPE type = close ? EVT_CAPT_DONE : EVT_CAPT_MORE;
 
     TF_Msg msg = {
@@ -28,7 +30,6 @@ static void UADC_JobSendBlockChunk(Job *job)
         .len = (TF_LEN) (1 /*seq*/ + count * sizeof(uint16_t)),
         .type = type,
     };
-
     TF_Respond_Multipart(comm, &msg);
     TF_Multipart_Payload(comm, &priv->stream_serial, 1);
     TF_Multipart_Payload(comm, (uint8_t *) (priv->dma_buffer + start), count * sizeof(uint16_t));
@@ -109,7 +110,7 @@ void UADC_ReportEndOfStream(Unit *unit)
 
     Job j = {
         .unit = unit,
-        .data1 = priv->stream_frame_id,
+        .data1 = priv->stream_frame_id, // copy the ID, it may be invalid by the time the cb gets executed
         .cb = UADC_JobSendEndOfStreamMsg
     };
     scheduleJob(&j);
@@ -128,14 +129,16 @@ static void handle_httc(Unit *unit, bool tc)
 
     if (ht) {
         end = (priv->buf_itemcount / 2);
-        LL_DMA_ClearFlag_HT(priv->DMAx, priv->dma_chnum);
     }
     else {
         end = priv->buf_itemcount;
-        LL_DMA_ClearFlag_TC(priv->DMAx, priv->dma_chnum);
     }
 
     if (start != end) {
+        if (end < start) {
+            trap("end < start! %d < %d, tc %d", (int)end, (int)start, (int)tc);
+        }
+
         uint32_t sgcount = (end - start) / priv->nb_channels;
 
         if (m_trigd || m_fixcpt) {
@@ -184,7 +187,7 @@ static void handle_httc(Unit *unit, bool tc)
         priv->stream_startpos = 0;
     }
     else {
-        priv->stream_startpos = end;
+        priv->stream_startpos = priv->buf_itemcount / 2;
     }
 }
 
@@ -207,6 +210,9 @@ void UADC_DMA_Handler(void *arg)
         const bool ht = LL_DMA_IsActiveFlag_HT(isrsnapshot, priv->dma_chnum);
         const bool te = LL_DMA_IsActiveFlag_TE(isrsnapshot, priv->dma_chnum);
 
+        if (ht) LL_DMA_ClearFlag_HT(priv->DMAx, priv->dma_chnum);
+        if (tc) LL_DMA_ClearFlag_TC(priv->DMAx, priv->dma_chnum);
+
         // check what mode we're in
         const bool m_trigd = priv->opmode == ADC_OPMODE_TRIGD;
         const bool m_stream = priv->opmode == ADC_OPMODE_STREAM;
@@ -214,8 +220,8 @@ void UADC_DMA_Handler(void *arg)
 
         if (m_trigd || m_stream || m_fixcpt) {
             if (ht || tc) {
+                const uint32_t half = (uint32_t) (priv->buf_itemcount / 2);
                 if (ht && tc) {
-                    const uint32_t half = (uint32_t) (priv->buf_itemcount / 2);
                     if (priv->stream_startpos > half) {
                         handle_httc(unit, true); // TC
                         handle_httc(unit, false); // HT
@@ -224,19 +230,16 @@ void UADC_DMA_Handler(void *arg)
                         handle_httc(unit, true); // TC
                     }
                 } else {
+                    if (ht && priv->stream_startpos > half) {
+                        // We missed the TC interrupt while e.g. setting up the stream / interrupt. catch up!
+                        handle_httc(unit, true); // TC
+                    }
                     handle_httc(unit, tc);
                 }
             }
         } else {
             // This shouldn't happen, the interrupt should be disabled in this opmode
             dbg("(!) not streaming, ADC DMA IT should be disabled");
-
-            if (ht) {
-                LL_DMA_ClearFlag_HT(priv->DMAx, priv->dma_chnum);
-            }
-            else {
-                LL_DMA_ClearFlag_TC(priv->DMAx, priv->dma_chnum);
-            }
         }
 
         if (te) {
@@ -351,6 +354,23 @@ void UADC_HandleTrigger(Unit *unit, uint8_t edge_type, uint64_t timestamp)
     UADC_SwitchMode(unit, ADC_OPMODE_TRIGD);
 }
 
+void UADC_AbortCapture(Unit *unit)
+{
+    struct priv *priv = unit->data;
+
+    const enum uadc_opmode old_opmode = priv->opmode;
+
+    priv->auto_rearm = false;
+
+    if (old_opmode == ADC_OPMODE_BLCAP ||
+        old_opmode == ADC_OPMODE_STREAM ||
+        old_opmode == ADC_OPMODE_TRIGD) {
+        UADC_ReportEndOfStream(unit);
+    }
+
+    UADC_SwitchMode(unit, ADC_OPMODE_IDLE);
+}
+
 void UADC_StartBlockCapture(Unit *unit, uint32_t len, TF_ID frame_id)
 {
     struct priv *priv = unit->data;
@@ -370,8 +390,6 @@ void UADC_StartStream(Unit *unit, TF_ID frame_id)
     if (priv->opmode == ADC_OPMODE_UNINIT) return;
 
     priv->stream_frame_id = frame_id;
-    priv->stream_startpos = DMA_POS(priv);
-    priv->stream_serial = 0;
     UADC_SwitchMode(unit, ADC_OPMODE_STREAM);
 }
 
@@ -394,9 +412,9 @@ void UADC_updateTick(Unit *unit)
     if (priv->opmode == ADC_OPMODE_EMERGENCY_SHUTDOWN) {
         adc_dbg("ADC recovering from emergency shutdown");
 
-        UADC_SwitchMode(unit, ADC_OPMODE_IDLE);
-        LL_TIM_EnableCounter(priv->TIMx);
         UADC_ReportEndOfStream(unit);
+        LL_TIM_EnableCounter(priv->TIMx);
+        UADC_SwitchMode(unit, ADC_OPMODE_IDLE);
         unit->tick_interval = 0;
         return;
     }
@@ -428,6 +446,8 @@ void UADC_SwitchMode(Unit *unit, enum uadc_opmode new_mode)
         adc_dbg("ADC switch -> UNINIT");
         // Stop the DMA, timer and disable ADC - this is called before tearing down the unit
         LL_TIM_DisableCounter(priv->TIMx);
+        LL_ADC_ClearFlag_EOS(priv->ADCx);
+        LL_ADC_DisableIT_EOS(priv->ADCx);
 
         // Switch off the ADC
         if (LL_ADC_IsEnabled(priv->ADCx)) {
@@ -444,6 +464,8 @@ void UADC_SwitchMode(Unit *unit, enum uadc_opmode new_mode)
         LL_DMA_DisableChannel(priv->DMAx, priv->dma_chnum);
         LL_DMA_DisableIT_HT(priv->DMAx, priv->dma_chnum);
         LL_DMA_DisableIT_TC(priv->DMAx, priv->dma_chnum);
+        LL_DMA_ClearFlag_HT(priv->DMAx, priv->dma_chnum);
+        LL_DMA_ClearFlag_TC(priv->DMAx, priv->dma_chnum);
     }
     else if (new_mode == ADC_OPMODE_IDLE || new_mode == ADC_OPMODE_REARM_PENDING) {
         // IDLE and ARMED are identical with the exception that the trigger condition is not checked
@@ -501,7 +523,6 @@ void UADC_SwitchMode(Unit *unit, enum uadc_opmode new_mode)
     else if (new_mode == ADC_OPMODE_TRIGD ||
         new_mode == ADC_OPMODE_STREAM ||
         new_mode == ADC_OPMODE_BLCAP) {
-
         adc_dbg("ADC switch -> CAPTURE");
         assert_param(old_mode == ADC_OPMODE_ARMED || old_mode == ADC_OPMODE_IDLE);
 
@@ -514,6 +535,12 @@ void UADC_SwitchMode(Unit *unit, enum uadc_opmode new_mode)
         LL_DMA_ClearFlag_HT(priv->DMAx, priv->dma_chnum);
         LL_DMA_ClearFlag_TC(priv->DMAx, priv->dma_chnum);
 
+        // those must be as close as possible to the enabling
+        // if not trig'd, we don't care for lost samples before (this could cause a DMA irq miss / ht/tc mismatch with the startpos)
+        if (new_mode != ADC_OPMODE_TRIGD) {
+            priv->stream_startpos = DMA_POS(priv);
+            priv->stream_serial = 0;
+        }
         LL_DMA_EnableIT_HT(priv->DMAx, priv->dma_chnum);
         LL_DMA_EnableIT_TC(priv->DMAx, priv->dma_chnum);
     }

units/adc/_adc_init.c

@@ -2,7 +2,6 @@
 // Created by MightyPork on 2018/02/03.
 //
 
-#include <stm32f072xb.h>
 #include "platform.h"
 #include "unit_base.h"
 
@@ -34,14 +33,12 @@ error_t UADC_SetSampleRate(Unit *unit, uint32_t hertz)
 
     uint16_t presc;
     uint32_t count;
-    if (!solve_timer(PLAT_APB1_HZ, hertz, true, &presc, &count,
+    if (!solve_timer(PLAT_APB1_HZ, hertz, true, &presc, &count, &priv->real_frequency)) {
-                     &priv->real_frequency)) {
         dbg("Failed to resolve timer params.");
         return E_BAD_VALUE;
     }
     adc_dbg("Frequency error %d ppm, presc %d, count %d",
-        (int) lrintf(1000000.0f *
+        (int) lrintf(1000000.0f *  ((priv->real_frequency - hertz) / (float) hertz)),
-                     ((priv->real_frequency - hertz) / (float) hertz)),
         (int) presc, (int) count);
 
     LL_TIM_SetPrescaler(priv->TIMx, (uint32_t) (presc - 1));
@@ -52,6 +49,44 @@ error_t UADC_SetSampleRate(Unit *unit, uint32_t hertz)
     return E_SUCCESS;
 }
 
+void UADC_SetupDMA(Unit *unit)
+{
+    struct priv *priv = unit->data;
+
+    adc_dbg("Setting up DMA");
+    {
+        uint32_t itemcount = priv->nb_channels * (priv->cfg.buffer_size / (priv->nb_channels));
+        if (itemcount % 2 == 1) itemcount -= priv->nb_channels; // ensure the count is even
+        priv->buf_itemcount = itemcount;
+
+        adc_dbg("DMA item count is %d (%d bytes), There are %d samples per group.",
+                (int)priv->buf_itemcount,
+                (int)(priv->buf_itemcount * sizeof(uint16_t)),
+                (int)priv->nb_channels);
+
+        {
+            LL_DMA_InitTypeDef init;
+            LL_DMA_StructInit(&init);
+            init.Direction = LL_DMA_DIRECTION_PERIPH_TO_MEMORY;
+
+            init.Mode = LL_DMA_MODE_CIRCULAR;
+            init.NbData = itemcount;
+
+            init.PeriphOrM2MSrcAddress = (uint32_t) &priv->ADCx->DR;
+            init.PeriphOrM2MSrcDataSize = LL_DMA_PDATAALIGN_HALFWORD;
+            init.PeriphOrM2MSrcIncMode = LL_DMA_PERIPH_NOINCREMENT;
+
+            init.MemoryOrM2MDstAddress = (uint32_t) priv->dma_buffer;
+            init.MemoryOrM2MDstDataSize = LL_DMA_MDATAALIGN_HALFWORD;
+            init.MemoryOrM2MDstIncMode = LL_DMA_MEMORY_INCREMENT;
+
+            assert_param(SUCCESS == LL_DMA_Init(priv->DMAx, priv->dma_chnum, &init));
+        }
+
+//        LL_DMA_EnableChannel(priv->DMAx, priv->dma_chnum);
+    }
+}
+
 /** Finalize unit set-up */
 error_t UADC_init(Unit *unit)
 {
@@ -118,6 +153,12 @@ error_t UADC_init(Unit *unit)
         }
     }
 
+    // ---------------- Alloc the buffer ----------------------
+    adc_dbg("Allocating buffer of size %d half-words", (int)priv->cfg.buffer_size);
+    priv->dma_buffer = calloc_ck(priv->cfg.buffer_size, sizeof(uint16_t));
+    if (NULL == priv->dma_buffer) return E_OUT_OF_MEM;
+    assert_param(((uint32_t) priv->dma_buffer & 3) == 0); // must be aligned
+
     // ------------------- ENABLE CLOCKS --------------------------
     {
         // enable peripherals clock
@@ -166,42 +207,7 @@ error_t UADC_init(Unit *unit)
     }
 
     // --------------------- CONFIGURE DMA -------------------------------
-    adc_dbg("Setting up DMA");
+    UADC_SetupDMA(unit);
-    {
-        uint32_t itemcount = priv->nb_channels * (priv->cfg.buffer_size / (priv->nb_channels));
-        if (itemcount % 2 == 1) itemcount -= priv->nb_channels; // ensure the count is even
-        priv->buf_itemcount = itemcount;
-
-        adc_dbg("DMA item count is %d (%d bytes), There are %d samples per group.",
-            priv->buf_itemcount,
-            priv->buf_itemcount * sizeof(uint16_t),
-            priv->nb_channels);
-
-        priv->dma_buffer = calloc_ck(priv->buf_itemcount, sizeof(uint16_t));
-        if (NULL == priv->dma_buffer) return E_OUT_OF_MEM;
-        assert_param(((uint32_t) priv->dma_buffer & 3) == 0); // must be aligned
-
-        {
-            LL_DMA_InitTypeDef init;
-            LL_DMA_StructInit(&init);
-            init.Direction = LL_DMA_DIRECTION_PERIPH_TO_MEMORY;
-
-            init.Mode = LL_DMA_MODE_CIRCULAR;
-            init.NbData = itemcount;
-
-            init.PeriphOrM2MSrcAddress = (uint32_t) &priv->ADCx->DR;
-            init.PeriphOrM2MSrcDataSize = LL_DMA_PDATAALIGN_HALFWORD;
-            init.PeriphOrM2MSrcIncMode = LL_DMA_PERIPH_NOINCREMENT;
-
-            init.MemoryOrM2MDstAddress = (uint32_t) priv->dma_buffer;
-            init.MemoryOrM2MDstDataSize = LL_DMA_MDATAALIGN_HALFWORD;
-            init.MemoryOrM2MDstIncMode = LL_DMA_MEMORY_INCREMENT;
-
-            assert_param(SUCCESS == LL_DMA_Init(priv->DMAx, priv->dma_chnum, &init));
-        }
-
-        LL_DMA_EnableChannel(priv->DMAx, priv->dma_chnum);
-    }
 
     // prepare the avg factor float for the ISR
     if (priv->cfg.averaging_factor > 1000) priv->cfg.averaging_factor = 1000; // normalize
@@ -238,10 +244,11 @@ void UADC_deInit(Unit *unit)
         irqd_detach(priv->ADCx, UADC_ADC_EOS_Handler);
 
         LL_DMA_DeInit(priv->DMAx, priv->dma_chnum);
-
-        free_ck(priv->dma_buffer);
     }
 
+    // free buffer if not NULL
+    free_ck(priv->dma_buffer);
+
     // Release all resources, deinit pins
     rsc_teardown(unit);
 

units/adc/_adc_internal.h

@@ -11,7 +11,7 @@
 
 #include "unit_base.h"
 
-//#define adc_dbg(...) dbg(##__VA_ARGS__)
+//#define adc_dbg dbg
 #define adc_dbg(...) do {} while(0)
 
 #define UADC_MAX_FREQ_FOR_AVERAGING 20000
@@ -115,6 +115,9 @@ error_t UADC_init(Unit *unit);
 /** Tear down the unit */
 void UADC_deInit(Unit *unit);
 
+/** Configure DMA (buffer count etc) */
+void UADC_SetupDMA(Unit *unit);
+
 // ------------------------------------------------------------------------
 
 /** DMA half/complete handler */
@@ -147,4 +150,7 @@ void UADC_StopStream(Unit *unit);
 /** Configure frequency */
 error_t UADC_SetSampleRate(Unit *unit, uint32_t hertz);
 
+/** Abort capture */
+void UADC_AbortCapture(Unit *unit);
+
 #endif //GEX_F072_ADC_INTERNAL_H

units/adc/unit_adc.c

@@ -27,6 +27,8 @@ enum AdcCmd_ {
     CMD_STREAM_STOP = 27,
     CMD_SET_SMOOTHING_FACTOR = 28,
     CMD_SET_SAMPLE_RATE = 29,
+    CMD_ENABLE_CHANNELS = 30,
+    CMD_SET_SAMPLE_TIME = 31,
 };
 
 /** Handle a request message */
@@ -35,8 +37,6 @@ static error_t UADC_handleRequest(Unit *unit, TF_ID frame_id, uint8_t command, P
     struct priv *priv = unit->data;
     PayloadBuilder pb = pb_start(unit_tmp512, UNIT_TMP_LEN, NULL);
 
-    // TODO toggling individual channels - would require DMA re-init and various changes in the usage of the struct
-
     switch (command) {
         /**
          * Get enabled channels.
@@ -81,6 +81,69 @@ static error_t UADC_handleRequest(Unit *unit, TF_ID frame_id, uint8_t command, P
             }
             return E_SUCCESS;
 
+        /**
+         * Set sample time
+         * pld: u8:0-7
+         */
+        case CMD_SET_SAMPLE_TIME:
+            {
+                uint8_t tim = pp_u8(pp);
+                if (tim > 7) return E_BAD_VALUE;
+                UADC_SwitchMode(unit, ADC_OPMODE_UNINIT);
+                {
+                    LL_ADC_SetSamplingTimeCommonChannels(priv->ADCx, LL_ADC_SAMPLETIMES[tim]);
+                }
+                UADC_SwitchMode(unit, ADC_OPMODE_IDLE);
+            }
+            return E_SUCCESS;
+
+        /**
+         * Enable channels. The channels must've been configured in the settings (except ch 16 and 17 which are available always)
+         * pld: u32: bitmap of channels
+         */
+        case CMD_ENABLE_CHANNELS:
+            {
+                uint32_t new_channels = pp_u32(pp);
+
+                // this tears down the peripherals sufficiently so we can re-configure them. Going back to IDLE re-inits this
+                UADC_SwitchMode(unit, ADC_OPMODE_UNINIT);
+
+                uint32_t illegal_channels = new_channels & ~(priv->cfg.channels | (1<<16) | (1<<17)); // 16 and 17 may be enabled always
+                if (illegal_channels != 0) {
+                    com_respond_str(MSG_ERROR, frame_id, "Some requested channels not available");
+                    UADC_SwitchMode(unit, ADC_OPMODE_IDLE);
+                    return E_FAILURE;
+                }
+
+                uint8_t nb_channels = 0;
+                // count the enabled channels
+                for(int i = 0; i < 32; i++) {
+                    if (new_channels & (1<<i)) {
+                        nb_channels++;
+                    }
+                }
+
+                if (nb_channels == 0) {
+                    com_respond_str(MSG_ERROR, frame_id, "Need at least 1 channel");
+                    UADC_SwitchMode(unit, ADC_OPMODE_IDLE);
+                    return E_FAILURE;
+                }
+
+                if (priv->cfg.buffer_size < nb_channels * 2) {
+                    com_respond_str(MSG_ERROR, frame_id, "Insufficient buf size");
+                    UADC_SwitchMode(unit, ADC_OPMODE_IDLE);
+                    return E_BAD_CONFIG;
+                }
+
+                priv->nb_channels = nb_channels;
+                priv->ADCx->CHSELR = new_channels; // apply it to the ADC
+                priv->channels_mask = new_channels;
+
+                UADC_SetupDMA(unit);
+                UADC_SwitchMode(unit, ADC_OPMODE_IDLE);
+            }
+            return E_SUCCESS;
+
         /**
          * Read raw values from the last measurement.
          * Response: interleaved (u8:channel, u16:value) for all channels
@@ -246,7 +309,7 @@ static error_t UADC_handleRequest(Unit *unit, TF_ID frame_id, uint8_t command, P
          */
         case CMD_ABORT:;
             adc_dbg("> Abort capture");
-            TRY(UU_ADC_AbortCapture(unit));
+            UADC_AbortCapture(unit);
             return E_SUCCESS;
 
         /**

units/adc/unit_adc.h

@@ -11,6 +11,4 @@
 
 extern const UnitDriver UNIT_ADC;
 
-error_t UU_ADC_AbortCapture(Unit *unit);
-
 #endif //U_TPL_H