From 55f8549b4f11c88cffea2be85b162825b9c4226b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gunics=20Bal=C3=A1zs?= Date: Wed, 11 Nov 2026 13:24:42 +0100 Subject: [PATCH 1/6] Add allSectionsFollowFirst: drive every implement section from AOG section 1 For implements that declare several sections that really are one, such as an RDS Artemis with three dispensers on one working width. With the setting on, every implement section follows AOG's section 1, and the heartbeat reports a single section to AOG (on while any implement section is on). Default off. Co-Authored-By: Claude Opus 5.5 --- include/settings.hpp | 10 ++++++++++ readme.md | 1 + src/app.cpp | 19 +++++++++++++++++++ src/settings.cpp | 13 +++++++++++++ 4 files changed, 43 insertions(+) diff --git a/include/settings.hpp b/include/settings.hpp index 06f0278..04d052a 100644 --- a/include/settings.hpp +++ b/include/settings.hpp @@ -98,6 +98,14 @@ class Settings */ bool set_vt_enabled(bool enabled, bool save = true); + /// @brief Check whether every implement section follows AOG's section 1. + /// @details For implements that declare several sections (or booms) that really are one, such as + /// several dispensers on one working width. AOG then sees the implement as a single section. + bool is_all_sections_follow_first() const; + + /// @brief Set whether every implement section follows AOG's section 1. + bool set_all_sections_follow_first(bool enabled, bool save = true); + /** * @brief Get the configured TC version * @return The configured version (0-4, default 3) @@ -161,6 +169,7 @@ class Settings constexpr static bool DEFAULT_NMEA_SEND_ENABLED = true; constexpr static bool DEFAULT_AOG_HEARTBEAT_ENABLED = true; constexpr static bool DEFAULT_VT_ENABLED = true; + constexpr static bool DEFAULT_ALL_SECTIONS_FOLLOW_FIRST = false; constexpr static std::uint8_t DEFAULT_TC_VERSION = 3; ///< SecondEditionDraft (V3 default for maximum implement compatibility) static const std::string DEFAULT_LANGUAGE_CODE; static const std::string DEFAULT_COUNTRY_CODE; @@ -169,6 +178,7 @@ class Settings bool nmeaSendEnabled = DEFAULT_NMEA_SEND_ENABLED; bool aogHeartbeatEnabled = DEFAULT_AOG_HEARTBEAT_ENABLED; bool vtEnabled = DEFAULT_VT_ENABLED; + bool allSectionsFollowFirst = DEFAULT_ALL_SECTIONS_FOLLOW_FIRST; std::uint8_t tcVersion = DEFAULT_TC_VERSION; std::string languageCode = DEFAULT_LANGUAGE_CODE; std::string countryCode = DEFAULT_COUNTRY_CODE; diff --git a/readme.md b/readme.md index 8c67159..1ab0222 100644 --- a/readme.md +++ b/readme.md @@ -64,6 +64,7 @@ AOG-TaskController reads its configuration from a `settings.json` file located i | `nmeaSendEnabled` | `bool` | `true` | Enables cyclic NMEA2000 COG/SOG transmission. Requires `tecuEnabled`. | | `aogHeartbeatEnabled` | `bool` | `true` | Sends the 100 ms zero-section heartbeat when no implement is connected. Disable for AgOpenGPS versions before v6.8.2 beta 5. | | `vtEnabled` | `bool` | `true` | Enables the ISOBUS Virtual Terminal UI. | +| `allSectionsFollowFirst` | `bool` | `false` | Every implement section follows AOG's section 1, and AOG sees the implement as one section (on while any section is on). For implements that declare several sections or booms that really are one, such as several dispensers on one working width. Set AOG to 1 section. | | `tcVersion` | `integer` | `3` | Task Controller version code, from `0` through `4`; `3` is Second Edition Draft. | | `languageCode` | `string` | `"en"` | Two-character language code advertised on ISOBUS. | | `countryCode` | `string` | `"US"` | Two-character country code advertised on ISOBUS. | diff --git a/src/app.cpp b/src/app.cpp index a697e8b..20cb58d 100644 --- a/src/app.cpp +++ b/src/app.cpp @@ -507,6 +507,11 @@ void Application::setup_udp_connections() sectionDemand = sectionStates; sectionDemandChangesSinceLog++; } + if (settings->is_all_sections_follow_first()) + { + // The implement's sections are really one: all follow AOG's section 1 (sectionDemand above stays as AOG sent it) + std::fill(sectionStates.begin(), sectionStates.end(), sectionStates.front()); + } tcServer->update_section_states(sectionStates); } else if (pgn == 0xF1 && !data.empty()) // 241 - Section Control @@ -739,6 +744,20 @@ bool Application::update() } anyClientWithSections = true; + + if (settings->is_all_sections_follow_first()) + { + // AOG is set up with one section: report it on while any implement section is on + bool anyOn = false; + for (std::uint8_t i = 0; i < state.get_number_of_sections(); i++) + { + anyOn = anyOn || (state.get_section_actual_state(i) == SectionState::ON); + } + std::vector oneSection = { state.is_section_control_enabled(), 1, static_cast(anyOn ? 1 : 0) }; + udpConnections->send(0x80, 0xF0, oneSection); + continue; + } + std::vector data = { state.is_section_control_enabled(), state.get_number_of_sections() }; std::uint8_t sectionIndex = 0; diff --git a/src/settings.cpp b/src/settings.cpp index 960927a..cdc73d8 100644 --- a/src/settings.cpp +++ b/src/settings.cpp @@ -70,6 +70,7 @@ bool Settings::load() nmeaSendEnabled = loadBoolean("nmeaSendEnabled", DEFAULT_NMEA_SEND_ENABLED); aogHeartbeatEnabled = loadBoolean("aogHeartbeatEnabled", DEFAULT_AOG_HEARTBEAT_ENABLED); vtEnabled = loadBoolean("vtEnabled", DEFAULT_VT_ENABLED); + allSectionsFollowFirst = loadBoolean("allSectionsFollowFirst", DEFAULT_ALL_SECTIONS_FOLLOW_FIRST); if (data.contains("tcVersion")) { @@ -143,6 +144,7 @@ bool Settings::save() const data["nmeaSendEnabled"] = nmeaSendEnabled; data["aogHeartbeatEnabled"] = aogHeartbeatEnabled; data["vtEnabled"] = vtEnabled; + data["allSectionsFollowFirst"] = allSectionsFollowFirst; data["tcVersion"] = tcVersion; data["languageCode"] = languageCode; data["countryCode"] = countryCode; @@ -261,6 +263,17 @@ bool Settings::set_vt_enabled(bool enabled, bool save) return set_boolean(vtEnabled, enabled, save); } +bool Settings::is_all_sections_follow_first() const +{ + std::scoped_lock lock(settingsMutex); + return allSectionsFollowFirst; +} + +bool Settings::set_all_sections_follow_first(bool enabled, bool save) +{ + return set_boolean(allSectionsFollowFirst, enabled, save); +} + bool Settings::is_aog_heartbeat_enabled() const { std::scoped_lock lock(settingsMutex); From a271aa75b3f50ddf4879d9abc0cac42d291ee1b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gunics=20Bal=C3=A1zs?= Date: Wed, 11 Nov 2026 13:52:14 +0100 Subject: [PATCH 2/6] Control each boom on its own; optionally merge overlapping sections into one AOG section Booms with their own condensed DDIs (e.g. an RDS Artemis with one Function per dispenser) number their sections from 1 each. The TC kept one element per DDI, so every setpoint and DDI 160 went to the last boom, with all sections packed into it; the other booms were never put in automatic. - Resolve, per section, the nearest element carrying DDI 290 (or settable 161). With two or more such owners, each gets its own setpoint (sections numbered within it) and its own DDI 160; DDI 161 and 160 reports are read per owner; automatic while any boom is. Implements with one owner keep the existing path (checked against the 24 sample DDOPs: none has more than one). - Request each section's Y offset and width when the DDOP only declares them as process data, and log them. - groupOverlappingSections (default off): sections with the same Y offset and width (10 mm) share one AOG section; the heartbeat reports AOG's sections. The mapping is logged even while the setting is off. - section_layout.hpp holds both decisions as pure functions, with unit tests; test_section_layout also describes captured DDOP files. Co-Authored-By: Claude Opus 5.5 --- CMakeLists.txt | 13 ++ include/section_layout.hpp | 186 ++++++++++++++++++ include/settings.hpp | 8 + include/task_controller.hpp | 45 +++++ readme.md | 1 + src/app.cpp | 11 +- src/settings.cpp | 13 ++ src/task_controller.cpp | 349 +++++++++++++++++++++++++++++++++- tools/test_section_layout.cpp | 245 ++++++++++++++++++++++++ 9 files changed, 861 insertions(+), 10 deletions(-) create mode 100644 include/section_layout.hpp create mode 100644 tools/test_section_layout.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 167ea70..d0938bf 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -207,6 +207,19 @@ if(AOG_TC_VALIDATE_IOP) add_dependencies(iop_validator test_element_hierarchy) add_test(NAME element_hierarchy COMMAND test_element_hierarchy) + # section_layout (condensed-DDI owners per boom, overlapping-section grouping) + # unit tests, built with the validation workflow for the same reason. + add_executable(test_section_layout + ${CMAKE_CURRENT_LIST_DIR}/tools/test_section_layout.cpp) + target_compile_features(test_section_layout PRIVATE cxx_std_20) + set_target_properties(test_section_layout PROPERTIES CXX_EXTENSIONS OFF) + target_include_directories(test_section_layout + PRIVATE ${CMAKE_CURRENT_LIST_DIR}/include) + target_link_libraries(test_section_layout PRIVATE isobus::Isobus + isobus::Utility) + add_dependencies(iop_validator test_section_layout) + add_test(NAME section_layout COMMAND test_section_layout) + # PASS_REGULAR_EXPRESSION makes CTest ignore the exit code, which is the part # a CI gate actually reads. add_test(NAME malformed_pool_exit_status COMMAND iop_validator diff --git a/include/section_layout.hpp b/include/section_layout.hpp new file mode 100644 index 0000000..e416d2e --- /dev/null +++ b/include/section_layout.hpp @@ -0,0 +1,186 @@ +/** + * @brief How an implement's sections map onto condensed-DDI owner elements and onto AOG's sections + * + * Pure functions, so they can be tested against sample DDOPs without a bus. + */ + +#pragma once + +#include "element_hierarchy.hpp" +#include "isobus/isobus/isobus_device_descriptor_object_pool.hpp" + +#include +#include +#include +#include +#include + +namespace section_layout +{ + /// @brief An element that carries condensed section DDIs (e.g. DDI 290 / 161) for a run of sections. + /// @details ISO 11783-10 numbers the sections of a condensed DDI within the element that carries it. + /// An implement with several booms (e.g. one Function per product) has one owner per boom, each with + /// its own section 1. + struct CondensedOwner + { + std::uint16_t elementNumber = 0; ///< Element carrying the condensed DDIs + std::uint8_t firstSection = 0; ///< Index of its first section in the TC's flat section list + std::uint8_t count = 0; ///< Number of consecutive sections it owns + }; + + /// @brief Whether an element links a process data object with a DDI in [firstDdi, lastDdi] + inline bool element_links_ddi(isobus::DeviceDescriptorObjectPool &pool, const isobus::task_controller_object::DeviceElementObject &element, std::uint16_t firstDdi, std::uint16_t lastDdi) + { + using isobus::task_controller_object::DeviceProcessDataObject; + using isobus::task_controller_object::ObjectTypes; + for (std::uint16_t childId : element.get_child_object_ids()) + { + auto child = pool.get_object_by_id(childId); + if (child && (child->get_object_type() == ObjectTypes::DeviceProcessData)) + { + const auto ddi = std::static_pointer_cast(child)->get_ddi(); + if ((ddi >= firstDdi) && (ddi <= lastDdi)) + { + return true; + } + } + } + return false; + } + + /// @brief Finds, for each section, the nearest element (the section itself or an ancestor) that links a + /// DDI in [firstDdi, lastDdi], and groups consecutive sections with the same owner. + /// @returns The owners in section order, or an empty vector when a section has no owner or one owner's + /// sections are not consecutive (callers then keep the single-element behaviour). + inline std::vector resolve_condensed_owners(isobus::DeviceDescriptorObjectPool &pool, + const element_hierarchy::ParentMap &parents, + const std::vector §ionElements, + std::uint16_t firstDdi, + std::uint16_t lastDdi) + { + using isobus::task_controller_object::DeviceElementObject; + using isobus::task_controller_object::ObjectTypes; + + std::map> elements; + for (std::uint16_t i = 0; i < pool.size(); i++) + { + auto object = pool.get_object_by_index(i); + if (object && (object->get_object_type() == ObjectTypes::DeviceElement)) + { + auto element = std::static_pointer_cast(object); + elements[element->get_element_number()] = element; + } + } + + std::vector owners; + for (std::size_t i = 0; i < sectionElements.size(); i++) + { + std::optional owner; + std::uint16_t current = sectionElements[i]; + for (std::size_t depth = 0; depth < 32; depth++) // depth guard against a cyclic pool + { + auto found = elements.find(current); + if ((found != elements.end()) && element_links_ddi(pool, *found->second, firstDdi, lastDdi)) + { + owner = current; + break; + } + auto parent = parents.find(current); + if (parent == parents.end()) + { + break; // Top of the element chain (the device element) + } + current = parent->second; + } + + if (!owner) + { + return {}; + } + if (!owners.empty() && (owners.back().elementNumber == *owner)) + { + owners.back().count++; + continue; + } + for (const auto &existing : owners) + { + if (existing.elementNumber == *owner) + { + return {}; // Sections of one owner are not consecutive + } + } + owners.push_back({ *owner, static_cast(i), 1 }); + } + return owners; + } + + /// @brief Lateral position of one section, as far as it is known + struct SectionGeometry + { + std::optional yOffset_mm; ///< DDI 135, left-/right+ + std::optional width_mm; ///< DDI 67 (or 70 / 68 when that is all there is) + }; + + /// @brief Result of grouping sections that cover the same strip of ground + struct AogSectionMap + { + std::vector aogSectionForSection; ///< Implement section -> AOG section + std::uint8_t numberOfAogSections = 0; + bool grouped = false; ///< true when at least two sections share an AOG section + }; + + /// @brief Identity mapping: implement section i is AOG section i + inline AogSectionMap identity_map(std::uint8_t numberOfSections) + { + AogSectionMap map; + for (std::uint8_t i = 0; i < numberOfSections; i++) + { + map.aogSectionForSection.push_back(i); + } + map.numberOfAogSections = numberOfSections; + return map; + } + + /// @brief Puts sections with the same lateral offset and width (within toleranceMm) into one AOG section. + /// @details AOG sections are numbered in the order their first section appears, so an implement without + /// overlapping sections keeps the identity mapping. Fore/aft (X) offset is ignored: several dispensers + /// one behind the other still cover the same strip. Returns the identity mapping unless every section + /// has a known Y offset and a width above zero. + inline AogSectionMap group_overlapping_sections(const std::vector §ions, std::int32_t toleranceMm = 10) + { + const auto count = static_cast(sections.size()); + for (const auto §ion : sections) + { + if (!section.yOffset_mm || !section.width_mm || (*section.width_mm <= 0)) + { + return identity_map(count); + } + } + + AogSectionMap map; + std::vector representative; // AOG section -> first implement section in it + for (std::size_t i = 0; i < sections.size(); i++) + { + std::optional aogSection; + for (std::size_t g = 0; g < representative.size(); g++) + { + const auto &first = sections[representative[g]]; + if ((std::abs(*first.yOffset_mm - *sections[i].yOffset_mm) <= toleranceMm) && + (std::abs(*first.width_mm - *sections[i].width_mm) <= toleranceMm)) + { + aogSection = static_cast(g); + break; + } + } + if (!aogSection) + { + aogSection = static_cast(representative.size()); + representative.push_back(i); + } + map.aogSectionForSection.push_back(*aogSection); + } + map.numberOfAogSections = static_cast(representative.size()); + map.grouped = (map.numberOfAogSections < count); + return map; + } +} // namespace section_layout diff --git a/include/settings.hpp b/include/settings.hpp index 04d052a..1b65de9 100644 --- a/include/settings.hpp +++ b/include/settings.hpp @@ -106,6 +106,12 @@ class Settings /// @brief Set whether every implement section follows AOG's section 1. bool set_all_sections_follow_first(bool enabled, bool save = true); + /// @brief Check whether sections that cover the same strip (same Y offset and width) share one AOG section. + bool is_group_overlapping_sections() const; + + /// @brief Set whether sections that cover the same strip share one AOG section. + bool set_group_overlapping_sections(bool enabled, bool save = true); + /** * @brief Get the configured TC version * @return The configured version (0-4, default 3) @@ -170,6 +176,7 @@ class Settings constexpr static bool DEFAULT_AOG_HEARTBEAT_ENABLED = true; constexpr static bool DEFAULT_VT_ENABLED = true; constexpr static bool DEFAULT_ALL_SECTIONS_FOLLOW_FIRST = false; + constexpr static bool DEFAULT_GROUP_OVERLAPPING_SECTIONS = false; constexpr static std::uint8_t DEFAULT_TC_VERSION = 3; ///< SecondEditionDraft (V3 default for maximum implement compatibility) static const std::string DEFAULT_LANGUAGE_CODE; static const std::string DEFAULT_COUNTRY_CODE; @@ -179,6 +186,7 @@ class Settings bool aogHeartbeatEnabled = DEFAULT_AOG_HEARTBEAT_ENABLED; bool vtEnabled = DEFAULT_VT_ENABLED; bool allSectionsFollowFirst = DEFAULT_ALL_SECTIONS_FOLLOW_FIRST; + bool groupOverlappingSections = DEFAULT_GROUP_OVERLAPPING_SECTIONS; std::uint8_t tcVersion = DEFAULT_TC_VERSION; std::string languageCode = DEFAULT_LANGUAGE_CODE; std::string countryCode = DEFAULT_COUNTRY_CODE; diff --git a/include/task_controller.hpp b/include/task_controller.hpp index d1c9ac9..f0fcc54 100644 --- a/include/task_controller.hpp +++ b/include/task_controller.hpp @@ -14,6 +14,7 @@ #include "isobus/isobus/isobus_device_descriptor_object_pool.hpp" #include "isobus/isobus/isobus_standard_data_description_indices.hpp" #include "isobus/isobus/isobus_task_controller_server.hpp" +#include "section_layout.hpp" #include #include @@ -66,6 +67,35 @@ class ClientState void set_element_work_state(std::uint16_t elementNumber, bool isWorking); bool try_get_element_work_state(std::uint16_t elementNumber, bool &isWorking) const; + /// @brief Elements carrying the condensed setpoint/actual DDIs, one per boom. Two or more means each boom + /// numbers its own sections and gets its own setpoint and DDI 160; zero or one keeps the single-element path. + void set_condensed_owners(std::vector owners, std::uint16_t setpointBaseDdi); + const std::vector &get_condensed_owners() const; + bool has_multiple_condensed_owners() const; + std::uint16_t get_condensed_setpoint_base_ddi() const; ///< DDI 290, or DDI 161 for the settable-actual method + /// @brief Maps an owner element's local condensed section to the flat section index. False if unknown. + bool try_get_owned_section(std::uint16_t ownerElement, std::uint8_t localSection, std::uint8_t §ion) const; + /// @brief Records one element's DDI 160 report; with several owners, automatic while any element is automatic + void set_element_section_control_state(std::uint16_t elementNumber, bool enabled); + /// @brief Elements that declare DDI 160 (Section Control State) + void add_section_control_state_element(std::uint16_t elementNumber); + const std::vector &get_section_control_state_elements() const; + + /// @brief Section element numbers in flat section order, as found in the DDOP geometry + void set_section_elements(const std::vector &elements); + const std::vector &get_section_elements() const; + /// @brief Stores a Y offset or width that a section reported (DDI 135 / 67 / 70 / 68). True if it changed. + bool set_section_geometry_value(std::uint16_t elementNumber, std::uint16_t ddi, std::int32_t value); + const std::vector &get_section_geometry() const; + void set_section_geometry(std::vector geometry); + /// @brief Sets which AOG section drives each implement section (identity until geometry says otherwise) + void set_aog_section_map(const section_layout::AogSectionMap &map); + const section_layout::AogSectionMap &get_aog_section_map() const; + std::uint8_t get_aog_section_for_section(std::uint8_t section) const; + std::uint8_t get_number_of_aog_sections() const; + /// @brief Whether any implement section in this AOG section is on + bool is_aog_section_on(std::uint8_t aogSection) const; + private: isobus::DeviceDescriptorObjectPool pool; ///< The device descriptor object pool (DDOP) for the TC bool areMeasurementCommandsSent = false; ///< Whether or not the measurement commands have been sent @@ -83,6 +113,13 @@ class ClientState bool isSectionControlEnabled = false; ///< Stores auto vs manual mode setting bool usesPerElementControl = false; ///< Legacy mode: use per-element setpoint instead of condensed std::uint16_t perElementSetpointDDI = 0; ///< The DDI to use for per-element setpoints (289 or 141), 0 if not applicable + std::vector condensedOwners; ///< One per boom with its own condensed DDIs + std::uint16_t condensedSetpointBaseDdi = 0; ///< First DDI of the condensed setpoint range the owners use + std::map elementSectionControlStates; ///< DDI 160 as reported per element + std::vector sectionControlStateElements; ///< Elements declaring DDI 160 + std::vector sectionElements; ///< Flat section index -> section element number (from the geometry) + std::vector sectionGeometry; ///< Lateral position per section, filled from the DDOP and requests + section_layout::AogSectionMap aogSectionMap; ///< Implement section -> AOG section }; // Create the task controller server object, this will handle all the ISOBUS communication for us @@ -120,17 +157,25 @@ class MyTCServer : public isobus::TaskControllerServer void update_section_states(std::vector §ionStates); void update_section_control_enabled(bool enabled); + /// @brief Whether sections that cover the same strip (same Y offset and width) share one AOG section (settings: groupOverlappingSections) + void set_group_overlapping_sections(bool enabled); + /// @brief Sends GNSS quality (DDI 514) to every client that declares that DDI. /// @param quality NMEA 2000 GNSS Method: 0=No GNSS, 1=GNSS, 2=DGNSS, 3=Precise, 4=RTK Fixed, 5=RTK Float, 6=Estimated, 7=Manual, 8=Simulated void send_gnss_quality(std::uint8_t quality); private: void send_section_setpoint_states(std::shared_ptr client, std::uint8_t ddiOffset); + /// @brief Sends each condensed owner its own setpoint, sections numbered within that owner + void send_owner_section_setpoint_states(std::shared_ptr client); + /// @brief Recomputes which AOG section drives each implement section from the known geometry, and logs a change + void update_aog_section_map(std::shared_ptr client, ClientState &state); void send_section_control_state(std::shared_ptr client, bool enabled); bool is_ddi_settable(std::shared_ptr client, std::uint16_t ddi); std::map, ClientState> clients; std::map, std::queue>> uploadedPools; + bool groupOverlappingSections = false; ///< Guarded by clientsMutex /// @brief Guards clients and uploadedPools. /// diff --git a/readme.md b/readme.md index 1ab0222..0f6a5e0 100644 --- a/readme.md +++ b/readme.md @@ -65,6 +65,7 @@ AOG-TaskController reads its configuration from a `settings.json` file located i | `aogHeartbeatEnabled` | `bool` | `true` | Sends the 100 ms zero-section heartbeat when no implement is connected. Disable for AgOpenGPS versions before v6.8.2 beta 5. | | `vtEnabled` | `bool` | `true` | Enables the ISOBUS Virtual Terminal UI. | | `allSectionsFollowFirst` | `bool` | `false` | Every implement section follows AOG's section 1, and AOG sees the implement as one section (on while any section is on). For implements that declare several sections or booms that really are one, such as several dispensers on one working width. Set AOG to 1 section. | +| `groupOverlappingSections` | `bool` | `false` | Sections with the same lateral offset and width (within 10 mm) share one AOG section, e.g. several dispensers on one working width. The TC requests the offsets and widths an implement only declares as process data. The resulting mapping is logged even while this is off. | | `tcVersion` | `integer` | `3` | Task Controller version code, from `0` through `4`; `3` is Second Edition Draft. | | `languageCode` | `string` | `"en"` | Two-character language code advertised on ISOBUS. | | `countryCode` | `string` | `"US"` | Two-character country code advertised on ISOBUS. | diff --git a/src/app.cpp b/src/app.cpp index 20cb58d..405dc91 100644 --- a/src/app.cpp +++ b/src/app.cpp @@ -381,6 +381,7 @@ void Application::setup_task_controller_server() languageInterface.set_language_code(settings->get_language_code()); languageInterface.set_country_code(settings->get_country_code()); tcServer->initialize(); + tcServer->set_group_overlapping_sections(settings->is_group_overlapping_sections()); tcServer->set_task_totals_active(true); // TODO: make this dynamic based on status in AOG tcFunctionalities = std::make_unique(tcCF); tcFunctionalities->set_functionality_is_supported( @@ -758,17 +759,19 @@ bool Application::update() continue; } - std::vector data = { state.is_section_control_enabled(), state.get_number_of_sections() }; + // AOG's sections: overlapping implement sections are reported as one (identity mapping otherwise) + const std::uint8_t aogSections = state.get_number_of_aog_sections(); + std::vector data = { state.is_section_control_enabled(), aogSections }; std::uint8_t sectionIndex = 0; - while (sectionIndex < state.get_number_of_sections()) + while (sectionIndex < aogSections) { std::uint8_t byte = 0; for (std::uint8_t i = 0; i < 8; i++) { - if (sectionIndex < state.get_number_of_sections()) + if (sectionIndex < aogSections) { - byte |= (state.get_section_actual_state(sectionIndex) == SectionState::ON) << i; + byte |= state.is_aog_section_on(sectionIndex) << i; sectionIndex++; } } diff --git a/src/settings.cpp b/src/settings.cpp index cdc73d8..6f5a050 100644 --- a/src/settings.cpp +++ b/src/settings.cpp @@ -71,6 +71,7 @@ bool Settings::load() aogHeartbeatEnabled = loadBoolean("aogHeartbeatEnabled", DEFAULT_AOG_HEARTBEAT_ENABLED); vtEnabled = loadBoolean("vtEnabled", DEFAULT_VT_ENABLED); allSectionsFollowFirst = loadBoolean("allSectionsFollowFirst", DEFAULT_ALL_SECTIONS_FOLLOW_FIRST); + groupOverlappingSections = loadBoolean("groupOverlappingSections", DEFAULT_GROUP_OVERLAPPING_SECTIONS); if (data.contains("tcVersion")) { @@ -145,6 +146,7 @@ bool Settings::save() const data["aogHeartbeatEnabled"] = aogHeartbeatEnabled; data["vtEnabled"] = vtEnabled; data["allSectionsFollowFirst"] = allSectionsFollowFirst; + data["groupOverlappingSections"] = groupOverlappingSections; data["tcVersion"] = tcVersion; data["languageCode"] = languageCode; data["countryCode"] = countryCode; @@ -274,6 +276,17 @@ bool Settings::set_all_sections_follow_first(bool enabled, bool save) return set_boolean(allSectionsFollowFirst, enabled, save); } +bool Settings::is_group_overlapping_sections() const +{ + std::scoped_lock lock(settingsMutex); + return groupOverlappingSections; +} + +bool Settings::set_group_overlapping_sections(bool enabled, bool save) +{ + return set_boolean(groupOverlappingSections, enabled, save); +} + bool Settings::is_aog_heartbeat_enabled() const { std::scoped_lock lock(settingsMutex); diff --git a/src/task_controller.cpp b/src/task_controller.cpp index a347816..a7bfa02 100644 --- a/src/task_controller.cpp +++ b/src/task_controller.cpp @@ -114,6 +114,8 @@ void ClientState::set_number_of_sections(std::uint8_t number) sectionSetpointStates.resize(number); sectionActualStates.resize(number); sectionToElementNumber.resize(number, 0); // Initialize all sections mapped to element 0 by default + sectionGeometry.resize(number); + aogSectionMap = section_layout::identity_map(number); } void ClientState::set_section_setpoint_state(std::uint8_t section, std::uint8_t state) @@ -239,6 +241,10 @@ bool ClientState::is_section_control_enabled() const void ClientState::set_section_control_enabled(bool state) { isSectionControlEnabled = state; + for (auto &entry : elementSectionControlStates) + { + entry.second = state; // Commanded for every boom; each boom's next DDI 160 report corrects its own entry + } } bool ClientState::uses_per_element_control() const @@ -328,6 +334,136 @@ bool ClientState::try_get_element_work_state(std::uint16_t elementNumber, bool & return false; } +void ClientState::set_condensed_owners(std::vector owners, std::uint16_t setpointBaseDdi) +{ + condensedOwners = std::move(owners); + condensedSetpointBaseDdi = setpointBaseDdi; +} + +const std::vector &ClientState::get_condensed_owners() const +{ + return condensedOwners; +} + +bool ClientState::has_multiple_condensed_owners() const +{ + return condensedOwners.size() > 1; +} + +std::uint16_t ClientState::get_condensed_setpoint_base_ddi() const +{ + return condensedSetpointBaseDdi; +} + +bool ClientState::try_get_owned_section(std::uint16_t ownerElement, std::uint8_t localSection, std::uint8_t §ion) const +{ + for (const auto &owner : condensedOwners) + { + if ((owner.elementNumber == ownerElement) && (localSection < owner.count)) + { + section = owner.firstSection + localSection; + return true; + } + } + return false; +} + +void ClientState::set_element_section_control_state(std::uint16_t elementNumber, bool enabled) +{ + elementSectionControlStates[elementNumber] = enabled; + bool anyEnabled = false; + for (const auto &entry : elementSectionControlStates) + { + anyEnabled = anyEnabled || entry.second; + } + isSectionControlEnabled = anyEnabled; +} + +void ClientState::add_section_control_state_element(std::uint16_t elementNumber) +{ + if (std::find(sectionControlStateElements.begin(), sectionControlStateElements.end(), elementNumber) == sectionControlStateElements.end()) + { + sectionControlStateElements.push_back(elementNumber); + } +} + +const std::vector &ClientState::get_section_control_state_elements() const +{ + return sectionControlStateElements; +} + +void ClientState::set_section_elements(const std::vector &elements) +{ + sectionElements = elements; +} + +const std::vector &ClientState::get_section_elements() const +{ + return sectionElements; +} + +bool ClientState::set_section_geometry_value(std::uint16_t elementNumber, std::uint16_t ddi, std::int32_t value) +{ + for (std::size_t i = 0; (i < sectionElements.size()) && (i < sectionGeometry.size()); i++) + { + if (sectionElements[i] != elementNumber) + { + continue; + } + auto &geometry = sectionGeometry[i]; + std::optional &target = (ddi == static_cast(isobus::DataDescriptionIndex::DeviceElementOffsetY)) ? geometry.yOffset_mm : geometry.width_mm; + if (target == value) + { + return false; + } + target = value; + return true; + } + return false; +} + +const std::vector &ClientState::get_section_geometry() const +{ + return sectionGeometry; +} + +void ClientState::set_section_geometry(std::vector geometry) +{ + sectionGeometry = std::move(geometry); +} + +void ClientState::set_aog_section_map(const section_layout::AogSectionMap &map) +{ + aogSectionMap = map; +} + +const section_layout::AogSectionMap &ClientState::get_aog_section_map() const +{ + return aogSectionMap; +} + +std::uint8_t ClientState::get_aog_section_for_section(std::uint8_t section) const +{ + return (section < aogSectionMap.aogSectionForSection.size()) ? aogSectionMap.aogSectionForSection[section] : section; +} + +std::uint8_t ClientState::get_number_of_aog_sections() const +{ + return (aogSectionMap.aogSectionForSection.size() == numberOfSections) ? aogSectionMap.numberOfAogSections : numberOfSections; +} + +bool ClientState::is_aog_section_on(std::uint8_t aogSection) const +{ + for (std::uint8_t i = 0; i < numberOfSections; i++) + { + if ((get_aog_section_for_section(i) == aogSection) && (get_section_actual_state(i) == SectionState::ON)) + { + return true; + } + } + return false; +} + MyTCServer::MyTCServer(std::shared_ptr internalControlFunction, isobus::TaskControllerServer::TaskControllerVersion version) : TaskControllerServer(internalControlFunction, @@ -501,6 +637,23 @@ bool MyTCServer::activate_object_pool(std::shared_ptr p { state.set_element_number_for_section(i, sectionElementNumbers[i]); } + state.set_section_elements(sectionElementNumbers); + + // Lateral geometry the DDOP gives as fixed values; the rest is requested after activation + std::vector geometry(numberOfSections); + for (std::size_t i = 0; (i < geometryRows.size()) && (i < geometry.size()); i++) + { + const auto §ion = geometryRows[i].section; + if (section.yOffset_mm.exists()) + { + geometry[i].yOffset_mm = section.yOffset_mm.get(); + } + if (section.width_mm.exists()) + { + geometry[i].width_mm = section.width_mm.get(); + } + } + state.set_section_geometry(geometry); // Scan the DDOP to determine which section control method the device supports bool hasCondensedSetpoint = false; // Modern: DDI 290+ (paired with DDI 289 for global work state) @@ -564,6 +717,24 @@ bool MyTCServer::activate_object_pool(std::shared_ptr p log("TC Server") << "WARNING: No supported section control method detected! " << "Device has no DDI 290, 161 (settable), or 141 (settable)." << std::endl; } + + // Several booms that each carry their own condensed DDIs (e.g. one Function per product) number their + // sections from 1 each; give every one of them its own setpoint and DDI 160. + if (hasCondensedSetpoint || hasSettableCondensedActual) + { + const auto firstDdi = static_cast(hasCondensedSetpoint ? isobus::DataDescriptionIndex::SetpointCondensedWorkState1_16 : isobus::DataDescriptionIndex::ActualCondensedWorkState1_16); + const auto lastDdi = static_cast(hasCondensedSetpoint ? isobus::DataDescriptionIndex::SetpointCondensedWorkState241_256 : isobus::DataDescriptionIndex::ActualCondensedWorkState241_256); + auto owners = section_layout::resolve_condensed_owners(state.get_pool(), element_hierarchy::build_parent_map(state.get_pool()), sectionElementNumbers, firstDdi, lastDdi); + if (owners.size() > 1) + { + log("TC Server") << "Sections are split over " << owners.size() << " elements with their own condensed DDIs:" << std::endl; + for (const auto &owner : owners) + { + log() << " Element " << owner.elementNumber << ": sections " << (owner.firstSection + 1) << "-" << (owner.firstSection + owner.count) << std::endl; + } + } + state.set_condensed_owners(std::move(owners), firstDdi); + } } else { @@ -576,6 +747,7 @@ bool MyTCServer::activate_object_pool(std::shared_ptr p clients[partnerCF] = state; log("TC Server") << "Client " << partnerCF->get_NAME().get_full_name() << " registered successfully with " << static_cast(state.get_number_of_sections()) << " sections." << std::endl; + update_aog_section_map(partnerCF, clients[partnerCF]); return true; } @@ -680,6 +852,20 @@ bool MyTCServer::on_value_command(std::shared_ptr partn { std::uint8_t sectionIndexOffset = NUMBER_SECTIONS_PER_CONDENSED_MESSAGE * static_cast(dataDescriptionIndex - static_cast(isobus::DataDescriptionIndex::ActualCondensedWorkState1_16)); + if (clients[partner].has_multiple_condensed_owners()) + { + // Sections are numbered within the reporting boom + for (std::uint_fast8_t i = 0; i < NUMBER_SECTIONS_PER_CONDENSED_MESSAGE; i++) + { + std::uint8_t section; + if (clients[partner].try_get_owned_section(elementNumber, static_cast(sectionIndexOffset + i), section)) + { + clients[partner].set_section_actual_state(section, ((processDataValue >> (2 * i)) & 0x03)); + } + } + break; + } + for (std::uint_fast8_t i = 0; i < NUMBER_SECTIONS_PER_CONDENSED_MESSAGE; i++) { std::uint8_t sectionState = ((processDataValue >> (2 * i)) & 0x03); @@ -691,7 +877,29 @@ bool MyTCServer::on_value_command(std::shared_ptr partn case static_cast(isobus::DataDescriptionIndex::SectionControlState): { - clients[partner].set_section_control_enabled(processDataValue == 1); + if (clients[partner].has_multiple_condensed_owners()) + { + clients[partner].set_element_section_control_state(elementNumber, processDataValue == 1); + } + else + { + clients[partner].set_section_control_enabled(processDataValue == 1); + } + } + break; + + case static_cast(isobus::DataDescriptionIndex::DeviceElementOffsetY): + case static_cast(isobus::DataDescriptionIndex::ActualWorkingWidth): + case static_cast(isobus::DataDescriptionIndex::MaximumWorkingWidth): + case static_cast(isobus::DataDescriptionIndex::DefaultWorkingWidth): + { + // Section geometry that the DDOP declares as process data, answered after request_measurement_commands() + if (clients[partner].set_section_geometry_value(elementNumber, dataDescriptionIndex, processDataValue)) + { + log("TC Server") << "Element " << elementNumber << " reports " << isobus::DataDictionary::get_entry(dataDescriptionIndex).to_string() + << " = " << processDataValue << " mm" << std::endl; + update_aog_section_map(partner, clients[partner]); + } } break; @@ -819,6 +1027,10 @@ void MyTCServer::request_measurement_commands() // TODO: This is a bit of a hack, but it works for now client.second.set_element_number_for_ddi(static_cast(processDataObject->get_ddi()), elementObject->get_element_number()); const auto &entryB = isobus::DataDictionary::get_entry(processDataObject->get_ddi()); + if (processDataObject->get_ddi() == static_cast(isobus::DataDescriptionIndex::SectionControlState)) + { + client.second.add_section_control_state_element(elementObject->get_element_number()); + } if (processDataObject->has_trigger_method(isobus::task_controller_object::DeviceProcessDataObject::AvailableTriggerMethods::OnChange)) { @@ -870,6 +1082,51 @@ void MyTCServer::request_measurement_commands() } } + // Ask each section for the lateral geometry the DDOP only declares as process data (not as fixed + // values), so sections covering the same strip can share one AOG section. + { + using isobus::DataDescriptionIndex; + auto &pool = client.second.get_pool(); + const auto §ionElements = client.second.get_section_elements(); + const auto &geometry = client.second.get_section_geometry(); + for (std::size_t s = 0; (s < sectionElements.size()) && (s < geometry.size()); s++) + { + std::shared_ptr element; + for (std::uint16_t i = 0; i < pool.size(); i++) + { + auto object = pool.get_object_by_index(i); + if (object && (object->get_object_type() == isobus::task_controller_object::ObjectTypes::DeviceElement) && + (std::static_pointer_cast(object)->get_element_number() == sectionElements[s])) + { + element = std::static_pointer_cast(object); + break; + } + } + if (!element) + { + continue; + } + auto request = [&](DataDescriptionIndex ddi) { + if (section_layout::element_links_ddi(pool, *element, static_cast(ddi), static_cast(ddi))) + { + send_request_value(client.first, static_cast(ddi), sectionElements[s]); + log() << "Requested " << isobus::DataDictionary::get_entry(static_cast(ddi)).to_string() << " from element " << sectionElements[s] << std::endl; + return true; + } + return false; + }; + if (!geometry[s].yOffset_mm) + { + request(DataDescriptionIndex::DeviceElementOffsetY); + } + if (!geometry[s].width_mm) + { + // Same priority as the DDOP helper: actual, then maximum, then default width + request(DataDescriptionIndex::ActualWorkingWidth) || request(DataDescriptionIndex::MaximumWorkingWidth) || request(DataDescriptionIndex::DefaultWorkingWidth); + } + } + } + log() << "Measurement commands sent." << std::endl; client.second.mark_measurement_commands_sent(); } @@ -894,10 +1151,12 @@ void MyTCServer::update_section_states(std::vector §ionStates) continue; } + const bool multipleOwners = state.has_multiple_condensed_owners(); bool requiresUpdate = false; + bool anyUpdate = false; for (std::uint8_t i = 0; i < state.get_number_of_sections(); i++) { - if ((i % NUMBER_SECTIONS_PER_CONDENSED_MESSAGE == 0) && requiresUpdate) + if ((i % NUMBER_SECTIONS_PER_CONDENSED_MESSAGE == 0) && requiresUpdate && !multipleOwners) { // Send the previous 16 sections std::uint8_t ddiOffset = (i / NUMBER_SECTIONS_PER_CONDENSED_MESSAGE) - 1; @@ -905,16 +1164,26 @@ void MyTCServer::update_section_states(std::vector §ionStates) requiresUpdate = false; } - if (i < sectionStates.size()) + // Overlapping sections share one AOG section (identity mapping unless grouped) + const std::uint8_t aogSection = state.get_aog_section_for_section(i); + if (aogSection < sectionStates.size()) { - if (sectionStates[i] != (state.get_section_setpoint_state(i) == SectionState::ON)) + if (sectionStates[aogSection] != (state.get_section_setpoint_state(i) == SectionState::ON)) { - state.set_section_setpoint_state(i, sectionStates[i] ? SectionState::ON : SectionState::OFF); + state.set_section_setpoint_state(i, sectionStates[aogSection] ? SectionState::ON : SectionState::OFF); requiresUpdate = true; + anyUpdate = true; } } } - if (requiresUpdate) + if (multipleOwners) + { + if (anyUpdate) + { + send_owner_section_setpoint_states(client.first); + } + } + else if (requiresUpdate) { std::uint8_t ddiOffset = (state.get_number_of_sections() - 1) / NUMBER_SECTIONS_PER_CONDENSED_MESSAGE; send_section_setpoint_states(client.first, ddiOffset); @@ -1039,9 +1308,77 @@ void MyTCServer::send_section_setpoint_states(std::shared_ptr lock(clientsMutex); + groupOverlappingSections = enabled; + for (auto &client : clients) + { + update_aog_section_map(client.first, client.second); + } +} + +void MyTCServer::update_aog_section_map(std::shared_ptr client, ClientState &state) +{ + std::lock_guard lock(clientsMutex); + const auto grouped = section_layout::group_overlapping_sections(state.get_section_geometry()); + const auto map = groupOverlappingSections ? grouped : section_layout::identity_map(state.get_number_of_sections()); + if (map.aogSectionForSection == state.get_aog_section_map().aogSectionForSection) + { + return; + } + state.set_aog_section_map(map); + + std::ostringstream text; + for (std::size_t i = 0; i < map.aogSectionForSection.size(); i++) + { + text << (i ? ", " : "") << (i + 1) << "->" << (map.aogSectionForSection[i] + 1); + } + log("TC Server") << "Client " << client->get_NAME().get_full_name() << ": " << static_cast(map.numberOfAogSections) + << " AOG section(s) for " << static_cast(state.get_number_of_sections()) << " implement section(s) (" + << text.str() << ")" << (map.grouped ? ", overlapping sections share an AOG section" : "") << std::endl; +} + +void MyTCServer::send_owner_section_setpoint_states(std::shared_ptr client) +{ + std::lock_guard lock(clientsMutex); + auto &state = clients[client]; + const std::uint16_t baseDdi = state.get_condensed_setpoint_base_ddi(); + for (const auto &owner : state.get_condensed_owners()) + { + for (std::uint8_t block = 0; block * NUMBER_SECTIONS_PER_CONDENSED_MESSAGE < owner.count; block++) + { + std::uint32_t value = 0; + for (std::uint8_t i = 0; i < NUMBER_SECTIONS_PER_CONDENSED_MESSAGE; i++) + { + const std::uint8_t local = block * NUMBER_SECTIONS_PER_CONDENSED_MESSAGE + i; + const std::uint8_t sectionState = (local < owner.count) ? state.get_section_setpoint_state(owner.firstSection + local) : SectionState::NOT_INSTALLED; + value |= (static_cast(sectionState) << (2 * i)); + } + send_set_value(client, static_cast(baseDdi + block), owner.elementNumber, value); + } + } + + const bool setpointWorkState = state.is_any_section_setpoint_on(); + if ((state.get_setpoint_work_state() != setpointWorkState) && state.has_element_number_for_ddi(isobus::DataDescriptionIndex::SetpointWorkState)) + { + send_set_value(client, static_cast(isobus::DataDescriptionIndex::SetpointWorkState), state.get_element_number_for_ddi(isobus::DataDescriptionIndex::SetpointWorkState), setpointWorkState ? 1 : 0); + state.set_setpoint_work_state(setpointWorkState); + } +} + void MyTCServer::send_section_control_state(std::shared_ptr client, bool enabled) { std::lock_guard lock(clientsMutex); + if (clients[client].has_multiple_condensed_owners()) + { + // Every boom has its own auto/manual switch + for (std::uint16_t element : clients[client].get_section_control_state_elements()) + { + send_set_value(client, static_cast(isobus::DataDescriptionIndex::SectionControlState), element, enabled ? 1 : 0); + } + return; + } send_set_value(client, static_cast(isobus::DataDescriptionIndex::SectionControlState), clients[client].get_element_number_for_ddi(isobus::DataDescriptionIndex::SectionControlState), enabled ? 1 : 0); } diff --git a/tools/test_section_layout.cpp b/tools/test_section_layout.cpp new file mode 100644 index 0000000..2188a86 --- /dev/null +++ b/tools/test_section_layout.cpp @@ -0,0 +1,245 @@ +/** + * @brief Unit tests for section_layout: condensed-DDI owners per boom and grouping of overlapping sections. + * + * The pools model the two layouts an RDS Artemis seeder controller uploads for three dispensers on one + * working width: three Functions with one Section each, or ("linked") one Function with three Sections. + * + * Returns 0 when every assertion passes, 1 otherwise. + */ + +#include "section_layout.hpp" + +#include "isobus/isobus/isobus_device_descriptor_object_pool_helpers.hpp" + +#include +#include +#include + +using Element = isobus::task_controller_object::DeviceElementObject; +using section_layout::SectionGeometry; + +static int failures = 0; + +static void check(bool condition, const char *label) +{ + if (!condition) + { + std::fprintf(stderr, " FAIL: %s\n", label); + ++failures; + } +} + +static constexpr std::uint16_t DDI_SETPOINT_CONDENSED_1_16 = 290; +static constexpr std::uint16_t DDI_SETPOINT_CONDENSED_241_256 = 305; + +static std::shared_ptr element_by_id(isobus::DeviceDescriptorObjectPool &pool, std::uint16_t objectId) +{ + return std::static_pointer_cast(pool.get_object_by_id(objectId)); +} + +// Device element 0 (object 1); Functions as elements 2.. (objects 3..); Sections as elements 6.. (objects 100..). +// Each Function links DDI 290 / 161, so each is a condensed owner. sectionsPerFunction lists the Sections per Function. +static bool build_seeder_pool(isobus::DeviceDescriptorObjectPool &pool, const std::vector §ionsPerFunction) +{ + bool ok = pool.add_device("Seeder", "1", "0", "SEED", {}, {}, 0); + ok = ok && pool.add_device_element("Device", 0, 0, Element::Type::Device, 1); + ok = ok && pool.add_device_process_data("Sections Set", DDI_SETPOINT_CONDENSED_1_16, 0xFFFF, 0x02, 0, 504); + ok = ok && pool.add_device_process_data("Sections Actual", 161, 0xFFFF, 0x01, 0x09, 503); + + std::uint16_t sectionElement = 6; + std::uint16_t sectionObject = 100; + for (std::size_t f = 0; f < sectionsPerFunction.size(); f++) + { + const auto functionElement = static_cast(2 + f); + const auto functionObject = static_cast(3 + f); + ok = ok && pool.add_device_element("Seeder", functionElement, 1, Element::Type::Function, functionObject); + if (!ok) + { + return false; + } + element_by_id(pool, functionObject)->add_reference_to_child_object(503); + element_by_id(pool, functionObject)->add_reference_to_child_object(504); + for (int s = 0; s < sectionsPerFunction[f]; s++) + { + ok = ok && pool.add_device_element("Section", sectionElement++, functionObject, Element::Type::Section, sectionObject++); + } + } + return ok; +} + +static std::vector section_elements(std::size_t count) +{ + std::vector elements; + for (std::size_t i = 0; i < count; i++) + { + elements.push_back(static_cast(6 + i)); + } + return elements; +} + +static std::vector owners_of(isobus::DeviceDescriptorObjectPool &pool, std::size_t sections) +{ + return section_layout::resolve_condensed_owners(pool, element_hierarchy::build_parent_map(pool), section_elements(sections), DDI_SETPOINT_CONDENSED_1_16, DDI_SETPOINT_CONDENSED_241_256); +} + +static void test_one_owner_per_function() +{ + isobus::DeviceDescriptorObjectPool pool; + check(build_seeder_pool(pool, { 1, 1, 1 }), "three-function pool builds"); + const auto owners = owners_of(pool, 3); + check(owners.size() == 3, "three Functions with their own DDI 290 give three owners"); + for (std::size_t i = 0; (i < owners.size()) && (i < 3); i++) + { + check(owners[i].elementNumber == 2 + i, "owner is the Function element"); + check(owners[i].firstSection == i, "owner starts at its own section"); + check(owners[i].count == 1, "owner has one section"); + } +} + +static void test_linked_layout_has_one_owner() +{ + isobus::DeviceDescriptorObjectPool pool; + check(build_seeder_pool(pool, { 3 }), "linked pool builds"); + const auto owners = owners_of(pool, 3); + check(owners.size() == 1, "one Function with three Sections gives one owner"); + check(!owners.empty() && (owners[0].elementNumber == 2) && (owners[0].count == 3), "the one owner has all three sections"); +} + +static void test_mixed_section_counts() +{ + isobus::DeviceDescriptorObjectPool pool; + check(build_seeder_pool(pool, { 1, 2 }), "two-function pool builds"); + const auto owners = owners_of(pool, 3); + check(owners.size() == 2, "two Functions give two owners"); + check((owners.size() == 2) && (owners[1].firstSection == 1) && (owners[1].count == 2), "second owner has sections 2-3"); +} + +static void test_owner_on_device_element() +{ + // A single-boom sprayer that puts DDI 290 on the device element: one owner, element 0 + isobus::DeviceDescriptorObjectPool pool; + bool ok = pool.add_device("Sprayer", "1", "0", "SPRAY", {}, {}, 0); + ok = ok && pool.add_device_element("Device", 0, 0, Element::Type::Device, 1); + ok = ok && pool.add_device_process_data("Sections Set", DDI_SETPOINT_CONDENSED_1_16, 0xFFFF, 0x02, 0, 504); + ok = ok && pool.add_device_element("Boom", 1, 1, Element::Type::Function, 3); + ok = ok && pool.add_device_element("Section", 6, 3, Element::Type::Section, 100); + ok = ok && pool.add_device_element("Section", 7, 3, Element::Type::Section, 101); + check(ok, "sprayer pool builds"); + element_by_id(pool, 1)->add_reference_to_child_object(504); + const auto owners = owners_of(pool, 2); + check((owners.size() == 1) && (owners[0].elementNumber == 0) && (owners[0].count == 2), "DDI 290 on the device element owns every section"); +} + +static void test_missing_owner_falls_back() +{ + isobus::DeviceDescriptorObjectPool pool; + bool ok = pool.add_device("Bare", "1", "0", "BARE", {}, {}, 0); + ok = ok && pool.add_device_element("Device", 0, 0, Element::Type::Device, 1); + ok = ok && pool.add_device_element("Section", 6, 1, Element::Type::Section, 100); + check(ok, "bare pool builds"); + check(owners_of(pool, 1).empty(), "no element with DDI 290 gives no owners"); +} + +static void test_grouping() +{ + using section_layout::group_overlapping_sections; + + auto same = group_overlapping_sections({ { 0, 3000 }, { 0, 3000 }, { 5, 3004 } }); + check(same.numberOfAogSections == 1 && same.grouped, "three dispensers on one width are one AOG section"); + check(same.aogSectionForSection == std::vector({ 0, 0, 0 }), "all three map to AOG section 1"); + + auto sideBySide = group_overlapping_sections({ { -1000, 1000 }, { 0, 1000 }, { 1000, 1000 } }); + check(!sideBySide.grouped && sideBySide.numberOfAogSections == 3, "side-by-side sections stay separate"); + check(sideBySide.aogSectionForSection == std::vector({ 0, 1, 2 }), "side-by-side keeps the identity mapping"); + + auto twoProducts = group_overlapping_sections({ { -500, 1000 }, { 500, 1000 }, { -500, 1000 }, { 500, 1000 } }); + check(twoProducts.numberOfAogSections == 2, "two products over two sections give two AOG sections"); + check(twoProducts.aogSectionForSection == std::vector({ 0, 1, 0, 1 }), "each product's section follows its strip"); + + auto unknown = group_overlapping_sections({ { 0, 3000 }, { std::nullopt, 3000 } }); + check(!unknown.grouped && unknown.numberOfAogSections == 2, "an unknown offset keeps the identity mapping"); + + auto noWidth = group_overlapping_sections({ { 0, 0 }, { 0, 0 } }); + check(!noWidth.grouped && noWidth.numberOfAogSections == 2, "zero widths (unfilled DDOP) keep the identity mapping"); + + auto differentWidth = group_overlapping_sections({ { 0, 3000 }, { 0, 1500 } }); + check(!differentWidth.grouped, "same centre but different width stays separate"); +} + +// Prints the owners and the DDOP's fixed geometry for a captured binary DDOP, in the TC's section order +static int describe_ddop(const char *path) +{ + std::ifstream file(path, std::ios::binary); + std::vector data((std::istreambuf_iterator(file)), std::istreambuf_iterator()); + isobus::DeviceDescriptorObjectPool pool; + pool.set_task_controller_compatibility_level(3); // As MyTCServer::activate_object_pool() does + if (data.empty() || !pool.deserialize_binary_object_pool(data.data(), static_cast(data.size()), isobus::NAME(0))) + { + std::fprintf(stderr, "%s: cannot read DDOP\n", path); + return 1; + } + std::vector sections; + std::vector geometry; + const auto implement = isobus::DeviceDescriptorObjectPoolHelper::get_implement_geometry(pool); + auto add = [&](const isobus::DeviceDescriptorObjectPoolHelper::Section §ion) { + sections.push_back(section.elementNumber); + geometry.push_back({ section.yOffset_mm.exists() ? std::optional(section.yOffset_mm.get()) : std::nullopt, + section.width_mm.exists() ? std::optional(section.width_mm.get()) : std::nullopt }); + }; + for (const auto &boom : implement.booms) + { + for (const auto &subBoom : boom.subBooms) + { + for (const auto §ion : subBoom.sections) + { + add(section); + } + } + for (const auto §ion : boom.sections) + { + add(section); + } + } + std::printf("%s: %zu section(s)\n", path, sections.size()); + for (std::size_t i = 0; i < sections.size(); i++) + { + std::printf(" section %zu: element %u\n", i + 1, sections[i]); + } + const auto owners = section_layout::resolve_condensed_owners(pool, element_hierarchy::build_parent_map(pool), sections, DDI_SETPOINT_CONDENSED_1_16, DDI_SETPOINT_CONDENSED_241_256); + std::printf(" %zu condensed owner(s)%s\n", owners.size(), owners.size() > 1 ? ", one setpoint each" : ""); + for (const auto &owner : owners) + { + std::printf(" element %u: sections %d-%d\n", owner.elementNumber, owner.firstSection + 1, owner.firstSection + owner.count); + } + const auto map = section_layout::group_overlapping_sections(geometry); + std::printf(" fixed geometry gives %d AOG section(s)%s\n", map.numberOfAogSections, map.grouped ? " (grouped)" : ""); + return 0; +} + +int main(int argc, char **argv) +{ + if (argc > 1) + { + int result = 0; + for (int i = 1; i < argc; i++) + { + result |= describe_ddop(argv[i]); + } + return result; + } + + test_one_owner_per_function(); + test_linked_layout_has_one_owner(); + test_mixed_section_counts(); + test_owner_on_device_element(); + test_missing_owner_falls_back(); + test_grouping(); + + if (failures != 0) + { + std::fprintf(stderr, "%d check(s) failed\n", failures); + return 1; + } + std::printf("section_layout: all checks passed\n"); + return 0; +} From 181b548d8250038e2f76f406eb85c92eb92f234e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gunics=20Bal=C3=A1zs?= Date: Wed, 11 Nov 2026 13:54:03 +0100 Subject: [PATCH 3/6] Log the grouping the geometry gives even while groupOverlappingSections is off Co-Authored-By: Claude Opus 5.5 --- include/task_controller.hpp | 4 ++++ src/task_controller.cpp | 39 ++++++++++++++++++++++++++++--------- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/include/task_controller.hpp b/include/task_controller.hpp index f0fcc54..ebb20d4 100644 --- a/include/task_controller.hpp +++ b/include/task_controller.hpp @@ -91,6 +91,9 @@ class ClientState /// @brief Sets which AOG section drives each implement section (identity until geometry says otherwise) void set_aog_section_map(const section_layout::AogSectionMap &map); const section_layout::AogSectionMap &get_aog_section_map() const; + /// @brief The grouping the geometry gives, whether or not it is applied (for the log) + void set_proposed_aog_section_map(const section_layout::AogSectionMap &map); + const section_layout::AogSectionMap &get_proposed_aog_section_map() const; std::uint8_t get_aog_section_for_section(std::uint8_t section) const; std::uint8_t get_number_of_aog_sections() const; /// @brief Whether any implement section in this AOG section is on @@ -120,6 +123,7 @@ class ClientState std::vector sectionElements; ///< Flat section index -> section element number (from the geometry) std::vector sectionGeometry; ///< Lateral position per section, filled from the DDOP and requests section_layout::AogSectionMap aogSectionMap; ///< Implement section -> AOG section + section_layout::AogSectionMap proposedAogSectionMap; ///< What the geometry gives; logged when it changes }; // Create the task controller server object, this will handle all the ISOBUS communication for us diff --git a/src/task_controller.cpp b/src/task_controller.cpp index a7bfa02..10d2f87 100644 --- a/src/task_controller.cpp +++ b/src/task_controller.cpp @@ -442,6 +442,16 @@ const section_layout::AogSectionMap &ClientState::get_aog_section_map() const return aogSectionMap; } +void ClientState::set_proposed_aog_section_map(const section_layout::AogSectionMap &map) +{ + proposedAogSectionMap = map; +} + +const section_layout::AogSectionMap &ClientState::get_proposed_aog_section_map() const +{ + return proposedAogSectionMap; +} + std::uint8_t ClientState::get_aog_section_for_section(std::uint8_t section) const { return (section < aogSectionMap.aogSectionForSection.size()) ? aogSectionMap.aogSectionForSection[section] : section; @@ -1322,21 +1332,32 @@ void MyTCServer::update_aog_section_map(std::shared_ptr { std::lock_guard lock(clientsMutex); const auto grouped = section_layout::group_overlapping_sections(state.get_section_geometry()); + auto describe = [](const section_layout::AogSectionMap &map) { + std::ostringstream text; + for (std::size_t i = 0; i < map.aogSectionForSection.size(); i++) + { + text << (i ? ", " : "") << (i + 1) << "->" << (map.aogSectionForSection[i] + 1); + } + return text.str(); + }; + + if (grouped.aogSectionForSection != state.get_proposed_aog_section_map().aogSectionForSection) + { + // What the geometry says, logged whether or not the grouping is applied + state.set_proposed_aog_section_map(grouped); + log("TC Server") << "Client " << client->get_NAME().get_full_name() << ": geometry gives " << static_cast(grouped.numberOfAogSections) + << " AOG section(s) for " << static_cast(state.get_number_of_sections()) << " implement section(s) (" << describe(grouped) << ")" + << (grouped.grouped ? (groupOverlappingSections ? ", applied" : ", not applied: groupOverlappingSections is off") : "") << std::endl; + } + const auto map = groupOverlappingSections ? grouped : section_layout::identity_map(state.get_number_of_sections()); if (map.aogSectionForSection == state.get_aog_section_map().aogSectionForSection) { return; } state.set_aog_section_map(map); - - std::ostringstream text; - for (std::size_t i = 0; i < map.aogSectionForSection.size(); i++) - { - text << (i ? ", " : "") << (i + 1) << "->" << (map.aogSectionForSection[i] + 1); - } - log("TC Server") << "Client " << client->get_NAME().get_full_name() << ": " << static_cast(map.numberOfAogSections) - << " AOG section(s) for " << static_cast(state.get_number_of_sections()) << " implement section(s) (" - << text.str() << ")" << (map.grouped ? ", overlapping sections share an AOG section" : "") << std::endl; + log("TC Server") << "Client " << client->get_NAME().get_full_name() << ": reporting " << static_cast(map.numberOfAogSections) + << " section(s) to AOG (" << describe(map) << ")" << std::endl; } void MyTCServer::send_owner_section_setpoint_states(std::shared_ptr client) From 46e3b32f57a91b3de1354ec56c77257884ae588c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gunics=20Bal=C3=A1zs?= Date: Wed, 11 Nov 2026 19:28:06 +0100 Subject: [PATCH 4/6] allSectionsFollowFirst: cover sections past 64, one combined heartbeat, demand from bit 1 only PGN 229 has 64 bits, so sections 65-255 kept their old setpoint; the follow-first vector now covers every section a DDOP can declare. With several section-controlled implements the heartbeat sent one one-section report per implement. It now sends a single report: auto while any implement is in auto, on while any implement section is on. check_section_demand looked at AOG's bit for each implement section; in follow-first mode only AOG's section 1 is demand, so a stray bit can no longer hide the working-without-demand warning. Addresses Sourcery's review of #130. Co-Authored-By: Claude Opus 5.5 --- readme.md | 2 +- src/app.cpp | 29 ++++++++++++++++++++--------- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/readme.md b/readme.md index 1ab0222..e769bf8 100644 --- a/readme.md +++ b/readme.md @@ -64,7 +64,7 @@ AOG-TaskController reads its configuration from a `settings.json` file located i | `nmeaSendEnabled` | `bool` | `true` | Enables cyclic NMEA2000 COG/SOG transmission. Requires `tecuEnabled`. | | `aogHeartbeatEnabled` | `bool` | `true` | Sends the 100 ms zero-section heartbeat when no implement is connected. Disable for AgOpenGPS versions before v6.8.2 beta 5. | | `vtEnabled` | `bool` | `true` | Enables the ISOBUS Virtual Terminal UI. | -| `allSectionsFollowFirst` | `bool` | `false` | Every implement section follows AOG's section 1, and AOG sees the implement as one section (on while any section is on). For implements that declare several sections or booms that really are one, such as several dispensers on one working width. Set AOG to 1 section. | +| `allSectionsFollowFirst` | `bool` | `false` | Every implement section follows AOG's section 1, and AOG sees the implement as one section (on while any section is on). For implements that declare several sections or booms that really are one, such as several dispensers on one working width. Set AOG to 1 section. Applies to every connected implement: with two section-controlled implements, both follow section 1 and AOG gets one combined report. | | `tcVersion` | `integer` | `3` | Task Controller version code, from `0` through `4`; `3` is Second Edition Draft. | | `languageCode` | `string` | `"en"` | Two-character language code advertised on ISOBUS. | | `countryCode` | `string` | `"US"` | Two-character country code advertised on ISOBUS. | diff --git a/src/app.cpp b/src/app.cpp index 20cb58d..82d6ba4 100644 --- a/src/app.cpp +++ b/src/app.cpp @@ -509,8 +509,9 @@ void Application::setup_udp_connections() } if (settings->is_all_sections_follow_first()) { - // The implement's sections are really one: all follow AOG's section 1 (sectionDemand above stays as AOG sent it) - std::fill(sectionStates.begin(), sectionStates.end(), sectionStates.front()); + // The implement's sections are really one: all follow AOG's section 1 (sectionDemand above stays as AOG sent it). + // PGN 229 carries 64 bits; cover every section a DDOP can declare (up to 255), not just the first 64. + sectionStates.assign(256, sectionStates.front()); } tcServer->update_section_states(sectionStates); } @@ -733,6 +734,9 @@ bool Application::update() if (isobus::SystemTiming::time_expired_ms(lastHeartbeatTransmit, 100)) { bool anyClientWithSections = false; + const bool followFirst = settings->is_all_sections_follow_first(); + bool followFirstEnabled = false; + bool followFirstOn = false; for (auto &client : tcServer->get_clients()) { auto &state = client.second; @@ -745,16 +749,15 @@ bool Application::update() anyClientWithSections = true; - if (settings->is_all_sections_follow_first()) + if (followFirst) { - // AOG is set up with one section: report it on while any implement section is on - bool anyOn = false; + // AOG is set up with one section, and section 1 drives every connected implement: + // collect them all into one report, on while any implement section is on + followFirstEnabled = followFirstEnabled || state.is_section_control_enabled(); for (std::uint8_t i = 0; i < state.get_number_of_sections(); i++) { - anyOn = anyOn || (state.get_section_actual_state(i) == SectionState::ON); + followFirstOn = followFirstOn || (state.get_section_actual_state(i) == SectionState::ON); } - std::vector oneSection = { state.is_section_control_enabled(), 1, static_cast(anyOn ? 1 : 0) }; - udpConnections->send(0x80, 0xF0, oneSection); continue; } @@ -777,6 +780,12 @@ bool Application::update() udpConnections->send(0x80, 0xF0, data); } + if (followFirst && anyClientWithSections) + { + std::vector oneSection = { followFirstEnabled, 1, static_cast(followFirstOn ? 1 : 0) }; + udpConnections->send(0x80, 0xF0, oneSection); + } + // If no clients with sections, send heartbeat with 0 sections if (settings->is_aog_heartbeat_enabled() && !anyClientWithSections) { @@ -1048,7 +1057,9 @@ void Application::check_section_demand() for (std::uint8_t i = 0; i < state.get_number_of_sections(); i++) { actual.push_back(SectionState::ON == state.get_section_actual_state(i)); - demanded = demanded || (arriving && (i < sectionDemand.size()) && sectionDemand[i]); + // With allSectionsFollowFirst, AOG has one section: only its bit is demand, any others are padding + const std::uint8_t demandIndex = settings->is_all_sections_follow_first() ? 0 : i; + demanded = demanded || (arriving && (demandIndex < sectionDemand.size()) && sectionDemand[demandIndex]); } const bool working = (actual.end() != std::find(actual.begin(), actual.end(), true)); From 25acc2b111adae7ae1a971ddcc1a9423ba8a5b04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gunics=20Bal=C3=A1zs?= Date: Wed, 11 Nov 2026 19:31:04 +0100 Subject: [PATCH 5/6] Per-boom control: settable owners only, seed boom modes, safe grouping math, mapped demand - A read-only DDI 161 only reports, so it no longer makes an element a condensed owner (only applies when the pool has no DDI 290). - Booms that have not reported DDI 160 yet take the last commanded mode, so one boom reporting manual first no longer switches section control off for booms that are still automatic. - group_overlapping_sections compares in 64 bits: offsets at the ends of the int32 range could overflow and merge distant sections. - check_section_demand reads AOG's bit for the AOG section each implement section maps to, not the implement section's own index. Addresses Sourcery's review of #131. Co-Authored-By: Claude Opus 5.5 --- include/section_layout.hpp | 21 ++++++++++++++------- src/app.cpp | 5 +++-- src/task_controller.cpp | 13 +++++++++++-- tools/test_section_layout.cpp | 17 +++++++++++++++++ 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/include/section_layout.hpp b/include/section_layout.hpp index e416d2e..38009d4 100644 --- a/include/section_layout.hpp +++ b/include/section_layout.hpp @@ -29,7 +29,9 @@ namespace section_layout }; /// @brief Whether an element links a process data object with a DDI in [firstDdi, lastDdi] - inline bool element_links_ddi(isobus::DeviceDescriptorObjectPool &pool, const isobus::task_controller_object::DeviceElementObject &element, std::uint16_t firstDdi, std::uint16_t lastDdi) + /// @param requireSettable Only count process data the TC may set (needed when the range is DDI 161+, whose + /// read-only form only reports and cannot be commanded) + inline bool element_links_ddi(isobus::DeviceDescriptorObjectPool &pool, const isobus::task_controller_object::DeviceElementObject &element, std::uint16_t firstDdi, std::uint16_t lastDdi, bool requireSettable = false) { using isobus::task_controller_object::DeviceProcessDataObject; using isobus::task_controller_object::ObjectTypes; @@ -38,8 +40,10 @@ namespace section_layout auto child = pool.get_object_by_id(childId); if (child && (child->get_object_type() == ObjectTypes::DeviceProcessData)) { - const auto ddi = std::static_pointer_cast(child)->get_ddi(); - if ((ddi >= firstDdi) && (ddi <= lastDdi)) + const auto processData = std::static_pointer_cast(child); + const auto ddi = processData->get_ddi(); + if ((ddi >= firstDdi) && (ddi <= lastDdi) && + (!requireSettable || processData->has_property(DeviceProcessDataObject::PropertiesBit::Settable))) { return true; } @@ -50,13 +54,15 @@ namespace section_layout /// @brief Finds, for each section, the nearest element (the section itself or an ancestor) that links a /// DDI in [firstDdi, lastDdi], and groups consecutive sections with the same owner. + /// @param requireSettable Only settable process data makes an owner (see element_links_ddi) /// @returns The owners in section order, or an empty vector when a section has no owner or one owner's /// sections are not consecutive (callers then keep the single-element behaviour). inline std::vector resolve_condensed_owners(isobus::DeviceDescriptorObjectPool &pool, const element_hierarchy::ParentMap &parents, const std::vector §ionElements, std::uint16_t firstDdi, - std::uint16_t lastDdi) + std::uint16_t lastDdi, + bool requireSettable = false) { using isobus::task_controller_object::DeviceElementObject; using isobus::task_controller_object::ObjectTypes; @@ -80,7 +86,7 @@ namespace section_layout for (std::size_t depth = 0; depth < 32; depth++) // depth guard against a cyclic pool { auto found = elements.find(current); - if ((found != elements.end()) && element_links_ddi(pool, *found->second, firstDdi, lastDdi)) + if ((found != elements.end()) && element_links_ddi(pool, *found->second, firstDdi, lastDdi, requireSettable)) { owner = current; break; @@ -165,8 +171,9 @@ namespace section_layout for (std::size_t g = 0; g < representative.size(); g++) { const auto &first = sections[representative[g]]; - if ((std::abs(*first.yOffset_mm - *sections[i].yOffset_mm) <= toleranceMm) && - (std::abs(*first.width_mm - *sections[i].width_mm) <= toleranceMm)) + // 64-bit differences: two int32 values can be further apart than an int32 holds + if ((std::llabs(static_cast(*first.yOffset_mm) - *sections[i].yOffset_mm) <= toleranceMm) && + (std::llabs(static_cast(*first.width_mm) - *sections[i].width_mm) <= toleranceMm)) { aogSection = static_cast(g); break; diff --git a/src/app.cpp b/src/app.cpp index 7e77c1e..5b6a91d 100644 --- a/src/app.cpp +++ b/src/app.cpp @@ -1060,8 +1060,9 @@ void Application::check_section_demand() for (std::uint8_t i = 0; i < state.get_number_of_sections(); i++) { actual.push_back(SectionState::ON == state.get_section_actual_state(i)); - // With allSectionsFollowFirst, AOG has one section: only its bit is demand, any others are padding - const std::uint8_t demandIndex = settings->is_all_sections_follow_first() ? 0 : i; + // Demand is AOG's bit for the AOG section this implement section belongs to (grouped sections share + // one); with allSectionsFollowFirst, AOG has one section, so any other bit is padding + const std::uint8_t demandIndex = settings->is_all_sections_follow_first() ? 0 : state.get_aog_section_for_section(i); demanded = demanded || (arriving && (demandIndex < sectionDemand.size()) && sectionDemand[demandIndex]); } const bool working = (actual.end() != std::find(actual.begin(), actual.end(), true)); diff --git a/src/task_controller.cpp b/src/task_controller.cpp index 10d2f87..919822c 100644 --- a/src/task_controller.cpp +++ b/src/task_controller.cpp @@ -241,9 +241,15 @@ bool ClientState::is_section_control_enabled() const void ClientState::set_section_control_enabled(bool state) { isSectionControlEnabled = state; + // Commanded for every boom, including booms that have not reported DDI 160 yet, so one boom's report + // is never aggregated against a missing entry for another; each boom's next report corrects its own entry + for (std::uint16_t element : sectionControlStateElements) + { + elementSectionControlStates[element] = state; + } for (auto &entry : elementSectionControlStates) { - entry.second = state; // Commanded for every boom; each boom's next DDI 160 report corrects its own entry + entry.second = state; } } @@ -384,6 +390,8 @@ void ClientState::add_section_control_state_element(std::uint16_t elementNumber) if (std::find(sectionControlStateElements.begin(), sectionControlStateElements.end(), elementNumber) == sectionControlStateElements.end()) { sectionControlStateElements.push_back(elementNumber); + // Until it reports, assume the boom is in the mode last commanded + elementSectionControlStates.try_emplace(elementNumber, isSectionControlEnabled); } } @@ -734,7 +742,8 @@ bool MyTCServer::activate_object_pool(std::shared_ptr p { const auto firstDdi = static_cast(hasCondensedSetpoint ? isobus::DataDescriptionIndex::SetpointCondensedWorkState1_16 : isobus::DataDescriptionIndex::ActualCondensedWorkState1_16); const auto lastDdi = static_cast(hasCondensedSetpoint ? isobus::DataDescriptionIndex::SetpointCondensedWorkState241_256 : isobus::DataDescriptionIndex::ActualCondensedWorkState241_256); - auto owners = section_layout::resolve_condensed_owners(state.get_pool(), element_hierarchy::build_parent_map(state.get_pool()), sectionElementNumbers, firstDdi, lastDdi); + // DDI 161 is only a command target where it is settable; a read-only one just reports + auto owners = section_layout::resolve_condensed_owners(state.get_pool(), element_hierarchy::build_parent_map(state.get_pool()), sectionElementNumbers, firstDdi, lastDdi, !hasCondensedSetpoint); if (owners.size() > 1) { log("TC Server") << "Sections are split over " << owners.size() << " elements with their own condensed DDIs:" << std::endl; diff --git a/tools/test_section_layout.cpp b/tools/test_section_layout.cpp index 2188a86..c944ad8 100644 --- a/tools/test_section_layout.cpp +++ b/tools/test_section_layout.cpp @@ -11,6 +11,7 @@ #include "isobus/isobus/isobus_device_descriptor_object_pool_helpers.hpp" +#include #include #include #include @@ -140,6 +141,17 @@ static void test_missing_owner_falls_back() check(owners_of(pool, 1).empty(), "no element with DDI 290 gives no owners"); } +static void test_read_only_actual_is_not_an_owner() +{ + // The seeder pool links DDI 161 read-only (properties 0x01): it reports, but cannot be commanded + isobus::DeviceDescriptorObjectPool pool; + check(build_seeder_pool(pool, { 1, 1, 1 }), "three-function pool builds"); + const auto parents = element_hierarchy::build_parent_map(pool); + check(section_layout::resolve_condensed_owners(pool, parents, section_elements(3), 161, 176).size() == 3, "any linked DDI 161 counts when settability is not asked for"); + check(section_layout::resolve_condensed_owners(pool, parents, section_elements(3), 161, 176, true).empty(), "a read-only DDI 161 is not a command target"); + check(section_layout::resolve_condensed_owners(pool, parents, section_elements(3), DDI_SETPOINT_CONDENSED_1_16, DDI_SETPOINT_CONDENSED_241_256, true).size() == 3, "a settable DDI 290 still is"); +} + static void test_grouping() { using section_layout::group_overlapping_sections; @@ -164,6 +176,10 @@ static void test_grouping() auto differentWidth = group_overlapping_sections({ { 0, 3000 }, { 0, 1500 } }); check(!differentWidth.grouped, "same centre but different width stays separate"); + + // Offsets at the ends of the int32 range differ by more than an int32 holds + auto extremes = group_overlapping_sections({ { INT32_MIN, 3000 }, { INT32_MAX, 3000 }, { 0, INT32_MAX }, { 0, 1 } }); + check(!extremes.grouped && extremes.numberOfAogSections == 4, "int32 extremes do not wrap around into one AOG section"); } // Prints the owners and the DDOP's fixed geometry for a captured binary DDOP, in the TC's section order @@ -233,6 +249,7 @@ int main(int argc, char **argv) test_mixed_section_counts(); test_owner_on_device_element(); test_missing_owner_falls_back(); + test_read_only_actual_is_not_an_owner(); test_grouping(); if (failures != 0) From 7c02ace6c922139d077aa13f6566101891f73cd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gunics=20Bal=C3=A1zs?= Date: Wed, 11 Nov 2026 19:59:35 +0100 Subject: [PATCH 6/6] Drop allSectionsFollowFirst: groupOverlappingSections covers it allSectionsFollowFirst was a quick override to validate the RDS Artemis on one width. Grouping by geometry does the same for that machine and also handles implements it cannot, such as a 3 + 3 m seeder with three products per half (two AOG sections), so the override is removed. The demand check keeps using each implement section's AOG section. Co-Authored-By: Claude Opus 5.5 --- include/settings.hpp | 10 ---------- readme.md | 3 +-- src/app.cpp | 32 ++------------------------------ src/settings.cpp | 13 ------------- 4 files changed, 3 insertions(+), 55 deletions(-) diff --git a/include/settings.hpp b/include/settings.hpp index 1b65de9..859e5f1 100644 --- a/include/settings.hpp +++ b/include/settings.hpp @@ -98,14 +98,6 @@ class Settings */ bool set_vt_enabled(bool enabled, bool save = true); - /// @brief Check whether every implement section follows AOG's section 1. - /// @details For implements that declare several sections (or booms) that really are one, such as - /// several dispensers on one working width. AOG then sees the implement as a single section. - bool is_all_sections_follow_first() const; - - /// @brief Set whether every implement section follows AOG's section 1. - bool set_all_sections_follow_first(bool enabled, bool save = true); - /// @brief Check whether sections that cover the same strip (same Y offset and width) share one AOG section. bool is_group_overlapping_sections() const; @@ -175,7 +167,6 @@ class Settings constexpr static bool DEFAULT_NMEA_SEND_ENABLED = true; constexpr static bool DEFAULT_AOG_HEARTBEAT_ENABLED = true; constexpr static bool DEFAULT_VT_ENABLED = true; - constexpr static bool DEFAULT_ALL_SECTIONS_FOLLOW_FIRST = false; constexpr static bool DEFAULT_GROUP_OVERLAPPING_SECTIONS = false; constexpr static std::uint8_t DEFAULT_TC_VERSION = 3; ///< SecondEditionDraft (V3 default for maximum implement compatibility) static const std::string DEFAULT_LANGUAGE_CODE; @@ -185,7 +176,6 @@ class Settings bool nmeaSendEnabled = DEFAULT_NMEA_SEND_ENABLED; bool aogHeartbeatEnabled = DEFAULT_AOG_HEARTBEAT_ENABLED; bool vtEnabled = DEFAULT_VT_ENABLED; - bool allSectionsFollowFirst = DEFAULT_ALL_SECTIONS_FOLLOW_FIRST; bool groupOverlappingSections = DEFAULT_GROUP_OVERLAPPING_SECTIONS; std::uint8_t tcVersion = DEFAULT_TC_VERSION; std::string languageCode = DEFAULT_LANGUAGE_CODE; diff --git a/readme.md b/readme.md index b080bd6..c5d5830 100644 --- a/readme.md +++ b/readme.md @@ -64,8 +64,7 @@ AOG-TaskController reads its configuration from a `settings.json` file located i | `nmeaSendEnabled` | `bool` | `true` | Enables cyclic NMEA2000 COG/SOG transmission. Requires `tecuEnabled`. | | `aogHeartbeatEnabled` | `bool` | `true` | Sends the 100 ms zero-section heartbeat when no implement is connected. Disable for AgOpenGPS versions before v6.8.2 beta 5. | | `vtEnabled` | `bool` | `true` | Enables the ISOBUS Virtual Terminal UI. | -| `allSectionsFollowFirst` | `bool` | `false` | Every implement section follows AOG's section 1, and AOG sees the implement as one section (on while any section is on). For implements that declare several sections or booms that really are one, such as several dispensers on one working width. Set AOG to 1 section. Applies to every connected implement: with two section-controlled implements, both follow section 1 and AOG gets one combined report. | -| `groupOverlappingSections` | `bool` | `false` | Sections with the same lateral offset and width (within 10 mm) share one AOG section, e.g. several dispensers on one working width. The TC requests the offsets and widths an implement only declares as process data. The resulting mapping is logged even while this is off. | +| `groupOverlappingSections` | `bool` | `false` | Sections with the same lateral offset and width (within 10 mm) share one AOG section, e.g. several dispensers on one working width, or a 3 + 3 m seeder with three products per half (two AOG sections). AOG's section count must match the grouped count. The TC requests the offsets and widths an implement only declares as process data. The resulting mapping is logged even while this is off. | | `tcVersion` | `integer` | `3` | Task Controller version code, from `0` through `4`; `3` is Second Edition Draft. | | `languageCode` | `string` | `"en"` | Two-character language code advertised on ISOBUS. | | `countryCode` | `string` | `"US"` | Two-character country code advertised on ISOBUS. | diff --git a/src/app.cpp b/src/app.cpp index 5b6a91d..2586389 100644 --- a/src/app.cpp +++ b/src/app.cpp @@ -508,12 +508,6 @@ void Application::setup_udp_connections() sectionDemand = sectionStates; sectionDemandChangesSinceLog++; } - if (settings->is_all_sections_follow_first()) - { - // The implement's sections are really one: all follow AOG's section 1 (sectionDemand above stays as AOG sent it). - // PGN 229 carries 64 bits; cover every section a DDOP can declare (up to 255), not just the first 64. - sectionStates.assign(256, sectionStates.front()); - } tcServer->update_section_states(sectionStates); } else if (pgn == 0xF1 && !data.empty()) // 241 - Section Control @@ -735,9 +729,6 @@ bool Application::update() if (isobus::SystemTiming::time_expired_ms(lastHeartbeatTransmit, 100)) { bool anyClientWithSections = false; - const bool followFirst = settings->is_all_sections_follow_first(); - bool followFirstEnabled = false; - bool followFirstOn = false; for (auto &client : tcServer->get_clients()) { auto &state = client.second; @@ -750,18 +741,6 @@ bool Application::update() anyClientWithSections = true; - if (followFirst) - { - // AOG is set up with one section, and section 1 drives every connected implement: - // collect them all into one report, on while any implement section is on - followFirstEnabled = followFirstEnabled || state.is_section_control_enabled(); - for (std::uint8_t i = 0; i < state.get_number_of_sections(); i++) - { - followFirstOn = followFirstOn || (state.get_section_actual_state(i) == SectionState::ON); - } - continue; - } - // AOG's sections: overlapping implement sections are reported as one (identity mapping otherwise) const std::uint8_t aogSections = state.get_number_of_aog_sections(); std::vector data = { state.is_section_control_enabled(), aogSections }; @@ -783,12 +762,6 @@ bool Application::update() udpConnections->send(0x80, 0xF0, data); } - if (followFirst && anyClientWithSections) - { - std::vector oneSection = { followFirstEnabled, 1, static_cast(followFirstOn ? 1 : 0) }; - udpConnections->send(0x80, 0xF0, oneSection); - } - // If no clients with sections, send heartbeat with 0 sections if (settings->is_aog_heartbeat_enabled() && !anyClientWithSections) { @@ -1060,9 +1033,8 @@ void Application::check_section_demand() for (std::uint8_t i = 0; i < state.get_number_of_sections(); i++) { actual.push_back(SectionState::ON == state.get_section_actual_state(i)); - // Demand is AOG's bit for the AOG section this implement section belongs to (grouped sections share - // one); with allSectionsFollowFirst, AOG has one section, so any other bit is padding - const std::uint8_t demandIndex = settings->is_all_sections_follow_first() ? 0 : state.get_aog_section_for_section(i); + // Demand is AOG's bit for the AOG section this implement section belongs to (grouped sections share one) + const std::uint8_t demandIndex = state.get_aog_section_for_section(i); demanded = demanded || (arriving && (demandIndex < sectionDemand.size()) && sectionDemand[demandIndex]); } const bool working = (actual.end() != std::find(actual.begin(), actual.end(), true)); diff --git a/src/settings.cpp b/src/settings.cpp index 6f5a050..857366d 100644 --- a/src/settings.cpp +++ b/src/settings.cpp @@ -70,7 +70,6 @@ bool Settings::load() nmeaSendEnabled = loadBoolean("nmeaSendEnabled", DEFAULT_NMEA_SEND_ENABLED); aogHeartbeatEnabled = loadBoolean("aogHeartbeatEnabled", DEFAULT_AOG_HEARTBEAT_ENABLED); vtEnabled = loadBoolean("vtEnabled", DEFAULT_VT_ENABLED); - allSectionsFollowFirst = loadBoolean("allSectionsFollowFirst", DEFAULT_ALL_SECTIONS_FOLLOW_FIRST); groupOverlappingSections = loadBoolean("groupOverlappingSections", DEFAULT_GROUP_OVERLAPPING_SECTIONS); if (data.contains("tcVersion")) @@ -145,7 +144,6 @@ bool Settings::save() const data["nmeaSendEnabled"] = nmeaSendEnabled; data["aogHeartbeatEnabled"] = aogHeartbeatEnabled; data["vtEnabled"] = vtEnabled; - data["allSectionsFollowFirst"] = allSectionsFollowFirst; data["groupOverlappingSections"] = groupOverlappingSections; data["tcVersion"] = tcVersion; data["languageCode"] = languageCode; @@ -265,17 +263,6 @@ bool Settings::set_vt_enabled(bool enabled, bool save) return set_boolean(vtEnabled, enabled, save); } -bool Settings::is_all_sections_follow_first() const -{ - std::scoped_lock lock(settingsMutex); - return allSectionsFollowFirst; -} - -bool Settings::set_all_sections_follow_first(bool enabled, bool save) -{ - return set_boolean(allSectionsFollowFirst, enabled, save); -} - bool Settings::is_group_overlapping_sections() const { std::scoped_lock lock(settingsMutex);