From ef009fdc4258a1d964cd16e3691edce5ed889856 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Mon, 25 May 2026 23:15:15 -0500 Subject: [PATCH 01/12] feat(spi): Add SPI component --- components/spi/CMakeLists.txt | 5 + components/spi/README.md | 27 + components/spi/example/CMakeLists.txt | 21 + components/spi/example/README.md | 9 + components/spi/example/main/CMakeLists.txt | 3 + components/spi/example/main/spi_example.cpp | 43 ++ components/spi/idf_component.yml | 21 + components/spi/include/spi.hpp | 354 +++++++++++++ components/spi/src/spi.cpp | 522 ++++++++++++++++++++ doc/en/spi.rst | 17 + doc/en/spi_example.md | 2 + 11 files changed, 1024 insertions(+) create mode 100644 components/spi/CMakeLists.txt create mode 100644 components/spi/README.md create mode 100644 components/spi/example/CMakeLists.txt create mode 100644 components/spi/example/README.md create mode 100644 components/spi/example/main/CMakeLists.txt create mode 100644 components/spi/example/main/spi_example.cpp create mode 100644 components/spi/idf_component.yml create mode 100644 components/spi/include/spi.hpp create mode 100644 components/spi/src/spi.cpp create mode 100644 doc/en/spi.rst create mode 100644 doc/en/spi_example.md diff --git a/components/spi/CMakeLists.txt b/components/spi/CMakeLists.txt new file mode 100644 index 000000000..432dcbcda --- /dev/null +++ b/components/spi/CMakeLists.txt @@ -0,0 +1,5 @@ +idf_component_register( + INCLUDE_DIRS "include" + SRC_DIRS "src" + REQUIRES "base_component" "display_drivers" "esp_driver_gpio" "esp_driver_spi" "task" + ) diff --git a/components/spi/README.md b/components/spi/README.md new file mode 100644 index 000000000..5df01a6a4 --- /dev/null +++ b/components/spi/README.md @@ -0,0 +1,27 @@ +# SPI Component + +[![Badge](https://components.espressif.com/components/espp/spi/badge.svg)](https://components.espressif.com/components/espp/spi) + +The `Spi` class provides a simple C++ wrapper around the ESP-IDF SPI master +driver. It manages SPI bus initialization, per-device registration, blocking +transfers, queued transactions, and bus acquisition. + +The component also provides `SpiPanelIo` (also aliased as `SpiCommandData`), a +helper for the common LCD / display pattern where a dedicated D/C pin selects +between command and data transactions while the pixel data is queued using DMA. + +`SpiPanelIo` treats command/data selection consistently: + +- `write_command()` queues the command byte with D/C low and automatically sends + any parameter bytes with D/C high. +- `queue_command()` queues a command transaction with D/C low. +- `queue_data()` and `queue_pixels()` queue payload transactions with D/C high. + +That means controller code should call `queue_command()` for register opcodes +and `queue_data()` / `queue_pixels()` for the corresponding payload without +manually OR-ing the D/C flag. + +## Example + +The [example](./example) shows how to create an SPI bus, attach a device, and +perform a simple addressed read transaction. diff --git a/components/spi/example/CMakeLists.txt b/components/spi/example/CMakeLists.txt new file mode 100644 index 000000000..cf50ea584 --- /dev/null +++ b/components/spi/example/CMakeLists.txt @@ -0,0 +1,21 @@ +# The following lines of boilerplate have to be in your project's CMakeLists +# in this exact order for cmake to work correctly +cmake_minimum_required(VERSION 3.20) + +set(ENV{IDF_COMPONENT_MANAGER} "0") +include($ENV{IDF_PATH}/tools/cmake/project.cmake) + +set(EXTRA_COMPONENT_DIRS + "../../../components/" +) + +set( + COMPONENTS + "main esptool_py driver task spi" + CACHE STRING + "List of components to include" + ) + +project(spi_example) + +set(CMAKE_CXX_STANDARD 20) diff --git a/components/spi/example/README.md b/components/spi/example/README.md new file mode 100644 index 000000000..94e67546d --- /dev/null +++ b/components/spi/example/README.md @@ -0,0 +1,9 @@ +# SPI Example + +This example shows the use of the `Spi` component to create an SPI bus, attach a +device, and perform a simple addressed read transaction. + +## How to use example + +The example is intended primarily as a compile-time reference for the `Spi` +API. Update the GPIO assignments to match your hardware before flashing. diff --git a/components/spi/example/main/CMakeLists.txt b/components/spi/example/main/CMakeLists.txt new file mode 100644 index 000000000..f5aaee7f5 --- /dev/null +++ b/components/spi/example/main/CMakeLists.txt @@ -0,0 +1,3 @@ +idf_component_register(SRC_DIRS "." + INCLUDE_DIRS "." + REQUIRES spi) diff --git a/components/spi/example/main/spi_example.cpp b/components/spi/example/main/spi_example.cpp new file mode 100644 index 000000000..9331af230 --- /dev/null +++ b/components/spi/example/main/spi_example.cpp @@ -0,0 +1,43 @@ +#include +#include + +#include "spi.hpp" + +extern "C" void app_main(void) { + constexpr spi_host_device_t spi_host = SPI2_HOST; + constexpr auto sclk_gpio = GPIO_NUM_36; + constexpr auto miso_gpio = GPIO_NUM_35; + constexpr auto cs_gpio = GPIO_NUM_37; + constexpr uint8_t read_address = 0x80; + + espp::Spi spi({ + .host = spi_host, + .sclk_io_num = sclk_gpio, + .mosi_io_num = GPIO_NUM_NC, + .miso_io_num = miso_gpio, + .max_transfer_sz = 16, + }); + + std::error_code ec; + auto device = spi.add_device( + { + .address_bits = 8, + .mode = 0, + .clock_speed_hz = 1 * 1000 * 1000, + .cs_io_num = cs_gpio, + .queue_size = 1, + }, + ec); + if (ec || !device) { + std::printf("Failed to create SPI device: %s\n", ec.message().c_str()); + return; + } + + std::array rx_data{}; + if (!device->read(rx_data, {.address = read_address}, ec)) { + std::printf("SPI read failed: %s\n", ec.message().c_str()); + return; + } + + std::printf("Read bytes: 0x%02x 0x%02x\n", rx_data[0], rx_data[1]); +} diff --git a/components/spi/idf_component.yml b/components/spi/idf_component.yml new file mode 100644 index 000000000..322a4044b --- /dev/null +++ b/components/spi/idf_component.yml @@ -0,0 +1,21 @@ +## IDF Component Manager Manifest File +license: "MIT" +description: "SPI component for ESP-IDF" +url: "https://github.com/esp-cpp/espp/tree/main/components/spi" +repository: "git://github.com/esp-cpp/espp.git" +maintainers: + - William Emfinger +documentation: "https://esp-cpp.github.io/espp/spi.html" +examples: + - path: example +tags: + - cpp + - Component + - SPI + - Display + - DMA +dependencies: + idf: + version: '>=5.0' + espp/base_component: '>=1.0' + espp/task: '>=1.0' diff --git a/components/spi/include/spi.hpp b/components/spi/include/spi.hpp new file mode 100644 index 000000000..c7866e9bc --- /dev/null +++ b/components/spi/include/spi.hpp @@ -0,0 +1,354 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +#include "base_component.hpp" +#include "display_drivers.hpp" + +namespace espp { +/// @brief SPI master wrapper and helpers. +/// @details +/// `Spi` owns a SPI bus and can create one or more attached `Device` instances. +/// Those devices support blocking reads/writes, queued transactions, and bus +/// acquisition. `SpiPanelIo` builds on top of `Spi::Device` for queued +/// command/data display transports. +/// +/// \section spi_example Example +/// \snippet spi_example.cpp spi example +class Spi : public BaseComponent { +public: + /// @brief Bus-level configuration for the SPI master. + struct Config { + int isr_core_id = -1; ///< Core on which to initialize the SPI bus. + spi_host_device_t host = SPI2_HOST; ///< SPI host controller to use. + gpio_num_t sclk_io_num = GPIO_NUM_NC; ///< Clock pin. + gpio_num_t mosi_io_num = GPIO_NUM_NC; ///< MOSI pin. + gpio_num_t miso_io_num = GPIO_NUM_NC; ///< MISO pin. + gpio_num_t quadwp_io_num = GPIO_NUM_NC; ///< Quad WP pin. + gpio_num_t quadhd_io_num = GPIO_NUM_NC; ///< Quad HD pin. + int max_transfer_sz = 0; ///< Maximum transfer size in bytes. + uint32_t bus_flags = 0; ///< ESP-IDF bus configuration flags. + int intr_flags = 0; ///< Interrupt allocation flags. + spi_dma_chan_t dma_channel = SPI_DMA_CH_AUTO; ///< DMA channel selection. + bool auto_init = true; ///< Automatically initialize on construction. + Logger::Verbosity log_level = Logger::Verbosity::WARN; ///< Logger verbosity. + }; + + /// @brief Per-device configuration for a device attached to the SPI bus. + struct DeviceConfig { + int command_bits = 0; ///< Number of command bits sent before payload data. + int address_bits = 0; ///< Number of address bits sent before payload data. + int dummy_bits = 0; ///< Number of dummy bits inserted before payload data. + uint8_t mode = 0; ///< SPI mode. + int clock_speed_hz = 1 * 1000 * 1000; ///< Device clock speed. + int input_delay_ns = 0; ///< Input delay in nanoseconds. + gpio_num_t cs_io_num = GPIO_NUM_NC; ///< Chip-select pin. + int queue_size = 1; ///< Queue depth for asynchronous transactions. + uint32_t flags = 0; ///< ESP-IDF device flags. + uint16_t cs_ena_pretrans = 0; ///< CS lead time in SPI bit-cycles. + uint16_t cs_ena_posttrans = 0; ///< CS hold time in SPI bit-cycles. + transaction_cb_t pre_cb = nullptr; ///< Optional pre-transfer callback. + transaction_cb_t post_cb = nullptr; ///< Optional post-transfer callback. + }; + + /// @brief Per-transaction overrides for read/write/transfer helpers. + struct TransactionConfig { + uint16_t command = 0; ///< Command value for this transaction. + uint64_t address = 0; ///< Address value for this transaction. + uint32_t flags = 0; ///< ESP-IDF transaction flags. + size_t rx_length_bits = 0; ///< Optional receive length override, in bits. + }; + + class Device; + + /// @brief RAII helper for holding an acquired SPI device bus lock. + class BusLock { + public: + /// @brief Construct an empty bus lock. + BusLock(); + + /// @brief Construct a bus lock from an acquired ESP-IDF SPI handle. + /// @param handle Acquired device handle. + explicit BusLock(spi_device_handle_t handle); + + BusLock(const BusLock &) = delete; + BusLock &operator=(const BusLock &) = delete; + + /// @brief Move-construct the bus lock. + /// @param other Lock to move from. + BusLock(BusLock &&other) noexcept; + + /// @brief Move-assign the bus lock. + /// @param other Lock to move from. + /// @return Reference to this lock. + BusLock &operator=(BusLock &&other) noexcept; + + /// @brief Release the held bus lock on destruction. + ~BusLock(); + + /// @brief Release the held SPI bus lock, if any. + void release(); + + /// @brief Check whether this object currently owns a bus lock. + /// @return True if a bus lock is held. + explicit operator bool() const; + + private: + spi_device_handle_t handle_{nullptr}; + }; + + /// @brief Construct the SPI bus wrapper. + /// @param config SPI bus configuration. + explicit Spi(const Config &config); + + /// @brief Deinitialize the SPI bus and remove any remaining devices. + ~Spi(); + + Spi(const Spi &) = delete; + Spi &operator=(const Spi &) = delete; + Spi(Spi &&) = delete; + Spi &operator=(Spi &&) = delete; + + /// @brief Convert a `Spi::Config` into an ESP-IDF `spi_bus_config_t`. + /// @param config High-level SPI bus configuration. + /// @return Equivalent ESP-IDF bus configuration. + static spi_bus_config_t make_bus_config(const Config &config); + + /// @brief Convert a `Spi::DeviceConfig` into an ESP-IDF device config. + /// @param config High-level SPI device configuration. + /// @return Equivalent ESP-IDF device interface configuration. + static spi_device_interface_config_t make_device_config(const DeviceConfig &config); + + /// @brief Initialize the SPI bus. + /// @param ec Error code populated on failure. + void init(std::error_code &ec); + + /// @brief Deinitialize the SPI bus. + /// @param ec Error code populated on failure. + void deinit(std::error_code &ec); + + /// @brief Check whether the SPI bus is initialized. + /// @return True if the SPI bus is initialized. + bool initialized() const; + + /// @brief Get the configured SPI host. + /// @return Configured SPI host controller. + spi_host_device_t host() const; + + /// @brief Add a device to the SPI bus. + /// @param config Device configuration. + /// @param ec Error code populated on failure. + /// @return Shared pointer to the registered device, or nullptr on failure. + std::shared_ptr add_device(const DeviceConfig &config, std::error_code &ec); + +private: + friend class Device; + + Config config_; + std::recursive_mutex mutex_; + bool initialized_{false}; + std::vector> devices_; +}; + +/// @brief Attached SPI device wrapper. +class Spi::Device : public BaseComponent { +public: + /// @brief Construct an unattached device wrapper. + /// @param spi Owning SPI bus wrapper. + /// @param config Device configuration. + explicit Device(Spi &spi, const DeviceConfig &config); + + /// @brief Remove the device from the SPI bus on destruction. + ~Device(); + + Device(const Device &) = delete; + Device &operator=(const Device &) = delete; + Device(Device &&) = delete; + Device &operator=(Device &&) = delete; + + /// @brief Check whether the device is attached to the SPI bus. + /// @return True if the device has a valid ESP-IDF handle. + bool initialized() const; + + /// @brief Get the underlying ESP-IDF device handle. + /// @return SPI device handle, or nullptr if unattached. + spi_device_handle_t handle() const; + + /// @brief Get the configuration used to create this device. + /// @return Device configuration. + const DeviceConfig &config() const; + + /// @brief Remove this device from the SPI bus. + /// @param ec Error code populated on failure. + void remove_device(std::error_code &ec); + + /// @brief Submit a blocking SPI transaction. + /// @param transaction Transaction to execute. + /// @param ec Error code populated on failure. + /// @return True on success. + bool transmit(spi_transaction_t &transaction, std::error_code &ec); + + /// @brief Submit a polling SPI transaction. + /// @param transaction Transaction to execute. + /// @param ec Error code populated on failure. + /// @return True on success. + bool polling_transmit(spi_transaction_t &transaction, std::error_code &ec); + + /// @brief Queue an asynchronous SPI transaction. + /// @param transaction Transaction to queue. + /// @param timeout Queue timeout in FreeRTOS ticks. + /// @param ec Error code populated on failure. + /// @return True on success. + bool queue_transaction(spi_transaction_t &transaction, TickType_t timeout, std::error_code &ec); + + /// @brief Wait for a queued SPI transaction to complete. + /// @param transaction Filled with the completed transaction pointer. + /// @param timeout Wait timeout in FreeRTOS ticks. + /// @param ec Error code populated on failure. + /// @return True on success. + bool get_transaction_result(spi_transaction_t **transaction, TickType_t timeout, + std::error_code &ec); + + /// @brief Acquire exclusive access to the SPI bus for this device. + /// @param timeout Acquire timeout in FreeRTOS ticks. + /// @param ec Error code populated on failure. + /// @return RAII bus lock object. + BusLock acquire_bus(TickType_t timeout, std::error_code &ec); + + /// @brief Write a transmit buffer using the configured per-transaction overrides. + /// @param data Data to transmit. + /// @param config Transaction overrides. + /// @param ec Error code populated on failure. + /// @return True on success. + bool write(std::span data, const TransactionConfig &config, std::error_code &ec); + + /// @brief Read data using the configured per-transaction overrides. + /// @param data Receive buffer to fill. + /// @param config Transaction overrides. + /// @param ec Error code populated on failure. + /// @return True on success. + bool read(std::span data, const TransactionConfig &config, std::error_code &ec); + + /// @brief Perform a combined transmit/receive transaction. + /// @param tx_data Transmit buffer. + /// @param rx_data Receive buffer. + /// @param config Transaction overrides. + /// @param ec Error code populated on failure. + /// @return True on success. + bool transfer(std::span tx_data, std::span rx_data, + const TransactionConfig &config, std::error_code &ec); + +private: + friend class Spi; + + Spi &spi_; + DeviceConfig config_; + spi_device_handle_t handle_{nullptr}; + std::recursive_mutex mutex_; +}; + +/// @brief LCD-style command/data helper built on top of `Spi`. +/// @details +/// `queue_command()` transmits with D/C low. `queue_data()` and +/// `queue_pixels()` always transmit with D/C high, so callers should treat them +/// as payload helpers rather than manually setting the D/C bit for normal panel +/// data transactions. +class SpiPanelIo : public display_drivers::PanelIo, public BaseComponent { +public: + /// @brief IRQ-safe callback invoked after matching queued transactions finish. + using post_transaction_callback_t = void (*)(uint32_t user_flags); + + /// @brief Configuration for `SpiPanelIo`. + struct Config { + Spi *spi = nullptr; ///< Bus on which to register the display device. + Spi::DeviceConfig device_config{}; ///< SPI device configuration. + gpio_num_t data_command_io = GPIO_NUM_NC; ///< D/C GPIO pin. + uint32_t data_command_bit_mask = 0; ///< User bit that selects data mode. + uint32_t post_transaction_callback_bit_mask = 0; ///< User bit mask that triggers the callback. + post_transaction_callback_t post_transaction_callback = + nullptr; ///< Optional IRQ-safe callback. + uint32_t pixel_data_flags = + SPI_TRANS_DMA_BUFFER_ALIGN_MANUAL; ///< Flags used for pixel payload transfers. + uint32_t timeout_ms = 10; ///< Queue/result timeout in milliseconds. + Logger::Verbosity log_level = Logger::Verbosity::WARN; ///< Logger verbosity. + }; + + /// @brief Construct a panel-I/O helper for SPI command/data displays. + /// @param config Panel transport configuration. + explicit SpiPanelIo(const Config &config); + + /// @brief Check whether the helper has a registered SPI device. + /// @return True if initialization succeeded. + bool initialized() const override; + + /// @brief Get the underlying SPI device wrapper. + /// @return Shared pointer to the attached SPI device. + std::shared_ptr device() const; + + /// @brief Wait for all queued transactions to complete. + void wait() override; + + /// @brief Send a command byte and optional parameter payload. + /// @param command Command byte sent with D/C low. + /// @param parameters Optional payload bytes sent with D/C high. + /// @param user_flags Additional user-defined flags passed to callbacks. + void write_command(uint8_t command, std::span parameters, + uint32_t user_flags = 0) override; + + /// @brief Queue a command transaction with D/C low. + /// @param command Command byte to transmit. + /// @param user_flags Additional user-defined flags passed to callbacks. + void queue_command(uint8_t command, uint32_t user_flags = 0) override; + + /// @brief Queue a non-pixel data payload with D/C high. + /// @param data Data bytes to transmit. + /// @param user_flags Additional user-defined flags passed to callbacks. + void queue_data(std::span data, uint32_t user_flags = 0) override; + + /// @brief Queue a pixel payload with D/C high. + /// @param data Pointer to the pixel buffer. + /// @param size Pixel payload size in bytes. + /// @param user_flags Additional user-defined flags passed to callbacks. + /// @param transaction_flags Optional SPI transaction flags overriding the default pixel flags. + void queue_pixels(const uint8_t *data, size_t size, uint32_t user_flags = 0, + uint32_t transaction_flags = 0) override; + +private: + struct TransactionContext { + SpiPanelIo *helper{nullptr}; + uint32_t user_flags{0}; + }; + + static void pre_transfer_callback(spi_transaction_t *transaction); + static void post_transfer_callback(spi_transaction_t *transaction); + + TickType_t timeout_ticks() const; + size_t prepare_transaction(uint32_t user_flags); + void queue_command_locked(uint8_t command, uint32_t user_flags); + void queue_data_locked(std::span data, uint32_t user_flags); + void queue_pixels_locked(const uint8_t *data, size_t size, uint32_t user_flags, + uint32_t transaction_flags); + void queue_transaction_locked(size_t index); + void wait_locked(); + + Config config_; + std::shared_ptr device_{}; + std::mutex mutex_; + std::vector transactions_{}; + std::vector contexts_{}; + int queued_transactions_{0}; +}; + +/// @brief Backward-compatible alias for the older SPI display transport name. +using SpiCommandData = SpiPanelIo; +} // namespace espp diff --git a/components/spi/src/spi.cpp b/components/spi/src/spi.cpp new file mode 100644 index 000000000..c28631a0e --- /dev/null +++ b/components/spi/src/spi.cpp @@ -0,0 +1,522 @@ +#include "spi.hpp" + +#include +#include +#include + +#include "run_on_core.hpp" + +namespace espp { +Spi::BusLock::BusLock() = default; + +Spi::BusLock::BusLock(spi_device_handle_t handle) + : handle_(handle) {} + +Spi::BusLock::BusLock(BusLock &&other) noexcept + : handle_(other.handle_) { + other.handle_ = nullptr; +} + +Spi::BusLock &Spi::BusLock::operator=(BusLock &&other) noexcept { + if (this == &other) { + return *this; + } + release(); + handle_ = other.handle_; + other.handle_ = nullptr; + return *this; +} + +Spi::BusLock::~BusLock() { release(); } + +void Spi::BusLock::release() { + if (handle_) { + spi_device_release_bus(handle_); + handle_ = nullptr; + } +} + +Spi::BusLock::operator bool() const { return handle_ != nullptr; } + +Spi::Spi(const Config &config) + : BaseComponent("SPI", config.log_level) + , config_(config) { + if (config.auto_init) { + std::error_code ec; + init(ec); + if (ec) { + logger_.error("auto init failed"); + } + } +} + +Spi::~Spi() { + std::error_code ec; + deinit(ec); + if (ec) { + logger_.error("deinit failed"); + } +} + +spi_bus_config_t Spi::make_bus_config(const Config &config) { + spi_bus_config_t bus_config{}; + bus_config.mosi_io_num = config.mosi_io_num; + bus_config.miso_io_num = config.miso_io_num; + bus_config.sclk_io_num = config.sclk_io_num; + bus_config.quadwp_io_num = config.quadwp_io_num; + bus_config.quadhd_io_num = config.quadhd_io_num; + bus_config.max_transfer_sz = config.max_transfer_sz; + bus_config.flags = config.bus_flags; + bus_config.intr_flags = config.intr_flags; + return bus_config; +} + +spi_device_interface_config_t Spi::make_device_config(const DeviceConfig &config) { + spi_device_interface_config_t device_config{}; + device_config.command_bits = config.command_bits; + device_config.address_bits = config.address_bits; + device_config.dummy_bits = config.dummy_bits; + device_config.mode = config.mode; + device_config.clock_speed_hz = config.clock_speed_hz; + device_config.input_delay_ns = config.input_delay_ns; + device_config.spics_io_num = config.cs_io_num; + device_config.queue_size = config.queue_size; + device_config.flags = config.flags; + device_config.cs_ena_pretrans = config.cs_ena_pretrans; + device_config.cs_ena_posttrans = config.cs_ena_posttrans; + device_config.pre_cb = config.pre_cb; + device_config.post_cb = config.post_cb; + return device_config; +} + +void Spi::init(std::error_code &ec) { + std::lock_guard lock(mutex_); + if (initialized_) { + logger_.warn("already initialized"); + ec = std::make_error_code(std::errc::protocol_error); + return; + } + + auto bus_config = make_bus_config(config_); + auto install_fn = [this, &bus_config]() -> esp_err_t { + return spi_bus_initialize(config_.host, &bus_config, config_.dma_channel); + }; + auto err = espp::task::run_on_core(install_fn, config_.isr_core_id); + if (err != ESP_OK) { + logger_.error("could not initialize SPI bus: {}", esp_err_to_name(err)); + ec = std::make_error_code(std::errc::io_error); + return; + } + initialized_ = true; + ec.clear(); +} + +void Spi::deinit(std::error_code &ec) { + std::vector> devices; + { + std::lock_guard lock(mutex_); + if (!initialized_) { + ec.clear(); + return; + } + for (auto &weak_device : devices_) { + if (auto device = weak_device.lock()) { + devices.push_back(device); + } + } + devices_.clear(); + } + + for (auto &device : devices) { + std::error_code device_ec; + device->remove_device(device_ec); + if (device_ec) { + logger_.error("could not remove SPI device before bus free"); + } + } + + std::lock_guard lock(mutex_); + auto err = spi_bus_free(config_.host); + if (err != ESP_OK) { + logger_.error("could not free SPI bus: {}", esp_err_to_name(err)); + ec = std::make_error_code(std::errc::io_error); + return; + } + initialized_ = false; + ec.clear(); +} + +bool Spi::initialized() const { return initialized_; } + +spi_host_device_t Spi::host() const { return config_.host; } + +std::shared_ptr Spi::add_device(const DeviceConfig &config, std::error_code &ec) { + std::lock_guard lock(mutex_); + if (!initialized_) { + logger_.error("bus not initialized"); + ec = std::make_error_code(std::errc::not_connected); + return nullptr; + } + auto device = std::make_shared(*this, config); + auto device_config = make_device_config(config); + auto err = spi_bus_add_device(this->config_.host, &device_config, &device->handle_); + if (err != ESP_OK) { + logger_.error("could not add SPI device: {}", esp_err_to_name(err)); + ec = std::make_error_code(std::errc::io_error); + return nullptr; + } + devices_.push_back(device); + ec.clear(); + return device; +} + +Spi::Device::Device(Spi &spi, const DeviceConfig &config) + : BaseComponent("SPI Device", spi.get_log_level()) + , spi_(spi) + , config_(config) {} + +Spi::Device::~Device() { + std::error_code ec; + remove_device(ec); + if (ec) { + logger_.error("device removal failed"); + } +} + +bool Spi::Device::initialized() const { return handle_ != nullptr; } + +spi_device_handle_t Spi::Device::handle() const { return handle_; } + +const Spi::DeviceConfig &Spi::Device::config() const { return config_; } + +void Spi::Device::remove_device(std::error_code &ec) { + std::lock_guard lock(mutex_); + if (!handle_) { + ec.clear(); + return; + } + auto err = spi_bus_remove_device(handle_); + if (err != ESP_OK) { + logger_.error("could not remove SPI device: {}", esp_err_to_name(err)); + ec = std::make_error_code(std::errc::io_error); + return; + } + handle_ = nullptr; + ec.clear(); +} + +bool Spi::Device::transmit(spi_transaction_t &transaction, std::error_code &ec) { + std::lock_guard lock(mutex_); + if (!handle_) { + logger_.error("device not initialized"); + ec = std::make_error_code(std::errc::not_connected); + return false; + } + auto err = spi_device_transmit(handle_, &transaction); + if (err != ESP_OK) { + logger_.error("could not transmit SPI transaction: {}", esp_err_to_name(err)); + ec = std::make_error_code(std::errc::io_error); + return false; + } + ec.clear(); + return true; +} + +bool Spi::Device::polling_transmit(spi_transaction_t &transaction, std::error_code &ec) { + std::lock_guard lock(mutex_); + if (!handle_) { + logger_.error("device not initialized"); + ec = std::make_error_code(std::errc::not_connected); + return false; + } + auto err = spi_device_polling_transmit(handle_, &transaction); + if (err != ESP_OK) { + logger_.error("could not polling-transmit SPI transaction: {}", esp_err_to_name(err)); + ec = std::make_error_code(std::errc::io_error); + return false; + } + ec.clear(); + return true; +} + +bool Spi::Device::queue_transaction(spi_transaction_t &transaction, TickType_t timeout, + std::error_code &ec) { + std::lock_guard lock(mutex_); + if (!handle_) { + logger_.error("device not initialized"); + ec = std::make_error_code(std::errc::not_connected); + return false; + } + auto err = spi_device_queue_trans(handle_, &transaction, timeout); + if (err != ESP_OK) { + logger_.error("could not queue SPI transaction: {}", esp_err_to_name(err)); + ec = std::make_error_code(std::errc::io_error); + return false; + } + ec.clear(); + return true; +} + +bool Spi::Device::get_transaction_result(spi_transaction_t **transaction, TickType_t timeout, + std::error_code &ec) { + std::lock_guard lock(mutex_); + if (!handle_) { + logger_.error("device not initialized"); + ec = std::make_error_code(std::errc::not_connected); + return false; + } + auto err = spi_device_get_trans_result(handle_, transaction, timeout); + if (err != ESP_OK) { + logger_.error("could not get SPI transaction result: {}", esp_err_to_name(err)); + ec = std::make_error_code(std::errc::io_error); + return false; + } + ec.clear(); + return true; +} + +Spi::BusLock Spi::Device::acquire_bus(TickType_t timeout, std::error_code &ec) { + std::lock_guard lock(mutex_); + if (!handle_) { + logger_.error("device not initialized"); + ec = std::make_error_code(std::errc::not_connected); + return {}; + } + auto err = spi_device_acquire_bus(handle_, timeout); + if (err != ESP_OK) { + logger_.error("could not acquire SPI bus: {}", esp_err_to_name(err)); + ec = std::make_error_code(std::errc::io_error); + return {}; + } + ec.clear(); + return BusLock(handle_); +} + +bool Spi::Device::write(std::span data, const TransactionConfig &config, + std::error_code &ec) { + return transfer(data, {}, config, ec); +} + +bool Spi::Device::read(std::span data, const TransactionConfig &config, + std::error_code &ec) { + return transfer({}, data, config, ec); +} + +bool Spi::Device::transfer(std::span tx_data, std::span rx_data, + const TransactionConfig &config, std::error_code &ec) { + spi_transaction_t transaction{}; + transaction.cmd = config.command; + transaction.addr = config.address; + transaction.flags = config.flags; + + if (!tx_data.empty()) { + transaction.length = tx_data.size() * 8; + if (tx_data.size() <= sizeof(transaction.tx_data)) { + std::memcpy(transaction.tx_data, tx_data.data(), tx_data.size()); + transaction.flags |= SPI_TRANS_USE_TXDATA; + } else { + transaction.tx_buffer = tx_data.data(); + } + } else if (!rx_data.empty()) { + transaction.length = rx_data.size() * 8; + } + + if (!rx_data.empty()) { + transaction.rxlength = config.rx_length_bits ? config.rx_length_bits : rx_data.size() * 8; + if (rx_data.size() <= sizeof(transaction.rx_data)) { + transaction.flags |= SPI_TRANS_USE_RXDATA; + } else { + transaction.rx_buffer = rx_data.data(); + } + } + + if (!transmit(transaction, ec)) { + return false; + } + + if (!rx_data.empty() && rx_data.size() <= sizeof(transaction.rx_data)) { + std::memcpy(rx_data.data(), transaction.rx_data, rx_data.size()); + } + ec.clear(); + return true; +} + +SpiPanelIo::SpiPanelIo(const Config &config) + : BaseComponent("SPI Panel IO", config.log_level) + , config_(config) { + auto queue_depth = static_cast(std::max(2, config_.device_config.queue_size)); + transactions_.resize(queue_depth); + contexts_.resize(queue_depth); + if (!config_.spi) { + logger_.error("missing SPI bus"); + return; + } + if (config_.data_command_io != GPIO_NUM_NC) { + gpio_set_direction(config_.data_command_io, GPIO_MODE_OUTPUT); + gpio_set_level(config_.data_command_io, 0); + } + auto device_config = config_.device_config; + device_config.pre_cb = &SpiPanelIo::pre_transfer_callback; + device_config.post_cb = &SpiPanelIo::post_transfer_callback; + std::error_code ec; + device_ = config_.spi->add_device(device_config, ec); + if (ec || !device_) { + logger_.error("could not initialize SPI command/data device"); + } +} + +bool SpiPanelIo::initialized() const { return static_cast(device_); } + +std::shared_ptr SpiPanelIo::device() const { return device_; } + +void SpiPanelIo::wait() { + std::lock_guard lock(mutex_); + wait_locked(); +} + +void SpiPanelIo::write_command(uint8_t command, std::span parameters, + uint32_t user_flags) { + std::lock_guard lock(mutex_); + if (!device_) { + logger_.error("device not initialized"); + return; + } + wait_locked(); + + queue_command_locked(command, user_flags); + + if (parameters.empty()) { + wait_locked(); + return; + } + + queue_data_locked(parameters, user_flags | config_.data_command_bit_mask); + wait_locked(); +} + +void SpiPanelIo::queue_command(uint8_t command, uint32_t user_flags) { + std::lock_guard lock(mutex_); + if (!device_) { + logger_.error("device not initialized"); + return; + } + queue_command_locked(command, user_flags); +} + +void SpiPanelIo::queue_data(std::span data, uint32_t user_flags) { + std::lock_guard lock(mutex_); + if (!device_) { + logger_.error("device not initialized"); + return; + } + if (data.empty()) { + return; + } + queue_data_locked(data, user_flags | config_.data_command_bit_mask); +} + +void SpiPanelIo::queue_pixels(const uint8_t *data, size_t size, uint32_t user_flags, + uint32_t transaction_flags) { + std::lock_guard lock(mutex_); + if (!device_) { + logger_.error("device not initialized"); + return; + } + if (!data || size == 0) { + logger_.error("cannot queue null or empty pixel data"); + return; + } + queue_pixels_locked(data, size, config_.data_command_bit_mask | user_flags, transaction_flags); +} + +void SpiPanelIo::pre_transfer_callback(spi_transaction_t *transaction) { + auto *context = static_cast(transaction->user); + if (!context || !context->helper) { + return; + } + if (context->helper->config_.data_command_io == GPIO_NUM_NC) { + return; + } + bool dc_level = (context->user_flags & context->helper->config_.data_command_bit_mask) != 0; + gpio_set_level(context->helper->config_.data_command_io, dc_level); +} + +void SpiPanelIo::post_transfer_callback(spi_transaction_t *transaction) { + auto *context = static_cast(transaction->user); + if (!context || !context->helper) { + return; + } + if (!context->helper->config_.post_transaction_callback) { + return; + } + if (context->helper->config_.post_transaction_callback_bit_mask != 0 && + (context->user_flags & context->helper->config_.post_transaction_callback_bit_mask) == 0) { + return; + } + context->helper->config_.post_transaction_callback(context->user_flags); +} + +TickType_t SpiPanelIo::timeout_ticks() const { return pdMS_TO_TICKS(config_.timeout_ms); } + +size_t SpiPanelIo::prepare_transaction(uint32_t user_flags) { + if (queued_transactions_ >= static_cast(transactions_.size())) { + wait_locked(); + } + auto index = static_cast(queued_transactions_); + std::memset(&transactions_[index], 0, sizeof(transactions_[index])); + contexts_[index] = {.helper = this, .user_flags = user_flags}; + transactions_[index].user = &contexts_[index]; + return index; +} + +void SpiPanelIo::queue_command_locked(uint8_t command, uint32_t user_flags) { + auto index = prepare_transaction(user_flags); + transactions_[index].length = 8; + transactions_[index].flags = SPI_TRANS_USE_TXDATA; + transactions_[index].tx_data[0] = command; + queue_transaction_locked(index); +} + +void SpiPanelIo::queue_data_locked(std::span data, uint32_t user_flags) { + auto index = prepare_transaction(user_flags); + transactions_[index].length = data.size() * 8; + if (data.size() <= sizeof(transactions_[index].tx_data)) { + std::memcpy(transactions_[index].tx_data, data.data(), data.size()); + transactions_[index].flags = SPI_TRANS_USE_TXDATA; + } else { + transactions_[index].tx_buffer = data.data(); + } + queue_transaction_locked(index); +} + +void SpiPanelIo::queue_pixels_locked(const uint8_t *data, size_t size, uint32_t user_flags, + uint32_t transaction_flags) { + auto index = prepare_transaction(user_flags); + transactions_[index].length = size * 8; + transactions_[index].flags = + transaction_flags != 0 ? transaction_flags : config_.pixel_data_flags; + transactions_[index].tx_buffer = data; + queue_transaction_locked(index); +} + +void SpiPanelIo::queue_transaction_locked(size_t index) { + std::error_code ec; + if (device_->queue_transaction(transactions_[index], timeout_ticks(), ec)) { + queued_transactions_++; + return; + } + logger_.error("could not queue SPI command/data transaction"); +} + +void SpiPanelIo::wait_locked() { + spi_transaction_t *completed = nullptr; + while (queued_transactions_ > 0) { + std::error_code ec; + if (!device_->get_transaction_result(&completed, timeout_ticks(), ec)) { + logger_.error("could not get queued SPI transaction result"); + } + (void)completed; + queued_transactions_--; + } +} +} // namespace espp diff --git a/doc/en/spi.rst b/doc/en/spi.rst new file mode 100644 index 000000000..6037422f5 --- /dev/null +++ b/doc/en/spi.rst @@ -0,0 +1,17 @@ +SPI +*** + +The `spi` component provides a C++ wrapper around ESP-IDF's SPI master driver. + +`Spi` owns the bus, `Spi::Device` owns attached peripherals on that bus, and +`SpiCommandData` provides a higher-level helper for LCD-style command/data +transports that queue transactions and toggle a D/C GPIO in the SPI callbacks. + +.. toctree:: + + spi_example + +API Reference +------------- + +.. include-build-file:: inc/spi.inc diff --git a/doc/en/spi_example.md b/doc/en/spi_example.md new file mode 100644 index 000000000..83b36be55 --- /dev/null +++ b/doc/en/spi_example.md @@ -0,0 +1,2 @@ +```{include} ../../components/spi/example/README.md +``` From 2385a1057de6ff46e0cad72dd03d6bba010f58ff Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Mon, 25 May 2026 23:15:42 -0500 Subject: [PATCH 02/12] update doc --- doc/Doxyfile | 3 ++- doc/en/index.rst | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/Doxyfile b/doc/Doxyfile index 404591115..0d06b80bb 100755 --- a/doc/Doxyfile +++ b/doc/Doxyfile @@ -145,6 +145,7 @@ EXAMPLE_PATH = \ $(PROJECT_PATH)/components/serialization/example/main/serialization_example.cpp \ $(PROJECT_PATH)/components/seeed-studio-round-display/example/main/seeed_studio_round_display_example.cpp \ $(PROJECT_PATH)/components/socket/example/main/socket_example.cpp \ + $(PROJECT_PATH)/components/spi/example/main/spi_example.cpp \ $(PROJECT_PATH)/components/st25dv/example/main/st25dv_example.cpp \ $(PROJECT_PATH)/components/state_machine/example/main/hfsm_example.cpp \ $(PROJECT_PATH)/components/tabulate/example/main/tabulate_example.cpp \ @@ -322,6 +323,7 @@ INPUT = \ $(PROJECT_PATH)/components/socket/include/socket.hpp \ $(PROJECT_PATH)/components/socket/include/udp_socket.hpp \ $(PROJECT_PATH)/components/socket/include/tcp_socket.hpp \ + $(PROJECT_PATH)/components/spi/include/spi.hpp \ $(PROJECT_PATH)/components/st25dv/include/st25dv.hpp \ $(PROJECT_PATH)/components/state_machine/include/deep_history_state.hpp \ $(PROJECT_PATH)/components/state_machine/include/shallow_history_state.hpp \ @@ -378,4 +380,3 @@ HAVE_DOT = NO GENERATE_LATEX = YES GENERATE_MAN = NO GENERATE_RTF = NO - diff --git a/doc/en/index.rst b/doc/en/index.rst index 5ffd2bb43..f967a65f6 100644 --- a/doc/en/index.rst +++ b/doc/en/index.rst @@ -66,6 +66,7 @@ This is the documentation for esp-idf c++ components, ESPP (`espp Date: Mon, 25 May 2026 23:16:10 -0500 Subject: [PATCH 03/12] update ci --- .github/workflows/build.yml | 2 ++ .github/workflows/upload_components.yml | 1 + 2 files changed, 3 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 82595040e..2a7b76dea 100755 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -183,6 +183,8 @@ jobs: target: esp32 - path: 'components/socket/example' target: esp32 + - path: 'components/spi/example' + target: esp32s3 - path: 'components/st25dv/example' target: esp32s3 - path: 'components/state_machine/example' diff --git a/.github/workflows/upload_components.yml b/.github/workflows/upload_components.yml index 7f2ef95a0..155705caa 100755 --- a/.github/workflows/upload_components.yml +++ b/.github/workflows/upload_components.yml @@ -112,6 +112,7 @@ jobs: components/seeed-studio-round-display components/serialization components/socket + components/spi components/st25dv components/state_machine components/t_keyboard From 2ceb1dc3e72dfafd9745ec5265068c4dd2fb980c Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Tue, 26 May 2026 16:52:54 -0500 Subject: [PATCH 04/12] remove spipanelio from the spi component --- components/spi/CMakeLists.txt | 2 +- components/spi/README.md | 15 --- components/spi/idf_component.yml | 1 - components/spi/include/spi.hpp | 99 +---------------- components/spi/src/spi.cpp | 180 ------------------------------- 5 files changed, 2 insertions(+), 295 deletions(-) diff --git a/components/spi/CMakeLists.txt b/components/spi/CMakeLists.txt index 432dcbcda..289418eae 100644 --- a/components/spi/CMakeLists.txt +++ b/components/spi/CMakeLists.txt @@ -1,5 +1,5 @@ idf_component_register( INCLUDE_DIRS "include" SRC_DIRS "src" - REQUIRES "base_component" "display_drivers" "esp_driver_gpio" "esp_driver_spi" "task" + REQUIRES "base_component" "esp_driver_gpio" "esp_driver_spi" "task" ) diff --git a/components/spi/README.md b/components/spi/README.md index 5df01a6a4..b27f82d23 100644 --- a/components/spi/README.md +++ b/components/spi/README.md @@ -6,21 +6,6 @@ The `Spi` class provides a simple C++ wrapper around the ESP-IDF SPI master driver. It manages SPI bus initialization, per-device registration, blocking transfers, queued transactions, and bus acquisition. -The component also provides `SpiPanelIo` (also aliased as `SpiCommandData`), a -helper for the common LCD / display pattern where a dedicated D/C pin selects -between command and data transactions while the pixel data is queued using DMA. - -`SpiPanelIo` treats command/data selection consistently: - -- `write_command()` queues the command byte with D/C low and automatically sends - any parameter bytes with D/C high. -- `queue_command()` queues a command transaction with D/C low. -- `queue_data()` and `queue_pixels()` queue payload transactions with D/C high. - -That means controller code should call `queue_command()` for register opcodes -and `queue_data()` / `queue_pixels()` for the corresponding payload without -manually OR-ing the D/C flag. - ## Example The [example](./example) shows how to create an SPI bus, attach a device, and diff --git a/components/spi/idf_component.yml b/components/spi/idf_component.yml index 322a4044b..21b97a95f 100644 --- a/components/spi/idf_component.yml +++ b/components/spi/idf_component.yml @@ -12,7 +12,6 @@ tags: - cpp - Component - SPI - - Display - DMA dependencies: idf: diff --git a/components/spi/include/spi.hpp b/components/spi/include/spi.hpp index c7866e9bc..8e4683098 100644 --- a/components/spi/include/spi.hpp +++ b/components/spi/include/spi.hpp @@ -13,15 +13,13 @@ #include #include "base_component.hpp" -#include "display_drivers.hpp" namespace espp { /// @brief SPI master wrapper and helpers. /// @details /// `Spi` owns a SPI bus and can create one or more attached `Device` instances. /// Those devices support blocking reads/writes, queued transactions, and bus -/// acquisition. `SpiPanelIo` builds on top of `Spi::Device` for queued -/// command/data display transports. +/// acquisition. /// /// \section spi_example Example /// \snippet spi_example.cpp spi example @@ -256,99 +254,4 @@ class Spi::Device : public BaseComponent { spi_device_handle_t handle_{nullptr}; std::recursive_mutex mutex_; }; - -/// @brief LCD-style command/data helper built on top of `Spi`. -/// @details -/// `queue_command()` transmits with D/C low. `queue_data()` and -/// `queue_pixels()` always transmit with D/C high, so callers should treat them -/// as payload helpers rather than manually setting the D/C bit for normal panel -/// data transactions. -class SpiPanelIo : public display_drivers::PanelIo, public BaseComponent { -public: - /// @brief IRQ-safe callback invoked after matching queued transactions finish. - using post_transaction_callback_t = void (*)(uint32_t user_flags); - - /// @brief Configuration for `SpiPanelIo`. - struct Config { - Spi *spi = nullptr; ///< Bus on which to register the display device. - Spi::DeviceConfig device_config{}; ///< SPI device configuration. - gpio_num_t data_command_io = GPIO_NUM_NC; ///< D/C GPIO pin. - uint32_t data_command_bit_mask = 0; ///< User bit that selects data mode. - uint32_t post_transaction_callback_bit_mask = 0; ///< User bit mask that triggers the callback. - post_transaction_callback_t post_transaction_callback = - nullptr; ///< Optional IRQ-safe callback. - uint32_t pixel_data_flags = - SPI_TRANS_DMA_BUFFER_ALIGN_MANUAL; ///< Flags used for pixel payload transfers. - uint32_t timeout_ms = 10; ///< Queue/result timeout in milliseconds. - Logger::Verbosity log_level = Logger::Verbosity::WARN; ///< Logger verbosity. - }; - - /// @brief Construct a panel-I/O helper for SPI command/data displays. - /// @param config Panel transport configuration. - explicit SpiPanelIo(const Config &config); - - /// @brief Check whether the helper has a registered SPI device. - /// @return True if initialization succeeded. - bool initialized() const override; - - /// @brief Get the underlying SPI device wrapper. - /// @return Shared pointer to the attached SPI device. - std::shared_ptr device() const; - - /// @brief Wait for all queued transactions to complete. - void wait() override; - - /// @brief Send a command byte and optional parameter payload. - /// @param command Command byte sent with D/C low. - /// @param parameters Optional payload bytes sent with D/C high. - /// @param user_flags Additional user-defined flags passed to callbacks. - void write_command(uint8_t command, std::span parameters, - uint32_t user_flags = 0) override; - - /// @brief Queue a command transaction with D/C low. - /// @param command Command byte to transmit. - /// @param user_flags Additional user-defined flags passed to callbacks. - void queue_command(uint8_t command, uint32_t user_flags = 0) override; - - /// @brief Queue a non-pixel data payload with D/C high. - /// @param data Data bytes to transmit. - /// @param user_flags Additional user-defined flags passed to callbacks. - void queue_data(std::span data, uint32_t user_flags = 0) override; - - /// @brief Queue a pixel payload with D/C high. - /// @param data Pointer to the pixel buffer. - /// @param size Pixel payload size in bytes. - /// @param user_flags Additional user-defined flags passed to callbacks. - /// @param transaction_flags Optional SPI transaction flags overriding the default pixel flags. - void queue_pixels(const uint8_t *data, size_t size, uint32_t user_flags = 0, - uint32_t transaction_flags = 0) override; - -private: - struct TransactionContext { - SpiPanelIo *helper{nullptr}; - uint32_t user_flags{0}; - }; - - static void pre_transfer_callback(spi_transaction_t *transaction); - static void post_transfer_callback(spi_transaction_t *transaction); - - TickType_t timeout_ticks() const; - size_t prepare_transaction(uint32_t user_flags); - void queue_command_locked(uint8_t command, uint32_t user_flags); - void queue_data_locked(std::span data, uint32_t user_flags); - void queue_pixels_locked(const uint8_t *data, size_t size, uint32_t user_flags, - uint32_t transaction_flags); - void queue_transaction_locked(size_t index); - void wait_locked(); - - Config config_; - std::shared_ptr device_{}; - std::mutex mutex_; - std::vector transactions_{}; - std::vector contexts_{}; - int queued_transactions_{0}; -}; - -/// @brief Backward-compatible alias for the older SPI display transport name. -using SpiCommandData = SpiPanelIo; } // namespace espp diff --git a/components/spi/src/spi.cpp b/components/spi/src/spi.cpp index c28631a0e..0400a7589 100644 --- a/components/spi/src/spi.cpp +++ b/components/spi/src/spi.cpp @@ -1,6 +1,5 @@ #include "spi.hpp" -#include #include #include @@ -340,183 +339,4 @@ bool Spi::Device::transfer(std::span tx_data, std::span ec.clear(); return true; } - -SpiPanelIo::SpiPanelIo(const Config &config) - : BaseComponent("SPI Panel IO", config.log_level) - , config_(config) { - auto queue_depth = static_cast(std::max(2, config_.device_config.queue_size)); - transactions_.resize(queue_depth); - contexts_.resize(queue_depth); - if (!config_.spi) { - logger_.error("missing SPI bus"); - return; - } - if (config_.data_command_io != GPIO_NUM_NC) { - gpio_set_direction(config_.data_command_io, GPIO_MODE_OUTPUT); - gpio_set_level(config_.data_command_io, 0); - } - auto device_config = config_.device_config; - device_config.pre_cb = &SpiPanelIo::pre_transfer_callback; - device_config.post_cb = &SpiPanelIo::post_transfer_callback; - std::error_code ec; - device_ = config_.spi->add_device(device_config, ec); - if (ec || !device_) { - logger_.error("could not initialize SPI command/data device"); - } -} - -bool SpiPanelIo::initialized() const { return static_cast(device_); } - -std::shared_ptr SpiPanelIo::device() const { return device_; } - -void SpiPanelIo::wait() { - std::lock_guard lock(mutex_); - wait_locked(); -} - -void SpiPanelIo::write_command(uint8_t command, std::span parameters, - uint32_t user_flags) { - std::lock_guard lock(mutex_); - if (!device_) { - logger_.error("device not initialized"); - return; - } - wait_locked(); - - queue_command_locked(command, user_flags); - - if (parameters.empty()) { - wait_locked(); - return; - } - - queue_data_locked(parameters, user_flags | config_.data_command_bit_mask); - wait_locked(); -} - -void SpiPanelIo::queue_command(uint8_t command, uint32_t user_flags) { - std::lock_guard lock(mutex_); - if (!device_) { - logger_.error("device not initialized"); - return; - } - queue_command_locked(command, user_flags); -} - -void SpiPanelIo::queue_data(std::span data, uint32_t user_flags) { - std::lock_guard lock(mutex_); - if (!device_) { - logger_.error("device not initialized"); - return; - } - if (data.empty()) { - return; - } - queue_data_locked(data, user_flags | config_.data_command_bit_mask); -} - -void SpiPanelIo::queue_pixels(const uint8_t *data, size_t size, uint32_t user_flags, - uint32_t transaction_flags) { - std::lock_guard lock(mutex_); - if (!device_) { - logger_.error("device not initialized"); - return; - } - if (!data || size == 0) { - logger_.error("cannot queue null or empty pixel data"); - return; - } - queue_pixels_locked(data, size, config_.data_command_bit_mask | user_flags, transaction_flags); -} - -void SpiPanelIo::pre_transfer_callback(spi_transaction_t *transaction) { - auto *context = static_cast(transaction->user); - if (!context || !context->helper) { - return; - } - if (context->helper->config_.data_command_io == GPIO_NUM_NC) { - return; - } - bool dc_level = (context->user_flags & context->helper->config_.data_command_bit_mask) != 0; - gpio_set_level(context->helper->config_.data_command_io, dc_level); -} - -void SpiPanelIo::post_transfer_callback(spi_transaction_t *transaction) { - auto *context = static_cast(transaction->user); - if (!context || !context->helper) { - return; - } - if (!context->helper->config_.post_transaction_callback) { - return; - } - if (context->helper->config_.post_transaction_callback_bit_mask != 0 && - (context->user_flags & context->helper->config_.post_transaction_callback_bit_mask) == 0) { - return; - } - context->helper->config_.post_transaction_callback(context->user_flags); -} - -TickType_t SpiPanelIo::timeout_ticks() const { return pdMS_TO_TICKS(config_.timeout_ms); } - -size_t SpiPanelIo::prepare_transaction(uint32_t user_flags) { - if (queued_transactions_ >= static_cast(transactions_.size())) { - wait_locked(); - } - auto index = static_cast(queued_transactions_); - std::memset(&transactions_[index], 0, sizeof(transactions_[index])); - contexts_[index] = {.helper = this, .user_flags = user_flags}; - transactions_[index].user = &contexts_[index]; - return index; -} - -void SpiPanelIo::queue_command_locked(uint8_t command, uint32_t user_flags) { - auto index = prepare_transaction(user_flags); - transactions_[index].length = 8; - transactions_[index].flags = SPI_TRANS_USE_TXDATA; - transactions_[index].tx_data[0] = command; - queue_transaction_locked(index); -} - -void SpiPanelIo::queue_data_locked(std::span data, uint32_t user_flags) { - auto index = prepare_transaction(user_flags); - transactions_[index].length = data.size() * 8; - if (data.size() <= sizeof(transactions_[index].tx_data)) { - std::memcpy(transactions_[index].tx_data, data.data(), data.size()); - transactions_[index].flags = SPI_TRANS_USE_TXDATA; - } else { - transactions_[index].tx_buffer = data.data(); - } - queue_transaction_locked(index); -} - -void SpiPanelIo::queue_pixels_locked(const uint8_t *data, size_t size, uint32_t user_flags, - uint32_t transaction_flags) { - auto index = prepare_transaction(user_flags); - transactions_[index].length = size * 8; - transactions_[index].flags = - transaction_flags != 0 ? transaction_flags : config_.pixel_data_flags; - transactions_[index].tx_buffer = data; - queue_transaction_locked(index); -} - -void SpiPanelIo::queue_transaction_locked(size_t index) { - std::error_code ec; - if (device_->queue_transaction(transactions_[index], timeout_ticks(), ec)) { - queued_transactions_++; - return; - } - logger_.error("could not queue SPI command/data transaction"); -} - -void SpiPanelIo::wait_locked() { - spi_transaction_t *completed = nullptr; - while (queued_transactions_ > 0) { - std::error_code ec; - if (!device_->get_transaction_result(&completed, timeout_ticks(), ec)) { - logger_.error("could not get queued SPI transaction result"); - } - (void)completed; - queued_transactions_--; - } -} } // namespace espp From 1e37297f3bd5bcb95c05d1a2d3b99ab31959e0ee Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Tue, 26 May 2026 16:57:33 -0500 Subject: [PATCH 05/12] fix comments --- components/spi/example/main/spi_example.cpp | 2 ++ components/spi/src/spi.cpp | 2 +- doc/en/spi.rst | 6 +++--- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/components/spi/example/main/spi_example.cpp b/components/spi/example/main/spi_example.cpp index 9331af230..cd2c6cba4 100644 --- a/components/spi/example/main/spi_example.cpp +++ b/components/spi/example/main/spi_example.cpp @@ -4,6 +4,7 @@ #include "spi.hpp" extern "C" void app_main(void) { + //! [spi example] constexpr spi_host_device_t spi_host = SPI2_HOST; constexpr auto sclk_gpio = GPIO_NUM_36; constexpr auto miso_gpio = GPIO_NUM_35; @@ -40,4 +41,5 @@ extern "C" void app_main(void) { } std::printf("Read bytes: 0x%02x 0x%02x\n", rx_data[0], rx_data[1]); + //! [spi example] } diff --git a/components/spi/src/spi.cpp b/components/spi/src/spi.cpp index 0400a7589..ca549d0a7 100644 --- a/components/spi/src/spi.cpp +++ b/components/spi/src/spi.cpp @@ -123,6 +123,7 @@ void Spi::deinit(std::error_code &ec) { devices.push_back(device); } } + initialized_ = false; devices_.clear(); } @@ -141,7 +142,6 @@ void Spi::deinit(std::error_code &ec) { ec = std::make_error_code(std::errc::io_error); return; } - initialized_ = false; ec.clear(); } diff --git a/doc/en/spi.rst b/doc/en/spi.rst index 6037422f5..09e4d2eff 100644 --- a/doc/en/spi.rst +++ b/doc/en/spi.rst @@ -3,9 +3,9 @@ SPI The `spi` component provides a C++ wrapper around ESP-IDF's SPI master driver. -`Spi` owns the bus, `Spi::Device` owns attached peripherals on that bus, and -`SpiCommandData` provides a higher-level helper for LCD-style command/data -transports that queue transactions and toggle a D/C GPIO in the SPI callbacks. +`Spi` owns the bus and `Spi::Device` owns attached peripherals on that bus. +Display-specific command/data helpers such as `SpiPanelIo` / `SpiCommandData` +now live in the `display_drivers` component. .. toctree:: From 5baab012fa4ebe03bdcc4a5c0f34e2125b135d7c Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Wed, 27 May 2026 10:08:51 -0500 Subject: [PATCH 06/12] address comments on spi component --- components/spi/include/spi.hpp | 6 ++++-- components/spi/src/spi.cpp | 27 ++++++++++++++++++++++++--- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/components/spi/include/spi.hpp b/components/spi/include/spi.hpp index 8e4683098..f43b2da15 100644 --- a/components/spi/include/spi.hpp +++ b/components/spi/include/spi.hpp @@ -150,10 +150,12 @@ class Spi : public BaseComponent { std::shared_ptr add_device(const DeviceConfig &config, std::error_code &ec); private: + void prune_expired_devices_locked(); + friend class Device; Config config_; - std::recursive_mutex mutex_; + mutable std::recursive_mutex mutex_; bool initialized_{false}; std::vector> devices_; }; @@ -252,6 +254,6 @@ class Spi::Device : public BaseComponent { Spi &spi_; DeviceConfig config_; spi_device_handle_t handle_{nullptr}; - std::recursive_mutex mutex_; + mutable std::recursive_mutex mutex_; }; } // namespace espp diff --git a/components/spi/src/spi.cpp b/components/spi/src/spi.cpp index ca549d0a7..2d957f0cd 100644 --- a/components/spi/src/spi.cpp +++ b/components/spi/src/spi.cpp @@ -1,5 +1,6 @@ #include "spi.hpp" +#include #include #include @@ -118,6 +119,7 @@ void Spi::deinit(std::error_code &ec) { ec.clear(); return; } + prune_expired_devices_locked(); for (auto &weak_device : devices_) { if (auto device = weak_device.lock()) { devices.push_back(device); @@ -145,7 +147,10 @@ void Spi::deinit(std::error_code &ec) { ec.clear(); } -bool Spi::initialized() const { return initialized_; } +bool Spi::initialized() const { + std::lock_guard lock(mutex_); + return initialized_; +} spi_host_device_t Spi::host() const { return config_.host; } @@ -156,6 +161,7 @@ std::shared_ptr Spi::add_device(const DeviceConfig &config, std::er ec = std::make_error_code(std::errc::not_connected); return nullptr; } + prune_expired_devices_locked(); auto device = std::make_shared(*this, config); auto device_config = make_device_config(config); auto err = spi_bus_add_device(this->config_.host, &device_config, &device->handle_); @@ -169,6 +175,10 @@ std::shared_ptr Spi::add_device(const DeviceConfig &config, std::er return device; } +void Spi::prune_expired_devices_locked() { + std::erase_if(devices_, [](const auto &device) { return device.expired(); }); +} + Spi::Device::Device(Spi &spi, const DeviceConfig &config) : BaseComponent("SPI Device", spi.get_log_level()) , spi_(spi) @@ -182,9 +192,15 @@ Spi::Device::~Device() { } } -bool Spi::Device::initialized() const { return handle_ != nullptr; } +bool Spi::Device::initialized() const { + std::lock_guard lock(mutex_); + return handle_ != nullptr; +} -spi_device_handle_t Spi::Device::handle() const { return handle_; } +spi_device_handle_t Spi::Device::handle() const { + std::lock_guard lock(mutex_); + return handle_; +} const Spi::DeviceConfig &Spi::Device::config() const { return config_; } @@ -303,6 +319,11 @@ bool Spi::Device::read(std::span data, const TransactionConfig &config, bool Spi::Device::transfer(std::span tx_data, std::span rx_data, const TransactionConfig &config, std::error_code &ec) { + if (tx_data.empty() && rx_data.empty()) { + ec.clear(); + return true; + } + spi_transaction_t transaction{}; transaction.cmd = config.command; transaction.addr = config.address; From 0f259b448aaef7a9b614df89aab778e337861570 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Wed, 27 May 2026 10:21:30 -0500 Subject: [PATCH 07/12] enforce valid rx/tx lengths --- components/spi/include/spi.hpp | 4 +++- components/spi/src/spi.cpp | 21 ++++++++++++++++++--- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/components/spi/include/spi.hpp b/components/spi/include/spi.hpp index f43b2da15..c627f27e8 100644 --- a/components/spi/include/spi.hpp +++ b/components/spi/include/spi.hpp @@ -64,7 +64,9 @@ class Spi : public BaseComponent { uint16_t command = 0; ///< Command value for this transaction. uint64_t address = 0; ///< Address value for this transaction. uint32_t flags = 0; ///< ESP-IDF transaction flags. - size_t rx_length_bits = 0; ///< Optional receive length override, in bits. + size_t rx_length_bits = 0; ///< Optional receive length override, in bits. Must not exceed the + ///< RX buffer capacity, or the TX bit length when both TX and RX are + ///< used in the same transaction. }; class Device; diff --git a/components/spi/src/spi.cpp b/components/spi/src/spi.cpp index 2d957f0cd..41d3ad0ce 100644 --- a/components/spi/src/spi.cpp +++ b/components/spi/src/spi.cpp @@ -324,13 +324,28 @@ bool Spi::Device::transfer(std::span tx_data, std::span return true; } + const size_t tx_length_bits = tx_data.size() * 8; + const size_t rx_buffer_bits = rx_data.size() * 8; + const size_t rx_length_bits = config.rx_length_bits ? config.rx_length_bits : rx_buffer_bits; + + if (!rx_data.empty() && rx_length_bits > rx_buffer_bits) { + logger_.error("requested RX length exceeds RX buffer capacity"); + ec = std::make_error_code(std::errc::invalid_argument); + return false; + } + if (!tx_data.empty() && !rx_data.empty() && rx_length_bits > tx_length_bits) { + logger_.error("requested RX length exceeds TX clock length"); + ec = std::make_error_code(std::errc::invalid_argument); + return false; + } + spi_transaction_t transaction{}; transaction.cmd = config.command; transaction.addr = config.address; transaction.flags = config.flags; if (!tx_data.empty()) { - transaction.length = tx_data.size() * 8; + transaction.length = tx_length_bits; if (tx_data.size() <= sizeof(transaction.tx_data)) { std::memcpy(transaction.tx_data, tx_data.data(), tx_data.size()); transaction.flags |= SPI_TRANS_USE_TXDATA; @@ -338,11 +353,11 @@ bool Spi::Device::transfer(std::span tx_data, std::span transaction.tx_buffer = tx_data.data(); } } else if (!rx_data.empty()) { - transaction.length = rx_data.size() * 8; + transaction.length = rx_length_bits; } if (!rx_data.empty()) { - transaction.rxlength = config.rx_length_bits ? config.rx_length_bits : rx_data.size() * 8; + transaction.rxlength = rx_length_bits; if (rx_data.size() <= sizeof(transaction.rx_data)) { transaction.flags |= SPI_TRANS_USE_RXDATA; } else { From a9bf72e8b24f6695a56646e882dc53e28883fafa Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Wed, 27 May 2026 15:38:46 -0500 Subject: [PATCH 08/12] address comments --- components/spi/example/sdkconfig.defaults | 4 ++++ components/spi/include/spi.hpp | 9 +++++---- components/spi/src/spi.cpp | 9 ++++++++- 3 files changed, 17 insertions(+), 5 deletions(-) create mode 100644 components/spi/example/sdkconfig.defaults diff --git a/components/spi/example/sdkconfig.defaults b/components/spi/example/sdkconfig.defaults new file mode 100644 index 000000000..c3667f3e3 --- /dev/null +++ b/components/spi/example/sdkconfig.defaults @@ -0,0 +1,4 @@ +# Common ESP-related +# +CONFIG_ESP_SYSTEM_EVENT_TASK_STACK_SIZE=4096 +CONFIG_ESP_MAIN_TASK_STACK_SIZE=8192 diff --git a/components/spi/include/spi.hpp b/components/spi/include/spi.hpp index c627f27e8..829b6a500 100644 --- a/components/spi/include/spi.hpp +++ b/components/spi/include/spi.hpp @@ -17,7 +17,7 @@ namespace espp { /// @brief SPI master wrapper and helpers. /// @details -/// `Spi` owns a SPI bus and can create one or more attached `Device` instances. +/// `Spi` owns an SPI bus and can create one or more attached `Device` instances. /// Those devices support blocking reads/writes, queued transactions, and bus /// acquisition. /// @@ -64,9 +64,10 @@ class Spi : public BaseComponent { uint16_t command = 0; ///< Command value for this transaction. uint64_t address = 0; ///< Address value for this transaction. uint32_t flags = 0; ///< ESP-IDF transaction flags. - size_t rx_length_bits = 0; ///< Optional receive length override, in bits. Must not exceed the - ///< RX buffer capacity, or the TX bit length when both TX and RX are - ///< used in the same transaction. + size_t rx_length_bits = 0; ///< Optional receive length override, in bits. For this + ///< byte-oriented API it must be a multiple of 8, must not exceed + ///< the RX buffer capacity, and must not exceed the TX bit length + ///< when both TX and RX are used in the same transaction. }; class Device; diff --git a/components/spi/src/spi.cpp b/components/spi/src/spi.cpp index 41d3ad0ce..3777127f4 100644 --- a/components/spi/src/spi.cpp +++ b/components/spi/src/spi.cpp @@ -327,6 +327,13 @@ bool Spi::Device::transfer(std::span tx_data, std::span const size_t tx_length_bits = tx_data.size() * 8; const size_t rx_buffer_bits = rx_data.size() * 8; const size_t rx_length_bits = config.rx_length_bits ? config.rx_length_bits : rx_buffer_bits; + const size_t rx_length_bytes = rx_length_bits / 8; + + if (!rx_data.empty() && (rx_length_bits % 8) != 0) { + logger_.error("requested RX length must be a multiple of 8 bits"); + ec = std::make_error_code(std::errc::invalid_argument); + return false; + } if (!rx_data.empty() && rx_length_bits > rx_buffer_bits) { logger_.error("requested RX length exceeds RX buffer capacity"); @@ -370,7 +377,7 @@ bool Spi::Device::transfer(std::span tx_data, std::span } if (!rx_data.empty() && rx_data.size() <= sizeof(transaction.rx_data)) { - std::memcpy(rx_data.data(), transaction.rx_data, rx_data.size()); + std::memcpy(rx_data.data(), transaction.rx_data, rx_length_bytes); } ec.clear(); return true; From 0b759c288217ccf7c40f9ad991ac65d22f3307ec Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Wed, 27 May 2026 16:29:40 -0500 Subject: [PATCH 09/12] address comments --- components/spi/include/spi.hpp | 1 + components/spi/src/spi.cpp | 31 +++++++++++++++++++++++++++++-- doc/en/spi.rst | 2 -- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/components/spi/include/spi.hpp b/components/spi/include/spi.hpp index 829b6a500..4d1d524e1 100644 --- a/components/spi/include/spi.hpp +++ b/components/spi/include/spi.hpp @@ -160,6 +160,7 @@ class Spi : public BaseComponent { Config config_; mutable std::recursive_mutex mutex_; bool initialized_{false}; + bool deinitializing_{false}; std::vector> devices_; }; diff --git a/components/spi/src/spi.cpp b/components/spi/src/spi.cpp index 3777127f4..e63137435 100644 --- a/components/spi/src/spi.cpp +++ b/components/spi/src/spi.cpp @@ -91,6 +91,11 @@ spi_device_interface_config_t Spi::make_device_config(const DeviceConfig &config void Spi::init(std::error_code &ec) { std::lock_guard lock(mutex_); + if (deinitializing_) { + logger_.warn("cannot initialize while deinitialization is in progress"); + ec = std::make_error_code(std::errc::device_or_resource_busy); + return; + } if (initialized_) { logger_.warn("already initialized"); ec = std::make_error_code(std::errc::protocol_error); @@ -113,20 +118,25 @@ void Spi::init(std::error_code &ec) { void Spi::deinit(std::error_code &ec) { std::vector> devices; + bool removal_failed = false; { std::lock_guard lock(mutex_); if (!initialized_) { ec.clear(); return; } + if (deinitializing_) { + logger_.warn("deinitialization already in progress"); + ec = std::make_error_code(std::errc::device_or_resource_busy); + return; + } + deinitializing_ = true; prune_expired_devices_locked(); for (auto &weak_device : devices_) { if (auto device = weak_device.lock()) { devices.push_back(device); } } - initialized_ = false; - devices_.clear(); } for (auto &device : devices) { @@ -134,16 +144,28 @@ void Spi::deinit(std::error_code &ec) { device->remove_device(device_ec); if (device_ec) { logger_.error("could not remove SPI device before bus free"); + removal_failed = true; } } + if (removal_failed) { + std::lock_guard lock(mutex_); + deinitializing_ = false; + ec = std::make_error_code(std::errc::io_error); + return; + } + std::lock_guard lock(mutex_); auto err = spi_bus_free(config_.host); if (err != ESP_OK) { logger_.error("could not free SPI bus: {}", esp_err_to_name(err)); + deinitializing_ = false; ec = std::make_error_code(std::errc::io_error); return; } + devices_.clear(); + initialized_ = false; + deinitializing_ = false; ec.clear(); } @@ -156,6 +178,11 @@ spi_host_device_t Spi::host() const { return config_.host; } std::shared_ptr Spi::add_device(const DeviceConfig &config, std::error_code &ec) { std::lock_guard lock(mutex_); + if (deinitializing_) { + logger_.error("bus is deinitializing"); + ec = std::make_error_code(std::errc::device_or_resource_busy); + return nullptr; + } if (!initialized_) { logger_.error("bus not initialized"); ec = std::make_error_code(std::errc::not_connected); diff --git a/doc/en/spi.rst b/doc/en/spi.rst index 09e4d2eff..623ea9b5a 100644 --- a/doc/en/spi.rst +++ b/doc/en/spi.rst @@ -4,8 +4,6 @@ SPI The `spi` component provides a C++ wrapper around ESP-IDF's SPI master driver. `Spi` owns the bus and `Spi::Device` owns attached peripherals on that bus. -Display-specific command/data helpers such as `SpiPanelIo` / `SpiCommandData` -now live in the `display_drivers` component. .. toctree:: From 9c7e8a69b3bd78f09cee4eec5c810eaa1010e9b6 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Wed, 27 May 2026 16:42:20 -0500 Subject: [PATCH 10/12] fix buslock --- components/spi/include/spi.hpp | 12 ++++--- components/spi/src/spi.cpp | 63 +++++++++++++++++++++++++++------- 2 files changed, 57 insertions(+), 18 deletions(-) diff --git a/components/spi/include/spi.hpp b/components/spi/include/spi.hpp index 4d1d524e1..111c0e586 100644 --- a/components/spi/include/spi.hpp +++ b/components/spi/include/spi.hpp @@ -71,6 +71,7 @@ class Spi : public BaseComponent { }; class Device; + struct BusLockState; /// @brief RAII helper for holding an acquired SPI device bus lock. class BusLock { @@ -78,10 +79,6 @@ class Spi : public BaseComponent { /// @brief Construct an empty bus lock. BusLock(); - /// @brief Construct a bus lock from an acquired ESP-IDF SPI handle. - /// @param handle Acquired device handle. - explicit BusLock(spi_device_handle_t handle); - BusLock(const BusLock &) = delete; BusLock &operator=(const BusLock &) = delete; @@ -105,7 +102,11 @@ class Spi : public BaseComponent { explicit operator bool() const; private: - spi_device_handle_t handle_{nullptr}; + explicit BusLock(std::shared_ptr state); + + std::shared_ptr state_{}; + + friend class Device; }; /// @brief Construct the SPI bus wrapper. @@ -258,6 +259,7 @@ class Spi::Device : public BaseComponent { Spi &spi_; DeviceConfig config_; spi_device_handle_t handle_{nullptr}; + std::shared_ptr bus_lock_state_{}; mutable std::recursive_mutex mutex_; }; } // namespace espp diff --git a/components/spi/src/spi.cpp b/components/spi/src/spi.cpp index e63137435..434d46d86 100644 --- a/components/spi/src/spi.cpp +++ b/components/spi/src/spi.cpp @@ -2,41 +2,55 @@ #include #include +#include #include #include "run_on_core.hpp" namespace espp { +struct Spi::BusLockState { + std::mutex mutex; + spi_device_handle_t handle{nullptr}; + bool lock_held{false}; +}; + Spi::BusLock::BusLock() = default; -Spi::BusLock::BusLock(spi_device_handle_t handle) - : handle_(handle) {} +Spi::BusLock::BusLock(std::shared_ptr state) + : state_(std::move(state)) {} Spi::BusLock::BusLock(BusLock &&other) noexcept - : handle_(other.handle_) { - other.handle_ = nullptr; -} + : state_(std::move(other.state_)) {} Spi::BusLock &Spi::BusLock::operator=(BusLock &&other) noexcept { if (this == &other) { return *this; } release(); - handle_ = other.handle_; - other.handle_ = nullptr; + state_ = std::move(other.state_); return *this; } Spi::BusLock::~BusLock() { release(); } void Spi::BusLock::release() { - if (handle_) { - spi_device_release_bus(handle_); - handle_ = nullptr; + auto state = std::move(state_); + if (state) { + std::lock_guard lock(state->mutex); + if (state->lock_held && state->handle) { + spi_device_release_bus(state->handle); + state->lock_held = false; + } } } -Spi::BusLock::operator bool() const { return handle_ != nullptr; } +Spi::BusLock::operator bool() const { + if (!state_) { + return false; + } + std::lock_guard lock(state_->mutex); + return state_->lock_held && state_->handle != nullptr; +} Spi::Spi(const Config &config) : BaseComponent("SPI", config.log_level) @@ -197,6 +211,11 @@ std::shared_ptr Spi::add_device(const DeviceConfig &config, std::er ec = std::make_error_code(std::errc::io_error); return nullptr; } + { + std::lock_guard bus_lock_state_lock(device->bus_lock_state_->mutex); + device->bus_lock_state_->handle = device->handle_; + device->bus_lock_state_->lock_held = false; + } devices_.push_back(device); ec.clear(); return device; @@ -209,7 +228,8 @@ void Spi::prune_expired_devices_locked() { Spi::Device::Device(Spi &spi, const DeviceConfig &config) : BaseComponent("SPI Device", spi.get_log_level()) , spi_(spi) - , config_(config) {} + , config_(config) + , bus_lock_state_(std::make_shared()) {} Spi::Device::~Device() { std::error_code ec; @@ -237,12 +257,24 @@ void Spi::Device::remove_device(std::error_code &ec) { ec.clear(); return; } + { + std::lock_guard bus_lock_state_lock(bus_lock_state_->mutex); + if (bus_lock_state_->lock_held && bus_lock_state_->handle == handle_) { + spi_device_release_bus(handle_); + bus_lock_state_->lock_held = false; + } + } auto err = spi_bus_remove_device(handle_); if (err != ESP_OK) { logger_.error("could not remove SPI device: {}", esp_err_to_name(err)); ec = std::make_error_code(std::errc::io_error); return; } + { + std::lock_guard bus_lock_state_lock(bus_lock_state_->mutex); + bus_lock_state_->handle = nullptr; + bus_lock_state_->lock_held = false; + } handle_ = nullptr; ec.clear(); } @@ -330,8 +362,13 @@ Spi::BusLock Spi::Device::acquire_bus(TickType_t timeout, std::error_code &ec) { ec = std::make_error_code(std::errc::io_error); return {}; } + { + std::lock_guard bus_lock_state_lock(bus_lock_state_->mutex); + bus_lock_state_->handle = handle_; + bus_lock_state_->lock_held = true; + } ec.clear(); - return BusLock(handle_); + return BusLock(bus_lock_state_); } bool Spi::Device::write(std::span data, const TransactionConfig &config, From e20cd913ac8c43747960bc742d905b3473a27d0e Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Wed, 27 May 2026 17:04:29 -0500 Subject: [PATCH 11/12] address comments --- components/spi/README.md | 3 ++- components/spi/example/README.md | 3 ++- components/spi/example/main/spi_example.cpp | 3 ++- components/spi/include/spi.hpp | 5 ++--- components/spi/src/spi.cpp | 7 +++---- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/components/spi/README.md b/components/spi/README.md index b27f82d23..e33663663 100644 --- a/components/spi/README.md +++ b/components/spi/README.md @@ -9,4 +9,5 @@ transfers, queued transactions, and bus acquisition. ## Example The [example](./example) shows how to create an SPI bus, attach a device, and -perform a simple addressed read transaction. +perform a simple addressed read transaction, with the address phase driven on +MOSI and the response read back on MISO. diff --git a/components/spi/example/README.md b/components/spi/example/README.md index 94e67546d..5ec95495a 100644 --- a/components/spi/example/README.md +++ b/components/spi/example/README.md @@ -6,4 +6,5 @@ device, and perform a simple addressed read transaction. ## How to use example The example is intended primarily as a compile-time reference for the `Spi` -API. Update the GPIO assignments to match your hardware before flashing. +API. Update the GPIO assignments to match your hardware before flashing; the +address phase is driven on MOSI and the response is read on MISO. diff --git a/components/spi/example/main/spi_example.cpp b/components/spi/example/main/spi_example.cpp index cd2c6cba4..9a874dfaa 100644 --- a/components/spi/example/main/spi_example.cpp +++ b/components/spi/example/main/spi_example.cpp @@ -7,6 +7,7 @@ extern "C" void app_main(void) { //! [spi example] constexpr spi_host_device_t spi_host = SPI2_HOST; constexpr auto sclk_gpio = GPIO_NUM_36; + constexpr auto mosi_gpio = GPIO_NUM_38; constexpr auto miso_gpio = GPIO_NUM_35; constexpr auto cs_gpio = GPIO_NUM_37; constexpr uint8_t read_address = 0x80; @@ -14,7 +15,7 @@ extern "C" void app_main(void) { espp::Spi spi({ .host = spi_host, .sclk_io_num = sclk_gpio, - .mosi_io_num = GPIO_NUM_NC, + .mosi_io_num = mosi_gpio, .miso_io_num = miso_gpio, .max_transfer_sz = 16, }); diff --git a/components/spi/include/spi.hpp b/components/spi/include/spi.hpp index 111c0e586..0db5eed67 100644 --- a/components/spi/include/spi.hpp +++ b/components/spi/include/spi.hpp @@ -169,9 +169,9 @@ class Spi : public BaseComponent { class Spi::Device : public BaseComponent { public: /// @brief Construct an unattached device wrapper. - /// @param spi Owning SPI bus wrapper. + /// @param log_level Logger verbosity inherited from the owning SPI bus. /// @param config Device configuration. - explicit Device(Spi &spi, const DeviceConfig &config); + explicit Device(Logger::Verbosity log_level, const DeviceConfig &config); /// @brief Remove the device from the SPI bus on destruction. ~Device(); @@ -256,7 +256,6 @@ class Spi::Device : public BaseComponent { private: friend class Spi; - Spi &spi_; DeviceConfig config_; spi_device_handle_t handle_{nullptr}; std::shared_ptr bus_lock_state_{}; diff --git a/components/spi/src/spi.cpp b/components/spi/src/spi.cpp index 434d46d86..6de44f9e5 100644 --- a/components/spi/src/spi.cpp +++ b/components/spi/src/spi.cpp @@ -203,7 +203,7 @@ std::shared_ptr Spi::add_device(const DeviceConfig &config, std::er return nullptr; } prune_expired_devices_locked(); - auto device = std::make_shared(*this, config); + auto device = std::make_shared(get_log_level(), config); auto device_config = make_device_config(config); auto err = spi_bus_add_device(this->config_.host, &device_config, &device->handle_); if (err != ESP_OK) { @@ -225,9 +225,8 @@ void Spi::prune_expired_devices_locked() { std::erase_if(devices_, [](const auto &device) { return device.expired(); }); } -Spi::Device::Device(Spi &spi, const DeviceConfig &config) - : BaseComponent("SPI Device", spi.get_log_level()) - , spi_(spi) +Spi::Device::Device(Logger::Verbosity log_level, const DeviceConfig &config) + : BaseComponent("SPI Device", log_level) , config_(config) , bus_lock_state_(std::make_shared()) {} From 38e38ac63776e862ec5bd586ab4d92398404d3d4 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Wed, 27 May 2026 22:28:18 -0500 Subject: [PATCH 12/12] Potential fix for pull request finding Ensure the TXDATA and RXDATA spi transaction flags are ignored and only set by this transfer call Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- components/spi/src/spi.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/components/spi/src/spi.cpp b/components/spi/src/spi.cpp index 6de44f9e5..bb0226ef4 100644 --- a/components/spi/src/spi.cpp +++ b/components/spi/src/spi.cpp @@ -412,7 +412,8 @@ bool Spi::Device::transfer(std::span tx_data, std::span spi_transaction_t transaction{}; transaction.cmd = config.command; transaction.addr = config.address; - transaction.flags = config.flags; + const auto managed_buffer_flags = SPI_TRANS_USE_TXDATA | SPI_TRANS_USE_RXDATA; + transaction.flags = config.flags & ~managed_buffer_flags; if (!tx_data.empty()) { transaction.length = tx_length_bits;