diff --git a/Modules/include/StatusMonitor.h b/Modules/include/StatusMonitor.h index bdf2640684d7b071c88caec115a289db83a58c96..da2e2467ab7654e62f923d57143a3655526783b7 100644 --- a/Modules/include/StatusMonitor.h +++ b/Modules/include/StatusMonitor.h @@ -42,6 +42,8 @@ namespace ChimeraTK { * type of the variable to be monitored. A non-template base class * facilitates checking for the type in the StatusAggregator, which * needs to identify any StatusMonitor. + * + * FIXME This distinction between StatusMonitorImpl and StatusMonitor is no longer required. Merge the classes! */ struct StatusMonitor : ApplicationModule { StatusMonitor(EntityOwner* owner, const std::string& name, const std::string& description, const std::string& input, @@ -217,30 +219,91 @@ namespace ChimeraTK { } }; - /** Module for status monitoring of an exact value. - * If value monitored is not exactly the same. an fault state will be - * reported.*/ + /** + * Module for status monitoring of an exact value. + * + * If monitored input value is not exactly the same as the requiredValue, a fault state will be reported. If the + * parameter variable "disable" is set to a non-zero value, the monitoring is disabled and the output status is + * always OFF. + * + * Note: It is strongly recommended to use this monitor only for integer data types or strings, as floating point + * data types should never be compared with exact equality. + */ template<typename T> - struct ExactMonitor : public StatusMonitorImpl<T> { - using StatusMonitorImpl<T>::StatusMonitorImpl; - /**FAULT state if value is not equal to requiredValue*/ - ScalarPushInput<T> requiredValue{this, "requiredValue", "", "", StatusMonitor::_parameterTags}; + struct ExactMonitor : ApplicationModule { + /** + * Constructor for exact monitoring module. + * + * inputPath: qualified path of the variable to monitor + * outputPath: qualified path of the status output variable + * parameterPath: qualified path of the VariableGroup holding the parameter variables requiredValue and disable + * + * All qualified paths can be either relative or absolute to the given owner. See HierarchyModifyingGroup for + * more details. + */ + ExactMonitor(EntityOwner* owner, const std::string& inputPath, const std::string& outputPath, + const std::string& parameterPath, const std::string& description, + const std::unordered_set<std::string>& outputTags = {}, + const std::unordered_set<std::string>& parameterTags = {}) + : ExactMonitor(owner, inputPath, outputPath, parameterPath + "/requiredValue", parameterPath + "/disable", + description, outputTags, parameterTags) {} - /**This is where state evaluation is done*/ + /** + * Constructor for exact monitoring module. + * + * inputPath: qualified path of the variable to monitor + * outputPath: qualified path of the status output variable + * requiredValuePath: qualified path of the parameter variable requiredValue + * disablePath: qualified path of the parameter variable disable + * + * All qualified paths can be either relative or absolute to the given owner. See HierarchyModifyingGroup for + * more details. + */ + ExactMonitor(EntityOwner* owner, const std::string& inputPath, const std::string& outputPath, + const std::string& requiredValuePath, const std::string& disablePath, const std::string& description, + const std::unordered_set<std::string>& outputTags = {}, + const std::unordered_set<std::string>& parameterTags = {}) + : ApplicationModule(owner, "hidden", description, HierarchyModifier::hideThis), + watch(this, inputPath, "", "Value to monitor"), + requiredValue(this, requiredValuePath, "", "Value to compare with", parameterTags), + disable(this, disablePath, "", "Disable the status monitor", parameterTags), + status(this, outputPath, "Resulting status", outputTags) {} + + ExactMonitor() = default; + + /** Variable to monitor */ + ModifyHierarchy<ScalarPushInput<T>> watch; + + /** The required value to compare with */ + ModifyHierarchy<ScalarPushInput<T>> requiredValue; + + /** Disable/enable the entire status monitor */ + ModifyHierarchy<ScalarPushInput<int>> disable; + + /** Result of the monitor */ + ModifyHierarchy<StatusOutput> status; + + /** This is where state evaluation is done */ void mainLoop() { - /** If there is a change either in value monitored or in requiredValue, the status is re-evaluated*/ - ReadAnyGroup group{StatusMonitorImpl<T>::oneUp.watch, StatusMonitorImpl<T>::disable, requiredValue}; + // If there is a change either in value monitored or in requiredValue, the status is re-evaluated + ReadAnyGroup group{watch.value, disable.value, requiredValue.value}; while(true) { - if(StatusMonitorImpl<T>::disable != 0) { - StatusMonitorImpl<T>::status = StatusOutput::Status::OFF; + StatusOutput::Status newStatus; + if(disable.value != 0) { + newStatus = StatusOutput::Status::OFF; } - else if(StatusMonitorImpl<T>::oneUp.watch != requiredValue) { - StatusMonitorImpl<T>::status = StatusOutput::Status::FAULT; + else if(watch.value != requiredValue.value) { + newStatus = StatusOutput::Status::FAULT; } else { - StatusMonitorImpl<T>::status = StatusOutput::Status::OK; + newStatus = StatusOutput::Status::OK; + } + + // update only if status has changed, but always in case of initial value + if(status.value != newStatus || status.value.getVersionNumber() == VersionNumber{nullptr}) { + status.value = newStatus; + status.value.write(); } - StatusMonitorImpl<T>::status.write(); group.readAny(); } } diff --git a/include/EntityOwner.h b/include/EntityOwner.h index d32d375ef066b9806139664213f4f667099d25c2..6e1f897df8708b580572333d23f8f1a17e8f40ed 100644 --- a/include/EntityOwner.h +++ b/include/EntityOwner.h @@ -20,10 +20,15 @@ namespace ChimeraTK { class Module; class VirtualModule; - /** Base class for owners of other EntityOwners (e.g. Modules) and Accessors. - * @todo Rename this class to "Owner" and make it more generic. It should - * basically just implement the "Composite Pattern". The classes AccessorBase, - * Module and Owner should have a common base class called "Component". + /** + * Convenience type definition which can optionally be used as a shortcut for the type which defines a list of + * tags. + */ + using TAGS = const std::unordered_set<std::string>; + + /** + * Base class for owners of other EntityOwners (e.g. Modules) and Accessors. + * FIXME: Unify with Module class (not straight forward!). */ class EntityOwner { public: diff --git a/include/HierarchyModifyingGroup.h b/include/HierarchyModifyingGroup.h index f81bff2b7999a6291af2168e9a354b0bae6c6354..9285209dce4ce8e00039273123cbc57849f18030 100644 --- a/include/HierarchyModifyingGroup.h +++ b/include/HierarchyModifyingGroup.h @@ -8,6 +8,8 @@ namespace ChimeraTK { + /********************************************************************************************************************/ + /** * A HierarchyModifyingGroup is a VariableGroup which can easily appear at a different place in the hierarchy by * specifying a qualified name. The qualifiedName can contain multiple hierarchy levels separated by slashes "/". @@ -50,6 +52,27 @@ namespace ChimeraTK { std::vector<std::string> _splittedPath; }; + /********************************************************************************************************************/ + + /** + * Convenience version of the HierarchyModifyingGroup with exactly one variable inside. The constructor takes the + * qualified name of the variable and splits it internally into the path name (for the HierarchyModifyingGroup) and + * the unqialified variable name. + * + * The template argument must be one of the Scalar*Input, ScalarOutput classes resp. their Array counterparts. + */ + template<typename ACCESSOR> + struct ModifyHierarchy : HierarchyModifyingGroup { + template<typename... Types> + ModifyHierarchy(Module* owner, const std::string& qualifiedName, Types... args) + : HierarchyModifyingGroup(owner, HierarchyModifyingGroup::getPathName(qualifiedName), ""), + value(this, HierarchyModifyingGroup::getUnqualifiedName(qualifiedName), args...) {} + + ACCESSOR value; + }; + + /********************************************************************************************************************/ + } // namespace ChimeraTK #endif // HIERARCHYMODIFYINGGROUP_H diff --git a/tests/executables_src/testStatusModule.cc b/tests/executables_src/testStatusModule.cc index 578c07b2e0c2d1e1d77cbe8b936a226b9a5155df..47e47ee2471bcfc07e6b6d499f2926492b2ab726 100644 --- a/tests/executables_src/testStatusModule.cc +++ b/tests/executables_src/testStatusModule.cc @@ -9,8 +9,8 @@ #include "ApplicationModule.h" #include "ControlSystemModule.h" #include "ScalarAccessor.h" -#include "TestFacility.h" #include "StatusMonitor.h" +#include "TestFacility.h" using namespace boost::unit_test_framework; namespace ctk = ChimeraTK; @@ -19,9 +19,9 @@ namespace ctk = ChimeraTK; template<typename T> struct TestApplication : public ctk::Application { TestApplication() : Application("testSuite") {} - ~TestApplication() { shutdown(); } + ~TestApplication() override { shutdown(); } - void defineConnections() { + void defineConnections() override { findTag(".*").connectTo(cs); // publish everything to CS findTag("MY_MONITOR").connectTo(cs["MyNiceMonitorCopy"]); // cable again, checking that the tag is applied correctly findTag("MON_PARAMS") @@ -34,6 +34,24 @@ struct TestApplication : public ctk::Application { {"MON_OUTPUT"}, {"MON_PARAMS"}, {"MY_MONITOR"}}; }; +/* dummy application - for new StatusMonitor interface */ +template<typename T> +struct TestApplicationNewInterface : public ctk::Application { + TestApplicationNewInterface() : Application("testSuite") {} + ~TestApplicationNewInterface() override { shutdown(); } + + void defineConnections() override { + findTag(".*").connectTo(cs); // publish everything to CS + findTag("MON_PARAMS") + .connectTo(cs["MonitorParameters"]); // cable the parameters in addition (checking that tags are set correctly) + findTag("MON_OUTPUT") + .connectTo(cs["MonitorOutput"]); // cable the parameters in addition (checking that tags are set correctly) + } + ctk::ControlSystemModule cs; + T monitor{this, "/input/path", "/output/path", "/parameters", "Now this is a nice monitor...", + ctk::TAGS{"MON_OUTPUT"}, ctk::TAGS{"MON_PARAMS"}}; +}; + /*********************************************************************************************************************/ BOOST_AUTO_TEST_CASE(testMaxMonitor) { @@ -552,34 +570,34 @@ BOOST_AUTO_TEST_CASE(testRangeMonitor) { BOOST_AUTO_TEST_CASE(testExactMonitor) { std::cout << "testExactMonitor" << std::endl; - TestApplication<ctk::ExactMonitor<float>> app; + TestApplicationNewInterface<ctk::ExactMonitor<int64_t>> app; // check that the reserved StatusOutput tag is present at the output, required for StatusAggregator integration - auto tags = ctk::VariableNetworkNode(app.monitor.status).getTags(); + auto tags = ctk::VariableNetworkNode(app.monitor.status.value).getTags(); BOOST_CHECK(tags.find(ctk::StatusOutput::tagStatusOutput) != tags.end()); ctk::TestFacility test; test.runApplication(); //app.dumpConnections(); - auto requiredValue = test.getScalar<float>(std::string("/Monitor/requiredValue")); - requiredValue = 40.9; + auto requiredValue = test.getScalar<int64_t>(std::string("/parameters/requiredValue")); + requiredValue = 409; requiredValue.write(); test.stepApplication(); - auto watch = test.getScalar<float>(std::string("/watch")); - watch = 40.9; + auto watch = test.getScalar<int64_t>(std::string("/input/path")); + watch = 409; watch.write(); test.stepApplication(); - auto status = test.getScalar<int32_t>(std::string("/Monitor/status")); + auto status = test.getScalar<int32_t>(std::string("/output/path")); status.readLatest(); //should be in OK state. BOOST_CHECK_EQUAL(status, static_cast<int>(ChimeraTK::StatusOutput::Status::OK)); // drop in a disable test. - auto disable = test.getScalar<int>("/Monitor/disable"); + auto disable = test.getScalar<int>("/parameters/disable"); disable = 1; disable.write(); test.stepApplication(); @@ -593,7 +611,7 @@ BOOST_AUTO_TEST_CASE(testExactMonitor) { BOOST_CHECK_EQUAL(status, static_cast<int>(ChimeraTK::StatusOutput::Status::OK)); //set watch value different than required value - watch = 41.4; + watch = 414; watch.write(); test.stepApplication(); status.readLatest(); @@ -613,7 +631,7 @@ BOOST_AUTO_TEST_CASE(testExactMonitor) { status.readLatest(); BOOST_CHECK_EQUAL(status, static_cast<int>(ChimeraTK::StatusOutput::Status::FAULT)); - watch = 40.9; + watch = 409; watch.write(); test.stepApplication(); status.readLatest(); @@ -621,7 +639,7 @@ BOOST_AUTO_TEST_CASE(testExactMonitor) { BOOST_CHECK_EQUAL(status, static_cast<int>(ChimeraTK::StatusOutput::Status::OK)); //set requiredValue value different than watch value - requiredValue = 41.3; + requiredValue = 413; requiredValue.write(); test.stepApplication(); status.readLatest(); @@ -629,7 +647,7 @@ BOOST_AUTO_TEST_CASE(testExactMonitor) { BOOST_CHECK_EQUAL(status, static_cast<int>(ChimeraTK::StatusOutput::Status::FAULT)); //set requiredValue value equals to watch value - requiredValue = 40.9; + requiredValue = 409; requiredValue.write(); test.stepApplication(); status.readLatest(); @@ -637,10 +655,12 @@ BOOST_AUTO_TEST_CASE(testExactMonitor) { BOOST_CHECK_EQUAL(status, static_cast<int>(ChimeraTK::StatusOutput::Status::OK)); // check that the tags are applied correctly - BOOST_CHECK_EQUAL(status, test.readScalar<int32_t>("/MyNiceMonitorCopy/Monitor/status")); - BOOST_CHECK_EQUAL(status, test.readScalar<int32_t>("/MonitorOutput/Monitor/status")); - BOOST_CHECK_EQUAL(watch, test.readScalar<float>("/MyNiceMonitorCopy/watch")); - BOOST_CHECK_EQUAL(requiredValue, test.readScalar<float>("/MonitorParameters/Monitor/requiredValue")); + BOOST_CHECK_EQUAL(status, test.readScalar<int32_t>("/MonitorOutput/output/path")); + BOOST_CHECK_EQUAL(requiredValue, test.readScalar<int64_t>("/MonitorParameters/parameters/requiredValue")); + disable = 1; + disable.write(); + test.stepApplication(); + BOOST_CHECK_EQUAL(disable, test.readScalar<int>("/MonitorParameters/parameters/disable")); } /*********************************************************************************************************************/