From 00d45c9ba07df411c080f18adcfe3f76a06fa240 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Tue, 7 Oct 2025 16:10:04 -0500 Subject: [PATCH 1/3] fix(pi4ioe5v): Fix component implementation to be correct --- .../example/main/pi4ioe5v_example.cpp | 90 ++- components/pi4ioe5v/include/pi4ioe5v.hpp | 514 ++++++++++++++---- 2 files changed, 461 insertions(+), 143 deletions(-) diff --git a/components/pi4ioe5v/example/main/pi4ioe5v_example.cpp b/components/pi4ioe5v/example/main/pi4ioe5v_example.cpp index 150311356..135385cd7 100644 --- a/components/pi4ioe5v/example/main/pi4ioe5v_example.cpp +++ b/components/pi4ioe5v/example/main/pi4ioe5v_example.cpp @@ -2,65 +2,99 @@ #include #include "i2c.hpp" +#include "logger.hpp" #include "pi4ioe5v.hpp" #include "task.hpp" using namespace std::chrono_literals; extern "C" void app_main(void) { - fmt::print("Starting PI4IOE5V example...\n"); + + espp::Logger logger({.tag = "PI4IOE5V example", .level = espp::Logger::Verbosity::INFO}); + + logger.info("Starting PI4IOE5V example..."); //! [pi4ioe5v_example] + // make the I2C that we'll use to communicate espp::I2c i2c({ .port = I2C_NUM_1, .sda_io_num = (gpio_num_t)CONFIG_EXAMPLE_I2C_SDA_GPIO, .scl_io_num = (gpio_num_t)CONFIG_EXAMPLE_I2C_SCL_GPIO, + .sda_pullup_en = GPIO_PULLUP_ENABLE, + .scl_pullup_en = GPIO_PULLUP_ENABLE, }); - // Configure PI4IOE5V: single 8-bit port as outputs + // Configure PI4IOE5V: 8-bit port with mixed inputs/outputs espp::Pi4ioe5v exp({ .device_address = espp::Pi4ioe5v::DEFAULT_ADDRESS, - .direction_mask = 0x00, // all outputs - .output_state = 0xFF, // initial state - .probe = std::bind(&espp::I2c::probe_device, &i2c, std::placeholders::_1), - .write = std::bind(&espp::I2c::write, &i2c, std::placeholders::_1, std::placeholders::_2, - std::placeholders::_3), - .read_register = - std::bind(&espp::I2c::read_at_register, &i2c, std::placeholders::_1, - std::placeholders::_2, std::placeholders::_3, std::placeholders::_4), - .write_then_read = - std::bind(&espp::I2c::write_read, &i2c, std::placeholders::_1, std::placeholders::_2, - std::placeholders::_3, std::placeholders::_4, std::placeholders::_5), - .log_level = espp::Logger::Verbosity::WARN, + .direction_mask = 0xF0, // upper 4 bits as outputs, lower 4 bits as inputs + .interrupt_mask = 0xFF, // all interrupts disabled for now + .initial_output = 0xA0, // initial pattern for outputs + .write = std::bind_front(&espp::I2c::write, &i2c), + .write_then_read = std::bind_front(&espp::I2c::write_read, &i2c), + .auto_init = false, + .log_level = espp::Logger::Verbosity::INFO, }); std::error_code ec; - exp.initialize(ec); - if (ec) { - fmt::print("PI4IOE5V init failed: {}\n", ec.message()); + // Initialize separately from the constructor since we set auto_init to false + if (!exp.initialize(ec)) { + logger.error("PI4IOE5V initialization failed: {}", ec.message()); return; } + // Configure pull resistors for input pins (upper 4 bits) + exp.set_pull_resistor_for_pin(0xF0, espp::Pi4ioe5v::PullResistor::PULL_UP, ec); + if (ec) { + logger.error("Failed to configure pull resistors: {}", ec.message()); + } + + // Create task to periodically toggle outputs and read inputs auto task_fn = [&](std::mutex &m, std::condition_variable &cv) { - static uint8_t value = 0xAA; - value ^= 0xFF; // toggle pattern - exp.write_outputs(value, ec); - if (!ec) { - uint8_t inputs = exp.read_inputs(ec); - if (!ec) { - fmt::print("OUT=0x{:02X} IN=0x{:02X}\n", value, inputs); - } + static auto start = std::chrono::high_resolution_clock::now(); + static uint8_t output_pattern = 0x50; + + auto now = std::chrono::high_resolution_clock::now(); + auto seconds = std::chrono::duration(now - start).count(); + + // Toggle output pattern on lower 4 bits + output_pattern ^= 0xF0; + exp.output(output_pattern, ec); + if (ec) { + logger.error("Failed to set outputs: {}", ec.message()); + return true; // stop the task + } + + // Read all pins + auto pins = exp.get_pins(ec); + if (ec) { + logger.error("Failed to read pins: {}", ec.message()); + return true; // stop the task } + + // Read current output state + auto outputs = exp.get_output(ec); + if (ec) { + logger.error("Failed to read outputs: {}", ec.message()); + return true; // stop the task + } + + fmt::print("{:.3f}, inputs: {:#04x}, outputs: {:#04x}\n", seconds, pins, outputs); + + // NOTE: sleeping in this way allows the sleep to exit early when the + // task is being stopped / destroyed { std::unique_lock lk(m); cv.wait_for(lk, 500ms); } + // don't want to stop the task return false; }; - espp::Task task({.callback = task_fn, - .task_config = {.name = "PI4IOE5V Task", .stack_size_bytes = 4 * 1024}, - .log_level = espp::Logger::Verbosity::WARN}); + auto task = espp::Task({.callback = task_fn, + .task_config = {.name = "PI4IOE5V Task", .stack_size_bytes = 4 * 1024}, + .log_level = espp::Logger::Verbosity::WARN}); + fmt::print("% time(s), inputs (lower 4 bits), outputs (upper 4 bits)\n"); task.start(); //! [pi4ioe5v_example] diff --git a/components/pi4ioe5v/include/pi4ioe5v.hpp b/components/pi4ioe5v/include/pi4ioe5v.hpp index 94a866e09..0c3ce9d56 100644 --- a/components/pi4ioe5v/include/pi4ioe5v.hpp +++ b/components/pi4ioe5v/include/pi4ioe5v.hpp @@ -1,166 +1,450 @@ #pragma once -#include #include +#include #include "base_peripheral.hpp" namespace espp { /** - * @brief PI4IOE5V I2C GPIO Expander family driver. + * @brief PI4IOE5V I2C GPIO Expander driver. * - * Provides APIs to configure direction, set/clear/read GPIOs, invert input - * polarity, and enable/configure pulls for PI4IOE5Vxxxx devices. Derives from - * `espp::BasePeripheral` (8-bit register addresses, addressed - * device). + * Class for communicating with and controlling a PI4IOE5V GPIO expander. + * The PI4IOE5V is an 8-bit I2C GPIO expander with interrupt capability + * and configurable pull-up/pull-down resistors. + * It operates at [1.65V, 5.5V] and supports I2C Fast-mode (400kHz) and + * Fast-mode Plus (1MHz). * - * The register layout used here matches common PI4IOE5V variants with two 8-bit - * ports (total 16 GPIOs). If your variant differs (e.g., pull register - * addresses), adjust the `Reg` map accordingly. + * The register layout used here matches the PI4IOE5V6408 datasheet. + * + * Datasheet for the PI4IOE5V can be found here: + * https://www.diodes.com/datasheet/download/PI4IOE5V6408.pdf * * @section pi4ioe5v_example Example * @snippet pi4ioe5v_example.cpp pi4ioe5v_example - * This component is typically bound to an `espp::I2c` instance using - * std::bind for the IO hooks. See the README for a short example. */ -class Pi4ioe5v : public BasePeripheral { +class Pi4ioe5v : public BasePeripheral<> { public: - /// Default I2C address when address pins are tied low (variant dependent) - static constexpr uint8_t DEFAULT_ADDRESS = 0x40; - - /// Register map (typical 8-GPIO variant: single 8-bit port) - enum class Reg : uint8_t { - INPUT = 0x00, ///< Input values (read-only) - OUTPUT = 0x01, ///< Output latch (read/write) - POLARITY_INV = 0x02, ///< Input polarity invert (1=invert) - DIRECTION = 0x03, ///< Direction (1=input, 0=output) - PULL_ENABLE = 0x46, ///< Pull enable (1=enabled) [variant-specific] - PULL_SELECT = 0x47, ///< Pull select (1=pull-up, 0=pull-down) [variant-specific] + static constexpr uint8_t ADDRESS_LOW = 0x43; ///< Address with addr pin low + static constexpr uint8_t ADDRESS_HIGH = 0x44; ///< Address with addr pin high + static constexpr uint8_t DEFAULT_ADDRESS = ADDRESS_LOW; ///< Default I2C address + + /** + * The Pull Resistor configuration. + */ + enum class PullResistor : uint8_t { + NO_PULL = 0, ///< No pull resistor enabled + PULL_UP = 1, ///< Pull-up resistor enabled + PULL_DOWN = 2, ///< Pull-down resistor enabled }; - /// Configuration structure for the PI4IOE5V driver + /** + * @brief Configuration information for the Pi4ioe5v. + */ struct Config { - uint8_t device_address = DEFAULT_ADDRESS; ///< I2C device address - /// Optional initial configuration for single 8-bit port (1=input, 0=output) - uint8_t direction_mask = 0xFF; - uint8_t output_state = 0x00; ///< Initial output latch state - uint8_t polarity_invert = 0x00; ///< Input invert mask (1=invert) - uint8_t pull_enable = 0x00; ///< Pull enable mask (1=enable) - uint8_t pull_up_select = 0x00; ///< Pull select mask (1=up, 0=down) - // IO hooks - BasePeripheral::probe_fn probe{nullptr}; ///< Optional bus probe - BasePeripheral::write_fn write{nullptr}; ///< Write function - BasePeripheral::read_register_fn read_register{nullptr}; ///< Read register function - BasePeripheral::write_then_read_fn write_then_read{nullptr}; ///< Write-then-read function - // Behavior - bool auto_init = true; ///< Initialize in ctor - Logger::Verbosity log_level{Logger::Verbosity::WARN}; ///< Log verbosity + uint8_t device_address = DEFAULT_ADDRESS; ///< I2C address to use to talk to this PI4IOE5V. + uint8_t direction_mask = 0x00; ///< Direction mask (0 = input, 1 = output). + uint8_t interrupt_mask = + 0x00; ///< Interrupt mask (0 = enabled interrupt, 1 = disable interrupt). + uint8_t input_defaults = 0x00; ///< Default input values for input interrupts. A change from + ///< this level will trigger an interrupt. + std::optional initial_output = + std::nullopt; ///< Initial output value (only for pins set as + ///< outputs). If not set, outputs will be low or high-z depending on + ///< high_z_mask. + std::optional high_z_mask = + std::nullopt; ///< Mask of pins to set as high impedance outputs. Overrides initial_output + ///< for these pins. All pins default to high-z on power-up. + std::optional pull_up_mask = + std::nullopt; ///< Mask of pins to enable pull-up resistors on. + std::optional pull_down_mask = + std::nullopt; ///< Mask of pins to enable pull-down resistors on. + BasePeripheral::write_fn write; ///< Function to write to the device. + BasePeripheral::write_then_read_fn + write_then_read; ///< Function to write then read from the device. + bool auto_init = true; ///< Automatically initialize the device. + Logger::Verbosity log_level{Logger::Verbosity::WARN}; ///< Log verbosity for the component. }; /** - * @brief Construct the PI4IOE5V expander. - * @param cfg Driver configuration, including IO hooks and initial state. + * @brief Construct the Pi4ioe5v. Will call initialize() if auto_init is true. + * @param config Configuration structure for configuring the PI4IOE5V */ - explicit Pi4ioe5v(const Config &cfg) - : BasePeripheral({.address = cfg.device_address, - .probe = cfg.probe, - .write = cfg.write, - .read_register = cfg.read_register, - .write_then_read = cfg.write_then_read}, - "Pi4ioe5v", cfg.log_level) - , config_(cfg) { - if (cfg.auto_init) { + explicit Pi4ioe5v(const Config &config) + : BasePeripheral({.address = config.device_address, + .write = config.write, + .write_then_read = config.write_then_read}, + "Pi4ioe5v", config.log_level) + , config_(config) { + if (config.auto_init) { std::error_code ec; if (!initialize(ec)) { - logger_.error("Pi4ioe5v init failed: {}", ec.message()); + logger_.error("Failed to initialize PI4IOE5V: {}", ec.message()); } } } /** - * @brief Initialize the expander with the provided configuration. - * @param ec Error code set on failure - * @return true on success, false on error + * @brief Initialize the component class. + * @param ec Error code to set if an error occurs. + * @return true if successful, false if an error occurred. */ bool initialize(std::error_code &ec) { return init(config_, ec); } - /// Set pin directions for the 8-bit port (1=input, 0=output) - /// @param mask_inputs Bit mask of inputs (bits 0..7) - /// @param ec Error code set on failure - void set_direction(uint8_t mask_inputs, std::error_code &ec) { - std::scoped_lock lock{base_mutex_}; - write_u8_to_register((uint8_t)Reg::DIRECTION, mask_inputs, ec); - } - - /// Write the output latch for the 8-bit port - /// @param value Latch values (bits 0..7) - /// @param ec Error code set on failure - void write_outputs(uint8_t value, std::error_code &ec) { - std::scoped_lock lock{base_mutex_}; - write_u8_to_register((uint8_t)Reg::OUTPUT, value, ec); - } - - /// Read current input pin states for the 8-bit port - /// @param ec Error code set on failure - /// @return Bit mask of inputs (bits 0..7) - uint8_t read_inputs(std::error_code &ec) { - std::scoped_lock lock{base_mutex_}; - return read_u8_from_register((uint8_t)Reg::INPUT, ec); - } - - /// Read current output latch for the 8-bit port - /// @param ec Error code set on failure - /// @return Bit mask of outputs (bits 0..7) - uint8_t read_outputs(std::error_code &ec) { - std::scoped_lock lock{base_mutex_}; - return read_u8_from_register((uint8_t)Reg::OUTPUT, ec); - } - - /// Configure input polarity inversion (1=invert) - /// @param mask Bit mask to invert (bits 0..7) - /// @param ec Error code set on failure - void set_polarity_invert(uint8_t mask, std::error_code &ec) { - std::scoped_lock lock{base_mutex_}; - write_u8_to_register((uint8_t)Reg::POLARITY_INV, mask, ec); - } - - /// Configure internal pull resistors - /// @param enable_mask Pull enable mask (1=enable) - /// @param up_select_mask Pull select mask (1=up, 0=down) - /// @param ec Error code set on failure - void configure_pull(uint8_t enable_mask, uint8_t up_select_mask, std::error_code &ec) { - std::scoped_lock lock{base_mutex_}; - write_u8_to_register((uint8_t)Reg::PULL_ENABLE, enable_mask, ec); + /** + * @brief Read the pin values. + * @param ec Error code to set if an error occurs. + * @return The pin values as an 8 bit mask. + */ + uint8_t get_pins(std::error_code &ec) { + return read_u8_from_register((uint8_t)Registers::INPUT, ec); + } + + /** + * @brief Write the pin values. + * @param value The pin values to apply. + * @param ec Error code to set if an error occurs. + * @return true if successful, false if an error occurred. + * @note This will overwrite any previous pin values for all output pins. + * @note Only affects pins configured as outputs. + * @note Pins configured as high-z will remain high-z. + * @note All pins default to high-z on power-up. + */ + bool output(uint8_t value, std::error_code &ec) { + write_u8_to_register((uint8_t)Registers::OUTPUT, value, ec); + return !ec; + } + + /** + * @brief Clear the pin values according to the provided mask. + * @details Reads the current pin values and clears any bits set in the mask. + * @param mask The pin values as an 8 bit mask to clear. + * @param ec Error code to set if an error occurs. + * @return true if successful, false if an error occurred. + */ + bool clear_pins(uint8_t mask, std::error_code &ec) { + std::lock_guard lock(base_mutex_); + auto data = read_u8_from_register((uint8_t)Registers::OUTPUT, ec); + if (ec) + return false; + data &= ~mask; + write_u8_to_register((uint8_t)Registers::OUTPUT, data, ec); + return !ec; + } + + /** + * @brief Set the pin values according to the provided value. + * @details Reads the current pin values and clears any bits set in the mask. + * @param value The pin values as an 8 bit mask to set. + * @param ec Error code to set if an error occurs. + * @return true if successful, false if an error occurred. + * @note Only affects pins configured as outputs. + */ + bool set_pins(uint8_t value, std::error_code &ec) { + std::lock_guard lock(base_mutex_); + auto data = read_u8_from_register((uint8_t)Registers::OUTPUT, ec); + if (ec) + return false; + data |= value; + write_u8_to_register((uint8_t)Registers::OUTPUT, data, ec); + return !ec; + } + + /** + * @brief Read the output pin values. + * @param ec Error code to set if an error occurs. + * @return The pin values as an 8 bit mask. + */ + uint8_t get_output(std::error_code &ec) { + return read_u8_from_register((uint8_t)Registers::OUTPUT, ec); + } + + /** + * @brief Read the input pin values. + * @param ec Error code to set if an error occurs. + * @return The pin values as an 8 bit mask. + */ + uint8_t get_input(std::error_code &ec) { + return read_u8_from_register((uint8_t)Registers::INPUT, ec); + } + + /** + * @brief Read the i/o direction for the pins. + * @param ec Error code to set if an error occurs. + * @return The direction as an 8 bit mask (1 = output, 0 = input). + */ + uint8_t get_direction(std::error_code &ec) { + return read_u8_from_register((uint8_t)Registers::DIRECTION, ec); + } + + /** + * @brief Set the i/o direction for the pins according to mask. + * @param mask The mask indicating direction (1 = output, 0 = input) + * @param ec Error code to set if an error occurs. + * @return true if successful, false if an error occurred. + */ + bool set_direction(uint8_t mask, std::error_code &ec) { + logger_.info("Setting direction (1=input) to {:#10b}", mask); + write_u8_to_register((uint8_t)Registers::DIRECTION, mask, ec); + return !ec; + } + + /** + * @brief Set the pull resistor for the provided pins. + * @param pin_mask The pin mask to configure for pull resistor. + * @param pull The pull resistor to configure. + * @param ec Error code to set if an error occurs. + * @return true if successful, false if an error occurred. + */ + bool set_pull_resistor_for_pin(uint8_t pin_mask, PullResistor pull, std::error_code &ec) { + std::lock_guard lock(base_mutex_); + + // Configure pull enable + auto enable_data = read_u8_from_register((uint8_t)Registers::PULL_ENABLE, ec); + if (ec) + return false; + + if (pull == PullResistor::NO_PULL) { + logger_.info("Disabling pull-up/down for pin {:#10b}", pin_mask); + enable_data &= ~pin_mask; + } else { + logger_.info("Enabling pull-up/down for pin {:#10b}", pin_mask); + enable_data |= pin_mask; + } + + write_u8_to_register((uint8_t)Registers::PULL_ENABLE, enable_data, ec); + if (ec) + return false; + + if (pull == PullResistor::NO_PULL) { + return true; + } + + // Configure pull select + auto select_data = read_u8_from_register((uint8_t)Registers::PULL_SELECT, ec); + if (ec) + return false; + + if (pull == PullResistor::PULL_UP) { + logger_.info("Setting pull-up for pin {:#10b}", pin_mask); + select_data |= pin_mask; + } else { + logger_.info("Setting pull-down for pin {:#10b}", pin_mask); + select_data &= ~pin_mask; + } + + write_u8_to_register((uint8_t)Registers::PULL_SELECT, select_data, ec); + return !ec; + } + + /** + * @brief Configure interrupt mask for the pins. + * @param mask The pin mask to configure for interrupt (1 = disable interrupt, 0 = enable). + * @param ec Error code to set if an error occurs. + * @return true if successful, false if an error occurred. + */ + bool set_interrupt_mask(uint8_t mask, std::error_code &ec) { + logger_.info("Setting interrupt mask (1=disabled) to {:#10b}", mask); + write_u8_to_register((uint8_t)Registers::INT_MASK, mask, ec); + return !ec; + } + + /** + * @brief Read interrupt status. + * @param ec Error code to set if an error occurs. + * @return The interrupt status as an 8 bit mask (1 = interrupt triggered). + */ + uint8_t get_interrupt_status(std::error_code &ec) { + logger_.info("Reading interrupt status"); + return read_u8_from_register((uint8_t)Registers::INT_STATUS, ec); + } + + /** + * @brief Set input default values for input interrupts. + * @param value The default value for the pins. An opposite value on input + * pins will trigger an interrupt. + * @param ec Error code to set if an error occurs. + * @return true if successful, false if an error occurred. + */ + bool set_input_default(uint8_t value, std::error_code &ec) { + logger_.info("Setting input default to {:#10b}", value); + write_u8_to_register((uint8_t)Registers::INPUT_DEFAULT, value, ec); + return !ec; + } + + /** + * @brief Set output high impedance control. + * @param mask The mask indicating which outputs should be high impedance (1 = high-Z). + * @param ec Error code to set if an error occurs. + * @return true if successful, false if an error occurred. + */ + bool set_output_high_impedance(uint8_t mask, std::error_code &ec) { + logger_.info("Setting output high impedance to {:#10b}", mask); + write_u8_to_register((uint8_t)Registers::OUTPUT_HIGH_IM, mask, ec); + return !ec; + } + + /** + * @brief Perform a software reset of the device. + * @param ec Error code to set if an error occurs. + * @return true if successful, false if an error occurred. + * @note This will reset all registers to their default values. + * @note This will cause the reset interrupt bit to be set in the CHIP_ID_CTRL + * register, which can be cleared by reading the register. + */ + bool reset(std::error_code &ec) { + logger_.info("Performing software reset"); + // Software reset is bit 0 of the CHIP_ID_CTRL register + write_u8_to_register((uint8_t)Registers::CHIP_ID_CTRL, 0x01, ec); + return !ec; + } + + /** + * @brief Read the manufacturer ID. + * @param ec Error code to set if an error occurs. + * @return The manufacturer ID. + * @note The manufacturer ID is contained in bits 5,6,7 of the CHIP_ID_CTRL + * register. The expected value is 0b101 (Diodes Incorporated). + * @note The manufacturer ID will be shifted to the least significant bits in + * the return value. + * @note This read will also clear the reset interrupt bit if it was set. + */ + uint8_t get_manufacturer_id(std::error_code &ec) { + // read the chip_id register and return bits 5,6,7 + auto chip_id = read_u8_from_register((uint8_t)Registers::CHIP_ID_CTRL, ec); if (ec) - return; - write_u8_to_register((uint8_t)Reg::PULL_SELECT, up_select_mask, ec); + return 0; + return (chip_id >> 5) & 0x07; + } + + /** + * @brief Read the firmware revision. + * @param ec Error code to set if an error occurs. + * @return The firmware revision. + * @note The firmware revision is contained in bits 2,3,4 of the CHIP_ID_CTRL + * register. + * @note The firmware revision will be shifted to the least significant bits in + * the return value. + * @note This read will also clear the reset interrupt bit if it was set. + */ + uint8_t get_firmware_revision(std::error_code &ec) { + // read the chip_id register and return bits 2,3,4 + auto chip_id = read_u8_from_register((uint8_t)Registers::CHIP_ID_CTRL, ec); + if (ec) + return 0; + return (chip_id >> 2) & 0x07; + } + + /** + * @brief Read the chip ID. + * @param ec Error code to set if an error occurs. + * @return The chip ID. + * @note This read will also clear the reset interrupt bit if it was set. + */ + uint8_t get_chip_id(std::error_code &ec) { + return read_u8_from_register((uint8_t)Registers::CHIP_ID_CTRL, ec); } protected: + /** + * @brief Register map for the PI4IOE5V + */ + enum class Registers : uint8_t { + CHIP_ID_CTRL = 0x01, ///< Chip ID and control + DIRECTION = 0x03, ///< Direction (0=input, 1=output), default is input + OUTPUT = 0x05, ///< Output latch (read/write) + OUTPUT_HIGH_IM = 0x07, ///< Output high-impedance control (1 = high-Z) + INPUT_DEFAULT = 0x09, ///< Input default state (used if input and no pull) + PULL_ENABLE = 0x0B, ///< Pull enable (1=enabled, 0=disabled) + PULL_SELECT = 0x0D, ///< Pull select (1=pull-up, 0=pull-down) + INPUT = 0x0F, ///< Input values (read-only) + INT_MASK = 0x11, ///< Interrupt mask (1=disabled, 0=enabled) + INT_STATUS = 0x13, ///< Interrupt status (read-only) + POLARITY_INV = 0x15, ///< Polarity inversion (1=inverted, 0=normal) - NOTE: This may not exist + ///< on all variants + }; + /** * @brief Apply initial configuration to the device. - * @param cfg Driver configuration + * @param config Driver configuration * @param ec Error code set on failure * @return true on success, false on error */ - bool init(const Config &cfg, std::error_code &ec) { - std::scoped_lock lock{base_mutex_}; - set_direction(cfg.direction_mask, ec); - if (ec) + bool init(const Config &config, std::error_code &ec) { + std::lock_guard lock(base_mutex_); + + logger_.info("Initializing PI4IOE5V with address {:#04x}", config.device_address); + + // perform a reset to ensure we start from a known state + if (!reset(ec)) return false; - set_polarity_invert(cfg.polarity_invert, ec); + + // read chip id and log it + auto chip_id = get_chip_id(ec); if (ec) return false; - configure_pull(cfg.pull_enable, cfg.pull_up_select, ec); - if (ec) + logger_.info("PI4IOE5V Chip ID: {:#04x}", chip_id); + + // Set the initial output value before setting the direction, to make sure + // there are no glitches on the output pins + if (config.initial_output.has_value()) { + auto initial_value = config.initial_output.value(); + logger_.info("Setting initial output value to {:#10b}", initial_value); + if (!output(initial_value, ec)) + return false; + } + + // Set high impedance outputs + if (config.high_z_mask.has_value()) { + if (!set_output_high_impedance(config.high_z_mask.value(), ec)) + return false; + } + + // Set pull-up resistors + if (config.pull_up_mask.has_value()) { + if (!set_pull_resistor_for_pin(config.pull_up_mask.value(), PullResistor::PULL_UP, ec)) + return false; + } + + // Set pull-down resistors + if (config.pull_down_mask.has_value()) { + if (!set_pull_resistor_for_pin(config.pull_down_mask.value(), PullResistor::PULL_DOWN, ec)) + return false; + } + + // Set direction + if (!set_direction(config.direction_mask, ec)) return false; - write_outputs(cfg.output_state, ec); - if (ec) + + // Set input defaults + if (!set_input_default(config.input_defaults, ec)) return false; + + // Set interrupt mask + if (!set_interrupt_mask(config.interrupt_mask, ec)) + return false; + return true; } - /// Stored configuration - Config config_{}; + Config config_; }; } // namespace espp + +// for printing of enums using libfmt +template <> struct fmt::formatter { + constexpr auto parse(format_parse_context &ctx) const { return ctx.begin(); } + template + auto format(const espp::Pi4ioe5v::PullResistor &p, FormatContext &ctx) const { + switch (p) { + case espp::Pi4ioe5v::PullResistor::PULL_UP: + return fmt::format_to(ctx.out(), "Pull Up"); + case espp::Pi4ioe5v::PullResistor::PULL_DOWN: + return fmt::format_to(ctx.out(), "Pull Down"); + case espp::Pi4ioe5v::PullResistor::NO_PULL: + return fmt::format_to(ctx.out(), "No Pull"); + default: + return fmt::format_to(ctx.out(), "Unknown"); + } + } +}; From 7a958bb8f3f05e85356a7de1b8242ba56f874a09 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Tue, 7 Oct 2025 16:14:57 -0500 Subject: [PATCH 2/3] fix inconsistency and remove unused reg def. --- components/pi4ioe5v/include/pi4ioe5v.hpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/components/pi4ioe5v/include/pi4ioe5v.hpp b/components/pi4ioe5v/include/pi4ioe5v.hpp index 0c3ce9d56..9cc3da8ac 100644 --- a/components/pi4ioe5v/include/pi4ioe5v.hpp +++ b/components/pi4ioe5v/include/pi4ioe5v.hpp @@ -184,7 +184,7 @@ class Pi4ioe5v : public BasePeripheral<> { * @return true if successful, false if an error occurred. */ bool set_direction(uint8_t mask, std::error_code &ec) { - logger_.info("Setting direction (1=input) to {:#10b}", mask); + logger_.info("Setting direction (1=output) to {:#10b}", mask); write_u8_to_register((uint8_t)Registers::DIRECTION, mask, ec); return !ec; } @@ -360,8 +360,6 @@ class Pi4ioe5v : public BasePeripheral<> { INPUT = 0x0F, ///< Input values (read-only) INT_MASK = 0x11, ///< Interrupt mask (1=disabled, 0=enabled) INT_STATUS = 0x13, ///< Interrupt status (read-only) - POLARITY_INV = 0x15, ///< Polarity inversion (1=inverted, 0=normal) - NOTE: This may not exist - ///< on all variants }; /** From 0a41b46ffdd46bffe25ee238eb2a7dbc27fe5e56 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Tue, 7 Oct 2025 16:19:50 -0500 Subject: [PATCH 3/3] fix inconsistencies --- components/pi4ioe5v/example/main/pi4ioe5v_example.cpp | 4 ++-- components/pi4ioe5v/include/pi4ioe5v.hpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/components/pi4ioe5v/example/main/pi4ioe5v_example.cpp b/components/pi4ioe5v/example/main/pi4ioe5v_example.cpp index 135385cd7..6af600be1 100644 --- a/components/pi4ioe5v/example/main/pi4ioe5v_example.cpp +++ b/components/pi4ioe5v/example/main/pi4ioe5v_example.cpp @@ -43,7 +43,7 @@ extern "C" void app_main(void) { return; } - // Configure pull resistors for input pins (upper 4 bits) + // Configure pull resistors for input pins (lower 4 bits) exp.set_pull_resistor_for_pin(0xF0, espp::Pi4ioe5v::PullResistor::PULL_UP, ec); if (ec) { logger.error("Failed to configure pull resistors: {}", ec.message()); @@ -57,7 +57,7 @@ extern "C" void app_main(void) { auto now = std::chrono::high_resolution_clock::now(); auto seconds = std::chrono::duration(now - start).count(); - // Toggle output pattern on lower 4 bits + // Toggle output pattern on upper 4 bits output_pattern ^= 0xF0; exp.output(output_pattern, ec); if (ec) { diff --git a/components/pi4ioe5v/include/pi4ioe5v.hpp b/components/pi4ioe5v/include/pi4ioe5v.hpp index 9cc3da8ac..d5d3cb759 100644 --- a/components/pi4ioe5v/include/pi4ioe5v.hpp +++ b/components/pi4ioe5v/include/pi4ioe5v.hpp @@ -134,7 +134,7 @@ class Pi4ioe5v : public BasePeripheral<> { /** * @brief Set the pin values according to the provided value. - * @details Reads the current pin values and clears any bits set in the mask. + * @details Reads the current pin values and sets any bits set in the mask. * @param value The pin values as an 8 bit mask to set. * @param ec Error code to set if an error occurs. * @return true if successful, false if an error occurred.