diff --git a/src/Autopilot/inputvalue.cxx b/src/Autopilot/inputvalue.cxx index 42c0b6a21..baaee2b95 100644 --- a/src/Autopilot/inputvalue.cxx +++ b/src/Autopilot/inputvalue.cxx @@ -79,6 +79,22 @@ InputValue::InputValue( SGPropertyNode& prop_root, parse(prop_root, cfg, value, offset, scale); } +InputValue::~InputValue() +{ + if (_pathNode) { + _pathNode->removeChangeListener(this); + } +} + +void InputValue::initPropertyFromInitialValue() +{ + double s = get_scale(); + if( s != 0 ) + _property->setDoubleValue( (_value - get_offset())/s ); + else + _property->setDoubleValue(0); // if scale is zero, value*scale is zero +} + //------------------------------------------------------------------------------ void InputValue::parse( SGPropertyNode& prop_root, SGPropertyNode& cfg, @@ -128,6 +144,25 @@ void InputValue::parse( SGPropertyNode& prop_root, return; } + if ((n = cfg.getChild("property-path"))) { + // cache the root node, in case of changes + _rootNode = &prop_root; + const auto trimmed = simgear::strutils::strip(n->getStringValue()); + _pathNode = prop_root.getNode(trimmed, true); + _pathNode->addChangeListener(this); + + // if <property> is defined, should we use it to initialise + // the path prop? not doing so for now. + + const auto path = simgear::strutils::strip(_pathNode->getStringValue()); + if (!path.empty()) { + _property = _rootNode->getNode(path); + } + + return; + } + + // if no <property> element, check for <prop> element for backwards // compatibility if( (n = cfg.getChild("property")) @@ -138,19 +173,12 @@ void InputValue::parse( SGPropertyNode& prop_root, _property = prop_root.getNode(trimmed, true); if( valueNode ) { - // initialize property with given value - // if both <prop> and <value> exist - double s = get_scale(); - if( s != 0 ) - _property->setDoubleValue( (_value - get_offset())/s ); - else - _property->setDoubleValue(0); // if scale is zero, value*scale is zero + initPropertyFromInitialValue(); } return; } // of have a <property> or <prop> - if( !valueNode ) { // no <value>, <prop> or <expression> element, use text node @@ -225,6 +253,21 @@ double InputValue::get_value() const return _abs ? fabs(value) : value; } +bool InputValue::is_enabled() const +{ + if (_pathNode && !_property) { + // if we have a configurable path, and it's currently not valid, + // mark ourselves as disabled + return false; + } + + if (_condition) { + return _condition->test(); + } + + return true; // default to enab;ed +} + void InputValue::collectDependentProperties(std::set<const SGPropertyNode*>& props) const { if (_property) props.insert(_property); @@ -233,11 +276,35 @@ void InputValue::collectDependentProperties(std::set<const SGPropertyNode*>& pro if (_min) _min->collectDependentProperties(props); if (_max) _max->collectDependentProperties(props); if (_expression) _expression->collectDependentProperties(props); + if (_pathNode) props.insert(_pathNode); } +void InputValue::valueChanged(SGPropertyNode *node) +{ + assert(node == _pathNode); + const auto path = simgear::strutils::strip(_pathNode->getStringValue()); + if (path.empty()) { + // don't consider an empty string to mean the root node, that's not + // useful behaviour + _property.reset(); + return; + } + + // important we don't create here: this allows an invalid path + // to give us a null _property, which causes us to be marked as + // disabled, allowing another input to be used + auto propNode = _rootNode->getNode(path); + if (propNode) { + _property = propNode; + } else { + _property.reset(); + } +} + void InputValueList::collectDependentProperties(std::set<const SGPropertyNode*>& props) const { for (auto& iv: *this) { iv->collectDependentProperties(props); } } + diff --git a/src/Autopilot/inputvalue.hxx b/src/Autopilot/inputvalue.hxx index aba7676d9..7b3437656 100644 --- a/src/Autopilot/inputvalue.hxx +++ b/src/Autopilot/inputvalue.hxx @@ -57,7 +57,7 @@ public: * and/or offset, clamped to min/max values, be periodical, bound to * conditions or evaluated from expressions. */ -class InputValue : public SGReferenced { +class InputValue : public SGReferenced, public SGPropertyChangeListener { private: double _value; // The value as a constant or initializer for the property bool _abs; // return absolute value @@ -69,14 +69,19 @@ private: PeriodicalValue_ptr _periodical; // SGSharedPtr<const SGCondition> _condition; SGSharedPtr<SGExpressiond> _expression; ///< expression to generate the value - + SGPropertyNode_ptr _pathNode; + SGPropertyNode_ptr _rootNode; + + void valueChanged(SGPropertyNode* node) override; + void initPropertyFromInitialValue(); public: InputValue( SGPropertyNode& prop_root, SGPropertyNode& node, double value = 0.0, double offset = 0.0, double scale = 1.0 ); - + ~InputValue(); + /** * * @param prop_root Root node for all properties with relative path @@ -105,9 +110,7 @@ public: return _offset == NULL ? 0.0 : _offset->get_value(); } - inline bool is_enabled() const { - return _condition == NULL ? true : _condition->test(); - } + bool is_enabled() const; void collectDependentProperties(std::set<const SGPropertyNode*>& props) const; }; diff --git a/test_suite/unit_tests/Autopilot/CMakeLists.txt b/test_suite/unit_tests/Autopilot/CMakeLists.txt index 455d86135..61b3ad1e2 100644 --- a/test_suite/unit_tests/Autopilot/CMakeLists.txt +++ b/test_suite/unit_tests/Autopilot/CMakeLists.txt @@ -4,6 +4,7 @@ set(TESTSUITE_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/testDigitalFilter.cxx ${CMAKE_CURRENT_SOURCE_DIR}/testPidController.cxx ${CMAKE_CURRENT_SOURCE_DIR}/testPidControllerData.cxx + ${CMAKE_CURRENT_SOURCE_DIR}/testInputValue.cxx PARENT_SCOPE ) @@ -12,5 +13,6 @@ set(TESTSUITE_HEADERS ${CMAKE_CURRENT_SOURCE_DIR}/testDigitalFilter.hxx ${CMAKE_CURRENT_SOURCE_DIR}/testPidController.hxx ${CMAKE_CURRENT_SOURCE_DIR}/testPidControllerData.hxx + ${CMAKE_CURRENT_SOURCE_DIR}/testInputValue.hxx PARENT_SCOPE ) diff --git a/test_suite/unit_tests/Autopilot/TestSuite.cxx b/test_suite/unit_tests/Autopilot/TestSuite.cxx index 4202aa205..26bb531bd 100644 --- a/test_suite/unit_tests/Autopilot/TestSuite.cxx +++ b/test_suite/unit_tests/Autopilot/TestSuite.cxx @@ -19,8 +19,10 @@ #include "testDigitalFilter.hxx" #include "testPidController.hxx" - +#include "testInputValue.hxx" // Set up the unit tests. CPPUNIT_TEST_SUITE_NAMED_REGISTRATION(DigitalFilterTests, "Unit tests"); CPPUNIT_TEST_SUITE_NAMED_REGISTRATION(PidControllerTests, "Unit tests"); +CPPUNIT_TEST_SUITE_NAMED_REGISTRATION(InputValueTests, "Unit tests"); + diff --git a/test_suite/unit_tests/Autopilot/testInputValue.cxx b/test_suite/unit_tests/Autopilot/testInputValue.cxx new file mode 100644 index 000000000..d62369dc3 --- /dev/null +++ b/test_suite/unit_tests/Autopilot/testInputValue.cxx @@ -0,0 +1,115 @@ +/* +SPDX-Copyright: James Turner +SPDX-License-Identifier: GPL-2.0-or-later +*/ + +#include "testInputValue.hxx" + +#include <strstream> + +#include "test_suite/FGTestApi/testGlobals.hxx" + + +#include <Autopilot/autopilot.hxx> +#include <Autopilot/inputvalue.hxx> +#include <Main/fg_props.hxx> +#include <Main/globals.hxx> + + +#include <simgear/math/sg_random.hxx> +#include <simgear/props/props_io.hxx> + +using namespace FGXMLAutopilot; + +// Set up function for each test. +void InputValueTests::setUp() +{ + FGTestApi::setUp::initTestGlobals("ap-inputvalue"); +} + + +// Clean up after each test. +void InputValueTests::tearDown() +{ + FGTestApi::tearDown::shutdownTestGlobals(); +} + + +SGPropertyNode_ptr InputValueTests::configFromString(const std::string& s) +{ + SGPropertyNode_ptr config = new SGPropertyNode; + + std::istringstream iss(s); + readProperties(iss, config); + return config; +} + +void InputValueTests::testPropertyPath() +{ + + sg_srandom(999); + + auto config = configFromString(R"(<?xml version="1.0" encoding="UTF-8"?> + <PropertyList> + <property-path>/test/altitude-ft-node-path</property-path> + <value>1.23</value> + </PropertyList> + )"); + + fgSetString("/test/altitude-ft-node-path", "/test/a"); + fgSetDouble("/test/a", 0.5); + + InputValue_ptr valueA = new InputValue(*globals->get_props(), *config); + + CPPUNIT_ASSERT(valueA->is_enabled()); + // check value is not written back + CPPUNIT_ASSERT_DOUBLES_EQUAL(0.5, valueA->get_value(), 0.001); + CPPUNIT_ASSERT_DOUBLES_EQUAL(0.5, fgGetDouble("/test/a"), 0.001); + + fgSetDouble("/test/a", 2.34); + CPPUNIT_ASSERT_DOUBLES_EQUAL(2.34, valueA->get_value(), 0.001); + + fgSetString("/test/altitude-ft-node-path", "blah"); + CPPUNIT_ASSERT(!valueA->is_enabled()); + // <value> is used + CPPUNIT_ASSERT_DOUBLES_EQUAL(1.23, valueA->get_value(), 0.001); + + + fgSetDouble("/foo/bpath", 99); + fgSetString("/test/altitude-ft-node-path", "/foo/bpath"); + CPPUNIT_ASSERT(valueA->is_enabled()); + CPPUNIT_ASSERT_DOUBLES_EQUAL(99.0, valueA->get_value(), 0.001); + + fgSetDouble("/foo/bpath", -45.1); + CPPUNIT_ASSERT_DOUBLES_EQUAL(-45.1, valueA->get_value(), 0.001); + +// start with different config + auto config2 = configFromString(R"(<?xml version="1.0" encoding="UTF-8"?> + <PropertyList> + <property-path>/test/indicated-knots-node-path</property-path> + </PropertyList> + )"); + + InputValue_ptr valueB = new InputValue(*globals->get_props(), *config2); + CPPUNIT_ASSERT(!valueB->is_enabled()); + CPPUNIT_ASSERT_DOUBLES_EQUAL(0.0, valueB->get_value(), 0.001); + + fgSetString("/test/indicated-knots-node-path", "/instruments/airspeed/output/knots"); + CPPUNIT_ASSERT(!valueB->is_enabled()); + CPPUNIT_ASSERT_DOUBLES_EQUAL(0.0, valueB->get_value(), 0.001); + + // create the property, but this does not trigger the change listener, so + // stays invalid + fgSetDouble("/instruments/airspeed/output/knots", 415); + CPPUNIT_ASSERT(!valueB->is_enabled()); + CPPUNIT_ASSERT_DOUBLES_EQUAL(0.0, valueB->get_value(), 0.001); + + // set the path again (with some whitespace, which is trimmed) + fgSetString("/test/indicated-knots-node-path", " /instruments/airspeed/output/knots "); + CPPUNIT_ASSERT(valueB->is_enabled()); + CPPUNIT_ASSERT_DOUBLES_EQUAL(415.0, valueB->get_value(), 0.001); + + fgSetString("/test/indicated-knots-node-path", ""); + CPPUNIT_ASSERT(!valueB->is_enabled()); + +} diff --git a/test_suite/unit_tests/Autopilot/testInputValue.hxx b/test_suite/unit_tests/Autopilot/testInputValue.hxx new file mode 100644 index 000000000..d303a383e --- /dev/null +++ b/test_suite/unit_tests/Autopilot/testInputValue.hxx @@ -0,0 +1,35 @@ +/* +SPDX-Copyright: James Turner +SPDX-License-Identifier: GPL-2.0-or-later + */ + + +#pragma once + +#include <cppunit/TestFixture.h> +#include <cppunit/extensions/HelperMacros.h> + +#include <simgear/props/props.hxx> + + +// The system tests. +class InputValueTests : public CppUnit::TestFixture +{ + // Set up the test suite. + CPPUNIT_TEST_SUITE(InputValueTests); + CPPUNIT_TEST(testPropertyPath); + CPPUNIT_TEST_SUITE_END(); + + SGPropertyNode_ptr configFromString(const std::string& s); + +public: + // Set up function for each test. + void setUp(); + + // Clean up after each test. + void tearDown(); + + + // The tests. + void testPropertyPath(); +};