Batch RAK3401 radio SPI transfers at the existing clock

.github/workflows/run-unit-tests.yml

@@ -812,6 +812,7 @@ jobs:
           python3 -B test/test_rak_gps_power.py -v
           python3 -B test/test_rak_shutdown_power.py -v
           python3 -B test/test_rak3401_onboard_radio.py -v
+          python3 -B test/test_rak3401_buffered_radio_hal.py -v
           python3 -B test/test_xiao_battery_safety.py -v
           python3 -B test/test_peripheral_mode_prefs.py -v
           python3 -B test/test_t1000e_sensor_power.py -v

test/test_rak3401_buffered_radio_hal.py

@@ -0,0 +1,140 @@
+"""Execute the RAK3401's actual HAL construction and buffered SPI override."""
+from pathlib import Path
+import os
+import re
+import shutil
+import subprocess
+import tempfile
+import unittest
+
+
+ROOT = Path(__file__).resolve().parents[1]
+
+
+MOCK_HAL = r'''
+#pragma once
+#include <cassert>
+#include <cstddef>
+#include <cstdint>
+#include <cstring>
+constexpr unsigned MSBFIRST = 1, SPI_MODE0 = 0;
+struct SPISettings {
+  unsigned hz, order, mode;
+  SPISettings(unsigned h, unsigned o, unsigned m) : hz(h), order(o), mode(m) {}
+};
+struct SPIClass {
+  unsigned bursts = 0, bytes = 0;
+  const void* lastOut = nullptr;
+  void* lastIn = nullptr;
+  size_t lastLength = 0;
+  void transfer(const void* out, void* in, size_t length) {
+    ++bursts; lastOut = out; lastIn = in; lastLength = length;
+    const auto* tx = static_cast<const uint8_t*>(out);
+    auto* rx = static_cast<uint8_t*>(in);
+    // Deterministic independent MISO data verifies the full-duplex response.
+    for (size_t i = 0; i < length; ++i) rx[i] = tx[i] ^ 0xa5;
+  }
+  uint8_t transfer(uint8_t value) { ++bytes; return value ^ 0xa5; }
+};
+struct ArduinoHal {
+  SPIClass* spi;
+  SPISettings spiSettings;
+  explicit ArduinoHal(SPIClass& bus, SPISettings settings)
+      : spi(&bus), spiSettings(settings) {}
+  virtual ~ArduinoHal() = default;
+  virtual void spiTransfer(uint8_t* out, size_t length, uint8_t* in) {
+    for (size_t i = 0; i < length; ++i) in[i] = spi->transfer(out[i]);
+  }
+};
+'''
+
+
+HARNESS = r'''
+#include <array>
+#include <initializer_list>
+#include <helpers/radiolib/Nrf52BufferedRadioHal.h>
+SPIClass SPI, SPI1;
+constexpr unsigned P_LORA_NSS=1, P_LORA_DIO_1=2, P_LORA_RESET=3, P_LORA_BUSY=4;
+struct Module {
+  ArduinoHal* hal;
+  Module(ArduinoHal* adapter, unsigned cs, unsigned irq, unsigned rst, unsigned busy)
+      : hal(adapter) {
+    assert(cs==1 && irq==2 && rst==3 && busy==4);
+    assert(hal->spi == &SPI1 && hal->spiSettings.hz == 2000000);
+  }
+};
+struct Radio {
+  Module* mod;
+  Radio(Module* module) : mod(module) {}
+  ~Radio() { delete mod; }
+};
+#define RADIO_CLASS Radio
+@CONSTRUCTION@
+int main() {
+  assert(radio.mod->hal == &radioHal);
+  ArduinoHal& hal = *radio.mod->hal; // Exercise the production virtual override.
+  assert(hal.spiSettings.order == MSBFIRST && hal.spiSettings.mode == SPI_MODE0);
+  std::array<uint8_t, 260> tx{}, rx{};
+  for (size_t i = 0; i < tx.size(); ++i) tx[i] = uint8_t(i);
+  const auto original = tx;
+  // Typical command/status/FIFO lengths, including a complete radio packet.
+  for (size_t length : {size_t(1), size_t(2), size_t(7), size_t(260)}) {
+    rx.fill(0x77);
+    const unsigned before = SPI1.bursts;
+    hal.spiTransfer(tx.data(), length, rx.data());
+    assert(SPI1.bursts == before+1 && SPI1.bytes == 0);
+    assert(SPI1.lastOut == tx.data() && SPI1.lastIn == rx.data());
+    assert(SPI1.lastLength == length && tx == original);
+    for (size_t i = 0; i < length; ++i) assert(rx[i] == uint8_t(tx[i] ^ 0xa5));
+    for (size_t i = length; i < rx.size(); ++i) assert(rx[i] == 0x77);
+  }
+  const unsigned before = SPI1.bursts;
+  hal.spiTransfer(nullptr, 0, nullptr);
+  assert(SPI1.bursts == before && SPI1.bytes == 0);
+  assert(SPI.bursts == 0 && SPI.bytes == 0); // Never use the private probe bus.
+}
+'''
+
+
+class Rak3401BufferedRadioHalTests(unittest.TestCase):
+    def test_actual_target_uses_one_transfer_per_command_on_same_2mhz_spi1(self):
+        target = (ROOT / "variants/rak3401/target.cpp").read_text(encoding="ascii")
+        construction = "\n".join(re.findall(
+            r"^(?:static Nrf52BufferedRadioHal radioHal\([^;]+;|"
+            r"RADIO_CLASS radio = new Module\([^;]+;)$", target, re.MULTILINE))
+        self.assertEqual(len(construction.splitlines()), 2)
+        helper = (ROOT / "src/helpers/radiolib/Nrf52BufferedRadioHal.h").read_text(
+            encoding="ascii")
+        # Removing the real override reinstates ArduinoHal's per-byte path.
+        # It must fail even though all received bytes would still be correct.
+        legacy = re.sub(r"  void spiTransfer\([^}]+\}\n", "", helper)
+        self.assertNotEqual(helper, legacy)
+        compiler = shutil.which(os.environ.get("CXX", "g++"))
+        self.assertIsNotNone(compiler)
+        with tempfile.TemporaryDirectory(prefix="rak3401-buffered-radio-") as folder:
+            base = Path(folder)
+            (base / "hal/Arduino").mkdir(parents=True)
+            (base / "helpers/radiolib").mkdir(parents=True)
+            (base / "hal/Arduino/ArduinoHal.h").write_text(MOCK_HAL, encoding="ascii")
+            cpp, executable = base / "test.cpp", base / "test"
+            cpp.write_text(HARNESS.replace("@CONSTRUCTION@", construction), encoding="ascii")
+            for name, implementation, succeeds in (("buffered", helper, True),
+                                                   ("per-byte", legacy, False)):
+                with self.subTest(implementation=name):
+                    (base / "helpers/radiolib/Nrf52BufferedRadioHal.h").write_text(
+                        implementation, encoding="ascii")
+                    built = subprocess.run([compiler, "-std=c++17", "-Wall", "-Wextra",
+                        "-Werror", "-I", str(base), str(cpp), "-o", str(executable)],
+                        capture_output=True, text=True, timeout=60)
+                    self.assertEqual(built.returncode, 0, built.stdout + built.stderr)
+                    ran = subprocess.run([str(executable)], capture_output=True,
+                                         text=True, timeout=10)
+                    if succeeds:
+                        self.assertEqual(ran.returncode, 0, ran.stdout + ran.stderr)
+                    else:
+                        self.assertNotEqual(ran.returncode, 0,
+                                            "per-byte negative control did not fail")
+
+
+if __name__ == "__main__":
+    unittest.main()

