spi_transfer Refactor Pointers into std::array

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",

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<FRAME_LEN>(spi_fds_[motor], tx, rx, SPI_SPEED_HZ);
 
     motor_status_[motor].frame_count++;
 

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<TMC_CMD_MSG_SIZE, TMC_CMD_MSG_SIZE>(
+        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;
     }
 

src/software/embedded/motor_controller/tmc_motor_controller.h

@@ -202,13 +202,13 @@ class TmcMotorController : public MotorController
     std::unique_ptr<Gpio> reset_gpio_;
 
     // Transfer Buffers for spiTransfer
-    uint8_t tx_[5] = {};
-    uint8_t rx_[5] = {};
+    std::array<uint8_t, 5> tx_ = {};
+    std::array<uint8_t, 5> rx_ = {};
 
     // Transfer Buffers for readThenWriteSpiTransfer
-    uint8_t write_tx_[5] = {};
-    uint8_t read_tx_[5]  = {};
-    uint8_t read_rx_[5]  = {};
+    std::array<uint8_t, 5> write_tx_ = {};
+    std::array<uint8_t, 5> read_tx_  = {};
+    std::array<uint8_t, 5> read_rx_  = {};
 
     // Transfer State
     bool transfer_started_  = false;

src/software/embedded/spi_utils.h

@@ -1,38 +1,91 @@
 #pragma once
 
-#include <string.h>
+#include <errno.h>
+#include <linux/spi/spidev.h>
 
+#include <array>
 #include <cstdint>
 
+#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 <unsigned len>
+void spiTransfer(int fd, const std::array<uint8_t, len>& tx, std::array<uint8_t, len>& rx,
+                 uint32_t spi_speed)
+{
+    int ret;
+
+    struct spi_ioc_transfer tr[1];
+    memset(tr, 0, sizeof(tr));
+
+    tr[0].tx_buf        = reinterpret_cast<uintptr_t>(tx.data());
+    tr[0].rx_buf        = reinterpret_cast<uintptr_t>(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 <uint32_t read_len, uint32_t write_len>
+void readThenWriteSpiTransfer(int fd, const std::array<uint8_t, read_len>& read_tx,
+                              const std::array<uint8_t, write_len>& write_tx,
+                              std::array<uint8_t, read_len>& 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<uintptr_t>(read_tx.data());
+    tr[0].rx_buf        = reinterpret_cast<uintptr_t>(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<uintptr_t>(write_tx.data());
+    tr[1].rx_buf        = reinterpret_cast<uintptr_t>(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);
+}