refactor(prefs): group NodePrefs fields thematically; add real NodePrefs unit tests

NodePrefs.h's field declaration order used to just be historical append
order (on-disk format is defined solely by DataStore's explicit rd()/wr()
sequence, not struct layout), making the file hard to navigate. Reordered
fields into thematic groups (radio, repeater, bot, GPS/trail/location,
display/keyboard, etc.) with no on-disk/schema change; fixed two comments
that had gone stale (favourite_contacts/_kinds' [del→...] tags only named
one of the two handlers that actually clear them; dashboard_fields was
miscategorized under favourites). sizeof(NodePrefs) shifted twice as a
side effect of packing (2760→2752→2760) — verified via real builds on all
four canonical envs and re-checked against the serialization tripwire.

Also replaced test_companion_node_prefs.cpp's dead body (a disabled test
against a saveSerial/loadSerial API this struct never got) with real
coverage of the pure helper functions NodePrefs.h already carries -- band
bucketing, repeater-profile bounds, alarm-repeat round-trip, and every
option-lookup table, including their inconsistent out-of-range fallback
behaviour.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Jakub
2026-09-01 21:56:57 +02:00
co-authored by Claude Sonnet 5
parent 1f7f1455a2
commit 300f5fab0b
2 changed files with 467 additions and 364 deletions
@@ -1,83 +1,165 @@
#include <gtest/gtest.h>
#include <cstdio>
#include <cstring>
#include <string>
#include <Arduino.h>
#include "../../examples/companion_radio/NodePrefs.h"
class ReplayStream : public Stream {
const char* _text;
int _pos = 0;
int _len;
// These target the pure, hardware-independent helpers declared alongside
// NodePrefs (free functions and NodePrefs:: static lookup tables). They build
// and run on the `native` host env with no board/filesystem mocking, unlike
// DataStore::savePrefs()/loadPrefsInt() (the actual on-disk read/write path),
// which pulls in FILESYSTEM/File (Adafruit_LittleFS or fs::FS, depending on
// platform) plus IdentityStore/ContactInfo/ChannelDetails/target.h — real
// coverage of that path needs a filesystem mock this test file doesn't have.
public:
explicit ReplayStream(const char* text) : _text(text), _len(strlen(text)) { }
TEST(DefaultRepeaterFreqForBand, PicksTheBandBelowTheCompanionFrequency) {
EXPECT_FLOAT_EQ(433.000f, defaultRepeaterFreqForBand(490.0f)); // < 500
EXPECT_FLOAT_EQ(869.495f, defaultRepeaterFreqForBand(500.0f)); // 500 boundary
EXPECT_FLOAT_EQ(869.495f, defaultRepeaterFreqForBand(868.0f)); // 500..890
EXPECT_FLOAT_EQ(918.000f, defaultRepeaterFreqForBand(890.0f)); // 890 boundary
EXPECT_FLOAT_EQ(918.000f, defaultRepeaterFreqForBand(915.0f)); // >= 890
}
int available() override { return _len - _pos; }
int read() override { return _pos < _len ? _text[_pos++] : -1; }
int peek() override { return _pos < _len ? _text[_pos] : -1; }
};
TEST(IsValidRepeaterProfile, AcceptsAProfileWithinAllBounds) {
EXPECT_TRUE(isValidRepeaterProfile(868.0f, 250.0f, 10, 5, 850.0f, 930.0f));
}
class CaptureStream : public Stream {
std::string _text;
TEST(IsValidRepeaterProfile, RejectsFrequencyOutsideTheChipRange) {
EXPECT_FALSE(isValidRepeaterProfile(800.0f, 250.0f, 10, 5, 850.0f, 930.0f));
EXPECT_FALSE(isValidRepeaterProfile(950.0f, 250.0f, 10, 5, 850.0f, 930.0f));
}
size_t emit(long long value) {
char text[24];
int length = snprintf(text, sizeof(text), "%lld", value);
return write(reinterpret_cast<const uint8_t*>(text), length);
TEST(IsValidRepeaterProfile, RejectsSpreadingFactorOutsideFiveToTwelve) {
EXPECT_FALSE(isValidRepeaterProfile(868.0f, 250.0f, 4, 5, 850.0f, 930.0f));
EXPECT_FALSE(isValidRepeaterProfile(868.0f, 250.0f, 13, 5, 850.0f, 930.0f));
}
TEST(IsValidRepeaterProfile, RejectsCodingRateOutsideFiveToEight) {
EXPECT_FALSE(isValidRepeaterProfile(868.0f, 250.0f, 10, 4, 850.0f, 930.0f));
EXPECT_FALSE(isValidRepeaterProfile(868.0f, 250.0f, 10, 9, 850.0f, 930.0f));
}
TEST(IsValidRepeaterProfile, RejectsBandwidthOutsideSevenTo510) {
EXPECT_FALSE(isValidRepeaterProfile(868.0f, 6.9f, 10, 5, 850.0f, 930.0f));
EXPECT_FALSE(isValidRepeaterProfile(868.0f, 510.1f, 10, 5, 850.0f, 930.0f));
}
TEST(SeedDefaultRepeaterProfile, SeedsFromTheCompanionFrequencyBand) {
NodePrefs prefs{};
prefs.freq = 915.0f;
seedDefaultRepeaterProfile(prefs);
EXPECT_EQ(1, prefs.repeater_use_profile);
EXPECT_FLOAT_EQ(918.000f, prefs.repeater_freq);
EXPECT_FLOAT_EQ((float)LORA_BW, prefs.repeater_bw);
EXPECT_EQ((uint8_t)LORA_SF, prefs.repeater_sf);
EXPECT_EQ((uint8_t)LORA_CR, prefs.repeater_cr);
}
TEST(AlarmRepeatRoundTrip, EveryPresetIndexRoundTripsThroughItsMask) {
for (uint8_t idx = 0; idx < NodePrefs::ALARM_REPEAT_COUNT; idx++) {
uint8_t mask = NodePrefs::alarmRepeatMaskForIdx(idx);
EXPECT_EQ(idx, NodePrefs::alarmRepeatIdxForMask(mask)) << "idx=" << (int)idx;
}
}
TEST(AlarmRepeatRoundTrip, OutOfRangeIndexClampsToNone) {
// Cast to a prvalue: EXPECT_EQ binds its arguments by const T&, which would
// otherwise ODR-use this in-class-initialized static const and need an
// out-of-line definition that doesn't exist.
EXPECT_EQ((uint8_t)NodePrefs::ALARM_REPEAT_NONE, NodePrefs::alarmRepeatMaskForIdx(99));
}
TEST(AlarmRepeatRoundTrip, AnArbitraryNonPresetMaskReadsAsOff) {
// Documents the reverse-lookup's documented behaviour: a mask that doesn't
// match a preset (e.g. a future custom-day picker's value) reads as index 0
// ("OFF") even though it isn't actually the all-zero NONE mask.
EXPECT_EQ(0, NodePrefs::alarmRepeatIdxForMask(0x15));
}
TEST(AlarmRepeatLabel, MatchesEachPresetIndex) {
EXPECT_STREQ("OFF", NodePrefs::alarmRepeatLabel(0));
EXPECT_STREQ("Daily", NodePrefs::alarmRepeatLabel(1));
EXPECT_STREQ("Weekdays", NodePrefs::alarmRepeatLabel(2));
EXPECT_STREQ("Weekends", NodePrefs::alarmRepeatLabel(3));
}
TEST(AlarmRepeatLabel, OutOfRangeIndexClampsToOff) {
EXPECT_STREQ("OFF", NodePrefs::alarmRepeatLabel(99));
}
TEST(KeyboardAlphabetLabel, MatchesEachAlphabetIndex) {
EXPECT_STREQ("Latin", NodePrefs::keyboardAlphabetLabel(NodePrefs::KB_ALPHABET_LATIN_ONLY));
EXPECT_STREQ("Cyrillic", NodePrefs::keyboardAlphabetLabel(NodePrefs::KB_ALPHABET_CYRILLIC));
EXPECT_STREQ("Greek", NodePrefs::keyboardAlphabetLabel(NodePrefs::KB_ALPHABET_GREEK));
}
public:
size_t write(uint8_t value) override {
_text.push_back(static_cast<char>(value));
return 1;
}
TEST(KeyboardAlphabetLabel, OutOfRangeIndexClampsToLatin) {
EXPECT_STREQ("Latin", NodePrefs::keyboardAlphabetLabel(99));
}
TEST(HomePageLabel, MatchesEveryBitIndexInOrder) {
EXPECT_STREQ("Clock", NodePrefs::homePageLabel(NodePrefs::HPB_CLOCK));
EXPECT_STREQ("Tools", NodePrefs::homePageLabel(NodePrefs::HPB_TOOLS));
EXPECT_STREQ("Messages", NodePrefs::homePageLabel(NodePrefs::HPB_QUICK_MSG));
EXPECT_STREQ("Favourites", NodePrefs::homePageLabel(NodePrefs::HPB_FAVOURITES));
EXPECT_STREQ("Map", NodePrefs::homePageLabel(NodePrefs::HPB_MAP));
}
TEST(HomePageLabel, OutOfRangeBitReturnsEmptyString) {
EXPECT_STREQ("", NodePrefs::homePageLabel(NodePrefs::HPB_COUNT));
}
TEST(OptionTables, InRangeIndicesReturnTheDocumentedValues) {
EXPECT_EQ(250, NodePrefs::locShareMoveMeters(2));
EXPECT_EQ(120, NodePrefs::locShareIntervalSecs(2));
EXPECT_EQ(900, NodePrefs::locShareHeartbeatSecs(2));
EXPECT_EQ(500, NodePrefs::locatorRadiusMeters(3));
EXPECT_STREQ("Both", NodePrefs::locatorModeLabel(2));
EXPECT_EQ(10, NodePrefs::gpsAvgSecs(2));
EXPECT_STREQ("10s", NodePrefs::gpsAvgLabel(2));
EXPECT_EQ(120, NodePrefs::trailAutoPauseSecs(2));
EXPECT_STREQ("2m", NodePrefs::trailAutoPauseLabel(2));
}
// Each option table clamps an out-of-range index independently, and they
// don't all agree on which in-range index to fall back to (1 for the
// loc-share/locator tables, 0 for the GPS-averaging/trail-autopause ones) —
// this pins down that existing, slightly inconsistent behaviour so a future
// change to any one table doesn't silently change another's fallback.
TEST(OptionTables, OutOfRangeIndexFallsBackPerTable) {
EXPECT_EQ(NodePrefs::locShareMoveMeters(1), NodePrefs::locShareMoveMeters(99));
EXPECT_EQ(NodePrefs::locShareIntervalSecs(1), NodePrefs::locShareIntervalSecs(99));
EXPECT_EQ(NodePrefs::locShareHeartbeatSecs(0), NodePrefs::locShareHeartbeatSecs(99));
EXPECT_EQ(NodePrefs::locatorRadiusMeters(1), NodePrefs::locatorRadiusMeters(99));
EXPECT_STREQ("Arrive", NodePrefs::locatorModeLabel(99));
EXPECT_EQ(NodePrefs::gpsAvgSecs(0), NodePrefs::gpsAvgSecs(99));
EXPECT_EQ(NodePrefs::trailAutoPauseSecs(0), NodePrefs::trailAutoPauseSecs(99));
}
TEST(BuildRTTTLString, EmptyWhenLengthIsZero) {
char buf[64] = "unchanged";
NodePrefs::buildRTTTLString(nullptr, 0, 0, buf, sizeof(buf));
EXPECT_STREQ("", buf);
}
TEST(BuildRTTTLString, EncodesANoteAndARestWithTheChosenTempo) {
// pitch=2('d'), octave offset=1 (-> octave 5), dur_idx=1 (-> 8th note)
const uint8_t note = 2 | (1 << 3) | (1 << 5);
// pitch=0 (rest), dur_idx=0 (-> 4th note)
const uint8_t rest = 0;
const uint8_t notes[2] = { note, rest };
size_t write(const uint8_t* buffer, size_t size) override {
_text.append(reinterpret_cast<const char*>(buffer), size);
return size;
}
char buf[64];
NodePrefs::buildRTTTLString(notes, 2, /*bpm_idx=*/1, buf, sizeof(buf));
EXPECT_STREQ("Ring:d=8,o=5,b=90:8d5,4p", buf);
}
size_t print(unsigned char value, int = DEC) override { return emit(value); }
size_t print(int value, int = DEC) override { return emit(value); }
size_t print(unsigned int value, int = DEC) override { return emit(value); }
size_t print(long value, int = DEC) override { return emit(value); }
size_t print(unsigned long value, int = DEC) override { return emit(value); }
size_t print(long long value, int = DEC) override { return emit(value); }
size_t print(unsigned long long value, int = DEC) override { return emit(value); }
const std::string& text() const { return _text; }
};
#if 0
// Re-enable test once we can SET fem_ values in companion
TEST(CompanionNodePrefs, RxGainSettingsRoundTripIndependently) {
NodePrefs saved;
saved.rx_boosted_gain = 0;
saved.radio_fem_rxgain = 1;
saved.radio_fem_txgain = 0;
CaptureStream output;
ASSERT_TRUE(saved.saveSerial(output));
EXPECT_NE(std::string::npos, output.text().find("rxgain:0"));
EXPECT_NE(std::string::npos, output.text().find("fem_rxgain:1"));
EXPECT_NE(std::string::npos, output.text().find("fem_txgain:0"));
ReplayStream input("{radio:{rxgain:1,fem_rxgain:0,fem_txgain:1}}");
NodePrefs loaded;
loaded.rx_boosted_gain = 0;
loaded.radio_fem_rxgain = 1;
loaded.radio_fem_txgain = 0;
ASSERT_TRUE(loaded.loadSerial(input));
EXPECT_EQ(1, loaded.rx_boosted_gain);
EXPECT_EQ(0, loaded.radio_fem_rxgain);
EXPECT_EQ(1, loaded.radio_fem_txgain);
TEST(BuildRTTTLString, OutOfRangeBpmIndexClampsToTheMiddlePreset) {
const uint8_t note = 0; // a single rest is enough to isolate the tempo
char buf[64];
NodePrefs::buildRTTTLString(&note, 1, /*bpm_idx=*/9, buf, sizeof(buf));
EXPECT_STREQ("Ring:d=8,o=5,b=120:4p", buf);
}
#endif
int main(int argc, char** argv) {
::testing::InitGoogleTest(&argc, argv);