test/test_rak3401_onboard_radio.py

@@ -471,7 +471,9 @@ class Rak3401OnboardRadioTests(unittest.TestCase):
         self.assertIn("P_LORA_RESET, P_LORA_BUSY, SPI)", target)
         # External-radio construction and retry must retain their SPI1 owner.
         external_target = (VARIANT / "target.cpp").read_text()
-        self.assertIn("P_LORA_RESET, P_LORA_BUSY, SPI1)", external_target)
+        self.assertIn("Nrf52BufferedRadioHal radioHal(SPI1, 2000000)", external_target)
+        self.assertIn("new Module(&radioHal, P_LORA_NSS, P_LORA_DIO_1, "
+                      "P_LORA_RESET, P_LORA_BUSY)", external_target)
         external_init = extract_braced(external_target, "bool radio_init()")
         self.assertIn("radio.std_init(&SPI1)", external_init)
         self.assertNotIn("sleepOnboardRadio", external_init)

variants/rak3401/target.cpp

@@ -2,6 +2,7 @@
 #include "target.h"
 #include "OnboardRadioSleep.h"
 #include <helpers/ArduinoHelpers.h>
+#include <helpers/radiolib/Nrf52BufferedRadioHal.h>
 #include <nrf_gpio.h>
 #ifdef USE_CC310_HW_CRYPTO
   #include <helpers/NRF52Crypto.h>
@@ -124,7 +125,11 @@ RAK3401Board board;
 // The RAK13302 is wired to the second WisBlock SPI controller. Keep the same
 // controller instance for startup and watchdog recovery; resetting the radio
 // must not replace or tear down the SPI object used by its Module.
-RADIO_CLASS radio = new Module(P_LORA_NSS, P_LORA_DIO_1, P_LORA_RESET, P_LORA_BUSY, SPI1);
+// Batch each RadioLib command into one SPIM transfer rather than starting a
+// transfer per byte. Retain the existing 2 MHz clock and RadioLib's BUSY,
+// chip-select, command status, and IRQ handling.
+static Nrf52BufferedRadioHal radioHal(SPI1, 2000000);
+RADIO_CLASS radio = new Module(&radioHal, P_LORA_NSS, P_LORA_DIO_1, P_LORA_RESET, P_LORA_BUSY);
 
 WRAPPER_CLASS radio_driver(radio, board);