diff --git a/src/shared/drivers/Xbee/Xbee.h b/src/shared/drivers/Xbee/Xbee.h index ea5625585ead196ab292591194e8f5e2942fcf47..cea5d667c63934b656b2da8f9647a8c34bb1c593 100644 --- a/src/shared/drivers/Xbee/Xbee.h +++ b/src/shared/drivers/Xbee/Xbee.h @@ -117,7 +117,7 @@ class Xbee : public Transceiver, public ActiveObject public: Xbee(SPIBusInterface& bus, GpioPin cs, GpioPin attn, GpioPin rst, unsigned int send_timeout = 1000) - : send_timeout(send_timeout), spi_xbee(bus, cs), attn(attn), rst(rst) + : send_timeout(send_timeout), spi_xbee(bus, cs, {}), attn(attn), rst(rst) { spi_xbee.config.clock_div = SPIClockDivider::DIV128; diff --git a/src/shared/drivers/spi/MockSPIBus.h b/src/shared/drivers/spi/MockSPIBus.h index 5bbcb07737e5ea0b15cd99d06ecf0c4b528a2f40..d3436ffe53216251065910730c580914af6e49f8 100644 --- a/src/shared/drivers/spi/MockSPIBus.h +++ b/src/shared/drivers/spi/MockSPIBus.h @@ -26,7 +26,7 @@ #include <cstdint> #include <vector> -#include "SPIInterface.h" +#include "SPIBusInterface.h" using std::vector; diff --git a/src/shared/drivers/spi/SPIBus.h b/src/shared/drivers/spi/SPIBus.h index eb02b313e4aa7fb938287342ecc0d12f2e39085b..ea732e79f9c53b42864d4f4439b13e0ccd653c2a 100644 --- a/src/shared/drivers/spi/SPIBus.h +++ b/src/shared/drivers/spi/SPIBus.h @@ -21,7 +21,7 @@ * THE SOFTWARE. */ -#include "SPIInterface.h" +#include "SPIBusInterface.h" #pragma once @@ -48,17 +48,11 @@ public: SPIBus& operator=(SPIBus&&) = delete; /** - * @brief Wether to apply slave-specific bus configuration before each - * transaction (BusTemplate compatibility mode). - * Only set to false to use SPIDriver alongside BusTemplate.h. - * Default value is true. - * - * @param value True: The slave configuration is applied to the SPI - * peripheral before each transaction. False: No configuration is ever - * applied to the SPI peripheral. The SPI peripheral retains the - * configuration set by BusTemplate.h + * @brief Disable bus configuration: calls to configure() will have no + * effect after calling this & the SPI peripheral will need to be configured + * manually. */ - void enableSlaveConfiguration(bool value) { config_enabled = value; } + void disableBusConfiguration() { config_enabled = false; } /** * @brief See SPIBusInterface::write() @@ -141,7 +135,7 @@ inline void SPIBus::write(uint8_t data) { write(&data); } inline void SPIBus::write(uint8_t* data, size_t size) { - for (size_t i = 0; i < size; i++) + for (size_t i = 0; i < size; ++i) { write(data + i); } @@ -157,7 +151,7 @@ inline uint8_t SPIBus::read() inline void SPIBus::read(uint8_t* data, size_t size) { - for (size_t i = 0; i < size; i++) + for (size_t i = 0; i < size; ++i) { read(data + i); } @@ -171,7 +165,7 @@ inline uint8_t SPIBus::transfer(uint8_t data) inline void SPIBus::transfer(uint8_t* data, size_t size) { - for (size_t i = 0; i < size; i++) + for (size_t i = 0; i < size; ++i) { transfer(data + i); } @@ -232,7 +226,7 @@ inline void SPIBus::read(uint8_t* byte) // Wait until the peripheral is ready to transmit while ((spi->SR & SPI_SR_TXE) == 0) ; - // Write the byte in the transmit buffer + // Write 0 in the transmit buffer spi->DR = 0; // Wait until byte is transmitted diff --git a/src/shared/drivers/spi/SPIInterface.h b/src/shared/drivers/spi/SPIBusInterface.h similarity index 98% rename from src/shared/drivers/spi/SPIInterface.h rename to src/shared/drivers/spi/SPIBusInterface.h index 7a3eaa1fd3497e99dc517492833fe82d69badd81..966ddae385d37096b554dd85f67c101d89262124 100644 --- a/src/shared/drivers/spi/SPIInterface.h +++ b/src/shared/drivers/spi/SPIBusInterface.h @@ -211,8 +211,6 @@ struct SPISlave ///> with the slave. GpioPin cs; ///> Chip select pin - SPISlave(SPIBusInterface& bus, GpioPin cs) : bus(bus), cs(cs) {} - SPISlave(SPIBusInterface& bus, GpioPin cs, SPIBusConfig config) : bus(bus), config(config), cs(cs) { diff --git a/src/shared/drivers/spi/SPIDriver.h b/src/shared/drivers/spi/SPIDriver.h index c90ba4743742cfef611f55a0217c44fcd81d46ed..171d35b8263da201a6d00deb1b79a285e24204f6 100644 --- a/src/shared/drivers/spi/SPIDriver.h +++ b/src/shared/drivers/spi/SPIDriver.h @@ -23,6 +23,6 @@ #pragma once -#include "SPIInterface.h" +#include "SPIBusInterface.h" #include "SPITransaction.h" #include "SPIBus.h" \ No newline at end of file diff --git a/src/shared/drivers/spi/SPITransaction.cpp b/src/shared/drivers/spi/SPITransaction.cpp index a6f026c6b821325aa8e4e836cbd8fbdb924aa4b3..a7e79fa6b85c6ca266d5fbe1836b24398d2bf2d1 100644 --- a/src/shared/drivers/spi/SPITransaction.cpp +++ b/src/shared/drivers/spi/SPITransaction.cpp @@ -38,22 +38,22 @@ SPITransaction::SPITransaction(SPISlave slave) void SPITransaction::write(uint8_t cmd) { bus.select(cs); - bus.write(&cmd, 1); + bus.write(cmd); bus.deselect(cs); } void SPITransaction::write(uint8_t reg, uint8_t val) { bus.select(cs); - bus.write(®, 1); - bus.write(&val, 1); + bus.write(reg); + bus.write(val); bus.deselect(cs); } void SPITransaction::write(uint8_t reg, uint8_t* data, size_t size) { bus.select(cs); - bus.write(®, 1); + bus.write(reg); bus.write(data, size); bus.deselect(cs); } @@ -75,11 +75,13 @@ void SPITransaction::transfer(uint8_t* data, size_t size) uint8_t SPITransaction::read(uint8_t reg, bool set_read_bit) { if (set_read_bit) + { reg = reg | 0x80; + } bus.select(cs); - bus.write(®, 1); - bus.read(®, 1); + bus.write(reg); + reg = bus.read(); bus.deselect(cs); return reg; } @@ -88,10 +90,12 @@ void SPITransaction::read(uint8_t reg, uint8_t* data, size_t size, bool set_read_bit) { if (set_read_bit) + { reg = reg | 0x80; + } bus.select(cs); - bus.write(®, 1); + bus.write(reg); bus.read(data, size); bus.deselect(cs); } diff --git a/src/shared/drivers/spi/SPITransaction.h b/src/shared/drivers/spi/SPITransaction.h index 2aed6c404cdbae94736c72ed2e86c9ff94fed219..af1243dbaff5b6d0cad049b45a8bf50fcf2d1f5c 100644 --- a/src/shared/drivers/spi/SPITransaction.h +++ b/src/shared/drivers/spi/SPITransaction.h @@ -25,7 +25,7 @@ #pragma once -#include "SPIInterface.h" +#include "SPIBusInterface.h" /** * @brief Provides high-level access to the SPI Bus for a single transaction. diff --git a/src/tests/catch/spidriver/FakeSPIBus.h b/src/tests/catch/spidriver/FakeSPIBus.h index 94087b088fa4957db9d30b9520a4f3edc7374dde..d1213bfd6e6f9441e453cb84eb68e631572ca863 100644 --- a/src/tests/catch/spidriver/FakeSPIBus.h +++ b/src/tests/catch/spidriver/FakeSPIBus.h @@ -22,7 +22,7 @@ */ #include "FakeSpiTypedef.h" -#include "drivers/spi/SPIInterface.h" +#include "drivers/spi/SPIBusInterface.h" #pragma once @@ -49,18 +49,7 @@ public: FakeSPIBus(FakeSPIBus&&) = delete; FakeSPIBus& operator=(FakeSPIBus&&) = delete; - /** - * @brief Wether to apply slave-specific bus configuration before each - * transaction (BusTemplate compatibility mode). - * Only set to false to use SPIDriver alongside BusTemplate.h. - * Default value is true. - * - * @param value True: The slave configuration is applied to the SPI - * peripheral before each transaction. False: No configuration is ever - * applied to the SPI peripheral. The SPI peripheral retains the - * configuration set by BusTemplate.h - */ - void enableSlaveConfiguration(bool value) { config_enabled = value; } + void disableBusConfiguration() { config_enabled = false; } /** * @brief See SPIBusInterface::write() diff --git a/src/tests/catch/spidriver/test-spidriver.cpp b/src/tests/catch/spidriver/test-spidriver.cpp index 8b2f47f7edea4fedcaeab4c58f4cd9874d97a343..e0facfd1b7da9cce148af88ea0ebe7075d605c3f 100644 --- a/src/tests/catch/spidriver/test-spidriver.cpp +++ b/src/tests/catch/spidriver/test-spidriver.cpp @@ -60,84 +60,82 @@ TEST_CASE("SPIBus - Bus Configuration") SECTION("Mode") { - config.mode = SPIMode::MODE0; - uint32_t expected_CR1 = 0x0344; + config.mode = SPIMode::MODE0; + uint32_t expected_CR1 = 0x0344; bus.configure(config); REQUIRE(spi.CR1 == expected_CR1); - config.mode = SPIMode::MODE1; - expected_CR1 = 0x0345; + config.mode = SPIMode::MODE1; + expected_CR1 = 0x0345; bus.configure(config); REQUIRE(spi.CR1 == expected_CR1); - config.mode = SPIMode::MODE2; - expected_CR1 = 0x0346; + config.mode = SPIMode::MODE2; + expected_CR1 = 0x0346; bus.configure(config); REQUIRE(spi.CR1 == expected_CR1); - config.mode = SPIMode::MODE3; - expected_CR1 = 0x0347; + config.mode = SPIMode::MODE3; + expected_CR1 = 0x0347; bus.configure(config); REQUIRE(spi.CR1 == expected_CR1); - } SECTION("Clock Divider") { - config.clock_div = SPIClockDivider::DIV2; - uint32_t expected_CR1 = 0x0344; + config.clock_div = SPIClockDivider::DIV2; + uint32_t expected_CR1 = 0x0344; bus.configure(config); REQUIRE(spi.CR1 == expected_CR1); config.clock_div = SPIClockDivider::DIV4; - expected_CR1 = 0x034C; + expected_CR1 = 0x034C; bus.configure(config); REQUIRE(spi.CR1 == expected_CR1); config.clock_div = SPIClockDivider::DIV8; - expected_CR1 = 0x0354; + expected_CR1 = 0x0354; bus.configure(config); REQUIRE(spi.CR1 == expected_CR1); config.clock_div = SPIClockDivider::DIV16; - expected_CR1 = 0x035C; + expected_CR1 = 0x035C; bus.configure(config); REQUIRE(spi.CR1 == expected_CR1); config.clock_div = SPIClockDivider::DIV32; - expected_CR1 = 0x0364; + expected_CR1 = 0x0364; bus.configure(config); REQUIRE(spi.CR1 == expected_CR1); - + config.clock_div = SPIClockDivider::DIV64; - expected_CR1 = 0x036C; + expected_CR1 = 0x036C; bus.configure(config); REQUIRE(spi.CR1 == expected_CR1); config.clock_div = SPIClockDivider::DIV128; - expected_CR1 = 0x0374; + expected_CR1 = 0x0374; bus.configure(config); REQUIRE(spi.CR1 == expected_CR1); config.clock_div = SPIClockDivider::DIV256; - expected_CR1 = 0x037C; + expected_CR1 = 0x037C; bus.configure(config); REQUIRE(spi.CR1 == expected_CR1); } SECTION("Bit order") { - config.bit_order = SPIBitOrder::MSB_FIRST; - uint32_t expected_CR1 = 0x0344; + config.bit_order = SPIBitOrder::MSB_FIRST; + uint32_t expected_CR1 = 0x0344; bus.configure(config); REQUIRE(spi.CR1 == expected_CR1); config.bit_order = SPIBitOrder::LSB_FIRST; - expected_CR1 = 0x03C4; + expected_CR1 = 0x03C4; bus.configure(config); REQUIRE(spi.CR1 == expected_CR1); } - } SECTION("Disable configuration") @@ -148,7 +146,7 @@ TEST_CASE("SPIBus - Bus Configuration") config.mode = SPIMode::MODE3; config.bit_order = SPIBitOrder::LSB_FIRST; - bus.enableSlaveConfiguration(false); + bus.disableBusConfiguration(); bus.configure(config); REQUIRE(spi.CR1 == 0); } @@ -224,7 +222,7 @@ TEST_CASE("SPIBus - Multi byte operations") { FakeSpiTypedef spi; - spi.DR.in_buf = {1, 2, 3, 4, 5, 6, 7, 8, 9}; + spi.DR.in_buf = {100, 101, 102, 103, 104, 105, 106, 107, 108}; spi.CR1_expected = 0x03DF; FakeSPIBus bus{&spi}; @@ -239,28 +237,38 @@ TEST_CASE("SPIBus - Multi byte operations") bus.select(spi.cs); // 2 identical buffers - uint8_t buf[] = {4, 3, 2, 1}; - uint8_t bufc[] = {4, 3, 2, 1}; + uint8_t buf[] = {5, 4, 3, 2, 1}; + uint8_t bufc[] = {5, 4, 3, 2, 1}; SECTION("Write") { bus.write(buf, 0); REQUIRE(spi.DR.out_buf.size() == 0); + bus.write(buf, 1); + REQUIRE(spi.DR.out_buf.size() == 1); + REQUIRE(spi.DR.out_buf.back() == bufc[0]); + bus.write(buf, 4); - REQUIRE(spi.DR.out_buf.size() == 4); - REQUIRE(bufcmp(bufc, spi.DR.out_buf.data(), 4)); + REQUIRE(spi.DR.out_buf.size() == 5); + REQUIRE(bufcmp(bufc, spi.DR.out_buf.data() + 1, 4)); } SECTION("Read") { bus.read(buf, 0); // Nothing read - REQUIRE(bufcmp(bufc, buf, 4)); + REQUIRE(bufcmp(bufc, buf, 5)); - bus.read(buf, 4); + bus.read(buf, 1); + REQUIRE(bufcmp(buf, spi.DR.in_buf.data(), 1)); + // No overflows + REQUIRE(bufcmp(bufc + 1, buf + 1, 4)); - REQUIRE(bufcmp(buf, spi.DR.in_buf.data(), 4)); + bus.read(buf, 4); + REQUIRE(bufcmp(buf, spi.DR.in_buf.data() + 1, 4)); + // No overflows + REQUIRE(bufcmp(bufc + 4, buf + 4, 1)); } SECTION("Transfer") @@ -271,11 +279,19 @@ TEST_CASE("SPIBus - Multi byte operations") // Nothing written REQUIRE(spi.DR.out_buf.size() == 0); - bus.transfer(buf, 4); - REQUIRE(spi.DR.out_buf.size() == 4); + bus.transfer(buf, 1); + REQUIRE(spi.DR.out_buf.size() == 1); + REQUIRE(bufcmp(bufc, spi.DR.out_buf.data(), 1)); + REQUIRE(bufcmp(buf, spi.DR.in_buf.data(), 1)); + // No overflows + REQUIRE(bufcmp(bufc + 1, buf + 1, 4)); - REQUIRE(bufcmp(bufc, spi.DR.out_buf.data(), 4)); - REQUIRE(bufcmp(buf, spi.DR.in_buf.data(), 4)); + bus.transfer(buf + 1, 3); + REQUIRE(spi.DR.out_buf.size() == 4); + REQUIRE(bufcmp(bufc + 1, spi.DR.out_buf.data() + 1, 3)); + REQUIRE(bufcmp(buf + 1, spi.DR.in_buf.data() + 1, 3)); + // No overflows + REQUIRE(bufcmp(bufc + 4, buf + 4, 1)); } } @@ -284,8 +300,8 @@ TEST_CASE("SPITransaction - writes") MockSPIBus bus{}; SPIBusConfig config1{}; - config1.mode = SPIMode::MODE1; - config1.clock_div = SPIClockDivider::DIV32; + config1.mode = SPIMode::MODE1; + config1.clock_div = SPIClockDivider::DIV32; bus.expected_config = config1; @@ -376,12 +392,12 @@ TEST_CASE("SPITransaction - reads") { MockSPIBus bus; - bus.in_buf = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}; + bus.in_buf = {100, 101, 102, 103, 104, 105, 106, 107, 108, 109}; SPIBusConfig config1; - config1.mode = SPIMode::MODE1; - config1.clock_div = SPIClockDivider::DIV32; + config1.mode = SPIMode::MODE1; + config1.clock_div = SPIClockDivider::DIV32; bus.expected_config = config1; @@ -395,19 +411,19 @@ TEST_CASE("SPITransaction - reads") SECTION("1 byte reg read") { - REQUIRE(spi.read(0x05) == 1); + REQUIRE(spi.read(0x05) == bus.in_buf[0]); REQUIRE_FALSE(bus.isSelected()); REQUIRE(bus.out_buf.size() == 1); REQUIRE(bus.out_buf.back() == 0x85); - REQUIRE(spi.read(0x05, true) == 2); + REQUIRE(spi.read(0x05, true) == bus.in_buf[1]); REQUIRE_FALSE(bus.isSelected()); REQUIRE(bus.out_buf.size() == 2); REQUIRE(bus.out_buf.back() == 0x85); - REQUIRE(spi.read(0x05, false) == 3); + REQUIRE(spi.read(0x05, false) == bus.in_buf[2]); REQUIRE_FALSE(bus.isSelected()); REQUIRE(bus.out_buf.size() == 3); @@ -416,48 +432,54 @@ TEST_CASE("SPITransaction - reads") SECTION("multi byte reg read") { - uint8_t read[4] = {0, 0, 0, 0}; - uint8_t zero[4] = {0, 0, 0, 0}; + const int buf_size = 7; + uint8_t buf[] = {1, 2, 3, 4, 5, 6, 7}; + uint8_t cmp[] = {1, 2, 3, 4, 5, 6, 7}; - spi.read(0x05, read, 0); + spi.read(0x05, buf, 0); REQUIRE_FALSE(bus.isSelected()); REQUIRE(bus.out_buf.size() == 1); REQUIRE(bus.out_buf.back() == 0x85); - REQUIRE(bufcmp(read, zero, 4)); + REQUIRE(bufcmp(buf, cmp, buf_size)); - spi.read(0x05, read, 3); + spi.read(0x05, buf, 3); REQUIRE_FALSE(bus.isSelected()); REQUIRE(bus.out_buf.size() == 2); REQUIRE(bus.out_buf.back() == 0x85); - REQUIRE(bufcmp(read, bus.in_buf.data(), 3)); + REQUIRE(bufcmp(buf, bus.in_buf.data(), 3)); + REQUIRE(bufcmp(buf + 3, cmp + 3, buf_size - 3)); - spi.read(0x05, read, 3, true); + spi.read(0x05, buf, 3, true); REQUIRE_FALSE(bus.isSelected()); REQUIRE(bus.out_buf.size() == 3); REQUIRE(bus.out_buf.back() == 0x85); - REQUIRE(bufcmp(read, bus.in_buf.data() + 3, 3)); + REQUIRE(bufcmp(buf, bus.in_buf.data() + 3, 3)); + REQUIRE(bufcmp(buf + 3, cmp + 3, buf_size - 3)); - spi.read(0x05, read, 4, false); + spi.read(0x05, buf, 4, false); REQUIRE_FALSE(bus.isSelected()); REQUIRE(bus.out_buf.size() == 4); REQUIRE(bus.out_buf.back() == 0x05); - REQUIRE(bufcmp(read, bus.in_buf.data() + 6, 4)); + REQUIRE(bufcmp(buf, bus.in_buf.data() + 6, 4)); + REQUIRE(bufcmp(buf + 4, cmp + 4, buf_size - 4)); } SECTION("multi byte raw read") { - uint8_t read[4] = {0, 0, 0, 0}; - uint8_t zero[4] = {0, 0, 0, 0}; + const int buf_size = 7; + uint8_t buf[] = {1, 2, 3, 4, 5, 6, 7}; + uint8_t cmp[] = {1, 2, 3, 4, 5, 6, 7}; - spi.read(read, 0); + spi.read(buf, 0); REQUIRE_FALSE(bus.isSelected()); REQUIRE(bus.out_buf.size() == 0); - REQUIRE(bufcmp(read, zero, 4)); + REQUIRE(bufcmp(buf, cmp, buf_size)); - spi.read(read, 3); + spi.read(buf, 3); REQUIRE_FALSE(bus.isSelected()); REQUIRE(bus.out_buf.size() == 0); - REQUIRE(bufcmp(read, bus.in_buf.data(), 3)); + REQUIRE(bufcmp(buf, bus.in_buf.data(), 3)); + REQUIRE(bufcmp(buf + 3, cmp + 3, buf_size - 3)); } } } @@ -470,8 +492,8 @@ TEST_CASE("SPITransaction - transfer") SPIBusConfig config1; - config1.mode = SPIMode::MODE1; - config1.clock_div = SPIClockDivider::DIV32; + config1.mode = SPIMode::MODE1; + config1.clock_div = SPIClockDivider::DIV32; bus.expected_config = config1; @@ -480,18 +502,20 @@ TEST_CASE("SPITransaction - transfer") SPISlave slave(bus, GpioPin(GPIOA_BASE, 1), config1); SPITransaction spi(slave); - uint8_t buf[4] = {4, 3, 2, 1}; - uint8_t cmp[4] = {4, 3, 2, 1}; + const int buf_size = 7; + uint8_t buf[] = {1, 2, 3, 4, 5, 6, 7}; + uint8_t cmp[] = {1, 2, 3, 4, 5, 6, 7}; spi.transfer(buf, 0); REQUIRE_FALSE(bus.isSelected()); REQUIRE(bus.out_buf.size() == 0); - REQUIRE(bufcmp(buf, cmp, 4)); + REQUIRE(bufcmp(buf, cmp, buf_size)); spi.transfer(buf, 4); REQUIRE_FALSE(bus.isSelected()); REQUIRE(bus.out_buf.size() == 4); REQUIRE(bufcmp(buf, bus.in_buf.data(), 4)); REQUIRE(bufcmp(cmp, bus.out_buf.data(), 4)); + REQUIRE(bufcmp(buf + 4, cmp + 4, buf_size - 4)); } } \ No newline at end of file