mirror of
https://github.com/agessaman/meshcore-bot.git
synced 2026-09-17 05:04:19 +00:00
fix(correlation): find the channel message's own RF row, not the newest (#255)
Verifying a channel message against the RF cache only ever checked the row strategy 4 returned — the most recent packet heard. That assumes the RF log row and the decoded CHAN event for one reception arrive back to back with nothing in between. On a dense mesh they do not: a repeater's echo of the very same packet is routinely logged in the gap. The reporter's message was heard directly (SNR 13.25, 0 hops) and again via repeater f0 185 ms later (SNR 12.0, 1 hop). Both rows carry packet hash 392926C85DCB87D0, but the check saw only the echo, disagreed on path length and SNR, and left the route unresolved — so the bot withheld a path it had decoded correctly and answered "No path information available in current message". The reporter's own observation that MultiTest still reported paths is the tell: multitest reads recent_rf_data directly and never consults the match tag, so the radio data was there the whole time. The cache is now searched for the row the payload matches rather than testing just the newest one. #80's guarantee is unchanged — a route is still only ever attributed on a positive match, never on recency — so this widens where the check looks, not what it accepts. When more than one row matches they must resolve to a single non-empty packet hash, which is only true of receptions of one packet; two unrelated packets that happen to agree on all three fields stay a fallback. A debug line now names the case where the newest row was not the message's packet, so this failure mode is visible in logs rather than silent. Side effect worth knowing: SNR and RSSI now come from the message's own reception too. The reporter's message was logged at SNR 12.0 / RSSI -10, the echo's figures, when its actual reception was 13.25 / -32. Five tests cover it, including a reproduction built from the issue's log. Three fail on the current code; the other two pin behaviour the fix must not break — newest-wins among receptions of one packet, and scope_eligible_only still filtering the search so the scope correlation cannot be handed an ineligible row.
This commit is contained in:
@@ -1540,11 +1540,21 @@ class MessageHandler:
|
||||
|
||||
# A channel message has no packet prefix or pubkey to match on, so everything
|
||||
# above lands on the most-recent-packet fallback. The decoded payload carries
|
||||
# its own copies of fields the RF row also has, though, so the fallback can be
|
||||
# checked rather than assumed.
|
||||
# its own copies of fields the RF row also has, though, so the cache can be
|
||||
# searched for this message's own packet rather than assuming the newest row.
|
||||
if recent_rf_data is not None and not rf_data_is_correlated(recent_rf_data):
|
||||
if self._rf_data_matches_chan_payload(recent_rf_data, payload):
|
||||
recent_rf_data = {**recent_rf_data, RF_MATCH_KEY: RF_MATCH_PAYLOAD}
|
||||
verified = self._find_rf_row_matching_chan_payload(
|
||||
payload, scope_eligible_only=scope_eligible_only
|
||||
)
|
||||
if verified is not None:
|
||||
if verified.get("packet_prefix") != recent_rf_data.get("packet_prefix"):
|
||||
self.logger.debug(
|
||||
"Most recent RF row %s is not this message's packet (a later "
|
||||
"reception of another packet); the payload matches %s instead",
|
||||
(recent_rf_data.get("packet_prefix") or "?")[:16],
|
||||
(verified.get("packet_prefix") or "?")[:16],
|
||||
)
|
||||
recent_rf_data = {**verified, RF_MATCH_KEY: RF_MATCH_PAYLOAD}
|
||||
self.logger.debug(
|
||||
"Verified RF row %s against the channel payload (GRP_TXT, path_len=%s, "
|
||||
"SNR=%s); treating it as this message's packet",
|
||||
@@ -1555,6 +1565,48 @@ class MessageHandler:
|
||||
|
||||
return recent_rf_data
|
||||
|
||||
def _find_rf_row_matching_chan_payload(
|
||||
self, payload: dict[str, Any] | None, *, scope_eligible_only: bool = False
|
||||
) -> dict[str, Any] | None:
|
||||
"""Return the cached RF row this channel message was received on, or None.
|
||||
|
||||
Checking only the newest row assumes the RF log row and the decoded CHAN
|
||||
event for one reception arrive back to back with nothing in between. On a
|
||||
dense mesh they do not: a repeater's echo of the very same packet is
|
||||
routinely logged in the gap, so the newest row is that echo — a different
|
||||
path length and a different measured SNR — and the message loses its route
|
||||
even though its own row is sitting in the cache (#255). Search the cache
|
||||
instead, and let _rf_data_matches_chan_payload decide which row is ours.
|
||||
|
||||
Two rows can only both match when they are receptions of the same packet
|
||||
(same hash) that also agree on path length and SNR; anything else is
|
||||
ambiguous and keeps the fallback tag, because a guess is what #80 cost.
|
||||
"""
|
||||
if not payload:
|
||||
return None
|
||||
|
||||
matches = [
|
||||
row
|
||||
for row in self.recent_rf_data
|
||||
if self._rf_data_matches_chan_payload(row, payload)
|
||||
and (not scope_eligible_only or self._is_rf_data_scope_eligible(row))
|
||||
]
|
||||
if not matches:
|
||||
return None
|
||||
|
||||
if len(matches) > 1:
|
||||
hashes = {row.get("packet_hash") for row in matches}
|
||||
if len(hashes) != 1 or not all(hashes):
|
||||
self.logger.debug(
|
||||
"%d cached RF rows agree with the channel payload across %d packet(s); "
|
||||
"leaving the route unresolved rather than guessing",
|
||||
len(matches),
|
||||
len(hashes),
|
||||
)
|
||||
return None
|
||||
|
||||
return max(matches, key=lambda row: row["timestamp"])
|
||||
|
||||
def _rf_data_matches_chan_payload(
|
||||
self, rf_data: dict[str, Any] | None, payload: dict[str, Any] | None
|
||||
) -> bool:
|
||||
@@ -1573,6 +1625,10 @@ class MessageHandler:
|
||||
a checked hypothesis. SNR is the discriminating one: it is the measured
|
||||
value of a single reception, quantised to 0.25 dB, so an unrelated packet
|
||||
matching all three is improbable rather than merely unlikely.
|
||||
|
||||
_find_rf_row_matching_chan_payload applies this to every cached row rather
|
||||
than only the newest, so a repeater echo logged in between cannot displace
|
||||
the message's own row (#255).
|
||||
"""
|
||||
if not rf_data or not payload:
|
||||
return False
|
||||
|
||||
Reference in New Issue
Block a user