Fix: carry over CF liveness when adopting an already-active VT/TC partner - #16
Open
Arjan-Woltjer wants to merge 4 commits into
Open
Arjan-Woltjer wants to merge 4 commits into
Arjan-Woltjer wants to merge 4 commits into
Conversation
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.
This was referenced Sep 5, 2026
Arjan-Woltjer
added a commit
to Arjan-Woltjer/NeptuneGPS_Triton
that referenced
this pull request
Sep 5, 2026
…not the terminal The InCommand 1200 almost certainly never stopped broadcasting its VT status message. Our VT partner control function gets wrongly evicted from AgIsoStack's own control-function table ~2-3s after connect (upstream bug Open-Agriculture/AgIsoStack-plus-plus#584: update_new_partners() adopts an already-active CF without carrying over claimedAddressSinceLastAddressClaim- Request, so the next PGN 60928 request anywhere on the bus gets it pruned). Once evicted, every subsequent status frame is silently dropped before reaching any listener -- including the independent vtstat counter added earlier this session, which is why it froze in lockstep with AgIsoStack's own tracking rather than actually corroborating a terminal-side stop. Vendor-patches the fix (documented as patch #3, applied to .pio/libdeps/teensy41_isobus per the existing convention -- gitignored, not in this diff) and bumps the VT client's log level Warning -> Info so a future capture shows AgIsoStack's [NM] control-function lifecycle lines directly. Also explains #18 (no auto-reconnect) as the same root cause rather than a separate gap: a compliant VT only claims once, so nothing naturally repopulates the evicted table slot afterward. Filed upstream as Open-Agriculture/AgIsoStack-Arduino#16. Not yet field-verified against the InCommand 1200 that surfaced this -- next hardware session's job. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Sep 5, 2026
Arjan-Woltjer
added a commit
to Arjan-Woltjer/NeptuneGPS_Triton
that referenced
this pull request
Sep 8, 2026
…ction root cause (#24) * Widen GuidanceSource calibration banner separator to match header width * Fix CANNetworkManager::CANNetwork() calls -- it's a static member, not a method AgIsoStack-Arduino 0.1.5 declares CANNetworkManager::CANNetwork as static CANNetworkManager CANNetwork; (a singleton object), not a factory method. All 11 call sites in IsobusGuidanceChannel.cpp and IsobusDebugMenu.cpp were calling it as CANNetwork(), which fails to compile against a genuinely fresh build (a stale cached object file was masking this). * Restore NanoLibcCompat.cpp swprintf shim -- confirmed load-bearing, not dead code Deleted it on a hunch it was an unneeded shim and rebuilt teensy41_isobus. Link failed exactly as its own header comment predicted: undefined reference to swprintf/getwc/ungetwc out of libstdc++_nano.a and libc_nano.a under TEENSY_OPT_SMALLEST_CODE's nano.specs. Restored verbatim, with a note recording the re-verification. See issue #13 for the still-open question of what exactly pulls in the wide-locale facet in the first place. * Fix legacy PGN source-address resolution, speed sentinel, and add retry + diagnostics Live bus test on a New Holland T6 (Ag Leader Dual Trac / InCommand 1200 VT) on 2026-08-08 found the guidance source only ever broadcasts legacy proprietary PGNs (65267/65256/65535), not the standard NMEA2000 set. - OnLegacySpeed had no 0xFFFF ("not available") sentinel guard, unlike its NMEA2000 sibling -- an unavailable reading was being converted into an impossible 131.70 m/s (256.00 kn). Guarded. - OnLegacyXteJohnDeere/OnLegacyXteTrimble filtered on msg.get_source_control_function(), which is permanently null for these senders: they never broadcast a real ISO Address Claim, so AgIsoStack's control-function table never resolves an entry for their address. Switched to reading the address straight off the CAN identifier instead, which doesn't require control-function resolution. - That unblocked the real address: kSourceAddressJohnDeere was 0x2A (an assumption inherited from the old VehicleGps fixed-CAN-ID era); the actual unit on this rig claims 0x80. Updated. - Added a bus-diagnostic snapshot to MessageCounters (last source address / raw values per PGN, captured ahead of any filter) so IsobusDebugMenu's status dump can show what's actually arriving even when a handler's own filtering drops it -- this is what surfaced the 0x80 vs 0x2A mismatch and the raw 0xFFFF speed sentinel in the first place. See Documentation/HardwareTestNotes.md for the full session log. * Add hardware test notes for ISOBUS sessions 1-2 Log of the New Holland T6 / Ag Leader Dual Trac live-bus test sessions: rig description, bugs found/fixed, still-open items, and code-level confirmation (address-claim renewal via VT Working Set Maintenance, PGN request behavior, VT object pool push) for the three questions raised before session 2. * minor changes to the layout added stubs in main to guide Claude in implementing the stuff * Fix P1-P5 guidance-channel prerequisites from TC client design doc Fixes flagged in Triton_TC_Client_Design.md sec.2 as blocking before adding a Task Controller client on top of the existing PGN guidance path: - P1: OnAllImplementStop now decodes the AISO 2-bit state (byte 7 bits 0-1) instead of stopping on any received frame. Layout confirmed against AgIsoStack's own ShortcutButtonInterface::process_message(). Previously every periodic Permit (01) broadcast halted the plough. - P2: OnXteNmea2000 no longer hardcodes quality=RTK on every message. PGN 129029 is deliberately not being pursued as a quality source (not reliably present across hardware, e.g. Ag Leader), so single-arg SetXte() is used instead -- this leaves quality unset rather than lying that the fix is RTK-equivalent, which previously defeated InterfacePlough's IsRtkQuality() interlock unconditionally. - P3: decode PGN 129283 byte 1's Navigation Terminated bit and skip the XTE update when set, instead of discarding the byte entirely. - P4: guard the second N2K sentinel value (0x7FFFFFFE, error) alongside the existing 0x7FFFFFFF (not available) check. - P5: renamed GuidanceSource::GetXteFixAge() to GetXteTimestamp() -- it returns a millis() timestamp, not an elapsed age. Updated the native test mock and all real call sites; mock's backing field renamed to lastXteFix to match the real class's private member name. Verified: teensy41_isobus builds clean; all 47 native AUnit tests pass (via test/native/run_tests_msvc.ps1, no GCC on this machine). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Add IsobusTcInterface -- ISOBUS Task Controller client Implements the TC client design from Triton_TC_Client_Design.md: connects to a Task Controller, uploads a DDOP, and receives DDI 513 (guidance line deviation at the implement's Device Reference Point) and DDI 514 (GNSS quality) via ValueCommand callbacks. Data source only -- no control authority over the plough, and deliberately not wired into GuidanceSource yet (design doc sec 6: DDI 513 is a different quantity than the tractor's own PGN 129283 XTE; start at cross-check-only, decide fusion later). Verified the AgIsoStack API against the vendored source actually used by the teensy41_isobus build (.pio/libdeps/teensy41_isobus/AgIsoStack) rather than trusting the design doc's sketch or the *other*, differently-versioned AgIsoStack copy under .pio/libdeps/teensy41/ (a stale leftover, unused by any current env). Two real divergences found this way: - TaskControllerClient's value callbacks use int32_t, not uint32_t. - There's no Object::NULL_OBJECT_ID member in this version; the "no object" sentinel is the free constant isobus::NULL_OBJECT_ID. Also corrected DDI 116 in the design doc's element-tree sketch: it's TotalArea, not ActualWorkingDepth (that's DDI 52) -- moot here since the working-depth DPD isn't declared in this first cut anyway (see below). Scope trimmed from the design doc's element tree: only Device/Connector/ Function elements and the two TC-writable process data variables (513, 514) plus the connector's X/Y offset properties (534/135) are declared. DDI 67/70 (working width) and 141 (work state) from the doc's sketch are NOT declared -- this implement controls plough *offset*, not width (ImplementPlough has no working-width concept at all), and has no engaged/disengaged state to report either. The doc's own test plan (Phases 1-3: connection, read-only 513/514, geometry validation) doesn't need them. The connector offsets are committed as named zero placeholders pending a real measurement (design doc sec 4.3 / Phase 3). Wires gTcInterface into main.cpp's setup()/loop(), replacing the stub comments already left there, matching gVtInterface's construction order and lifecycle exactly. Verified: teensy41_isobus builds clean; teensy41_serial (no ISOBUS) still builds clean with the new files compiling to empty translation units. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Wire IsobusTcInterface diagnostics into IsobusDebugMenu There was no visibility into TC connection state at all before this -- IsobusTcInterface exposed IsConnected()/GetDrpDeviationMm()/etc. but nothing called them. Needed for tomorrow's hardware test. IsobusDebugMenu takes an optional IsobusTcInterface* (nullptr-safe, so the TC section is skipped if unset) and now prints: connected state, task active (advisory only, matching the same caveat already on the interface itself), DRP deviation (DDI 513) with age, and TC GNSS quality (DDI 514) with age -- both in the full status dump and the periodic one-liner. main.cpp now passes gTcInterface through. Verified: teensy41_isobus and teensy41_serial both build clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Correct overstated VT claim in hardware test notes Session 2's item 3 claimed the object-pool upload path "matches session 1's observation of the controller actually appearing/working in the InCommand 1200's VT" -- session 1 only observed Ploegbesturing visible as a connected ECU (address claim/device presence), never the actual working set/data mask (the plough control screen) rendered. Confirmed directly: the Ploughcontrol screen has still not been seen on a VT as of today. Code running without error is not evidence it rendered. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Add VT connect/upload progress to IsobusDebugMenu Same gap as the TC side: IsobusVtInterface had no debug output at all -- only get_is_connected() existed anywhere, a single boolean true only once VT connect + object pool upload + activation (a ~22-step handshake) fully completes. No way to tell where it's stuck. AgIsoStack's VirtualTerminalClient tracks its internal state machine but never exposed it publicly (unlike TaskControllerClient, which already has an unpatched get_state()) -- patched the vendored source to add the missing accessor. Documented in the new Documentation/ AgIsoStackVendorPatches.md, since .pio/libdeps/ is gitignored and this patch will be silently lost on any pio pkg update / clean lib reinstall if not reapplied from that doc. Also used the occasion to correct a stale claim in that same area: the CANNetworkManager Meyer's-singleton patch from the 2026-08-04 CAN bring-up session is no longer present in the currently vendored copy (confirmed by reading the file directly) -- wiped by a lib reinstall between 2026-08-04 and 2026-08-08, never reapplied, not needed since the real hang fix was the Teensy optimization flag, not the singleton conversion. IsobusVtInterface::GetStateStep()/GetStateTotalSteps()/GetStateName() expose the state as "step N of ~22" plus a human-readable name (e.g. "12/22 UploadObjectPool") -- not byte-accurate upload percentage, which would need the transport-protocol session's own progress counter; there's no public path to reach that session either (CANNetworkManager's per-channel TransportProtocolManager is private with no accessor). Judged disproportionate to patch for a first pass. IsobusDebugMenu now takes an optional IsobusVtInterface*, printing VT connect state + step in both the full dump and periodic line, same nullptr-safe pattern as the TC section added earlier today. Verified: teensy41_isobus and teensy41_serial both build clean (the former recompiles the patched AgIsoStack VT client files without error). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fix VT/TC partner registration and TC DDOP tree; add TC/VT diagnostics Both IsobusVtInterface and IsobusTcInterface constructed their PartneredControlFunction directly instead of via CANNetworkManager::create_partnered_control_function(), so neither could ever be matched against an incoming Address Claim -- VT and TC sat at "not connected" indefinitely regardless of what was on the bus. Fixed by using the factory, matching AgIsoStack's own examples. The TC DDOP was also missing its mandatory root DeviceElement of Type::Device (ISO 11783-10 requires exactly one) and never called add_reference_to_child_object() to attach the DPT/DPD children to their owning element, leaving them unattached. Fixed; structure label bumped TC01 -> TC02. Confirmed on hardware (Fendt + Trimble rig, 2026-08-10): TC now connects, reaches task-active, and correctly identifies both declared DDIs via Value Requests. VT object pool is still rejected by the real terminal despite the same fix getting its handshake to the upload step -- root cause not yet found via three live bisection rounds or static analysis; see HardwareTestNotes.md for the full session log and next steps (VT via AgIsoVirtualTerminal simulator; real XTE PGN via CAN sniff). Also adds: VT negotiated-version reporting, TC value-command/request activity counters (any DDI) for IsobusDebugMenu, and disabled VT pool bisection scaffolding left in place for future reuse. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Add TC-GEO connected-server capability query to IsobusTcInterface AgIsoStack's TaskControllerClient already decodes the connected TC's reported ServerOptions bits (SupportsTCGEOWithPositionBasedControl / ...WithoutPositionBasedControl) from its ParameterVersion handshake message, but nothing read them. Session 3's hardware notes leading theory for "DDI 513/514 never arrived" was that the Fendt+Trimble rig's TC simply doesn't implement TC-GEO -- this exposes that as a direct, on-screen Y/N in IsobusDebugMenu instead of inferring it from DDI silence, ahead of testing against a Trimble (expected N) and an Ag Leader InCommand 1200 (expected Y) rig today. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Add TC-GEO manufacturer licensing research and field test log; note offline VT pool reference-parser verification ISOBUS_TC_Manufacturer_Comparison.md: how John Deere, Trimble, Raven, CNH, and Ag Leader each gate TC-GEO (licensing tier, activation mechanism, per-model quirks) -- research backing session 3's "does this TC implement TC-GEO at all" open question. TCGEO_Field_Test_Log.md: tracks confirming the above against real farm terminals, starting with Bos (Trimble+Fendt, TC-GEO expected absent -- consistent with their separately-purchased plough control system) and van Mastwijk (Ag Leader InCommand 1200+CNH, TC-GEO expected present out of the box). HardwareTestNotes.md: adds a follow-up section documenting an offline verification that fed VTObjectPool.cpp's exact pool bytes into AgIsoStack++'s own upstream IOP parser (the same parser AgIsoVirtualTerminal uses) -- accepted cleanly, all 23 objects, no errors, shifting the VT-rejection theory from "bug in our bytes" toward "Fendt-UT-specific rejection reason." Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fold TCGEO_Field_Test_Log.md into HardwareTestNotes.md Bos and van Mastwijk are the same rigs already tracked as Session 3 and Session 1/4 in the hardware test log -- keeping TC-GEO field results in a separate doc split the same real-world test event across two files. Bos's TC-GEO findings move into Session 3 as a subsection (explaining the DDI 513/514 silence via Trimble's licensing model); van Mastwijk's pre-visit checklist becomes a new Session 4 placeholder for this afternoon's second visit. ISOBUS_TC_Manufacturer_Comparison.md (the underlying per-brand licensing research) stays a separate reference doc -- only the field-test-log/session-log split is removed, giving one clean, session-numbered file for the laptop session doing today's testing to fill in. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Decode the Session 3 VT rejection's error bitmask against ISO 11783-6 EndOfObjectPool_ErrorBitmask_Research.md reads the actual standard text (End of Object Pool Response, PGN 1810) rather than relying on AgIsoStack's own reference server, which hardcodes that byte to 0 on send and so never decodes it. "Bitmask value 9" = bit 0 (method or attribute not supported by the VT) + bit 3 (pool deleted from volatile memory -- boilerplate on any rejection per the standard). Corrects HardwareTestNotes.md's Session 3 follow-up, which had called the byte "opaque" / "Fendt-proprietary" after only checking AgIsoStack itself. The real complaint is specific: something in the WorkingSet object's own attribute fields (not its children/masks, which is all Session 3's bisection varied) isn't supported by this VT. Points Session 4's checklist at a sharper bisection target if the same rejection recurs there. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Add WorkingSet-attribute bisection scaffold; patch AgIsoStack to decode object pool error bits VTObjectPool.cpp: appendWorkingSet() gains optional selectable/ numLanguages/languageCode params (defaulted to current production values, so this is a no-op until used) plus a VT_WORKINGSET_BISECT_VARIANT switch in BuildObjectPool() to try alternate WorkingSet field values against the REAL 23-object pool, not a stripped-down substitute. Session 3's own bisection only varied DataMask/SoftKeyMask content; EndOfObjectPool_ErrorBitmask_Research.md points at the WorkingSet's own attributes instead, which nothing has tested yet -- this is a bisection tool, not a confirmed fix, since there's no way to verify against a real terminal without hardware. .pio/libdeps/teensy41_isobus/AgIsoStack (vendor patch, documented in AgIsoStackVendorPatches.md #2, NOT tracked by git -- reapply on any other machine/clean .pio/ per that doc's own caveat): decodes the End of Object Pool response's error-codes byte into named ISO 11783-6 bits directly in the serial log ("method or attribute not supported by the VT" etc.) instead of a raw integer, so a future rejection is readable without a manual standards lookup. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fix VT object pool rejection: WorkingSet needs a child, and a field-order bug The VT object pool has been rejected by every real terminal tested since session 1 -- confirmed today, by elimination, that it was never about pool content (WorkingSet selectable/language/colour, DataMask/SoftKeyMask content, version labels, VT3-vs-VT4 negotiation, and a real but unrelated manufacturer_code bug -- was 64, belonged to another manufacturer, now 1407 -- all ruled out one at a time against a real CNH terminal). Decisive test: uploaded AgIsoStack's own reference pool (examples/ VirtualTerminal/ObjectPool.cpp, pulled in verbatim via #include, not copied) against the same terminal. It connected and rendered -- the first working VT pool this project has ever confirmed on real hardware, proving the upload mechanism itself was sound and the bug was in our pool's content specifically. Field-by-field comparison found it: the reference WorkingSet has 1 child object reference; ours always had 0. Adding one exposed a second, latent bug in appendWorkingSet() -- it wrote the language-code bytes immediately after the header, but the correct ISO 11783-6 order is children, then macros, then language codes. With 0 children this was invisible (nothing to reorder around); the moment a child existed, the terminal read our language code "nl" (bytes 0x6E,0x6C) as the child's object ID (0x6C6E = 27758) and correctly rejected it as unknown -- confirmed by exact byte arithmetic against the terminal's own error response. Fixed both: appendWorkingSet() now only writes the header, callers append children then the language code (new appendLanguageCode()) in the correct order; production pool gets a real child. Confirmed working after the fix -- pool connects and renders. VT_WORKINGSET_BISECT_VARIANT keeps 1-4 available (now all with the child-reference fix retained) for re-testing against Fendt, which was never retried with a WorkingSet child at all despite implicating the same object. VT pool version label bumped MW01 -> MW02 since the structure genuinely changed, matching the TC DDOP's TC01 -> TC02 discipline from session 3. Full session log, including the still-open post-connect stability timeout (VT_STATUS_TIMEOUT_MS = 3000, cause not yet found) and what to check next for DDI 513/514 and Ag Leader, in HardwareTestNotes.md Session 4. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Stop flooding the VT with unconditional numeric-value updates updateVtVariables() sent 4 send_change_numeric_value() commands every 100ms (40 msg/s) the whole time connected, regardless of whether any value changed. AgIsoStack's own reference example only sends on a button press. Session 4's decisive VT-pool test swapped in that reference pool and it stayed connected stably, while our own pool drops intermittently with "[VT]: Status Timeout" -- the one thing that differs behaviorally (not just structurally) between those two tests is this unconditional flood: against our own pool the 4 object IDs are real, bound OutputNumber widgets, so the VT does actual redraw work on every message, unlike the reference pool's IDs, which almost certainly don't resolve to a NumberVariable and get rejected cheaply. Sustained redraw load intermittently starving the VT's own periodic status broadcast past AgIsoStack's 3s VT_STATUS_TIMEOUT_MS is a plausible mechanism, not yet confirmed on hardware. Fix: send only on real value change, plus a 1s heartbeat resend so a dropped CAN frame can't leave the VT stale forever. Cuts steady-state traffic ~10x for slow-changing plough telemetry with no functional loss. Needs re-testing on hardware to confirm/rule out. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Add an app-switcher icon for the plough working set Closes #14. Decoded AgIsoStack's own reference pool's WorkingSet object by hand (it stayed connected/rendered on real hardware during Session 4's decisive test) and found its 1 declared child is specifically a PictureGraphic "avatar" icon at x=0, y=-4 -- that's the actual mechanism VTs read for an implement's app-switcher entry, not just "any child object" as the earlier fix treated it. Adds appendPictureGraphic() (byte layout confirmed against AgIsoStack's own parser, not guessed) and a hand-authored 16x16 monochrome plough pictogram (hitch narrowing to a share on a ground line). WorkingSet's child swaps from the placeholder OutputString to this icon, matching the reference pool's exact pattern. Version label bumped MW02 -> MW03 per this project's established discipline for pool structure changes. Builds clean on teensy41_isobus/teensy41_serial. Not yet seen on a real VT's app switcher -- needs field verification next session. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Wire VT Wider/Narrower soft keys into the button arbitration ladder Follows this project's existing (currently disabled) joystick-input precedent: rather than a separate VT control path racing InterfacePlough::Update()'s existing button-driven Adjust() call, IsobusVtInterface::ConsumeWiderPress()/ConsumeNarrowerPress() are consume-once signals that OR directly into CheckButtons()'s existing LEFT_BUTTON_2/RIGHT_BUTTON_2 conditions -- same debounce/arbitration logic, no new code path. Direction mapping confirmed by tracing ImplementPlough::Adjust(): direction=-1 (LEFT_BUTTON_2) -> Wider(), direction=+1 (RIGHT_BUTTON_2) -> Narrower(), so Key_Wider feeds the LEFT slot and Key_Narrower the RIGHT slot. Key_Auto stays log-only -- AUTO mode is derived from GPS/hitch state, not user-settable, so there's no existing target for it. InterfacePlough::Update()/CheckButtons() gain two defaulted bool params (vtWiderPressed/vtNarrowerPressed = false) so the class stays framework-agnostic; main.cpp only passes real values under #ifdef ISOBUS. All 47 native AUnit tests still pass (verified via a one-off MSVC run excluding test_VehicleGps.cpp, whose missing include path in run_tests_msvc.ps1 is a pre-existing, unrelated gap). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Add ISO 11783-6:2004 standard PDF as reference Primary source for the End of Object Pool error-bitmask decode in EndOfObjectPool_ErrorBitmask_Research.md (bitmask 9 = bit 0 + bit 3). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Add VT object pool import/export tooling for AgIsoTerminalDesigner export_pool_msvc.ps1 builds VTObjectPool.cpp's BuildObjectPool() natively (reusing the existing MSVC/Arduino-stub pattern from the AUnit tests, since the pool code has no AgIsoStack/CAN dependency) and dumps the current production pool to a raw .iop file for visual inspection/editing in AgIsoTerminalDesigner (https://open-agriculture.github.io/AgIsoTerminalDesigner/). iop_to_header.ps1 goes the other way, converting an edited .iop file into a C++ header. VTObjectPool.cpp gains a VT_POOL_USE_DESIGNED_POOL toggle (mirroring the existing VT_POOL_USE_AGISOSTACK_REFERENCE one) to swap in a designer-authored pool for on-hardware testing without touching the hand-authored append() pool. Verified round-trip byte-for-byte identical; teensy41_isobus and teensy41_serial both still build clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Link the two upstream AgIsoStack PRs from the vendor-patch TODO list get_state() and the EOP error-bit decoding are now open PRs (#14, #15) on Arjan-Woltjer/AgIsoStack-Arduino against upstream, not just a TODO -- notes what to do (bump lib_deps, drop the local .pio patch) once either merges. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Drop unwantedRecommendations from .vscode/extensions.json Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Recommend C++ tooling extensions for Ploegbesturing Isobus Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Reorganize Ploegbesturing Isobus into calibration/config/implement/isobus folders, extract testable PGN decode - Split lib/PloegbesturingCore/src's flat layout into calibration/, config/, and implement/ subfolders; InterfacePlough stays at src root. - Extract IsobusGuidanceChannel's byte-level PGN decode into a new, framework-free IsobusPgnDecode unit, native-testable like the serial/ parsers -- 31 new tests, including regression coverage for two corrections: the JD/Ag-Leader-Raven legacy XTE PGN accepts either known-valid source address (0x2A and 0x80, not one replacing the other), and its quality check is a nibble range, not one exact byte. - Remove the unused VehicleGps native test (never exercised anything in this project) and its now-dead build wiring. - Convert every include inside PloegbesturingCore to a relative path (matching Salacia's SalaciaFirmwareCore convention) instead of a bare filename resolved via per-subfolder -I flags; drops every such -I entry from platformio.ini in favor of a single library-root -I for [env:native] (teensy envs get it for free via PlatformIO's LDF). - Delete the native GuidanceSource fake this enabled removing: tests now construct and drive the real class directly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Drop unused AUnit/test_framework from Ploegbesturing Isobus's real-board envs teensy41_isobus/teensy41_serial carried bxparks/AUnit and test_framework = custom since the very first commit that split them off the old Ploeg ISOBUS prototype's single env, even though this project's tests have always run under [env:native] only. Nothing in src/ or lib/PloegbesturingCore/src/ includes AUnit, so PlatformIO's LDF never actually compiled it in -- confirmed empirically, teensy_size's FLASH/RAM1 totals are byte-for-byte identical with or without it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Move Ploegbesturing Isobus's IMXRT1062 size-check fix to the shared teensy41 base extra_script.py's corrected combined ITCM+DTCM FlexRAM check was only wired up for teensy41_isobus; teensy41_serial extends the same [env:teensy41] base but never inherited it, silently falling back to PlatformIO's stock per-bank check. Moved to [env:teensy41] so both variants get it -- low risk today given teensy41_serial's much smaller footprint, but the same shared hardware pool applies to it too. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Rename cryptic single-letter parser params, add GN* NMEA talker ID support Renamed h/n/t parameters to header/termNumber/term across NmeaParser, TrimbleParser, and CanSerialParser for readability, no behavior change. NmeaParser also now claims GNGGA/GNVTG/GNXTE alongside GPGGA/GPVTG/GPXTE, for multi-constellation GNSS receivers that use the GN talker ID instead of GP. Fixed a missing closing paren in NmeaParser's VTG case introduced by the rename (newSpeed = atof(term) -- would not have compiled). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Document Trimble's mixed-checksum handling in SerialGuidanceChannel, drop unreachable disjunct Explain why dispatchTerm() doesn't commit at the Trimble outer-packet end marker (isChecksumTerm is never set there) and why the explicit commitTo() right after it is safe (the outer packet's sum-based CRC already passed). Removes the earlier confused "why does this call again?" note now that this is traced and understood -- see conversation. Also comments out dispatchTerm()'s activeParse->useParityAsChecksum() disjunct: unreachable today (TrimbleParser is the only parser reporting true, and ROXTE sentences never terminate on '*', so they never reach this branch at all -- their integrity is fully covered by the outer packet's own checksum). Kept commented rather than deleted since a future '*'-terminated, checksum-less parser would need it back. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Log Session 5 field test (Ag Leader InCommand 1200): VT status timeout root-caused HMI object pool upload/render confirmed working. Found and root-caused a reproducible VT Status Timeout ~2-3s after connect: the InCommand 1200 stops sending its own VT status broadcast after exactly 4 messages, independent of anything we send it (isolation-tested by disabling our post-connect variable burst entirely -- identical failure). Ruled out as downstream of no auto-reconnect and TC never connecting either, both filed as separate issues (#17, #18, #19). Adds IsobusVtInterface::GetVtStatusMessageCount()/GetVtStatusMessageAgeMs(), an independent PGN 0xE600 listener used to make this diagnosis, surfaced in IsobusDebugMenu as vtstat=<count>/<age>ms. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Root-cause #17's VT Status Timeout to an AgIsoStack CF-eviction bug, not the terminal The InCommand 1200 almost certainly never stopped broadcasting its VT status message. Our VT partner control function gets wrongly evicted from AgIsoStack's own control-function table ~2-3s after connect (upstream bug Open-Agriculture/AgIsoStack-plus-plus#584: update_new_partners() adopts an already-active CF without carrying over claimedAddressSinceLastAddressClaim- Request, so the next PGN 60928 request anywhere on the bus gets it pruned). Once evicted, every subsequent status frame is silently dropped before reaching any listener -- including the independent vtstat counter added earlier this session, which is why it froze in lockstep with AgIsoStack's own tracking rather than actually corroborating a terminal-side stop. Vendor-patches the fix (documented as patch #3, applied to .pio/libdeps/teensy41_isobus per the existing convention -- gitignored, not in this diff) and bumps the VT client's log level Warning -> Info so a future capture shows AgIsoStack's [NM] control-function lifecycle lines directly. Also explains #18 (no auto-reconnect) as the same root cause rather than a separate gap: a compliant VT only claims once, so nothing naturally repopulates the evicted table slot afterward. Filed upstream as Open-Agriculture/AgIsoStack-Arduino#16. Not yet field-verified against the InCommand 1200 that surfaced this -- next hardware session's job. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Extend #17's root-cause analysis to cover #19 (TC never connects) IsobusTcInterface::Begin() binds the TC partner via the identical NAMEFilter + create_partnered_control_function() pattern as the VT, so it's subject to the same AgIsoStack-plus-plus#584 eviction bug. TaskControllerClient's WaitForServerStatusMessage state has no timeout at all -- it waits indefinitely for the TC server's first status broadcast, gated through the same process_can_message_for_global_and_partner_callbacks() dispatch that dropped the VT's status frames. If the TC partner is evicted before that first broadcast arrives, the client sits stuck forever, matching #19's observed symptom independently of the VT's own connection state. No source change needed -- the vendor patch already applied for #17 fixes this generically for any late-bound partner. Documentation only, plus posting the analysis to #19. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Log Session 6: #17/#19 fixed and verified, plus a second eviction bug found and fixed on the tractor Field session against the Ag Leader InCommand 1200 and the New Holland tractor's own VT. Verifies the off-tractor root-cause work in 590f2fa/935de81 and then goes considerably further. Confirmed fixed (issues #17, #19, both closed): - VT holds indefinitely against a single terminal; 160s unbroken with vtstat climbing at 1Hz, where it previously died in ~2-3s every time. - Task Controller connects and activates its DDOP for the FIRST TIME in the project's history, settling #19. The "downstream of the VT" theory was wrong; the TC was a third, independently-evicted partner. Second bug found, root-caused and fixed live (vendor patch #4): Patch #3 fixed only the initial partner adoption. Every subsequent PGN 60928 address-claim roll-call clears the liveness flag on all tracked CFs and prunes 755ms later, so partners kept being evicted out from under live sessions -- and update_address_table() restores a pruned CF without ever setting that flag, making it self-sustaining (observed as unbroken offline/online churn for 300+ seconds). Patch #4 exempts Partnered CFs from the prune, alongside the existing Internal exemption. Verified in the configuration that had been dying in ~3s all afternoon: 235 prune cycles survived over 800s with zero partner losses, then the worst case -- a second terminal enumerating onto the bus mid-session, previously fatal within ~4s -- survived with 185 consecutive vt=Y + tc=Y samples and ~21 minutes of unbroken VT+TC uptime. Also found, filed as new issues: - #20: the legacy XTE decode reads the wrong vendor's payload on this rig. Ground truth swung 144 -> 8 -> 118 -> 14 cm while our value took two distinct values in ~400 samples. Session 1 added SA 0x80 (Ag Leader) to the John Deere decoder assuming a shared layout on proprietary PGN 0xFFFF; it is not shared. Same cause explains quality always reading 0. - #21: the TC reports TC-GEO-with-position support with an active task and an AB line selected, yet sends zero Value Commands and zero Value Requests. Kills the Session 3 TC-GEO licensing theory. - #22: upstream patches #3 and #4 plus the update_address_table restore fix. Recorded honestly in the notes: an intermediate XTE analysis concluded a 100x scale error from a single ground-truth point that matched almost perfectly. Further readings disproved it. Raw captures are committed alongside the annotated excerpts precisely because re-deriving that took counting distinct values across ~400 samples. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Stop decoding Ag Leader payloads with John Deere's layout (#20) Session 1 (2026-08-18) widened DecodeLegacyXteJohnDeere() to accept source address 0x80 alongside 0x2A, assuming Ag Leader/Raven and John Deere share a payload layout on the overloaded proprietary PGN 65535. Session 6 (2026-09-05) disproved that against live ground truth: while the terminal's own XTE swung 144 -> 8 -> 118 -> 14 cm, data[3..4] produced only two distinct values across ~400 consecutive samples, and data[1] reads 0x03, which the John Deere quality nibble check can never accept. So on an Ag Leader rig we were producing a confidently wrong XTE and a permanently failing quality gate -- worse than no reading, since the number looks plausible. 0x80 is now diagnostics-only until a raw capture establishes its real layout. 0x2A remains decoded as John Deere, confirmed by the user this session. Note that path is only John Deere's *proprietary* XTE; the same equipment family also broadcasts standard NMEA2000 XTE (PGN 129283, legacy CAN ID 1DF9031C, SA 0x1C), already handled unfiltered by DecodeXteNmea2000(). Raw diagnostics (rawWord/rawByte1) are deliberately still captured for 0x80 and now stored regardless of `valid` -- they are exactly what deriving the Ag Leader layout needs off a live bus, so they must keep reaching IsobusDebugMenu. This also brings the call site in line with IsobusPgnDecode.hpp's already-documented convention that diagnostic fields populate independent of `valid`. Does not close #20: this stops us reporting wrong data, but the rig still has no usable XTE source, which is what actually blocks plough steering. 79/79 native tests pass; teensy41_isobus builds clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Add a Tramline Control probe to the DDOP (#21) DDI 513/514 are optional members of the AEF Tramline Control DDI set, not standalone process data. Per "Tramline Control -- Basic Requirements v1.16" (ISO 11783-11 DDE supplement, attached to the DDI 505 entity on isobus.net), a Level 1 system also requires DDIs 505, 506, 515, 507, 508, 509, 510 and 511 in the DDOP. This DDOP declared only 513 and 514 -- precisely the two optional ones -- leaving them as orphan objects a TC has no reason to write to. That is the most likely reason no Value Command has ever been observed. Rather than build out a tramline capability a plough does not have (Level 1 means "the implement calculates the tramline tracks"), this declares only the handshake pair as a probe: DDI 505 as a DPT with NO levels claimed, and DDI 506 which the TC writes back. A tramline-capable TC is required to answer with 506 even when there is no common level (value 0), so any arrival proves the terminal implements the feature and that 513/514 are reachable; silence suggests it does not, and no DDOP work would ever produce them from it. DDI 506's arrival is latched separately from lastValueCommandDdi (which a later DDI would overwrite) and surfaced in IsobusDebugMenu, since the result is that it arrived at all rather than what it said. Structure label bumped TC02 -> TC03: the tree changed, terminals cache pools by that label, and AgIsoStack logs an explicit error on a reused one. Also notable, and why the previous reasoning went wrong: the TC's ServerOptions capability bits have no tramline flag at all, so the "TC-GEO (with/without pos): Y/N" line says nothing about whether 513/514 will ever arrive. 79/79 native tests pass; teensy41_isobus builds clean. Not yet field-tested. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Research which Task Controllers implement Tramline Control (#21) Background for #21, where DDI 513/514 were traced to the AEF Tramline Control feature. Establishes what can be expected from real terminals before spending tractor time on it. Headline: Tramline Control only became AEF-certifiable on 2026-08-31, released as TRACK Generation 1 together with its conformance test. Before that it was a DDI definition plus a requirements document with no certification path, so any tramline interop in the field today was built vendor-to-vendor. Certification data barely exists yet -- absence from the AEF ISOBUS Database means nothing for at least a year. Gen 1 covers Levels 1 and 2 only; Level 3 (the TC calculates tramlines) is deferred to an unreleased Generation 2, so no TC can be certified for it today. For the InCommand 1200 specifically: no public evidence either way, and a trap worth knowing about. It does have a feature called "Tramlines", but it sits in the Guidance and Steering chapter next to AB lines and SmartPath -- it flashes the pass number under the on-screen lightbar and no implement is in the loop. Searching "InCommand tramline" lands on that and misleads. Confirmed TC-side implementers from their own current pages: Fendt, and Amazone (the only vendor naming a level -- Level 1). CCI likely. John Deere leads the AEF TRACK project team but publishes nothing about shipping support; the report flags that as its most over-readable item. Also finds that tramline control is mostly NOT negotiated over ISOBUS in practice -- the drill usually does its own logic -- and that even where it is, at Levels 1-2 the TC only publishes guidance-track info and honours the 505/506 handshake rather than computing anything. Sec 7 flags a miscitation in ISOBUS_TC_Manufacturer_Comparison.md (reference 2 cites the TRAM-becomes-TRACK article for a TC-GEO naming point; that article is entirely about tramline control). Flagged, not edited. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Fix a miscited (and incorrect) TC-GEO naming claim ISOBUS_TC_Manufacturer_Comparison.md sec. 1 stated that "Variable Rate Control" is AEF's customer-facing marketing term for TC-GEO, citing the AEF article "AEF Announces Update: TRAM becomes TRACK". That article is entirely about the Tramline Control functionality being renamed TRACK and says nothing about TC-GEO -- found while researching Tramline Control for #21. Checking the claim rather than just re-citing it showed the claim itself was wrong. Two AEF primary sources: the AEF Tour page (already reference 1) lists "TC-GEO (Task-Controller geo-based)" and never uses the phrase "variable rate control"; AEF's own "ISOBUS in Functionalities" leaflet expands it as "Task Controller geo-based (variables)". AEF publishes no customer-facing marketing name for TC-GEO. "Variable rate control" is industry and vendor description of what TC-GEO enables, not an AEF functionality name -- a distinction that matters when reading a certificate or the AEF database. The pattern the original line reached for is real: AEF does separate technical from customer-facing names, which is exactly what the TRAM/TRACK article describes for tramline control. It just doesn't apply to TC-GEO. Corrects the text, adds a dated correction note per the convention used elsewhere in these docs, and repoints reference 2 at the leaflet. Also updates TramlineControl_TC_Support_Research.md sec. 7, which said it was flagging rather than editing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Record that vendor patch #4 diverges from upstream design intent (#22) Filed the roll-call eviction findings upstream as AgIsoStack-plus-plus#717. While preparing it, upstream's own CoreTest.InvalidatingControlFunctions turned out to explicitly assert that a partnered control function which misses a roll-call goes address-invalid and reports Offline. That is a named, intentional test, so pruning partners is upstream design rather than an oversight, and patch #4 is a deliberate local divergence rather than a bug fix awaiting acceptance. Only the companion fix -- crediting the liveness flag when update_address_table() restores a pruned CF -- was offered as a PR (AgIsoStack-plus-plus#718), since it breaks no existing test and is unambiguous. The pruning policy itself was put to the maintainers as a design question, with the field evidence, rather than asserted as a defect. Matters for whoever reapplies these after a clean .pio rebuild: unlike patches #1-#3, patch #4 has no upstream release that will eventually retire it. If #718 lands, re-test whether patch #4 is still needed at all -- a durable restore may degrade the failure from permanent to transient on its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Apply the restored-CF liveness fix locally as vendor patch #5 (#22) update_address_table()'s restore branch returns a pruned control function to controlFunctionTable without setting claimedAddressSinceLastAddressClaim- Request, so it re-enters already eligible for pruning and is evicted again on the next roll-call. That is what made Session 6's failure self-sustaining rather than transient. Applied to the gitignored .pio/libdeps tree per the existing convention and documented here as patch #5. Build-verified (teensy41_isobus builds clean); not field-verified. With patch #4 also applied this should be a no-op for our own VT/TC partners by construction -- it matters for every other CF on the bus, and for anyone taking patch #4's reasoning without patch #4 -- which is why it was safe to apply without a tractor, though that reasoning is itself worth confirming in the field rather than assuming. Offered upstream as AgIsoStack-plus-plus#718 and mirrored to the fork we actually consume as AgIsoStack-Arduino#17; the latter is the one that would retire this entry. Patch #4 was deliberately NOT offered as a PR, since upstream's own CoreTest.InvalidatingControlFunctions asserts the behaviour it changes -- that went to the maintainers as a design question instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Log the full 8-byte PGN 65535 payload for the Ag Leader capture (#20) Deriving Ag Leader's XTE layout on this overloaded proprietary PGN needs every byte of the payload logged against ground-truth XTE read off the terminal. Until now only 3 of the 8 bytes were exposed (rawWord = d[3..4], rawByte1 = d[1]) -- and those three are the John Deere layout's fields specifically, which is exactly the assumption #20 exists to replace. A field session without this would come back with the same data it went out with. Captured on the same terms as the existing diagnostics: length guard passed and the source address recognised, independent of `valid`, so an Ag Leader sender still yields a full capture even though it is no longer decoded. Printed in two places, for two different jobs: - Full dump: the whole payload with an age, for reading on the spot. - Periodic line: `xteraw=<SA>:<16 hex>`, so a serial capture is time-correlated. That is the one that matters -- matching bytes against values an operator calls out only works if each sample carries the same timestamp as everything else on the line. Sampled at the periodic 1 Hz rather than per message: the PGN arrives ~10 Hz but real XTE moves over seconds, so 1 Hz is ample and keeps the log readable. Source address is included because the whole problem is that this PGN is shared between vendors with different layouts. Hex is zero-padded via a local helper -- Arduino's print(x, HEX) drops leading zeros, which would shift byte boundaries in an offline analysis (0x03 must read "03", not "3"). 80/80 native tests pass; teensy41_isobus builds clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Add VT/TC reconnect watchdogs and partner-address visibility (#18) Both clients now force a clean re-attempt after a sustained outage that follows a successful connection, so a transient bus event degrades to a stutter instead of a dead session needing a Teensy power cycle. Two things the issue's proposed fix would have got wrong, found by reading AgIsoStack rather than assuming: - VirtualTerminalClient::initialize() is guarded by `if (!initialized)`, so calling it again on a live client -- which is what #18 suggested -- is a no-op and would not have restarted anything. A real re-attempt needs terminate() first, which drops the PGN callbacks and resets the state machine so initialize() actually rebuilds them. terminate()'s delete-object-pool branch only runs when Connected, which we are not, so this is just teardown plus a reset. - TaskControllerClient has a purpose-built restart(), and its initialize() registers PGN callbacks *unconditionally* -- so calling initialize() twice there would double-register them. restart() is used instead. Neither watchdog arms until the first successful connection: the initial handshake has its own timing (the TC's includes a six-second startup delay), and firing during it would interrupt a connection that was progressing. That deliberately leaves the never-connected case alone -- that was #19, whose cause was the partner being evicted from AgIsoStack's control-function table, whichE restarting the client would not have fixed. Thresholds are 10 s (VT) and 15 s (TC), both comfortably longer than the clients' own timeouts and retry paths, with equal retry spacing so a partner that is genuinely gone produces one log line per interval rather than churning callback registrations. Also surfaces each partner's address and get_address_valid() in the debug menu, with a reconnect-attempt count. That is the signal whose absence made #17 take a source-diving session to find: a partner reading valid=N while the terminal is plainly alive on screen is the control-function eviction, and now says so directly. 80/80 native tests pass; teensy41_isobus builds clean. Not field-verified -- testing it means deliberately power-cycling a terminal mid-session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Clamp the VT's XTE variable, and give the TC client our VT for language data Two small fixes from the #20/#21 follow-up lists. Clamp (#20): updateVtVariables() biases XTE by +1000 so the VT's uint32 variable stays non-negative, which silently assumes +/-10 m. Out of that range the value wrapped through the cast and rendered as a plausible-looking huge number -- Session 6 saw the VT display 42949532.47, which decodes as (uint32)(-14049). Enforce the assumption instead of implying it, so a bad reading pegs visibly at the limit. Lower priority since 0x80 no longer commits an XTE at all, but correct regardless of where the value comes from. Language partner (#21): TaskControllerClient's third constructor argument is the primary VT's *partnered control function*, not a VirtualTerminalClient -- we were passing nullptr. AgIsoStack's select_language_command_partner() uses it to source ISO 11783-7 language/unit data whenever the TC server is older than version 4, falling back to a global request with "no VT was provided ... might not be ideal" when it is null. Every TC we have met reports version 3, so that fallback is always what ran. Checked before changing something that currently works: the VT is only ever consulted for language data, the request is bounded by the same six-second timeout as before, and the state machine proceeds to ProcessDDOP regardless. Worst case is unchanged behaviour with a different addressee. The parameter defaults to nullptr, so passing nothing restores exactly the old path. main.cpp already constructed the TC interface after gVtInterface->Begin(), so the VT partner exists by then; both call sites now say why the order matters. 80/80 native tests pass; teensy41_isobus and teensy41_serial both build clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Decode lat/lon from both position PGNs (Session 1 bug #6) The debug dump has read "Lat/Lon: 0.000000 / 0.000000" on live rigs since Session 1, with a fresh fix age next to it. Both ISOBUS position handlers only stamped the fix timer and never published coordinates -- and the NMEA2000 one had already decoded them, then thrown them away. Only the serial build's parsers ever called SetPosition(), so the ISOBUS build could never show a position at all. Session 1 deferred this pending a decision on whether anything needs live coordinates. Answered by checking: nothing in the control path reads them. The sole consumer of GuidanceSource::GetLatitude/GetLongitude is IsobusDebugMenu's dump -- ImplementPlough and InterfacePlough use XTE, speed, quality and fix ages. So this is a diagnostics fix, which is also why it is low risk. Kept the fix-age semantics identical, deliberately. NoteGgaFixReceived() is still gated on fixPresent alone, exactly as before; coordinates are published separately behind hasCoordinates. An implausible reading therefore costs a coordinate but never a fix -- which matters because InterfacePlough drops to HOLD on a stale GGA fix, so widening that gate would have been a safety change disguised as a diagnostics one. SetPosition() stamps the same timestamp, so the fix age cannot shift either way. Legacy decode (PGN 65267) follows CanSerialParser's CAN_POS case, which reads the same wire format off the serial transport: little-endian uint32 biased by 2100000000, 1e-7 degree units. Cross-validated in a test against that decoder's own known-good fixture "$0CFEF31C,00072A9C80652680" (lat=52.0degN, lon=5.0degE) -- the same cross-transport trick the John Deere XTE test uses, so this rests on math verified against real hardware years ago rather than on my reading of it. Added a shared plausibility guard (lat +/-90, lon +/-180) since the legacy encoding has no documented not-available sentinel; the NMEA2000 path keeps its 0x7FFFFFFF check as the primary one. 83/83 native tests pass; teensy41_isobus builds clean. Coordinates themselves are not field-verified -- worth a glance at the dump next session to confirm they match the terminal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Field-test the DDI 505/506 Tramline Control handshake on an Ag Leader InCommand 1200 (#21) (#28) * Probe the DDI 505/506 Tramline Control handshake on an InCommand 1200 (#21) Field session 7. The InCommand 1200 sends no DDI 506 reply under either declaration: 16 min with DDI 505 = 0 (no level claimed), and 6 min with 505 = 0x01 (Level 1), zero Value Commands and zero Value Requests in both. Both runs logged "DDOP Activated without error", so the pool carrying 505/506 was accepted each time. Run 2 exists to close the ambiguity the probe's own comment flagged: a TC that short-circuits the handshake for a zero-capability implement is indistinguishable from one with no Tramline Control at all. Claiming Level 1 removes that escape. Structure label bumped TC03 -> TC04 because the DDOP's declared value changed and terminals cache pools by that label -- without it the terminal serves run 1's pool and the re-run proves nothing. The Level-1 value is NOT shippable: Level 1 means "the implement calculates the tramline tracks", which a plough does not do. It is a diagnostic claim made to force a reply, which is why this stays on a spike branch. Revert to 0, or build the feature out honestly, before this reaches isobus-tc-client. The result is provisional on one unclosed confound, recorded in the notes: zero Value *Requests* is equally consistent with our implement never having been mapped into the running task on the terminal, in which case both runs measured nothing. "Task active: Y" does not settle it -- AgIsoStack warns that flag is unreliable per brand. Also field-verified for the first time, independent of the TC work: all three VT soft keys arrive from a real terminal -- BREDER x5, SMALLER x3, AUTO x2 (AUTO's "not wired to control" is intended). Wired 2026-08-18, never pressed on hardware until today. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Revert the Level-1 probe claim, and record the CANedge capture for #20 Session 7's run 2 declared DDI 505 = 0x01 (Level 1) to force a Tramline Control reply out of the InCommand 1200. It did not produce one, and the claim is not honest firmware: Level 1 means "the implement calculates the tramline tracks", which a plough does not do. Reverted to 0, and the structure label with it (TC04 -> TC03), which the reverted pool matches again. Firmware behaviour is therefore identical to where the session started; only findings, logs and commentary change. TC04 is recorded in the source as burned -- terminals that took part in run 2 still cache a Level-1 pool under that label, so the next real DDOP tree change must go to TC05, not back to TC04. Also records the CANedge full-bus capture taken during the session, with ground truth called out live at 71 cm, then 1 cm, then 0 cm. That descending sequence is what #20 has needed since session 3: a capture containing changes of known size cannot be satisfied by a frozen field, unlike session 6's single static reading that matched by coincidence. Notes the better use of it too -- those figures are what the terminal displayed, so the terminal was transmitting that quantity on the bus throughout. Identifying the frames that pass through 71, 1 and 0 in order yields continuous ground truth at full rate, and any zero crossing in it would settle the sign convention that has been open since session 6 without another tractor session. 83/83 native tests pass (MSVC runner); teensy41_isobus builds clean. Note `pio test -e native` fails to discover the suite on this machine, identically on an unmodified tree -- pre-existing, unrelated to this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Record the full XTE ground-truth sequence, including both zero crossings (#20) The CANedge session log is accompanied by five live-called ground-truth points, not three: 71 cm, 1 cm, 0 cm, 99 cm on the other side of the line, then back to 0 cm. The crossings are the valuable part. Session 6 left "sign convention remains unverified" open because the operator's left/right calls were corrected mid-sequence, leaving four magnitudes reliable but unsigned. Here the ordering itself carries the sign, and it crosses zero twice -- out to the far side and back. A wrong hypothesis can fake one sign flip via an unrelated bit that toggled once; reproducing an out-and-back through zero on the correct field is not something it does by accident. So the derivation target for #20 is now stricter and better posed: find the frames whose decode tracks all five magnitudes AND the flip. That should settle both the layout and the sign convention from this log alone, with no further tractor session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Add the sixth XTE ground-truth point and the capture's end marker (#20) Sequence is now 71 / 1 / 0 / 99-far-side / 0 / 52-far-side: six points, three zero crossings, and two distinct magnitudes on the far side rather than one. The second far-side value matters beyond redundancy. A decode that saturates or latches when the sign flips can still look correct against a single far-side reading; it has to render 99 and 52 distinctly to survive this sequence. Three ordered crossings likewise rule out a wrong field whose sign happens to toggle once. Also records that the capture ends with the plough control being disconnected, which gives a clean end marker on the bus -- our control function drops off and everything after is other traffic. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Arjan-Woltjer
marked this pull request as ready for review
September 11, 2026 21:32
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
CANNetworkManager::update_new_partners()copiesaddressandcontrolFunctionNAMEonto aPartneredControlFunctionwhen it's late-boundto 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
falseeventhough 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 nevertrueto begin with, the partner gets evicted 755ms later even though the real
device never stopped being valid.
Once evicted, its
controlFunctionTableslot is nulled, andprocess_can_message_for_global_and_partner_callbacks()silently dropsevery subsequent broadcast from that source address, since
message.get_source_control_function()no longer resolves. This is notlimited 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
mainas of 2026-07-09). @sujandumaru's commentthere 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
currentActiveControlFunctionrather than hardcodingtrue.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 owninternal 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.