From 7e790c6764941cd34b22c9ffb10bf9274abf97f0 Mon Sep 17 00:00:00 2001 From: iabdalkader Date: Tue, 4 Apr 2023 10:19:52 +0200 Subject: [PATCH 1/8] src/DMABuffer.h: Fix DMA buffer Queue memory issues. * The current underlying DMA buffer Queue is not thread-safe and in some cases it can corrupt the memory. Replace the underlying DMA buffer container with a thread-safe implementation. --- src/DMABuffer.h | 61 +++++++++++++++++++++++++++++++--------------- src/Queue.h | 65 +++++++++++++++++++++++++++++-------------------- 2 files changed, 81 insertions(+), 45 deletions(-) diff --git a/src/DMABuffer.h b/src/DMABuffer.h index 0f495df..50289fe 100644 --- a/src/DMABuffer.h +++ b/src/DMABuffer.h @@ -73,10 +73,8 @@ template class DMABuffer { uint32_t flags; public: - DMABuffer *next; - DMABuffer(Pool *pool=nullptr, size_t samples=0, size_t channels=0, T *mem=nullptr): - pool(pool), n_samples(samples), n_channels(channels), ptr(mem), ts(0), flags(0), next(nullptr) { + pool(pool), n_samples(samples), n_channels(channels), ptr(mem), ts(0), flags(0) { } T *data() { @@ -158,36 +156,61 @@ template class DMABufferPool { private: Queue*> wr_queue; Queue*> rd_queue; - std::unique_ptr[]> buffers; std::unique_ptr::free)> pool; public: DMABufferPool(size_t n_samples, size_t n_channels, size_t n_buffers): - buffers(nullptr), pool(nullptr, AlignedAlloc::free) { + wr_queue(n_buffers), rd_queue(n_buffers), pool(nullptr, AlignedAlloc::free) { // Round up to next multiple of alignment. size_t bufsize = AlignedAlloc::round(n_samples * n_channels * sizeof(T)); - if (bufsize) { - // Allocate non-aligned memory for the DMA buffers objects. - buffers.reset(new DMABuffer[n_buffers]); - - // Allocate aligned memory pool for DMA buffers pointers. + if (bufsize && rd_queue && wr_queue) { + // Allocate an aligned memory pool for DMA buffers. pool.reset((uint8_t *) AlignedAlloc::malloc(n_buffers * bufsize)); - } - if (buffers && pool) { - // Init DMA buffers using aligned pointers to dma buffers memory. + if (!pool) { + // Failed to allocate memory pool. + return; + } + // Allocate the DMA buffers, initialize them using aligned + // pointers from the pool, and add them to the ready queue. for (size_t i=0; i(this, n_samples, n_channels, (T *) &pool.get()[i * bufsize]); - wr_queue.push(&buffers[i]); + DMABuffer *buf = new DMABuffer( + this, n_samples, n_channels, (T *) &pool.get()[i * bufsize] + ); + if (buf == nullptr) { + break; + } + wr_queue.push(buf); } } } - size_t writable() { - return wr_queue.size(); + ~DMABufferPool() { + size_t count = 0; + DMABuffer *buf = nullptr; + + while (readable()) { + delete dequeue(); + count ++; + } + + while (writable()) { + delete allocate(); + count ++; + } + } + + bool writable() { + return !(wr_queue.empty()); } - size_t readable() { - return rd_queue.size(); + bool readable() { + return !(rd_queue.empty()); + } + + void flush() { + while (readable()) { + release(dequeue()); + } } DMABuffer *allocate() { diff --git a/src/Queue.h b/src/Queue.h index de2dc1b..0b7e9f0 100644 --- a/src/Queue.h +++ b/src/Queue.h @@ -24,45 +24,58 @@ template class Queue { private: - T head; - T tail; - size_t _size; + size_t capacity; + volatile size_t tail; + volatile size_t head; + std::unique_ptr buff; + + private: + inline size_t next_pos(size_t x) { + return (((x) + 1) % (capacity)); + } public: - Queue(): head(nullptr), tail(nullptr), _size(0) { + Queue(size_t size=0): + capacity(size), tail(0), head(0), buff(nullptr) { + if (size) { + tail = head = 0; + capacity = size + 1; + buff.reset(new T[capacity]); + } + } + void reset() { + tail = head = 0; } - size_t size() { - return _size; + size_t empty() { + return tail == head; } - bool empty() { - return !_size; + operator bool() const { + return buff.get() != nullptr; } - void push(T value) { - _size++; - value->next = nullptr; - if (head == nullptr) { - head = value; - } - if (tail) { - tail->next = value; + bool push(T data) { + bool ret = false; + size_t next = next_pos(head); + if (buff && (next != tail)) { + buff[head] = data; + head = next; + ret = true; } - tail = value; + return ret; } - T pop() { - _size--; - T value = head; - if (head) { - head = head->next; - } - if (head == nullptr) { - tail = nullptr; + T pop(bool peek=false) { + if (buff && (tail != head)) { + T data = buff[tail]; + if (!peek) { + tail = next_pos(tail); + } + return data; } - return value; + return T(); } }; #endif //__QUEUE_H__ From 17811ad3f054bb21d18c7ebf66896321776fd6f4 Mon Sep 17 00:00:00 2001 From: iabdalkader Date: Tue, 4 Apr 2023 10:24:24 +0200 Subject: [PATCH 2/8] src/AdvancedDAC.cpp: Misc improvements to make DAC more robust. * Detect DAC underrun in available() and reset if needed. If DAC stops for any reason, no buffers will become free again. We need to detect this and abort. * Flush all pending buffers when DAC is stopped. This releases back all the buffers to the ready queue, and fixes an edge case where DAC stops while holding all of the ready buffers. * Check for NULL descriptor pointers in every function. * Increase DMA stream priority to highest. --- src/AdvancedDAC.cpp | 44 ++++++++++++++++++++++++++++++++------------ src/HALConfig.cpp | 2 +- 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/AdvancedDAC.cpp b/src/AdvancedDAC.cpp index 644f500..54684dd 100644 --- a/src/AdvancedDAC.cpp +++ b/src/AdvancedDAC.cpp @@ -29,6 +29,7 @@ struct dac_descr_t { TIM_HandleTypeDef tim; uint32_t tim_trig; uint32_t resolution; + uint32_t dmaudr_flag; DMABufferPool *pool; DMABuffer *dmabuf[2]; }; @@ -37,10 +38,10 @@ struct dac_descr_t { static DAC_HandleTypeDef dac = {0}; static dac_descr_t dac_descr_all[] = { - {&dac, DAC_CHANNEL_1, {DMA1_Stream4, {DMA_REQUEST_DAC1_CH1}}, DMA1_Stream4_IRQn, - {TIM4}, DAC_TRIGGER_T4_TRGO, DAC_ALIGN_12B_R, nullptr, {nullptr, nullptr}}, - {&dac, DAC_CHANNEL_2, {DMA1_Stream5, {DMA_REQUEST_DAC1_CH2}}, DMA1_Stream5_IRQn, - {TIM5}, DAC_TRIGGER_T5_TRGO, DAC_ALIGN_12B_R, nullptr, {nullptr, nullptr}}, + {&dac, DAC_CHANNEL_1, {DMA1_Stream4, {DMA_REQUEST_DAC1_CH1}}, DMA1_Stream4_IRQn, {TIM4}, + DAC_TRIGGER_T4_TRGO, DAC_ALIGN_12B_R, DAC_FLAG_DMAUDR1, nullptr, {nullptr, nullptr}}, + {&dac, DAC_CHANNEL_2, {DMA1_Stream5, {DMA_REQUEST_DAC1_CH2}}, DMA1_Stream5_IRQn, {TIM5}, + DAC_TRIGGER_T5_TRGO, DAC_ALIGN_12B_R, DAC_FLAG_DMAUDR2, nullptr, {nullptr, nullptr}}, }; static uint32_t DAC_RES_LUT[] = { @@ -73,10 +74,12 @@ static dac_descr_t *dac_descr_get(uint32_t channel) { } static void dac_descr_deinit(dac_descr_t *descr, bool dealloc_pool) { - if (descr) { + if (descr != nullptr) { HAL_TIM_Base_Stop(&descr->tim); HAL_DAC_Stop_DMA(descr->dac, descr->channel); + __HAL_DAC_CLEAR_FLAG(descr->dac, descr->dmaudr_flag); + for (size_t i=0; idmabuf); i++) { if (descr->dmabuf[i]) { descr->dmabuf[i]->release(); @@ -89,13 +92,17 @@ static void dac_descr_deinit(dac_descr_t *descr, bool dealloc_pool) { delete descr->pool; } descr->pool = nullptr; + } else { + descr->pool->flush(); } - } } bool AdvancedDAC::available() { if (descr != nullptr) { + if (__HAL_DAC_GET_FLAG(descr->dac, descr->dmaudr_flag)) { + dac_descr_deinit(descr, false); + } return descr->pool->writable(); } return false; @@ -113,11 +120,17 @@ DMABuffer &AdvancedDAC::dequeue() { } void AdvancedDAC::write(DMABuffer &dmabuf) { + static uint32_t buf_count = 0; + + if (descr == nullptr) { + return; + } + // Make sure any cached data is flushed. dmabuf.flush(); descr->pool->enqueue(&dmabuf); - if (descr->dmabuf[0] == nullptr && descr->pool->readable() > 2) { + if (descr->dmabuf[0] == nullptr && (++buf_count % 3) == 0) { descr->dmabuf[0] = descr->pool->dequeue(); descr->dmabuf[1] = descr->pool->dequeue(); @@ -126,7 +139,9 @@ void AdvancedDAC::write(DMABuffer &dmabuf) { (uint32_t *) descr->dmabuf[0]->data(), descr->dmabuf[0]->size(), descr->resolution); // Re/enable DMA double buffer mode. + HAL_NVIC_DisableIRQ(descr->dma_irqn); hal_dma_enable_dbm(&descr->dma, descr->dmabuf[0]->data(), descr->dmabuf[1]->data()); + HAL_NVIC_EnableIRQ(descr->dma_irqn); // Start trigger timer. HAL_TIM_Base_Start(&descr->tim); @@ -135,7 +150,7 @@ void AdvancedDAC::write(DMABuffer &dmabuf) { int AdvancedDAC::begin(uint32_t resolution, uint32_t frequency, size_t n_samples, size_t n_buffers) { // Sanity checks. - if (resolution >= AN_ARRAY_SIZE(DAC_RES_LUT) || (descr && descr->pool)) { + if (resolution >= AN_ARRAY_SIZE(DAC_RES_LUT) || descr != nullptr) { return 0; } @@ -147,13 +162,14 @@ int AdvancedDAC::begin(uint32_t resolution, uint32_t frequency, size_t n_samples uint32_t function = pinmap_function(dac_pins[0], PinMap_DAC); descr = dac_descr_get(DAC_CHAN_LUT[STM_PIN_CHANNEL(function) - 1]); - if (descr == nullptr || descr->pool) { + if (descr == nullptr) { return 0; } // Allocate DMA buffer pool. descr->pool = new DMABufferPool(n_samples, n_channels, n_buffers); if (descr->pool == nullptr) { + descr = nullptr; return 0; } descr->resolution = DAC_RES_LUT[resolution]; @@ -178,13 +194,16 @@ int AdvancedDAC::begin(uint32_t resolution, uint32_t frequency, size_t n_samples int AdvancedDAC::stop() { - dac_descr_deinit(descr, true); + if (descr != nullptr) { + dac_descr_deinit(descr, true); + descr = nullptr; + } return 1; } int AdvancedDAC::frequency(uint32_t const frequency) { - if (descr && descr->pool) { + if (descr != nullptr) { // Reconfigure the trigger timer. dac_descr_deinit(descr, false); hal_tim_config(&descr->tim, frequency); @@ -200,9 +219,10 @@ extern "C" { void DAC_DMAConvCplt(DMA_HandleTypeDef *dma, uint32_t channel) { dac_descr_t *descr = dac_descr_get(channel); + // Release the DMA buffer that was just done, allocate a new one, // and update the next DMA memory address target. - if (descr->pool->readable()) { + if (descr && descr->pool->readable()) { // NOTE: CT bit is inverted, to get the DMA buffer that's Not currently in use. size_t ct = ! hal_dma_get_ct(dma); descr->dmabuf[ct]->release(); diff --git a/src/HALConfig.cpp b/src/HALConfig.cpp index 7c2de12..225029a 100644 --- a/src/HALConfig.cpp +++ b/src/HALConfig.cpp @@ -74,7 +74,7 @@ int hal_dma_config(DMA_HandleTypeDef *dma, IRQn_Type irqn, uint32_t direction) { // DMA Init dma->Init.Mode = DMA_DOUBLE_BUFFER_M0; - dma->Init.Priority = DMA_PRIORITY_LOW; + dma->Init.Priority = DMA_PRIORITY_VERY_HIGH; dma->Init.Direction = direction; dma->Init.FIFOMode = DMA_FIFOMODE_ENABLE; dma->Init.FIFOThreshold = DMA_FIFO_THRESHOLD_FULL; From e21c57bb4bf698012532d93638ff6f1cab8007d0 Mon Sep 17 00:00:00 2001 From: iabdalkader Date: Tue, 4 Apr 2023 10:31:07 +0200 Subject: [PATCH 3/8] examples/Waveform_Generator.ino: Update example. * Add new command to stop DAC ("k"). * Print memory usage. * Increase max frequency to 128KHz. * Use a smaller DMA buffer size and rework the wave generators to support it. --- .../Waveform_Generator/Waveform_Generator.ino | 105 ++++++++++++------ 1 file changed, 69 insertions(+), 36 deletions(-) diff --git a/examples/Beginner/Waveform_Generator/Waveform_Generator.ino b/examples/Beginner/Waveform_Generator/Waveform_Generator.ino index 5ba5494..a5dbb21 100644 --- a/examples/Beginner/Waveform_Generator/Waveform_Generator.ino +++ b/examples/Beginner/Waveform_Generator/Waveform_Generator.ino @@ -1,22 +1,43 @@ // This example generates different waveforms based on user input on A12/DAC1. #include +#include -#define N_SAMPLES (256) -#define DEFAULT_FREQUENCY (16000) +#define N_SAMPLES (64) +#define DEFAULT_FREQUENCY (32000) AdvancedDAC dac1(A12); uint8_t SAMPLES_BUFFER[N_SAMPLES]; -size_t dac_frequency = DEFAULT_FREQUENCY; -void generate_waveform(int cmd) -{ +uint32_t get_current_heap() { + mbed_stats_heap_t heap_stats; + mbed_stats_heap_get(&heap_stats); + return heap_stats.current_size; +} + +void print_menu() { + Serial.println(); + Serial.println("Enter a command:"); + Serial.println("t: Triangle wave"); + Serial.println("q: Square wave"); + Serial.println("s: Sine wave"); + Serial.println("r: Sawtooth wave"); + Serial.println("k: stop DAC"); + Serial.println("+: Increase frequency"); + Serial.println("-: Decrease frequency"); +} + +void generate_waveform(int cmd) { + static bool dac_started = false; + static size_t dac_frequency = DEFAULT_FREQUENCY; + static size_t starting_heap = get_current_heap(); + switch (cmd) { case 't': // Triangle wave Serial.print("Waveform: Triangle "); - for (int i=0; i 1000) { - dac_frequency /= 2; + dac_frequency /= 2; } else { - break; + break; } // Change frequency. dac1.frequency(dac_frequency * N_SAMPLES); break; - + + case 'k': + dac1.stop(); + dac_started = false; + break; + default: Serial.print("Unknown command "); Serial.println((char) cmd); return; } - - Serial.print(dac_frequency/1000); - Serial.println("KHz"); + + if (cmd == 'k') { + Serial.println("DAC stopped!"); + print_menu(); + } else { + Serial.print(dac_frequency/1000); + Serial.println("KHz"); + + if (dac_started == false) { + // Initialize and start the DAC. + if (!dac1.begin(AN_RESOLUTION_8, dac_frequency * N_SAMPLES, N_SAMPLES, 32)) { + Serial.println("Failed to start DAC1 !"); + while (1); + } + dac_started = true; + } + } + + Serial.print("Used memory: "); + Serial.print(get_current_heap() - starting_heap); + Serial.println(" bytes"); } void setup() { @@ -77,32 +121,21 @@ void setup() { } - - Serial.println("Enter a command:"); - Serial.println("t: Triangle wave"); - Serial.println("q: Square wave"); - Serial.println("s: Sine wave"); - Serial.println("r: Sawtooth wave"); - Serial.println("+: Increase frequency"); - Serial.println("-: Decrease frequency"); - + // Print list of commands. + print_menu(); + + // Start generating a sine wave. generate_waveform('s'); - - // DAC initialization - if (!dac1.begin(AN_RESOLUTION_8, DEFAULT_FREQUENCY * N_SAMPLES, N_SAMPLES, 32)) { - Serial.println("Failed to start DAC1 !"); - while (1); - } } void loop() { if (Serial.available() > 0) { int cmd = Serial.read(); if (cmd != '\n') { - generate_waveform(cmd); + generate_waveform(cmd); } } - + if (dac1.available()) { // Get a free buffer for writing. SampleBuffer buf = dac1.dequeue(); From 53ffa5d85bee370d69591e558100f1918cf08c1d Mon Sep 17 00:00:00 2001 From: Thomas Friedrichsmeier Date: Wed, 5 Apr 2023 21:43:08 +0200 Subject: [PATCH 4/8] Allow setting list of pins to sample after construction Arguably, in user code, the list of pins to sample will typically be known and fixed at compile-time. However, when wrapping this into a library (Mozzi; for the purpose of providing a cross-platform analog read mechanism), it will be very helpful to have a way to adjust the pins to sample after construction. --- src/AdvancedADC.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/AdvancedADC.h b/src/AdvancedADC.h index 647250b..46321d1 100644 --- a/src/AdvancedADC.h +++ b/src/AdvancedADC.h @@ -42,10 +42,18 @@ class AdvancedADC { adc_pins[n_channels++] = analogPinToPinName(p); } } + AdvancedADC(): n_channels(0), descr(nullptr) {} ~AdvancedADC(); bool available(); SampleBuffer read(); int begin(uint32_t resolution, uint32_t sample_rate, size_t n_samples, size_t n_buffers); + int begin(uint32_t resolution, uint32_t sample_rate, size_t n_samples, size_t n_buffers, size_t n_pins, pin_size_t *pins) { + static_assert(n_pins < AN_MAX_ADC_CHANNELS, "A maximum of 5 channels can be sampled successively."); + for (size_t i = 0; i < n_pins; ++i) { + adc_pins[i] = analogPinToPinName(pins[i]); + } + n_channels = n_pins; + } int stop(); }; From 04f127541f158381d63418c35be36b92c5d34e1e Mon Sep 17 00:00:00 2001 From: Thomas Friedrichsmeier Date: Wed, 5 Apr 2023 21:50:52 +0200 Subject: [PATCH 5/8] Enforce limit at runtime rather than static assertion --- src/AdvancedADC.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AdvancedADC.h b/src/AdvancedADC.h index 46321d1..3754bf1 100644 --- a/src/AdvancedADC.h +++ b/src/AdvancedADC.h @@ -48,7 +48,7 @@ class AdvancedADC { SampleBuffer read(); int begin(uint32_t resolution, uint32_t sample_rate, size_t n_samples, size_t n_buffers); int begin(uint32_t resolution, uint32_t sample_rate, size_t n_samples, size_t n_buffers, size_t n_pins, pin_size_t *pins) { - static_assert(n_pins < AN_MAX_ADC_CHANNELS, "A maximum of 5 channels can be sampled successively."); + if (n_pins > AN_MAX_ADC_CHANNELS) n_pins = AN_MAX_ADC_CHANNELS; for (size_t i = 0; i < n_pins; ++i) { adc_pins[i] = analogPinToPinName(pins[i]); } From 82ae24a2a9b46e92b58b212c72abb0127644ee17 Mon Sep 17 00:00:00 2001 From: Thomas Friedrichsmeier Date: Wed, 5 Apr 2023 22:20:05 +0200 Subject: [PATCH 6/8] Add missing bit I shall test my PR before submitting... --- src/AdvancedADC.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/AdvancedADC.h b/src/AdvancedADC.h index 3754bf1..844985b 100644 --- a/src/AdvancedADC.h +++ b/src/AdvancedADC.h @@ -53,6 +53,7 @@ class AdvancedADC { adc_pins[i] = analogPinToPinName(pins[i]); } n_channels = n_pins; + return begin(resolution, sample_rate, n_samples, n_buffers); } int stop(); }; From 4c365697fd74d2220439ad40bb84a61baa181f77 Mon Sep 17 00:00:00 2001 From: Thomas Friedrichsmeier Date: Wed, 12 Apr 2023 14:14:12 +0200 Subject: [PATCH 7/8] Add usage example for dynamic ADC multi channel setup --- .../ADC_Multi_Channel_Dynamic.ino | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 examples/Advanced/ADC_Multi_Channel_Dynamic/ADC_Multi_Channel_Dynamic.ino diff --git a/examples/Advanced/ADC_Multi_Channel_Dynamic/ADC_Multi_Channel_Dynamic.ino b/examples/Advanced/ADC_Multi_Channel_Dynamic/ADC_Multi_Channel_Dynamic.ino new file mode 100644 index 0000000..c208289 --- /dev/null +++ b/examples/Advanced/ADC_Multi_Channel_Dynamic/ADC_Multi_Channel_Dynamic.ino @@ -0,0 +1,74 @@ +/* ADC Multi Channel sampling usage demo + * + * Queries for pin numbers to sample on the Serial Monitor, then records and prints three readings on + * each of those pins at a leisurely rate of 2 Hz. + */ + +#include + +AdvancedADC adc; +uint64_t last_millis = 0; +pin_size_t active_pins[AN_MAX_ADC_CHANNELS]; +int num_active_pins = 0; +const int samples_per_round = 3; + +void queryPins() { + Serial.println("Enter pins to sample (number only, e.g. 3,4 for A3, and A4). Enter to repeat previous round."); + + int old_num_active_pins = num_active_pins; + num_active_pins = 0; + String buf; + int c; + do { + c = Serial.read(); + if (c < 0) continue; + + if (c == ',' || c == '\n') { + buf.trim(); + if (buf.length()) { + active_pins[num_active_pins++] = buf.toInt() + A0; + buf = String(); + } + } else { + buf += (char) c; + } + } while (!(c == '\n' || num_active_pins >= AN_MAX_ADC_CHANNELS)); + + // No (valid) input? Repeat previous measurement cylce + if (!num_active_pins) { + num_active_pins = old_num_active_pins; + } +} + +void setup() { + Serial.begin(9600); + while (!Serial) {}; +} + +void loop() { + queryPins(); + if (num_active_pins) { + // Resolution, sample rate, number of samples per buffer per channel, queue depth, number of pins, array of pins. + if (!adc.begin(AN_RESOLUTION_16, 2, 1, samples_per_round, num_active_pins, active_pins)) { + Serial.println("Failed to start analog acquisition!"); + while (1); + } + + for (int i = 0; i < samples_per_round; ++i) { + while(!adc.available()) {}; // Your code could do something useful while waiting! + + SampleBuffer buf = adc.read(); + + for (int i = 0; i < num_active_pins; ++i) { + Serial.print(buf[i]); + Serial.print(" "); + } + Serial.println(); + + // Release the buffer to return it to the pool. + buf.release(); + } + + adc.stop(); + } +} From 4ecffdddb5b327950cf0c7dc7e8ee17b87d7d8a1 Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Fri, 12 May 2023 08:03:34 +0200 Subject: [PATCH 8/8] Release v1.2.0. --- library.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library.properties b/library.properties index 86894c7..dca862c 100644 --- a/library.properties +++ b/library.properties @@ -1,5 +1,5 @@ name=Arduino_AdvancedAnalog -version=1.1.0 +version=1.2.0 author=Arduino maintainer=Arduino sentence=Advanced analog functionalities for STM32H7 boards