Merge pull request #576 from mtconnect/575_fix_quality_for_samples

wsobel · web-flow · commit aab008074bbb · 2025-11-11T19:19:07.000+01:00
Fixed validation for samples
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -2,7 +2,7 @@
 set(AGENT_VERSION_MAJOR 2)
 set(AGENT_VERSION_MINOR 6)
 set(AGENT_VERSION_PATCH 0)
-set(AGENT_VERSION_BUILD 5)
+set(AGENT_VERSION_BUILD 6)
 set(AGENT_VERSION_RC "")
 
 # This minimum version is to support Visual Studio 2019 and C++ feature checking and FetchContent
diff --git a/src/mtconnect/configuration/agent_config.cpp b/src/mtconnect/configuration/agent_config.cpp
@@ -940,6 +940,8 @@ namespace mtconnect::configuration {
 
     loadAdapters(config, options);
 
+    m_afterAgentHooks.exec(*this);
+    
 #ifdef WITH_PYTHON
     configurePython(config, options);
 #endif
diff --git a/src/mtconnect/configuration/agent_config.hpp b/src/mtconnect/configuration/agent_config.hpp
@@ -93,6 +93,9 @@ namespace mtconnect {
       /// @brief Get the callback manager after the agent is created
       /// @return the callback manager
       auto &afterAgentHooks() { return m_afterAgentHooks; }
+      /// @brief Get the callback manager after the config has completed
+      /// @return the callback manager
+      auto &afterConfigHooks() { return m_afterConfigHooks; }
       /// @brief Get the callback manager after the agent is started
       /// @return the callback manager
       auto &beforeStartHooks() { return m_beforeStartHooks; }
@@ -402,6 +405,7 @@ namespace mtconnect {
 #endif
 
       HookManager<AgentConfiguration> m_afterAgentHooks;
+      HookManager<AgentConfiguration> m_afterConfigHooks;
       HookManager<AgentConfiguration> m_beforeStartHooks;
       HookManager<AgentConfiguration> m_beforeStopHooks;
     };
diff --git a/src/mtconnect/device_model/device.cpp b/src/mtconnect/device_model/device.cpp
@@ -84,7 +84,7 @@ namespace mtconnect {
       {
         if (auto it = m_dataItems.get<ByName>().find(*name); it != m_dataItems.get<ByName>().end())
         {
-          LOG(warning) << "Device " << getName() << ": Duplicate source '" << *name
+          LOG(warning) << "Device " << getName() << ": Duplicate name '" << *name
                        << "' found in data item '" << di->getId() << "'. Previous data item: '"
                        << it->lock()->getId() << '\'';
           LOG(warning) << "    Name '" << *name
diff --git a/src/mtconnect/entity/entity.hpp b/src/mtconnect/entity/entity.hpp
@@ -716,5 +716,77 @@ namespace mtconnect {
 
       return changed;
     }
+    
+    /// @brief variant visitor to output Value to stream
+    struct StreamOutputVisitor
+    {
+      /// @brief constructor
+      /// @param os the output stream
+      StreamOutputVisitor(std::ostream& os) : m_os(os) {}
+      
+      void operator()(const std::monostate&) {
+        m_os << "null";
+      }
+      
+      void operator()(const EntityPtr& entity) {
+        const auto &id = entity->getIdentity();
+        m_os << "Entity(" << entity->getName() << ":";
+        StreamOutputVisitor visitor(m_os);
+        std::visit(visitor, id);
+        m_os << ")";
+      }
+      
+      void operator()(const EntityList& list) {
+        m_os << "EntityList[";
+        for (auto e : list) {
+          StreamOutputVisitor visitor(m_os);
+          visitor(e);
+          m_os << " ";
+        }
+        m_os << "]";
+      }
+      
+      void operator()(const DataSet& dataSet) {
+        m_os << "DataSet(" << dataSet.size() << " items)";
+      }
+      
+      void operator()(const QName& qname) {
+        m_os << qname.str();
+      }
+      
+      void operator()(const Vector& vec) {
+        m_os << "Vector[";
+        for (const auto &v : vec) {
+          m_os << v << " ";
+        }
+        m_os << "]";
+      }
+      
+      template <typename T>
+      void operator()(const T& value) {
+        m_os << value;
+      }
+      
+      std::ostream& m_os;
+    };
+    
+    /// @brief output operator for Value
+    /// @param os the output stream
+    /// @param v the Value to output
+    inline std::ostream& operator<<(std::ostream& os, const Value &v) {
+      StreamOutputVisitor visitor(os);
+      std::visit(visitor, v);
+      return os;
+    }
+    
+    /// @brief output operator for Value
+    /// @param os the output stream
+    /// @param v the Value to output
+    inline std::ostream& operator<<(std::ostream& os, const EntityPtr &v) {
+      StreamOutputVisitor visitor(os);
+      visitor(v);
+      return os;
+    }
+
   }  // namespace entity
 }  // namespace mtconnect
diff --git a/src/mtconnect/pipeline/validator.hpp b/src/mtconnect/pipeline/validator.hpp
@@ -37,11 +37,11 @@ namespace mtconnect::pipeline {
   public:
     Validator(const Validator &) = default;
     Validator(PipelineContextPtr context)
-      : Transform("Validator"), m_contract(context->m_contract.get())
+    : Transform("Validator"), m_contract(context->m_contract.get())
     {
       m_guard = TypeGuard<observation::Observation>(RUN) || TypeGuard<entity::Entity>(SKIP);
     }
-
+    
     /// @brief validate the Event
     /// @param entity The Event entity
     /// @returns modified entity with quality and deprecated properties
@@ -50,78 +50,78 @@ namespace mtconnect::pipeline {
       using namespace observation;
       using namespace mtconnect::validation::observations;
       auto obs = std::dynamic_pointer_cast<Observation>(entity);
-
+      auto &value = obs->getValue();
+      
+      bool valid = true;
       auto di = obs->getDataItem();
-      if (obs->isUnavailable() || di->isDataSet())
-      {
-        obs->setProperty("quality", std::string("VALID"));
-      }
-      else if (auto evt = std::dynamic_pointer_cast<observation::Event>(obs))
+      if (!obs->isUnavailable() && !di->isDataSet())
       {
-        auto &value = evt->getValue<std::string>();
-
-        // Optimize
-        auto vocab = ControlledVocabularies.find(evt->getName());
-        if (vocab != ControlledVocabularies.end())
+        if (auto evt = std::dynamic_pointer_cast<observation::Event>(obs))
         {
-          auto &lits = vocab->second;
-          if (lits.size() != 0)
+          auto vocab = ControlledVocabularies.find(evt->getName());
+          if (vocab != ControlledVocabularies.end())
           {
-            auto lit = lits.find(value);
-            if (lit != lits.end())
+            auto sv = std::get_if<std::string>(&value);
+            auto &lits = vocab->second;
+            if (lits.size() != 0 && sv != nullptr)
             {
-              // Check if it has not been introduced yet
-              if (lit->second.first > 0 && m_contract->getSchemaVersion() < lit->second.first)
+              auto lit = lits.find(*sv);
+              if (lit != lits.end())
               {
-                evt->setProperty("quality", std::string("INVALID"));
+                // Check if it has not been introduced yet
+                if (lit->second.first > 0 && m_contract->getSchemaVersion() < lit->second.first)
+                  valid = false;
+                
+                // Check if deprecated
+                if (lit->second.second > 0 && m_contract->getSchemaVersion() >= lit->second.second)
+                {
+                  evt->setProperty("deprecated", true);
+                }
               }
               else
               {
-                evt->setProperty("quality", std::string("VALID"));
-              }
-
-              // Check if deprecated
-              if (lit->second.second > 0 && m_contract->getSchemaVersion() >= lit->second.second)
-              {
-                evt->setProperty("deprecated", true);
+                valid = false;
               }
             }
-            else
+            else if (lits.size() != 0)
             {
-              evt->setProperty("quality", std::string("INVALID"));
-              // Log once
-              auto &id = di->getId();
-              if (m_logOnce.count(id) < 1)
-              {
-                LOG(warning) << "DataItem '" << id << "': Invalid value for '" << evt->getName()
-                             << "': '" << evt->getValue<std::string>() << '\'';
-                m_logOnce.insert(id);
-              }
-              else
-              {
-                LOG(trace) << "DataItem '" << id << "': Invalid value for '" << evt->getName()
-                           << "': '" << evt->getValue<std::string>() << '\'';
-              }
+              valid = false;
             }
           }
           else
           {
-            evt->setProperty("quality", std::string("VALID"));
+            evt->setProperty("quality", std::string("UNVERIFIABLE"));
           }
         }
-        else
+        else if (auto spl = std::dynamic_pointer_cast<observation::Sample>(obs))
         {
-          evt->setProperty("quality", std::string("UNVERIFIABLE"));
+          if (!(spl->hasProperty("quality") || std::holds_alternative<double>(value) ||
+              std::holds_alternative<int64_t>(value)))
+            valid = false;
         }
       }
-      else if (auto spl = std::dynamic_pointer_cast<observation::Sample>(obs))
+    
+      if (!valid)
       {
+        obs->setProperty("quality", std::string("INVALID"));
+        // Log once
+        auto &id = di->getId();
+        if (m_logOnce.count(id) < 1)
+        {
+          LOG(warning) << "DataItem '" << id << "': Invalid value for '" << obs->getName()
+            << "': '" << value << '\'';
+          m_logOnce.insert(id);
+        }
+        else
+        {
+          LOG(trace) << "DataItem '" << id << "': Invalid value for '" << obs->getName();
+        }
       }
-      else
+      else if (!obs->hasProperty("quality"))
       {
         obs->setProperty("quality", std::string("VALID"));
       }
-
+      
       return next(std::move(obs));
     }
 
diff --git a/test_package/observation_validation_test.cpp b/test_package/observation_validation_test.cpp
@@ -246,7 +246,7 @@ TEST_F(ObservationValidationTest, should_be_invalid_if_entry_has_not_been_introd
   ASSERT_FALSE(evt->hasProperty("deprecated"));
 }
 
-TEST_F(ObservationValidationTest, should_validate_data_item_types)
+TEST_F(ObservationValidationTest, should_validate_invalid_sample_value)
 {
   auto contract = static_cast<MockPipelineContract *>(m_context->m_contract.get());
   contract->m_schemaVersion = SCHEMA_VERSION(2, 5);
@@ -284,6 +284,59 @@ TEST_F(ObservationValidationTest, should_validate_data_item_types)
   }
 }
 
+TEST_F(ObservationValidationTest, should_validate_sample)
+{
+  auto contract = static_cast<MockPipelineContract *>(m_context->m_contract.get());
+  contract->m_schemaVersion = SCHEMA_VERSION(2, 5);
+  
+  shared_ptr<ShdrTokenMapper> mapper;
+  mapper = make_shared<ShdrTokenMapper>(m_context, "", 2);
+  mapper->bind(m_validator);
+  
+  ErrorList errors;
+  m_dataItem = DataItem::make(
+                              {{"id", "pos"s}, {"category", "SAMPLE"s}, {"type", "POSITION"s}, {"units", "MILLIMETER"s}},
+                              errors);
+  
+  auto ts = make_shared<Timestamped>();
+  ts->m_tokens = {{"pos"s, "1.234"s}};
+  ts->m_timestamp = chrono::system_clock::now();
+  ts->setProperty("timestamp", ts->m_timestamp);
+  
+  auto observations = (*mapper)(ts);
+  auto &r = *observations;
+  ASSERT_EQ(typeid(Observations), typeid(r));
+  
+  auto oblist = observations->getValue<EntityList>();
+  ASSERT_EQ(1, oblist.size());
+  
+  auto oi = oblist.begin();
+  
+  {
+    auto sample = dynamic_pointer_cast<Sample>(*oi++);
+    ASSERT_TRUE(sample);
+    ASSERT_EQ(m_dataItem, sample->getDataItem());
+    ASSERT_FALSE(sample->isUnavailable());
+    
+    ASSERT_EQ("VALID", sample->get<string>("quality"));
+  }
+}
+
+TEST_F(ObservationValidationTest, should_validate_sample_with_int64_value)
+{
+  ErrorList errors;
+  m_dataItem = DataItem::make(
+                              {{"id", "pos"s}, {"category", "SAMPLE"s}, {"type", "POSITION"s}, {"units", "MILLIMETER"s}},
+                              errors);
+  
+  auto obs = Observation::make(m_dataItem, {{"VALUE", int64_t(100)}}, m_time, errors);
+  
+  auto evt = (*m_validator)(std::move(obs));
+  auto quality = evt->get<string>("quality");
+  ASSERT_EQ("VALID", quality);
+}
+
+
 TEST_F(ObservationValidationTest, should_not_validate_if_validation_is_off)
 {
   auto contract = static_cast<MockPipelineContract *>(m_context->m_contract.get());
@@ -369,3 +422,14 @@ TEST_F(ObservationValidationTest, should_validate_json_data_item_types)
     ASSERT_EQ("INVALID", sample->get<string>("quality"));
   }
 }
+
+/// @test Validate a sample
+TEST_F(ObservationValidationTest, should_validate_sample_double_value)
+{
+  ErrorList errors;
+  auto event = Observation::make(m_dataItem, {{"VALUE", "READY"s}}, m_time, errors);
+  
+  auto evt = (*m_validator)(std::move(event));
+  auto quality = evt->get<string>("quality");
+  ASSERT_EQ("VALID", quality);
+}

Original file line number	Diff line number	Diff line change
`@@ -84,7 +84,7 @@ namespace mtconnect {`
`84`	`84`	`{`
`85`	`85`	`if (auto it = m_dataItems.get<ByName>().find(*name); it != m_dataItems.get<ByName>().end())`
`86`	`86`	`{`
`87`		`- LOG(warning) << "Device " << getName() << ": Duplicate source '" << *name`
	`87`	`+ LOG(warning) << "Device " << getName() << ": Duplicate name '" << *name`
`88`	`88`	`<< "' found in data item '" << di->getId() << "'. Previous data item: '"`
`89`	`89`	`<< it->lock()->getId() << '\'';`
`90`	`90`	`LOG(warning) << " Name '" << *name`