Conversation
… implement as of now.
…askController into tramlineSymmetrica18m6mhack # Conflicts: # include/task_controller.hpp # src/app.cpp # src/task_controller.cpp
…assymetrical. Requires a special AgOpenGPS version that sends the track information.
Contributor
Author
|
To test we need this version of AgOpenGPS: There's also a PR: |
… understand the random crash of our TC.
Same fixes already applied on fix/crash-handling-and-thread-safety (the develop-targeted branch this code was ported to): - clang-format the file (was never run before the previous commit). - cmake-format the new CMakeLists.txt comment block. - The POSIX fatal_signal_handler() called std::strlen() inside signal context; strlen isn't on POSIX's async-signal-safe function list, so using it here could itself deadlock/crash while handling SIGSEGV/SIGABRT. Replaced with a hand-rolled length count. - std::snprintf() was used without including <cstdio>, compiling only via a transitive include on this toolchain. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… Data ActualWorkingWidth/MaximumWorkingWidth/DefaultWorkingWidth (DDI 67/68/70) can be declared as a Device Process Data (DPD) object instead of a Device Property (DPT). A DPD has no value embedded in the DDOP at all - only a definition - so DeviceDescriptorObjectPoolHelper::get_implement_geometry(), a pure static-pool parser, could never see it. The KUHN ESPRO reports its width this way, which is why our logs never showed it despite the DDOP containing valid section/sub-boom structure. Subscribe to these DDIs per element in request_measurement_commands(), capture the reported value in on_value_command(), and fall back to it in derive_implement_details() whenever the static pool doesn't have a value - same Actual > Maximum > Default priority the library already uses. Verified against a live KUHN ESPRO: "Implement: ESPRO (2 sections, 6.00 m)". Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
process_rx_messages() dereferenced rxMessage.get_source_control_function() unguarded in the TechnicalCapabilities case (directly via get_address() in two LOG_INFO calls, and indirectly via get_active_client(...)->reportedVersion). The source control function can legitimately be null - address-claim churn can queue a message before the network manager has resolved a ControlFunction for its source address - and process_rx_messages() drains that queue on a delay via the store_rx_message()/process_rx_messages() split, so a burst of claim activity (an implement timing out while a second VT claims an address) could crash with an access violation well after the message was received. Confirmed via a field minidump: SEH exception 0xC0000005 reading 0x19 inside ControlFunction::get_address(), called from TaskControllerServer::process_rx_messages(), called from TaskControllerServer::update(), called from Application::update() Line 787 - a real call stack from Visual Studio, not a heuristic guess. Fixed upstream in gunicsba/AgIsoStack-plus-plus@e79699e (null-checks both the source control function and get_active_client()'s return, mirroring the existing guard pattern in the DeviceDescriptor case). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
AOG's own PGN 0xF4 guidance reference ID is only a 16-bit value scoped to whichever field is currently open in AOG - switching fields can land on the same raw ID a different field already used. An implement that caches per-track state (e.g. an offset) keyed on DDI 508 (Unique A-B Guidance Reference Line ID) alone can then apply a stale offset from the wrong field. Add a FieldRegistry that assigns each field name a stable, persistent index (backed by its own field_registry.csv under the app's config directory, kept separate from settings.json so it can be reset independently), and fold that index into the upper 16 bits of the 32-bit DDI 508 value while leaving AOG's own 16-bit ID in the lower 16 bits untouched - 100 tracks/field x 1000 fields needs 17 bits; this gives 16+16 with room to spare. Wires up PGN 0xF3 (Field Name) handling, which didn't exist in this codebase before. The documented wire layout (a length byte at offset 4, name at 5+) turned out not to match what AOG actually sends, confirmed against a live packet: the whole payload is just the raw UTF-8 name, no length prefix, and an empty payload means the field is closed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
1 participant
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.
This pull request introduces significant enhancements to both the codebase and documentation, primarily focused on supporting new ISOBUS features, improving protocol clarity, and expanding test coverage. The main themes are: expanded documentation for new PGNs and DDIs (especially for tramline/TRACK and tractor facilities), new code to support these features (including time/date and tractor facilities interfaces), and the addition of unit tests for tractor facilities encoding/decoding.
Documentation improvements and protocol clarifications:
docs/PROTOCOL.mdto document support for new inbound/outbound PGNs, especially GPS/IMU data (0xD6), machine data (0xEF), and guidance track context (0xF4). Added detailed explanations of their payloads, usage, and fallback behaviors. Also clarified source addresses and maximum payload sizes. [1] [2] [3] [4]Codebase enhancements for new ISOBUS features:
include/app.hppto add members and interfaces forTractorFacilities,TimeDateInterface, and related state (e.g., FEE6 broadcast timing, GNSS fix quality, guidance track context, and tramline/TRACK control state). This enables the application to broadcast tractor facilities, time/date, and track context, and to track AOG connectivity and GNSS quality. [1] [2] [3] [4]Testing improvements:
test_tractor_facilities) for Tractor Facilities (PGN 65033) encode/decode logic, ensuring correctness of this new protocol feature.Documentation and protocol expansion:
0xD6,0xEF,0xF4) and clarified their payloads, sources, and fallback behaviors indocs/PROTOCOL.md. [1] [2] [3] [4]ISOBUS feature support in code:
Applicationclass ininclude/app.hppto support tractor facilities, time/date interface, GNSS quality tracking, guidance track context, and tramline/TRACK control state. [1] [2] [3] [4]Testing:
CMakeLists.txt).