adding comments to adc

units/adc/_adc_core.c

@@ -1,6 +1,8 @@
 //
 // Created by MightyPork on 2018/02/04.
 //
+// The core functionality of the ADC unit is defined here.
+//
 
 #include "platform.h"
 #include "unit_base.h"
@@ -11,6 +13,16 @@
 
 #define DMA_POS(priv) ((priv)->buf_itemcount - (priv)->DMA_CHx->CNDTR)
 
+/**
+ * Async job to send a chunk of the DMA buffer to PC.
+ * This can't be done directly because the interrupt couldn't wait for the TinyFrame mutex.
+ *
+ * unit  - unit
+ * data1 - start index
+ * data2 - number of samples to send
+ * data3 - bit flags: 0x80 if this is the last sample and we should close
+ *                    0x01 if this was the TC interrupt (otherwise it's HT)
+ */
 static void UADC_JobSendBlockChunk(Job *job)
 {
     Unit *unit = job->unit;
@@ -21,9 +33,7 @@ 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);
+    const TF_TYPE type = close ? EVT_CAPT_DONE : EVT_CAPT_MORE;
-
-    TF_TYPE type = close ? EVT_CAPT_DONE : EVT_CAPT_MORE;
 
     TF_Msg msg = {
         .frame_id = priv->stream_frame_id,
@@ -35,12 +45,22 @@ static void UADC_JobSendBlockChunk(Job *job)
     TF_Multipart_Payload(comm, (uint8_t *) (priv->dma_buffer + start), count * sizeof(uint16_t));
     TF_Multipart_Close(comm);
 
+    // Clear the "busy" flags - those are checked in the DMA ISR to detect overrun
     if (tc) priv->tc_pending = false;
     else priv->ht_pending = false;
 
     priv->stream_serial++;
 }
 
+/**
+ * Async job to send the trigger header.
+ * The header includes info about the trigger + the pre-trigger buffer.
+ *
+ * data1 - index in the DMA buffer at which the captured data willl start
+ * data2 - edge type - 1 rise, 2 fall, 3 forced
+ * timestamp - event stamp
+ * unit - unit
+ */
 static void UADC_JobSendTriggerCaptureHeader(Job *job)
 {
     Unit *unit = job->unit;
@@ -50,7 +70,12 @@ static void UADC_JobSendTriggerCaptureHeader(Job *job)
         .unit = unit,
         .type = EVT_CAPT_START,
         .timestamp = job->timestamp,
-        .length = (priv->pretrig_len+1) * priv->nb_channels * sizeof(uint16_t) + 4 /*pretrig len*/ + 1 /*edge*/ + 1 /* seq */
+        .length = (priv->pretrig_len + 1) * // see below why +1
+                      priv->nb_channels *
+                      sizeof(uint16_t) +
+                  4 /*pretrig len*/ +
+                  1 /*edge*/ +
+                  1 /* seq */
     };
 
     uint32_t index_trigd = job->data1;
@@ -70,7 +95,9 @@ static void UADC_JobSendTriggerCaptureHeader(Job *job)
 
         if (priv->pretrig_len > 0) {
             // pretrig
-            uint32_t pretrig_remain = (priv->pretrig_len + 1) * priv->nb_channels; // +1 because we want pretrig 0 to exactly start with the triggering sample
+
+            // +1 because we want pretrig 0 to exactly start with the triggering sample
+            uint32_t pretrig_remain = (priv->pretrig_len + 1) * priv->nb_channels;
 
             assert_param(index_trigd <= priv->buf_itemcount);
 
@@ -95,6 +122,9 @@ static void UADC_JobSendTriggerCaptureHeader(Job *job)
     EventReport_End();
 }
 
