Skip to content

Fix: carry over CF liveness when adopting an already-active VT/TC partner - #16

Draft
Arjan-Woltjer wants to merge 4 commits into
Open-Agriculture:mainfrom
Arjan-Woltjer:fix-partner-cf-liveness-carryover
Draft

Fix: carry over CF liveness when adopting an already-active VT/TC partner#16
Arjan-Woltjer wants to merge 4 commits into
Open-Agriculture:mainfrom
Arjan-Woltjer:fix-partner-cf-liveness-carryover

Conversation

@Arjan-Woltjer

Copy link
Copy Markdown

What

CANNetworkManager::update_new_partners() copies address and
controlFunctionNAME onto a PartneredControlFunction when it's late-bound
to a control function that already claimed its address before the partner
filter existed. It never copies claimedAddressSinceLastAddressClaimRequest,
so the newly-adopted partner starts that flag at its default false even
though the control function it was just adopted from already has it true.

Why this matters

The very next PGN 60928 (Address Claim) request seen anywhere on the bus —
routine when another node notices a newly-joined implement — resets that
flag network-wide and starts prune_inactive_control_functions()'s 755ms
(MAX_ADDRESS_CLAIM_RESOLUTION_TIME) clock. Since the flag was never true
to begin with, the partner gets evicted 755ms later even though the real
device never stopped being valid.

Once evicted, its controlFunctionTable slot is nulled, and
process_can_message_for_global_and_partner_callbacks() silently drops
every subsequent broadcast from that source address, since
message.get_source_control_function() no longer resolves. This is not
limited to the VT client's own internal tracking — any global PGN callback
watching that traffic goes dark at the same moment, which can make an
otherwise fully-functional device look like it stopped transmitting.

This is the same root cause reported in #584 (still open, independently
reproduced against current main as of 2026-07-09). @sujandumaru's comment
there correctly identifies that the flag, not just address/NAME, needs to be
propagated — this PR implements exactly that, copying the flag from the
already-active currentActiveControlFunction rather than hardcoding true.

Verification

Root-caused against real hardware in a downstream project
(Triton's Ploegbesturing
ISOBUS plough controller): an Ag Leader InCommand 1200 VT reproduced a VT
Status Timeout 100% of the time, ~2-3 seconds after every connect, because
the VT (the tractor's own screen) was already on the bus and already claimed
before the client powered up — exactly this late-binding path. An
independent, from-scratch global PGN 0xE600 listener added purely for
diagnosis saw the exact same cutoff as VirtualTerminalClient's own
internal status tracking, which is what led to tracing the drop to this
function rather than to the terminal itself. Not yet re-tested on that same
hardware with this fix applied — that's the next step on our end — but the
control-flow bug itself is confirmed by direct source reading, not
inference.

Closes #584.

ad3154 and others added 4 commits October 20, 2024 11:54
Updated AgIsoStack to 29dab887a48bb204aae983b06052d52b0f2314d5
Added a basic example showing how to receive machine speed messages.
update_new_partners() copies address and controlFunctionNAME onto a
PartneredControlFunction when it's late-bound to a control function that
already claimed its address before the partner filter existed, but it never
copied claimedAddressSinceLastAddressClaimRequest. That flag starts false on
a freshly constructed partner, so the very next PGN 60928 (Address Claim)
request seen anywhere on the bus -- routine when another node notices a
newly-joined implement -- resets it network-wide and starts the 755ms
prune_inactive_control_functions() clock. Since it was never true to begin
with, the partner gets evicted 755ms later even though the real device never
stopped being valid.

Root-caused against real hardware: an Ag Leader InCommand 1200 VT reproduced
this 100% of the time, ~2-3s after every connect, because the VT (the
tractor's own screen) was already on the bus and already claimed before our
client powered up -- exactly this late-binding path. Once the table slot is
nulled, CANNetworkManager::process_can_message_for_global_and_partner_callbacks()
silently drops every subsequent broadcast from that address (source control
function resolves to nullptr), so even a from-scratch independent PGN
listener alongside VirtualTerminalClient's own status tracking saw the exact
same cutoff -- it looked like the VT itself stopped broadcasting, but the
frames were being dropped downstream of a stale control-function table entry.

Closes #584.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants