Harden observer preferences and portable controls

test/mocks/Arduino.h

@@ -2,6 +2,7 @@
 
 #include <cstdint>
 #include <cmath>
+#include <cstdio>
 #include "Stream.h"
 
 inline uint32_t g_mock_millis = 0;
@@ -15,3 +16,12 @@ inline uint32_t millis() {
 inline void delay(uint32_t ms) {
   g_mock_millis += ms;
 }
+
+inline char* ltoa(long value, char* dest, int base) {
+  if (base == 10) {
+    std::snprintf(dest, 16, "%ld", value);
+  } else {
+    dest[0] = '\0';
+  }
+  return dest;
+}

test/test_cli_command_utils/test_cli_command_utils.cpp

@@ -40,6 +40,31 @@ TEST(CLICommandUtils, HandlesLeadingWhitespaceAndSingleWordCommands) {
   EXPECT_STREQ("powersaving", single_command);
 }
 
+TEST(CLICommandUtils, ParsesStrictDecimalValues) {
+  float value = 0.0f;
+
+  EXPECT_TRUE(mesh::cli::parseDecimalStrict("910.525", value));
+  EXPECT_FLOAT_EQ(910.525f, value);
+  EXPECT_TRUE(mesh::cli::parseDecimalStrict(" -122.25 ", value));
+  EXPECT_FLOAT_EQ(-122.25f, value);
+  EXPECT_TRUE(mesh::cli::parseDecimalStrict("+.5", value));
+  EXPECT_FLOAT_EQ(0.5f, value);
+  EXPECT_TRUE(mesh::cli::parseDecimalStrict("7.", value));
+  EXPECT_FLOAT_EQ(7.0f, value);
+}
+
+TEST(CLICommandUtils, RejectsNonDecimalOrOverflowValues) {
+  float value = 123.0f;
+
+  EXPECT_FALSE(mesh::cli::parseDecimalStrict(nullptr, value));
+  EXPECT_FALSE(mesh::cli::parseDecimalStrict("", value));
+  EXPECT_FALSE(mesh::cli::parseDecimalStrict(".", value));
+  EXPECT_FALSE(mesh::cli::parseDecimalStrict("1.2x", value));
+  EXPECT_FALSE(mesh::cli::parseDecimalStrict("1e3", value));
+  EXPECT_FALSE(mesh::cli::parseDecimalStrict("4294967296", value));
+  EXPECT_FLOAT_EQ(123.0f, value);
+}
+
 int main(int argc, char** argv) {
   ::testing::InitGoogleTest(&argc, argv);
   return RUN_ALL_TESTS();

test/test_mqtt_payload_builder/test_mqtt_payload_builder.cpp

@@ -7,6 +7,7 @@
 #include <vector>
 
 #include "helpers/MQTTPayloadBuilder.h"
+#include "helpers/TxtDataHelpers.h"
 
 namespace {
 
@@ -134,6 +135,19 @@ TEST(MQTTPayloadBuilder, RxPacketIncludesMetricsScaledScoreAndPath) {
   EXPECT_STREQ("012f", parsed_path[1].as<const char*>());
 }
 
+TEST(MQTTPayloadBuilder, FixedFloatFormattingDoesNotRequirePrintfFloat) {
+  char value[20];
+
+  EXPECT_TRUE(StrHelper::ftoaFixed(value, sizeof(value), 915.0f, 6));
+  EXPECT_STREQ("915.000000", value);
+  EXPECT_TRUE(StrHelper::ftoaFixed(value, sizeof(value), 10.26f, 1));
+  EXPECT_STREQ("10.3", value);
+  EXPECT_TRUE(StrHelper::ftoaFixed(value, sizeof(value), -87.25f, 2));
+  EXPECT_STREQ("-87.25", value);
+  EXPECT_FALSE(StrHelper::ftoaFixed(value, sizeof(value), std::nanf(""), 1));
+  EXPECT_STREQ("0", value);
+}
+
 TEST(MQTTPayloadBuilder, TxPacketOmitsReceiveOnlyMetricsAndAbsentPath) {
   JsonDocument scratch;
   char buffer[2048];

test/test_mqtt_prefs_atomic_store/test_mqtt_prefs_atomic_store.cpp

@@ -5,10 +5,12 @@
 #include <string>
 #include <vector>
 
+#include "helpers/CommonPrefsRecovery.h"
 #include "helpers/MQTTPrefsAtomicStore.h"
 #include "helpers/MQTTPrefsRecovery.h"
 
 namespace AtomicStore = MQTTPrefsAtomicStore;
+namespace CommonRecovery = CommonPrefsRecovery;
 namespace Recovery = MQTTPrefsRecovery;
 
 namespace {
@@ -160,7 +162,9 @@ public:
     ++begin_calls;
     _files.erase("/com_prefs.tmp");
     _staging.clear();
+    _finished = false;
     _open = _failure != FailurePoint::Begin;
+    _owns_temp = _open;
     return _open;
   }
 
@@ -178,6 +182,7 @@ public:
     _open = false;
     if (_failure == FailurePoint::Finish) return false;
     _files["/com_prefs.tmp"] = _staging;
+    _finished = true;
     return true;
   }
 
@@ -186,6 +191,7 @@ public:
     if (_failure == FailurePoint::Commit) return false;
     _files["/com_prefs"] = _files["/com_prefs.tmp"];
     _files.erase("/com_prefs.tmp");
+    _finished = false;
     return true;
   }
 
@@ -193,7 +199,18 @@ public:
     ++abort_calls;
     _open = false;
     _staging.clear();
-    _files.erase("/com_prefs.tmp");
+    if (_owns_temp && !_finished) _files.erase("/com_prefs.tmp");
+    _finished = false;
+    _owns_temp = false;
+  }
+
+  void recover() {
+    const CommonRecovery::Action action = CommonRecovery::select(
+        destinationExists(), tempExists(), false);
+    if (action == CommonRecovery::Action::KeepPrimary ||
+        action == CommonRecovery::Action::DiscardTemp) {
+      _files.erase("/com_prefs.tmp");
+    }
   }
 
   void removeNodeSource() { _files.erase("/node_prefs"); }
@@ -215,6 +232,8 @@ public:
 private:
   FailurePoint _failure;
   bool _open = false;
+  bool _finished = false;
+  bool _owns_temp = false;
   std::vector<uint8_t> _staging;
   std::map<std::string, std::vector<uint8_t>> _files;
 };
