TomandGitHub
2024bb8384
* Add explicit presence for MeshPacket.rx_time (arrival time) rx_time is now proto3 optional with a has_rx_time presence bit, matching the rx_rssi treatment. A node with no GPS and no phone connected yet has no time source at all, so a bare 0 was indistinguishable from a genuine 1970-01-01 reading; downstream consumers (replay packets, JSON serialization) now check has_rx_time instead of the value. * Dedupe rx_time stamping into a shared helper; trim a debug log string Extract the repeated haveTime/rx_time/has_rx_time stamp logic (5 call sites across Router.cpp, MeshBeaconModule.cpp, MeshService.cpp) into Router::computeRxTimeStamp()/stampRxTime(). Also shorten the new RTC.cpp LOG_DEBUG string. Saves 48 bytes of flash on rak4631 (measured), no behavior change. * Fix has_rx_rssi presence carried unconditionally through StoreForward replay preparePayload() set has_rx_rssi = true unconditionally on replay, regardless of whether the packet's rx_rssi at store time was a genuine measurement (e.g. MQTT-relayed packets carry no real RSSI). Store the presence bit alongside rx_rssi in PacketHistoryStruct and restore it on replay instead. Flagged by Copilot on #11271 (same root cause the has_rx_time explicit presence work fixes) but never addressed before that PR merged. * Trim comment blocks to the repo's 1-2 line guideline .github/copilot-instructions.md:338 caps code comments at 1-2 lines; several blocks added across the rx_time explicit-presence work ran well past that. Also consolidates Time.cpp's file-level doc comment into Time.h, where the rest of the Time:: API contract already lives. No behavior change. * Add rx_time explicit-presence test coverage - test_meshpacket_serializer: has_rx_time=false fixture plus tests asserting JsonSerialize/JsonSerializeEncrypted emit 0 rather than leaking the millis() placeholder, alongside the has_rx_time=true baseline. - test_stream_api: two tests driving a real PhoneAPI handshake (want_config_id through STATE_SEND_PACKETS) that simulate a phone time-giving transaction arriving before vs. after a queued packet is drained - covering both the reconciled and the ships-with-placeholder-absent paths of MeshService::reconcilePendingRxTimes(). * Fix three correctness issues flagged in review - Time.h: drop the reserved-identifier include guard (_MT_TIME_H); pragma once already covers it, matching convention elsewhere (e.g. RTC.h). - Time.cpp: rebase getMillis64()'s wrap accumulator when the test seam swaps clock sources, so a real<->injected clock jump isn't miscounted as a genuine 32-bit wrap. - NodeInfoModule: the 12h reply-suppression window is a local dedup duration, not a wall-clock reading - switch it to Time::getMillis64() so RTC-quality jumps and replayed packets' stale rx_time can't perturb it. - StoreForwardModule: has_rx_time was derived from *current* RTC quality at replay time rather than stored at capture time, so a history entry saved while time-blind could be misreported as a valid epoch once the clock later improved. Persist the presence bit in PacketHistoryStruct instead. * tryfix CI * post review fixes * more test fixes
77 lines
3.0 KiB
C++
77 lines
3.0 KiB
C++
#pragma once
|
|
|
|
#include "serialization/MeshPacketSerializer.h"
|
|
#include <Arduino.h>
|
|
#include <json/json.h>
|
|
#include <memory>
|
|
#include <meshtastic/mesh.pb.h>
|
|
#include <meshtastic/mqtt.pb.h>
|
|
#include <meshtastic/telemetry.pb.h>
|
|
#include <pb_decode.h>
|
|
#include <pb_encode.h>
|
|
#include <unity.h>
|
|
|
|
// Parse a JSON string into a Json::Value; returns Json::nullValue on failure.
|
|
static inline Json::Value parse_json(const std::string &s)
|
|
{
|
|
Json::CharReaderBuilder b;
|
|
Json::Value root;
|
|
std::string errs;
|
|
std::unique_ptr<Json::CharReader> reader(b.newCharReader());
|
|
if (!reader->parse(s.c_str(), s.c_str() + s.size(), &root, &errs))
|
|
return Json::Value();
|
|
return root;
|
|
}
|
|
|
|
// Helper function to create a test packet with the given port and payload
|
|
static meshtastic_MeshPacket create_test_packet(meshtastic_PortNum port, const uint8_t *payload, size_t payload_size,
|
|
int payload_variant = meshtastic_MeshPacket_decoded_tag)
|
|
{
|
|
meshtastic_MeshPacket packet = meshtastic_MeshPacket_init_zero;
|
|
|
|
packet.id = 0x9999;
|
|
packet.from = 0x11223344;
|
|
packet.to = 0x55667788;
|
|
packet.channel = 0;
|
|
packet.hop_limit = 3;
|
|
packet.want_ack = false;
|
|
packet.priority = meshtastic_MeshPacket_Priority_UNSET;
|
|
packet.rx_time = 1609459200;
|
|
packet.has_rx_time = true; // rx_time has explicit presence; mark this synthetic reading as measured
|
|
packet.rx_snr = 10.5f;
|
|
packet.hop_start = 3;
|
|
packet.rx_rssi = -85;
|
|
packet.has_rx_rssi = true; // rx_rssi has explicit presence; mark this synthetic reading as measured
|
|
packet.delayed = meshtastic_MeshPacket_Delayed_NO_DELAY;
|
|
|
|
// Set decoded variant
|
|
packet.which_payload_variant = payload_variant;
|
|
packet.decoded.portnum = port;
|
|
if (payload_variant == meshtastic_MeshPacket_encrypted_tag && payload) {
|
|
packet.encrypted.size = payload_size;
|
|
memcpy(packet.encrypted.bytes, payload, packet.encrypted.size);
|
|
}
|
|
if (payload && payload_size)
|
|
memcpy(packet.decoded.payload.bytes, payload, payload_size);
|
|
packet.decoded.payload.size = payload_size;
|
|
packet.decoded.want_response = false;
|
|
packet.decoded.dest = 0x55667788;
|
|
packet.decoded.source = 0x11223344;
|
|
packet.decoded.request_id = 0;
|
|
packet.decoded.reply_id = 0;
|
|
packet.decoded.emoji = 0;
|
|
|
|
return packet;
|
|
}
|
|
|
|
// Same as create_test_packet(), but with rx_time absent (has_rx_time = false) and a nonzero
|
|
// millis()-style placeholder in rx_time, to verify serializers treat it as unknown, not a reading.
|
|
static meshtastic_MeshPacket create_test_packet_no_rx_time(meshtastic_PortNum port, const uint8_t *payload, size_t payload_size,
|
|
int payload_variant = meshtastic_MeshPacket_decoded_tag)
|
|
{
|
|
meshtastic_MeshPacket packet = create_test_packet(port, payload, payload_size, payload_variant);
|
|
packet.rx_time = 123456; // a plausible millis() placeholder, not a real epoch
|
|
packet.has_rx_time = false;
|
|
return packet;
|
|
}
|