+/**
+ * Async job to notify about end of stream
+ */
 static void UADC_JobSendEndOfStreamMsg(Job *job)
 {
     TF_Msg msg = {
@@ -104,6 +134,10 @@ static void UADC_JobSendEndOfStreamMsg(Job *job)
     TF_Respond(comm, &msg);
 }
 
+/**
+ * Schedule sending a event report to the PC that the current stream has ended.
+ * The client library should handle this appropriately.
+ */
 void UADC_ReportEndOfStream(Unit *unit)
 {
     struct priv *priv = unit->data;
@@ -116,6 +150,15 @@ void UADC_ReportEndOfStream(Unit *unit)
     scheduleJob(&j);
 }
 
+/**
+ * This is a helper function for the ADC DMA interrupt for handing the different interrupt types (half / full transfer).
+ * It sends the part of the buffer that was just captured via an async job, or aborts on overrun.
+ *
+ * It's split off here to allow calling it for the different flags without repeating code.
+ *
+ * @param unit
+ * @param tc - true if this is the TC interrupt, else HT
+ */
 static void handle_httc(Unit *unit, bool tc)
 {
     struct priv *priv = unit->data;
@@ -134,8 +177,9 @@ static void handle_httc(Unit *unit, bool tc)
         end = priv->buf_itemcount;
     }
 
-    if (start != end) {
+    if (start != end) { // this sometimes happened after a trigger, may be unnecessary now
         if (end < start) {
+            // this was a trap for a bug with missed TC irq, it's hopefully fixed now
             trap("end < start! %d < %d, tc %d", (int)end, (int)start, (int)tc);
         }
 
@@ -146,7 +190,8 @@ static void handle_httc(Unit *unit, bool tc)
             priv->trig_stream_remain -= sgcount;
         }
 
-        bool close = !m_stream && priv->trig_stream_remain == 0;
+        // Check for the closing condition
+        const bool close = !m_stream && priv->trig_stream_remain == 0;
 
         if ((tc && priv->tc_pending) || (ht && priv->ht_pending)) {
             dbg("(!) ADC DMA not handled in time, abort capture");
@@ -176,13 +221,15 @@ static void handle_httc(Unit *unit, bool tc)
         }
 
         if (close) {
-            // If auto-arm enabled, we need to re-arm again.
+            // If auto-arm is enabled, we need to re-arm again.
-            // However, EOS irq is disabled during the capture.
+            // However, EOS irq is disabled during the capture so the trigger edge detection would
-            // We have to wait for the next EOS interrupt to occur.
+            // work on stale data from before this trigger. We have to wait for the next full
+            // conversion (EOS) before arming.
             UADC_SwitchMode(unit, (priv->auto_rearm && m_trigd) ? ADC_OPMODE_REARM_PENDING : ADC_OPMODE_IDLE);
         }
     }
 
+    // Advance the starting position
     if (tc) {
         priv->stream_startpos = 0;
     }
@@ -191,21 +238,38 @@ static void handle_httc(Unit *unit, bool tc)
     }
 }
 
