From 56952f4f55ad6a050cbcf029efefe8c5e0bc8997 Mon Sep 17 00:00:00 2001 From: Jacob Dahl Date: Thu, 21 May 2026 21:46:48 -0600 Subject: [PATCH] fix(loader): reset per-load state so msg_ids don't leak between logs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All parser bookkeeping lives in DataLoadAPBIN member variables (has_fmt / has_fmtu / has_instance / instance_idx / instance_offset / formats / format_units / msg_id2name / msg_name2id / field_name2idx / messages_map / multipliers / units), but the plugin instance is reused for every file load in a session and readDataFromFile never reset any of them. ArduPilot recycles non-static msg_ids per boot, so loading two logs in sequence can put a different message at the same id. Because the FMTU handler guards the '#' check with `if (!has_instance[msg_id])`, a stale `true` from the previous file's message is never cleared. The next log's message at that id gets treated as instanced and a byte from its payload (at the previous file's instance_offset) is read as the instance number, producing dozens of bogus "#N" children — e.g. HEAT exploded into 300+ spurious instances after a log where msg_id 251 was SURF. The accumulated messages_map also caused already-multiplied timestamps from earlier loads to be re-multiplied and re-time-synced, leaving a stray data point on the far right of plots. Clearing the state at the top of readDataFromFile fixes both. --- DataLoadAPBin/dataload_apbin.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/DataLoadAPBin/dataload_apbin.cpp b/DataLoadAPBin/dataload_apbin.cpp index b1fa5eb..c8a7596 100644 --- a/DataLoadAPBin/dataload_apbin.cpp +++ b/DataLoadAPBin/dataload_apbin.cpp @@ -61,6 +61,25 @@ const std::vector& DataLoadAPBIN::compatibleFileExtensions() const bool DataLoadAPBIN::readDataFromFile(FileLoadInfo* info, PlotDataMapRef& plot_data) { + // Reset all per-load state. Without this, member variables persist across + // file loads in the same PlotJuggler session: stale has_instance / instance_offset + // entries from a previous log get applied to the current one whenever ArduPilot + // recycles a msg_id, producing bogus per-instance topics (e.g. HEAT split into + // dozens of "#N" children) and skewed timestamps from re-applied multipliers. + std::fill(std::begin(has_fmt), std::end(has_fmt), false); + std::fill(std::begin(has_fmtu), std::end(has_fmtu), false); + std::fill(std::begin(has_instance), std::end(has_instance), false); + std::fill(std::begin(instance_idx), std::end(instance_idx), -1); + std::fill(std::begin(instance_offset), std::end(instance_offset), 0u); + for (auto& f : formats) { f = {}; } + for (auto& fu : format_units) { fu = {}; } + for (auto& n : msg_id2name) { n.clear(); } + msg_name2id.clear(); + field_name2idx.clear(); + messages_map.clear(); + multipliers.clear(); + units.clear(); + QFile file(info->filename); if (!file.open(QFile::ReadOnly)) {