CommRadio: fix tuning 25Khz and 75Khz channels

In 8.3 mode, ensure that 25 and 75Khz channels can be tuned using the
abbreviated syntax (eg 118.02 for 118.025). 

As part of this, add test coverage of the CommRadio for basic and 8.3-
mode operation.

src/Instrumentation/commradio.cxx

@@ -131,7 +131,9 @@ void AtisSpeaker::valueChanged(SGPropertyNode * node)
   }
 
   FGSoundManager * smgr = globals->get_subsystem<FGSoundManager>();
-  assert(smgr != NULL);
+  if (!smgr) {
+    return;
+  }
 
   SG_LOG(SG_INSTR, SG_DEBUG,"AtisSpeaker voice is " << voice );
   FLITEVoiceSynthesizer * synthesizer = dynamic_cast<FLITEVoiceSynthesizer*>(smgr->getSynthesizer(voice));
@@ -378,18 +380,32 @@ private:
 	EightPointThreeFrequencyFormatter( const EightPointThreeFrequencyFormatter & );
 	EightPointThreeFrequencyFormatter & operator = ( const EightPointThreeFrequencyFormatter & );
 
-	void valueChanged (SGPropertyNode * prop) {
+    void valueChanged (SGPropertyNode * prop)
-		if( prop == _channel.node() )
+    {
-			setFrequency(prop->getDoubleValue());
+      if( prop == _channel.node() )
-		else if( prop == _channelNum.node() )
+        setFrequency(prop->getDoubleValue());
-			setChannel(prop->getIntValue());
+      else if( prop == _channelNum.node() )
-	}
+        setChannel(prop->getIntValue());
+    }
 
 	void setChannel( int channel ) {
 		channel %= 3040;
 		if( channel < 0 ) channel += 3040;
-		double f = 118.000 + 0.025*(channel/4) + 0.005*(channel%4);
+      
-		if( f != _channel )	_channel = f;
+        double f = 118.000 + 0.025*(channel/4);
+        if (channel % 4) {
+           f += 0.005*(channel%4); // 8.3khz channel
+        } else {
+          // 25Khz channel, but we need to adjust to displayed (not real)
+          // frequency back down for the xxx.x25 and xxx.x75 cases
+          const int lowDigits = channel % 16;
+          const bool is25KhzQuarterChannel = (lowDigits == 4) | (lowDigits == 12);
+          if (is25KhzQuarterChannel) {
+            f -= 0.005;
+          }
+        }
+
+		_channel = f; // recurses to update frequency values
 	}
 
 	void setFrequency( double channel ) {
@@ -405,17 +421,22 @@ private:
 		// sanitize range and round to nearest kHz.
 		unsigned c = static_cast<int>(SGMiscd::round(SGMiscd::clip( channel, 118.0, 136.99 ) * 1000));
 
+        const int lowDigits = c % 100;
+        const bool is25KhzQuarterChannel = (lowDigits == 20) | (lowDigits == 70);
+        if (is25KhzQuarterChannel) {
+          c += 5;
+        }
+      
 		if ( (c % 25)  == 0 ) {
 			// legacy 25kHz channels continue to be just that.
 			_channelSpacing = 25.0;
 			_frequency = c / 1000.0;
 
 			int channelNum = (c-118000)/25*4;
-			if( channelNum != _channelNum ) _channelNum = channelNum;
+          
-
+          if ( channelNum != _channelNum ) {
-			if( _frequency != channel ) {
+            _channelNum = channelNum;
-				_channel = _frequency; //triggers recursion
+          }
-			}
 		} else {
 			_channelSpacing = 8.33;
 
@@ -448,7 +469,6 @@ private:
     PropertyObject<double> _channelSpacing;
     PropertyObject<string> _formattedChannel;
     PropertyObject<int>   _channelNum;
-
 };
 
 

test_suite/unit_tests/Instrumentation/CMakeLists.txt

@@ -2,11 +2,13 @@ set(TESTSUITE_SOURCES
     ${TESTSUITE_SOURCES}
     ${CMAKE_CURRENT_SOURCE_DIR}/TestSuite.cxx
     ${CMAKE_CURRENT_SOURCE_DIR}/test_navRadio.cxx
+    ${CMAKE_CURRENT_SOURCE_DIR}/test_commRadio.cxx
     PARENT_SCOPE
 )
 
 set(TESTSUITE_HEADERS
     ${TESTSUITE_HEADERS}
     ${CMAKE_CURRENT_SOURCE_DIR}/test_navRadio.hxx
+    ${CMAKE_CURRENT_SOURCE_DIR}/test_commRadio.hxx
     PARENT_SCOPE
 )

test_suite/unit_tests/Instrumentation/TestSuite.cxx

@@ -18,6 +18,9 @@
  */
 
 #include "test_navRadio.hxx"
+#include "test_commRadio.hxx"
 
 // Set up the unit tests.
 CPPUNIT_TEST_SUITE_NAMED_REGISTRATION(NavRadioTests, "Unit tests");
+CPPUNIT_TEST_SUITE_NAMED_REGISTRATION(CommRadioTests, "Unit tests");
+

test_suite/unit_tests/Instrumentation/test_commRadio.cxx

@@ -0,0 +1,209 @@
+#include "test_commRadio.hxx"
+
+#include <memory>
+#include <cstring>
+
+#include "test_suite/FGTestApi/globals.hxx"
+#include "test_suite/FGTestApi/NavDataCache.hxx"
+
+#include <Navaids/NavDataCache.hxx>
+#include <Airports/airport.hxx>
+
+#include <Instrumentation/commradio.hxx>
+
+using namespace Instrumentation;
+
+// Set up function for each test.
+void CommRadioTests::setUp()
+{
+    FGTestApi::setUp::initTestGlobals("commradio");
+    FGTestApi::setUp::initNavDataCache();
+}
+
+
+// Clean up after each test.
+void CommRadioTests::tearDown()
+{
+    FGTestApi::tearDown::shutdownTestGlobals();
+}
+
+void CommRadioTests::setPositionAndStabilise(SGSubsystem* r, const SGGeod& g)
+{
+    FGTestApi::setPosition(g);
+    for (int i=0; i<60; ++i) {
+        r->update(0.1);
+    }
+}
+
+void CommRadioTests::testBasic()
+{
+    SGPropertyNode_ptr configNode(new SGPropertyNode);
+    configNode->setStringValue("name", "commtest");
+    configNode->setIntValue("number", 2);
+    
+    auto sub = Instrumentation::CommRadio::createInstance(configNode);
+    std::unique_ptr<SGSubsystem> r{sub};
+
+    r->bind();
+    r->init();
+
+    SGPropertyNode_ptr node = globals->get_props()->getNode("instrumentation/commtest[2]");
+    node->setBoolValue("serviceable", true);
+    // needed for the radio to power up
+    globals->get_props()->setDoubleValue("systems/electrical/outputs/commtest", 6.0);
+
+    // invalid frequency, of course
+    node->setDoubleValue("frequencies/selected-mhz", 140.0);
+    
+    // let's use BIKF / Keflavik for testing
+    auto bikf = FGAirport::getByIdent("BIKF");
+    setPositionAndStabilise(r.get(), bikf->geod());
+
+    // should be powered up
+    CPPUNIT_ASSERT_EQUAL(true, node->getBoolValue("operable"));
+    CPPUNIT_ASSERT_EQUAL(0.0, node->getDoubleValue("signal-quality-norm"));
+    std::string id = node->getStringValue("airport-id");
+    CPPUNIT_ASSERT(id.empty());
+
+    // published frequencies in our apt.dat:
+//    50 12830 ATIS
+//    53 12190 GND
+//    54 11830 TWR
+//    54 13190 AIR GND
+//    55 11930 APP
+//    56 11930 DEP
+    
+    node->setDoubleValue("frequencies/selected-mhz", 128.30);
+    setPositionAndStabilise(r.get(), bikf->geod());
+    
+    id = node->getStringValue("airport-id");
+    CPPUNIT_ASSERT_EQUAL(std::string{"BIKF"}, id);
+    CPPUNIT_ASSERT_EQUAL(1.0, node->getDoubleValue("signal-quality-norm"));
+    
+    std::string type = node->getStringValue("station-type");
+    CPPUNIT_ASSERT_EQUAL(std::string{"atis"}, type);
+
+    
+    // and now let's re-tune
+    
+    node->setDoubleValue("frequencies/selected-mhz", 118.30);
+    setPositionAndStabilise(r.get(), bikf->geod());
+    
+    id = node->getStringValue("airport-id");
+    CPPUNIT_ASSERT_EQUAL(std::string{"BIKF"}, id);
+    CPPUNIT_ASSERT_EQUAL(1.0, node->getDoubleValue("signal-quality-norm"));
+    
+    type = node->getStringValue("station-type");
+    CPPUNIT_ASSERT_EQUAL(std::string{"tower"}, type);
+
+}
+
+void CommRadioTests::testWith8Point3Mode()
+{
+    SGPropertyNode_ptr configNode(new SGPropertyNode);
+    configNode->setStringValue("name", "commtest");
+    configNode->setIntValue("number", 2);
+    configNode->setBoolValue("eight-point-three", true);
+    auto sub = Instrumentation::CommRadio::createInstance(configNode);
+    std::unique_ptr<SGSubsystem> r{sub};
+    
+    r->bind();
+    r->init();
+    
+    SGPropertyNode_ptr node = globals->get_props()->getNode("instrumentation/commtest[2]");
+    node->setBoolValue("serviceable", true);
+    // needed for the radio to power up
+    globals->get_props()->setDoubleValue("systems/electrical/outputs/commtest", 6.0);
+    
+    
+    // EDDF ATIS is a known problem case
+    auto eddf = FGAirport::getByIdent("EDDF");
+    setPositionAndStabilise(r.get(), eddf->geod());
+    
+    // should be powered up
+    CPPUNIT_ASSERT_EQUAL(true, node->getBoolValue("operable"));
+    
+    // published frequencies in our apt.dat:
+//    50 11802 ATIS
+//    50 11872 ATIS 2
+//    52 12190 Delivery
+//    53 12180 Ground
+//    53 12195 Apron East
+//    53 12185 Apron South
+//    53 12170 Apron West
+//    54 11990 Tower
+//    54 12485 Tower
+//    54 12732 Tower
+//    55 11845 Langen Radar
+//    55 12655 Langen Radar
+//    55 13612 Langen Radar
+//    56 12015 Langen Radar
+//    56 12080 Langen Radar
+    
+    node->setDoubleValue("frequencies/selected-mhz", 118.02);
+    setPositionAndStabilise(r.get(), eddf->geod());
+    
+    std::string id = node->getStringValue("airport-id");
+    CPPUNIT_ASSERT_EQUAL(std::string{"EDDF"}, id);
+    CPPUNIT_ASSERT_EQUAL(1.0, node->getDoubleValue("signal-quality-norm"));
+    
+    std::string type = node->getStringValue("station-type");
+    CPPUNIT_ASSERT_EQUAL(std::string{"atis"}, type);
+    
+    CPPUNIT_ASSERT_EQUAL(std::string{"118.020"}, std::string{node->getStringValue("frequencies/selected-mhz-fmt")});
+    CPPUNIT_ASSERT_EQUAL(4, node->getIntValue("frequencies/selected-channel"));
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(118.020, node->getDoubleValue("frequencies/selected-mhz"), 0.001);
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(118.025, node->getDoubleValue("frequencies/selected-real-frequency-mhz"), 0.001);
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(1.0, node->getDoubleValue("signal-quality-norm"), 0.01);
+    
+    
+// EDME / EGGENFELDEN
+    auto edme = FGAirport::getByIdent("EDME");
+    node->setDoubleValue("frequencies/selected-mhz", 125.07);
+    setPositionAndStabilise(r.get(), edme->geod());
+    
+    id = node->getStringValue("airport-id");
+    CPPUNIT_ASSERT_EQUAL(std::string{"EDME"}, id);
+    CPPUNIT_ASSERT_EQUAL(1.0, node->getDoubleValue("signal-quality-norm"));
+    
+    CPPUNIT_ASSERT_EQUAL(std::string{"125.070"}, std::string{node->getStringValue("frequencies/selected-mhz-fmt")});
+    CPPUNIT_ASSERT_EQUAL(1132, node->getIntValue("frequencies/selected-channel"));
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(125.070, node->getDoubleValue("frequencies/selected-mhz"), 0.01);
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(125.075, node->getDoubleValue("frequencies/selected-real-frequency-mhz"), 0.001);
+    
+// let's try EDDF but passing the real 25ks frequency in instead
+    node->setDoubleValue("frequencies/selected-mhz", 118.025);
+    setPositionAndStabilise(r.get(), eddf->geod());
+    
+    id = node->getStringValue("airport-id");
+    CPPUNIT_ASSERT_EQUAL(std::string{"EDDF"}, id);
+    CPPUNIT_ASSERT_EQUAL(1.0, node->getDoubleValue("signal-quality-norm"));
+    
+    type = node->getStringValue("station-type");
+    CPPUNIT_ASSERT_EQUAL(std::string{"atis"}, type);
+    
+    CPPUNIT_ASSERT_EQUAL(std::string{"118.020"}, std::string{node->getStringValue("frequencies/selected-mhz-fmt")});
+    CPPUNIT_ASSERT_EQUAL(4, node->getIntValue("frequencies/selected-channel"));
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(118.020, node->getDoubleValue("frequencies/selected-mhz"), 0.01);
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(118.025, node->getDoubleValue("frequencies/selected-real-frequency-mhz"), 0.01);
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(1.0, node->getDoubleValue("signal-quality-norm"), 0.01);
+    
+// let's try some actual 8.3 spacing channels - note we don't have these in apt.dat right now so
+// they won't tune in
+    node->setDoubleValue("frequencies/selected-mhz", 125.015);
+    setPositionAndStabilise(r.get(), edme->geod());
+    
+    CPPUNIT_ASSERT_EQUAL(std::string{"125.015"}, std::string{node->getStringValue("frequencies/selected-mhz-fmt")});
+    CPPUNIT_ASSERT_EQUAL(1123, node->getIntValue("frequencies/selected-channel"));
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(125.0150, node->getDoubleValue("frequencies/selected-mhz"), 0.001);
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(125.0167, node->getDoubleValue("frequencies/selected-real-frequency-mhz"), 0.001);
+    
+    
+    node->setDoubleValue("frequencies/selected-mhz", 120.810);
+    setPositionAndStabilise(r.get(), edme->geod());
+    
+    CPPUNIT_ASSERT_EQUAL(std::string{"120.810"}, std::string{node->getStringValue("frequencies/selected-mhz-fmt")});
+    CPPUNIT_ASSERT_EQUAL(450, node->getIntValue("frequencies/selected-channel"));
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(120.810, node->getDoubleValue("frequencies/selected-mhz"), 0.001);
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(120.80833, node->getDoubleValue("frequencies/selected-real-frequency-mhz"), 0.00001);
+}

test_suite/unit_tests/Instrumentation/test_commRadio.hxx

@@ -0,0 +1,55 @@
+/*
+ * 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/>.
+ */
+
+
+#ifndef _FG_COMMRADIO_UNIT_TESTS_HXX
+#define _FG_COMMRADIO_UNIT_TESTS_HXX
+
+
+#include <cppunit/extensions/HelperMacros.h>
+#include <cppunit/TestFixture.h>
+
+class SGSubsystem;
+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(testWith8Point3Mode);
+    
+    CPPUNIT_TEST_SUITE_END();
+
+    void setPositionAndStabilise(SGSubsystem* r, const SGGeod& g);
+
+public:
+    // Set up function for each test.
+    void setUp();
+
+    // Clean up after each test.
+    void tearDown();
+
+    // The tests.
+    void testBasic();
+    void testWith8Point3Mode();
+};
+
+#endif  // _FG_COMMRADIO_UNIT_TESTS_HXX