From a7d846fbfff4abb61f4c0dc397625d23bae17add Mon Sep 17 00:00:00 2001 From: Jessica Janiuk Date: Sat, 19 Sep 2026 08:43:27 -0700 Subject: [PATCH 1/2] fix: pad short PAN IDs for the XBee, and force router role at boot Short PAN IDs (e.g. "4133") could be saved and displayed, but switching to one failed with "Set PAN failed": XbeeControl::setPanId requires exactly 16 hex digits and rejected it before sending anything. The boot-time droid-name lookup also strcmp'd the saved "4133" against the module's zero-padded "0000000000004133", so it never matched. - Add PanId::normalize/equivalent (pure, tested): uppercase + left-pad to 16 digits. - DroidSwitcher pads before calling the transport; invalid IDs now report "Invalid PAN ID" without touching the network. - DroidStore::findByPanId does the boot-time match with equivalent(); the .ino uses it instead of strcmp. - Manage Droids: finishing PAN ID entry with nothing typed restarts the entry instead of saving an ID that would pad to all zeros (the "unconfigured" value Factory Reset uses). Also, at boot, read the XBee's CE parameter and, if it isn't 0 (router), write CE=0, WR, AC. The check is pure (ensureRouterRole over an XbeeRoleTransport) with the AT sequencing in XbeeControl. Co-Authored-By: Claude Sonnet 5 --- include/droid_store.h | 9 +- include/droid_switcher.h | 4 + include/pan_id.h | 24 +++++ include/xbee_control.h | 9 +- include/xbee_role.h | 29 ++++++ src/SnipsController.ino | 26 ++++-- src/droid_store.cpp | 10 +++ src/droid_switcher.cpp | 8 +- src/menu.cpp | 8 ++ src/pan_id.cpp | 39 ++++++++ src/xbee_control.cpp | 21 +++++ src/xbee_role.cpp | 22 +++++ test/test_droid_store/test_droid_store.cpp | 21 +++++ .../test_droid_switcher.cpp | 28 +++++- test/test_menu/test_menu.cpp | 44 +++++++++ test/test_pan_id/test_pan_id.cpp | 77 ++++++++++++++++ test/test_xbee_role/test_xbee_role.cpp | 89 +++++++++++++++++++ 17 files changed, 458 insertions(+), 10 deletions(-) create mode 100644 include/pan_id.h create mode 100644 include/xbee_role.h create mode 100644 src/pan_id.cpp create mode 100644 src/xbee_role.cpp create mode 100644 test/test_pan_id/test_pan_id.cpp create mode 100644 test/test_xbee_role/test_xbee_role.cpp diff --git a/include/droid_store.h b/include/droid_store.h index 53aaae1..73a1031 100644 --- a/include/droid_store.h +++ b/include/droid_store.h @@ -2,12 +2,14 @@ #include +#include "pan_id.h" + // Pure in-memory droid list (name + PAN ID pairs), the data a user builds // up via the Manage Droids menu screen. Knows nothing about persistence — // see droid_persistence.h for the thin NVS adapter that saves/restores it. struct DroidEntry { static constexpr size_t kMaxNameLength = 16; - static constexpr size_t kMaxPanIdLength = 16; // 64-bit PAN ID, hex + static constexpr size_t kMaxPanIdLength = PanId::kHexLength; char name[kMaxNameLength + 1] = {}; char panId[kMaxPanIdLength + 1] = {}; @@ -29,6 +31,11 @@ class DroidStore { // Returns false (no-op) if index is out of range. bool remove(size_t index); + // Finds the first droid whose PAN ID is equivalent to panId (see + // PanId::equivalent — "4133" matches "0000000000004133"), writing its + // index to outIndex. Returns false, leaving outIndex untouched, if none. + bool findByPanId(const char *panId, size_t *outIndex) const; + private: DroidEntry entries_[kMaxDroids]; size_t count_ = 0; diff --git a/include/droid_switcher.h b/include/droid_switcher.h index 78a38a3..bec3202 100644 --- a/include/droid_switcher.h +++ b/include/droid_switcher.h @@ -14,6 +14,7 @@ class XbeeTransport { enum class DroidSwitchResult { kSuccess, + kInvalidPanId, kLeaveFailed, kSetPanFailed, kRejoinFailed, @@ -26,6 +27,9 @@ enum class DroidSwitchResult { // XbeeControl itself is hardware-dependent and excluded from that build. class DroidSwitcher { public: + // panId may be short ("4133") — it's normalized to the full 16-digit + // form (see pan_id.h) before reaching the transport, and rejected with + // kInvalidPanId (before touching the network) if it isn't valid hex. // transport may be null (returns kNoTransport without touching it). static DroidSwitchResult switchTo(const char *panId, XbeeTransport *transport); diff --git a/include/pan_id.h b/include/pan_id.h new file mode 100644 index 0000000..5cc916f --- /dev/null +++ b/include/pan_id.h @@ -0,0 +1,24 @@ +#pragma once + +#include + +// Pure helpers for PAN IDs entered as short hex strings (e.g. "4133"). +// The XBee's ID parameter is a full 64-bit value and reports it back as +// 16 zero-padded hex characters, so anything that hands a user-entered +// PAN ID to the radio or compares one against what the radio reports has +// to go through normalize() first. +namespace PanId { + +constexpr size_t kHexLength = 16; // 64-bit PAN ID, hex + +// Uppercases and left-pads with zeros to kHexLength characters, writing +// them plus a null terminator into out (needs kHexLength + 1 bytes). +// Returns false, leaving out untouched, for null, empty, over-long, or +// non-hex input. +bool normalize(const char *in, char *out); + +// True if both normalize to the same value ("4133" == "0000000000004133"). +// False if either is invalid. +bool equivalent(const char *a, const char *b); + +} // namespace PanId diff --git a/include/xbee_control.h b/include/xbee_control.h index 503c1a0..c4e0940 100644 --- a/include/xbee_control.h +++ b/include/xbee_control.h @@ -3,6 +3,7 @@ #include #include "droid_switcher.h" +#include "xbee_role.h" #include "xbee_spi.h" // Real XbeeTransport (see droid_switcher.h), over the SPI transport in @@ -11,7 +12,7 @@ // reading of Digi's XBee3 manual, not yet validated against real // hardware — confirm during this PR's bring-up and adjust here if the // sequence needs correcting. -class XbeeControl : public XbeeTransport { +class XbeeControl : public XbeeTransport, public XbeeRoleTransport { public: void begin() { spi_.begin(); } @@ -19,6 +20,12 @@ class XbeeControl : public XbeeTransport { bool setPanId(const char *panId) override; bool rejoinNetwork() override; + // "CE": 0 = join a network (router), 1 = form one (coordinator). Setting + // it commits ("WR") and applies ("AC") — same best-effort, not yet + // hardware-validated sequencing as the rest of this class. + bool queryCoordinatorEnable(uint8_t *outValue) override; + bool setCoordinatorEnable(uint8_t value) override; + // Queries the module's own 64-bit address (low 32 bits, "SL") for // display in the Device Info menu screen — this is what a user reads // off-screen to enter into Amidala. Writes up to 8 hex chars + a null diff --git a/include/xbee_role.h b/include/xbee_role.h new file mode 100644 index 0000000..82c5e75 --- /dev/null +++ b/include/xbee_role.h @@ -0,0 +1,29 @@ +#pragma once + +#include + +// Abstraction over reading/writing the XBee's CE ("Coordinator Enable") +// parameter — 0 = join a network (router), 1 = form one (coordinator). +// XbeeControl (xbee_control.h) is the real, hardware-dependent +// implementation; tests use a fake. +class XbeeRoleTransport { + public: + virtual ~XbeeRoleTransport() = default; + virtual bool queryCoordinatorEnable(uint8_t *outValue) = 0; + // Writes CE, commits it to the module's flash, and applies it. + virtual bool setCoordinatorEnable(uint8_t value) = 0; +}; + +enum class XbeeRoleResult { + kAlreadyRouter, + kSwitchedToRouter, + kQueryFailed, + kSetFailed, + kNoTransport, +}; + +// Every controller must be a router: a coordinator would form its own +// network instead of joining a droid's. Run once at boot — reads CE and, +// if it's anything other than 0, sets it to 0 (see XbeeRoleTransport). +// transport may be null (returns kNoTransport without touching it). +XbeeRoleResult ensureRouterRole(XbeeRoleTransport *transport); diff --git a/src/SnipsController.ino b/src/SnipsController.ino index 4f139d0..133f120 100644 --- a/src/SnipsController.ino +++ b/src/SnipsController.ino @@ -260,6 +260,24 @@ void setup() { } xbeeControl.begin(); + + // A controller must join a droid's network, never form its own — CE=1 + // (coordinator) would strand it on a PAN of its own. + switch (ensureRouterRole(&xbeeControl)) { + case XbeeRoleResult::kSwitchedToRouter: + Serial.println("XBee was set to coordinator; switched to router."); + break; + case XbeeRoleResult::kQueryFailed: + Serial.println("XBee role (CE) query failed at boot."); + break; + case XbeeRoleResult::kSetFailed: + Serial.println("XBee role (CE) could not be set to router at boot."); + break; + case XbeeRoleResult::kAlreadyRouter: + case XbeeRoleResult::kNoTransport: + break; + } + menuController.setXbeeTransport(&xbeeControl); // The XBee module remembers its own PAN ID across power cycles once @@ -271,11 +289,9 @@ void setup() { char currentPanId[17]; if (xbeeControl.queryPanId(currentPanId, sizeof(currentPanId))) { const DroidStore &droidStore = menuController.droidStore(); - for (size_t i = 0; i < droidStore.count(); ++i) { - if (std::strcmp(droidStore.at(i).panId, currentPanId) == 0) { - menuController.setCurrentDroidName(droidStore.at(i).name); - break; - } + size_t matchIndex; + if (droidStore.findByPanId(currentPanId, &matchIndex)) { + menuController.setCurrentDroidName(droidStore.at(matchIndex).name); } } else { Serial.println("XBee PAN ID query failed at boot."); diff --git a/src/droid_store.cpp b/src/droid_store.cpp index e05897e..77561a4 100644 --- a/src/droid_store.cpp +++ b/src/droid_store.cpp @@ -33,3 +33,13 @@ bool DroidStore::remove(size_t index) { --count_; return true; } + +bool DroidStore::findByPanId(const char *panId, size_t *outIndex) const { + for (size_t i = 0; i < count_; ++i) { + if (PanId::equivalent(entries_[i].panId, panId)) { + *outIndex = i; + return true; + } + } + return false; +} diff --git a/src/droid_switcher.cpp b/src/droid_switcher.cpp index 9635581..dfc6f21 100644 --- a/src/droid_switcher.cpp +++ b/src/droid_switcher.cpp @@ -1,14 +1,20 @@ #include "droid_switcher.h" +#include "pan_id.h" + DroidSwitchResult DroidSwitcher::switchTo(const char *panId, XbeeTransport *transport) { if (transport == nullptr) { return DroidSwitchResult::kNoTransport; } + char fullPanId[PanId::kHexLength + 1]; + if (!PanId::normalize(panId, fullPanId)) { + return DroidSwitchResult::kInvalidPanId; + } if (!transport->leaveNetwork()) { return DroidSwitchResult::kLeaveFailed; } - if (!transport->setPanId(panId)) { + if (!transport->setPanId(fullPanId)) { return DroidSwitchResult::kSetPanFailed; } if (!transport->rejoinNetwork()) { diff --git a/src/menu.cpp b/src/menu.cpp index 3e0936a..65f6be7 100644 --- a/src/menu.cpp +++ b/src/menu.cpp @@ -257,6 +257,13 @@ void MenuController::onEnter(int rawTrigger, int rawStickX, int rawStickY) { case MenuScreen::kManageDroidsEnterPanId: panIdEntry_.commitChar(); if (panIdEntry_.done()) { + if (panIdEntry_.length() == 0) { + // An empty PAN ID would pad out to all zeros — the "unconfigured" + // value Factory Reset uses — so start the entry over instead. + panIdEntry_.reset(TextEntryWidget::CharSet::kHex, + DroidEntry::kMaxPanIdLength); + break; + } droidStore_.add(nameEntry_.text(), panIdEntry_.text()); droidStoreChanged_ = true; droidListIndex_ = 0; @@ -459,6 +466,7 @@ void formatSeconds(int totalSeconds, char *out, size_t outCapacity) { const char *switchResultText(DroidSwitchResult result) { switch (result) { case DroidSwitchResult::kSuccess: return "Success!"; + case DroidSwitchResult::kInvalidPanId: return "Invalid PAN ID"; case DroidSwitchResult::kLeaveFailed: return "Leave failed"; case DroidSwitchResult::kSetPanFailed: return "Set PAN failed"; case DroidSwitchResult::kRejoinFailed: return "Rejoin failed"; diff --git a/src/pan_id.cpp b/src/pan_id.cpp new file mode 100644 index 0000000..e14d9b2 --- /dev/null +++ b/src/pan_id.cpp @@ -0,0 +1,39 @@ +#include "pan_id.h" + +#include + +namespace PanId { + +bool normalize(const char *in, char *out) { + if (in == nullptr) return false; + const size_t length = std::strlen(in); + if (length == 0 || length > kHexLength) return false; + + char padded[kHexLength + 1]; + const size_t padding = kHexLength - length; + std::memset(padded, '0', padding); + for (size_t i = 0; i < length; ++i) { + const char c = in[i]; + if (c >= '0' && c <= '9') { + padded[padding + i] = c; + } else if (c >= 'A' && c <= 'F') { + padded[padding + i] = c; + } else if (c >= 'a' && c <= 'f') { + padded[padding + i] = static_cast(c - 'a' + 'A'); + } else { + return false; + } + } + padded[kHexLength] = '\0'; + std::memcpy(out, padded, kHexLength + 1); + return true; +} + +bool equivalent(const char *a, const char *b) { + char normalizedA[kHexLength + 1]; + char normalizedB[kHexLength + 1]; + return normalize(a, normalizedA) && normalize(b, normalizedB) && + std::strcmp(normalizedA, normalizedB) == 0; +} + +} // namespace PanId diff --git a/src/xbee_control.cpp b/src/xbee_control.cpp index c837925..b904ac5 100644 --- a/src/xbee_control.cpp +++ b/src/xbee_control.cpp @@ -63,6 +63,27 @@ bool XbeeControl::rejoinNetwork() { return spi_.sendAtCommand("AC", nullptr, 0, nullptr, 0, nullptr); } +bool XbeeControl::queryCoordinatorEnable(uint8_t *outValue) { + uint8_t value[1]; + uint8_t valueLength = 0; + if (!spi_.sendAtCommand("CE", nullptr, 0, value, sizeof(value), + &valueLength) || + valueLength != sizeof(value)) { + return false; + } + *outValue = value[0]; + return true; +} + +bool XbeeControl::setCoordinatorEnable(uint8_t value) { + if (!spi_.sendAtCommand("CE", &value, sizeof(value), nullptr, 0, nullptr)) { + return false; + } + // "WR" so the role survives power cycles, "AC" so it takes effect now. + return spi_.sendAtCommand("WR", nullptr, 0, nullptr, 0, nullptr) && + spi_.sendAtCommand("AC", nullptr, 0, nullptr, 0, nullptr); +} + bool XbeeControl::querySerialLow(char *outHex, size_t outHexCapacity) { if (outHexCapacity < 9) { return false; // 8 hex chars + null diff --git a/src/xbee_role.cpp b/src/xbee_role.cpp new file mode 100644 index 0000000..0097566 --- /dev/null +++ b/src/xbee_role.cpp @@ -0,0 +1,22 @@ +#include "xbee_role.h" + +namespace { +constexpr uint8_t kRouterCoordinatorEnable = 0; +} // namespace + +XbeeRoleResult ensureRouterRole(XbeeRoleTransport *transport) { + if (transport == nullptr) { + return XbeeRoleResult::kNoTransport; + } + uint8_t coordinatorEnable = 0; + if (!transport->queryCoordinatorEnable(&coordinatorEnable)) { + return XbeeRoleResult::kQueryFailed; + } + if (coordinatorEnable == kRouterCoordinatorEnable) { + return XbeeRoleResult::kAlreadyRouter; + } + if (!transport->setCoordinatorEnable(kRouterCoordinatorEnable)) { + return XbeeRoleResult::kSetFailed; + } + return XbeeRoleResult::kSwitchedToRouter; +} diff --git a/test/test_droid_store/test_droid_store.cpp b/test/test_droid_store/test_droid_store.cpp index 2b88d82..e224ecc 100644 --- a/test/test_droid_store/test_droid_store.cpp +++ b/test/test_droid_store/test_droid_store.cpp @@ -68,6 +68,25 @@ void test_remove_out_of_range_is_noop() { TEST_ASSERT_EQUAL_INT(1, store.count()); } +void test_find_by_pan_id_matches_short_saved_id_against_padded_query() { + // Regression: the XBee reports "0000000000004133" but the user saved + // "4133", so the boot-time droid-name lookup never found it. + DroidStore store; + store.add("R2-D2", "1111111111111111"); + store.add("BB-8", "4133"); + size_t index = 99; + TEST_ASSERT_TRUE(store.findByPanId("0000000000004133", &index)); + TEST_ASSERT_EQUAL_INT(1, index); +} + +void test_find_by_pan_id_returns_false_when_absent() { + DroidStore store; + store.add("R2-D2", "1111111111111111"); + size_t index = 99; + TEST_ASSERT_FALSE(store.findByPanId("0000000000004133", &index)); + TEST_ASSERT_EQUAL_INT(99, index); +} + int main(int argc, char **argv) { UNITY_BEGIN(); RUN_TEST(test_starts_empty); @@ -78,5 +97,7 @@ int main(int argc, char **argv) { RUN_TEST(test_at_out_of_range_returns_empty_entry); RUN_TEST(test_remove_shifts_subsequent_entries_down); RUN_TEST(test_remove_out_of_range_is_noop); + RUN_TEST(test_find_by_pan_id_matches_short_saved_id_against_padded_query); + RUN_TEST(test_find_by_pan_id_returns_false_when_absent); return UNITY_END(); } diff --git a/test/test_droid_switcher/test_droid_switcher.cpp b/test/test_droid_switcher/test_droid_switcher.cpp index cedf757..7481e2f 100644 --- a/test/test_droid_switcher/test_droid_switcher.cpp +++ b/test/test_droid_switcher/test_droid_switcher.cpp @@ -1,5 +1,7 @@ #include +#include + #include "droid_switcher.h" void setUp(void) {} @@ -18,7 +20,7 @@ class FakeTransport : public XbeeTransport { bool leaveCalled = false; bool setPanCalled = false; bool rejoinCalled = false; - const char *lastPanId = nullptr; + std::string lastPanId; bool leaveNetwork() override { leaveCalled = true; @@ -44,7 +46,7 @@ void test_switch_succeeds_when_every_step_succeeds() { TEST_ASSERT_TRUE(transport.leaveCalled); TEST_ASSERT_TRUE(transport.setPanCalled); TEST_ASSERT_TRUE(transport.rejoinCalled); - TEST_ASSERT_EQUAL_STRING("1111111111111111", transport.lastPanId); + TEST_ASSERT_EQUAL_STRING("1111111111111111", transport.lastPanId.c_str()); } void test_switch_with_null_transport_fails_without_crashing() { @@ -78,6 +80,26 @@ void test_switch_reports_rejoin_failure() { DroidSwitcher::switchTo("1111111111111111", &transport)); } +void test_switch_pads_short_pan_id_to_full_length() { + // Regression: "4133" reached XbeeControl::setPanId as-is, which needs + // exactly 16 hex digits, so switching failed with "Set PAN failed". + FakeTransport transport; + TEST_ASSERT_TRUE(DroidSwitchResult::kSuccess == + DroidSwitcher::switchTo("4133", &transport)); + TEST_ASSERT_EQUAL_STRING("0000000000004133", transport.lastPanId.c_str()); +} + +void test_switch_rejects_invalid_pan_id_before_touching_the_network() { + FakeTransport transport; + TEST_ASSERT_TRUE(DroidSwitchResult::kInvalidPanId == + DroidSwitcher::switchTo("", &transport)); + TEST_ASSERT_TRUE(DroidSwitchResult::kInvalidPanId == + DroidSwitcher::switchTo("41G3", &transport)); + TEST_ASSERT_FALSE(transport.leaveCalled); + TEST_ASSERT_FALSE(transport.setPanCalled); + TEST_ASSERT_FALSE(transport.rejoinCalled); +} + // XbeeControl itself (the real XbeeTransport, using XbeeSpi) is // hardware-dependent now and excluded from native builds — see // platformio.ini. Nothing here instantiates it directly; DroidSwitcher is @@ -90,5 +112,7 @@ int main(int argc, char **argv) { RUN_TEST(test_switch_stops_after_leave_failure); RUN_TEST(test_switch_stops_after_set_pan_failure); RUN_TEST(test_switch_reports_rejoin_failure); + RUN_TEST(test_switch_pads_short_pan_id_to_full_length); + RUN_TEST(test_switch_rejects_invalid_pan_id_before_touching_the_network); return UNITY_END(); } diff --git a/test/test_menu/test_menu.cpp b/test/test_menu/test_menu.cpp index a583110..f188e3e 100644 --- a/test/test_menu/test_menu.cpp +++ b/test/test_menu/test_menu.cpp @@ -451,6 +451,48 @@ void test_manage_droids_pan_id_backspace_and_cancel() { TEST_ASSERT_EQUAL_INT(0, menu.droidStore().count()); } +void test_manage_droids_empty_pan_id_is_not_saved() { + // An empty PAN ID would pad to all zeros (Factory Reset's "unconfigured" + // value), so finishing entry with nothing typed restarts the entry. + MenuController menu; + selectMainMenuItem(&menu, MainMenuItem::kManageDroids); + menu.onEnter(0, 0, 0); // -> kManageDroidsList + menu.onEnter(0, 0, 0); // -> enter name + menu.onUp(); // space -> DONE + menu.onEnter(0, 0, 0); // finish name -> enter PAN ID + + menu.onUp(); // '0' -> wraps to DONE + menu.onEnter(0, 0, 0); // finish with nothing typed + + TEST_ASSERT_TRUE(MenuScreen::kManageDroidsEnterPanId == + menu.currentScreen()); + TEST_ASSERT_FALSE(menu.panIdEntry().done()); + TEST_ASSERT_EQUAL_INT(0, menu.droidStore().count()); + TEST_ASSERT_FALSE(menu.consumeDroidStoreChanged()); + + // Entry is usable again afterwards. + menu.onDown(); // '0' -> '1' + menu.onEnter(0, 0, 0); // commit '1' + menu.onUp(); // -> DONE + menu.onEnter(0, 0, 0); + TEST_ASSERT_TRUE(MenuScreen::kManageDroidsList == menu.currentScreen()); + TEST_ASSERT_EQUAL_STRING("1", menu.droidStore().at(0).panId); +} + +void test_switch_droid_pads_short_saved_pan_id() { + MenuController menu; + FakeTransport transport; + menu.setXbeeTransport(&transport); + DroidStore store; + store.add("R2-D2", "4133"); + menu.setDroidStore(store); + selectMainMenuItem(&menu, MainMenuItem::kSwitchDroid); + menu.onEnter(0, 0, 0); // -> kSwitchDroidList + menu.onEnter(0, 0, 0); // select R2-D2, switch + TEST_ASSERT_TRUE(DroidSwitchResult::kSuccess == menu.lastSwitchResult()); + TEST_ASSERT_EQUAL_STRING("0000000000004133", transport.lastPanId.c_str()); +} + // ---- manage droids: delete ----------------------------------------------------- void test_manage_droids_delete_flow_removes_entry() { @@ -969,6 +1011,8 @@ int main(int argc, char **argv) { RUN_TEST(test_manage_droids_back_with_text_backspaces_instead_of_cancelling); RUN_TEST(test_manage_droids_list_navigation_wraps_over_entries_and_add_new); RUN_TEST(test_manage_droids_pan_id_backspace_and_cancel); + RUN_TEST(test_manage_droids_empty_pan_id_is_not_saved); + RUN_TEST(test_switch_droid_pads_short_saved_pan_id); RUN_TEST(test_manage_droids_delete_flow_removes_entry); RUN_TEST(test_manage_droids_delete_confirm_back_cancels); RUN_TEST(test_display_config_cycles_selected_slot_source); diff --git a/test/test_pan_id/test_pan_id.cpp b/test/test_pan_id/test_pan_id.cpp new file mode 100644 index 0000000..8a14acc --- /dev/null +++ b/test/test_pan_id/test_pan_id.cpp @@ -0,0 +1,77 @@ +#include + +#include "pan_id.h" + +void setUp(void) {} +void tearDown(void) {} + +void test_normalize_left_pads_short_id_with_zeros() { + char out[PanId::kHexLength + 1]; + TEST_ASSERT_TRUE(PanId::normalize("4133", out)); + TEST_ASSERT_EQUAL_STRING("0000000000004133", out); +} + +void test_normalize_leaves_full_length_id_unchanged() { + char out[PanId::kHexLength + 1]; + TEST_ASSERT_TRUE(PanId::normalize("0013A20041A7B3C2", out)); + TEST_ASSERT_EQUAL_STRING("0013A20041A7B3C2", out); +} + +void test_normalize_accepts_single_digit() { + char out[PanId::kHexLength + 1]; + TEST_ASSERT_TRUE(PanId::normalize("1", out)); + TEST_ASSERT_EQUAL_STRING("0000000000000001", out); +} + +void test_normalize_uppercases_lowercase_hex() { + char out[PanId::kHexLength + 1]; + TEST_ASSERT_TRUE(PanId::normalize("ab12", out)); + TEST_ASSERT_EQUAL_STRING("000000000000AB12", out); +} + +void test_normalize_accepts_all_zeros() { + // Factory Reset's "unconfigured" PAN ID must still be sendable. + char out[PanId::kHexLength + 1]; + TEST_ASSERT_TRUE(PanId::normalize("0000000000000000", out)); + TEST_ASSERT_EQUAL_STRING("0000000000000000", out); +} + +void test_normalize_rejects_null_empty_overlong_and_non_hex() { + char out[PanId::kHexLength + 1] = "untouched"; + TEST_ASSERT_FALSE(PanId::normalize(nullptr, out)); + TEST_ASSERT_FALSE(PanId::normalize("", out)); + TEST_ASSERT_FALSE(PanId::normalize("00000000000000001", out)); + TEST_ASSERT_FALSE(PanId::normalize("41G3", out)); + TEST_ASSERT_FALSE(PanId::normalize("41 3", out)); + TEST_ASSERT_EQUAL_STRING("untouched", out); +} + +void test_equivalent_matches_short_and_padded_forms() { + // Regression: the XBee reports "0000000000004133" but the saved droid + // holds "4133"; a plain strcmp never matched them at boot. + TEST_ASSERT_TRUE(PanId::equivalent("4133", "0000000000004133")); + TEST_ASSERT_TRUE(PanId::equivalent("0000000000004133", "4133")); + TEST_ASSERT_TRUE(PanId::equivalent("4133", "4133")); + TEST_ASSERT_TRUE(PanId::equivalent("abcd", "ABCD")); +} + +void test_equivalent_rejects_different_or_invalid() { + TEST_ASSERT_FALSE(PanId::equivalent("4133", "4134")); + TEST_ASSERT_FALSE(PanId::equivalent("4133", "1400000000004133")); + TEST_ASSERT_FALSE(PanId::equivalent("", "")); + TEST_ASSERT_FALSE(PanId::equivalent("zz", "zz")); + TEST_ASSERT_FALSE(PanId::equivalent(nullptr, "4133")); +} + +int main(int argc, char **argv) { + UNITY_BEGIN(); + RUN_TEST(test_normalize_left_pads_short_id_with_zeros); + RUN_TEST(test_normalize_leaves_full_length_id_unchanged); + RUN_TEST(test_normalize_accepts_single_digit); + RUN_TEST(test_normalize_uppercases_lowercase_hex); + RUN_TEST(test_normalize_accepts_all_zeros); + RUN_TEST(test_normalize_rejects_null_empty_overlong_and_non_hex); + RUN_TEST(test_equivalent_matches_short_and_padded_forms); + RUN_TEST(test_equivalent_rejects_different_or_invalid); + return UNITY_END(); +} diff --git a/test/test_xbee_role/test_xbee_role.cpp b/test/test_xbee_role/test_xbee_role.cpp new file mode 100644 index 0000000..3da3cf4 --- /dev/null +++ b/test/test_xbee_role/test_xbee_role.cpp @@ -0,0 +1,89 @@ +#include + +#include "xbee_role.h" + +void setUp(void) {} +void tearDown(void) {} + +namespace { + +class FakeRoleTransport : public XbeeRoleTransport { + public: + bool queryResult = true; + uint8_t coordinatorEnable = 0; + bool setResult = true; + + bool setCalled = false; + uint8_t lastSetValue = 0xFF; + + bool queryCoordinatorEnable(uint8_t *outValue) override { + if (!queryResult) return false; + *outValue = coordinatorEnable; + return true; + } + bool setCoordinatorEnable(uint8_t value) override { + setCalled = true; + lastSetValue = value; + return setResult; + } +}; + +} // namespace + +void test_router_is_left_alone() { + FakeRoleTransport transport; + transport.coordinatorEnable = 0; + TEST_ASSERT_TRUE(XbeeRoleResult::kAlreadyRouter == + ensureRouterRole(&transport)); + TEST_ASSERT_FALSE(transport.setCalled); +} + +void test_coordinator_is_switched_to_router() { + FakeRoleTransport transport; + transport.coordinatorEnable = 1; + TEST_ASSERT_TRUE(XbeeRoleResult::kSwitchedToRouter == + ensureRouterRole(&transport)); + TEST_ASSERT_TRUE(transport.setCalled); + TEST_ASSERT_EQUAL_UINT8(0, transport.lastSetValue); +} + +void test_any_nonzero_ce_is_switched_to_router() { + FakeRoleTransport transport; + transport.coordinatorEnable = 2; + TEST_ASSERT_TRUE(XbeeRoleResult::kSwitchedToRouter == + ensureRouterRole(&transport)); + TEST_ASSERT_EQUAL_UINT8(0, transport.lastSetValue); +} + +void test_query_failure_does_not_write() { + FakeRoleTransport transport; + transport.queryResult = false; + TEST_ASSERT_TRUE(XbeeRoleResult::kQueryFailed == + ensureRouterRole(&transport)); + TEST_ASSERT_FALSE(transport.setCalled); +} + +void test_set_failure_is_reported() { + FakeRoleTransport transport; + transport.coordinatorEnable = 1; + transport.setResult = false; + TEST_ASSERT_TRUE(XbeeRoleResult::kSetFailed == ensureRouterRole(&transport)); +} + +void test_null_transport_fails_without_crashing() { + TEST_ASSERT_TRUE(XbeeRoleResult::kNoTransport == ensureRouterRole(nullptr)); +} + +// XbeeControl itself (the real XbeeRoleTransport, using XbeeSpi) is +// hardware-dependent and excluded from native builds — see platformio.ini. + +int main(int argc, char **argv) { + UNITY_BEGIN(); + RUN_TEST(test_router_is_left_alone); + RUN_TEST(test_coordinator_is_switched_to_router); + RUN_TEST(test_any_nonzero_ce_is_switched_to_router); + RUN_TEST(test_query_failure_does_not_write); + RUN_TEST(test_set_failure_is_reported); + RUN_TEST(test_null_transport_fails_without_crashing); + return UNITY_END(); +} From eff2c29b1794a1904fa60743da2c14a530b88f67 Mon Sep 17 00:00:00 2001 From: Jessica Janiuk Date: Sat, 19 Sep 2026 08:48:52 -0700 Subject: [PATCH 2/2] test: cover the "Invalid PAN ID" switch result screen Co-Authored-By: Claude Sonnet 5 --- test/test_menu/test_menu.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/test_menu/test_menu.cpp b/test/test_menu/test_menu.cpp index f188e3e..4f25f55 100644 --- a/test/test_menu/test_menu.cpp +++ b/test/test_menu/test_menu.cpp @@ -493,6 +493,24 @@ void test_switch_droid_pads_short_saved_pan_id() { TEST_ASSERT_EQUAL_STRING("0000000000004133", transport.lastPanId.c_str()); } +void test_switch_droid_with_invalid_saved_pan_id_shows_invalid_pan_id() { + MenuController menu; + FakeTransport transport; + menu.setXbeeTransport(&transport); + DroidStore store; + store.add("Bad", "ZZ"); + menu.setDroidStore(store); + selectMainMenuItem(&menu, MainMenuItem::kSwitchDroid); + menu.onEnter(0, 0, 0); // -> kSwitchDroidList + menu.onEnter(0, 0, 0); // select Bad, attempt switch + + TEST_ASSERT_TRUE(DroidSwitchResult::kInvalidPanId == menu.lastSwitchResult()); + TEST_ASSERT_FALSE(transport.leaveCalled); + ScreenBuffer screen; + renderMenuScreen(menu, &screen); + TEST_ASSERT_EQUAL_STRING("Invalid PAN ID", screen.line(1)); +} + // ---- manage droids: delete ----------------------------------------------------- void test_manage_droids_delete_flow_removes_entry() { @@ -1013,6 +1031,7 @@ int main(int argc, char **argv) { RUN_TEST(test_manage_droids_pan_id_backspace_and_cancel); RUN_TEST(test_manage_droids_empty_pan_id_is_not_saved); RUN_TEST(test_switch_droid_pads_short_saved_pan_id); + RUN_TEST(test_switch_droid_with_invalid_saved_pan_id_shows_invalid_pan_id); RUN_TEST(test_manage_droids_delete_flow_removes_entry); RUN_TEST(test_manage_droids_delete_confirm_back_cancels); RUN_TEST(test_display_config_cycles_selected_slot_source);