From 85ad886589e29ec1dfb3f41ab00f97f3adc6d730 Mon Sep 17 00:00:00 2001 From: rado <6479266+rbrus@users.noreply.github.com> Date: Sat, 17 Oct 2026 09:36:33 +0200 Subject: [PATCH 1/2] Fix unsigned conversion of negative TX power in CommonRadioPrefs --- examples/companion_radio/NodePrefs.h | 4 +- src/helpers/CommonCLI.h | 4 +- src/helpers/CommonRadioPrefs.h | 4 +- .../test_companion_node_prefs.cpp | 57 +++++++++++++++++++ 4 files changed, 63 insertions(+), 6 deletions(-) diff --git a/examples/companion_radio/NodePrefs.h b/examples/companion_radio/NodePrefs.h index 3cceed56668..1cc5b501826 100644 --- a/examples/companion_radio/NodePrefs.h +++ b/examples/companion_radio/NodePrefs.h @@ -102,8 +102,8 @@ class NodePrefs : public ConfigSerializer { // persisted to file void setIntThresh(uint8_t t) override { _parent->interference_threshold = t; markDirty(); } uint8_t getRxGain() const override { return _parent->rx_boosted_gain; } void setRxGain(uint8_t g) override { _parent->rx_boosted_gain = g; markDirty(); } - uint8_t getTxPower() const override { return _parent->tx_power_dbm; } - void setTxPower(uint8_t dbm) override { _parent->tx_power_dbm = dbm; markDirty(); } + int8_t getTxPower() const override { return _parent->tx_power_dbm; } + void setTxPower(int8_t dbm) override { _parent->tx_power_dbm = dbm; markDirty(); } float getRxDelay() const override { return _parent->rx_delay_base; } void setRxDelay(float d) override { _parent->rx_delay_base = d; markDirty(); } uint16_t getAgcResetInt() const override { return _parent->agc_reset_interval * 4; } diff --git a/src/helpers/CommonCLI.h b/src/helpers/CommonCLI.h index 2759b0c4f0e..675df7a4be7 100644 --- a/src/helpers/CommonCLI.h +++ b/src/helpers/CommonCLI.h @@ -115,8 +115,8 @@ class NodePrefs : public ConfigSerializer { void setIntThresh(uint8_t t) override { _parent->interference_threshold = t; markDirty(); } uint8_t getRxGain() const override { return _parent->rx_boosted_gain; } void setRxGain(uint8_t g) override { _parent->rx_boosted_gain = g; markDirty(); } - uint8_t getTxPower() const override { return _parent->tx_power_dbm; } - void setTxPower(uint8_t dbm) override { _parent->tx_power_dbm = dbm; markDirty(); } + int8_t getTxPower() const override { return _parent->tx_power_dbm; } + void setTxPower(int8_t dbm) override { _parent->tx_power_dbm = dbm; markDirty(); } float getRxDelay() const override { return _parent->rx_delay_base; } void setRxDelay(float d) override { _parent->rx_delay_base = d; markDirty(); } uint16_t getAgcResetInt() const override { return _parent->agc_reset_interval * 4; } diff --git a/src/helpers/CommonRadioPrefs.h b/src/helpers/CommonRadioPrefs.h index 0e83ca9dfdb..d0083b8c73b 100644 --- a/src/helpers/CommonRadioPrefs.h +++ b/src/helpers/CommonRadioPrefs.h @@ -35,8 +35,8 @@ class CommonRadioPrefs : public ConfigSerializer, public KeyValueStore { virtual uint8_t getRxGain() const = 0; virtual void setRxGain(uint8_t g) = 0; - virtual uint8_t getTxPower() const = 0; - virtual void setTxPower(uint8_t dbm) = 0; + virtual int8_t getTxPower() const = 0; + virtual void setTxPower(int8_t dbm) = 0; virtual float getRxDelay() const = 0; virtual void setRxDelay(float d) = 0; diff --git a/test/test_companion_node_prefs/test_companion_node_prefs.cpp b/test/test_companion_node_prefs/test_companion_node_prefs.cpp index 433c9710601..ab5e0f90846 100644 --- a/test/test_companion_node_prefs/test_companion_node_prefs.cpp +++ b/test/test_companion_node_prefs/test_companion_node_prefs.cpp @@ -6,6 +6,32 @@ #include "../../examples/companion_radio/NodePrefs.h" +bool CommonRadioPrefs::getByKey(const char* key, char* value, size_t max_len) { + if (strcmp(key, "fem_rxgain") == 0) { + snprintf(value, max_len, "%d", (uint32_t)getFEMRxGain()); + return true; + } + if (strcmp(key, "fem_txgain") == 0) { + snprintf(value, max_len, "%d", (uint32_t)getFEMTxGain()); + return true; + } + return false; +} + +bool CommonRadioPrefs::setByKey(const char* key, const char* value) { + if (strcmp(key, "fem_rxgain") == 0) { + setFEMRxGain(atoi(value)); + markDirty(); + return true; + } + if (strcmp(key, "fem_txgain") == 0) { + setFEMTxGain(atoi(value)); + markDirty(); + return true; + } + return false; +} + class ReplayStream : public Stream { const char* _text; int _pos = 0; @@ -77,6 +103,37 @@ TEST(CompanionNodePrefs, RxGainSettingsRoundTripIndependently) { } #endif +TEST(CompanionNodePrefs, NegativeTxPowerPreservedAsSigned) { + NodePrefs prefs; + CommonRadioPrefs* radio = prefs.getRadioPrefs(); + + radio->setTxPower(-9); + EXPECT_EQ(-9, prefs.tx_power_dbm); + EXPECT_EQ(-9, radio->getTxPower()); + EXPECT_EQ(-9, static_cast(radio->getTxPower())); + + char reply[32]; + snprintf(reply, sizeof(reply), "> %d", static_cast(radio->getTxPower())); + EXPECT_STREQ("> -9", reply); +} + +TEST(CompanionNodePrefs, NegativeTxPowerRoundTripsSerial) { + NodePrefs saved; + saved.getRadioPrefs()->setTxPower(-9); + + CaptureStream output; + ASSERT_TRUE(saved.saveSerial(output)); + EXPECT_NE(std::string::npos, output.text().find("tx:-9")); + + ReplayStream input("{radio:{tx:-9}}"); + NodePrefs loaded; + loaded.tx_power_dbm = 22; + + ASSERT_TRUE(loaded.loadSerial(input)); + EXPECT_EQ(-9, loaded.tx_power_dbm); + EXPECT_EQ(-9, loaded.getRadioPrefs()->getTxPower()); +} + int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); From 3e4f71d9a22ce990128f1dc49139adccdc746de2 Mon Sep 17 00:00:00 2001 From: rado <6479266+rbrus@users.noreply.github.com> Date: Tue, 27 Oct 2026 07:21:24 +0100 Subject: [PATCH 2/2] Drop unit tests per review feedback --- .../test_companion_node_prefs.cpp | 57 ------------------- 1 file changed, 57 deletions(-) diff --git a/test/test_companion_node_prefs/test_companion_node_prefs.cpp b/test/test_companion_node_prefs/test_companion_node_prefs.cpp index ab5e0f90846..433c9710601 100644 --- a/test/test_companion_node_prefs/test_companion_node_prefs.cpp +++ b/test/test_companion_node_prefs/test_companion_node_prefs.cpp @@ -6,32 +6,6 @@ #include "../../examples/companion_radio/NodePrefs.h" -bool CommonRadioPrefs::getByKey(const char* key, char* value, size_t max_len) { - if (strcmp(key, "fem_rxgain") == 0) { - snprintf(value, max_len, "%d", (uint32_t)getFEMRxGain()); - return true; - } - if (strcmp(key, "fem_txgain") == 0) { - snprintf(value, max_len, "%d", (uint32_t)getFEMTxGain()); - return true; - } - return false; -} - -bool CommonRadioPrefs::setByKey(const char* key, const char* value) { - if (strcmp(key, "fem_rxgain") == 0) { - setFEMRxGain(atoi(value)); - markDirty(); - return true; - } - if (strcmp(key, "fem_txgain") == 0) { - setFEMTxGain(atoi(value)); - markDirty(); - return true; - } - return false; -} - class ReplayStream : public Stream { const char* _text; int _pos = 0; @@ -103,37 +77,6 @@ TEST(CompanionNodePrefs, RxGainSettingsRoundTripIndependently) { } #endif -TEST(CompanionNodePrefs, NegativeTxPowerPreservedAsSigned) { - NodePrefs prefs; - CommonRadioPrefs* radio = prefs.getRadioPrefs(); - - radio->setTxPower(-9); - EXPECT_EQ(-9, prefs.tx_power_dbm); - EXPECT_EQ(-9, radio->getTxPower()); - EXPECT_EQ(-9, static_cast(radio->getTxPower())); - - char reply[32]; - snprintf(reply, sizeof(reply), "> %d", static_cast(radio->getTxPower())); - EXPECT_STREQ("> -9", reply); -} - -TEST(CompanionNodePrefs, NegativeTxPowerRoundTripsSerial) { - NodePrefs saved; - saved.getRadioPrefs()->setTxPower(-9); - - CaptureStream output; - ASSERT_TRUE(saved.saveSerial(output)); - EXPECT_NE(std::string::npos, output.text().find("tx:-9")); - - ReplayStream input("{radio:{tx:-9}}"); - NodePrefs loaded; - loaded.tx_power_dbm = 22; - - ASSERT_TRUE(loaded.loadSerial(input)); - EXPECT_EQ(-9, loaded.tx_power_dbm); - EXPECT_EQ(-9, loaded.getRadioPrefs()->getTxPower()); -} - int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS();