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/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..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) @@ -582,7 +575,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 +601,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..d7d23da7b1 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 deleted file mode 100644 index 207f02e90b..0000000000 --- a/src/software/embedded/spi_utils.cpp +++ /dev/null @@ -1,59 +0,0 @@ -#include "software/embedded/spi_utils.h" - -#include -#include -#include - -#include "software/logger/logger.h" - -void spiTransfer(int fd, uint8_t const* tx, uint8_t const* rx, unsigned len, - uint32_t spi_speed) -{ - int ret; - - 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].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); -} - -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) -{ - uint8_t write_rx[5] = {0}; - - 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].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].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 1b38e14f22..78c52273e8 100644 --- a/src/software/embedded/spi_utils.h +++ b/src/software/embedded/spi_utils.h @@ -1,38 +1,91 @@ #pragma once -#include +#include +#include +#include #include +#include "software/embedded/spi_utils.h" +#include "software/logger/logger.h" + // 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 * * 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 - * @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, - uint32_t spi_speed); +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); +} /** * Performs two back to back SPI transactions, first a read and then a write. * * NOTE: read_tx and write_tx are both expected to be in BIG ENDIAN * + * @tparam read_len the length of the read_tx and read_rx buffers + * @tparam write_len the length of the write_tx buffer + * * @param fd the SPI file descriptor to transfer data over * @param read_tx pointer to the buffer containing the address for reading * @param write_tx pointer to the buffer containing the address + data for write * @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) +{ + 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); +}