From da6bf703fcb2da0f45714bb584a71a38abee6ba0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gunics=20Bal=C3=A1zs?= Date: Tue, 13 Oct 2026 22:48:37 +0200 Subject: [PATCH 1/3] Bump AgIsoStack++ to upstream main and patch in PR #718 Moves the pin from c0e1ebb (19 Sep) to f3c37b8 (13 Oct). For us this brings the VT client fixes for a slow pool parse (#774) and a VT that stops sending its status (#719), working Extended Version commands (#768), and the CAN driver being closed on stop with a reported reason when it fails to open (#722, #747, #759). AgIsoStack++ PR #718 is applied on top as a patch until it is merged: a control function restored to the address table gets credit for the claim that restored it, and the roll-call no longer credits the first inactive CF on the channel. Without it a CF can be pruned on every address claim request and its broadcasts dropped meanwhile. ApplyPatchOnce.cmake skips an already applied patch, so a rerun of the FetchContent patch step does not fail, and the patch is kept LF. Since #747, CANHardwareInterface::start() can return false after it started the update thread, so Application::stop() now checks is_running() instead of the start() result. The CAN start failure log includes the driver's get_last_error(). Co-Authored-By: Claude Opus 5.5 --- CMakeLists.txt | 12 +++++-- cmake/ApplyPatchOnce.cmake | 22 ++++++++++++ cmake/patches/.gitattributes | 2 ++ ...stack-pr718-restored-cf-claim-credit.patch | 34 +++++++++++++++++++ include/app.hpp | 1 - src/app.cpp | 10 +++--- 6 files changed, 74 insertions(+), 7 deletions(-) create mode 100644 cmake/ApplyPatchOnce.cmake create mode 100644 cmake/patches/.gitattributes create mode 100644 cmake/patches/agisostack-pr718-restored-cf-claim-credit.patch diff --git a/CMakeLists.txt b/CMakeLists.txt index b49f5af..3e3fa0a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -59,11 +59,19 @@ FetchContent_Declare( USES_TERMINAL_DOWNLOAD TRUE DOWNLOAD_EXTRACT_TIMESTAMP TRUE) FetchContent_MakeAvailable(Boost) +# Upstream main plus AgIsoStack++ PR #718 (a control function restored to the +# address table is pruned again on the next address claim request). Drop the +# patch once that PR is merged. +find_package(Git REQUIRED) FetchContent_Declare( isobus GIT_REPOSITORY https://github.com/Open-Agriculture/AgIsoStack-plus-plus.git - GIT_TAG c0e1ebb06a2ad9d5bf86c98b3d028ab8fa350130 - DOWNLOAD_EXTRACT_TIMESTAMP TRUE) + GIT_TAG f3c37b84d017df2228cb00e4366a5f7972da1176 + PATCH_COMMAND + ${CMAKE_COMMAND} -DGIT_EXECUTABLE=${GIT_EXECUTABLE} + -DPATCH_FILE=${CMAKE_CURRENT_SOURCE_DIR}/cmake/patches/agisostack-pr718-restored-cf-claim-credit.patch + -P ${CMAKE_CURRENT_SOURCE_DIR}/cmake/ApplyPatchOnce.cmake + DOWNLOAD_EXTRACT_TIMESTAMP TRUE) FetchContent_MakeAvailable(isobus) FetchContent_Declare( diff --git a/cmake/ApplyPatchOnce.cmake b/cmake/ApplyPatchOnce.cmake new file mode 100644 index 0000000..82b3e47 --- /dev/null +++ b/cmake/ApplyPatchOnce.cmake @@ -0,0 +1,22 @@ +# Applies PATCH_FILE to the current directory with git, unless it is already +# applied. FetchContent can run the patch step again on a source tree it already +# patched, which a plain `git apply` would fail on. +# +# Usage: cmake -DGIT_EXECUTABLE=git -DPATCH_FILE=x.patch -P ApplyPatchOnce.cmake + +execute_process( + COMMAND "${GIT_EXECUTABLE}" apply --reverse --check --ignore-whitespace + "${PATCH_FILE}" + RESULT_VARIABLE alreadyApplied + OUTPUT_QUIET ERROR_QUIET) +if(alreadyApplied EQUAL 0) + message(STATUS "Patch already applied: ${PATCH_FILE}") + return() +endif() + +execute_process(COMMAND "${GIT_EXECUTABLE}" apply --ignore-whitespace + "${PATCH_FILE}" RESULT_VARIABLE result) +if(NOT result EQUAL 0) + message(FATAL_ERROR "Failed to apply ${PATCH_FILE}") +endif() +message(STATUS "Applied patch: ${PATCH_FILE}") diff --git a/cmake/patches/.gitattributes b/cmake/patches/.gitattributes new file mode 100644 index 0000000..6c7c9e5 --- /dev/null +++ b/cmake/patches/.gitattributes @@ -0,0 +1,2 @@ +# Patches for fetched dependencies must keep LF so git apply matches them on every checkout. +*.patch text eol=lf diff --git a/cmake/patches/agisostack-pr718-restored-cf-claim-credit.patch b/cmake/patches/agisostack-pr718-restored-cf-claim-credit.patch new file mode 100644 index 0000000..7a1a518 --- /dev/null +++ b/cmake/patches/agisostack-pr718-restored-cf-claim-credit.patch @@ -0,0 +1,34 @@ +diff --git a/isobus/src/can_network_manager.cpp b/isobus/src/can_network_manager.cpp +index 5cb43bbd..e680c9c7 100644 +--- a/isobus/src/can_network_manager.cpp ++++ b/isobus/src/can_network_manager.cpp +@@ -590,6 +590,11 @@ namespace isobus + if (result != inactiveControlFunctions.end()) + { + targetControlFunction = *result; ++ // This CF is being restored to the table precisely because it just announced its ++ // address, so credit it for this roll-call. Without this it re-enters the table with ++ // the flag still false and is eligible for pruning again on the very next roll-call, ++ // which turns a single eviction into an unbroken offline/online cycle. ++ targetControlFunction->claimedAddressSinceLastAddressClaimRequest = true; + + LOG_DEBUG("[NM]: %s CF '%016llx' is now active at address '%d' on channel '%d'.", + targetControlFunction->get_type_string().c_str(), +@@ -610,14 +615,9 @@ namespace isobus + { + lastAddressClaimRequestTimestamp_ms.at(channelIndex) = SystemTiming::get_timestamp_ms(); + +- // Reset the claimedAddressSinceLastAddressClaimRequest flag for all control functions on the port +- auto result = std::find_if(inactiveControlFunctions.begin(), inactiveControlFunctions.end(), [channelIndex](std::shared_ptr controlFunction) { +- return (channelIndex == controlFunction->get_can_port()); +- }); +- if (result != inactiveControlFunctions.end()) +- { +- (*result)->claimedAddressSinceLastAddressClaimRequest = true; +- } ++ // Reset the claimedAddressSinceLastAddressClaimRequest flag for all control functions in the table on the port. ++ // Inactive CFs are left alone: the prune only looks at the table, and an inactive CF is credited when ++ // its own address claim restores it to the table. + std::for_each(controlFunctionTable[channelIndex].begin(), controlFunctionTable[channelIndex].end(), [](std::shared_ptr controlFunction) { + if (nullptr != controlFunction) + { diff --git a/include/app.hpp b/include/app.hpp index 6ee3ca7..8bc4ce5 100644 --- a/include/app.hpp +++ b/include/app.hpp @@ -92,7 +92,6 @@ class Application std::unique_ptr vtUpdateHelper; std::vector vtObjectPool; bool vtClientStarted = false; - bool canHardwareStarted = false; ///< CANHardwareInterface::start() succeeded, so stop() has a thread to stop bool vtConfigSynced = false; bool vtWasConnected = false; bool vtConnectionWarningLogged = false; diff --git a/src/app.cpp b/src/app.cpp index ed61f32..0d2586c 100644 --- a/src/app.cpp +++ b/src/app.cpp @@ -195,10 +195,10 @@ bool Application::setup_can_hardware() isobus::CANHardwareInterface::set_number_of_can_channels(1); isobus::CANHardwareInterface::assign_can_channel_frame_handler(0, canDriver); - canHardwareStarted = isobus::CANHardwareInterface::start(); - if ((!canHardwareStarted) || (!canDriver->get_is_valid())) + if ((!isobus::CANHardwareInterface::start()) || (!canDriver->get_is_valid())) { - log() << "Failed to start CAN hardware interface." << std::endl; + const std::string reason = canDriver->get_last_error(); + log() << "Failed to start CAN hardware interface." << (reason.empty() ? "" : " " + reason) << std::endl; return false; } @@ -1448,7 +1448,9 @@ void Application::stop() { tcServer->terminate(); } - if (canHardwareStarted) + // start() also starts the update thread when the driver fails to open, so check is_running() + // rather than whether start() succeeded. + if (isobus::CANHardwareInterface::is_running()) { isobus::CANHardwareInterface::stop(); } From 2a05699e360952e9a0ee3a76775ba09fd9e2c1e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gunics=20Bal=C3=A1zs?= Date: Wed, 14 Oct 2026 07:53:27 +0200 Subject: [PATCH 2/3] Pin AgIsoStack++ at the upstream #718 merge and drop the local patch AgIsoStack++ PR #718 is merged upstream (fbf5236), with the same can_network_manager.cpp change as the patch we carried, so pin that commit and remove the patch, ApplyPatchOnce.cmake and the PATCH_COMMAND. This also brings #782 (Extended Input Attributes sizing in the VT client autoscale). Autoscale now fails instead of hanging on an object it cannot size; all 88 objects in resources/AOG_TC.iop are sized by the new code. Co-Authored-By: Claude Opus 5.5 --- CMakeLists.txt | 12 ++----- cmake/ApplyPatchOnce.cmake | 22 ------------ cmake/patches/.gitattributes | 2 -- ...stack-pr718-restored-cf-claim-credit.patch | 34 ------------------- 4 files changed, 2 insertions(+), 68 deletions(-) delete mode 100644 cmake/ApplyPatchOnce.cmake delete mode 100644 cmake/patches/.gitattributes delete mode 100644 cmake/patches/agisostack-pr718-restored-cf-claim-credit.patch diff --git a/CMakeLists.txt b/CMakeLists.txt index 3e3fa0a..fc9f171 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -59,19 +59,11 @@ FetchContent_Declare( USES_TERMINAL_DOWNLOAD TRUE DOWNLOAD_EXTRACT_TIMESTAMP TRUE) FetchContent_MakeAvailable(Boost) -# Upstream main plus AgIsoStack++ PR #718 (a control function restored to the -# address table is pruned again on the next address claim request). Drop the -# patch once that PR is merged. -find_package(Git REQUIRED) FetchContent_Declare( isobus GIT_REPOSITORY https://github.com/Open-Agriculture/AgIsoStack-plus-plus.git - GIT_TAG f3c37b84d017df2228cb00e4366a5f7972da1176 - PATCH_COMMAND - ${CMAKE_COMMAND} -DGIT_EXECUTABLE=${GIT_EXECUTABLE} - -DPATCH_FILE=${CMAKE_CURRENT_SOURCE_DIR}/cmake/patches/agisostack-pr718-restored-cf-claim-credit.patch - -P ${CMAKE_CURRENT_SOURCE_DIR}/cmake/ApplyPatchOnce.cmake - DOWNLOAD_EXTRACT_TIMESTAMP TRUE) + GIT_TAG fbf52360eaf6bb37bc76e7b324d9b28081f7ed53 + DOWNLOAD_EXTRACT_TIMESTAMP TRUE) FetchContent_MakeAvailable(isobus) FetchContent_Declare( diff --git a/cmake/ApplyPatchOnce.cmake b/cmake/ApplyPatchOnce.cmake deleted file mode 100644 index 82b3e47..0000000 --- a/cmake/ApplyPatchOnce.cmake +++ /dev/null @@ -1,22 +0,0 @@ -# Applies PATCH_FILE to the current directory with git, unless it is already -# applied. FetchContent can run the patch step again on a source tree it already -# patched, which a plain `git apply` would fail on. -# -# Usage: cmake -DGIT_EXECUTABLE=git -DPATCH_FILE=x.patch -P ApplyPatchOnce.cmake - -execute_process( - COMMAND "${GIT_EXECUTABLE}" apply --reverse --check --ignore-whitespace - "${PATCH_FILE}" - RESULT_VARIABLE alreadyApplied - OUTPUT_QUIET ERROR_QUIET) -if(alreadyApplied EQUAL 0) - message(STATUS "Patch already applied: ${PATCH_FILE}") - return() -endif() - -execute_process(COMMAND "${GIT_EXECUTABLE}" apply --ignore-whitespace - "${PATCH_FILE}" RESULT_VARIABLE result) -if(NOT result EQUAL 0) - message(FATAL_ERROR "Failed to apply ${PATCH_FILE}") -endif() -message(STATUS "Applied patch: ${PATCH_FILE}") diff --git a/cmake/patches/.gitattributes b/cmake/patches/.gitattributes deleted file mode 100644 index 6c7c9e5..0000000 --- a/cmake/patches/.gitattributes +++ /dev/null @@ -1,2 +0,0 @@ -# Patches for fetched dependencies must keep LF so git apply matches them on every checkout. -*.patch text eol=lf diff --git a/cmake/patches/agisostack-pr718-restored-cf-claim-credit.patch b/cmake/patches/agisostack-pr718-restored-cf-claim-credit.patch deleted file mode 100644 index 7a1a518..0000000 --- a/cmake/patches/agisostack-pr718-restored-cf-claim-credit.patch +++ /dev/null @@ -1,34 +0,0 @@ -diff --git a/isobus/src/can_network_manager.cpp b/isobus/src/can_network_manager.cpp -index 5cb43bbd..e680c9c7 100644 ---- a/isobus/src/can_network_manager.cpp -+++ b/isobus/src/can_network_manager.cpp -@@ -590,6 +590,11 @@ namespace isobus - if (result != inactiveControlFunctions.end()) - { - targetControlFunction = *result; -+ // This CF is being restored to the table precisely because it just announced its -+ // address, so credit it for this roll-call. Without this it re-enters the table with -+ // the flag still false and is eligible for pruning again on the very next roll-call, -+ // which turns a single eviction into an unbroken offline/online cycle. -+ targetControlFunction->claimedAddressSinceLastAddressClaimRequest = true; - - LOG_DEBUG("[NM]: %s CF '%016llx' is now active at address '%d' on channel '%d'.", - targetControlFunction->get_type_string().c_str(), -@@ -610,14 +615,9 @@ namespace isobus - { - lastAddressClaimRequestTimestamp_ms.at(channelIndex) = SystemTiming::get_timestamp_ms(); - -- // Reset the claimedAddressSinceLastAddressClaimRequest flag for all control functions on the port -- auto result = std::find_if(inactiveControlFunctions.begin(), inactiveControlFunctions.end(), [channelIndex](std::shared_ptr controlFunction) { -- return (channelIndex == controlFunction->get_can_port()); -- }); -- if (result != inactiveControlFunctions.end()) -- { -- (*result)->claimedAddressSinceLastAddressClaimRequest = true; -- } -+ // Reset the claimedAddressSinceLastAddressClaimRequest flag for all control functions in the table on the port. -+ // Inactive CFs are left alone: the prune only looks at the table, and an inactive CF is credited when -+ // its own address claim restores it to the table. - std::for_each(controlFunctionTable[channelIndex].begin(), controlFunctionTable[channelIndex].end(), [](std::shared_ptr controlFunction) { - if (nullptr != controlFunction) - { From 5adc99a23f564121b5c87ba8cccece2a06b726fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gunics=20Bal=C3=A1zs?= Date: Sun, 18 Oct 2026 22:59:41 +0200 Subject: [PATCH 3/3] Bump AgIsoStack++ to 1d081d9 (DataSpan element indexing fix) Co-Authored-By: Claude Sonnet 5.5 --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index fc9f171..07d6ca1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -62,7 +62,7 @@ FetchContent_MakeAvailable(Boost) FetchContent_Declare( isobus GIT_REPOSITORY https://github.com/Open-Agriculture/AgIsoStack-plus-plus.git - GIT_TAG fbf52360eaf6bb37bc76e7b324d9b28081f7ed53 + GIT_TAG 1d081d9a9df61e256015efda3683d15c8fa3d580 DOWNLOAD_EXTRACT_TIMESTAMP TRUE) FetchContent_MakeAvailable(isobus)