diff --git a/zephcore/app/CompanionMesh.cpp b/zephcore/app/CompanionMesh.cpp index de19d91..646eb32 100644 --- a/zephcore/app/CompanionMesh.cpp +++ b/zephcore/app/CompanionMesh.cpp @@ -190,7 +190,6 @@ CompanionMesh::CompanionMesh(mesh::Radio &radio, mesh::MillisecondClock &ms, mes _sync_pending = false; memset(_ack_table, 0, sizeof(_ack_table)); memset(_advert_paths, 0, sizeof(_advert_paths)); - _next_advert_path_idx = 0; _sign_data = nullptr; _sign_data_len = 0; _sign_data_capacity = 0; @@ -752,14 +751,33 @@ void CompanionMesh::onDiscoveredContact(ContactInfo &contact, bool is_new, uint8 // Update advert path table if (path && mesh::Packet::isValidPathLen(path_len)) { - AdvertPath *ap = &_advert_paths[_next_advert_path_idx]; + /* Update this node's existing slot if it has one, else take the least + * recently heard (an empty slot has recv_timestamp 0 and wins first). + * + * This used to round-robin into a write cursor with no lookup, so a + * node heard N times occupied N of the slots, evicting other nodes, + * and findAdvertPath() -- which returns the first array match, not the + * newest -- could hand the app a stale path for a node whose route had + * since changed. */ + AdvertPath *ap = &_advert_paths[0]; + uint32_t oldest = 0xFFFFFFFF; + for (int i = 0; i < ADVERT_PATH_TABLE_SIZE; i++) { + if (memcmp(_advert_paths[i].pubkey_prefix, contact.id.pub_key, + sizeof(AdvertPath::pubkey_prefix)) == 0) { + ap = &_advert_paths[i]; + break; + } + if (_advert_paths[i].recv_timestamp < oldest) { + oldest = _advert_paths[i].recv_timestamp; + ap = &_advert_paths[i]; + } + } memcpy(ap->pubkey_prefix, contact.id.pub_key, 7); memcpy(ap->name, contact.name, sizeof(ap->name)); ap->recv_timestamp = (uint32_t)getRTCClock()->getCurrentTime(); /* path source is from inbound advert; upstream parser bounds it within * the packet payload. AdvertPath::path is MAX_PATH_SIZE-sized. */ ap->path_len = mesh::Packet::copyPath(ap->path, path, MAX_PATH_SIZE, path_len); - _next_advert_path_idx = (_next_advert_path_idx + 1) % ADVERT_PATH_TABLE_SIZE; } // Send push notification diff --git a/zephcore/app/CompanionMesh.h b/zephcore/app/CompanionMesh.h index 64aed0a..cd01194 100644 --- a/zephcore/app/CompanionMesh.h +++ b/zephcore/app/CompanionMesh.h @@ -400,7 +400,6 @@ private: /* Advert path table for tracking recently heard nodes */ AdvertPath _advert_paths[ADVERT_PATH_TABLE_SIZE]; - int _next_advert_path_idx; /* Signing state */ uint8_t *_sign_data; diff --git a/zephcore/app/RepeaterDataStore.cpp b/zephcore/app/RepeaterDataStore.cpp index ea64d31..f60ec32 100644 --- a/zephcore/app/RepeaterDataStore.cpp +++ b/zephcore/app/RepeaterDataStore.cpp @@ -193,7 +193,7 @@ bool RepeaterDataStore::loadPrefs(NodePrefs& prefs) { strcpy(prefs.node_name, "Repeater"); prefs.advert_loc_policy = ADVERT_LOC_PREFS; prefs.loop_detect = LOOP_DETECT_MODERATE; - prefs.path_hash_mode = 1; + /* path_hash_mode = 1 comes from initNodePrefs() now. */ prefs.gps_interval = CONFIG_ZEPHCORE_REPEATER_GPS_INTERVAL_SEC; // repeater default (48h) /* Persist defaults so flash always has a prefs file from boot 1. * Lets later code (e.g. tempradio revert) trust that flash is diff --git a/zephcore/app/RepeaterMesh.cpp b/zephcore/app/RepeaterMesh.cpp index 2d5aed4..b9fb723 100644 --- a/zephcore/app/RepeaterMesh.cpp +++ b/zephcore/app/RepeaterMesh.cpp @@ -1035,7 +1035,9 @@ RepeaterMesh::RepeaterMesh(mesh::MainBoard& board, mesh::Radio& radio, mesh::Mil strcpy(_prefs.node_name, "Repeater"); _prefs.advert_loc_policy = ADVERT_LOC_PREFS; // Repeaters always advertise prefs coordinates _prefs.loop_detect = LOOP_DETECT_MODERATE; - _prefs.path_hash_mode = 1; + /* path_hash_mode = 1 moved into initNodePrefs() -- it is the default for + * every role, and a default listed only at one call site is invisible to + * the others (the companion never got it). */ #if IS_ENABLED(CONFIG_ZEPHCORE_REPEATER_UPLINK) && IS_ENABLED(CONFIG_MQTT_LIB) memset(&_uplink_creds, 0, sizeof(_uplink_creds)); observer_creds_init(&_uplink_creds); diff --git a/zephcore/app/RoomServerMesh.cpp b/zephcore/app/RoomServerMesh.cpp index fb4333e..d045d77 100644 --- a/zephcore/app/RoomServerMesh.cpp +++ b/zephcore/app/RoomServerMesh.cpp @@ -781,7 +781,7 @@ RoomServerMesh::RoomServerMesh(mesh::MainBoard& board, mesh::Radio& radio, mesh: initNodePrefs(&_prefs); strcpy(_prefs.node_name, "Room"); _prefs.advert_loc_policy = ADVERT_LOC_PREFS; // advertise prefs coordinates - _prefs.path_hash_mode = 1; + /* path_hash_mode = 1 moved into initNodePrefs() -- see RepeaterMesh. */ _prefs.disable_fwd = 1; // a room server is an endpoint, never repeats /* Room server: circular post buffer + round-robin push state */ diff --git a/zephcore/helpers/NodePrefs.h b/zephcore/helpers/NodePrefs.h index 82f245a..68e1db4 100644 --- a/zephcore/helpers/NodePrefs.h +++ b/zephcore/helpers/NodePrefs.h @@ -341,7 +341,11 @@ static inline void sanitizeNodePrefs(NodePrefs* p) { if (p->advert_loc_policy > ADVERT_LOC_PREFS) p->advert_loc_policy = ADVERT_LOC_NONE; if (p->loop_detect > LOOP_DETECT_STRICT) p->loop_detect = LOOP_DETECT_MINIMAL; - if (p->path_hash_mode > 2) p->path_hash_mode = 0; + /* Fallback must be the initNodePrefs() default (1 = 2-byte hashes). It was + * 0, so a corrupt byte silently dropped the node to 1-byte path hashes -- + * a value no role defaults to, and one the CLI would not report as the + * default either. */ + if (p->path_hash_mode > 2) p->path_hash_mode = 1; p->autoadd_max_hops = clampPref(p->autoadd_max_hops, 0, 64); p->flood_max = clampPref(p->flood_max, 0, 64); p->flood_max_unscoped = clampPref(p->flood_max_unscoped, 0, 64); @@ -414,6 +418,16 @@ static inline void initNodePrefs(NodePrefs* prefs) { prefs->flood_max = 64; // max hops for flood packets (0 = blocking all!) prefs->flood_max_unscoped = 64; // un-scoped flood hop limit (defaults to flood_max) prefs->flood_max_advert = 8; // ADVERT flood hop limit (upstream default) + /* 2-byte path hashes (mode + 1 = hash size) for everything this node + * originates. ZephCore's default, not upstream's -- Arduino MeshCore + * ships 0 (1-byte) for every role, which collides far more often on a + * dense mesh. It lived only in the RepeaterMesh and RoomServerMesh + * constructors until now, so a fresh companion quietly originated 1-byte + * floods and reported mode 0 to the app, and `set path.hash.mode default` + * wrote 0 even on a repeater that had booted at 1. Deployed nodes keep + * whatever they stored: path_hash_mode sits at a fixed prefs offset that + * always loads, so this only changes a factory-fresh node. */ + prefs->path_hash_mode = 1; // 2-byte path hashes prefs->interference_threshold = 0; prefs->leds_disabled = 0; // LEDs on prefs->leds_radio_mode = LEDS_RADIO_TX; // activity LED on transmit, as before diff --git a/zephcore/src/Packet.cpp b/zephcore/src/Packet.cpp index 6e8fbb3..7497fa3 100644 --- a/zephcore/src/Packet.cpp +++ b/zephcore/src/Packet.cpp @@ -41,8 +41,22 @@ size_t Packet::writePath(uint8_t *dest, const uint8_t *src, size_t src_len, uint uint8_t Packet::copyPath(uint8_t *dest, const uint8_t *src, size_t src_len, uint8_t path_len) { - size_t written = writePath(dest, src, src_len, path_len); - return written > 0 ? path_len : 0; + /* A zero-hop path copies no bytes, so writePath()'s 0 return is ambiguous + * there: it means both "rejected" and "nothing to copy". Taking it as a + * rejection discards the hash-size bits (7:6), which are all a zero-hop + * path_len carries -- a directly-heard 2-byte-hash advert (0x40) came out + * as 0x00 and every consumer then read it as 1-byte. Decode the hop count + * first so the empty case is validated instead of copied. + * + * isValidPathLen() still rejects the reserved hash size 4, so + * OUT_PATH_UNKNOWN (0xFF) and 0xC0 keep returning 0 as before; callers + * that test `copyPath(...) == 0` for a malformed path are unaffected, as + * long as they already special-case a path_len of 0 (which is itself a + * legal zero-hop, 1-byte-hash path and returns 0 either way). */ + if ((path_len & 0x3F) == 0) { + return isValidPathLen(path_len) ? path_len : 0; + } + return writePath(dest, src, src_len, path_len) > 0 ? path_len : 0; } int Packet::getRawLength() const