Fix CommRadio in 8.3Khz mode, when frequency does not match channel

Fix a bug where we would copy-assign a PropertyObject, when we
actually wanted to copy the value.

Thanks to Sascha Reißner for tracking the problem down.

src/Instrumentation/commradio.cxx

@@ -134,7 +134,9 @@ void AtisSpeaker::valueChanged(SGPropertyNode * node)
   }
 
   FGSoundManager * smgr = globals->get_subsystem<FGSoundManager>();
-  assert(smgr != NULL);
+  if (!smgr) {
+      return;
+  }
 
   SG_LOG(SG_INSTR, SG_DEBUG,"node->getPath()=" << node->getPath() << " AtisSpeaker voice is " << voice );
   FLITEVoiceSynthesizer * synthesizer = dynamic_cast<FLITEVoiceSynthesizer*>(smgr->getSynthesizer(voice));
@@ -417,8 +419,9 @@ private:
 			if( channelNum != _channelNum ) _channelNum = channelNum;
 
 			if( _frequency != channel ) {
-				_channel = _frequency; //triggers recursion
+                const double channelValue = c / 1000.0;
-			}
+                _channel = channelValue;
+            }
 		} else {
 			_channelSpacing = 8.33;
 

test_suite/unit_tests/Instrumentation/CMakeLists.txt

@@ -6,6 +6,7 @@ set(TESTSUITE_SOURCES
     ${CMAKE_CURRENT_SOURCE_DIR}/test_hold_controller.cxx
     ${CMAKE_CURRENT_SOURCE_DIR}/test_rnav_procedures.cxx
     ${CMAKE_CURRENT_SOURCE_DIR}/test_dme.cxx
+    ${CMAKE_CURRENT_SOURCE_DIR}/test_commRadio.cxx
     PARENT_SCOPE
 )
 
@@ -16,5 +17,6 @@ set(TESTSUITE_HEADERS
     ${CMAKE_CURRENT_SOURCE_DIR}/test_hold_controller.hxx
     ${CMAKE_CURRENT_SOURCE_DIR}/test_rnav_procedures.hxx
     ${CMAKE_CURRENT_SOURCE_DIR}/test_dme.hxx
+    ${CMAKE_CURRENT_SOURCE_DIR}/test_commRadio.hxx
     PARENT_SCOPE
 )

test_suite/unit_tests/Instrumentation/TestSuite.cxx

@@ -17,6 +17,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "test_commRadio.hxx"
 #include "test_dme.hxx"
 #include "test_gps.hxx"
 #include "test_hold_controller.hxx"
@@ -29,3 +30,4 @@ CPPUNIT_TEST_SUITE_NAMED_REGISTRATION(GPSTests, "Unit tests");
 CPPUNIT_TEST_SUITE_NAMED_REGISTRATION(HoldControllerTests, "Unit tests");
 CPPUNIT_TEST_SUITE_NAMED_REGISTRATION(RNAVProcedureTests, "Unit tests");
 CPPUNIT_TEST_SUITE_NAMED_REGISTRATION(DMEReceiverTests, "Unit tests");
+CPPUNIT_TEST_SUITE_NAMED_REGISTRATION(CommRadioTests, "Unit tests");

test_suite/unit_tests/Instrumentation/test_commRadio.cxx

@@ -0,0 +1,118 @@
+#include "test_commRadio.hxx"
+
+#include <cstring>
+#include <memory>
+
+#include "test_suite/FGTestApi/NavDataCache.hxx"
+#include "test_suite/FGTestApi/testGlobals.hxx"
+
+#include <Airports/airport.hxx>
+#include <Navaids/NavDataCache.hxx>
+
+#include <Instrumentation/commradio.hxx>
+#include <Main/fg_props.hxx>
+#include <Main/locale.hxx>
+
+// Set up function for each test.
+void CommRadioTests::setUp()
+{
+    FGTestApi::setUp::initTestGlobals("commradio");
+    FGTestApi::setUp::initNavDataCache();
+
+    // otherwise ATCSPeech will call locale functions and assert
+    globals->get_locale()->selectLanguage({});
+}
+
+
+// Clean up after each test.
+void CommRadioTests::tearDown()
+{
+    FGTestApi::tearDown::shutdownTestGlobals();
+}
+
+
+// std::string NavRadioTests::formatFrequency(double f)
+// {
+//     char buf[16];
+//     ::snprintf(buf, 16, "%3.2f", f);
+//     return buf;
+// }
+
+void CommRadioTests::testBasic()
+{
+}
+
+void CommRadioTests::testEightPointThree()
+{
+    SGPropertyNode_ptr configNode(new SGPropertyNode);
+    configNode->setStringValue("name", "commtest");
+    configNode->setIntValue("number", 2);
+    configNode->setBoolValue("eight-point-three", true);
+    auto r = Instrumentation::CommRadio::createInstance(configNode);
+
+
+    fgSetBool("/sim/atis/enabled", false);
+
+    r->bind();
+    r->init();
+
+    globals->add_subsystem("comm-radio", r);
+
+    FGAirportRef apt = FGAirport::getByIdent("EGKK");
+    FGTestApi::setPositionAndStabilise(apt->geod());
+
+    SGPropertyNode_ptr n = globals->get_props()->getNode("instrumentation/commtest[2]");
+
+    // EGKK ATIS
+    n->setDoubleValue("frequencies/selected-mhz", 136.525);
+
+    r->update(1.0);
+
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(25, n->getDoubleValue("frequencies/selected-channel-width-khz"), 1e-3);
+    CPPUNIT_ASSERT_EQUAL("136.525"s, string{n->getStringValue("frequencies/selected-mhz-fmt")});
+
+    // random 8.3Khz station
+
+    n->setDoubleValue("frequencies/selected-mhz", 120.11);
+    r->update(1.0);
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(8.33, n->getDoubleValue("frequencies/selected-channel-width-khz"), 1e-3);
+    CPPUNIT_ASSERT_EQUAL("120.110"s, string{n->getStringValue("frequencies/selected-mhz-fmt")});
+    CPPUNIT_ASSERT_EQUAL(338, n->getIntValue("frequencies/selected-channel"));
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(120.10833, n->getDoubleValue("frequencies/selected-real-frequency-mhz"), 1e-6);
+
+    // select station by channel, on 8.3khz boundary
+    n->setIntValue("frequencies/selected-channel", 2561);
+    r->update(1.0);
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(8.33, n->getDoubleValue("frequencies/selected-channel-width-khz"), 1e-3);
+    CPPUNIT_ASSERT_EQUAL("134.005"s, string{n->getStringValue("frequencies/selected-mhz-fmt")});
+    CPPUNIT_ASSERT_EQUAL(2561, n->getIntValue("frequencies/selected-channel"));
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(134.000, n->getDoubleValue("frequencies/selected-real-frequency-mhz"), 1e-6);
+
+    // select station by channel, on 25Khz boundary
+    n->setIntValue("frequencies/selected-channel", 2560);
+    r->update(1.0);
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(25, n->getDoubleValue("frequencies/selected-channel-width-khz"), 1e-3);
+    CPPUNIT_ASSERT_EQUAL("134.000"s, string{n->getStringValue("frequencies/selected-mhz-fmt")});
+    CPPUNIT_ASSERT_EQUAL(2560, n->getIntValue("frequencies/selected-channel"));
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(134.000, n->getDoubleValue("frequencies/selected-real-frequency-mhz"), 1e-6);
+
+    // select by frequency
+    n->setDoubleValue("frequencies/selected-mhz", 120.035);
+    r->update(1.0);
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(8.33, n->getDoubleValue("frequencies/selected-channel-width-khz"), 1e-3);
+    CPPUNIT_ASSERT_EQUAL("120.035"s, string{n->getStringValue("frequencies/selected-mhz-fmt")});
+    CPPUNIT_ASSERT_EQUAL(326, n->getIntValue("frequencies/selected-channel"));
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(120.03333, n->getDoubleValue("frequencies/selected-real-frequency-mhz"), 1e-6);
+
+    n->setDoubleValue("frequencies/selected-mhz", 117.99);
+    r->update(1.0);
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(25.0, n->getDoubleValue("frequencies/selected-channel-width-khz"), 1e-3);
+    CPPUNIT_ASSERT_EQUAL(0, n->getIntValue("frequencies/selected-channel"));
+
+    n->setDoubleValue("frequencies/selected-mhz", 118.705);
+    r->update(1.0);
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(8.33, n->getDoubleValue("frequencies/selected-channel-width-khz"), 1e-3);
+    CPPUNIT_ASSERT_EQUAL("118.705"s, string{n->getStringValue("frequencies/selected-mhz-fmt")});
+    CPPUNIT_ASSERT_EQUAL(113, n->getIntValue("frequencies/selected-channel"));
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(118.700, n->getDoubleValue("frequencies/selected-real-frequency-mhz"), 1e-6);
+}

test_suite/unit_tests/Instrumentation/test_commRadio.hxx

@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2018 Edward d'Auvergne
+ *
+ * This file is part of the program FlightGear.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+
+#pragma once
+
+
+#include <cppunit/TestFixture.h>
+#include <cppunit/extensions/HelperMacros.h>
+
+class FGNavRadio;
+class SGGeod;
+
+// The flight plan unit tests.
+class CommRadioTests : public CppUnit::TestFixture
+{
+    // Set up the test suite.
+    CPPUNIT_TEST_SUITE(CommRadioTests);
+
+    CPPUNIT_TEST(testBasic);
+    CPPUNIT_TEST(testEightPointThree);
+
+    CPPUNIT_TEST_SUITE_END();
+
+public:
+    // Set up function for each test.
+    void setUp();
+
+    // Clean up after each test.
+    void tearDown();
+
+    //  std::string formatFrequency(double f);
+
+    // The tests.
+    void testBasic();
+    void testEightPointThree();
+};