diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index de86ec9..54b4250 100644 --- a/.github/workflows/build.yml +++ b/.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 \ No newline at end of file + path: AOG-TaskController-linux-${{ matrix.arch }}.tar.gz diff --git a/.github/workflows/validate-iop.yml b/.github/workflows/validate-iop.yml new file mode 100644 index 0000000..13fd36f --- /dev/null +++ b/.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 diff --git a/CMakeLists.txt b/CMakeLists.txt index 44eb3f4..263757e 100644 --- a/CMakeLists.txt +++ b/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} diff --git a/tools/iop_validator.cpp b/tools/iop_validator.cpp new file mode 100644 index 0000000..04a5848 --- /dev/null +++ b/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 +#include +#include +#include +#include + +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(data.size())); + } + }; + + std::vector 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> &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 + void check_min_max(std::uint16_t objectID, const char *label, const std::shared_ptr &object) + { + auto typed = std::static_pointer_cast(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 \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(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(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(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(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(entry.first, "OutputMeter", entry.second); + break; + + case isobus::VirtualTerminalObjectType::OutputLinearBarGraph: + check_min_max(entry.first, "OutputLinearBarGraph", entry.second); + break; + + case isobus::VirtualTerminalObjectType::OutputArchedBarGraph: + check_min_max(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; +} diff --git a/tools/testdata/malformed_pool.iop b/tools/testdata/malformed_pool.iop new file mode 100644 index 0000000..4196bfc Binary files /dev/null and b/tools/testdata/malformed_pool.iop differ