From d99d9c6e3a0199b0d7a5a0d541f8f81d384758cb Mon Sep 17 00:00:00 2001 From: raphaelcoeffic <1050031+raphaelcoeffic@users.noreply.github.com> Date: Sun, 7 Jun 2026 15:14:09 +0200 Subject: [PATCH] fix(cpn): initialise all serialised fields in FrSkyData::clear() FrSkyData::clear() resets its fields one by one rather than with a memset, and it omitted varioCenterSilent and ignoreSensorIds. Both are serialised to a model's YAML, so a freshly-cleared model emitted whatever happened to be in those bytes. For varioCenterSilent (a bool written as int, read back through a 1-bit radio field) this showed up as a non-idempotent round-trip: a stray byte like 45 was emitted, then masked to 1 on reload. Initialise both fields in clear(). Add a regression test that poisons the storage before clearing, so it fails deterministically if a serialised field is ever left out again. Found via the model YAML round-trip test added in #7438; the regression test here builds/runs once that PR reactivates the companion gtests. Co-Authored-By: Claude Opus 4.8 (1M context) --- companion/src/firmwares/telem_data.cpp | 2 + companion/src/tests/frsky_clear_test.cpp | 47 ++++++++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 companion/src/tests/frsky_clear_test.cpp diff --git a/companion/src/firmwares/telem_data.cpp b/companion/src/firmwares/telem_data.cpp index 15515e2a104..d474d48bc3d 100644 --- a/companion/src/firmwares/telem_data.cpp +++ b/companion/src/firmwares/telem_data.cpp @@ -61,9 +61,11 @@ void FrSkyData::clear() varioCenterMin = 0; // if increment in 0.2m/s = 3.0m/s max varioCenterMax = 0; varioMax = 0; + varioCenterSilent = false; mAhPersistent = 0; storedMah = 0; fasOffset = 0; + ignoreSensorIds = false; for (int i=0; i<4; i++) screens[i].clear(); varioSource = 0; diff --git a/companion/src/tests/frsky_clear_test.cpp b/companion/src/tests/frsky_clear_test.cpp new file mode 100644 index 00000000000..fb3b1827ab9 --- /dev/null +++ b/companion/src/tests/frsky_clear_test.cpp @@ -0,0 +1,47 @@ +/* + * Copyright (C) EdgeTX + * + * Based on code named + * opentx - https://github.com/opentx/opentx + * th9x - http://code.google.com/p/th9x + * er9x - http://code.google.com/p/er9x + * gruvin9x - http://code.google.com/p/gruvin9x + * + * License GPLv2: http://www.gnu.org/licenses/gpl-2.0.html + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include "gtests.h" + +#include + +#include "firmwares/telem_data.h" + +// FrSkyData::clear() is a hand-written field-by-field reset (not a memset), +// so any field it forgets is left uninitialised. varioCenterSilent and +// ignoreSensorIds were both omitted yet are serialised to YAML, which made a +// freshly-cleared model emit non-deterministic values (e.g. centerSilent +// emitted as a stray byte, then masked to 0/1 on reload -> non-idempotent +// round-trip). +// +// Poison the storage first so the test fails deterministically if clear() +// ever stops initialising one of these fields. +TEST(FrSkyDataClear, InitialisesAllSerialisedFields) +{ + FrSkyData fs; + memset(reinterpret_cast(&fs), 0x2D, sizeof(fs)); // 0x2D == 45 + fs.clear(); + + EXPECT_FALSE(fs.varioCenterSilent) + << "varioCenterSilent left uninitialised by FrSkyData::clear()"; + EXPECT_FALSE(fs.ignoreSensorIds) + << "ignoreSensorIds left uninitialised by FrSkyData::clear()"; +}