From 1c189cbf8b7fa0e75f097eedb5ecbf2dedf7fb7e Mon Sep 17 00:00:00 2001 From: adrianchan787 Date: Sat, 30 May 2026 16:28:50 -0700 Subject: [PATCH 1/7] Refactored pointers into arrays --- .../stspin_motor_controller.cpp | 2 +- .../motor_controller/tmc_motor_controller.cpp | 4 +-- .../motor_controller/tmc_motor_controller.h | 10 ++++---- src/software/embedded/spi_utils.cpp | 25 +++++++++++-------- src/software/embedded/spi_utils.h | 17 ++++++------- 5 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/software/embedded/motor_controller/stspin_motor_controller.cpp b/src/software/embedded/motor_controller/stspin_motor_controller.cpp index 1c29915b4d..751dceabad 100644 --- a/src/software/embedded/motor_controller/stspin_motor_controller.cpp +++ b/src/software/embedded/motor_controller/stspin_motor_controller.cpp @@ -280,7 +280,7 @@ void StSpinMotorController::sendAndReceiveFrame(const MotorIndex motor, populateTx(outgoing_frame, tx); - spiTransfer(spi_fds_[motor], tx.data(), rx.data(), FRAME_LEN, SPI_SPEED_HZ); + spiTransfer(spi_fds_[motor], tx, rx, SPI_SPEED_HZ); motor_status_[motor].frame_count++; diff --git a/src/software/embedded/motor_controller/tmc_motor_controller.cpp b/src/software/embedded/motor_controller/tmc_motor_controller.cpp index 468919fd4c..87a74e45bd 100644 --- a/src/software/embedded/motor_controller/tmc_motor_controller.cpp +++ b/src/software/embedded/motor_controller/tmc_motor_controller.cpp @@ -582,7 +582,7 @@ uint8_t TmcMotorController::readWriteByte(const uint8_t motor, const uint8_t dat // The first byte should contain the address on a read operation. // Trigger a transfer (1 byte) and buffer the response (4 bytes) tx_[position_] = data; - spiTransfer(file_descriptors_[motor], tx_, rx_, 5, spi_speed); + spiTransfer<5>(file_descriptors_[motor], tx_, rx_, spi_speed); currently_reading_ = true; currently_writing_ = false; @@ -608,7 +608,7 @@ uint8_t TmcMotorController::readWriteByte(const uint8_t motor, const uint8_t dat { // we have all the bytes for this transfer, lets trigger the transfer and // reset state - spiTransfer(file_descriptors_[motor], tx_, rx_, 5, spi_speed); + spiTransfer<5>(file_descriptors_[motor], tx_, rx_, spi_speed); transfer_started_ = false; } diff --git a/src/software/embedded/motor_controller/tmc_motor_controller.h b/src/software/embedded/motor_controller/tmc_motor_controller.h index 71a8a090f3..511e60bbe9 100644 --- a/src/software/embedded/motor_controller/tmc_motor_controller.h +++ b/src/software/embedded/motor_controller/tmc_motor_controller.h @@ -202,13 +202,13 @@ class TmcMotorController : public MotorController std::unique_ptr reset_gpio_; // Transfer Buffers for spiTransfer - uint8_t tx_[5] = {}; - uint8_t rx_[5] = {}; + std::array tx_ = {}; + std::array rx_ = {}; // Transfer Buffers for readThenWriteSpiTransfer - uint8_t write_tx_[5] = {}; - uint8_t read_tx_[5] = {}; - uint8_t read_rx_[5] = {}; + std::array write_tx_ = {}; + std::array read_tx_ = {}; + std::array read_rx_ = {}; // Transfer State bool transfer_started_ = false; diff --git a/src/software/embedded/spi_utils.cpp b/src/software/embedded/spi_utils.cpp index 207f02e90b..125a521ad7 100644 --- a/src/software/embedded/spi_utils.cpp +++ b/src/software/embedded/spi_utils.cpp @@ -6,7 +6,8 @@ #include "software/logger/logger.h" -void spiTransfer(int fd, uint8_t const* tx, uint8_t const* rx, unsigned len, +template +void spiTransfer(int fd, const std::array& tx, std::array& rx, uint32_t spi_speed) { int ret; @@ -14,8 +15,8 @@ void spiTransfer(int fd, uint8_t const* tx, uint8_t const* rx, unsigned len, struct spi_ioc_transfer tr[1]; memset(tr, 0, sizeof(tr)); - tr[0].tx_buf = (unsigned long)tx; - tr[0].rx_buf = (unsigned long)rx; + tr[0].tx_buf = reinterpret_cast(tx.data()); + tr[0].rx_buf = reinterpret_cast(rx.data()); tr[0].len = len; tr[0].delay_usecs = 0; tr[0].speed_hz = spi_speed; @@ -27,24 +28,26 @@ void spiTransfer(int fd, uint8_t const* tx, uint8_t const* rx, unsigned len, << strerror(errno); } -void readThenWriteSpiTransfer(int fd, const uint8_t* read_tx, const uint8_t* write_tx, - const uint8_t* read_rx, const uint32_t read_len, - const uint32_t write_len, uint32_t spi_speed) +template +void readThenWriteSpiTransfer(int fd, const std::array& read_tx, + std::array& write_tx, + const std::array& read_rx, + int32_t spi_speed) { - uint8_t write_rx[5] = {0}; + uint8_t write_rx[5] = {}; struct spi_ioc_transfer tr[2]; memset(tr, 0, sizeof(tr)); - tr[0].tx_buf = (unsigned long)read_tx; - tr[0].rx_buf = (unsigned long)read_rx; + tr[0].tx_buf = reinterpret_cast(read_tx); + tr[0].rx_buf = reinterpret_cast(read_rx); tr[0].len = read_len; tr[0].delay_usecs = 0; tr[0].speed_hz = spi_speed; tr[0].bits_per_word = 8; tr[0].cs_change = 0; - tr[1].tx_buf = (unsigned long)write_tx; - tr[1].rx_buf = (unsigned long)write_rx; + tr[1].tx_buf = reinterpret_cast(write_tx); + tr[1].rx_buf = reinterpret_cast(write_rx); tr[1].len = write_len; tr[1].delay_usecs = 0; tr[1].speed_hz = spi_speed; diff --git a/src/software/embedded/spi_utils.h b/src/software/embedded/spi_utils.h index 1b38e14f22..1a2433eecc 100644 --- a/src/software/embedded/spi_utils.h +++ b/src/software/embedded/spi_utils.h @@ -1,11 +1,9 @@ #pragma once -#include - +#include #include // TODO: #3747 Wrap in spi_utils namespace -// TODO: #3751 Use std::array instead of raw pointers for tx and rx buffers. /** * Trigger an SPI transfer over an open SPI connection * @@ -14,11 +12,10 @@ * @param fd the SPI file descriptor to transfer data over * @param tx the tx buffer, data to send out * @param rx the rx buffer, will be updated with data from the full-duplex transfer - * @param len the length of the tx and rx buffer * @param spi_speed the speed to run spi at in Hz - * */ -void spiTransfer(int fd, uint8_t const* tx, uint8_t const* rx, unsigned len, +template +void spiTransfer(int fd, const std::array& tx, std::array& rx, uint32_t spi_speed); /** @@ -32,7 +29,7 @@ void spiTransfer(int fd, uint8_t const* tx, uint8_t const* rx, unsigned len, * @param read_rx the buffer our read response will be placed in * @param spi_speed the speed to run spi at in Hz */ -void readThenWriteSpiTransfer( - int fd, const uint8_t* read_tx, const uint8_t* write_tx, const uint8_t* read_rx, - const uint32_t read_len, const uint32_t write_len, - uint32_t spi_speed); // refactor to take std::array, not raw pointers +template +void readThenWriteSpiTransfer(int fd, const std::array& read_tx, + const std::array& write_tx, + std::array& read_rx, uint32_t spi_speed); From 2a74486bd40c22e4de83832d99bc5093335d3657 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci-lite[bot]" <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Date: Sat, 30 May 2026 23:39:37 +0000 Subject: [PATCH 2/7] [pre-commit.ci lite] apply automatic fixes --- src/software/embedded/motor_controller/tmc_motor_controller.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/software/embedded/motor_controller/tmc_motor_controller.h b/src/software/embedded/motor_controller/tmc_motor_controller.h index 511e60bbe9..d7d23da7b1 100644 --- a/src/software/embedded/motor_controller/tmc_motor_controller.h +++ b/src/software/embedded/motor_controller/tmc_motor_controller.h @@ -203,7 +203,7 @@ class TmcMotorController : public MotorController // Transfer Buffers for spiTransfer std::array tx_ = {}; - std::array rx_ = {}; + std::array rx_ = {}; // Transfer Buffers for readThenWriteSpiTransfer std::array write_tx_ = {}; From 81e3ad4305c1df68b42201d2f4c35aa2e2346ab5 Mon Sep 17 00:00:00 2001 From: adrianchan787 Date: Sat, 30 May 2026 17:34:05 -0700 Subject: [PATCH 3/7] Fixed memset; changes didn't save first time around --- .../motor_controller/tmc_motor_controller.cpp | 13 +++---------- .../motor_controller/tmc_motor_controller.h | 2 +- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/software/embedded/motor_controller/tmc_motor_controller.cpp b/src/software/embedded/motor_controller/tmc_motor_controller.cpp index 87a74e45bd..7fc0f9d08e 100644 --- a/src/software/embedded/motor_controller/tmc_motor_controller.cpp +++ b/src/software/embedded/motor_controller/tmc_motor_controller.cpp @@ -423,11 +423,6 @@ int TmcMotorController::readThenWriteValue(const MotorIndex motor, spi_demux_select_0_->setValue(GpioState::HIGH); spi_demux_select_1_->setValue(GpioState::LOW); - // ensure tx_ and rx_ are cleared - memset(read_tx_, 0, 5); - memset(write_tx_, 0, 5); - memset(read_rx_, 0, 5); - // Trinamic transactions looks like this: // + - - - + - - - + - - - + - - - + - - - + // | ADDR | DATA | @@ -446,9 +441,9 @@ int TmcMotorController::readThenWriteValue(const MotorIndex motor, write_tx_[4 - i] = byte_to_copy; } - readThenWriteSpiTransfer(file_descriptors_[CHIP_SELECTS.at(motor)], read_tx_, - write_tx_, read_rx_, TMC_CMD_MSG_SIZE, TMC_CMD_MSG_SIZE, - TMC4671_SPI_SPEED); + readThenWriteSpiTransfer( + file_descriptors_[CHIP_SELECTS.at(motor)], read_tx_, write_tx_, read_rx_, + TMC4671_SPI_SPEED); int32_t value = read_rx_[0]; for (int i = 1; i < 5; i++) @@ -566,8 +561,6 @@ uint8_t TmcMotorController::readWriteByte(const uint8_t motor, const uint8_t dat if (!transfer_started_) { - memset(tx_, 0, sizeof(tx_)); - memset(rx_, 0, sizeof(rx_)); position_ = 0; if (data & TMC_WRITE_BIT) diff --git a/src/software/embedded/motor_controller/tmc_motor_controller.h b/src/software/embedded/motor_controller/tmc_motor_controller.h index 511e60bbe9..d7d23da7b1 100644 --- a/src/software/embedded/motor_controller/tmc_motor_controller.h +++ b/src/software/embedded/motor_controller/tmc_motor_controller.h @@ -203,7 +203,7 @@ class TmcMotorController : public MotorController // Transfer Buffers for spiTransfer std::array tx_ = {}; - std::array rx_ = {}; + std::array rx_ = {}; // Transfer Buffers for readThenWriteSpiTransfer std::array write_tx_ = {}; From d1818d3aedfc340ac6ed285e2ecfa8881c365eb6 Mon Sep 17 00:00:00 2001 From: adrianchan787 Date: Sat, 30 May 2026 18:40:42 -0700 Subject: [PATCH 4/7] Further changes that didn't save --- src/software/embedded/spi_utils.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/software/embedded/spi_utils.cpp b/src/software/embedded/spi_utils.cpp index 125a521ad7..78540cb1a8 100644 --- a/src/software/embedded/spi_utils.cpp +++ b/src/software/embedded/spi_utils.cpp @@ -30,23 +30,23 @@ void spiTransfer(int fd, const std::array& tx, std::array void readThenWriteSpiTransfer(int fd, const std::array& read_tx, - std::array& write_tx, - const std::array& read_rx, - int32_t spi_speed) + const std::array& write_tx, + std::array& read_rx, + uint32_t spi_speed) { uint8_t write_rx[5] = {}; struct spi_ioc_transfer tr[2]; memset(tr, 0, sizeof(tr)); - tr[0].tx_buf = reinterpret_cast(read_tx); - tr[0].rx_buf = reinterpret_cast(read_rx); + tr[0].tx_buf = reinterpret_cast(read_tx.data()); + tr[0].rx_buf = reinterpret_cast(read_rx.data()); tr[0].len = read_len; tr[0].delay_usecs = 0; tr[0].speed_hz = spi_speed; tr[0].bits_per_word = 8; tr[0].cs_change = 0; - tr[1].tx_buf = reinterpret_cast(write_tx); + tr[1].tx_buf = reinterpret_cast(write_tx.data()); tr[1].rx_buf = reinterpret_cast(write_rx); tr[1].len = write_len; tr[1].delay_usecs = 0; From 5d06fcf52abca9a32f311958c132278a941ed96c Mon Sep 17 00:00:00 2001 From: adrianchan787 Date: Sat, 30 May 2026 18:41:26 -0700 Subject: [PATCH 5/7] Formatting --- src/software/embedded/spi_utils.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/software/embedded/spi_utils.cpp b/src/software/embedded/spi_utils.cpp index 78540cb1a8..b9fb7cf185 100644 --- a/src/software/embedded/spi_utils.cpp +++ b/src/software/embedded/spi_utils.cpp @@ -31,8 +31,7 @@ void spiTransfer(int fd, const std::array& tx, std::array void readThenWriteSpiTransfer(int fd, const std::array& read_tx, const std::array& write_tx, - std::array& read_rx, - uint32_t spi_speed) + std::array& read_rx, uint32_t spi_speed) { uint8_t write_rx[5] = {}; From 9540d6901564dfcabf45b8d3d972abf46ba44d4f Mon Sep 17 00:00:00 2001 From: adrianchan787 Date: Sat, 30 May 2026 21:43:14 -0700 Subject: [PATCH 6/7] Moved template function to .h --- src/software/embedded/BUILD | 1 - src/software/embedded/spi_utils.cpp | 61 ----------------------------- src/software/embedded/spi_utils.h | 53 ++++++++++++++++++++++++- 3 files changed, 51 insertions(+), 64 deletions(-) delete mode 100644 src/software/embedded/spi_utils.cpp diff --git a/src/software/embedded/BUILD b/src/software/embedded/BUILD index e5bba4bd99..7238ff8f88 100644 --- a/src/software/embedded/BUILD +++ b/src/software/embedded/BUILD @@ -44,7 +44,6 @@ cc_library( cc_library( name = "spi_utils", - srcs = ["spi_utils.cpp"], hdrs = ["spi_utils.h"], deps = [ "//software/logger", diff --git a/src/software/embedded/spi_utils.cpp b/src/software/embedded/spi_utils.cpp deleted file mode 100644 index b9fb7cf185..0000000000 --- a/src/software/embedded/spi_utils.cpp +++ /dev/null @@ -1,61 +0,0 @@ -#include "software/embedded/spi_utils.h" - -#include -#include -#include - -#include "software/logger/logger.h" - -template -void spiTransfer(int fd, const std::array& tx, std::array& rx, - uint32_t spi_speed) -{ - int ret; - - struct spi_ioc_transfer tr[1]; - memset(tr, 0, sizeof(tr)); - - tr[0].tx_buf = reinterpret_cast(tx.data()); - tr[0].rx_buf = reinterpret_cast(rx.data()); - tr[0].len = len; - tr[0].delay_usecs = 0; - tr[0].speed_hz = spi_speed; - tr[0].bits_per_word = 8; - - ret = ioctl(fd, SPI_IOC_MESSAGE(1), &tr); - - CHECK(ret >= 1) << "SPI Transfer to motor failed, not safe to proceed: errno " - << strerror(errno); -} - -template -void readThenWriteSpiTransfer(int fd, const std::array& read_tx, - const std::array& write_tx, - std::array& read_rx, uint32_t spi_speed) -{ - uint8_t write_rx[5] = {}; - - struct spi_ioc_transfer tr[2]; - memset(tr, 0, sizeof(tr)); - - tr[0].tx_buf = reinterpret_cast(read_tx.data()); - tr[0].rx_buf = reinterpret_cast(read_rx.data()); - tr[0].len = read_len; - tr[0].delay_usecs = 0; - tr[0].speed_hz = spi_speed; - tr[0].bits_per_word = 8; - tr[0].cs_change = 0; - tr[1].tx_buf = reinterpret_cast(write_tx.data()); - tr[1].rx_buf = reinterpret_cast(write_rx); - tr[1].len = write_len; - tr[1].delay_usecs = 0; - tr[1].speed_hz = spi_speed; - tr[1].bits_per_word = 8; - tr[1].cs_change = 0; - - int ret1 = ioctl(fd, SPI_IOC_MESSAGE(1), &tr); - int ret2 = ioctl(fd, SPI_IOC_MESSAGE(1), &tr[1]); - - CHECK(ret1 >= 1 && ret2 >= 1) - << "SPI Transfer to motor failed, not safe to proceed: errno " << strerror(errno); -} diff --git a/src/software/embedded/spi_utils.h b/src/software/embedded/spi_utils.h index 1a2433eecc..b0b08ca3c2 100644 --- a/src/software/embedded/spi_utils.h +++ b/src/software/embedded/spi_utils.h @@ -2,6 +2,10 @@ #include #include +#include "software/embedded/spi_utils.h" +#include +#include +#include "software/logger/logger.h" // TODO: #3747 Wrap in spi_utils namespace /** @@ -16,7 +20,25 @@ */ template void spiTransfer(int fd, const std::array& tx, std::array& rx, - uint32_t spi_speed); + uint32_t spi_speed) +{ + int ret; + + struct spi_ioc_transfer tr[1]; + memset(tr, 0, sizeof(tr)); + + tr[0].tx_buf = reinterpret_cast(tx.data()); + tr[0].rx_buf = reinterpret_cast(rx.data()); + tr[0].len = len; + tr[0].delay_usecs = 0; + tr[0].speed_hz = spi_speed; + tr[0].bits_per_word = 8; + + ret = ioctl(fd, SPI_IOC_MESSAGE(1), &tr); + + CHECK(ret >= 1) << "SPI Transfer to motor failed, not safe to proceed: errno " + << strerror(errno); +} /** * Performs two back to back SPI transactions, first a read and then a write. @@ -32,4 +54,31 @@ void spiTransfer(int fd, const std::array& tx, std::array void readThenWriteSpiTransfer(int fd, const std::array& read_tx, const std::array& write_tx, - std::array& read_rx, uint32_t spi_speed); + std::array& read_rx, uint32_t spi_speed) +{ + uint8_t write_rx[5] = {}; + + struct spi_ioc_transfer tr[2]; + memset(tr, 0, sizeof(tr)); + + tr[0].tx_buf = reinterpret_cast(read_tx.data()); + tr[0].rx_buf = reinterpret_cast(read_rx.data()); + tr[0].len = read_len; + tr[0].delay_usecs = 0; + tr[0].speed_hz = spi_speed; + tr[0].bits_per_word = 8; + tr[0].cs_change = 0; + tr[1].tx_buf = reinterpret_cast(write_tx.data()); + tr[1].rx_buf = reinterpret_cast(write_rx); + tr[1].len = write_len; + tr[1].delay_usecs = 0; + tr[1].speed_hz = spi_speed; + tr[1].bits_per_word = 8; + tr[1].cs_change = 0; + + int ret1 = ioctl(fd, SPI_IOC_MESSAGE(1), &tr); + int ret2 = ioctl(fd, SPI_IOC_MESSAGE(1), &tr[1]); + + CHECK(ret1 >= 1 && ret2 >= 1) + << "SPI Transfer to motor failed, not safe to proceed: errno " << strerror(errno); +} From fb35e5cb769f4a411a3742db715872a62a147af5 Mon Sep 17 00:00:00 2001 From: adrianchan787 Date: Sun, 31 May 2026 10:03:23 -0700 Subject: [PATCH 7/7] Added tparam to function specs --- src/software/embedded/spi_utils.h | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/software/embedded/spi_utils.h b/src/software/embedded/spi_utils.h index b0b08ca3c2..78c52273e8 100644 --- a/src/software/embedded/spi_utils.h +++ b/src/software/embedded/spi_utils.h @@ -1,10 +1,12 @@ #pragma once +#include +#include + #include #include + #include "software/embedded/spi_utils.h" -#include -#include #include "software/logger/logger.h" // TODO: #3747 Wrap in spi_utils namespace @@ -13,6 +15,8 @@ * * NOTE: tx is expected to be in BIG ENDIAN * + * @tparam len the length of the tx and rx buffer + * * @param fd the SPI file descriptor to transfer data over * @param tx the tx buffer, data to send out * @param rx the rx buffer, will be updated with data from the full-duplex transfer @@ -45,6 +49,9 @@ void spiTransfer(int fd, const std::array& tx, std::array