From 5ac5f3ec3fd5942ffdd03da298d88e49555c0749 Mon Sep 17 00:00:00 2001 From: l5y <220195275+l5yth@users.noreply.github.com> Date: Sat, 4 Oct 2025 21:11:16 +0200 Subject: [PATCH] Update node last seen when events are received (#212) * Update node last seen timestamps from event receive times * run rufo * fix tests --- web/app.rb | 70 ++++++++++++++++++++++------------- web/spec/app_spec.rb | 87 +++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 126 insertions(+), 31 deletions(-) diff --git a/web/app.rb b/web/app.rb index 87e73f1..62c419c 100644 --- a/web/app.rb +++ b/web/app.rb @@ -731,6 +731,46 @@ def ensure_unknown_node(db, node_ref, fallback_num = nil, heard_time: nil) inserted end +# Ensure the node's last_seen timestamp reflects the provided receive time. +# +# @param db [SQLite3::Database] open database handle. +# @param node_ref [Object] raw identifier used to resolve the node. +# @param fallback_num [Object] optional numeric identifier. +# @param rx_time [Object] receive timestamp that should update the node. +def touch_node_last_seen(db, node_ref, fallback_num = nil, rx_time: nil) + timestamp = coerce_integer(rx_time) + return unless timestamp + + node_id = nil + + parts = canonical_node_parts(node_ref, fallback_num) + node_id, = parts if parts + + unless node_id + trimmed = string_or_nil(node_ref) + if trimmed + node_id = normalize_node_id(db, trimmed) || trimmed + elsif fallback_num + fallback_parts = canonical_node_parts(fallback_num, nil) + node_id, = fallback_parts if fallback_parts + end + end + + return unless node_id + + with_busy_retry do + db.execute <<~SQL, [timestamp, timestamp, timestamp, node_id] + UPDATE nodes + SET last_heard = CASE + WHEN COALESCE(last_heard, 0) >= ? THEN last_heard + ELSE ? + END, + first_heard = COALESCE(first_heard, ?) + WHERE node_id = ? + SQL + end +end + # Insert or update a node row with the most recent metrics. # # @param db [SQLite3::Database] open database handle. @@ -974,6 +1014,7 @@ def insert_position(db, payload) node_id = canonical if canonical ensure_unknown_node(db, node_id || node_num, node_num, heard_time: rx_time) + touch_node_last_seen(db, node_id || node_num, node_num, rx_time: rx_time) to_id = string_or_nil(payload["to_id"] || payload["to"]) @@ -1123,7 +1164,7 @@ def insert_position(db, payload) ) end -def update_node_from_telemetry(db, node_id, node_num, _rx_time, metrics = {}) +def update_node_from_telemetry(db, node_id, node_num, rx_time, metrics = {}) num = coerce_integer(node_num) id = string_or_nil(node_id) if id&.start_with?("!") @@ -1132,9 +1173,8 @@ def update_node_from_telemetry(db, node_id, node_num, _rx_time, metrics = {}) id ||= format("!%08x", num & 0xFFFFFFFF) if num return unless id - now = Time.now.to_i - - ensure_unknown_node(db, id, num, heard_time: now) + ensure_unknown_node(db, id, num, heard_time: rx_time) + touch_node_last_seen(db, id, num, rx_time: rx_time) battery = coerce_float(metrics[:battery_level] || metrics["battery_level"]) voltage = coerce_float(metrics[:voltage] || metrics["voltage"]) @@ -1150,12 +1190,6 @@ def update_node_from_telemetry(db, node_id, node_num, _rx_time, metrics = {}) params << num end - assignments << "last_heard = ?" - params << now - - assignments << "first_heard = COALESCE(first_heard, ?)" - params << now - metric_updates = { "battery_level" => battery, "voltage" => voltage, @@ -1391,9 +1425,7 @@ def insert_message(db, m) encrypted = string_or_nil(m["encrypted"]) ensure_unknown_node(db, from_id || raw_from_id, m["from_num"], heard_time: rx_time) - - update_sender_last_heard = - encrypted && !encrypted.strip.empty? && from_id && rx_time + touch_node_last_seen(db, from_id || raw_from_id || m["from_num"], m["from_num"], rx_time: rx_time) row = [ msg_id, @@ -1464,18 +1496,6 @@ def insert_message(db, m) end end end - - if update_sender_last_heard - with_busy_retry do - db.execute <<~SQL, [rx_time, rx_time, from_id, rx_time] - UPDATE nodes - SET last_heard = ?, - first_heard = COALESCE(first_heard, ?) - WHERE node_id = ? - AND COALESCE(last_heard, 0) <= ? - SQL - end - end end # Resolve a node reference to the canonical node ID when possible. diff --git a/web/spec/app_spec.rb b/web/spec/app_spec.rb index 99a63b7..ac3b4de 100644 --- a/web/spec/app_spec.rb +++ b/web/spec/app_spec.rb @@ -897,14 +897,14 @@ RSpec.describe "Potato Mesh Sinatra app" do expect_same_value(metrics_node["channel_utilization"], payload[0]["device_metrics"]["channelUtilization"]) expect_same_value(metrics_node["air_util_tx"], payload[0]["device_metrics"]["airUtilTx"]) expect(metrics_node["uptime_seconds"]).to eq(payload[0]["device_metrics"]["uptimeSeconds"]) - expect(metrics_node["last_heard"]).to eq(reference_time.to_i) - expect(metrics_node["first_heard"]).to eq(reference_time.to_i) + expect(metrics_node["last_heard"]).to eq(payload[0]["rx_time"]) + expect(metrics_node["first_heard"]).to eq(payload[0]["rx_time"]) env_node = db.get_first_row( "SELECT last_heard, battery_level, voltage FROM nodes WHERE node_id = ?", [payload[1]["node_id"]], ) - expect(env_node["last_heard"]).to eq(reference_time.to_i) + expect(env_node["last_heard"]).to eq(payload[1]["rx_time"]) expect(env_node["battery_level"]).to be_nil expect(env_node["voltage"]).to be_nil @@ -914,7 +914,7 @@ RSpec.describe "Potato Mesh Sinatra app" do ) expect_same_value(local_node["battery_level"], payload[2]["device_metrics"]["battery_level"]) expect(local_node["uptime_seconds"]).to eq(payload[2]["device_metrics"]["uptime_seconds"]) - expect(local_node["last_heard"]).to eq(reference_time.to_i) + expect(local_node["last_heard"]).to eq(payload[2]["rx_time"]) end end @@ -1107,7 +1107,7 @@ RSpec.describe "Potato Mesh Sinatra app" do [sender_id], ) - expect(node_row["last_heard"]).to eq(reference_time.to_i) + expect(node_row["last_heard"]).to eq(payload["rx_time"]) end get "/api/messages" @@ -1118,6 +1118,44 @@ RSpec.describe "Potato Mesh Sinatra app" do expect(messages).to be_empty end + it "updates node last_heard for plaintext messages" do + node_id = "!plainmsg01" + initial_first = reference_time.to_i - 600 + initial_last = reference_time.to_i - 300 + + with_db do |db| + db.execute( + "INSERT INTO nodes(node_id, last_heard, first_heard) VALUES (?,?,?)", + [node_id, initial_last, initial_first], + ) + end + + rx_time = reference_time.to_i - 120 + payload = { + "packet_id" => 888_001, + "rx_time" => rx_time, + "rx_iso" => Time.at(rx_time).utc.iso8601, + "from_id" => node_id, + "text" => "plaintext update", + } + + post "/api/messages", payload.to_json, auth_headers + + expect(last_response).to be_ok + expect(JSON.parse(last_response.body)).to eq("status" => "ok") + + with_db(readonly: true) do |db| + db.results_as_hash = true + row = db.get_first_row( + "SELECT last_heard, first_heard FROM nodes WHERE node_id = ?", + [node_id], + ) + + expect(row["last_heard"]).to eq(rx_time) + expect(row["first_heard"]).to eq(initial_first) + end + end + it "stores messages containing SQL control characters without executing them" do payload = { "packet_id" => 404, @@ -1315,6 +1353,38 @@ RSpec.describe "Potato Mesh Sinatra app" do node_aliases[num.to_s] ||= canonical end + latest_rx_by_node = {} + messages_fixture.each do |message| + rx_time = message["rx_time"] + next unless rx_time + + canonical = nil + from_id = message["from_id"] + + if from_id.is_a?(String) + trimmed = from_id.strip + unless trimmed.empty? + if trimmed.match?(/\A[0-9]+\z/) + canonical = node_aliases[trimmed] || trimmed + else + canonical = trimmed + end + end + end + + canonical ||= message.dig("node", "node_id") + + if canonical.nil? + num = message.dig("node", "num") + canonical = node_aliases[num.to_s] if num + end + + next unless canonical + + existing = latest_rx_by_node[canonical] + latest_rx_by_node[canonical] = [existing, rx_time].compact.max + end + messages_fixture.each do |message| expected = message.reject { |key, _| key == "node" } actual_row = actual_by_id.fetch(message["id"]) @@ -1367,7 +1437,12 @@ RSpec.describe "Potato Mesh Sinatra app" do expect_same_value(node_actual["snr"], node_expected["snr"]) expect_same_value(node_actual["battery_level"], node_expected["battery_level"]) expect_same_value(node_actual["voltage"], node_expected["voltage"]) - expect(node_actual["last_heard"]).to eq(node_expected["last_heard"]) + expected_last_heard = node_expected["last_heard"] + latest_rx = latest_rx_by_node[node_expected["node_id"]] + if latest_rx + expected_last_heard = [expected_last_heard, latest_rx].compact.max + end + expect(node_actual["last_heard"]).to eq(expected_last_heard) expect(node_actual["first_heard"]).to eq(node_expected["first_heard"]) expect_same_value(node_actual["latitude"], node_expected["latitude"]) expect_same_value(node_actual["longitude"], node_expected["longitude"])