diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index eab409f40..0d333b769 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.github/workflows/run-unit-tests.yml @@ -222,6 +222,7 @@ jobs: python3 -B test/test_rs232_runtime.py -v python3 -B test/test_rs232_mqtt_runtime.py -v python3 -B test/test_rs232_driver_lifecycle.py -v + python3 -B test/test_rs232_nrf_uart_lifecycle.py -v python3 -B test/test_rs232_heartbeat.py -v python3 -B test/test_rs232_heartbeat_owner.py -v python3 -B test/test_rs232_power_settings.py -v diff --git a/src/helpers/bridges/RS232UartUtils.h b/src/helpers/bridges/RS232UartUtils.h index 259f9ed85..afcb57725 100644 --- a/src/helpers/bridges/RS232UartUtils.h +++ b/src/helpers/bridges/RS232UartUtils.h @@ -5,7 +5,10 @@ namespace bridge { template inline void prepareNrfUart(UartType& uart, PinType rx, PinType tx) { - uart.end(); + // Adafruit Uart::end() requires a begun peripheral and a live TX semaphore. + // Its public bool reports that state; a fresh or already stopped UART must + // not wait for stop events or delete an unallocated semaphore. + if (uart) uart.end(); uart.setPins(rx, tx); } diff --git a/test/fixtures/nrf52_uart_lifecycle/core_uart.inc b/test/fixtures/nrf52_uart_lifecycle/core_uart.inc new file mode 100644 index 000000000..51e2231e3 --- /dev/null +++ b/test/fixtures/nrf52_uart_lifecycle/core_uart.inc @@ -0,0 +1,76 @@ +/* + Copyright (c) 2015 Arduino LLC. All right reserved. + Copyright (c) 2016 Sandeep Mistry. All right reserved. + + This library is free software; you can redistribute it and/or modify it + under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2.1 of the License, or (at + your option) any later version. + + This library is distributed in the hope that it will be useful, but WITHOUT + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + for more details. + + You should have received a copy of the GNU Lesser General Public License + along with this library; if not, write to the Free Software Foundation, + Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. +*/ + +// Frozen narrow core methods, independent of a CI package installation. +// Source: framework-arduinoadafruitnrf52 1.10701.0, cores/nRF5/Uart.cpp. +// Repository: https://github.com/meshcore-dev/Adafruit_nRF52_Arduino +// Complete source SHA256: +// 369f8847272f7884aabacce10a395099616212df614f7728931e212f3fce9df5 +// Uart.h public bool returns _begun; complete header SHA256: +// f26b2a82c40925d5ebcdaa5806dd7767941098389d22aa55379c7ba7c190dacf +// Constructor, pin setter and end body below are copied without changes. +Uart::Uart(NRF_UARTE_Type *_nrfUart, IRQn_Type _IRQn, uint8_t _pinRX, uint8_t _pinTX) +{ + nrfUart = _nrfUart; + IRQn = _IRQn; + uc_pinRX = g_ADigitalPinMap[_pinRX]; + uc_pinTX = g_ADigitalPinMap[_pinTX]; + uc_hwFlow = 0; + + _end_tx_sem = NULL; + _begun = false; +} + +void Uart::setPins(uint8_t pin_rx, uint8_t pin_tx) +{ + uc_pinRX = g_ADigitalPinMap[pin_rx]; + uc_pinTX = g_ADigitalPinMap[pin_tx]; +} + +void Uart::end() +{ + NVIC_DisableIRQ(IRQn); + + nrfUart->INTENCLR = UARTE_INTENSET_ENDRX_Msk | UARTE_INTENSET_ENDTX_Msk; + + nrfUart->EVENTS_RXTO = 0; + nrfUart->EVENTS_TXSTOPPED = 0; + + nrfUart->TASKS_STOPRX = 0x1UL; + nrfUart->TASKS_STOPTX = 0x1UL; + + // Wait for TXSTOPPED event and for RXTO event + // This is required before disabling UART to fully power down transceiver PHY. + // Otherwise transceiver will continue to consume ~900uA + while ( !(nrfUart->EVENTS_TXSTOPPED && nrfUart->EVENTS_RXTO) ) yield(); + + nrfUart->ENABLE = UARTE_ENABLE_ENABLE_Disabled; + + nrfUart->PSEL.TXD = 0xFFFFFFFF; + nrfUart->PSEL.RXD = 0xFFFFFFFF; + + nrfUart->PSEL.RTS = 0xFFFFFFFF; + nrfUart->PSEL.CTS = 0xFFFFFFFF; + + rxBuffer.clear(); + + vSemaphoreDelete(_end_tx_sem); + _end_tx_sem = NULL; + _begun = false; +} diff --git a/test/fixtures/nrf52_uart_lifecycle/main.cpp b/test/fixtures/nrf52_uart_lifecycle/main.cpp new file mode 100644 index 000000000..ba4616bf7 --- /dev/null +++ b/test/fixtures/nrf52_uart_lifecycle/main.cpp @@ -0,0 +1,111 @@ +#include +#include +#include +#include +#include + +using IRQn_Type = int; +struct NRF_UARTE_Type { + uint32_t INTENCLR = 0, EVENTS_RXTO = 0, EVENTS_TXSTOPPED = 0; + uint32_t TASKS_STOPRX = 0, TASKS_STOPTX = 0, ENABLE = 0; + struct { uint32_t TXD = 0, RXD = 0, RTS = 0, CTS = 0; } PSEL; +}; +constexpr uint32_t UARTE_INTENSET_ENDRX_Msk = 1, UARTE_INTENSET_ENDTX_Msk = 2; +constexpr uint32_t UARTE_ENABLE_ENABLE_Disabled = 0; +const uint32_t g_ADigitalPinMap[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; +static NRF_UARTE_Type* peripheral = nullptr; +static bool hardware_completes = false; +static unsigned stops = 0, yields = 0, deleted = 0, null_deletes = 0; +void NVIC_DisableIRQ(IRQn_Type) { ++stops; } +void yield() { + // Disabled hardware does not promise stop events. Bound the regression + // instead of hanging a test process inside the real core's stop loop. + if (hardware_completes) { + peripheral->EVENTS_RXTO = peripheral->EVENTS_TXSTOPPED = 1; + } else if (++yields == 3) { + throw std::runtime_error("unstarted UART stop wait"); + } +} +void vSemaphoreDelete(void* semaphore) { + // FreeRTOS vQueueDelete asserts a non-null queue. Even an optimistic + // disabled-peripheral completion cannot make end() valid before begin(). + if (!semaphore) { + ++null_deletes; + throw std::runtime_error("FreeRTOS null semaphore assertion"); + } + ++deleted; +} +struct Uart { + NRF_UARTE_Type* nrfUart = nullptr; + IRQn_Type IRQn = 0; + uint8_t uc_pinRX = 0, uc_pinTX = 0, uc_hwFlow = 0; + void* _end_tx_sem = nullptr; + bool _begun = false; + struct { unsigned clears = 0; void clear() { ++clears; } } rxBuffer; + Uart(NRF_UARTE_Type*, IRQn_Type, uint8_t, uint8_t); + void end(); + void setPins(uint8_t, uint8_t); + // The public accessor's body is the real Uart.h contract. + operator bool () + { + return _begun; + } + // UART start is an explicit I/O boundary, not an emulation of baud/DMA. + void startForTest() { + assert(!_begun && !_end_tx_sem); + nrfUart->ENABLE = 8; + nrfUart->PSEL.RXD = uc_pinRX; + nrfUart->PSEL.TXD = uc_pinTX; + _end_tx_sem = this; + _begun = true; + } +}; +#include "core_uart.inc" + +int main() try { + NRF_UARTE_Type registers; + peripheral = ®isters; + Uart uart(®isters, 1, 0, 1); + assert(!uart && !uart._end_tx_sem && !registers.ENABLE); + + // Negative controls exercise the frozen end() precondition itself. They + // prove why a harmless-looking unconditional stop is not valid cleanup. + bool failed = false; + try { uart.end(); } + catch (const std::runtime_error&) { failed = true; } + assert(failed && yields == 3 && stops == 1 && !deleted && !null_deletes); + hardware_completes = true; + failed = false; + try { uart.end(); } + catch (const std::runtime_error&) { failed = true; } + assert(failed && stops == 2 && !deleted && null_deletes == 1); + + // Fresh first enable and an already-stopped standby resume do not call + // end(), touch the stop IRQ, or delete a semaphore that was never created. + for (unsigned retry = 0; retry < 2; ++retry) { + mesh::bridge::prepareNrfUart(uart, uint8_t(2), uint8_t(3)); + assert(!uart && !uart._end_tx_sem && stops == 2 && !deleted); + assert(null_deletes == 1 && uart.uc_pinRX == 2 && uart.uc_pinTX == 3); + } + + // A begun UART still must finish stop before pin remapping. All four PSEL + // values and the semaphore reflect the actual core's complete shutdown. + for (unsigned cycle = 0; cycle < 100; ++cycle) { + uart.startForTest(); + assert(uart && registers.ENABLE == 8 && uart._end_tx_sem); + const uint8_t rx = uint8_t(cycle % 4 + 2), tx = uint8_t(cycle % 4 + 6); + mesh::bridge::prepareNrfUart(uart, rx, tx); + assert(!uart && !uart._end_tx_sem && !registers.ENABLE); + assert(deleted == cycle + 1 && stops == cycle + 3 && null_deletes == 1); + assert(registers.PSEL.RXD == UINT32_MAX && registers.PSEL.TXD == UINT32_MAX); + assert(registers.PSEL.RTS == UINT32_MAX && registers.PSEL.CTS == UINT32_MAX); + assert(uart.uc_pinRX == rx && uart.uc_pinTX == tx); + const unsigned stopped = stops, freed = deleted; + mesh::bridge::prepareNrfUart(uart, rx, tx); + assert(!uart && stops == stopped && deleted == freed); + } + puts("PASS: nRF UART lifecycle; fresh/stopped guards and 100 running remaps"); +} catch (const std::exception& error) { + std::fprintf(stderr, "%s\n", error.what()); + return 2; +} diff --git a/test/test_rs232_nrf_uart_lifecycle.py b/test/test_rs232_nrf_uart_lifecycle.py new file mode 100644 index 000000000..55ebec626 --- /dev/null +++ b/test/test_rs232_nrf_uart_lifecycle.py @@ -0,0 +1,56 @@ +#!/usr/bin/env python3 +"""Exercise the real helper with frozen Adafruit UART lifecycle methods. + +Only register completion and semaphore deletion are host boundaries. The +constructor, public started accessor, pin setter and stop logic preserve the +pinned framework's semantics without downloads, PlatformIO or hardware. +""" +from pathlib import Path +import shutil +import subprocess +import tempfile +import unittest + +ROOT = Path(__file__).resolve().parents[1] +FIXTURE = ROOT / "test/fixtures/nrf52_uart_lifecycle" + + +class RS232NrfUartLifecycleTests(unittest.TestCase): + def compile_and_run(self, old_helper=False): + compiler = shutil.which("g++") or shutil.which("clang++") + self.assertIsNotNone(compiler, "a host C++ compiler is required") + with tempfile.TemporaryDirectory(prefix="rs232-nrf-uart-") as directory: + work = Path(directory) + binary = work / "test" + include = [] + if old_helper: + source = (ROOT / "src/helpers/bridges/RS232UartUtils.h").read_text(encoding="ascii") + self.assertEqual(source.count("if (uart) uart.end();"), 1) + header = work / "helpers/bridges/RS232UartUtils.h" + header.parent.mkdir(parents=True) + header.write_text(source.replace("if (uart) uart.end();", "uart.end();"), encoding="ascii") + include = ["-I" + str(work)] + built = subprocess.run([ + compiler, "-std=c++17", "-Wall", "-Wextra", "-Werror", + "-fsanitize=address,undefined", "-fno-sanitize-recover=all", + "-fno-pie", "-no-pie", *include, "-I" + str(ROOT / "src"), + str(FIXTURE / "main.cpp"), "-o", str(binary), + ], capture_output=True, text=True, timeout=60) + self.assertEqual(built.returncode, 0, built.stdout + built.stderr) + checked = subprocess.run([str(binary)], capture_output=True, + text=True, timeout=10) + return checked + + def test_fresh_stopped_running_and_repeated_remapping(self): + checked = self.compile_and_run() + self.assertEqual(checked.returncode, 0, checked.stdout + checked.stderr) + self.assertIn("PASS: nRF UART lifecycle", checked.stdout) + + def test_unconditional_stop_negative_control_reaches_null_semaphore(self): + checked = self.compile_and_run(old_helper=True) + self.assertNotEqual(checked.returncode, 0, checked.stdout + checked.stderr) + self.assertIn("FreeRTOS null semaphore assertion", checked.stderr) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_rs232_uart/test_rs232_uart.cpp b/test/test_rs232_uart/test_rs232_uart.cpp index d83f0e261..aab4f0841 100644 --- a/test/test_rs232_uart/test_rs232_uart.cpp +++ b/test/test_rs232_uart/test_rs232_uart.cpp @@ -6,7 +6,9 @@ struct FakeUart { std::vector calls; - void end() { calls.push_back(1); } + bool begun = false; + explicit operator bool() const { return begun; } + void end() { calls.push_back(1); begun = false; } void setPins(int rx, int tx) { calls.push_back(2); calls.push_back(rx); @@ -16,12 +18,27 @@ struct FakeUart { TEST(RS232Uart, StopsPeripheralBeforeChangingPins) { FakeUart uart; + uart.begun = true; mesh::bridge::prepareNrfUart(uart, 7, 8); ASSERT_EQ(uart.calls.size(), 4u); EXPECT_EQ(uart.calls[0], 1); EXPECT_EQ(uart.calls[1], 2); EXPECT_EQ(uart.calls[2], 7); EXPECT_EQ(uart.calls[3], 8); + EXPECT_FALSE(uart.begun); +} + +TEST(RS232Uart, FreshAndAlreadyStoppedUartsOnlyChangePins) { + FakeUart uart; + for (int attempt = 0; attempt < 2; ++attempt) { + mesh::bridge::prepareNrfUart(uart, 7, 8); + ASSERT_EQ(uart.calls.size(), 3u); + EXPECT_EQ(uart.calls[0], 2); + EXPECT_EQ(uart.calls[1], 7); + EXPECT_EQ(uart.calls[2], 8); + EXPECT_FALSE(uart.begun); + uart.calls.clear(); + } } int main(int argc, char** argv) {