From ebda44c677dacaca40e7ba4491e35e2de6107da2 Mon Sep 17 00:00:00 2001 From: Jonathan Bennett Date: Tue, 3 Nov 2026 10:58:19 -0600 Subject: [PATCH] NodeDB: hold environment satellites protobuf-encoded Each environment satellite held a full meshtastic_EnvironmentMetrics, 304 B for 38 optional fields, of which a real sensor sets a handful. At the nRF52 cap of 40 that is ~12 KB of heap holding about 1 KB of data. Store the encoded bytes instead (~15-40 B per entry) and decode on read: copyNodeEnvironment() keeps its signature. The save writes the stored bytes straight out as the entry's environment_metrics, so nodes.proto is unchanged; the load packs each decoded entry. hasNodeEnvironment() becomes a key lookup, and MemAudit counts the real blob sizes. Co-Authored-By: Claude Opus 5.5 --- src/mesh/NodeDB.cpp | 75 +++++++++++++---- src/mesh/NodeDB.h | 17 ++-- test/test_nodedb_v25_roundtrip/test_main.cpp | 84 ++++++++++++++++++++ 3 files changed, 156 insertions(+), 20 deletions(-) diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index e93c39fb73..245c085025 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -253,7 +253,7 @@ std::map *s_decodePositionsTarget = nullptr; std::map *s_decodeTelemetryTarget = nullptr; #endif #if !MESHTASTIC_EXCLUDE_ENVIRONMENTDB -std::map *s_decodeEnvironmentTarget = nullptr; +std::map *s_decodeEnvironmentTarget = nullptr; #endif #if !MESHTASTIC_EXCLUDE_STATUSDB std::map *s_decodeStatusTarget = nullptr; @@ -268,7 +268,7 @@ const std::map *s_encodePositionsSource = null const std::map *s_encodeTelemetrySource = nullptr; #endif #if !MESHTASTIC_EXCLUDE_ENVIRONMENTDB -const std::map *s_encodeEnvironmentSource = nullptr; +const std::map *s_encodeEnvironmentSource = nullptr; #endif #if !MESHTASTIC_EXCLUDE_STATUSDB const std::map *s_encodeStatusSource = nullptr; @@ -293,6 +293,18 @@ template } return true; } + +#if !MESHTASTIC_EXCLUDE_ENVIRONMENTDB +// One NodeEnvironmentEntry body: num, then the stored environment_metrics bytes as-is (what pb_encode would write) +bool encodePackedEnvironmentEntryBody(pb_ostream_t *ostream, NodeNum num, const PackedEnvironmentMetrics &packed) +{ + if (num && + (!pb_encode_tag(ostream, PB_WT_VARINT, meshtastic_NodeEnvironmentEntry_num_tag) || !pb_encode_varint(ostream, num))) + return false; + return pb_encode_tag(ostream, PB_WT_STRING, meshtastic_NodeEnvironmentEntry_environment_metrics_tag) && + pb_encode_string(ostream, packed.bytes.data(), packed.bytes.size()); +} +#endif } // namespace bool meshtastic_NodeDatabase_callback(pb_istream_t *istream, pb_ostream_t *ostream, const pb_field_t *field) @@ -442,14 +454,16 @@ bool meshtastic_NodeDatabase_callback(pb_istream_t *istream, pb_ostream_t *ostre case meshtastic_NodeDatabase_environment_tag: { if (ostream) { #if !MESHTASTIC_EXCLUDE_ENVIRONMENTDB - if (s_encodeEnvironmentSource) - return encodeSatelliteMap( - ostream, iter, meshtastic_NodeEnvironmentEntry_fields, *s_encodeEnvironmentSource, - [](meshtastic_NodeEnvironmentEntry &e, NodeNum num, const meshtastic_EnvironmentMetrics &v) { - e.num = num; - e.has_environment_metrics = true; - e.environment_metrics = v; - }); + if (s_encodeEnvironmentSource) { + for (const auto &kv : *s_encodeEnvironmentSource) { + pb_ostream_t sizing = PB_OSTREAM_SIZING; + if (!encodePackedEnvironmentEntryBody(&sizing, kv.first, kv.second) || + !pb_encode_tag_for_field(ostream, iter) || !pb_encode_varint(ostream, sizing.bytes_written) || + !encodePackedEnvironmentEntryBody(ostream, kv.first, kv.second)) + return false; + } + return true; + } #endif const auto *vec = static_cast *>(iter->pData); for (const auto &item : *vec) { @@ -465,7 +479,7 @@ bool meshtastic_NodeDatabase_callback(pb_istream_t *istream, pb_ostream_t *ostre #if !MESHTASTIC_EXCLUDE_ENVIRONMENTDB if (s_decodeEnvironmentTarget) { if (entry.has_environment_metrics && isUsableSatelliteKey(entry.num)) - (*s_decodeEnvironmentTarget)[entry.num] = entry.environment_metrics; + (*s_decodeEnvironmentTarget)[entry.num].pack(entry.environment_metrics); return true; } #endif @@ -2076,10 +2090,41 @@ bool NodeDB::copyNodeEnvironment(NodeNum n, meshtastic_EnvironmentMetrics &out) return false; #else concurrency::LockGuard guard(&satelliteMutex); - return copySatelliteEntry(nodeEnvironment, n, out); + auto it = nodeEnvironment.find(n); + return it != nodeEnvironment.end() && it->second.unpack(out); #endif } +bool NodeDB::hasNodeEnvironment(NodeNum n) const +{ +#if MESHTASTIC_EXCLUDE_ENVIRONMENTDB + (void)n; + return false; +#else + concurrency::LockGuard guard(&satelliteMutex); + return nodeEnvironment.count(n) != 0; +#endif +} + +bool PackedEnvironmentMetrics::pack(const meshtastic_EnvironmentMetrics &metrics) +{ + uint8_t buf[meshtastic_EnvironmentMetrics_size]; + pb_ostream_t stream = pb_ostream_from_buffer(buf, sizeof(buf)); + if (!pb_encode(&stream, meshtastic_EnvironmentMetrics_fields, &metrics)) { + bytes.clear(); + return false; + } + std::vector(buf, buf + stream.bytes_written).swap(bytes); // exact capacity, no growth slack + return true; +} + +bool PackedEnvironmentMetrics::unpack(meshtastic_EnvironmentMetrics &out) const +{ + out = meshtastic_EnvironmentMetrics_init_zero; + pb_istream_t stream = pb_istream_from_buffer(bytes.data(), bytes.size()); + return pb_decode(&stream, meshtastic_EnvironmentMetrics_fields, &out); +} + bool NodeDB::copyNodeStatus(NodeNum n, meshtastic_StatusMessage &out) const { #if MESHTASTIC_EXCLUDE_STATUSDB @@ -2244,7 +2289,9 @@ bool NodeDB::enforceSatelliteCaps() satBytes += nodeTelemetry.size() * (sizeof(decltype(nodeTelemetry)::value_type) + 44); #endif #if !MESHTASTIC_EXCLUDE_ENVIRONMENTDB - satBytes += nodeEnvironment.size() * (sizeof(decltype(nodeEnvironment)::value_type) + 44); + // plus each entry's own encoded bytes, a separate allocation (~8 B allocator header) + for (const auto &kv : nodeEnvironment) + satBytes += sizeof(decltype(nodeEnvironment)::value_type) + 44 + kv.second.bytes.capacity() + 8; #endif #if !MESHTASTIC_EXCLUDE_STATUSDB satBytes += nodeStatus.size() * (sizeof(decltype(nodeStatus)::value_type) + 44); @@ -3838,7 +3885,7 @@ void NodeDB::updateTelemetry(uint32_t nodeId, const meshtastic_Telemetry &t, RxS #if !MESHTASTIC_EXCLUDE_ENVIRONMENTDB concurrency::LockGuard guard(&satelliteMutex); evictSatelliteOverCap(*this, nodeEnvironment, nodeId); - nodeEnvironment[nodeId] = t.variant.environment_metrics; + nodeEnvironment[nodeId].pack(t.variant.environment_metrics); #endif } else { diff --git a/src/mesh/NodeDB.h b/src/mesh/NodeDB.h index 73af9cf17d..243be6a81c 100644 --- a/src/mesh/NodeDB.h +++ b/src/mesh/NodeDB.h @@ -297,6 +297,15 @@ struct LoraSlotSnapshot { /// Normalises to the modem fields actually in force, so editing a dormant one is not a slot change. LoraSlotSnapshot loraSlotSnapshotFrom(const meshtastic_Config_LoRaConfig &lora, const char *primaryChannelName); +/// An EnvironmentMetrics held protobuf-encoded. A real sensor sets a handful of its 38 optional fields, so this is +/// ~15-40 B where the decoded struct is 304 B. The bytes are exactly what nodes.proto stores for the submessage. +struct PackedEnvironmentMetrics { + std::vector bytes; + + bool pack(const meshtastic_EnvironmentMetrics &metrics); + bool unpack(meshtastic_EnvironmentMetrics &out) const; +}; + class NodeDB { // NodeNum provisionalNodeNum; // if we are trying to find a node num this is our current attempt @@ -322,7 +331,7 @@ class NodeDB std::map nodeTelemetry; #endif #if !MESHTASTIC_EXCLUDE_ENVIRONMENTDB - std::map nodeEnvironment; + std::map nodeEnvironment; #endif #if !MESHTASTIC_EXCLUDE_STATUSDB std::map nodeStatus; @@ -579,11 +588,7 @@ class NodeDB meshtastic_DeviceMetrics scratch; return copyNodeTelemetry(n, scratch); } - bool hasNodeEnvironment(NodeNum n) const - { - meshtastic_EnvironmentMetrics scratch; - return copyNodeEnvironment(n, scratch); - } + bool hasNodeEnvironment(NodeNum n) const; bool hasNodeStatus(NodeNum n) const { meshtastic_StatusMessage scratch; diff --git a/test/test_nodedb_v25_roundtrip/test_main.cpp b/test/test_nodedb_v25_roundtrip/test_main.cpp index 66db1da693..64adc53297 100644 --- a/test/test_nodedb_v25_roundtrip/test_main.cpp +++ b/test/test_nodedb_v25_roundtrip/test_main.cpp @@ -443,6 +443,84 @@ static void test_fullRoundTrip_headerAndSatelliteFidelity(void) #endif } +#if !MESHTASTIC_EXCLUDE_ENVIRONMENTDB +// Environment satellites are held protobuf-encoded rather than as the 304 B nanopb struct: a real sensor +// sets a handful of EnvironmentMetrics' 38 optional fields. At the nRF52 cap of 40 entries the struct +// form cost ~12 KB of heap and ran T114/T096 sensor nodes out of memory. Pins that a BME680-shaped +// reading packs to a few dozen bytes, unpacks exactly, and that unset fields stay unset (has_* false) +// rather than coming back as present zeros. +static void test_packedEnvironment_compactAndLossless(void) +{ + meshtastic_EnvironmentMetrics in = meshtastic_EnvironmentMetrics_init_zero; + in.has_temperature = true; + in.temperature = 9.97f; + in.has_relative_humidity = true; + in.relative_humidity = 31.9f; + in.has_barometric_pressure = true; + in.barometric_pressure = 1019.5f; + in.has_gas_resistance = true; + in.gas_resistance = 598.2f; + in.has_iaq = true; + in.iaq = 75; + + PackedEnvironmentMetrics packed; + TEST_ASSERT_TRUE(packed.pack(in)); + TEST_ASSERT_LESS_OR_EQUAL_UINT32(40, packed.bytes.size()); + TEST_ASSERT_EQUAL_UINT32(packed.bytes.size(), packed.bytes.capacity()); + + meshtastic_EnvironmentMetrics out; + TEST_ASSERT_TRUE(packed.unpack(out)); + TEST_ASSERT_TRUE(out.has_temperature); + TEST_ASSERT_EQUAL_FLOAT(9.97f, out.temperature); + TEST_ASSERT_TRUE(out.has_relative_humidity); + TEST_ASSERT_EQUAL_FLOAT(31.9f, out.relative_humidity); + TEST_ASSERT_TRUE(out.has_barometric_pressure); + TEST_ASSERT_EQUAL_FLOAT(1019.5f, out.barometric_pressure); + TEST_ASSERT_TRUE(out.has_gas_resistance); + TEST_ASSERT_EQUAL_FLOAT(598.2f, out.gas_resistance); + TEST_ASSERT_TRUE(out.has_iaq); + TEST_ASSERT_EQUAL_UINT16(75, out.iaq); + TEST_ASSERT_FALSE(out.has_lux); + TEST_ASSERT_FALSE(out.has_voltage); + TEST_ASSERT_FALSE(out.has_one_wire_temperature_ch7); +} + +// The save writes each environment entry by hand from the stored bytes instead of through +// pb_encode_submessage. Pins that the result is still ordinary protobuf: the plain nanopb decoder +// (decode targets disarmed, as an older firmware or a backup reader would decode it) recovers the +// node number and every set field. A framing slip here would corrupt nodes.proto for every reader. +static void test_packedEnvironment_savedEntryDecodesAsPlainProtobuf(void) +{ + const uint32_t E = 0x54000103; + addUserNode(E, 0x23); + meshtastic_Telemetry env = meshtastic_Telemetry_init_zero; + env.which_variant = meshtastic_Telemetry_environment_metrics_tag; + env.variant.environment_metrics.has_temperature = true; + env.variant.environment_metrics.temperature = -4.25f; + env.variant.environment_metrics.has_iaq = true; + env.variant.environment_metrics.iaq = 312; + nodeDB->updateTelemetry(E, env); + TEST_ASSERT_TRUE(db->saveDatabase()); + + meshtastic_NodeDatabase disk; + decodeNodesFile(disk); + const meshtastic_NodeEnvironmentEntry *found = nullptr; + for (const auto &entry : disk.environment) { + if (entry.num == E) + found = &entry; + } + TEST_ASSERT_NOT_NULL_MESSAGE(found, "environment entry missing from nodes.proto"); + TEST_ASSERT_TRUE(found->has_environment_metrics); + TEST_ASSERT_TRUE(found->environment_metrics.has_temperature); + TEST_ASSERT_EQUAL_FLOAT(-4.25f, found->environment_metrics.temperature); + TEST_ASSERT_TRUE(found->environment_metrics.has_iaq); + TEST_ASSERT_EQUAL_UINT16(312, found->environment_metrics.iaq); + TEST_ASSERT_FALSE(found->environment_metrics.has_relative_humidity); + + nodeDB->eraseNodeSatellites(E); +} +#endif + // --- Keyless-save skip (part of the PKI-DM key-amnesia diagnosis) --- static void test_keylessDevice_skipsNodesProtoWrite(void) @@ -728,6 +806,12 @@ NDBR_TEST_ENTRY void setup() printf("\n=== Satellite projection/rehydration ===\n"); RUN_TEST(test_fullRoundTrip_headerAndSatelliteFidelity); +#if !MESHTASTIC_EXCLUDE_ENVIRONMENTDB + printf("\n=== Packed environment satellites ===\n"); + RUN_TEST(test_packedEnvironment_compactAndLossless); + RUN_TEST(test_packedEnvironment_savedEntryDecodesAsPlainProtobuf); +#endif + printf("\n=== Keyless-save gate ===\n"); RUN_TEST(test_keylessDevice_skipsNodesProtoWrite);