From 381d91e7bc71c76d258b62b7f0b74cd19799adcf Mon Sep 17 00:00:00 2001 From: Frederik Walk Date: Wed, 13 Nov 2024 21:05:49 +0100 Subject: [PATCH] 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 | 3 +- include/peripherals/Drum.h | 14 ++- libs/mcp3204/CMakeLists.txt | 2 +- libs/mcp3204/include/mcp3204/Mcp3204Dma.h | 22 ++++ libs/mcp3204/src/Mcp3204Dma.cpp | 133 ++++++++++++++++++++++ src/peripherals/Drum.cpp | 52 ++++----- 6 files changed, 188 insertions(+), 38 deletions(-) create mode 100644 libs/mcp3204/include/mcp3204/Mcp3204Dma.h create mode 100644 libs/mcp3204/src/Mcp3204Dma.cpp diff --git a/include/GlobalConfiguration.h b/include/GlobalConfiguration.h index 6a240e0..467bb0d 100644 --- a/include/GlobalConfiguration.h +++ b/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 }, }; diff --git a/include/peripherals/Drum.h b/include/peripherals/Drum.h index acfee9a..5fb3a49 100644 --- a/include/peripherals/Drum.h +++ b/include/peripherals/Drum.h @@ -5,8 +5,9 @@ #include "hardware/spi.h" -#include +#include +#include #include #include #include @@ -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. - virtual uint16_t read(uint8_t channel) = 0; + // Those are expected to be 12bit values + virtual std::array read() = 0; }; class InternalAdc : public AdcInterface { public: InternalAdc(const Config::AdcInputs &adc_inputs); - virtual uint16_t read(uint8_t channel) final; + virtual std::array 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 read() final; }; Config m_config; diff --git a/libs/mcp3204/CMakeLists.txt b/libs/mcp3204/CMakeLists.txt index ce7aca7..b3bd0ca 100644 --- a/libs/mcp3204/CMakeLists.txt +++ b/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) diff --git a/libs/mcp3204/include/mcp3204/Mcp3204Dma.h b/libs/mcp3204/include/mcp3204/Mcp3204Dma.h new file mode 100644 index 0000000..17d7f58 --- /dev/null +++ b/libs/mcp3204/include/mcp3204/Mcp3204Dma.h @@ -0,0 +1,22 @@ +#ifndef _MCP3204_MCP3204DMA_H_ +#define _MCP3204_MCP3204DMA_H_ + +#include "hardware/spi.h" + +#include + +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 take_maximums(); +}; + +#endif // _MCP3204_MCP3204DMA_H_ \ No newline at end of file diff --git a/libs/mcp3204/src/Mcp3204Dma.cpp b/libs/mcp3204/src/Mcp3204Dma.cpp new file mode 100644 index 0000000..d291b22 --- /dev/null +++ b/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(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(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 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 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; +} \ No newline at end of file diff --git a/src/peripherals/Drum.cpp b/src/peripherals/Drum.cpp index d7c55cd..486fdef 100644 --- a/src/peripherals/Drum.cpp +++ b/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) { - adc_select_input(channel); - return adc_read(); +std::array Drum::InternalAdc::read() { + // TODO: This should probably also use DMA + + std::array 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_set_dir(0, GPIO_OUT); - gpio_put(0, true); + gpio_init(spi_config.level_shifter_enable_pin); + gpio_set_dir(spi_config.level_shifter_enable_pin, GPIO_OUT); + 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 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::sampleInputs() { - std::map 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 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;