From f8edea9c6895031880997be8e7f947dc450cfea5 Mon Sep 17 00:00:00 2001 From: Daniel Schloms Date: Sun, 4 Feb 2024 16:16:49 +0100 Subject: [PATCH 01/10] WIP: Refactor simple things (Span(), some magic numbers) --- Sts1CobcSw/Periphery/Rf.cpp | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/Sts1CobcSw/Periphery/Rf.cpp b/Sts1CobcSw/Periphery/Rf.cpp index 0d4e165c..0051a89e 100644 --- a/Sts1CobcSw/Periphery/Rf.cpp +++ b/Sts1CobcSw/Periphery/Rf.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include @@ -84,6 +85,12 @@ auto paEnablePin = hal::GpioPin(hal::rfPaEnablePin); // TODO: This should probably be somewhere else as it is not directly related to the RF module auto watchdogResetGpioPin = hal::GpioPin(hal::watchdogClearPin); +// Pause values for pin setting/resetting and PoR +// TODO: Could not find all of them in datasheet, ask Jakob +constexpr auto csPinAfterResetPause = 20 * MICROSECONDS; +constexpr auto porPause = 20 * MILLISECONDS; // Time for Power on Reset (PoR) itself +constexpr auto porCircuitSettlePause = + 100 * MILLISECONDS; // Time until PoR circuit settles after applying power // --- Private function declarations --- @@ -623,8 +630,8 @@ auto Initialize(TxType txType) -> void auto ReadPartInfo() -> std::uint16_t { - auto const sendBuffer = std::to_array({cmdPartInfo}); - auto responseBuffer = SendCommandWithResponse(sendBuffer); + auto sendBuffer = std::to_array({cmdPartInfo}); + auto responseBuffer = SendCommandWithResponse(Span(sendBuffer)); // NOLINTNEXTLINE(hicpp-signed-bitwise) return static_cast(static_cast(responseBuffer[1]) << CHAR_BIT @@ -664,9 +671,9 @@ auto InitializeGpioAndSpi() -> void } // Enable Si4463 and wait for PoR to finish - AT(NOW() + 100 * MILLISECONDS); + AT(NOW() + porCircuitSettlePause); sdnGpioPin.Reset(); - AT(NOW() + 20 * MILLISECONDS); + AT(NOW() + porPause); } @@ -683,7 +690,7 @@ auto PowerUp(PowerUpBootOptions bootOptions, static_cast(xoFrequency >> (CHAR_BIT)), // NOLINT(hicpp-signed-bitwise) static_cast(xoFrequency)}); - SendCommandNoResponse(powerUpBuffer); + SendCommandNoResponse(Span(powerUpBuffer)); } @@ -694,7 +701,7 @@ auto PowerUp(PowerUpBootOptions bootOptions, { // RODOS::PRINTF("SendCommand()\n"); csGpioPin.Reset(); - AT(NOW() + 20 * MICROSECONDS); + AT(NOW() + csPinAfterResetPause); spi.write(data, length); AT(NOW() + 2 * MICROSECONDS); csGpioPin.Set(); @@ -705,7 +712,7 @@ auto PowerUp(PowerUpBootOptions bootOptions, { AT(NOW() + 20 * MICROSECONDS); csGpioPin.Reset(); - AT(NOW() + 20 * MICROSECONDS); + AT(NOW() + csPinAfterResetPause); spi.writeRead(std::data(req), std::size(req), std::data(cts), std::size(cts)); if(cts[1] != 0xFF) { @@ -727,7 +734,7 @@ auto PowerUp(PowerUpBootOptions bootOptions, auto SendCommandNoResponse(std::span commandBuffer) -> void { csGpioPin.Reset(); - AT(NOW() + 20 * MICROSECONDS); + AT(NOW() + csPinAfterResetPause); hal::WriteTo(&spi, commandBuffer); AT(NOW() + 2 * MICROSECONDS); csGpioPin.Set(); @@ -742,7 +749,7 @@ auto SendCommandWithResponse(std::span commandBuffer) -> std::array { csGpioPin.Reset(); - AT(NOW() + 20 * MICROSECONDS); + AT(NOW() + csPinAfterResetPause); hal::WriteTo(&spi, commandBuffer); AT(NOW() + 2 * MICROSECONDS); csGpioPin.Set(); @@ -750,7 +757,7 @@ auto SendCommandWithResponse(std::span commandBuffer) auto responseBuffer = std::array{}; WaitOnCts(); // WaitOnCts leaves CS pin low, read response afterwards - hal::ReadFrom(&spi, std::span(responseBuffer)); + hal::ReadFrom(&spi, Span(&responseBuffer)); csGpioPin.Set(); return responseBuffer; @@ -765,11 +772,11 @@ auto WaitOnCts() -> void { AT(NOW() + 20 * MICROSECONDS); csGpioPin.Reset(); - AT(NOW() + 20 * MICROSECONDS); + AT(NOW() + csPinAfterResetPause); - hal::WriteTo(&spi, std::span(sendBuffer)); + hal::WriteTo(&spi, Span(sendBuffer)); auto ctsBuffer = std::array{}; - hal::ReadFrom(&spi, std::span(ctsBuffer)); + hal::ReadFrom(&spi, Span(&ctsBuffer)); if(ctsBuffer[0] != readyCtsByte) { From 1b00ae73f8d2f48659d8ab1c1dc4b559cfc93ae6 Mon Sep 17 00:00:00 2001 From: Daniel Schloms Date: Sun, 4 Feb 2024 16:44:54 +0100 Subject: [PATCH 02/10] Move pause time before setting CS pin into variable --- Sts1CobcSw/Periphery/Rf.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/Sts1CobcSw/Periphery/Rf.cpp b/Sts1CobcSw/Periphery/Rf.cpp index 0051a89e..2566cd22 100644 --- a/Sts1CobcSw/Periphery/Rf.cpp +++ b/Sts1CobcSw/Periphery/Rf.cpp @@ -88,6 +88,7 @@ auto watchdogResetGpioPin = hal::GpioPin(hal::watchdogClearPin); // Pause values for pin setting/resetting and PoR // TODO: Could not find all of them in datasheet, ask Jakob constexpr auto csPinAfterResetPause = 20 * MICROSECONDS; +constexpr auto csPinPreSetPause = 2 * MICROSECONDS; constexpr auto porPause = 20 * MILLISECONDS; // Time for Power on Reset (PoR) itself constexpr auto porCircuitSettlePause = 100 * MILLISECONDS; // Time until PoR circuit settles after applying power @@ -703,7 +704,7 @@ auto PowerUp(PowerUpBootOptions bootOptions, csGpioPin.Reset(); AT(NOW() + csPinAfterResetPause); spi.write(data, length); - AT(NOW() + 2 * MICROSECONDS); + AT(NOW() + csPinPreSetPause); csGpioPin.Set(); auto cts = std::to_array({0x00, 0x00}); @@ -716,7 +717,7 @@ auto PowerUp(PowerUpBootOptions bootOptions, spi.writeRead(std::data(req), std::size(req), std::data(cts), std::size(cts)); if(cts[1] != 0xFF) { - AT(NOW() + 2 * MICROSECONDS); + AT(NOW() + csPinPreSetPause); csGpioPin.Set(); } } while(cts[1] != 0xFF); @@ -726,7 +727,7 @@ auto PowerUp(PowerUpBootOptions bootOptions, spi.read(responseData, responseLength); } - AT(NOW() + 2 * MICROSECONDS); + AT(NOW() + csPinPreSetPause); csGpioPin.Set(); } @@ -736,7 +737,7 @@ auto SendCommandNoResponse(std::span commandBuffer) -> void csGpioPin.Reset(); AT(NOW() + csPinAfterResetPause); hal::WriteTo(&spi, commandBuffer); - AT(NOW() + 2 * MICROSECONDS); + AT(NOW() + csPinPreSetPause); csGpioPin.Set(); WaitOnCts(); // No response -> just set the CS pin again @@ -751,7 +752,7 @@ auto SendCommandWithResponse(std::span commandBuffer) csGpioPin.Reset(); AT(NOW() + csPinAfterResetPause); hal::WriteTo(&spi, commandBuffer); - AT(NOW() + 2 * MICROSECONDS); + AT(NOW() + csPinPreSetPause); csGpioPin.Set(); auto responseBuffer = std::array{}; @@ -780,7 +781,7 @@ auto WaitOnCts() -> void if(ctsBuffer[0] != readyCtsByte) { - AT(NOW() + 2 * MICROSECONDS); + AT(NOW() + csPinPreSetPause); csGpioPin.Set(); } else From f4dc40a0b50bad8bb3acb6a9b702693b17830b71 Mon Sep 17 00:00:00 2001 From: Daniel Schloms Date: Sun, 4 Feb 2024 16:57:28 +0100 Subject: [PATCH 03/10] Add variables for all the various magic number timings --- Sts1CobcSw/Periphery/Rf.cpp | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/Sts1CobcSw/Periphery/Rf.cpp b/Sts1CobcSw/Periphery/Rf.cpp index 2566cd22..4acd89a7 100644 --- a/Sts1CobcSw/Periphery/Rf.cpp +++ b/Sts1CobcSw/Periphery/Rf.cpp @@ -86,12 +86,20 @@ auto paEnablePin = hal::GpioPin(hal::rfPaEnablePin); auto watchdogResetGpioPin = hal::GpioPin(hal::watchdogClearPin); // Pause values for pin setting/resetting and PoR -// TODO: Could not find all of them in datasheet, ask Jakob -constexpr auto csPinAfterResetPause = 20 * MICROSECONDS; -constexpr auto csPinPreSetPause = 2 * MICROSECONDS; -constexpr auto porPause = 20 * MILLISECONDS; // Time for Power on Reset (PoR) itself +// Jakob: Pause times are VERY generously overestimated +constexpr auto csPinAfterResetPause = + 20 * MICROSECONDS; // Pause time after pulling NSEL (here CS) low +constexpr auto csPinPreSetPause = + 2 * MICROSECONDS; // Pause time before pulling NSEL (here CS) high +constexpr auto porRunningPause = + 20 * MILLISECONDS; // Pause time to wait for Power on Reset to finish constexpr auto porCircuitSettlePause = 100 * MILLISECONDS; // Time until PoR circuit settles after applying power +constexpr auto waitOnCtsStartPause = + 20 * MICROSECONDS; // Pause time at the beginning of the CTS wait loop +constexpr auto watchDogResetPinPause = + 1 * MILLISECONDS; // Pause time for the sequence reset -> pause -> set -> pause -> reset in + // initialization // --- Private function declarations --- @@ -657,9 +665,9 @@ auto InitializeGpioAndSpi() -> void watchdogResetGpioPin.Direction(hal::PinDirection::out); watchdogResetGpioPin.Reset(); - AT(NOW() + 1 * MILLISECONDS); + AT(NOW() + watchDogResetPinPause); watchdogResetGpioPin.Set(); - AT(NOW() + 1 * MILLISECONDS); + AT(NOW() + watchDogResetPinPause); watchdogResetGpioPin.Reset(); constexpr auto baudrate = 10'000'000; @@ -674,7 +682,7 @@ auto InitializeGpioAndSpi() -> void // Enable Si4463 and wait for PoR to finish AT(NOW() + porCircuitSettlePause); sdnGpioPin.Reset(); - AT(NOW() + porPause); + AT(NOW() + porRunningPause); } @@ -711,7 +719,7 @@ auto PowerUp(PowerUpBootOptions bootOptions, auto req = std::to_array({0x44, 0x00}); do { - AT(NOW() + 20 * MICROSECONDS); + AT(NOW() + waitOnCtsStartPause); csGpioPin.Reset(); AT(NOW() + csPinAfterResetPause); spi.writeRead(std::data(req), std::size(req), std::data(cts), std::size(cts)); @@ -771,7 +779,7 @@ auto WaitOnCts() -> void auto sendBuffer = std::to_array({cmdReadCmdBuff}); do { - AT(NOW() + 20 * MICROSECONDS); + AT(NOW() + waitOnCtsStartPause); csGpioPin.Reset(); AT(NOW() + csPinAfterResetPause); From d9223e735044595d1f362a4ce20d76944bac5c7a Mon Sep 17 00:00:00 2001 From: Daniel Schloms Date: Sun, 4 Feb 2024 17:04:23 +0100 Subject: [PATCH 04/10] Initialize RF SPI with our hal::Initialize() --- Sts1CobcSw/Periphery/Rf.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/Sts1CobcSw/Periphery/Rf.cpp b/Sts1CobcSw/Periphery/Rf.cpp index 4acd89a7..8b9a5598 100644 --- a/Sts1CobcSw/Periphery/Rf.cpp +++ b/Sts1CobcSw/Periphery/Rf.cpp @@ -671,13 +671,7 @@ auto InitializeGpioAndSpi() -> void watchdogResetGpioPin.Reset(); constexpr auto baudrate = 10'000'000; - auto spiError = spi.init(baudrate, /*slave=*/false, /*tiMode=*/false); - if(spiError == -1) - { - RODOS::PRINTF("Error initializing RF SPI!\n"); - // TODO: proper error handling - return; - } + hal::Initialize(&spi, baudrate); // Enable Si4463 and wait for PoR to finish AT(NOW() + porCircuitSettlePause); From f7f36b1b6bfbb6e55c449122edc28438b5b94fba Mon Sep 17 00:00:00 2001 From: Daniel Schloms Date: Thu, 15 Feb 2024 10:41:52 +0100 Subject: [PATCH 05/10] Remove periphery namespace This is done to adhere to #139 --- Sts1CobcSw/Periphery/Rf.cpp | 2 +- Sts1CobcSw/Periphery/Rf.hpp | 2 +- Tests/HardwareTests/Rf.test.cpp | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Sts1CobcSw/Periphery/Rf.cpp b/Sts1CobcSw/Periphery/Rf.cpp index 8b9a5598..609fd93d 100644 --- a/Sts1CobcSw/Periphery/Rf.cpp +++ b/Sts1CobcSw/Periphery/Rf.cpp @@ -12,7 +12,7 @@ #include -namespace sts1cobcsw::periphery::rf +namespace sts1cobcsw::rf { using RODOS::AT; using RODOS::MICROSECONDS; diff --git a/Sts1CobcSw/Periphery/Rf.hpp b/Sts1CobcSw/Periphery/Rf.hpp index a874233f..830f442c 100644 --- a/Sts1CobcSw/Periphery/Rf.hpp +++ b/Sts1CobcSw/Periphery/Rf.hpp @@ -4,7 +4,7 @@ #include -namespace sts1cobcsw::periphery::rf +namespace sts1cobcsw::rf { enum class TxType { diff --git a/Tests/HardwareTests/Rf.test.cpp b/Tests/HardwareTests/Rf.test.cpp index d6557a93..27d44786 100644 --- a/Tests/HardwareTests/Rf.test.cpp +++ b/Tests/HardwareTests/Rf.test.cpp @@ -28,12 +28,12 @@ class RfTest : public RODOS::StaticThread<> { PRINTF("\nRF test\n\n"); - periphery::rf::Initialize(periphery::rf::TxType::morse); + rf::Initialize(rf::TxType::morse); PRINTF("RF module initialized\n"); PRINTF("\n"); auto correctPartInfo = 0x4463; - auto partInfo = periphery::rf::ReadPartInfo(); + auto partInfo = rf::ReadPartInfo(); PRINTF("Part info: 0x%4x == 0x%4x\n", partInfo, correctPartInfo); Check(partInfo == correctPartInfo); From 61209dce57504f71e18b6c96ffd1eb1d31c9f109 Mon Sep 17 00:00:00 2001 From: Patrick Kappl Date: Sun, 3 Mar 2024 14:29:11 +0000 Subject: [PATCH 06/10] Fix inconsistent template parameter name --- Sts1CobcSw/Periphery/Flash.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sts1CobcSw/Periphery/Flash.cpp b/Sts1CobcSw/Periphery/Flash.cpp index a2121b6c..5d35e88e 100644 --- a/Sts1CobcSw/Periphery/Flash.cpp +++ b/Sts1CobcSw/Periphery/Flash.cpp @@ -222,8 +222,8 @@ auto DisableWriting() -> void // TODO:: Maybe this is one level of indirection too much? -template -inline auto Write(std::span data) -> void +template +inline auto Write(std::span data) -> void { hal::WriteTo(&spi, data); } From d95329173139f0562ea6c2c45761c759f656a68b Mon Sep 17 00:00:00 2001 From: Patrick Kappl Date: Sun, 3 Mar 2024 14:35:45 +0000 Subject: [PATCH 07/10] Rename ReadPartInfo() to ReadPartNumber() The function only returns the part of the part info that the application note calls "part number". --- Sts1CobcSw/Periphery/Rf.cpp | 2 +- Sts1CobcSw/Periphery/Rf.hpp | 2 +- Tests/HardwareTests/Rf.test.cpp | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Sts1CobcSw/Periphery/Rf.cpp b/Sts1CobcSw/Periphery/Rf.cpp index 609fd93d..b79d850e 100644 --- a/Sts1CobcSw/Periphery/Rf.cpp +++ b/Sts1CobcSw/Periphery/Rf.cpp @@ -637,7 +637,7 @@ auto Initialize(TxType txType) -> void } -auto ReadPartInfo() -> std::uint16_t +auto ReadPartNumber() -> std::uint16_t { auto sendBuffer = std::to_array({cmdPartInfo}); auto responseBuffer = SendCommandWithResponse(Span(sendBuffer)); diff --git a/Sts1CobcSw/Periphery/Rf.hpp b/Sts1CobcSw/Periphery/Rf.hpp index 830f442c..fa19becc 100644 --- a/Sts1CobcSw/Periphery/Rf.hpp +++ b/Sts1CobcSw/Periphery/Rf.hpp @@ -14,5 +14,5 @@ enum class TxType auto Initialize(TxType txType) -> void; -auto ReadPartInfo() -> std::uint16_t; +auto ReadPartNumber() -> std::uint16_t; } \ No newline at end of file diff --git a/Tests/HardwareTests/Rf.test.cpp b/Tests/HardwareTests/Rf.test.cpp index 27d44786..68d249c5 100644 --- a/Tests/HardwareTests/Rf.test.cpp +++ b/Tests/HardwareTests/Rf.test.cpp @@ -32,10 +32,10 @@ class RfTest : public RODOS::StaticThread<> PRINTF("RF module initialized\n"); PRINTF("\n"); - auto correctPartInfo = 0x4463; - auto partInfo = rf::ReadPartInfo(); - PRINTF("Part info: 0x%4x == 0x%4x\n", partInfo, correctPartInfo); - Check(partInfo == correctPartInfo); + auto correctPartNumber = 0x4463; + auto partNumber = rf::ReadPartNumber(); + PRINTF("Part number: 0x%4x == 0x%4x\n", partNumber, correctPartNumber); + Check(partNumber == correctPartNumber); // Here comes the rest of the RF test } From 5209247bd1d67f07f2372f4211b47ed35783d15d Mon Sep 17 00:00:00 2001 From: Patrick Kappl Date: Sun, 3 Mar 2024 15:54:18 +0000 Subject: [PATCH 08/10] Refactor WaitOnCts() and rename it to WaitForCts() --- Sts1CobcSw/Periphery/Rf.cpp | 47 +++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/Sts1CobcSw/Periphery/Rf.cpp b/Sts1CobcSw/Periphery/Rf.cpp index b79d850e..a922e99a 100644 --- a/Sts1CobcSw/Periphery/Rf.cpp +++ b/Sts1CobcSw/Periphery/Rf.cpp @@ -66,9 +66,6 @@ constexpr auto cmdReadCmdBuff = 0x44_b; // Command response lengths constexpr auto partInfoResponseLength = 8U; -// Check for this value when waiting for the Si4463 (WaitOnCts()) -constexpr auto readyCtsByte = 0xFF_b; - // Max. number of properties that can be set in a single command constexpr auto maxNProperties = 12; constexpr auto setPropertyHeaderSize = 4; @@ -87,6 +84,10 @@ auto watchdogResetGpioPin = hal::GpioPin(hal::watchdogClearPin); // Pause values for pin setting/resetting and PoR // Jakob: Pause times are VERY generously overestimated +// TODO: Patrick: Are those delays really necessary? I have never seen something like that for SPI +// communication +// TODO: Use delay instead of pause, because that's how we did it everywhere else +// TODO: Do not use trailing comments since they cause line breaks constexpr auto csPinAfterResetPause = 20 * MICROSECONDS; // Pause time after pulling NSEL (here CS) low constexpr auto csPinPreSetPause = @@ -95,7 +96,7 @@ constexpr auto porRunningPause = 20 * MILLISECONDS; // Pause time to wait for Power on Reset to finish constexpr auto porCircuitSettlePause = 100 * MILLISECONDS; // Time until PoR circuit settles after applying power -constexpr auto waitOnCtsStartPause = +constexpr auto initialWaitForCtsDelay = 20 * MICROSECONDS; // Pause time at the beginning of the CTS wait loop constexpr auto watchDogResetPinPause = 1 * MILLISECONDS; // Pause time for the sequence reset -> pause -> set -> pause -> reset in @@ -117,7 +118,7 @@ template auto SendCommandWithResponse(std::span commandBuffer) -> std::array; -auto WaitOnCts() -> void; +auto WaitForCts() -> void; auto SetTxType(TxType txType) -> void; template requires(nProperties >= 1 and nProperties <= maxNProperties) @@ -713,7 +714,7 @@ auto PowerUp(PowerUpBootOptions bootOptions, auto req = std::to_array({0x44, 0x00}); do { - AT(NOW() + waitOnCtsStartPause); + AT(NOW() + initialWaitForCtsDelay); csGpioPin.Reset(); AT(NOW() + csPinAfterResetPause); spi.writeRead(std::data(req), std::size(req), std::data(cts), std::size(cts)); @@ -741,9 +742,7 @@ auto SendCommandNoResponse(std::span commandBuffer) -> void hal::WriteTo(&spi, commandBuffer); AT(NOW() + csPinPreSetPause); csGpioPin.Set(); - WaitOnCts(); - // No response -> just set the CS pin again - csGpioPin.Set(); + WaitForCts(); } @@ -758,8 +757,9 @@ auto SendCommandWithResponse(std::span commandBuffer) csGpioPin.Set(); auto responseBuffer = std::array{}; - WaitOnCts(); - // WaitOnCts leaves CS pin low, read response afterwards + WaitForCts(); + csGpioPin.Reset(); + AT(NOW() + csPinAfterResetPause); hal::ReadFrom(&spi, Span(&responseBuffer)); csGpioPin.Set(); @@ -768,29 +768,26 @@ auto SendCommandWithResponse(std::span commandBuffer) //! @brief Polls the CTS byte until 0xFF is received (i.e. Si4463 is ready for command). -auto WaitOnCts() -> void +auto WaitForCts() -> void { - auto sendBuffer = std::to_array({cmdReadCmdBuff}); + auto const dataIsReadyValue = 0xFF_b; + AT(NOW() + initialWaitForCtsDelay); do { - AT(NOW() + waitOnCtsStartPause); csGpioPin.Reset(); AT(NOW() + csPinAfterResetPause); - - hal::WriteTo(&spi, Span(sendBuffer)); - auto ctsBuffer = std::array{}; - hal::ReadFrom(&spi, Span(&ctsBuffer)); - - if(ctsBuffer[0] != readyCtsByte) - { - AT(NOW() + csPinPreSetPause); - csGpioPin.Set(); - } - else + hal::WriteTo(&spi, Span(cmdReadCmdBuff)); + auto cts = 0x00_b; + hal::ReadFrom(&spi, Span(&cts)); + AT(NOW() + csPinPreSetPause); + csGpioPin.Set(); + if(cts == dataIsReadyValue) { break; } } while(true); + // TODO: We need to get rid of this infinite loop once we do proper error handling for the whole + // RF code } From 7e44f5583914cace8a9768888054b49db4911bab Mon Sep 17 00:00:00 2001 From: Patrick Kappl Date: Sun, 3 Mar 2024 16:01:08 +0000 Subject: [PATCH 09/10] Refactor SendCommand...() and rename them to SendCommand() --- Sts1CobcSw/Periphery/Rf.cpp | 36 ++++++++++++++---------------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/Sts1CobcSw/Periphery/Rf.cpp b/Sts1CobcSw/Periphery/Rf.cpp index a922e99a..c65db994 100644 --- a/Sts1CobcSw/Periphery/Rf.cpp +++ b/Sts1CobcSw/Periphery/Rf.cpp @@ -64,7 +64,7 @@ constexpr auto cmdSetProperty = 0x11_b; constexpr auto cmdReadCmdBuff = 0x44_b; // Command response lengths -constexpr auto partInfoResponseLength = 8U; +constexpr auto partInfoAnswerLength = 8U; // Max. number of properties that can be set in a single command constexpr auto maxNProperties = 12; @@ -113,10 +113,9 @@ auto PowerUp(PowerUpBootOptions bootOptions, std::size_t length, std::uint8_t * responseData, std::size_t responseLength) -> void; -auto SendCommandNoResponse(std::span commandBuffer) -> void; -template -auto SendCommandWithResponse(std::span commandBuffer) - -> std::array; +auto SendCommand(std::span data) -> void; +template +auto SendCommand(std::span data) -> std::array; auto WaitForCts() -> void; auto SetTxType(TxType txType) -> void; @@ -694,7 +693,7 @@ auto PowerUp(PowerUpBootOptions bootOptions, static_cast(xoFrequency >> (CHAR_BIT)), // NOLINT(hicpp-signed-bitwise) static_cast(xoFrequency)}); - SendCommandNoResponse(Span(powerUpBuffer)); + SendCommand(Span(powerUpBuffer)); } @@ -735,35 +734,28 @@ auto PowerUp(PowerUpBootOptions bootOptions, } -auto SendCommandNoResponse(std::span commandBuffer) -> void +auto SendCommand(std::span data) -> void { csGpioPin.Reset(); AT(NOW() + csPinAfterResetPause); - hal::WriteTo(&spi, commandBuffer); + hal::WriteTo(&spi, data); AT(NOW() + csPinPreSetPause); csGpioPin.Set(); WaitForCts(); } -template -auto SendCommandWithResponse(std::span commandBuffer) - -> std::array +template +auto SendCommand(std::span data) -> std::array { + SendCommand(data); + auto answer = std::array{}; csGpioPin.Reset(); AT(NOW() + csPinAfterResetPause); - hal::WriteTo(&spi, commandBuffer); + hal::ReadFrom(&spi, Span(&answer)); AT(NOW() + csPinPreSetPause); csGpioPin.Set(); - - auto responseBuffer = std::array{}; - WaitForCts(); - csGpioPin.Reset(); - AT(NOW() + csPinAfterResetPause); - hal::ReadFrom(&spi, Span(&responseBuffer)); - csGpioPin.Set(); - - return responseBuffer; + return answer; } @@ -842,6 +834,6 @@ auto SetProperty(PropertyGroup propertyGroup, bufferIndex++; } - SendCommandNoResponse(setPropertyBuffer); + SendCommand(setPropertyBuffer); } } From 75d608e8c6e18c51af1d1c91c10a40d42cb0bfa2 Mon Sep 17 00:00:00 2001 From: Patrick Kappl Date: Sun, 3 Mar 2024 16:26:02 +0000 Subject: [PATCH 10/10] Refactor ReadPartNumber() --- Sts1CobcSw/Periphery/Rf.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/Sts1CobcSw/Periphery/Rf.cpp b/Sts1CobcSw/Periphery/Rf.cpp index c65db994..4ed56099 100644 --- a/Sts1CobcSw/Periphery/Rf.cpp +++ b/Sts1CobcSw/Periphery/Rf.cpp @@ -3,11 +3,13 @@ #include #include #include +#include #include #include #include +#include #include #include @@ -63,7 +65,10 @@ constexpr auto cmdPowerUp = 0x02_b; constexpr auto cmdSetProperty = 0x11_b; constexpr auto cmdReadCmdBuff = 0x44_b; -// Command response lengths +// Command answer lengths +// TODO: We should do it similarly to SimpleInstruction and SendInstruction() in Flash.cpp. Unless +// this remains the only command with an answer. Then we should probably get rid of SendCommand<>() +// instead. constexpr auto partInfoAnswerLength = 8U; // Max. number of properties that can be set in a single command @@ -102,6 +107,7 @@ constexpr auto watchDogResetPinPause = 1 * MILLISECONDS; // Pause time for the sequence reset -> pause -> set -> pause -> reset in // initialization + // --- Private function declarations --- auto InitializeGpioAndSpi() -> void; @@ -639,12 +645,8 @@ auto Initialize(TxType txType) -> void auto ReadPartNumber() -> std::uint16_t { - auto sendBuffer = std::to_array({cmdPartInfo}); - auto responseBuffer = SendCommandWithResponse(Span(sendBuffer)); - - // NOLINTNEXTLINE(hicpp-signed-bitwise) - return static_cast(static_cast(responseBuffer[1]) << CHAR_BIT - | static_cast(responseBuffer[2])); + auto answer = SendCommand(Span(cmdPartInfo)); + return Deserialize(Span(answer).subspan<1, 2>()); }