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); } diff --git a/Sts1CobcSw/Periphery/Rf.cpp b/Sts1CobcSw/Periphery/Rf.cpp index 0d4e165c..4ed56099 100644 --- a/Sts1CobcSw/Periphery/Rf.cpp +++ b/Sts1CobcSw/Periphery/Rf.cpp @@ -3,15 +3,18 @@ #include #include #include +#include +#include #include #include +#include #include #include -namespace sts1cobcsw::periphery::rf +namespace sts1cobcsw::rf { using RODOS::AT; using RODOS::MICROSECONDS; @@ -62,11 +65,11 @@ constexpr auto cmdPowerUp = 0x02_b; constexpr auto cmdSetProperty = 0x11_b; 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; +// 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 constexpr auto maxNProperties = 12; @@ -84,6 +87,26 @@ 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 +// 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 = + 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 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 + // initialization + // --- Private function declarations --- @@ -96,12 +119,11 @@ 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 WaitOnCts() -> void; +auto WaitForCts() -> void; auto SetTxType(TxType txType) -> void; template requires(nProperties >= 1 and nProperties <= maxNProperties) @@ -621,14 +643,10 @@ auto Initialize(TxType txType) -> void } -auto ReadPartInfo() -> std::uint16_t +auto ReadPartNumber() -> std::uint16_t { - auto const sendBuffer = std::to_array({cmdPartInfo}); - auto responseBuffer = SendCommandWithResponse(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>()); } @@ -649,24 +667,18 @@ 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; - 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() + 100 * MILLISECONDS); + AT(NOW() + porCircuitSettlePause); sdnGpioPin.Reset(); - AT(NOW() + 20 * MILLISECONDS); + AT(NOW() + porRunningPause); } @@ -683,7 +695,7 @@ auto PowerUp(PowerUpBootOptions bootOptions, static_cast(xoFrequency >> (CHAR_BIT)), // NOLINT(hicpp-signed-bitwise) static_cast(xoFrequency)}); - SendCommandNoResponse(powerUpBuffer); + SendCommand(Span(powerUpBuffer)); } @@ -694,22 +706,22 @@ 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); + AT(NOW() + csPinPreSetPause); csGpioPin.Set(); auto cts = std::to_array({0x00, 0x00}); auto req = std::to_array({0x44, 0x00}); do { - AT(NOW() + 20 * MICROSECONDS); + AT(NOW() + initialWaitForCtsDelay); 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) { - AT(NOW() + 2 * MICROSECONDS); + AT(NOW() + csPinPreSetPause); csGpioPin.Set(); } } while(cts[1] != 0xFF); @@ -719,68 +731,57 @@ auto PowerUp(PowerUpBootOptions bootOptions, spi.read(responseData, responseLength); } - AT(NOW() + 2 * MICROSECONDS); + AT(NOW() + csPinPreSetPause); csGpioPin.Set(); } -auto SendCommandNoResponse(std::span commandBuffer) -> void +auto SendCommand(std::span data) -> void { csGpioPin.Reset(); - AT(NOW() + 20 * MICROSECONDS); - hal::WriteTo(&spi, commandBuffer); - AT(NOW() + 2 * MICROSECONDS); - csGpioPin.Set(); - WaitOnCts(); - // No response -> just set the CS pin again + AT(NOW() + csPinAfterResetPause); + 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() + 20 * MICROSECONDS); - hal::WriteTo(&spi, commandBuffer); - AT(NOW() + 2 * MICROSECONDS); - csGpioPin.Set(); - - auto responseBuffer = std::array{}; - WaitOnCts(); - // WaitOnCts leaves CS pin low, read response afterwards - hal::ReadFrom(&spi, std::span(responseBuffer)); + AT(NOW() + csPinAfterResetPause); + hal::ReadFrom(&spi, Span(&answer)); + AT(NOW() + csPinPreSetPause); csGpioPin.Set(); - - return responseBuffer; + return answer; } //! @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() + 20 * MICROSECONDS); csGpioPin.Reset(); - AT(NOW() + 20 * MICROSECONDS); - - hal::WriteTo(&spi, std::span(sendBuffer)); - auto ctsBuffer = std::array{}; - hal::ReadFrom(&spi, std::span(ctsBuffer)); - - if(ctsBuffer[0] != readyCtsByte) - { - AT(NOW() + 2 * MICROSECONDS); - csGpioPin.Set(); - } - else + AT(NOW() + csPinAfterResetPause); + 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 } @@ -835,6 +836,6 @@ auto SetProperty(PropertyGroup propertyGroup, bufferIndex++; } - SendCommandNoResponse(setPropertyBuffer); + SendCommand(setPropertyBuffer); } } diff --git a/Sts1CobcSw/Periphery/Rf.hpp b/Sts1CobcSw/Periphery/Rf.hpp index a874233f..fa19becc 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 { @@ -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 d6557a93..68d249c5 100644 --- a/Tests/HardwareTests/Rf.test.cpp +++ b/Tests/HardwareTests/Rf.test.cpp @@ -28,14 +28,14 @@ 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(); - 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 }