Test iop validation during CI build on AOG-TaskController

.github/workflows/build.yml

@@ -22,7 +22,7 @@ jobs:
       - name: Compile
         run: |
           mkdir build
-          cmake -S . -B build -DBUILD_EXAMPLES=OFF -DBUILD_TESTING=OFF -Wno-dev
+          cmake -S . -B build -DBUILD_EXAMPLES=OFF -DBUILD_TESTING=OFF -DAOG_TC_VALIDATE_IOP=OFF -Wno-dev
           cmake --build build --config Release --target package
       - name: 'Upload Windows Installer'
         uses: actions/upload-artifact@v4
@@ -52,7 +52,7 @@ jobs:
           sudo apt-get install -y --no-install-recommends \
             build-essential cmake ninja-build
       - name: Configure (CMake)
-        run: cmake -S . -B build -G Ninja -DBUILD_EXAMPLES=OFF -DBUILD_TESTING=OFF -DCMAKE_BUILD_TYPE=Release -Wno-dev
+        run: cmake -S . -B build -G Ninja -DBUILD_EXAMPLES=OFF -DBUILD_TESTING=OFF -DAOG_TC_VALIDATE_IOP=OFF -DCMAKE_BUILD_TYPE=Release -Wno-dev
       - name: Build
         run: cmake --build build --config Release
       - name: Stage tarball
@@ -69,4 +69,4 @@ jobs:
         uses: actions/upload-artifact@v4
         with:
           name: 'Linux Tarball (${{ matrix.arch }})'
-          path: AOG-TaskController-linux-${{ matrix.arch }}.tar.gz
+          path: AOG-TaskController-linux-${{ matrix.arch }}.tar.gz

.github/workflows/validate-iop.yml

@@ -0,0 +1,40 @@
+name: Validate IOP
+
+on:
+  push:
+    branches:
+      - main
+      - develop
+  pull_request:
+    types: [opened, synchronize, reopened]
+
+jobs:
+  validate_iop:
+    name: Validate object pool
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+        with:
+          submodules: recursive
+
+      - name: Install build deps
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y --no-install-recommends \
+            build-essential cmake ninja-build
+
+      - name: Configure (CMake)
+        run: >
+          cmake -S . -B build -G Ninja
+          -DBUILD_EXAMPLES=OFF
+          -DBUILD_TESTING=OFF
+          -DAOG_TC_VALIDATE_IOP=ON
+          -DCMAKE_BUILD_TYPE=Release
+          -Wno-dev
+
+      - name: Build
+        run: cmake --build build --config Release --target iop_validator
+
+      - name: Validate object pool
+        run: ctest --test-dir build --output-on-failure

CMakeLists.txt

