From 2b357e443065e3b1c23455585caf40c24a690fad Mon Sep 17 00:00:00 2001 From: Sujan Dumaru Date: Wed, 12 Aug 2026 22:19:02 -0500 Subject: [PATCH 1/3] test: validate object pool references and parser warnings, add negative fixtures --- .github/workflows/build.yml | 2 + CMakeLists.txt | 41 +++++++ tools/iop_validator.cpp | 184 ++++++++++++++++++++++++++++++ tools/testdata/malformed_pool.iop | Bin 0 -> 11949 bytes 4 files changed, 227 insertions(+) create mode 100644 tools/iop_validator.cpp create mode 100644 tools/testdata/malformed_pool.iop diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index de86ec9..e82617b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -55,6 +55,8 @@ jobs: run: cmake -S . -B build -G Ninja -DBUILD_EXAMPLES=OFF -DBUILD_TESTING=OFF -DCMAKE_BUILD_TYPE=Release -Wno-dev - name: Build run: cmake --build build --config Release + - name: Validate object pool + run: ctest --test-dir build --output-on-failure - name: Stage tarball run: | set -euo pipefail 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 0000000000000000000000000000000000000000..4196bfc1c0ee7053193f450a6f25c54ab2250c23 GIT binary patch literal 11949 zcmeHMTW}lI8UFv>)y0-0OSX|rkjX}b#I~HoMr4vg2-(<06&VQ1TFF`p^s4_JvZY`~7EkwY##N3$#o- zgEKmN&VM`q{hZwpQH1aA(n#A{B>a6mxQX^qr8ZOflg^QsI%9No*JavI?_m5Eus`HD z{bR6)67&hi1N2XfRr(a;%kyo!08|Ewgn)4M}G)IslIEK+hu zo)6RenE#l%F#i#KfcfvJ8}rNbA?8W$!Mu$>!u%)v7bPQ1kK{zg*D)TTZ{)6EJcaop zeG}vJRL1yA{;RX!t5|i=uQ2YX*K!|XyoB))dL83;>DPe&o_-?`enatH#`q%r7LXU| zT}Q???A}3lVjQFGLsu~F!2Ch($2iXefXvV!#vf1)W#%+=1AvkS{+b?&i>S#OjZkJimE>yIh1qkWEV zBPau7j9e&}+!5hj^cFPiCB)oaAnCfB3%IApzxa4Cc9?qU)3;9Y>MDn5{Ma$mL;}Ws zPA0K}QkbmCf;lmTjP!#$sI*wA%$WV>=69bRK%)AQQF?fK-0Yv9A3*NLUf~Bsm^f~R z{GOYeU#wQ_)Lo9GyBc zessJzZI-63>Ek6@bqKDU)IqR`N+(Ox<40XA_2g~QmLA|&12=6^0HYM9}bWfAnmigS9o-8rd&H+F^|mE zD&|<_vAJ?%ZlN|ndM*~{ZLZ)>df+=V)w$V*x$}T&9e=opY>18VW*2`8mC$d@;?Z%- zoLyd+H?1?1iecqtg?UMbkWbxPYKf zj3E{i( z_~TXyoD}UAIwJiCff*UuyDz`iHS}sAhCRMy-!1+Xzn9R)5vzmZWJ&l_WWlcxJ2o~s zwW)nmdkIm6pCz@$@d7%ruzb!uF*nmVi$Wg|h$6HFnnx|g=Yrj*Bd3l7BX-%#jf4a> z&<${w1PR6_O4gwx<6~}1z%k<|=%Cu}vWqiu`oKqM*ml?XzKE~eCe+14PLek1ubT{C zkO1ePUs<~p-e`%V_}W~Uovl|I^?S^R%^VJw%8+@|%q`EIIoqJi&2o~a)k%jIP|TBx zULt^#)xSp~;1IEtwhh54ZnbiZ#wM&nak4Z$FPIrGZh|uKW!$&5H<2di3 zlxjng%?YxOSjUc{KaNfoK`=T^{X%fc4P{C~S+4foaL@3=yTH?IE>6J&f-00akqtwqMsyA>adUl!A5_u9$HS7drX88*GWHjS{-x>WA-CU8s+7N zxm;NqppQh+y-tonqtUY!~j9JSpNQ^sGjF-biw< zMtYX}Qy4XhCS`RCcr@@bwI|t`_Xw573`l}8?}4Na2ipxAV7KZ?P8)e%!V@$ip-Z?- zth=>5>w2C?Fjo>41{{DbF-FCMU6Zz~B`wKyJ+BFVFXm}mUc%7^)R3hC54p_3t@d;9 zTfxx{3RuVIfs0_)4SLZ_V`!8Ry%=jqi7Hyb=~)HG-dY}>fOJmVFwDe)^AXt(astq$ z^`JT^vGbEk&!qUX6A^%qWjCG0K9MN?cv4v?9$$%jG@`dq>_v?BfqEnSKH$YqL2ikG zlJDp4gw<;QJns|zln3h25FYkh1YSTDh2Pd*a46pdF+P~W@(8@`_>CnJb_A6+_U;(F zWIuiaHruhnKDNG!Vpu4E>v!VW4~-Dbute0>S~w9!m3P>cyQ6s)F6_%@Qlg980!(6z zy}FRC;t@#Lt0xVtc^NeDQV4v&2;UAs5h30oRl%M7q4K@Zt#pH*&m;GH4P?GlRy(Ay zDf3V^nharn3t`+IPKL87)TDMHiYIFcVBBY#5?S0YwK!nz^H$U($ZmT0MbGh+Cv|wQVNDurk_HV*2kG%2eQLnTX+Y* zh@A3ijYZo^iv?l$0h(RJ9KJMM9im9fYLgN^7N5b>I?oCXFx7P^Bo^=s zC?t;SP_zkt-jNX9BHo0i@N+|Sh2NkFo)KGuYaML32!C%0E4#yv)nb?JB|qGSxHzRv zM0k`=*bT9|7=k@v+vlPwJkHgyCD9UpsHj1gFl^W=v8W0};|~0whiD0E6rJQuLFLD# zE*p-WICF<&S|Nfnh|^(?SSUc7Qj0Ke>3}7C5jJIn7C1yiqyTrQ9UKx9tWZJ7XT(5o z@LQDATCW{5cvC>zOfJHGA*!|EZ4A?=yG%M5>lFA3% zB73D-hu|H}GWv`(EQ+EJfm-j=WtACIIA;|dhb|f_PU;yl3I)iJm1a#~1{h`!Vjs3Z z0q%!TpGVZboH+`LB59dbI!oEF?q*unK8@w6&gzk`?q;|xeRVhc>TV`CrZ4qwHYn{# z{^3<|StlYhxta82hHw?8&GPysuR)9%#*Jn%T$c4bMu(2<5DoO+It~hrJU_+3&m3HS95ITaE7GMtHH^<-)5PulTr<{5WJ`cZ_=>U!>X+@sf@8qP-vZqgW}N zlAEyMYNyAsn}Y+zUdSI|2M?(trQVb(pAynMY;*f8R`r!ZoozQ|^7UmTpqCUr zetwt~E3Crk6$oaaf?XJcP?0EK9{f-(k9T#omsf(ADM$#keyHptu(b`zZZ%%f+pVaz zA<5d6?KLFqFC}c-Am1QJAd-;GMiQx}E9>%@G0MQf$jnf2GmtVK6p~~GS~R%kTt!() z1TAn_LvCBXITwg0`9DD6?Y*HM>*%lS<(OFQv(@&H-ReBw+*o7rjwxTM1saVi#9nw} ziC5qS)oPlhjhew%9AM-Lo6s2SW}zt1j5fmIwaJkO$w96J%6}t?3DAnv^>&l^Y>boU zB>Fq;%hu9obD+Hp`P$1}t?dPlwG-z$C;ZI@zzc-daMz8@erM+2K65#=uJ(EZsZ;Oz9}P`Fv{E2TjDK%qfD{NxT| zdqLeW>=x3*%I(GVQG|P2u;0elwk5}d_~z1rJrCS@2fLOxf|6IQ9rAIrY2l6H-ehAF zz8erUMKil0xXyYEWHQ?j~*L5JW*Hwg6?djbZz_kHJXha)_=c>nZO-$V7!PEB|APoU}D``TZ8iWei zH8EnV;t`tzO=JU%(1&y^gBAEf=;f6gNBy7kh z(mk1&*>?riw4kxFLP=_|L97vKpM^K$9q5Y4D%(B3$ZVq+bhP?6@%bQYb82nwNfshv zzWhY?wq&fzXJ1wa4@$R%cx}a3TrJH+eJgofz23>UkR>z#=j#Q^Qtk=E_+El|AUBt| z4hj!DqM&vg!c8rv8UwiS>LND Date: Thu, 13 Aug 2026 06:16:23 +0000 Subject: [PATCH 2/3] ci: extract IOP validation into reusable validate-iop.yml workflow Co-authored-by: gunicsba <3919203+gunicsba@users.noreply.github.com> --- .github/workflows/build.yml | 27 +++++++++++++++--- .github/workflows/validate-iop.yml | 45 ++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 4 deletions(-) create mode 100644 .github/workflows/validate-iop.yml diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e82617b..7b886b7 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -52,11 +52,9 @@ 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: Validate object pool - run: ctest --test-dir build --output-on-failure - name: Stage tarball run: | set -euo pipefail @@ -71,4 +69,25 @@ 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 + + validate_iop_linux_x86_64: + name: Validate IOP (Linux x86_64) + uses: ./.github/workflows/validate-iop.yml + with: + os: ubuntu-latest + arch: x86_64 + + validate_iop_linux_aarch64: + name: Validate IOP (Linux aarch64) + uses: ./.github/workflows/validate-iop.yml + with: + os: ubuntu-24.04-arm + arch: aarch64 + + validate_iop_windows: + name: Validate IOP (Windows) + uses: ./.github/workflows/validate-iop.yml + with: + os: windows-latest + arch: win64 diff --git a/.github/workflows/validate-iop.yml b/.github/workflows/validate-iop.yml new file mode 100644 index 0000000..4fd07e5 --- /dev/null +++ b/.github/workflows/validate-iop.yml @@ -0,0 +1,45 @@ +name: Validate IOP + +on: + workflow_call: + inputs: + os: + description: 'Runner OS label (e.g. ubuntu-latest, windows-latest)' + required: true + type: string + arch: + description: 'Architecture label used in job names (e.g. x86_64, aarch64, win64)' + required: true + type: string + +jobs: + validate: + name: Validate IOP (${{ inputs.arch }}) + runs-on: ${{ inputs.os }} + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + submodules: recursive + + - name: Install build deps (Linux) + if: runner.os == 'Linux' + 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 + -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 From eadfb99b948724789933bfa97558eed2ded5be6a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 13 Aug 2026 06:23:44 +0000 Subject: [PATCH 3/3] ci: make IOP validation a single platform-independent workflow like linting Co-authored-by: gunicsba <3919203+gunicsba@users.noreply.github.com> --- .github/workflows/build.yml | 23 +---------------------- .github/workflows/validate-iop.yml | 27 +++++++++++---------------- 2 files changed, 12 insertions(+), 38 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7b886b7..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 @@ -70,24 +70,3 @@ jobs: with: name: 'Linux Tarball (${{ matrix.arch }})' path: AOG-TaskController-linux-${{ matrix.arch }}.tar.gz - - validate_iop_linux_x86_64: - name: Validate IOP (Linux x86_64) - uses: ./.github/workflows/validate-iop.yml - with: - os: ubuntu-latest - arch: x86_64 - - validate_iop_linux_aarch64: - name: Validate IOP (Linux aarch64) - uses: ./.github/workflows/validate-iop.yml - with: - os: ubuntu-24.04-arm - arch: aarch64 - - validate_iop_windows: - name: Validate IOP (Windows) - uses: ./.github/workflows/validate-iop.yml - with: - os: windows-latest - arch: win64 diff --git a/.github/workflows/validate-iop.yml b/.github/workflows/validate-iop.yml index 4fd07e5..13fd36f 100644 --- a/.github/workflows/validate-iop.yml +++ b/.github/workflows/validate-iop.yml @@ -1,29 +1,24 @@ name: Validate IOP on: - workflow_call: - inputs: - os: - description: 'Runner OS label (e.g. ubuntu-latest, windows-latest)' - required: true - type: string - arch: - description: 'Architecture label used in job names (e.g. x86_64, aarch64, win64)' - required: true - type: string + push: + branches: + - main + - develop + pull_request: + types: [opened, synchronize, reopened] jobs: - validate: - name: Validate IOP (${{ inputs.arch }}) - runs-on: ${{ inputs.os }} + validate_iop: + name: Validate object pool + runs-on: ubuntu-latest steps: - name: Checkout uses: actions/checkout@v4 with: submodules: recursive - - name: Install build deps (Linux) - if: runner.os == 'Linux' + - name: Install build deps run: | sudo apt-get update sudo apt-get install -y --no-install-recommends \ @@ -31,7 +26,7 @@ jobs: - name: Configure (CMake) run: > - cmake -S . -B build + cmake -S . -B build -G Ninja -DBUILD_EXAMPLES=OFF -DBUILD_TESTING=OFF -DAOG_TC_VALIDATE_IOP=ON