+/**
+ * IRQ handler for the DMA flags.
+ *
+ * We handle flags:
+ * TC - transfer complete
+ * HT - half transfer
+ * TE - transfer error (this should never happen unless there's a bug)
+ *
+ * The buffer works in a circular mode, so we always handle the previous half
+ * or what of it should be sent (if capture started somewhere inside).
+ *
+ * @param arg - the unit, passed via the irq dispatcher
+ */
 void UADC_DMA_Handler(void *arg)
 {
     Unit *unit = arg;
     struct priv *priv = unit->data;
 
+    // First thing, grab the flags. They may change during the function.
+    // Working on the live register might cause race conditions.
+    const uint32_t isrsnapshot = priv->DMAx->ISR;
+
     if (priv->opmode == ADC_OPMODE_UNINIT) {
+        // the IRQ occured while switching mode, clear flags and do nothing else
         LL_DMA_ClearFlag_HT(priv->DMAx, priv->dma_chnum);
         LL_DMA_ClearFlag_TC(priv->DMAx, priv->dma_chnum);
         LL_DMA_ClearFlag_TE(priv->DMAx, priv->dma_chnum);
         return;
     }
 
-    const uint32_t isrsnapshot = priv->DMAx->ISR;
-
     if (LL_DMA_IsActiveFlag_G(isrsnapshot, priv->dma_chnum)) {
+        // we have some flags set - check which
         const bool tc = LL_DMA_IsActiveFlag_TC(isrsnapshot, priv->dma_chnum);
         const bool ht = LL_DMA_IsActiveFlag_HT(isrsnapshot, priv->dma_chnum);
         const bool te = LL_DMA_IsActiveFlag_TE(isrsnapshot, priv->dma_chnum);
@@ -213,43 +277,52 @@ void UADC_DMA_Handler(void *arg)
         if (ht) LL_DMA_ClearFlag_HT(priv->DMAx, priv->dma_chnum);
         if (tc) LL_DMA_ClearFlag_TC(priv->DMAx, priv->dma_chnum);
 
+        if (te) {
+            // this shouldn't happen - error
+            adc_dbg("ADC DMA TE!");
+            LL_DMA_ClearFlag_TE(priv->DMAx, priv->dma_chnum);
+            return;
+        }
+
         // check what mode we're in
         const bool m_trigd = priv->opmode == ADC_OPMODE_TRIGD;
         const bool m_stream = priv->opmode == ADC_OPMODE_STREAM;
         const bool m_fixcpt = priv->opmode == ADC_OPMODE_BLCAP;
-
         if (m_trigd || m_stream || m_fixcpt) {
-            if (ht || tc) {
+            const uint32_t half = (uint32_t) (priv->buf_itemcount / 2);
-                const uint32_t half = (uint32_t) (priv->buf_itemcount / 2);
+            if (ht && tc) {
-                if (ht && tc) {
+                // dual event interrupt - may happen if we missed both and they were pending after
-                    if (priv->stream_startpos > half) {
+                // interrupts became enabled again (this can happen due to the EOS or other higher prio irq's)
-                        handle_httc(unit, true); // TC
+
-                        handle_httc(unit, false); // HT
+                if (priv->stream_startpos > half) {
-                    } else {
+                    handle_httc(unit, true); // TC
-                        handle_httc(unit, false); // HT
+                    handle_httc(unit, false); // HT
-                        handle_httc(unit, true); // TC
-                    }
                 } else {
-                    if (ht && priv->stream_startpos > half) {
+                    handle_httc(unit, false); // HT
-                        // We missed the TC interrupt while e.g. setting up the stream / interrupt. catch up!
+                    handle_httc(unit, true); // TC
-                        handle_httc(unit, true); // TC
+                }
-                    }
+            } else {
-                    handle_httc(unit, tc);
+                if (ht && priv->stream_startpos > half) {
+                    // We missed the TC interrupt while e.g. setting up the stream / interrupt. catch up!
+                    // This fixes a bug with "negative size" for report.
+                    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 (te) {
-            // this shouldn't happen - error
-            adc_dbg("ADC DMA TE!");
-            LL_DMA_ClearFlag_TE(priv->DMAx, priv->dma_chnum);
-        }
     }
 }
 
+/**
+ * End of measurement group interrupt handler.
+ * This interrupt records the measured values and checks for trigger.
+ *
+ * @param arg - unit, passed b y irq dispatcher
+ */
 void UADC_ADC_EOS_Handler(void *arg)
 {
     Unit *unit = arg;
@@ -299,11 +372,11 @@ void UADC_ADC_EOS_Handler(void *arg)
 
         if ((priv->trig_prev_level < priv->trig_level) && val >= priv->trig_level && (bool) (priv->trig_edge & 0b01)) {
             // Rising edge
-            UADC_HandleTrigger(unit, 1, timestamp);
+            UADC_HandleTrigger(unit, 0b01, timestamp);
         }
         else if ((priv->trig_prev_level > priv->trig_level) && val <= priv->trig_level && (bool) (priv->trig_edge & 0b10)) {
             // Falling edge
-            UADC_HandleTrigger(unit, 2, timestamp);
+            UADC_HandleTrigger(unit, 0b10, timestamp);
         }
         priv->trig_prev_level = val;
     }
@@ -321,6 +394,13 @@ void UADC_ADC_EOS_Handler(void *arg)
     }
 }
 
+/**
+ * Handle a detected trigger - start capture if we're not in hold-off
+ *
+ * @param unit
+ * @param edge_type - edge type, is included in the report
+ * @param timestamp - event time
+ */
 void UADC_HandleTrigger(Unit *unit, uint8_t edge_type, uint64_t timestamp)
 {
     struct priv *priv = unit->data;
@@ -342,6 +422,7 @@ void UADC_HandleTrigger(Unit *unit, uint8_t edge_type, uint64_t timestamp)
     priv->trig_stream_remain = priv->trig_len;
     priv->stream_serial = 0;
 
+    // This func may be called from the EOS interrupt, so it's safer to send the header message asynchronously
     Job j = {
         .unit = unit,
         .timestamp = timestamp,
@@ -354,6 +435,9 @@ void UADC_HandleTrigger(Unit *unit, uint8_t edge_type, uint64_t timestamp)
     UADC_SwitchMode(unit, ADC_OPMODE_TRIGD);
 }
 
+/**
+ * Abort ongoing capture.
+ */
 void UADC_AbortCapture(Unit *unit)
 {
     struct priv *priv = unit->data;
@@ -371,6 +455,13 @@ void UADC_AbortCapture(Unit *unit)
     UADC_SwitchMode(unit, ADC_OPMODE_IDLE);
 }
 
+/**
+ * Start a manual block capture.
+ *
+ * @param unit
+ * @param len - number of samples (groups)
+ * @param frame_id - TF session to re-use for the report (client has a listener set up)
+ */
 void UADC_StartBlockCapture(Unit *unit, uint32_t len, TF_ID frame_id)
 {
     struct priv *priv = unit->data;
@@ -383,7 +474,11 @@ void UADC_StartBlockCapture(Unit *unit, uint32_t len, TF_ID frame_id)
     UADC_SwitchMode(unit, ADC_OPMODE_BLCAP);
 }
 
-/** Start stream */
+/**
+ * Start a stream
+ *
+ * @param frame_id - TF session to re-use for the frames (client has a listener set up)
+ */
 void UADC_StartStream(Unit *unit, TF_ID frame_id)
 {
     struct priv *priv = unit->data;
@@ -393,7 +488,9 @@ void UADC_StartStream(Unit *unit, TF_ID frame_id)
     UADC_SwitchMode(unit, ADC_OPMODE_STREAM);
 }
 
-/** End stream */
+/**
+ * End a stream by user request.
+ */
 void UADC_StopStream(Unit *unit)
 {
     struct priv *priv = unit->data;
@@ -403,7 +500,10 @@ void UADC_StopStream(Unit *unit)
     UADC_SwitchMode(unit, ADC_OPMODE_IDLE);
 }
 
-/** Handle unit update tick - expire the trigger hold-off */
+/**
+ * Handle unit update tick - expire the trigger hold-off.
+ * We also check for the emergency shutdown condition and clear it.
+ */
 void UADC_updateTick(Unit *unit)
 {
     struct priv *priv = unit->data;
@@ -429,12 +529,17 @@ void UADC_updateTick(Unit *unit)
     }
 }
 
+/**
+ * Switch the ADC operational mode.
+ *
+ * @param unit
+ * @param new_mode - mode to set
+ */
 void UADC_SwitchMode(Unit *unit, enum uadc_opmode new_mode)
 {
     struct priv *priv = unit->data;
 
     const enum uadc_opmode old_mode = priv->opmode;
-
     if (new_mode == old_mode) return; // nothing to do
 
     // if un-itied, can go only to IDLE
@@ -520,10 +625,9 @@ void UADC_SwitchMode(Unit *unit, enum uadc_opmode new_mode)
         // avoid firing immediately by the value jumping across the scale
         priv->trig_prev_level = priv->last_samples[priv->trigger_source];
     }
-    else if (new_mode == ADC_OPMODE_TRIGD ||
+    else if (new_mode == ADC_OPMODE_TRIGD || new_mode == ADC_OPMODE_STREAM || new_mode == ADC_OPMODE_BLCAP) {
-        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);
 
         // during the capture, we disallow direct readout and averaging to reduce overhead

units/adc/_adc_init.c

@@ -1,6 +1,8 @@
 //
 // Created by MightyPork on 2018/02/03.
 //
+// ADC unit init and de-init functions
+//
 
 #include "platform.h"
 #include "unit_base.h"
@@ -15,10 +17,10 @@ error_t UADC_preInit(Unit *unit)
     if (priv == NULL) return E_OUT_OF_MEM;
 
     priv->cfg.channels = 1<<16; // Tsense by default - always available, easy testing
-    priv->cfg.sample_time = 0b010; // 13.5c
+    priv->cfg.sample_time = 0b010; // 13.5c - good enough and the default 0b00 value really is useless
     priv->cfg.frequency = 1000;
-    priv->cfg.buffer_size = 256;
+    priv->cfg.buffer_size = 256; // in half-words
-    priv->cfg.averaging_factor = 500;
+    priv->cfg.averaging_factor = 500; // 0.5
 
     priv->opmode = ADC_OPMODE_UNINIT;
 
@@ -49,6 +51,13 @@ error_t UADC_SetSampleRate(Unit *unit, uint32_t hertz)
     return E_SUCCESS;
 }
 
+/**
+ * Set up the ADC DMA.
+ * This is split to its own function because it's also called when the user adjusts the
+ * enabled channels and we need to re-configure it.
+ *
+ * @param unit
+ */
 void UADC_SetupDMA(Unit *unit)
 {
     struct priv *priv = unit->data;
@@ -83,7 +92,7 @@ void UADC_SetupDMA(Unit *unit)
             assert_param(SUCCESS == LL_DMA_Init(priv->DMAx, priv->dma_chnum, &init));
         }
 
-//        LL_DMA_EnableChannel(priv->DMAx, priv->dma_chnum);
+//        LL_DMA_EnableChannel(priv->DMAx, priv->dma_chnum); // this is done in the switch mode func now
     }
 }
 
@@ -225,8 +234,6 @@ error_t UADC_init(Unit *unit)
     return E_SUCCESS;
 }
 
