Use DMA for reading external ADC

The external ADC will constantly convert, reading will give
the maximum value since the last read. This should avoid
missing any peaks.

The internal ADC will now only do one conversion upon read,
consider it abandoned for now.

include/GlobalConfiguration.h

@@ -56,8 +56,9 @@ const Peripherals::Drum::Config drum_config = {
         4,       // MISO Pin
         2,       // SCLK Pin
         1,       // SCSn Pin
+        0,       // Level Shifter Enable Pin
         spi0,    // Block
-        2000000, // Speed // TODO Does lowring help?
+        2000000, // Speed
     },
 };
 

include/peripherals/Drum.h

@@ -5,8 +5,9 @@
 
 #include "hardware/spi.h"
 
-#include <mcp3204/Mcp3204.h>
+#include <mcp3204/Mcp3204Dma.h>
 
+#include <array>
 #include <map>
 #include <memory>
 #include <stdint.h>
@@ -44,6 +45,7 @@ class Drum {
             uint8_t miso_pin;
             uint8_t sclk_pin;
             uint8_t scsn_pin;
+            uint8_t level_shifter_enable_pin;
             spi_inst_t *block;
             uint speed_hz;
         } external_adc_spi_config;
@@ -73,23 +75,23 @@ class Drum {
 
     class AdcInterface {
       public:
-        // This is expected to return a 12bit value.
+        // Those are expected to be 12bit values
-        virtual uint16_t read(uint8_t channel) = 0;
+        virtual std::array<uint16_t, 4> read() = 0;
     };
 
     class InternalAdc : public AdcInterface {
       public:
         InternalAdc(const Config::AdcInputs &adc_inputs);
-        virtual uint16_t read(uint8_t channel) final;
+        virtual std::array<uint16_t, 4> read() final;
     };
 
     class ExternalAdc : public AdcInterface {
       private:
-        Mcp3204 m_mcp3204;
+        Mcp3204Dma m_mcp3204;
 
       public:
         ExternalAdc(const Config::Spi &spi_config);
-        virtual uint16_t read(uint8_t channel) final;
+        virtual std::array<uint16_t, 4> read() final;
     };
 
     Config m_config;

libs/mcp3204/CMakeLists.txt

@@ -7,4 +7,4 @@ target_include_directories(
   PUBLIC ${CMAKE_CURRENT_LIST_DIR}/include
   PRIVATE ${CMAKE_CURRENT_LIST_DIR}/include/mcp3204)
 
-target_link_libraries(mcp3204 PUBLIC pico_stdlib hardware_spi)
+target_link_libraries(mcp3204 PUBLIC pico_stdlib hardware_spi hardware_dma)

libs/mcp3204/include/mcp3204/Mcp3204Dma.h

@@ -0,0 +1,22 @@
+#ifndef _MCP3204_MCP3204DMA_H_
+#define _MCP3204_MCP3204DMA_H_
+
+#include "hardware/spi.h"
+
+#include <array>
+
+class Mcp3204Dma {
+  public:
+    static constexpr size_t channel_count = 4;
+
+  public:
+    Mcp3204Dma(spi_inst *spi, uint8_t cs_pin);
+    ~Mcp3204Dma();
+
+    void run();
+    void stop();
+
+    std::array<uint16_t, channel_count> take_maximums();
+};
+
+#endif // _MCP3204_MCP3204DMA_H_

libs/mcp3204/src/Mcp3204Dma.cpp

@@ -0,0 +1,133 @@
+#include "Mcp3204Dma.h"
+
+#include "hardware/dma.h"
+#include "hardware/gpio.h"
+#include "pico/time.h"
+
+namespace {
+constexpr size_t transfer_length = 3;
+
+volatile uint cs_pin = 0;
+
+volatile int rx_channel = -1;
+volatile int tx_channel = -1;
+
+volatile uint8_t rx_buffer[transfer_length] = {};
+volatile uint8_t tx_buffer[transfer_length] = {
+    0x06, // '00000' to align the ADC's output,
+          // '1' as start bit,
+          // '1' for single-ended read,
+          // '0' for D2 (which is 'don't care' on MCP3204)
+    0x00, // Channel bits D1 and D0, followed by '0's as clocks to receive result
+    0x00, // Further '0's to receive result
+};
+
+volatile uint8_t current_channel = 0;
+volatile uint16_t current_max_readings[Mcp3204Dma::channel_count] = {};
+
+// Alarm handler to instantly (re)start DMA reading of the next channel.
+int64_t start_dma_read(alarm_id_t id, void *user_data) {
+    (void)user_data;
+    (void)id;
+
+    // Reset addresses
+    dma_channel_set_read_addr(tx_channel, tx_buffer, false);
+    dma_channel_set_write_addr(rx_channel, rx_buffer, false);
+
+    // Pull down CS pin and start both TX and RX at the same time
+    gpio_put(cs_pin, false);
+    dma_start_channel_mask((1 << tx_channel) | (1 << rx_channel));
+
+    // Do not reschedule alarm
+    return 0;
+}
+
+// Pull up CS pin and start DMA reading after a 1us delay, this is because MCP3204
+// needs to be the CS pin high for at least 500ns.
+void trigger_dma_read() {
+    gpio_put(cs_pin, true);
+
+    add_alarm_in_us(2, start_dma_read, nullptr, true);
+}
+
+void read_handler() {
+    // The 12 result bits are at the end of the ADC's output.
+    const uint16_t value = (static_cast<uint16_t>(rx_buffer[1] & 0x0F) << 8) | rx_buffer[2];
+
+    // We only care for the maximum value since the last read
+    if (value > current_max_readings[current_channel]) {
+        current_max_readings[current_channel] = value;
+    }
+
+    // Advance to the next channel
+    current_channel = (current_channel + 1) % Mcp3204Dma::channel_count;
+    tx_buffer[1] = static_cast<uint8_t>(current_channel << 6);
+
+    dma_channel_acknowledge_irq0(rx_channel);
+
+    trigger_dma_read();
+}
+
+} // namespace
+
+Mcp3204Dma::Mcp3204Dma(spi_inst *spi, uint8_t cs_pin) {
+
+    ::cs_pin = cs_pin;
+    ::tx_channel = dma_claim_unused_channel(true);
+    ::rx_channel = dma_claim_unused_channel(true);
+
+    // Configure TX Channel
+    dma_channel_config tx_channel_config = dma_channel_get_default_config(tx_channel);
+    channel_config_set_transfer_data_size(&tx_channel_config, DMA_SIZE_8);
+    channel_config_set_dreq(&tx_channel_config, spi_get_dreq(spi, true));
+    channel_config_set_read_increment(&tx_channel_config, true);
+    channel_config_set_write_increment(&tx_channel_config, false);
+    dma_channel_configure(tx_channel, &tx_channel_config, &spi_get_hw(spi)->dr, tx_buffer, transfer_length, false);
+
+    // Configure RX Channel
+    dma_channel_config rx_channel_config = dma_channel_get_default_config(rx_channel);
+    channel_config_set_transfer_data_size(&rx_channel_config, DMA_SIZE_8);
+    channel_config_set_dreq(&rx_channel_config, spi_get_dreq(spi, false));
+    channel_config_set_read_increment(&rx_channel_config, false);
+    channel_config_set_write_increment(&rx_channel_config, true);
+    dma_channel_configure(rx_channel, &rx_channel_config, rx_buffer, &spi_get_hw(spi)->dr, transfer_length, false);
+
+    irq_set_exclusive_handler(DMA_IRQ_0, read_handler);
+}
+
+Mcp3204Dma::~Mcp3204Dma() {
+    stop();
+
+    dma_channel_unclaim(rx_channel);
+    dma_channel_unclaim(tx_channel);
+}
+
+void Mcp3204Dma::run() {
+    stop();
+
+    dma_channel_set_irq0_enabled(rx_channel, true);
+    irq_set_enabled(DMA_IRQ_0, true);
+
+    trigger_dma_read();
+}
+
+void Mcp3204Dma::stop() {
+    irq_set_enabled(DMA_IRQ_0, false);
+    dma_channel_set_irq0_enabled(rx_channel, false);
+
+    dma_channel_wait_for_finish_blocking(rx_channel);
+    dma_channel_wait_for_finish_blocking(tx_channel);
+}
+
+std::array<uint16_t, Mcp3204Dma::channel_count> Mcp3204Dma::take_maximums() {
+    // TODO: theoretically we should need to pause conversion for reading the values,
+    //       but so far this does not seem to pose any issue.
+
+    std::array<uint16_t, Mcp3204Dma::channel_count> result;
+    std::copy(std::begin(current_max_readings), std::end(current_max_readings), std::begin(result));
+
+    // Reset values to zero
+    std::fill(std::begin(current_max_readings), std::end(current_max_readings), 0);
+
+    return result;
+}

src/peripherals/Drum.cpp

@@ -19,38 +19,37 @@ Drum::InternalAdc::InternalAdc(const Drum::Config::AdcInputs &adc_inputs) {
     adc_init();
 }
 
-uint16_t Drum::InternalAdc::read(uint8_t channel) {
+std::array<uint16_t, 4> Drum::InternalAdc::read() {
-    adc_select_input(channel);
+    // TODO: This should probably also use DMA
-    return adc_read();
+
+    std::array<uint16_t, 4> result;
+
+    for (size_t idx = 0; idx < 4; ++idx) {
+        adc_select_input(idx);
+        result[idx] = adc_read();
+    }
+
+    return result;
 }
 
 Drum::ExternalAdc::ExternalAdc(const Drum::Config::Spi &spi_config) : m_mcp3204(spi_config.block, spi_config.scsn_pin) {
     // Enable level shifter
-    gpio_init(0);
+    gpio_init(spi_config.level_shifter_enable_pin);
-    gpio_set_dir(0, GPIO_OUT);
+    gpio_set_dir(spi_config.level_shifter_enable_pin, GPIO_OUT);
-    gpio_put(0, true);
+    gpio_put(spi_config.level_shifter_enable_pin, true);
 
     gpio_set_function(spi_config.miso_pin, GPIO_FUNC_SPI);
     gpio_set_function(spi_config.mosi_pin, GPIO_FUNC_SPI);
     gpio_set_function(spi_config.sclk_pin, GPIO_FUNC_SPI);
-    // gpio_set_function(spi_config.scsn_pin, GPIO_FUNC_SPI);
-
     spi_init(spi_config.block, spi_config.speed_hz);
 
-    // Theoretically the ADC should work in SPI 1,1 mode.
-    // In this mode date will be clocked out on
-    // a falling edge and latched from the ADC on a rising edge.
-    // Also the CS will be held low in-between bytes as required
-    // by the mcp3204.
-    // However this mode causes glitches during continuous reading,
-    // so we need to use default mode and set CS manually.
-    // spi_set_format(m_spi, 8, SPI_CPOL_1, SPI_CPHA_1, SPI_MSB_FIRST);
     gpio_init(spi_config.scsn_pin);
     gpio_set_dir(spi_config.scsn_pin, GPIO_OUT);
-    gpio_put(spi_config.scsn_pin, true);
+
+    m_mcp3204.run();
 }
 
-uint16_t Drum::ExternalAdc::read(uint8_t channel) { return m_mcp3204.read(channel); }
+std::array<uint16_t, 4> Drum::ExternalAdc::read() { return m_mcp3204.take_maximums(); }
 
 Drum::Pad::Pad(const uint8_t channel) : channel(channel), last_change(0), active(false) {}
 
@@ -81,19 +80,12 @@ Drum::Drum(const Config &config) : m_config(config) {
 }
 
 std::map<Drum::Id, uint16_t> Drum::sampleInputs() {
-    std::map<Id, uint32_t> values;
-
-    // Oversample ADC inputs to get rid of ADC noise
-    for (uint8_t sample_number = 0; sample_number < m_config.sample_count; ++sample_number) {
-        for (const auto &pad : m_pads) {
-            values[pad.first] += m_adc->read(pad.second.getChannel());
-        }
-    }
-
-    // Take average of all samples
     std::map<Id, uint16_t> result;
-    for (auto &value : values) {
+
-        result[value.first] = value.second / m_config.sample_count;
+    const auto adc_values = m_adc->read();
+
+    for (const auto &pad : m_pads) {
+        result[pad.first] = adc_values[pad.second.getChannel()];
     }
 
     return result;