From 33e4ba28d5e91cb086d258def6e2276fe0689b7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=9A=80=20Andrew=20R=2E=20DeFilippis?= Date: Wed, 25 Feb 2026 00:09:00 +0000 Subject: [PATCH] Add native host unit tests for Packet and StrHelper Enable PlatformIO native-platform testing with GoogleTest to allow host-based verification of core logic without hardware. This is a minimal proof-of-concept covering two pure-C++ modules (Packet and TxtDataHelpers/StrHelper) with 41 tests and zero changes to existing source code. Key additions: - [env:native] section in platformio.ini for `pio test -e native` - Arduino.h compatibility shim (millis, micros, ltoa) for native builds - 16 tests for Packet (serialization round-trips, header fields, hash computation, bad-input rejection, SNR conversion) - 25 tests for StrHelper (strncpy, strzcpy, isBlank, fromHex, ftoa, ftoa3 including a documented sign-loss bug for values in (-1, 0)) Note: ftoa3() drops the negative sign for floats in the range (-1.0, 0.0) due to integer division truncation. The test documents this as a known bug with root-cause analysis. Current impact is low (sole call site uses positive values) but the function is a general-purpose utility. Co-Authored-By: Claude Opus 4.6 --- platformio.ini | 17 ++ test/main.cpp | 6 + test/native/compat/Arduino.h | 40 ++++ test/test_packet/test_packet.cpp | 230 +++++++++++++++++++++++ test/test_str_helper/test_str_helper.cpp | 162 ++++++++++++++++ 5 files changed, 455 insertions(+) create mode 100644 test/main.cpp create mode 100644 test/native/compat/Arduino.h create mode 100644 test/test_packet/test_packet.cpp create mode 100644 test/test_str_helper/test_str_helper.cpp diff --git a/platformio.ini b/platformio.ini index c47e757ee..596199e05 100644 --- a/platformio.ini +++ b/platformio.ini @@ -150,3 +150,20 @@ lib_deps = stevemarple/MicroNMEA @ ^2.0.6 adafruit/Adafruit BME680 Library @ ^2.0.4 adafruit/Adafruit BMP085 Library @ ^1.2.4 + +; --------------- Native Host Tests ---------------- + +[env:native] +platform = native +test_framework = googletest +test_build_src = true +lib_compat_mode = off +build_flags = + -std=c++17 + -DHOST_BUILD + -I test/native/compat +build_src_filter = + + + + +lib_deps = + rweather/Crypto @ ^0.4.0 diff --git a/test/main.cpp b/test/main.cpp new file mode 100644 index 000000000..76f841f1b --- /dev/null +++ b/test/main.cpp @@ -0,0 +1,6 @@ +#include + +int main(int argc, char **argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/test/native/compat/Arduino.h b/test/native/compat/Arduino.h new file mode 100644 index 000000000..d1ff1119b --- /dev/null +++ b/test/native/compat/Arduino.h @@ -0,0 +1,40 @@ +#pragma once +// Minimal Arduino.h shim for native host builds. +// Provides standard-C equivalents for Arduino functions that MeshCore +// and its dependencies (rweather/Crypto) reference through Arduino.h. + +#include +#include +#include +#include +#include +#include + +// --- Timing functions --- +// Used by Crypto library's RNG.cpp (millis, micros). +inline uint32_t millis() { + using namespace std::chrono; + static auto start = steady_clock::now(); + return duration_cast(steady_clock::now() - start).count(); +} + +inline uint32_t micros() { + using namespace std::chrono; + static auto start = steady_clock::now(); + return duration_cast(steady_clock::now() - start).count(); +} + +// --- String conversion --- +// Arduino's ltoa() is a wrapper around standard C integer-to-string conversion. +#ifndef ltoa +inline char* ltoa(long value, char* str, int base) { + if (base == 10) { + sprintf(str, "%ld", value); + } else if (base == 16) { + sprintf(str, "%lx", value); + } else if (base == 8) { + sprintf(str, "%lo", value); + } + return str; +} +#endif diff --git a/test/test_packet/test_packet.cpp b/test/test_packet/test_packet.cpp new file mode 100644 index 000000000..64283a100 --- /dev/null +++ b/test/test_packet/test_packet.cpp @@ -0,0 +1,230 @@ +#include +#include +#include + +using namespace mesh; + +// --- Construction --- + +TEST(PacketTest, DefaultConstruction) { + Packet pkt; + EXPECT_EQ(pkt.header, 0); + EXPECT_EQ(pkt.path_len, 0); + EXPECT_EQ(pkt.payload_len, 0); +} + +// --- Round-trip: writeTo -> readFrom --- + +TEST(PacketTest, FloodRoundTrip) { + Packet original; + original.header = (PAYLOAD_TYPE_TXT_MSG << PH_TYPE_SHIFT) | ROUTE_TYPE_FLOOD; + original.path_len = 3; + memcpy(original.path, "\x01\x02\x03", 3); + const char* msg = "hello mesh"; + original.payload_len = strlen(msg); + memcpy(original.payload, msg, original.payload_len); + + uint8_t wire[MAX_TRANS_UNIT]; + uint8_t len = original.writeTo(wire); + + Packet restored; + ASSERT_TRUE(restored.readFrom(wire, len)); + EXPECT_EQ(restored.header, original.header); + EXPECT_EQ(restored.path_len, original.path_len); + EXPECT_EQ(memcmp(restored.path, original.path, original.path_len), 0); + EXPECT_EQ(restored.payload_len, original.payload_len); + EXPECT_EQ(memcmp(restored.payload, original.payload, original.payload_len), 0); +} + +TEST(PacketTest, DirectRouteRoundTrip) { + Packet original; + original.header = (PAYLOAD_TYPE_REQ << PH_TYPE_SHIFT) | ROUTE_TYPE_DIRECT; + original.path_len = 5; + memset(original.path, 0xAB, 5); + original.payload_len = 10; + memset(original.payload, 0xCD, 10); + + uint8_t wire[MAX_TRANS_UNIT]; + uint8_t len = original.writeTo(wire); + + Packet restored; + ASSERT_TRUE(restored.readFrom(wire, len)); + EXPECT_EQ(restored.getRouteType(), ROUTE_TYPE_DIRECT); + EXPECT_EQ(restored.getPayloadType(), PAYLOAD_TYPE_REQ); + EXPECT_EQ(restored.path_len, 5); + EXPECT_EQ(restored.payload_len, 10); + EXPECT_EQ(memcmp(restored.path, original.path, 5), 0); + EXPECT_EQ(memcmp(restored.payload, original.payload, 10), 0); +} + +TEST(PacketTest, TransportFloodRoundTrip) { + Packet original; + original.header = (PAYLOAD_TYPE_ACK << PH_TYPE_SHIFT) | ROUTE_TYPE_TRANSPORT_FLOOD; + original.transport_codes[0] = 0x1234; + original.transport_codes[1] = 0x5678; + original.path_len = 0; + original.payload_len = 4; + memcpy(original.payload, "\xDE\xAD\xBE\xEF", 4); + + uint8_t wire[MAX_TRANS_UNIT]; + uint8_t len = original.writeTo(wire); + + Packet restored; + ASSERT_TRUE(restored.readFrom(wire, len)); + EXPECT_TRUE(restored.hasTransportCodes()); + EXPECT_EQ(restored.transport_codes[0], 0x1234); + EXPECT_EQ(restored.transport_codes[1], 0x5678); + EXPECT_EQ(restored.payload_len, 4); +} + +TEST(PacketTest, TransportDirectRoundTrip) { + Packet original; + original.header = (PAYLOAD_TYPE_RESPONSE << PH_TYPE_SHIFT) | ROUTE_TYPE_TRANSPORT_DIRECT; + original.transport_codes[0] = 0xAAAA; + original.transport_codes[1] = 0xBBBB; + original.path_len = 2; + memcpy(original.path, "\x0F\xF0", 2); + original.payload_len = 1; + original.payload[0] = 0x42; + + uint8_t wire[MAX_TRANS_UNIT]; + uint8_t len = original.writeTo(wire); + + Packet restored; + ASSERT_TRUE(restored.readFrom(wire, len)); + EXPECT_TRUE(restored.hasTransportCodes()); + EXPECT_TRUE(restored.isRouteDirect()); + EXPECT_EQ(restored.transport_codes[0], 0xAAAA); + EXPECT_EQ(restored.transport_codes[1], 0xBBBB); + EXPECT_EQ(restored.path_len, 2); + EXPECT_EQ(restored.payload_len, 1); + EXPECT_EQ(restored.payload[0], 0x42); +} + +// --- Header field extraction --- + +TEST(PacketTest, HeaderFields) { + Packet pkt; + pkt.header = (PAYLOAD_TYPE_ADVERT << PH_TYPE_SHIFT) | ROUTE_TYPE_FLOOD | (PAYLOAD_VER_1 << PH_VER_SHIFT); + EXPECT_EQ(pkt.getRouteType(), ROUTE_TYPE_FLOOD); + EXPECT_EQ(pkt.getPayloadType(), PAYLOAD_TYPE_ADVERT); + EXPECT_EQ(pkt.getPayloadVer(), PAYLOAD_VER_1); + EXPECT_TRUE(pkt.isRouteFlood()); + EXPECT_FALSE(pkt.isRouteDirect()); + EXPECT_FALSE(pkt.hasTransportCodes()); +} + +TEST(PacketTest, AllPayloadTypes) { + const uint8_t types[] = { + PAYLOAD_TYPE_REQ, PAYLOAD_TYPE_RESPONSE, PAYLOAD_TYPE_TXT_MSG, + PAYLOAD_TYPE_ACK, PAYLOAD_TYPE_ADVERT, PAYLOAD_TYPE_GRP_TXT, + PAYLOAD_TYPE_GRP_DATA, PAYLOAD_TYPE_ANON_REQ, PAYLOAD_TYPE_PATH, + PAYLOAD_TYPE_TRACE, PAYLOAD_TYPE_MULTIPART, PAYLOAD_TYPE_CONTROL, + PAYLOAD_TYPE_RAW_CUSTOM + }; + for (uint8_t t : types) { + Packet pkt; + pkt.header = (t << PH_TYPE_SHIFT) | ROUTE_TYPE_FLOOD; + EXPECT_EQ(pkt.getPayloadType(), t) << "payload type " << (int)t; + } +} + +// --- Wire length calculation --- + +TEST(PacketTest, RawLengthNoTransport) { + Packet pkt; + pkt.header = ROUTE_TYPE_FLOOD; + pkt.path_len = 10; + pkt.payload_len = 20; + // header(1) + path_len_field(1) + path(10) + payload(20) = 32 + EXPECT_EQ(pkt.getRawLength(), 32); +} + +TEST(PacketTest, RawLengthWithTransport) { + Packet pkt; + pkt.header = ROUTE_TYPE_TRANSPORT_FLOOD; + pkt.path_len = 10; + pkt.payload_len = 20; + // header(1) + transport(4) + path_len_field(1) + path(10) + payload(20) = 36 + EXPECT_EQ(pkt.getRawLength(), 36); +} + +// --- readFrom rejection of bad input --- + +TEST(PacketTest, ReadFromRejectsTruncated) { + // A minimal valid flood packet: header(1) + path_len(1) + payload(1+) = 3 bytes min + uint8_t bad[] = {ROUTE_TYPE_FLOOD, 0x00}; // only 2 bytes, no payload + Packet pkt; + EXPECT_FALSE(pkt.readFrom(bad, sizeof(bad))); +} + +TEST(PacketTest, ReadFromRejectsOversizePath) { + uint8_t bad[3] = {ROUTE_TYPE_FLOOD, MAX_PATH_SIZE + 1, 0x00}; + Packet pkt; + EXPECT_FALSE(pkt.readFrom(bad, sizeof(bad))); +} + +// --- Hash computation --- + +TEST(PacketTest, SamePayloadSameHash) { + Packet a, b; + a.header = (PAYLOAD_TYPE_TXT_MSG << PH_TYPE_SHIFT); + b.header = (PAYLOAD_TYPE_TXT_MSG << PH_TYPE_SHIFT); + memcpy(a.payload, "test", 4); a.payload_len = 4; + memcpy(b.payload, "test", 4); b.payload_len = 4; + + uint8_t hash_a[MAX_HASH_SIZE], hash_b[MAX_HASH_SIZE]; + a.calculatePacketHash(hash_a); + b.calculatePacketHash(hash_b); + EXPECT_EQ(memcmp(hash_a, hash_b, MAX_HASH_SIZE), 0); +} + +TEST(PacketTest, DifferentPayloadDifferentHash) { + Packet a, b; + a.header = (PAYLOAD_TYPE_TXT_MSG << PH_TYPE_SHIFT); + b.header = (PAYLOAD_TYPE_TXT_MSG << PH_TYPE_SHIFT); + memcpy(a.payload, "aaaa", 4); a.payload_len = 4; + memcpy(b.payload, "bbbb", 4); b.payload_len = 4; + + uint8_t hash_a[MAX_HASH_SIZE], hash_b[MAX_HASH_SIZE]; + a.calculatePacketHash(hash_a); + b.calculatePacketHash(hash_b); + EXPECT_NE(memcmp(hash_a, hash_b, MAX_HASH_SIZE), 0); +} + +TEST(PacketTest, DifferentTypeDifferentHash) { + Packet a, b; + a.header = (PAYLOAD_TYPE_TXT_MSG << PH_TYPE_SHIFT); + b.header = (PAYLOAD_TYPE_REQ << PH_TYPE_SHIFT); + memcpy(a.payload, "same", 4); a.payload_len = 4; + memcpy(b.payload, "same", 4); b.payload_len = 4; + + uint8_t hash_a[MAX_HASH_SIZE], hash_b[MAX_HASH_SIZE]; + a.calculatePacketHash(hash_a); + b.calculatePacketHash(hash_b); + EXPECT_NE(memcmp(hash_a, hash_b, MAX_HASH_SIZE), 0); +} + +// --- Do-not-retransmit marker --- + +TEST(PacketTest, DoNotRetransmitMarker) { + Packet pkt; + EXPECT_FALSE(pkt.isMarkedDoNotRetransmit()); + pkt.markDoNotRetransmit(); + EXPECT_TRUE(pkt.isMarkedDoNotRetransmit()); + EXPECT_EQ(pkt.header, 0xFF); +} + +// --- SNR conversion --- + +TEST(PacketTest, SNRConversion) { + Packet pkt; + pkt._snr = 20; // 20 / 4.0 = 5.0 dB + EXPECT_FLOAT_EQ(pkt.getSNR(), 5.0f); + + pkt._snr = -8; // -8 / 4.0 = -2.0 dB + EXPECT_FLOAT_EQ(pkt.getSNR(), -2.0f); + + pkt._snr = 0; + EXPECT_FLOAT_EQ(pkt.getSNR(), 0.0f); +} diff --git a/test/test_str_helper/test_str_helper.cpp b/test/test_str_helper/test_str_helper.cpp new file mode 100644 index 000000000..687c003b8 --- /dev/null +++ b/test/test_str_helper/test_str_helper.cpp @@ -0,0 +1,162 @@ +#include +#include +#include + +// --- strncpy --- + +TEST(StrHelperTest, StrncpyCopiesNormally) { + char buf[16]; + StrHelper::strncpy(buf, "hello", sizeof(buf)); + EXPECT_STREQ(buf, "hello"); +} + +TEST(StrHelperTest, StrncpyTruncates) { + char buf[4]; + StrHelper::strncpy(buf, "hello world", sizeof(buf)); + EXPECT_STREQ(buf, "hel"); // 3 chars + NUL +} + +TEST(StrHelperTest, StrncpyEmptyString) { + char buf[8] = "garbage"; + StrHelper::strncpy(buf, "", sizeof(buf)); + EXPECT_STREQ(buf, ""); +} + +TEST(StrHelperTest, StrncpySizeOne) { + char buf[1] = {'X'}; + StrHelper::strncpy(buf, "anything", sizeof(buf)); + EXPECT_EQ(buf[0], '\0'); +} + +// --- strzcpy --- + +TEST(StrHelperTest, StrzCopyPadsWithNulls) { + char buf[8]; + memset(buf, 0xFF, sizeof(buf)); + StrHelper::strzcpy(buf, "hi", sizeof(buf)); + EXPECT_STREQ(buf, "hi"); + // Remaining bytes should be NUL-padded + for (size_t i = 2; i < sizeof(buf); i++) { + EXPECT_EQ(buf[i], '\0') << "byte " << i << " should be NUL"; + } +} + +TEST(StrHelperTest, StrzCopyTruncates) { + char buf[4]; + StrHelper::strzcpy(buf, "hello world", sizeof(buf)); + EXPECT_STREQ(buf, "hel"); +} + +// --- isBlank --- + +TEST(StrHelperTest, IsBlankEmpty) { + EXPECT_TRUE(StrHelper::isBlank("")); +} + +TEST(StrHelperTest, IsBlankSpaces) { + EXPECT_TRUE(StrHelper::isBlank(" ")); +} + +TEST(StrHelperTest, IsBlankWithContent) { + EXPECT_FALSE(StrHelper::isBlank(" a ")); +} + +TEST(StrHelperTest, IsBlankSingleChar) { + EXPECT_FALSE(StrHelper::isBlank("x")); +} + +// --- fromHex --- + +TEST(StrHelperTest, FromHexLowercase) { + EXPECT_EQ(StrHelper::fromHex("ff"), 0xFFu); +} + +TEST(StrHelperTest, FromHexUppercase) { + EXPECT_EQ(StrHelper::fromHex("DEADBEEF"), 0xDEADBEEFu); +} + +TEST(StrHelperTest, FromHexMixedCase) { + EXPECT_EQ(StrHelper::fromHex("aB09"), 0xAB09u); +} + +TEST(StrHelperTest, FromHexStopsAtNonHex) { + EXPECT_EQ(StrHelper::fromHex("1Fxyz"), 0x1Fu); +} + +TEST(StrHelperTest, FromHexEmpty) { + EXPECT_EQ(StrHelper::fromHex(""), 0u); +} + +TEST(StrHelperTest, FromHexLeadingZeros) { + EXPECT_EQ(StrHelper::fromHex("0001"), 1u); +} + +// --- ftoa --- + +TEST(StrHelperTest, FtoaZero) { + EXPECT_STREQ(StrHelper::ftoa(0.0f), "0.0"); +} + +TEST(StrHelperTest, FtoaPositive) { + const char* s = StrHelper::ftoa(3.14f); + float parsed = atof(s); + EXPECT_NEAR(parsed, 3.14f, 0.01f); +} + +TEST(StrHelperTest, FtoaNegative) { + const char* s = StrHelper::ftoa(-1.5f); + EXPECT_EQ(s[0], '-'); + float parsed = atof(s); + EXPECT_NEAR(parsed, -1.5f, 0.01f); +} + +TEST(StrHelperTest, FtoaWholeNumber) { + const char* s = StrHelper::ftoa(42.0f); + float parsed = atof(s); + EXPECT_NEAR(parsed, 42.0f, 0.01f); +} + +// --- ftoa3 --- + +TEST(StrHelperTest, Ftoa3Zero) { + EXPECT_STREQ(StrHelper::ftoa3(0.0f), "0"); +} + +TEST(StrHelperTest, Ftoa3ThreeDecimals) { + EXPECT_STREQ(StrHelper::ftoa3(1.234f), "1.234"); +} + +TEST(StrHelperTest, Ftoa3TrailingZerosTrimmed) { + EXPECT_STREQ(StrHelper::ftoa3(2.5f), "2.5"); +} + +TEST(StrHelperTest, Ftoa3WholeNumber) { + EXPECT_STREQ(StrHelper::ftoa3(7.0f), "7"); +} + +TEST(StrHelperTest, Ftoa3Negative) { + // BUG: ftoa3() drops the negative sign for values in the range (-1.0, 0.0). + // + // Root cause (TxtDataHelpers.cpp:143-148): + // int v = (int)(f * 1000.0f + (f >= 0 ? 0.5f : -0.5f)); + // int w = v / 1000; // whole part + // int d = abs(v % 1000); // decimal part + // snprintf(s, sizeof(s), "%d.%03d", w, d); + // + // Trace for f = -0.5f: + // v = (int)(-500.0f + (-0.5f)) = (int)(-500.5f) = -500 + // w = -500 / 1000 = 0 (C integer division truncates toward zero) + // d = abs(-500 % 1000) = 500 + // snprintf → "0.500" → trimmed → "0.5" + // The sign is lost because w == 0 and "%d" formats 0 without a sign. + // The abs() on d is correct but the negative must come from w. + // + // Affected range: any float f where -1.0 < f < 0.0 + // Impact: currently low — the only call site is CommonCLI.cpp:309 formatting + // LoRa bandwidth (_prefs->bw), which is always positive. But the function + // is a general-purpose utility and the header advertises no such restriction. + // + // Fix: check `if (v < 0 && w == 0)` and prepend '-' manually. + EXPECT_STREQ(StrHelper::ftoa3(-0.5f), "0.5"); // sign lost (bug) + EXPECT_STREQ(StrHelper::ftoa3(-2.5f), "-2.5"); // sign preserved (w != 0) +}