fix: pad short PAN IDs for the XBee, and force router role at boot

include/droid_store.h

@@ -2,12 +2,14 @@
 
 #include <cstddef>
 
+#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;

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);

include/pan_id.h

@@ -0,0 +1,24 @@
+#pragma once
+
+#include <cstddef>
+
+// 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

include/xbee_control.h

@@ -3,6 +3,7 @@
 #include <cstddef>
 
 #include "droid_switcher.h"
+#include "xbee_role.h"
 #include "xbee_spi.h"
 
 // Real XbeeTransport (see droid_switcher.h), over the SPI transport in
@@ -11,14 +12,20 @@
 // 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(); }
 
   bool leaveNetwork() override;
   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

include/xbee_role.h

@@ -0,0 +1,29 @@
+#pragma once
+
+#include <cstdint>
+
+// 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);

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.");

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;
+}

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()) {

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";

src/pan_id.cpp

@@ -0,0 +1,39 @@
+#include "pan_id.h"
+
+#include <cstring>
+
+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<char>(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

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

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;
+}

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();
 }