@@ -133,6 +133,47 @@ target_link_libraries(
 
 install(TARGETS ${PROJECT_NAME} RUNTIME DESTINATION bin COMPONENT applications)
 
+option(AOG_TC_VALIDATE_IOP "Build and register the object pool validator test"
+       ON)
+if(AOG_TC_VALIDATE_IOP)
+  # BUILD_TESTING stays off so FetchContent dependencies do not add their suites
+  enable_testing()
+  add_executable(iop_validator
+                 ${CMAKE_CURRENT_LIST_DIR}/tools/iop_validator.cpp)
+  target_compile_features(iop_validator PRIVATE cxx_std_20)
+  set_target_properties(iop_validator PROPERTIES CXX_EXTENSIONS OFF)
+  target_link_libraries(iop_validator PRIVATE isobus::Isobus isobus::Utility)
+  add_test(NAME object_pool_attributes_in_range COMMAND iop_validator
+                                                        "${AOG_TC_IOP_SOURCE}")
+
+  # A deliberately broken pool, so the suite asserts more than "today's pool
+  # passes". One case per message, since a shared exit code stays green as long
+  # as any one check still fires.
+  set(AOG_TC_MALFORMED_IOP
+      ${CMAKE_CURRENT_LIST_DIR}/tools/testdata/malformed_pool.iop)
+  add_test(NAME malformed_pool_coerced_attribute
+           COMMAND iop_validator "${AOG_TC_MALFORMED_IOP}")
+  set_tests_properties(
+    malformed_pool_coerced_attribute
+    PROPERTIES PASS_REGULAR_EXPRESSION "format byte has undefined value")
+  add_test(NAME malformed_pool_dangling_reference
+           COMMAND iop_validator "${AOG_TC_MALFORMED_IOP}")
+  set_tests_properties(
+    malformed_pool_dangling_reference
+    PROPERTIES PASS_REGULAR_EXPRESSION "active mask references object 4660")
+  add_test(NAME malformed_pool_value_out_of_range
+           COMMAND iop_validator "${AOG_TC_MALFORMED_IOP}")
+  set_tests_properties(
+    malformed_pool_value_out_of_range PROPERTIES PASS_REGULAR_EXPRESSION
+                                                 "InputBoolean value 24")
+
+  # PASS_REGULAR_EXPRESSION makes CTest ignore the exit code, which is the part
+  # a CI gate actually reads.
+  add_test(NAME malformed_pool_exit_status COMMAND iop_validator
+                                                   "${AOG_TC_MALFORMED_IOP}")
+  set_tests_properties(malformed_pool_exit_status PROPERTIES WILL_FAIL TRUE)
+endif()
+
 if(WIN32)
   add_custom_command(
     TARGET ${PROJECT_NAME}

tools/iop_validator.cpp

@@ -0,0 +1,184 @@
+#include "isobus/isobus/can_constants.hpp"
+#include "isobus/isobus/can_stack_logger.hpp"
+#include "isobus/isobus/isobus_virtual_terminal_objects.hpp"
+#include "isobus/isobus/isobus_virtual_terminal_working_set_base.hpp"
+#include "isobus/utility/iop_file_interface.hpp"
+
+#include <cstdio>
+#include <map>
+#include <memory>
+#include <string>
+#include <vector>
+
+namespace
+{
+	class ObjectPool : public isobus::VirtualTerminalWorkingSetBase
+	{
+	public:
+		bool load(const std::string &path)
+		{
+			auto data = isobus::IOPFileInterface::read_iop_file(path);
+			return !data.empty() && parse_iop_into_objects(data.data(), static_cast<std::uint32_t>(data.size()));
+		}
+	};
+
+	std::vector<std::string> violations;
+
+	void flag(std::uint16_t objectID, const std::string &reason)
+	{
+		violations.push_back("object " + std::to_string(objectID) + ": " + reason);
+	}
+
+	// The parser repairs malformed attributes as it reads them — an out-of-range OutputNumber format
+	// byte just becomes exponential — so the finished object looks clean and its log holds the only
+	// evidence. Caveat: all warnings are fatal here, including two routine Auxiliary Type 1 notices.
+	class FailOnParserComplaint : public isobus::CANStackLogger
+	{
+	public:
+		void sink_CAN_stack_log(LoggingLevel level, const std::string &logText) override
+		{
+			if (level >= LoggingLevel::Warning)
+			{
+				violations.push_back("parser: " + logText);
+			}
+		}
+	};
+
+	// get_is_valid() skips child IDs that resolve to nothing instead of rejecting them, and never
+	// examines a WorkingSet's active mask, so those dangling references reach the VT unreported.
+	void check_reference(std::uint16_t objectID,
+	                     const char *label,
+	                     std::uint16_t referencedID,
+	                     const std::map<std::uint16_t, std::shared_ptr<isobus::VTObject>> &tree)
+	{
+		if ((isobus::NULL_OBJECT_ID != referencedID) && (0 == tree.count(referencedID)))
+		{
+			flag(objectID,
+			     std::string(label) + " references object " + std::to_string(referencedID) +
+			       " which is not in the pool");
+		}
+	}
+
+	template<typename T>
+	void check_min_max(std::uint16_t objectID, const char *label, const std::shared_ptr<isobus::VTObject> &object)
+	{
+		auto typed = std::static_pointer_cast<T>(object);
+		if ((typed->get_value() < typed->get_min_value()) || (typed->get_value() > typed->get_max_value()))
+		{
+			flag(objectID,
+			     std::string(label) + " value " + std::to_string(typed->get_value()) + " outside [" +
+			       std::to_string(typed->get_min_value()) + ", " + std::to_string(typed->get_max_value()) + "]");
+		}
+	}
+}
+
+int main(int argc, char **argv)
+{
+	if (2 != argc)
+	{
+		std::fprintf(stderr, "usage: iop_validator <path to .iop>\n");
+		return 2;
+	}
+
+	FailOnParserComplaint parserLog;
+	isobus::CANStackLogger::set_can_stack_logger_sink(&parserLog);
+
+	ObjectPool pool;
+	if (!pool.load(argv[1]))
+	{
+		std::fprintf(stderr, "FAIL: could not read or parse %s\n", argv[1]);
+		return 1;
+	}
+
+	const auto &tree = pool.get_object_tree();
+	for (const auto &entry : tree)
+	{
+		if (nullptr == entry.second)
+		{
+			continue;
+		}
+
+		if (!entry.second->get_is_valid(tree))
+		{
+			flag(entry.first, "failed object pool structural validation");
+		}
+
+		for (std::uint16_t i = 0; i < entry.second->get_number_children(); i++)
+		{
+			check_reference(entry.first, "child", entry.second->get_child_id(i), tree);
+		}
+
+		if (auto workingSet = std::dynamic_pointer_cast<isobus::WorkingSet>(entry.second))
+		{
+			check_reference(entry.first, "active mask", workingSet->get_active_mask(), tree);
+		}
+
+		switch (entry.second->get_object_type())
+		{
+			case isobus::VirtualTerminalObjectType::InputBoolean:
+			{
+				auto typed = std::static_pointer_cast<isobus::InputBoolean>(entry.second);
+				if (typed->get_value() > 1)
+				{
+					flag(entry.first, "InputBoolean value " + std::to_string(typed->get_value()) + " is not 0 or 1");
+				}
+			}
+			break;
+
+			case isobus::VirtualTerminalObjectType::InputList:
+			case isobus::VirtualTerminalObjectType::OutputList:
+			{
+				auto typed = std::static_pointer_cast<isobus::ListVTObject>(entry.second);
+				const auto itemCount = typed->get_number_children();
+				if ((0 != itemCount) && (0xFF != typed->get_value()) && (typed->get_value() >= itemCount))
+				{
+					flag(entry.first,
+					     "list value " + std::to_string(typed->get_value()) + " selects item beyond the " +
+					       std::to_string(itemCount) + " present");
+				}
+			}
+			break;
+
+			case isobus::VirtualTerminalObjectType::InputNumber:
+			{
+				auto typed = std::static_pointer_cast<isobus::InputNumber>(entry.second);
+				if ((typed->get_value() < typed->get_minimum_value()) || (typed->get_value() > typed->get_maximum_value()))
+				{
+					flag(entry.first,
+					     "InputNumber value " + std::to_string(typed->get_value()) + " outside [" +
+					       std::to_string(typed->get_minimum_value()) + ", " +
+					       std::to_string(typed->get_maximum_value()) + "]");
+				}
+			}
+			break;
+
+			case isobus::VirtualTerminalObjectType::OutputMeter:
+				check_min_max<isobus::OutputMeter>(entry.first, "OutputMeter", entry.second);
+				break;
+
+			case isobus::VirtualTerminalObjectType::OutputLinearBarGraph:
+				check_min_max<isobus::OutputLinearBarGraph>(entry.first, "OutputLinearBarGraph", entry.second);
+				break;
+
+			case isobus::VirtualTerminalObjectType::OutputArchedBarGraph:
+				check_min_max<isobus::OutputArchedBarGraph>(entry.first, "OutputArchedBarGraph", entry.second);
+				break;
+
+			default:
+				break;
+		}
+	}
+
+	if (!violations.empty())
+	{
+		std::fprintf(stderr, "FAIL: %s\n", argv[1]);
+		for (const auto &violation : violations)
+		{
+			std::fprintf(stderr, "  %s\n", violation.c_str());
+		}
+		return 1;
+	}
+
+	std::printf("OK: %s, %zu objects\n", argv[1], tree.size());
+	return 0;
+}