@@ -497,6 +516,10 @@ TEST(MQTTPrefsAtomicStore, NodePrefsMigrationFailurePreservesSourceAndNeverPrefe
   for (const auto& test_case : cases) {
     InMemoryCommonPrefsStore store(test_case.point);
     EXPECT_EQ(test_case.expected, runCommonPrefsImage(&store));
+    if (test_case.point == FailurePoint::Commit) {
+      EXPECT_TRUE(store.tempExists());  // complete, but uncommitted first image
+    }
+    store.recover();
     EXPECT_EQ(legacy_node, store.nodeSource());
     EXPECT_TRUE(store.nodeSourceIsPreferred());
     EXPECT_FALSE(store.destinationExists());
@@ -617,6 +640,19 @@ TEST(MQTTPrefsAtomicStore, RecoveryNeverOverwritesOpaqueNewerLayout) {
                              Recovery::FileState::Preserve));
 }
 
+TEST(MQTTPrefsAtomicStore, CommonPrefsRecoveryUsesOnlyVerifiedTempWithBackup) {
+  EXPECT_EQ(CommonRecovery::Action::KeepPrimary,
+            CommonRecovery::select(true, true, true));
+  EXPECT_EQ(CommonRecovery::Action::PromoteTemp,
+            CommonRecovery::select(false, true, true));
+  EXPECT_EQ(CommonRecovery::Action::PromoteBackup,
+            CommonRecovery::select(false, false, true));
+  EXPECT_EQ(CommonRecovery::Action::DiscardTemp,
+            CommonRecovery::select(false, true, false));
+  EXPECT_EQ(CommonRecovery::Action::None,
+            CommonRecovery::select(false, false, false));
+}
+
 int main(int argc, char** argv) {
   ::testing::InitGoogleTest(&argc, argv);
   return RUN_ALL_TESTS();