-
-
 /** Tear down the unit */
 void UADC_deInit(Unit *unit)
 {

units/adc/_adc_internal.h

@@ -1,6 +1,8 @@
 //
 // Created by MightyPork on 2018/02/03.
 //
+// Defines and prototypes used internally by the ADC unit.
+//
 
 #ifndef GEX_F072_ADC_INTERNAL_H
 #define GEX_F072_ADC_INTERNAL_H

units/adc/_adc_settings.c

@@ -1,6 +1,8 @@
 //
 // Created by MightyPork on 2018/02/03.
 //
+// ADC unit settings reading / parsing
+//
 
 #include "platform.h"
 #include "unit_base.h"

units/adc/unit_adc.c

@@ -51,6 +51,10 @@ static error_t UADC_handleRequest(Unit *unit, TF_ID frame_id, uint8_t command, P
             com_respond_pb(frame_id, MSG_SUCCESS, &pb);
             return E_SUCCESS;
 
+        /**
+         * Set the sample rate in Hz
+         * plad: hz:u32
+         */
         case CMD_SET_SAMPLE_RATE:
             {
                 uint32_t freq = pp_u32(pp);

units/adc/unit_adc.h

@@ -1,7 +1,8 @@
 //
 // Created by MightyPork on 2017/11/25.
 //
-// Digital input unit; single or multiple pin read access on one port (A-F)
+// ADC unit with several DSO-like features, like triggering, pre-trigger, block capture,
+// streaming, smoothing...
 //
 
 #ifndef U_TPL_H