From af3cb3c3bb5ab33952a0c68a7b64147a4b3bca4e Mon Sep 17 00:00:00 2001
From: d-rothe <39566710+d-rothe@users.noreply.github.com>
Date: Wed, 15 Jun 2022 11:23:36 +0200
Subject: [PATCH] Wip/9838 error reporting concept (#73)
* concept for error reporting to set_error
- destination is DOOCS EqFct::set_error(code, message)
- StatusMonitor variables and/or Device status variables as input
- configuration per location via xml
- only single data source for set_error
- consistent updates of error code and error message, to remove
duplicate log lines
this depends on changes in ApplicationCore/DeviceModule!
* remove deprecated use of boost/test/test_case_template.hpp
* make configuration of status message input unneccesary
it will be found from the naming convention
* some refactoring
which was neccesary because Status definitions moved to
ControlSystemAdapter
* add docu for set_error
* add basic test for set_error tag
Co-authored-by: Dietrich Rothe <dietrich.rothe@desy.de>
---
doc/mainpage.dox | 14 +++-
include/CSAdapterEqFct.h | 13 ++--
include/PropertyDescription.h | 7 ++
include/StatusHandler.h | 37 +++++++++++
include/VariableMapper.h | 5 ++
src/CSAdapterEqFct.cc | 17 +++++
src/StatusHandler.cc | 65 +++++++++++++++++++
src/VariableMapper.cc | 21 ++++++
...serverTestSetError-DoocsVariableConfig.xml | 9 +++
tests/serverTestSetError.conf | 17 +++++
tests/src/serverTestSetError.cpp | 49 ++++++++++++++
tests/src/testDoocsPVFactory.cpp | 2 +-
tests/src/testDoocsProcessScalar.cpp | 2 +-
tests/src/testDoocsSpectrum.cpp | 2 +-
14 files changed, 248 insertions(+), 12 deletions(-)
create mode 100644 include/StatusHandler.h
create mode 100644 src/StatusHandler.cc
create mode 100644 tests/serverTestSetError-DoocsVariableConfig.xml
create mode 100644 tests/serverTestSetError.conf
create mode 100644 tests/src/serverTestSetError.cpp
diff --git a/doc/mainpage.dox b/doc/mainpage.dox
index 764a719..1d50c04 100644
--- a/doc/mainpage.dox
+++ b/doc/mainpage.dox
@@ -151,7 +151,7 @@ The `D_array` tag takes the following arguments through XML attributes of the D_
For an example, see above in Section \ref mapping_file.
-\subsection D_xy D_xy
+\subsubsection D_xy D_xy
The `D_xy` tag takes the following arguments through properties:
- `x_source` : Name of process variable which should be used for x-axis values
- `y_source` : Name of process variable which should be used for y-axis values
@@ -162,8 +162,18 @@ The `D_xy` tag takes the following arguments through sub-tags:
\include tests/serverTestXy-DoocsVariableConfig.xml
+\subsection set_error Error reporting
+The special tag `set_error` is allowed only once per location. XML attributes:
+- `statusCodeSource` : Path to the process variable which should be used for reading error/status codes.
+
+DoocsAdapter will automatically look for an associated variable with error/status messages, and if present, read both
+consistently.
+Errors indicated via the status variable will be published via DOOCS set_error function (of the location), which
+sets the properties ERROR, ERROR.STR, STS.ERROR, STS.NEWERROR, and also logs into LOG, LOG.LAST, LOG_HTML and LOG_HTML.LAST.
+Compare DOOCS documentation for error properties and propagation to the overall error counting per server, SVR.ERROR_COUNT.
+
-\subsection specifie_code Specifie `eq_fct_code` in the mapping file
+\subsection specifie_code Specify `eq_fct_code` in the mapping file
It is possible to specify the `eq_fct_code` which must match `eq_fct_type` in the configuration file with `code` in
the \<location\> tag of the mapping file.
diff --git a/include/CSAdapterEqFct.h b/include/CSAdapterEqFct.h
index f1e9ed8..a7dd6a8 100644
--- a/include/CSAdapterEqFct.h
+++ b/include/CSAdapterEqFct.h
@@ -1,5 +1,4 @@
-#ifndef CS_ADAPTER_EQ_FCT_H
-#define CS_ADAPTER_EQ_FCT_H
+#pragma once
#include <boost/noncopyable.hpp>
#include <boost/scoped_ptr.hpp>
@@ -9,13 +8,13 @@
#include <ChimeraTK/ControlSystemAdapter/ControlSystemPVManager.h>
#include <ChimeraTK/ControlSystemAdapter/ProcessVariable.h>
-#include "DoocsUpdater.h"
-#include "PropertyDescription.h"
-
namespace ChimeraTK {
template<typename DOOCS_T, typename DOOCS_PRIMITIVE_T>
class DoocsProcessArray;
+ class StatusHandler;
+ class DoocsUpdater;
+ class PropertyDescription;
class CSAdapterEqFct : public EqFct, boost::noncopyable {
protected:
@@ -28,6 +27,8 @@ namespace ChimeraTK {
static bool emptyLocationVariablesHandled;
boost::shared_ptr<DoocsUpdater> updater_;
+ boost::shared_ptr<StatusHandler> statusHandler_;
+
public:
// The fctName (location name ) is usually coming from the config file and
// should be left empty. Only for testing without actually running a DOOCS
@@ -48,5 +49,3 @@ namespace ChimeraTK {
};
} // namespace ChimeraTK
-
-#endif // CS_ADAPTER_EQ_FCT_H
diff --git a/include/PropertyDescription.h b/include/PropertyDescription.h
index ddc07f2..91247f0 100644
--- a/include/PropertyDescription.h
+++ b/include/PropertyDescription.h
@@ -215,6 +215,13 @@ namespace ChimeraTK {
useDataMatchingDefault(useDataMatchingDefault_) {}
};
+ // parsed info about error reporting to locations
+ struct ErrorReportingInfo {
+ ChimeraTK::RegisterPath statusCodeSource;
+ ChimeraTK::RegisterPath statusStringSource;
+ std::string targetLocation;
+ };
+
} // namespace ChimeraTK
#endif // CHIMERATK_DOOCS_ADAPTER_PROPERTY_DESCRIPTION_H
diff --git a/include/StatusHandler.h b/include/StatusHandler.h
new file mode 100644
index 0000000..9eba4c0
--- /dev/null
+++ b/include/StatusHandler.h
@@ -0,0 +1,37 @@
+#pragma once
+
+#include <ChimeraTK/ControlSystemAdapter/StatusWithMessageReader.h>
+#include <boost/shared_ptr.hpp>
+#include <boost/noncopyable.hpp>
+
+class EqFct;
+namespace ChimeraTK {
+
+ class DoocsAdapter;
+ class DoocsUpdater;
+
+ /**
+ * StatusHandler implements an update function for reporting errors to locations.
+ * Input should be e.g. status of a DeviceModule or StatusAggregator.
+ * Status codes are mapped to DOOCS and written via set_error(code, string)
+ */
+ class StatusHandler : public boost::noncopyable {
+ boost::shared_ptr<DoocsUpdater> _doocsUpdater;
+ EqFct* _eqFct;
+ /// accessors to the status and string variables we want to monitor
+ StatusWithMessageReader _varPair;
+
+ public:
+ void updateError(TransferElementID transferElementId);
+
+ /// statusScalar and statusString are the variables to monitor. statusScalar is mandatory, statusString not.
+ /// If both are set, updates must come in consistently
+ StatusHandler(EqFct* eqFct, boost::shared_ptr<DoocsUpdater> const& updater,
+ boost::shared_ptr<ChimeraTK::NDRegisterAccessor<int32_t>> const& statusScalar,
+ boost::shared_ptr<ChimeraTK::NDRegisterAccessor<std::string>> const& statusString = nullptr);
+
+ /// mapping function from Device.status/StatusOutput to DOOCS error codes
+ int statusCodeMapping(int x);
+ };
+
+} // namespace ChimeraTK
diff --git a/include/VariableMapper.h b/include/VariableMapper.h
index f9c0a93..2d05d0b 100644
--- a/include/VariableMapper.h
+++ b/include/VariableMapper.h
@@ -56,6 +56,8 @@ namespace ChimeraTK {
const std::set<std::string>& getUsedVariables() const { return _usedInputVariables; }
+ const std::list<ErrorReportingInfo>& getErrorReportingInfos() const { return _errorReportingInfos; }
+
protected:
VariableMapper() = default;
@@ -65,11 +67,14 @@ namespace ChimeraTK {
std::set<std::string> _usedInputVariables; // For tracing which variables are
// not to be imported.
+ std::list<ErrorReportingInfo> _errorReportingInfos;
+
void processLocationNode(xmlpp::Node const* locationNode);
void processNode(xmlpp::Node const* propertyNode, std::string locationName);
void processSpectrumNode(xmlpp::Node const* node, std::string locationName);
void processXyNode(xmlpp::Node const* node, std::string& locationName);
void processIfffNode(xmlpp::Node const* node, std::string& locationName);
+ void processSetErrorNode(xmlpp::Node const* node, std::string& locationName);
void processImportNode(xmlpp::Node const* importNode, std::string importLocationName = std::string());
void processCode(xmlpp::Element const* location, std::string locationName);
diff --git a/src/CSAdapterEqFct.cc b/src/CSAdapterEqFct.cc
index c2046a4..33f06e3 100644
--- a/src/CSAdapterEqFct.cc
+++ b/src/CSAdapterEqFct.cc
@@ -1,8 +1,10 @@
+#include "StatusHandler.h" // include this first to avoid name clash with #define from DOOCS
#include "CSAdapterEqFct.h"
#include "DoocsPVFactory.h"
#include "DoocsUpdater.h"
#include "VariableMapper.h"
#include "DoocsProcessArray.h"
+#include "PropertyDescription.h"
namespace ChimeraTK {
@@ -22,6 +24,21 @@ namespace ChimeraTK {
name_.assign(fctName);
}
registerProcessVariablesInDoocs();
+
+ // construct and populate the StatusHandler for this location
+ for(const ErrorReportingInfo& errorReportingInfo :
+ ChimeraTK::VariableMapper::getInstance().getErrorReportingInfos()) {
+ if(name_.get_value() != errorReportingInfo.targetLocation) continue;
+
+ assert(!statusHandler_); // only single StatusHandler may be requested per location
+ auto statusCodeVariable = controlSystemPVManager_->getProcessArray<int32_t>(errorReportingInfo.statusCodeSource);
+ if(!statusCodeVariable)
+ throw ChimeraTK::logic_error("illegal/non-existing statusCodeSource: " + errorReportingInfo.statusCodeSource);
+ // this one is optional
+ ProcessArray<std::string>::SharedPtr statusStringVariable =
+ controlSystemPVManager_->getProcessArray<std::string>(errorReportingInfo.statusStringSource);
+ statusHandler_.reset(new StatusHandler(this, updater_, statusCodeVariable, statusStringVariable));
+ }
}
CSAdapterEqFct::~CSAdapterEqFct() {
diff --git a/src/StatusHandler.cc b/src/StatusHandler.cc
new file mode 100644
index 0000000..96c5b2f
--- /dev/null
+++ b/src/StatusHandler.cc
@@ -0,0 +1,65 @@
+
+// name clash with DOOCS-#defined FAULT, so include this first and then #undef
+#include <eq_errors.h>
+#include <eq_fct.h>
+#undef FAULT
+
+#include "StatusHandler.h"
+
+#include "DoocsUpdater.h"
+#include "DoocsAdapter.h"
+#include <ChimeraTK/ScalarRegisterAccessor.h>
+#include <ChimeraTK/DataConsistencyGroup.h>
+#include <ChimeraTK/ControlSystemAdapter/StatusWithMessageReader.h>
+#include <boost/shared_ptr.hpp>
+
+namespace ChimeraTK {
+
+ void StatusHandler::updateError(TransferElementID transferElementId) {
+ if(!_varPair.update(transferElementId)) return;
+ int statusCode = _varPair._status;
+ int err_no = statusCodeMapping(statusCode);
+ std::string err_str = _varPair.getMessage();
+
+ // Note: we already own the location lock by specification of the DoocsUpdater
+ if(err_no == no_error)
+ _eqFct->set_error(err_no);
+ else
+ _eqFct->set_error(err_no, err_str);
+ }
+
+ StatusHandler::StatusHandler(EqFct* eqFct, boost::shared_ptr<DoocsUpdater> const& updater,
+ boost::shared_ptr<ChimeraTK::NDRegisterAccessor<int32_t>> const& statusScalar,
+ boost::shared_ptr<ChimeraTK::NDRegisterAccessor<std::string>> const& statusString)
+ : _doocsUpdater(updater), _eqFct(eqFct), _varPair(ChimeraTK::ScalarRegisterAccessor<int32_t>(statusScalar)) {
+ if(!statusScalar->isReadable()) throw ChimeraTK::logic_error(statusScalar->getName() + " is not readable!");
+
+ _doocsUpdater->addVariable(
+ _varPair._status, _eqFct, std::bind(&StatusHandler::updateError, this, statusScalar->getId()));
+ if(statusString) {
+ if(!statusString->isReadable()) throw ChimeraTK::logic_error(statusString->getName() + " is not readable!");
+ _varPair.setMessageSource(ChimeraTK::ScalarRegisterAccessor<std::string>(statusString));
+
+ _doocsUpdater->addVariable(
+ _varPair._message, _eqFct, std::bind(&StatusHandler::updateError, this, statusString->getId()));
+ }
+ }
+
+ int StatusHandler::statusCodeMapping(int x) {
+ auto err = StatusAccessorBase::Status(x);
+ // a mapping for StatusOutput values -> DOOCS error codes
+ switch(err) {
+ case StatusAccessorBase::Status::OK:
+ return no_error;
+ case StatusAccessorBase::Status::FAULT:
+ return not_available;
+ case StatusAccessorBase::Status::OFF:
+ return device_offline;
+ case StatusAccessorBase::Status::WARNING:
+ return warning;
+ default:
+ return x;
+ }
+ }
+
+} // namespace ChimeraTK
diff --git a/src/VariableMapper.cc b/src/VariableMapper.cc
index 7121a57..137ec49 100644
--- a/src/VariableMapper.cc
+++ b/src/VariableMapper.cc
@@ -85,6 +85,9 @@ namespace ChimeraTK {
else if(node->get_name() == "D_ifff") {
processIfffNode(node, locationName);
}
+ else if(node->get_name() == "set_error") {
+ processSetErrorNode(node, locationName);
+ }
else {
throw std::invalid_argument(std::string("Error parsing xml file in location ") + locationName +
": Unknown node '" + node->get_name() + "'");
@@ -386,6 +389,24 @@ namespace ChimeraTK {
addDescription(ifffDescription, absoluteSources);
}
+ /********************************************************************************************************************/
+ void VariableMapper::processSetErrorNode(xmlpp::Node const* node, std::string& locationName) {
+ for(auto const& errRepInfo : _errorReportingInfos) {
+ if(errRepInfo.targetLocation == locationName)
+ throw std::invalid_argument(std::string("Error parsing xml file in location ") + locationName +
+ ": tag <set_error> is allowed only once per location.");
+ }
+ ErrorReportingInfo errInfo;
+ errInfo.targetLocation = locationName;
+
+ auto setErrorXml = asXmlElement(node);
+ std::string s = getAttributeValue(setErrorXml, "statusCodeSource");
+ errInfo.statusCodeSource = s;
+ // matching status message source is found automatically by naming convention - if it exists
+ errInfo.statusStringSource = s + "_message";
+ _errorReportingInfos.push_back(errInfo);
+ }
+
/********************************************************************************************************************/
void VariableMapper::processImportNode(xmlpp::Node const* importNode, std::string importLocationName) {
diff --git a/tests/serverTestSetError-DoocsVariableConfig.xml b/tests/serverTestSetError-DoocsVariableConfig.xml
new file mode 100644
index 0000000..f2b8161
--- /dev/null
+++ b/tests/serverTestSetError-DoocsVariableConfig.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<device_server xmlns="https://github.com/ChimeraTK/ControlSystemAdapter-DoocsAdapter">
+ <location name="INT">
+ <set_error statusCodeSource="/INT/FROM_DEVICE_SCALAR" />
+ <D_spectrum name="TEST" source="/FLOAT/FROM_DEVICE_ARRAY" />
+ </location>
+
+ <import>/</import>
+</device_server>
diff --git a/tests/serverTestSetError.conf b/tests/serverTestSetError.conf
new file mode 100644
index 0000000..5cf1901
--- /dev/null
+++ b/tests/serverTestSetError.conf
@@ -0,0 +1,17 @@
+eq_conf:
+
+oper_uid: -1
+oper_gid: 405
+xpert_uid: 1000
+xpert_gid: 1000
+ring_buffer: 10000
+memory_buffer: 500
+
+eq_fct_name: "ERROR_TEST._SVR"
+eq_fct_type: 1
+{
+SVR.RPC_NUMBER: 700000004
+SVR.NAME: "ERROR_TEST._SVR"
+SVR.BPN: 6000
+}
+
diff --git a/tests/src/serverTestSetError.cpp b/tests/src/serverTestSetError.cpp
new file mode 100644
index 0000000..454823d
--- /dev/null
+++ b/tests/src/serverTestSetError.cpp
@@ -0,0 +1,49 @@
+#define BOOST_TEST_MODULE serverTestSetError
+
+#include <boost/test/included/unit_test.hpp>
+
+#include <ChimeraTK/ControlSystemAdapter/Testing/ReferenceTestApplication.h>
+
+extern const char* object_name;
+#include <doocs-server-test-helper/ThreadedDoocsServer.h>
+#include <doocs-server-test-helper/doocsServerTestHelper.h>
+
+#include <eq_fct.h>
+
+#include "DoocsAdapter.h"
+#include "serverBasedTestTools.h"
+
+using namespace boost::unit_test_framework;
+using namespace ChimeraTK;
+
+DOOCS_ADAPTER_DEFAULT_FIXTURE_STATIC_APPLICATION
+
+std::string lastErrString = "(unset)";
+
+// basic check for error code handling, without associated message source
+BOOST_AUTO_TEST_CASE(testErrorNoMessageSource) {
+ auto check = [] {
+ auto location = getLocationFromPropertyAddress("//INT/FROM_DEVICE_SCALAR");
+ location->lock();
+ int code = location->get_error();
+ lastErrString = location->get_errorstr();
+ location->unlock();
+
+ return static_cast<eq_errors>(code);
+ };
+ // Testing initial state
+ GlobalFixture::referenceTestApplication.dataValidity = DataValidity::ok;
+ DoocsServerTestHelper::doocsSet<int>("//INT/TO_DEVICE_SCALAR", 0);
+
+ GlobalFixture::referenceTestApplication.runMainLoopOnce();
+ checkWithTimeout<eq_errors>(check, no_error);
+ BOOST_CHECK(lastErrString == "ok");
+
+ // Check fault state
+ GlobalFixture::referenceTestApplication.dataValidity = DataValidity::faulty;
+ DoocsServerTestHelper::doocsSet<int>("//INT/TO_DEVICE_SCALAR", 1);
+ GlobalFixture::referenceTestApplication.runMainLoopOnce();
+ checkWithTimeout<eq_errors>(check, not_available);
+ // there must be a generic error message
+ BOOST_CHECK(lastErrString != "ok");
+}
diff --git a/tests/src/testDoocsPVFactory.cpp b/tests/src/testDoocsPVFactory.cpp
index f4eec6f..dfe213c 100644
--- a/tests/src/testDoocsPVFactory.cpp
+++ b/tests/src/testDoocsPVFactory.cpp
@@ -3,7 +3,7 @@
// Only after defining the name include the unit test header.
#include <boost/mpl/list.hpp>
#include <boost/test/included/unit_test.hpp>
-#include <boost/test/test_case_template.hpp>
+#include <boost/test/unit_test.hpp>
#include <sstream>
#include <typeinfo>
diff --git a/tests/src/testDoocsProcessScalar.cpp b/tests/src/testDoocsProcessScalar.cpp
index 45e252d..62c3ada 100644
--- a/tests/src/testDoocsProcessScalar.cpp
+++ b/tests/src/testDoocsProcessScalar.cpp
@@ -2,7 +2,7 @@
// Only after defining the name include the unit test header.
#include <boost/mpl/list.hpp>
#include <boost/test/included/unit_test.hpp>
-#include <boost/test/test_case_template.hpp>
+#include <boost/test/unit_test.hpp>
#include "DoocsProcessScalar.h"
#include "DoocsUpdater.h"
diff --git a/tests/src/testDoocsSpectrum.cpp b/tests/src/testDoocsSpectrum.cpp
index 34f7c6d..e54ce86 100644
--- a/tests/src/testDoocsSpectrum.cpp
+++ b/tests/src/testDoocsSpectrum.cpp
@@ -2,7 +2,7 @@
// Only after defining the name include the unit test header.
#include <boost/mpl/list.hpp>
#include <boost/test/included/unit_test.hpp>
-#include <boost/test/test_case_template.hpp>
+#include <boost/test/unit_test.hpp>
#include "DoocsSpectrum.h"
#include <ChimeraTK/ControlSystemAdapter/ControlSystemPVManager.h>
--
GitLab