Implement command busy state tracking in the TaskControllerServer#689
Implement command busy state tracking in the TaskControllerServer#689gunicsba wants to merge 32 commits into
Conversation
Also adds extra measurement commands logging and clarifies some documentation
According to: https://www.isobus.net/isobus/attachments/345/ISO11783-11-DDI-290-SetpointWorkState-v1.pdf Each Section Device Element shall at least provide one type of Working Width. If more than one type of Working Width is provided, then the Section Controller shall be capable to use the different Working Width types with the following priority: 1. Actual Working Width (DDI 67) 2. Maximum Working Width (DDI 70) 3. Default Working Width (DDI 68)
…IsoStack-plus-plus into tc-fix-working-width
width_mm is kept for backward compatibility.
… into tc-fix-working-width
…ssDDOP stated dopLocalizationLabel is a std::array<std::uint8_t, 7>, so .empty() always returns false. This prevented process_labels_from_ddop() from ever being called for user-provided binary DDOPs, causing the state machine to proceed with uninitialized label data. Replaced with a comparison against a zero-initialized array to correctly detect the sentinel "not yet populated" state.
This change modifies the behavior of our address claiming process to more readily evict a control function on the bus from its address if our internal control function would win contention. Previously we were being "nice" and always finding an open address. Although our earlier behavior was supported by the "Build an address table" section of ISO 11783-5, This change will improve an internal control function's ability to be placed at a desired address. This flexibility is probably good, but if you have low quality J1939 devices that fail to properly do the address claim process, you might still want to avoid conflicting with them in your preferred address.
… B.8.1 Implement B.6 command busy state tracking in TaskControllerServer to inform clients when the server is busy executing device descriptor commands. Changes: - Add set_b6_command_busy() public API for manual busy state control - Initialize currentCommandSourceAddress to 0x00 (was NULL_CAN_ADDRESS) - Track busy state during ObjectPoolTransfer command processing - Track busy state during ObjectPoolActivateDeactivate command processing - Clear busy state automatically on client timeout - Force immediate TC Status message broadcast on busy state changes - Add comprehensive unit tests for all busy state scenarios This implementation is ISO 11783-10 B.8.1 compliant for TC Status message Bytes 5-7 (currentCommandSourceAddress, currentCommandByte).
…ulture/AgIsoStack-plus-plus into feat_tc_b6_state_tracking
…IsoStack-plus-plus into feat_tc_b6_state_tracking
| /// @param[in] isBusy Whether the server is busy executing a B.6 command. | ||
| /// @param[in] clientAddress The CAN address of the client that sent the command (0x00 if not busy). | ||
| /// @param[in] commandByte The B.6 command byte being executed (0x00 if not busy). | ||
| void set_b6_command_busy(bool isBusy, std::uint8_t clientAddress = 0, std::uint8_t commandByte = 0); |
There was a problem hiding this comment.
We don't use ISO document naming convention anywhere else in any function, so I'd suggest just removing it for consistency.
Also, 0x00 isn't a great default, since 0 is a valid client address... FF or FE would maybe be better? Or maybe an overload or different signature for when not busy that just excludes this?
There was a problem hiding this comment.
I come to the conclusion about the 0x00 from the TC running on a Valtra screen. I'll try to gather more data when I can about what others use as a standard.
There was a problem hiding this comment.
But that's what the PDF says. Send 00 by default unless we have a DDOP upload in progress in which case it will change to the client address.
| numberChannelsSupportedForPositionBasedControlToReport(numberChannelsSupportedForPositionBasedControl), | ||
| optionsBitfieldToReport(options.get_bitfield()) | ||
| optionsBitfieldToReport(options.get_bitfield()), | ||
| currentCommandSourceAddress(0x00) |
There was a problem hiding this comment.
I mentioned in another comment, but 0 is a bad default, since G.2 of the ISO doc specifies explicitly that it should be FF, FE, or the address of the VT when no command is executing.
There was a problem hiding this comment.
ISO_11783-10_2015(en).pdf Annex B 8.2 says otherwise.
| currentCommandSourceAddress = 0x00; | ||
| currentCommandByte = 0x00; | ||
| } | ||
| lastStatusMessageTimestamp_ms = 0; // Force a status message to be sent on the next update. |
There was a problem hiding this comment.
Can theoretically violate the part of the ISO spec that states "at least 200 ms shall elapse between Task Controller Status messages". I think there probably needs to be a bit more logic to make something like this work in a compliant way
|
@ad3154 I fortunately have some logs of 2 different John Deere tractors. SF3000 and 6000 antennas one of them even had an ISOBUS sprayer attached. I asked chatgpt to analyse the logs and see what status they send. Here's the result: Task Controller Status bytes 6–8: comparison with John Deere / Valtra logs I checked several CAN logs from John Deere / Valtra systems to see what real OEM Task Controllers send for the Task Controller Status Process Data message. The relevant message is: PGN: 0x00CB00 / Process Data Observed OEM TC Status frames From Valtra SmartTouch: ID = 0x0CCBFFF7 From John Deere Task Controllers, identified by Address Claim NAME: manufacturer = 33 Observed TC Status payloads: FE FF FF FF 00 00 00 00 So JD / Valtra appear to use: Byte 1 = FE // element number high nibble not available + TC Status command 0xE when the TC is not currently reporting an active B.6 / DDOP command. Comparison with current AOG / AgIsoStack behaviour In our logs we saw two TC Status-like frames from the same TC source address: 0x0CCBFFF7 FE FF FF FF 01 FE 00 FF The first four bytes match the OEM pattern: FE FF FF FF But bytes 5–8 differ. The suspicious one is: FE FF FF FF 01 FE 00 FF Here: Byte 5 = 01 // task totals active This does not match the JD / Valtra logs I checked. About byte 6 and "0x00 could be a valid CAN source address" I agree that "0x00" should not be used as a "no client" sentinel if byte 6 is currently valid, because "0x00" can be a valid CAN source address. However, the OEM logs suggest a different interpretation: If the TC is not busy with a B.6 / DDOP command: In that state, byte 6 and byte 7 are not meaningful because the status byte does not mark them as active/valid. Only when the TC status indicates that it is busy executing a B.6 / DDOP command should byte 6 and byte 7 carry meaningful values: If the TC is busy with a B.6 / DDOP command: So the concern about "0x00" being a valid source address is valid for the busy/active case, but JD / Valtra still appear to use "00 00 00" for bytes 6–8 when no B.6 command is active. Suggested payloads Idle / no active task totals: FE FF FF FF 00 00 00 00 Compatibility mode / task totals active: FE FF FF FF 01 00 00 00 Busy with a B.6 / DDOP command: FE FF FF FF 00 Recommendation I think we should stop sending: FE FF FF FF 01 FE 00 FF and instead align with the OEM behaviour: FE FF FF FF 00 00 00 00 or, if we intentionally want to advertise task totals active: FE FF FF FF 01 00 00 00 The main point is that byte 6 should not be "0xFE" unless the status byte says the B.6 command fields are valid, and byte 8 should likely be "0x00", matching the JD / Valtra examples. |
200ms minimum interval is respected. removed the ISO references from the function names and comments.
gunicsba
changed the title
4 participants
Summary
Implements B.6 command busy state tracking in the TaskControllerServer according to ISO 11783-10 B.8.1 specification. This feature informs ISOBUS clients when the server is busy executing device descriptor commands (ObjectPoolTransfer or ObjectPoolActivateDeactivate), preventing command collisions and improving protocol compliance.
We often have to restart the implements and there are still some that refuse to talk with us. I suspect it's because our TC status message prior to this change looked like we're busy with a DDOP upload from NULL_CAN_ADDRESS ECU.
Changes
Core Implementation
set_b6_command_busy(bool isBusy, std::uint8_t clientAddress = 0, std::uint8_t commandByte = 0)currentCommandSourceAddressinitialization fromNULL_CAN_ADDRESSto0x00ISO 11783-10 Compliance
Implements TC Status message format per B.8.1:
currentCommandSourceAddress): CAN address of client executing B.6 command (0x00 when idle)currentCommandByte): B.6 command byte being executed (0x00 when idle)Testing
Added three comprehensive unit tests:
B6CommandBusyStateTracking- Direct API testingB6CommandBusyState_ObjectPoolTransfer- Integration test for transfer commandsB6CommandBusyState_ObjectPoolActivateDeactivate- Integration test for activate/deactivate commandsFiles Modified
isobus/include/isobus/isobus/isobus_task_controller_server.hpp- Added public APIisobus/src/isobus_task_controller_server.cpp- Implementationtest/tc_server_tests.cpp- Unit testsBenefits
✅ Prevents command collisions during DDOP operations
✅ Improves ISOBUS protocol compliance
✅ Better client-server coordination
✅ Automatic cleanup on timeout scenarios
✅ Fully backward compatible (no breaking changes)
Testing
References