From 6c7debbfe5558d8c9ef9dc4af6e4b6205bad0811 Mon Sep 17 00:00:00 2001 From: Sujan Dumaru Date: Wed, 30 Sep 2026 00:14:20 -0500 Subject: [PATCH 1/6] Add tinyxml2 as a submodule --- .gitmodules | 3 +++ CMakeLists.txt | 11 +++++++++-- submodules/tinyxml2 | 1 + 3 files changed, 13 insertions(+), 2 deletions(-) create mode 160000 submodules/tinyxml2 diff --git a/.gitmodules b/.gitmodules index fa7cbef..10dd70a 100644 --- a/.gitmodules +++ b/.gitmodules @@ -7,3 +7,6 @@ [submodule "submodules/agisostack"] path = submodules/agisostack url = https://github.com/Open-Agriculture/AgIsoStack-plus-plus.git +[submodule "submodules/tinyxml2"] + path = submodules/tinyxml2 + url = https://github.com/leethomason/tinyxml2 diff --git a/CMakeLists.txt b/CMakeLists.txt index e66b5d3..37eb9cb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -33,6 +33,7 @@ find_package(OpenGL REQUIRED) set(BUILD_TESTING OFF) add_subdirectory(submodules/agisostack) add_subdirectory(submodules/sdl) +add_subdirectory(submodules/tinyxml2 EXCLUDE_FROM_ALL) target_sources( AgIsoDDOPGenerator @@ -51,8 +52,14 @@ target_include_directories( submodules/imgui/backends submodules/sdl/include) target_link_libraries( - AgIsoDDOPGenerator PRIVATE isobus::Isobus isobus::Utility OpenGL::GL SDL2 - SDL2main ${CMAKE_DL_LIBS}) + AgIsoDDOPGenerator + PRIVATE isobus::Isobus + isobus::Utility + OpenGL::GL + SDL2 + SDL2main + tinyxml2::tinyxml2 + ${CMAKE_DL_LIBS}) enable_testing() add_subdirectory(test) diff --git a/submodules/tinyxml2 b/submodules/tinyxml2 new file mode 160000 index 0000000..9148bdf --- /dev/null +++ b/submodules/tinyxml2 @@ -0,0 +1 @@ +Subproject commit 9148bdf719e997d1f474be6bcc7943881046dba1 From 4932a387b6a2787009b3ae9627022a9ccf20b61b Mon Sep 17 00:00:00 2001 From: Sujan Dumaru Date: Wed, 30 Sep 2026 00:40:00 -0500 Subject: [PATCH 2/6] Import a DDOP from an ISOXML TASKDATA.XML --- .github/workflows/build.yml | 2 +- CMakeLists.txt | 1 + include/task_data_import.hpp | 24 ++++ src/task_data_import.cpp | 237 +++++++++++++++++++++++++++++++++ test/CMakeLists.txt | 16 ++- test/example_pool_test.cpp | 31 ++++- test/task_data_import_test.cpp | 139 +++++++++++++++++++ 7 files changed, 444 insertions(+), 6 deletions(-) create mode 100644 include/task_data_import.hpp create mode 100644 src/task_data_import.cpp create mode 100644 test/task_data_import_test.cpp diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d2ee118..0b3fa90 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -99,7 +99,7 @@ jobs: - name: Configure run: cmake -S . -B build -DBUILD_EXAMPLES=OFF -DBUILD_TESTING=OFF -DCAN_DRIVER=None -DCMAKE_BUILD_TYPE=Release - name: Compile tests - run: cmake --build build --target example_pool_test + run: cmake --build build --target example_pool_test task_data_import_test - name: Run tests run: ctest --test-dir build --output-on-failure diff --git a/CMakeLists.txt b/CMakeLists.txt index 37eb9cb..656017e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -39,6 +39,7 @@ target_sources( AgIsoDDOPGenerator PRIVATE src/main.cpp src/gui.cpp + src/task_data_import.cpp submodules/imgui/imgui.cpp submodules/imgui/imgui_demo.cpp submodules/imgui/imgui_draw.cpp diff --git a/include/task_data_import.hpp b/include/task_data_import.hpp new file mode 100644 index 0000000..c48f995 --- /dev/null +++ b/include/task_data_import.hpp @@ -0,0 +1,24 @@ +//================================================================================================ +/// @file task_data_import.hpp +/// +/// @brief Builds a DDOP from a device description in an ISOXML TASKDATA.XML +/// @author Sujan Dumaru +/// +/// @copyright 2026 The Open-Agriculture developers +//================================================================================================ +#ifndef TASK_DATA_IMPORT_HPP +#define TASK_DATA_IMPORT_HPP + +#include "isobus/isobus/isobus_device_descriptor_object_pool.hpp" + +#include +#include +#include + +/// The DVC elements of a TASKDATA.XML as "A B" labels, in file order. Empty if it does not parse or has no inline DVC. +std::vector list_task_data_devices(const std::string &taskData); + +/// Adds the objects of the deviceIndex-th DVC to an empty pool whose TC level is already set. +bool import_task_data_device(const std::string &taskData, std::size_t deviceIndex, isobus::DeviceDescriptorObjectPool &pool); + +#endif // TASK_DATA_IMPORT_HPP diff --git a/src/task_data_import.cpp b/src/task_data_import.cpp new file mode 100644 index 0000000..4d8d391 --- /dev/null +++ b/src/task_data_import.cpp @@ -0,0 +1,237 @@ +//================================================================================================ +/// @file task_data_import.cpp +/// +/// @brief Builds a DDOP from a device description in an ISOXML TASKDATA.XML +/// @author Sujan Dumaru +/// +/// @copyright 2026 The Open-Agriculture developers +//================================================================================================ +#include "task_data_import.hpp" +#include "isobus/isobus/can_constants.hpp" +#include "isobus/isobus/can_stack_logger.hpp" +#include "tinyxml2.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +static bool log_invalid_attribute(const tinyxml2::XMLElement &element, const char *attribute) +{ + const char *id = element.Attribute("A"); + LOG_ERROR("[DDOP]: TASKDATA %s \"%s\" has a missing or invalid %s attribute.", element.Name(), (nullptr != id) ? id : "", attribute); + return false; +} + +static std::string read_text(const tinyxml2::XMLElement &element, const char *attribute) +{ + const char *text = element.Attribute(attribute); + return (nullptr != text) ? text : ""; +} + +static bool read_hex(const tinyxml2::XMLElement &element, const char *attribute, std::size_t digits, std::uint64_t &value) +{ + const char *text = element.Attribute(attribute); + const bool valid = (nullptr != text) && (digits == std::strlen(text)) && + std::all_of(text, text + digits, [](char digit) { return 0 != std::isxdigit(static_cast(digit)); }); + + if (valid) + { + value = std::strtoull(text, nullptr, 16); + } + else + { + log_invalid_attribute(element, attribute); + } + return valid; +} + +// ISOXML writes a label's last byte first, so the first hex pair is byte 6 and the lowest byte of the number is byte 0. +static std::array to_label(std::uint64_t value) +{ + std::array label = {}; + + for (std::size_t i = 0; i < label.size(); i++) + { + label[i] = static_cast(value >> (8 * i)); + } + return label; +} + +// The classic locale keeps a decimal comma locale from misreading a scale such as 0.001. +template +static bool parse_number(const char *text, T &value) +{ + std::istringstream stream((nullptr != text) ? text : ""); + stream.imbue(std::locale::classic()); + stream >> value; + return (!stream.fail()) && stream.eof(); +} + +template +static bool read_integer(const tinyxml2::XMLElement &element, + const char *attribute, + T &value, + std::int64_t minimum = std::numeric_limits::min(), + std::int64_t maximum = std::numeric_limits::max()) +{ + std::int64_t number = 0; + const bool valid = parse_number(element.Attribute(attribute), number) && (minimum <= number) && (number <= maximum); + + if (valid) + { + value = static_cast(number); + } + else + { + log_invalid_attribute(element, attribute); + } + return valid; +} + +static bool import_object(const tinyxml2::XMLElement &element, isobus::DeviceDescriptorObjectPool &pool) +{ + const std::string name = element.Name(); + std::uint16_t objectID = 0; + std::uint64_t ddi = 0; + std::uint16_t presentationID = isobus::NULL_OBJECT_ID; + bool success = true; + + if ("DET" == name) + { + std::uint8_t type = 0; + std::uint16_t elementNumber = 0; + std::uint16_t parentID = 0; + + success = read_integer(element, "B", objectID) && + read_integer(element, "C", type, 1, 7) && + read_integer(element, "E", elementNumber) && + read_integer(element, "F", parentID) && + pool.add_device_element(read_text(element, "D"), elementNumber, parentID, static_cast(type), objectID); + + for (auto reference = element.FirstChildElement("DOR"); success && (nullptr != reference); reference = reference->NextSiblingElement("DOR")) + { + std::uint16_t childID = 0; + success = read_integer(*reference, "A", childID); + + if (success) + { + std::static_pointer_cast(pool.get_object_by_id(objectID))->add_reference_to_child_object(childID); + } + } + } + else if ("DPD" == name) + { + std::uint8_t properties = 0; + std::uint8_t triggers = 0; + + success = read_integer(element, "A", objectID) && + read_hex(element, "B", 4, ddi) && + read_integer(element, "C", properties) && + read_integer(element, "D", triggers) && + ((nullptr == element.Attribute("F")) || read_integer(element, "F", presentationID)) && + pool.add_device_process_data(read_text(element, "E"), static_cast(ddi), presentationID, properties, triggers, objectID); + } + else if ("DPT" == name) + { + std::int32_t value = 0; + + success = read_integer(element, "A", objectID) && + read_hex(element, "B", 4, ddi) && + read_integer(element, "C", value) && + ((nullptr == element.Attribute("E")) || read_integer(element, "E", presentationID)) && + pool.add_device_property(read_text(element, "D"), value, static_cast(ddi), presentationID, objectID); + } + else if ("DVP" == name) + { + std::int32_t offset = 0; + float scale = 0.0f; + std::uint8_t decimals = 0; + + success = read_integer(element, "A", objectID) && + read_integer(element, "B", offset) && + (parse_number(element.Attribute("C"), scale) || log_invalid_attribute(element, "C")) && + read_integer(element, "D", decimals) && + pool.add_device_value_presentation(read_text(element, "E"), offset, scale, decimals, objectID); + } + return success; +} + +static const tinyxml2::XMLElement *first_device(tinyxml2::XMLDocument &document, const std::string &taskData) +{ + const tinyxml2::XMLElement *device = nullptr; + + if (tinyxml2::XML_SUCCESS != document.Parse(taskData.data(), taskData.size())) + { + LOG_ERROR("[DDOP]: The file is not valid XML: %s", document.ErrorStr()); + } + else if (nullptr == document.FirstChildElement("ISO11783_TaskData")) + { + LOG_ERROR("[DDOP]: The XML file has no ISO11783_TaskData root element."); + } + else + { + device = document.FirstChildElement("ISO11783_TaskData")->FirstChildElement("DVC"); + + if (nullptr == device) + { + LOG_ERROR("[DDOP]: The TASKDATA.XML has no inline DVC element. Device descriptions in files referenced through XFR are not supported."); + } + } + return device; +} + +std::vector list_task_data_devices(const std::string &taskData) +{ + tinyxml2::XMLDocument document; + std::vector labels; + + for (auto device = first_device(document, taskData); nullptr != device; device = device->NextSiblingElement("DVC")) + { + labels.push_back(read_text(*device, "A") + " " + read_text(*device, "B")); + } + return labels; +} + +bool import_task_data_device(const std::string &taskData, std::size_t deviceIndex, isobus::DeviceDescriptorObjectPool &pool) +{ + tinyxml2::XMLDocument document; + const tinyxml2::XMLElement *device = first_device(document, taskData); + + for (std::size_t i = 0; (nullptr != device) && (i < deviceIndex); i++) + { + device = device->NextSiblingElement("DVC"); + + if (nullptr == device) + { + LOG_ERROR("[DDOP]: The TASKDATA.XML has no DVC number %zu.", deviceIndex + 1); + } + } + + std::uint64_t isoName = 0; + std::uint64_t structureLabel = 0; + std::uint64_t localizationLabel = 0; + bool success = (nullptr != device) && + read_hex(*device, "D", 16, isoName) && + // ponytail: extended structure label unsupported until a real v4 file shows its byte order + read_hex(*device, "F", 14, structureLabel) && + read_hex(*device, "G", 14, localizationLabel); + + if (success) + { + const std::array structureBytes = to_label(structureLabel); + success = pool.add_device(read_text(*device, "B"), read_text(*device, "C"), read_text(*device, "E"), std::string(structureBytes.begin(), structureBytes.end()), to_label(localizationLabel), {}, isoName); + } + + for (auto child = success ? device->FirstChildElement() : nullptr; success && (nullptr != child); child = child->NextSiblingElement()) + { + success = import_object(*child, pool); + } + return success; +} diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 1d65776..66f8ca5 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -1,10 +1,18 @@ -add_executable(example_pool_test example_pool_test.cpp) -set_property(TARGET example_pool_test PROPERTY CXX_STANDARD 17) -set_property(TARGET example_pool_test PROPERTY CXX_STANDARD_REQUIRED true) -target_link_libraries(example_pool_test PRIVATE isobus::Isobus isobus::Utility) +foreach(test_target example_pool_test task_data_import_test) + add_executable(${test_target} ${test_target}.cpp + ${PROJECT_SOURCE_DIR}/src/task_data_import.cpp) + set_property(TARGET ${test_target} PROPERTY CXX_STANDARD 17) + set_property(TARGET ${test_target} PROPERTY CXX_STANDARD_REQUIRED true) + target_include_directories(${test_target} + PRIVATE ${PROJECT_SOURCE_DIR}/include) + target_link_libraries(${test_target} PRIVATE isobus::Isobus isobus::Utility + tinyxml2::tinyxml2) +endforeach() add_test(NAME example_v3_pool_round_trip COMMAND example_pool_test "${PROJECT_SOURCE_DIR}/EXAMPLE.ddop" 3) add_test(NAME example_v4_pool_round_trip COMMAND example_pool_test "${PROJECT_SOURCE_DIR}/EXAMPLE_V4.ddop" 4) + +add_test(NAME task_data_import COMMAND task_data_import_test) diff --git a/test/example_pool_test.cpp b/test/example_pool_test.cpp index 9500748..2e7e29c 100644 --- a/test/example_pool_test.cpp +++ b/test/example_pool_test.cpp @@ -7,7 +7,9 @@ /// @copyright 2026 The Open-Agriculture developers //================================================================================================ #include "isobus/isobus/isobus_device_descriptor_object_pool.hpp" +#include "task_data_import.hpp" +#include #include #include #include @@ -65,6 +67,33 @@ int main(int argc, char **argv) return 1; } - std::printf("PASS: %u objects, %zu bytes, byte-identical round trip\n", pool.size(), savedBytes.size()); + std::string exportedXml; + isobus::DeviceDescriptorObjectPool importedPool(pool.get_task_controller_compatibility_level()); + std::vector importedBytes; + std::string reexportedXml; + + if (!pool.generate_task_data_iso_xml(exportedXml) || + (1 != list_task_data_devices(exportedXml).size()) || + !import_task_data_device(exportedXml, 0, importedPool) || + !importedPool.generate_binary_object_pool(importedBytes) || + !importedPool.generate_task_data_iso_xml(reexportedXml)) + { + std::fprintf(stderr, "FAIL: %s did not survive an ISOXML export and import\n", argv[1]); + return 1; + } + + if (exportedXml != reexportedXml) + { + const auto offset = static_cast(std::mismatch(exportedXml.begin(), exportedXml.end(), reexportedXml.begin(), reexportedXml.end()).first - exportedXml.begin()); + std::fprintf(stderr, + "FAIL: re-exporting the imported ISOXML of %s differs at byte %zu:\n%.60s\n%.60s\n", + argv[1], + offset, + exportedXml.c_str() + offset, + reexportedXml.c_str() + offset); + return 1; + } + + std::printf("PASS: %u objects, %zu bytes, byte-identical round trip, identical ISOXML round trip\n", pool.size(), savedBytes.size()); return 0; } diff --git a/test/task_data_import_test.cpp b/test/task_data_import_test.cpp new file mode 100644 index 0000000..db142b1 --- /dev/null +++ b/test/task_data_import_test.cpp @@ -0,0 +1,139 @@ +//================================================================================================ +/// @file task_data_import_test.cpp +/// +/// @brief Checks that a hand-written TASKDATA.XML device description imports into a DDOP +/// @author Sujan Dumaru +/// +/// @copyright 2026 The Open-Agriculture developers +//================================================================================================ +#include "task_data_import.hpp" +#include "isobus/isobus/can_constants.hpp" +#include "isobus/isobus/isobus_device_descriptor_object_pool.hpp" + +#include +#include +#include +#include +#include +#include +#include + +static const std::string TASK_DATA = R"( + + + + + + + + + + + + + + + + + + + +)"; + +static int failureCount = 0; + +static void check(bool condition, const char *description) +{ + if (!condition) + { + std::fprintf(stderr, "FAIL: %s\n", description); + failureCount++; + } +} + +template +static std::shared_ptr get_object(isobus::DeviceDescriptorObjectPool &pool, std::uint16_t objectID) +{ + return std::dynamic_pointer_cast(pool.get_object_by_id(objectID)); +} + +int main() +{ + const std::vector labels = list_task_data_devices(TASK_DATA); + check((2 == labels.size()) && ("DVC-1 Tractor ECU" == labels.at(0)) && ("DVC-2 Sprayer & Boom" == labels.at(1)), + "list_task_data_devices returns both DVCs as \"A B\""); + + isobus::DeviceDescriptorObjectPool pool(4); + if (!import_task_data_device(TASK_DATA, 1, pool)) + { + std::fprintf(stderr, "FAIL: the second DVC did not import\n"); + return 1; + } + + const auto device = get_object(pool, 0); + const auto rootElement = get_object(pool, 5001); + const auto section = get_object(pool, 5003); + const auto workState = get_object(pool, 5006); + const auto offsetX = get_object(pool, 5005); + const auto hours = get_object(pool, 6001); + const auto hectares = get_object(pool, 6002); + + if ((nullptr == device) || (nullptr == rootElement) || (nullptr == section) || (nullptr == workState) || + (nullptr == offsetX) || (nullptr == hours) || (nullptr == hectares)) + { + std::fprintf(stderr, "FAIL: an imported object is missing or has the wrong type\n"); + return 1; + } + + check(9 == pool.size(), "the pool holds the device and its 8 objects"); + check(0xA00086000CE01234 == device->get_iso_name(), "D is read as a big-endian NAME"); + check("Sprayer & Boom" == device->get_designator(), "the designator is entity-decoded"); + check(device->get_serial_number().empty(), "a DVC without E has an empty serial number"); + check("7654321" == device->get_structure_label(), "F is read in reversed byte order"); + check((0x65 == device->get_localization_label().at(0)) && (0x6E == device->get_localization_label().at(1)) && + (0xFF == device->get_localization_label().at(6)), + "G is read in reversed byte order"); + + check(isobus::task_controller_object::DeviceElementObject::Type::Device == rootElement->get_type(), "the root DET is a Device element"); + check(5001 == section->get_parent_object(), "the section DET has the root DET as its parent"); + check(isobus::task_controller_object::DeviceElementObject::Type::Section == section->get_type(), "the section DET has type 4"); + check(section->get_designator().empty(), "a DET without D has an empty designator"); + check((2 == section->get_number_child_objects()) && (5005 == section->get_child_object_id(0)) && + (5006 == section->get_child_object_id(1)), + "the section DET keeps its DOR children in order"); + + check(0x008D == workState->get_ddi(), "a DPD DDI is read as hex"); + check(isobus::NULL_OBJECT_ID == workState->get_device_value_presentation_object_id(), "a DPD without F has no DVP"); + check(-1200 == offsetX->get_value(), "a DPT value can be negative"); + check(isobus::NULL_OBJECT_ID == offsetX->get_device_value_presentation_object_id(), "a DPT without E has no DVP"); + check(std::fabs(hours->get_scale() - 0.0000012345) < 1e-9, "the hours scale keeps its precision"); + check(std::fabs(hectares->get_scale() - 0.000099999997474) < 1e-9, "the hectares scale keeps its precision"); + check("ha " == hectares->get_designator(), "a space-padded unit is kept as is"); + + std::vector binaryPool; + check(pool.generate_binary_object_pool(binaryPool), "the imported pool serializes"); + + const std::vector> rejects = { + { R"(B="5003" C="4")", R"(B="5003" C="9")" }, { R"(B="5003" C="4")", R"(B="5003" C="-1")" }, { R"(B="5003" C="4")", R"(B="5003" C="4x")" }, { R"(F="31323334353637")", R"(F="3132333435363738")" }, { R"( " + reject.second + " is rejected").c_str()); + } + + for (const std::string ¬TaskData : { std::string(), std::string("DVC\x01\x02"), std::string("") }) + { + isobus::DeviceDescriptorObjectPool emptyPool(4); + check(list_task_data_devices(notTaskData).empty(), "input that is not a TASKDATA.XML lists no devices"); + check(!import_task_data_device(notTaskData, 0, emptyPool), "input that is not a TASKDATA.XML does not import"); + } + + if (0 == failureCount) + { + std::printf("PASS: TASKDATA.XML device import\n"); + } + return (0 == failureCount) ? 0 : 1; +} From 1121f747f7b9b4c50c9a352886140f3605133190 Mon Sep 17 00:00:00 2001 From: Sujan Dumaru Date: Wed, 30 Sep 2026 12:40:59 -0500 Subject: [PATCH 3/6] Import a device from an ISOXML TASKDATA.XML --- README.md | 2 + include/gui.hpp | 4 ++ src/gui.cpp | 104 +++++++++++++++++++++++++++++++++---- src/task_data_import.cpp | 2 +- test/example_pool_test.cpp | 18 ++++--- 5 files changed, 112 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index bf10944..e5d722b 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,8 @@ When used in combination with AgIsoStack (or any other TC client), it provides a **File → New** starts an empty DDOP and asks for your device information; **File → Open** loads an existing one. `EXAMPLE.ddop` and `EXAMPLE_V4.ddop` in this repository are there to open and poke at. +**File → Import ISOXML** reads the device description in an ISOXML TASKDATA.XML, and asks you to pick a device if the file holds several. **File → Save** then asks for a new file name, as it writes a binary DDOP. + The left pane is the object tree, and whatever you select in it is edited in the right pane. **Create Object** adds a device element, process data, property or value presentation, and a device element gains children by referencing objects that already exist. **Edit → Check for Errors** serializes the pool and reports what a task controller would reject, including device elements that share an element number. **File → Save** writes the binary DDOP, and **File → Export as ISOXML** writes the same pool as a TASKDATA.XML. diff --git a/include/gui.hpp b/include/gui.hpp index c87090a..e2d5706 100644 --- a/include/gui.hpp +++ b/include/gui.hpp @@ -79,6 +79,9 @@ class DDOPGeneratorGUI char hexIsoNameBuffer[17] = { 0 }; char languageCodeBuffer[3] = { 0 }; std::string lastFileName; + std::string pendingTaskData; + std::vector taskDataDevices; + int selectedTaskDataDevice = 0; int elementNumberBuffer = 0; int parentObjectBuffer = 0; int ddiBuffer = 0; @@ -94,6 +97,7 @@ class DDOPGeneratorGUI std::array propertiesBitfieldBuffer = { false }; std::array triggerBitfieldBuffer = { false }; bool openFileDialogue = false; + bool importTaskDataDialogue = false; bool saveModal = false; bool saveAsModal = false; bool exportModal = false; diff --git a/src/gui.cpp b/src/gui.cpp index 274f1b2..2cf4b9d 100644 --- a/src/gui.cpp +++ b/src/gui.cpp @@ -16,7 +16,9 @@ #include "isobus/isobus/isobus_data_dictionary.hpp" #include "isobus/utility/iop_file_interface.hpp" #include "logsink.hpp" +#include "task_data_import.hpp" +#include #include #include #include @@ -328,6 +330,13 @@ bool DDOPGeneratorGUI::render_menu_bar() { FileDialog::file_dialog_open = true; openFileDialogue = true; + importTaskDataDialogue = false; + } + if (ImGui::MenuItem("Import ISOXML", "Load a device from an ISOXML TASKDATA.XML file")) + { + FileDialog::file_dialog_open = true; + importTaskDataDialogue = true; + openFileDialogue = false; } if (!currentPoolValid) @@ -549,17 +558,45 @@ bool DDOPGeneratorGUI::render_menu_bar() return retVal; } +/// @brief Renders a modal listing the logged errors of the last load attempt +/// @param[in] title The popup title, which is also the ID to open it with +/// @param[in] heading The text shown above the log +static void render_log_error_popup(const char *title, const char *heading) +{ + if (ImGui::BeginPopupModal(title, NULL, ImGuiWindowFlags_AlwaysAutoResize)) + { + ImGui::Text("%s", heading); + ImGui::Separator(); + + for (auto &logString : logger.logHistory) + { + ImGui::Text("%s", logString.logText.c_str()); + } + + ImGui::SetItemDefaultFocus(); + if (ImGui::Button("OK", ImVec2(120, 0))) + { + ImGui::CloseCurrentPopup(); + } + ImGui::EndPopup(); + } +} + void DDOPGeneratorGUI::render_open_file_menu() { + bool importSelectedDevice = false; + if (FileDialog::file_dialog_open) { FileDialog::ShowFileDialog(&FileDialog::file_dialog_open, filePathBuffer, FILE_PATH_BUFFER_MAX_LENGTH, FileDialog::FileDialogType::OpenFile); } - else if (openFileDialogue) + else if (openFileDialogue || importTaskDataDialogue) { std::string selectedFileToRead(filePathBuffer); memset(filePathBuffer, 0, FILE_PATH_BUFFER_MAX_LENGTH); + const bool importingTaskData = importTaskDataDialogue; openFileDialogue = false; + importTaskDataDialogue = false; if (!selectedFileToRead.empty()) { @@ -581,7 +618,28 @@ void DDOPGeneratorGUI::render_open_file_menu() currentObjectPool->set_task_controller_compatibility_level(4); } - if (true == currentObjectPool->deserialize_binary_object_pool(loadedIopData, isobus::NAME(0))) + if (importingTaskData) + { + currentPoolValid = false; + pendingTaskData.assign(loadedIopData.begin(), loadedIopData.end()); + taskDataDevices = list_task_data_devices(pendingTaskData); + selectedTaskDataDevice = 0; + + if (taskDataDevices.empty()) + { + currentObjectPool.reset(); + ImGui::OpenPopup("Error Loading XML File"); + } + else if (1 == taskDataDevices.size()) + { + importSelectedDevice = true; + } + else + { + ImGui::OpenPopup("Select Device"); + } + } + else if (true == currentObjectPool->deserialize_binary_object_pool(loadedIopData, isobus::NAME(0))) { // Valid pool? currentPoolValid = true; @@ -602,23 +660,51 @@ void DDOPGeneratorGUI::render_open_file_menu() } } - if (ImGui::BeginPopupModal("Error Loading DDOP", NULL, ImGuiWindowFlags_AlwaysAutoResize)) + if (ImGui::BeginPopupModal("Select Device", NULL, ImGuiWindowFlags_AlwaysAutoResize)) { - ImGui::Text("There were errors loading the DDOP. Make sure you selected the correct TC version."); - ImGui::Separator(); - - for (auto &logString : logger.logHistory) + std::vector deviceLabels; + float listWidth = 0.0f; + for (const auto &label : taskDataDevices) { - ImGui::Text("%s", logString.logText.c_str()); + deviceLabels.push_back(label.c_str()); + listWidth = std::max(listWidth, ImGui::CalcTextSize(label.c_str()).x); } + ImGui::SetNextItemWidth(listWidth + ImGui::GetStyle().ScrollbarSize + 2.0f * ImGui::GetStyle().WindowPadding.x); + ImGui::ListBox("##Devices", &selectedTaskDataDevice, deviceLabels.data(), static_cast(deviceLabels.size())); - ImGui::SetItemDefaultFocus(); if (ImGui::Button("OK", ImVec2(120, 0))) { + importSelectedDevice = true; + ImGui::CloseCurrentPopup(); + } + ImGui::SameLine(); + if (ImGui::Button("Cancel", ImVec2(120, 0))) + { + currentObjectPool.reset(); + currentPoolValid = false; ImGui::CloseCurrentPopup(); } ImGui::EndPopup(); } + + // Outside the Select Device modal, so the error popup opens with the ID it is rendered under below. + if (importSelectedDevice) + { + if (import_task_data_device(pendingTaskData, static_cast(selectedTaskDataDevice), *currentObjectPool)) + { + currentPoolValid = true; + lastFileName.clear(); + } + else + { + currentObjectPool.reset(); + currentPoolValid = false; + ImGui::OpenPopup("Error Loading XML File"); + } + } + + render_log_error_popup("Error Loading DDOP", "There were errors loading the DDOP. Make sure you selected the correct TC version."); + render_log_error_popup("Error Loading XML File", "There were errors reading the TASKDATA.XML:"); } void DDOPGeneratorGUI::parseElementChildrenOfElement(std::uint16_t aObjectID) diff --git a/src/task_data_import.cpp b/src/task_data_import.cpp index 4d8d391..71a115d 100644 --- a/src/task_data_import.cpp +++ b/src/task_data_import.cpp @@ -6,6 +6,7 @@ /// /// @copyright 2026 The Open-Agriculture developers //================================================================================================ + #include "task_data_import.hpp" #include "isobus/isobus/can_constants.hpp" #include "isobus/isobus/can_stack_logger.hpp" @@ -219,7 +220,6 @@ bool import_task_data_device(const std::string &taskData, std::size_t deviceInde std::uint64_t localizationLabel = 0; bool success = (nullptr != device) && read_hex(*device, "D", 16, isoName) && - // ponytail: extended structure label unsupported until a real v4 file shows its byte order read_hex(*device, "F", 14, structureLabel) && read_hex(*device, "G", 14, localizationLabel); diff --git a/test/example_pool_test.cpp b/test/example_pool_test.cpp index 2e7e29c..6c63a89 100644 --- a/test/example_pool_test.cpp +++ b/test/example_pool_test.cpp @@ -6,12 +6,14 @@ /// /// @copyright 2026 The Open-Agriculture developers //================================================================================================ + #include "isobus/isobus/isobus_device_descriptor_object_pool.hpp" #include "task_data_import.hpp" #include #include #include +#include #include #include #include @@ -26,7 +28,7 @@ int main(int argc, char **argv) if (!validArgumentCount || !validVersion) { std::fprintf(stderr, "usage: %s [task controller version: 3 or 4 (default)]\n", argv[0]); - return 1; + return EXIT_FAILURE; } std::ifstream inputFile(argv[1], std::ios::binary); @@ -34,7 +36,7 @@ int main(int argc, char **argv) if (!inputFile) { std::fprintf(stderr, "FAIL: cannot open %s\n", argv[1]); - return 1; + return EXIT_FAILURE; } const std::vector fileBytes((std::istreambuf_iterator(inputFile)), @@ -46,7 +48,7 @@ int main(int argc, char **argv) if (!pool.deserialize_binary_object_pool(loadedBytes, isobus::NAME(0))) { std::fprintf(stderr, "FAIL: %s did not deserialize\n", argv[1]); - return 1; + return EXIT_FAILURE; } std::vector savedBytes; @@ -54,7 +56,7 @@ int main(int argc, char **argv) if (!pool.generate_binary_object_pool(savedBytes)) { std::fprintf(stderr, "FAIL: %s loaded but cannot be saved again\n", argv[1]); - return 1; + return EXIT_FAILURE; } if (savedBytes != fileBytes) @@ -64,7 +66,7 @@ int main(int argc, char **argv) argv[1], fileBytes.size(), savedBytes.size()); - return 1; + return EXIT_FAILURE; } std::string exportedXml; @@ -79,7 +81,7 @@ int main(int argc, char **argv) !importedPool.generate_task_data_iso_xml(reexportedXml)) { std::fprintf(stderr, "FAIL: %s did not survive an ISOXML export and import\n", argv[1]); - return 1; + return EXIT_FAILURE; } if (exportedXml != reexportedXml) @@ -91,9 +93,9 @@ int main(int argc, char **argv) offset, exportedXml.c_str() + offset, reexportedXml.c_str() + offset); - return 1; + return EXIT_FAILURE; } std::printf("PASS: %u objects, %zu bytes, byte-identical round trip, identical ISOXML round trip\n", pool.size(), savedBytes.size()); - return 0; + return EXIT_SUCCESS; } From 18c8a1800176e2342862d232acc5b7c154887c04 Mon Sep 17 00:00:00 2001 From: Sujan Dumaru Date: Wed, 30 Sep 2026 13:16:52 -0500 Subject: [PATCH 4/6] Move the example DDOPs into examples/ and add an example TASKDATA.XML --- README.md | 2 +- EXAMPLE.ddop => examples/EXAMPLE.ddop | Bin EXAMPLE_V4.ddop => examples/EXAMPLE_V4.ddop | Bin examples/TASKDATA.XML | 98 +++++ test/CMakeLists.txt | 10 +- test/task_data_import_test.cpp | 419 +++++++++++++++++--- 6 files changed, 477 insertions(+), 52 deletions(-) rename EXAMPLE.ddop => examples/EXAMPLE.ddop (100%) rename EXAMPLE_V4.ddop => examples/EXAMPLE_V4.ddop (100%) create mode 100644 examples/TASKDATA.XML diff --git a/README.md b/README.md index e5d722b..2cb2e63 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ When used in combination with AgIsoStack (or any other TC client), it provides a ### Using it -**File → New** starts an empty DDOP and asks for your device information; **File → Open** loads an existing one. `EXAMPLE.ddop` and `EXAMPLE_V4.ddop` in this repository are there to open and poke at. +**File → New** starts an empty DDOP and asks for your device information; **File → Open** loads an existing one. `EXAMPLE.ddop`, `EXAMPLE_V4.ddop` and `TASKDATA.XML` in [examples/](examples/) are there to open and poke at; the XML goes in through **File → Import ISOXML**. **File → Import ISOXML** reads the device description in an ISOXML TASKDATA.XML, and asks you to pick a device if the file holds several. **File → Save** then asks for a new file name, as it writes a binary DDOP. diff --git a/EXAMPLE.ddop b/examples/EXAMPLE.ddop similarity index 100% rename from EXAMPLE.ddop rename to examples/EXAMPLE.ddop diff --git a/EXAMPLE_V4.ddop b/examples/EXAMPLE_V4.ddop similarity index 100% rename from EXAMPLE_V4.ddop rename to examples/EXAMPLE_V4.ddop diff --git a/examples/TASKDATA.XML b/examples/TASKDATA.XML new file mode 100644 index 0000000..bcab9c1 --- /dev/null +++ b/examples/TASKDATA.XML @@ -0,0 +1,98 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 66f8ca5..2c725ff 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -10,9 +10,13 @@ foreach(test_target example_pool_test task_data_import_test) endforeach() add_test(NAME example_v3_pool_round_trip - COMMAND example_pool_test "${PROJECT_SOURCE_DIR}/EXAMPLE.ddop" 3) + COMMAND example_pool_test + "${PROJECT_SOURCE_DIR}/examples/EXAMPLE.ddop" 3) add_test(NAME example_v4_pool_round_trip - COMMAND example_pool_test "${PROJECT_SOURCE_DIR}/EXAMPLE_V4.ddop" 4) + COMMAND example_pool_test + "${PROJECT_SOURCE_DIR}/examples/EXAMPLE_V4.ddop" 4) -add_test(NAME task_data_import COMMAND task_data_import_test) +add_test(NAME task_data_import + COMMAND task_data_import_test + "${PROJECT_SOURCE_DIR}/examples/TASKDATA.XML") diff --git a/test/task_data_import_test.cpp b/test/task_data_import_test.cpp index db142b1..af56551 100644 --- a/test/task_data_import_test.cpp +++ b/test/task_data_import_test.cpp @@ -1,7 +1,7 @@ //================================================================================================ /// @file task_data_import_test.cpp /// -/// @brief Checks that a hand-written TASKDATA.XML device description imports into a DDOP +/// @brief Checks that the devices in a TASKDATA.XML import into DDOPs that survive an ISOXML and a binary round trip /// @author Sujan Dumaru /// /// @copyright 2026 The Open-Agriculture developers @@ -9,45 +9,43 @@ #include "task_data_import.hpp" #include "isobus/isobus/can_constants.hpp" #include "isobus/isobus/isobus_device_descriptor_object_pool.hpp" +#include "tinyxml2.h" +#include +#include #include #include #include +#include +#include +#include #include #include #include #include -static const std::string TASK_DATA = R"( - - - - - - - - - - - - - - - - - - - -)"; +using isobus::task_controller_object::DeviceElementObject; +using isobus::task_controller_object::DeviceObject; +using isobus::task_controller_object::DeviceProcessDataObject; +using isobus::task_controller_object::DevicePropertyObject; +using isobus::task_controller_object::DeviceValuePresentationObject; + +static const std::string A_UMLAUT = "\xC3\xA4"; +static const std::uint16_t NO_PRESENTATION = isobus::NULL_OBJECT_ID; static int failureCount = 0; -static void check(bool condition, const char *description) +static void fail(const std::string &description) +{ + std::fprintf(stderr, "FAIL: %s\n", description.c_str()); + failureCount++; +} + +static void check(bool condition, const std::string &description) { if (!condition) { - std::fprintf(stderr, "FAIL: %s\n", description); - failureCount++; + fail(description); } } @@ -57,32 +55,180 @@ static std::shared_ptr get_object(isobus::DeviceDescriptorObjectPool &pool, s return std::dynamic_pointer_cast(pool.get_object_by_id(objectID)); } -int main() +static bool import_device(const std::string &taskData, std::size_t deviceIndex, isobus::DeviceDescriptorObjectPool &pool) +{ + const bool imported = import_task_data_device(taskData, deviceIndex, pool); + + check(imported, "DVC number " + std::to_string(deviceIndex + 1) + " imports"); + return imported; +} + +static void check_device(isobus::DeviceDescriptorObjectPool &pool, + const std::string &label, + const std::string &designator, + const std::string &softwareVersion, + const std::string &serialNumber, + std::uint64_t name, + const std::string &structureLabel, + const std::array &localizationLabel) +{ + const auto device = get_object(pool, 0); + + if (nullptr == device) + { + fail(label + " has no device object"); + return; + } + check(designator == device->get_designator(), label + " designator"); + check(softwareVersion == device->get_software_version(), label + " software version"); + check(serialNumber == device->get_serial_number(), label + " serial number"); + check(name == device->get_iso_name(), label + " NAME"); + check(structureLabel == device->get_structure_label(), label + " structure label"); + check(localizationLabel == device->get_localization_label(), label + " localization label"); +} + +static void check_element(isobus::DeviceDescriptorObjectPool &pool, + std::uint16_t objectID, + DeviceElementObject::Type type, + std::uint16_t elementNumber, + std::uint16_t parentID, + const std::string &designator, + const std::vector &children) +{ + const std::string label = "DET " + std::to_string(objectID); + const auto element = get_object(pool, objectID); + + if (nullptr == element) + { + fail(label + " is missing or not a DET"); + return; + } + + std::vector importedChildren; + for (std::uint16_t i = 0; i < element->get_number_child_objects(); i++) + { + importedChildren.push_back(element->get_child_object_id(i)); + } + + check(type == element->get_type(), label + " type"); + check(elementNumber == element->get_element_number(), label + " element number"); + check(parentID == element->get_parent_object(), label + " parent"); + check(designator == element->get_designator(), label + " designator"); + check(children == importedChildren, label + " keeps its DORs in file order"); +} + +static void check_process_data(isobus::DeviceDescriptorObjectPool &pool, + std::uint16_t objectID, + std::uint16_t ddi, + std::uint8_t properties, + std::uint8_t triggers, + const std::string &designator, + std::uint16_t presentationID) { - const std::vector labels = list_task_data_devices(TASK_DATA); - check((2 == labels.size()) && ("DVC-1 Tractor ECU" == labels.at(0)) && ("DVC-2 Sprayer & Boom" == labels.at(1)), - "list_task_data_devices returns both DVCs as \"A B\""); + const std::string label = "DPD " + std::to_string(objectID); + const auto processData = get_object(pool, objectID); + if (nullptr == processData) + { + fail(label + " is missing or not a DPD"); + return; + } + check(ddi == processData->get_ddi(), label + " DDI"); + check(properties == processData->get_properties_bitfield(), label + " properties"); + check(triggers == processData->get_trigger_methods_bitfield(), label + " trigger methods"); + check(designator == processData->get_designator(), label + " designator"); + check(presentationID == processData->get_device_value_presentation_object_id(), label + " DVP reference"); +} + +static void check_property(isobus::DeviceDescriptorObjectPool &pool, + std::uint16_t objectID, + std::uint16_t ddi, + std::int32_t value, + const std::string &designator, + std::uint16_t presentationID) +{ + const std::string label = "DPT " + std::to_string(objectID); + const auto property = get_object(pool, objectID); + + if (nullptr == property) + { + fail(label + " is missing or not a DPT"); + return; + } + check(ddi == property->get_ddi(), label + " DDI"); + check(value == property->get_value(), label + " value"); + check(designator == property->get_designator(), label + " designator"); + check(presentationID == property->get_device_value_presentation_object_id(), label + " DVP reference"); +} + +static void check_presentation(isobus::DeviceDescriptorObjectPool &pool, + std::uint16_t objectID, + std::int32_t offset, + float scale, + std::uint8_t decimals, + const std::string &unit) +{ + const std::string label = "DVP " + std::to_string(objectID); + const auto presentation = get_object(pool, objectID); + + if (nullptr == presentation) + { + fail(label + " is missing or not a DVP"); + return; + } + check(offset == presentation->get_offset(), label + " offset"); + check(std::fabs(presentation->get_scale() - scale) <= (std::fabs(scale) * 1e-5f), label + " scale"); + check(decimals == presentation->get_number_of_decimals(), label + " decimals"); + check(unit == presentation->get_designator(), label + " unit"); +} + +static void check_device_list(const std::string &taskData) +{ + const std::vector labels = list_task_data_devices(taskData); + + check(4 == labels.size(), "list_task_data_devices finds 4 DVCs past the skipped CTR, TSK, PFD and VPN elements"); + if (4 == labels.size()) + { + check("DVC-1 Tractor ECU" == labels.at(0), "the first DVC is listed as \"A B\""); + check("DVC-2 Sprayer & Boom" == labels.at(1), "the entity in the second DVC designator is decoded in the list"); + check("DVC-3 Sprayer Plus" == labels.at(2), "the third DVC is listed after a skipped PFD"); + check(R"(DVC-4 Quote "Q" & 'a')" == labels.at(3), "the fourth DVC is listed after a skipped VPN"); + } +} + +static void check_first_device(const std::string &taskData) +{ isobus::DeviceDescriptorObjectPool pool(4); - if (!import_task_data_device(TASK_DATA, 1, pool)) + + if (import_device(taskData, 0, pool)) { - std::fprintf(stderr, "FAIL: the second DVC did not import\n"); - return 1; + check(1 == pool.size(), "DVC-1 holds only its device"); + check_device(pool, "DVC-1", "Tractor ECU", "1.0", "1234", 0xA00084000C200002, std::string("\x01\0\0\0\0\0\0", 7), { 0x64, 0x65, 0, 0, 0, 0, 0xFF }); } +} + +static void check_second_device(const std::string &taskData) +{ + isobus::DeviceDescriptorObjectPool pool(4); - const auto device = get_object(pool, 0); - const auto rootElement = get_object(pool, 5001); - const auto section = get_object(pool, 5003); - const auto workState = get_object(pool, 5006); - const auto offsetX = get_object(pool, 5005); - const auto hours = get_object(pool, 6001); - const auto hectares = get_object(pool, 6002); + if (!import_device(taskData, 1, pool)) + { + return; + } + + const auto device = get_object(pool, 0); + const auto rootElement = get_object(pool, 5001); + const auto section = get_object(pool, 5003); + const auto workState = get_object(pool, 5006); + const auto offsetX = get_object(pool, 5005); + const auto hours = get_object(pool, 6001); + const auto hectares = get_object(pool, 6002); if ((nullptr == device) || (nullptr == rootElement) || (nullptr == section) || (nullptr == workState) || (nullptr == offsetX) || (nullptr == hours) || (nullptr == hectares)) { - std::fprintf(stderr, "FAIL: an imported object is missing or has the wrong type\n"); - return 1; + fail("DVC-2: an imported object is missing or has the wrong type"); + return; } check(9 == pool.size(), "the pool holds the device and its 8 objects"); @@ -94,9 +240,9 @@ int main() (0xFF == device->get_localization_label().at(6)), "G is read in reversed byte order"); - check(isobus::task_controller_object::DeviceElementObject::Type::Device == rootElement->get_type(), "the root DET is a Device element"); + check(DeviceElementObject::Type::Device == rootElement->get_type(), "the root DET is a Device element"); check(5001 == section->get_parent_object(), "the section DET has the root DET as its parent"); - check(isobus::task_controller_object::DeviceElementObject::Type::Section == section->get_type(), "the section DET has type 4"); + check(DeviceElementObject::Type::Section == section->get_type(), "the section DET has type 4"); check(section->get_designator().empty(), "a DET without D has an empty designator"); check((2 == section->get_number_child_objects()) && (5005 == section->get_child_object_id(0)) && (5006 == section->get_child_object_id(1)), @@ -112,16 +258,94 @@ int main() std::vector binaryPool; check(pool.generate_binary_object_pool(binaryPool), "the imported pool serializes"); +} +static void check_third_device(const std::string &taskData) +{ + isobus::DeviceDescriptorObjectPool pool(4); + + if (!import_device(taskData, 2, pool)) + { + return; + } + + std::string sixteenUmlauts; + for (std::size_t i = 0; i < 16; i++) + { + sixteenUmlauts += A_UMLAUT; + } + + check(23 == pool.size(), "DVC-3 holds the device and its 22 objects, all listed out of order"); + check_device(pool, "DVC-3", "Sprayer Plus", "3.1", "SN-0003", 0xA00086000CE05678, "GFEDCBA", { 0x72, 0x73, 0, 0, 0, 0, 0xFF }); + + check_element(pool, 7001, DeviceElementObject::Type::Device, 0, 0, "Sprayer Device", { 7104, 7101, 7114, 7102 }); + check_element(pool, 7002, DeviceElementObject::Type::Function, 1, 7001, "Function", {}); + check_element(pool, 7003, DeviceElementObject::Type::Bin, 2, 7002, "Tank " + A_UMLAUT, { 7113, 7111 }); + check_element(pool, 7004, DeviceElementObject::Type::Section, 3, 7002, "Sec" + A_UMLAUT + "tion", { 7101, 7106 }); + check_element(pool, 7005, DeviceElementObject::Type::Unit, 4, 7004, "Unit Hi", { 7103, 7105 }); + check_element(pool, 7006, DeviceElementObject::Type::Connector, 5, 7001, sixteenUmlauts, { 7112 }); + check_element(pool, 7007, DeviceElementObject::Type::NavigationReference, 6, 7001, "Nav", { 7115 }); + check(32 == sixteenUmlauts.size(), "the connector designator is 32 bytes"); + + check_process_data(pool, 7101, 0x0074, 1, 8, "Total Area", 7201); + check_process_data(pool, 7102, 0x0077, 3, 9, "Total Time", 7202); + check_process_data(pool, 7103, 0x008D, 0, 0, "", NO_PRESENTATION); + check_process_data(pool, 7104, 0x8001, 5, 31, "Proprietary Rate", NO_PRESENTATION); + check_process_data(pool, 7105, 0x00AF, 1, 2, "Distance Driven", 7203); + check_process_data(pool, 7106, 0x9ABC, 2, 4, "ABCDEFGHIJKLMNOPQRSTUVWXYZ012345", NO_PRESENTATION); + const auto thirtyTwoBytes = get_object(pool, 7106); + check((nullptr != thirtyTwoBytes) && (32 == thirtyTwoBytes->get_designator().size()), "the DPD designator is 32 bytes"); + + check_property(pool, 7111, 0x0086, 0, "Zero Offset", NO_PRESENTATION); + check_property(pool, 7112, 0x0087, -1200, "Negative Offset", 7202); + check_property(pool, 7113, 0x0088, INT32_MAX, "Int32 Max", NO_PRESENTATION); + check_property(pool, 7114, 0x0086, INT32_MIN, "Int32 Min", NO_PRESENTATION); + check_property(pool, 7115, 0x0089, 65536, "Large With DVP", 7204); + + check_presentation(pool, 7201, 0, 1.0f, 0, "m"); + check_presentation(pool, 7202, -100, 0.001f, 3, "mm"); + check_presentation(pool, 7203, 2000000000, 1e-7f, 7, ""); + check_presentation(pool, 7204, INT32_MIN, 100.0f, 0, "%"); +} + +static void check_fourth_device(const std::string &taskData) +{ + isobus::DeviceDescriptorObjectPool pool(4); + + if (!import_device(taskData, 3, pool)) + { + return; + } + + check(6 == pool.size(), "DVC-4 holds the device and its 5 objects"); + check_device(pool, "DVC-4", R"(Quote "Q" & 'a')", "<4>", "&", 0xA00086000CE09ABC, "GFEDCBA", { 0x72, 0x73, 0, 0, 0, 0, 0xFF }); + check_element(pool, 8001, DeviceElementObject::Type::Device, 0, 0, "Root <1>", {}); + check_element(pool, 8002, DeviceElementObject::Type::Section, 1, 8001, "a & b", { 8101, 8102 }); + check_process_data(pool, 8101, 0x0074, 1, 8, R"(Rate "x")", 8201); + check_property(pool, 8102, 0x0086, 7, "It's " + A_UMLAUT, 8201); + check_presentation(pool, 8201, 0, 1.0f, 0, "<&>"); +} + +static void check_rejects(const std::string &taskData) +{ const std::vector> rejects = { { R"(B="5003" C="4")", R"(B="5003" C="9")" }, { R"(B="5003" C="4")", R"(B="5003" C="-1")" }, { R"(B="5003" C="4")", R"(B="5003" C="4x")" }, { R"(F="31323334353637")", R"(F="3132333435363738")" }, { R"( " + reject.second + " is rejected"; + + if (std::string::npos == position) + { + fail(reject.first + " is not in the file, so its reject case edits nothing"); + continue; + } + + std::string badTaskData = taskData; + badTaskData.replace(position, reject.first.size(), reject.second); isobus::DeviceDescriptorObjectPool badPool(4); - check(!import_task_data_device(badTaskData, 1, badPool), (reject.first + " -> " + reject.second + " is rejected").c_str()); + check(!import_task_data_device(badTaskData, 1, badPool), description); } for (const std::string ¬TaskData : { std::string(), std::string("DVC\x01\x02"), std::string("") }) @@ -130,10 +354,109 @@ int main() check(list_task_data_devices(notTaskData).empty(), "input that is not a TASKDATA.XML lists no devices"); check(!import_task_data_device(notTaskData, 0, emptyPool), "input that is not a TASKDATA.XML does not import"); } +} + +static void check_binary_round_trip(isobus::DeviceDescriptorObjectPool &pool, const std::string &label) +{ + std::vector binaryPool; + std::vector loadedBytes; + std::vector savedBytes; + isobus::DeviceDescriptorObjectPool loadedPool(pool.get_task_controller_compatibility_level()); + + if (!pool.generate_binary_object_pool(binaryPool)) + { + fail(label + ": the imported pool does not serialize"); + return; + } + + loadedBytes = binaryPool; + if (!loadedPool.deserialize_binary_object_pool(loadedBytes, isobus::NAME(0)) || !loadedPool.generate_binary_object_pool(savedBytes)) + { + fail(label + ": the imported pool's binary does not deserialize and serialize again"); + return; + } + + if (savedBytes != binaryPool) + { + const auto offset = static_cast(std::mismatch(binaryPool.begin(), binaryPool.end(), savedBytes.begin(), savedBytes.end()).first - binaryPool.begin()); + fail(label + ": saving the deserialized binary differs at byte " + std::to_string(offset) + " (" + std::to_string(binaryPool.size()) + " in, " + std::to_string(savedBytes.size()) + " out)"); + } +} + +static void check_xml_round_trip(isobus::DeviceDescriptorObjectPool &pool, const std::string &label) +{ + std::string exportedXml; + std::string reexportedXml; + isobus::DeviceDescriptorObjectPool importedPool(pool.get_task_controller_compatibility_level()); + + if (!pool.generate_task_data_iso_xml(exportedXml)) + { + fail(label + ": the imported pool does not export to ISOXML"); + return; + } + + if (!import_task_data_device(exportedXml, 0, importedPool)) + { + tinyxml2::XMLDocument document; + const bool parses = tinyxml2::XML_SUCCESS == document.Parse(exportedXml.data(), exportedXml.size()); + fail(label + ": the exported ISOXML does not import again (" + (parses ? std::string("it parses but an attribute is invalid") : std::string("it is not valid XML: ") + document.ErrorStr()) + ")"); + return; + } + + if (!importedPool.generate_task_data_iso_xml(reexportedXml)) + { + fail(label + ": the re-imported pool does not export to ISOXML"); + return; + } + + if (exportedXml != reexportedXml) + { + const auto offset = static_cast(std::mismatch(exportedXml.begin(), exportedXml.end(), reexportedXml.begin(), reexportedXml.end()).first - exportedXml.begin()); + fail(label + ": re-exporting the imported ISOXML differs at byte " + std::to_string(offset) + ":\n" + exportedXml.substr(offset, 60) + "\n" + reexportedXml.substr(offset, 60)); + } +} + +int main(int argc, char **argv) +{ + if (2 != argc) + { + std::fprintf(stderr, "usage: %s \n", argv[0]); + return EXIT_FAILURE; + } + + std::ifstream inputFile(argv[1], std::ios::binary); + + if (!inputFile) + { + std::fprintf(stderr, "FAIL: cannot open %s\n", argv[1]); + return EXIT_FAILURE; + } + + const std::string taskData((std::istreambuf_iterator(inputFile)), std::istreambuf_iterator()); + + check_device_list(taskData); + check_first_device(taskData); + check_second_device(taskData); + check_third_device(taskData); + check_fourth_device(taskData); + check_rejects(taskData); + + const std::size_t deviceCount = list_task_data_devices(taskData).size(); + for (std::size_t i = 0; i < deviceCount; i++) + { + const std::string label = "DVC number " + std::to_string(i + 1) + " round trip"; + isobus::DeviceDescriptorObjectPool pool(4); + + if (import_device(taskData, i, pool)) + { + check_binary_round_trip(pool, label); + check_xml_round_trip(pool, label); + } + } if (0 == failureCount) { - std::printf("PASS: TASKDATA.XML device import\n"); + std::printf("PASS: TASKDATA.XML device import, %zu devices round tripped\n", deviceCount); } - return (0 == failureCount) ? 0 : 1; + return (0 == failureCount) ? EXIT_SUCCESS : EXIT_FAILURE; } From 7f5956c3760032f6cd44e59022979873d2cef219 Mon Sep 17 00:00:00 2001 From: Sujan Dumaru Date: Wed, 30 Sep 2026 21:26:07 -0500 Subject: [PATCH 5/6] Keep the open pool when an ISOXML import fails or is cancelled, and range-check imported values --- examples/TASKDATA.XML | 4 +- src/gui.cpp | 73 ++++++++++++++++++---------------- src/task_data_import.cpp | 18 ++++++--- test/task_data_import_test.cpp | 11 ++++- 4 files changed, 64 insertions(+), 42 deletions(-) diff --git a/examples/TASKDATA.XML b/examples/TASKDATA.XML index bcab9c1..0e187de 100644 --- a/examples/TASKDATA.XML +++ b/examples/TASKDATA.XML @@ -34,8 +34,8 @@ A="DVC-3"> - - + + diff --git a/src/gui.cpp b/src/gui.cpp index 2cf4b9d..8f31ff6 100644 --- a/src/gui.cpp +++ b/src/gui.cpp @@ -582,6 +582,13 @@ static void render_log_error_popup(const char *title, const char *heading) } } +static std::unique_ptr make_pool() +{ + auto pool = std::make_unique(); + pool->set_task_controller_compatibility_level((0 == FileDialog::versions_current_idx) ? 3 : 4); + return pool; +} + void DDOPGeneratorGUI::render_open_file_menu() { bool importSelectedDevice = false; @@ -602,44 +609,42 @@ void DDOPGeneratorGUI::render_open_file_menu() { loadedIopData = isobus::IOPFileInterface::read_iop_file(selectedFileToRead); - if (!loadedIopData.empty()) + if (loadedIopData.empty()) + { + if (importingTaskData) + { + logger.logHistory.clear(); + LOG_ERROR("[DDOP]: Could not read \"%s\", or it is empty.", selectedFileToRead.c_str()); + ImGui::OpenPopup("Error Loading XML File"); + } + } + else if (importingTaskData) { - selectedObjectID = 0xFFFF; logger.logHistory.clear(); - currentObjectPool.reset(); - currentObjectPool = std::make_unique(); + pendingTaskData.assign(loadedIopData.begin(), loadedIopData.end()); + taskDataDevices = list_task_data_devices(pendingTaskData); + selectedTaskDataDevice = 0; - if (0 == FileDialog::versions_current_idx) + if (taskDataDevices.empty()) { - currentObjectPool->set_task_controller_compatibility_level(3); + ImGui::OpenPopup("Error Loading XML File"); } - else + else if (1 == taskDataDevices.size()) { - currentObjectPool->set_task_controller_compatibility_level(4); + importSelectedDevice = true; } - - if (importingTaskData) + else { - currentPoolValid = false; - pendingTaskData.assign(loadedIopData.begin(), loadedIopData.end()); - taskDataDevices = list_task_data_devices(pendingTaskData); - selectedTaskDataDevice = 0; - - if (taskDataDevices.empty()) - { - currentObjectPool.reset(); - ImGui::OpenPopup("Error Loading XML File"); - } - else if (1 == taskDataDevices.size()) - { - importSelectedDevice = true; - } - else - { - ImGui::OpenPopup("Select Device"); - } + ImGui::OpenPopup("Select Device"); } - else if (true == currentObjectPool->deserialize_binary_object_pool(loadedIopData, isobus::NAME(0))) + } + else + { + selectedObjectID = 0xFFFF; + logger.logHistory.clear(); + currentObjectPool = make_pool(); + + if (true == currentObjectPool->deserialize_binary_object_pool(loadedIopData, isobus::NAME(0))) { // Valid pool? currentPoolValid = true; @@ -680,8 +685,6 @@ void DDOPGeneratorGUI::render_open_file_menu() ImGui::SameLine(); if (ImGui::Button("Cancel", ImVec2(120, 0))) { - currentObjectPool.reset(); - currentPoolValid = false; ImGui::CloseCurrentPopup(); } ImGui::EndPopup(); @@ -690,15 +693,17 @@ void DDOPGeneratorGUI::render_open_file_menu() // Outside the Select Device modal, so the error popup opens with the ID it is rendered under below. if (importSelectedDevice) { - if (import_task_data_device(pendingTaskData, static_cast(selectedTaskDataDevice), *currentObjectPool)) + auto importedPool = make_pool(); + + if (import_task_data_device(pendingTaskData, static_cast(selectedTaskDataDevice), *importedPool)) { + currentObjectPool = std::move(importedPool); + selectedObjectID = 0xFFFF; currentPoolValid = true; lastFileName.clear(); } else { - currentObjectPool.reset(); - currentPoolValid = false; ImGui::OpenPopup("Error Loading XML File"); } } diff --git a/src/task_data_import.cpp b/src/task_data_import.cpp index 71a115d..7f8989b 100644 --- a/src/task_data_import.cpp +++ b/src/task_data_import.cpp @@ -23,10 +23,16 @@ #include #include +static constexpr std::int64_t MAX_ELEMENT_NUMBER = 4095; +static constexpr std::int64_t MAX_TRIGGER_METHODS = 31; +static constexpr std::int64_t MAX_DECIMALS = 7; +static constexpr std::size_t MAX_SOFTWARE_VERSION_LENGTH = std::numeric_limits::max(); // The serialized length is one byte + static bool log_invalid_attribute(const tinyxml2::XMLElement &element, const char *attribute) { const char *id = element.Attribute("A"); - LOG_ERROR("[DDOP]: TASKDATA %s \"%s\" has a missing or invalid %s attribute.", element.Name(), (nullptr != id) ? id : "", attribute); + const char *value = element.Attribute(attribute); + LOG_ERROR("[DDOP]: TASKDATA %s \"%s\" has a missing or invalid %s attribute (value \"%.40s\").", element.Name(), (nullptr != id) ? id : "", attribute, (nullptr != value) ? value : ""); return false; } @@ -112,7 +118,7 @@ static bool import_object(const tinyxml2::XMLElement &element, isobus::DeviceDes success = read_integer(element, "B", objectID) && read_integer(element, "C", type, 1, 7) && - read_integer(element, "E", elementNumber) && + read_integer(element, "E", elementNumber, 0, MAX_ELEMENT_NUMBER) && read_integer(element, "F", parentID) && pool.add_device_element(read_text(element, "D"), elementNumber, parentID, static_cast(type), objectID); @@ -135,7 +141,7 @@ static bool import_object(const tinyxml2::XMLElement &element, isobus::DeviceDes success = read_integer(element, "A", objectID) && read_hex(element, "B", 4, ddi) && read_integer(element, "C", properties) && - read_integer(element, "D", triggers) && + read_integer(element, "D", triggers, 0, MAX_TRIGGER_METHODS) && ((nullptr == element.Attribute("F")) || read_integer(element, "F", presentationID)) && pool.add_device_process_data(read_text(element, "E"), static_cast(ddi), presentationID, properties, triggers, objectID); } @@ -157,8 +163,9 @@ static bool import_object(const tinyxml2::XMLElement &element, isobus::DeviceDes success = read_integer(element, "A", objectID) && read_integer(element, "B", offset) && - (parse_number(element.Attribute("C"), scale) || log_invalid_attribute(element, "C")) && - read_integer(element, "D", decimals) && + // Zero is accepted because the exporter writes a scale below 1e-6 as 0.000000 and must be able to read its own output. + ((parse_number(element.Attribute("C"), scale) && (0.0f <= scale)) || log_invalid_attribute(element, "C")) && + read_integer(element, "D", decimals, 0, MAX_DECIMALS) && pool.add_device_value_presentation(read_text(element, "E"), offset, scale, decimals, objectID); } return success; @@ -219,6 +226,7 @@ bool import_task_data_device(const std::string &taskData, std::size_t deviceInde std::uint64_t structureLabel = 0; std::uint64_t localizationLabel = 0; bool success = (nullptr != device) && + ((read_text(*device, "C").size() <= MAX_SOFTWARE_VERSION_LENGTH) || log_invalid_attribute(*device, "C")) && read_hex(*device, "D", 16, isoName) && read_hex(*device, "F", 14, structureLabel) && read_hex(*device, "G", 14, localizationLabel); diff --git a/test/task_data_import_test.cpp b/test/task_data_import_test.cpp index af56551..66102e4 100644 --- a/test/task_data_import_test.cpp +++ b/test/task_data_import_test.cpp @@ -329,7 +329,16 @@ static void check_fourth_device(const std::string &taskData) static void check_rejects(const std::string &taskData) { const std::vector> rejects = { - { R"(B="5003" C="4")", R"(B="5003" C="9")" }, { R"(B="5003" C="4")", R"(B="5003" C="-1")" }, { R"(B="5003" C="4")", R"(B="5003" C="4x")" }, { R"(F="31323334353637")", R"(F="3132333435363738")" }, { R"( Date: Wed, 30 Sep 2026 22:03:08 -0500 Subject: [PATCH 6/6] Reject imported text the TC version cannot hold instead of truncating it, trim comments, and keep the open pool when a .ddop fails to load --- examples/TASKDATA.XML | 21 ++---- src/gui.cpp | 19 ++---- src/task_data_import.cpp | 61 +++++++++++------ test/task_data_import_test.cpp | 121 +++++++++++++++------------------ 4 files changed, 107 insertions(+), 115 deletions(-) diff --git a/examples/TASKDATA.XML b/examples/TASKDATA.XML index 0e187de..0fc7667 100644 --- a/examples/TASKDATA.XML +++ b/examples/TASKDATA.XML @@ -1,11 +1,9 @@  - + - - @@ -23,7 +21,7 @@ - + - - - - @@ -49,18 +43,16 @@ - - - + @@ -69,11 +61,10 @@ - - + - + @@ -84,7 +75,7 @@ - + diff --git a/src/gui.cpp b/src/gui.cpp index 8f31ff6..1db943c 100644 --- a/src/gui.cpp +++ b/src/gui.cpp @@ -559,8 +559,6 @@ bool DDOPGeneratorGUI::render_menu_bar() } /// @brief Renders a modal listing the logged errors of the last load attempt -/// @param[in] title The popup title, which is also the ID to open it with -/// @param[in] heading The text shown above the log static void render_log_error_popup(const char *title, const char *heading) { if (ImGui::BeginPopupModal(title, NULL, ImGuiWindowFlags_AlwaysAutoResize)) @@ -607,20 +605,19 @@ void DDOPGeneratorGUI::render_open_file_menu() if (!selectedFileToRead.empty()) { + logger.logHistory.clear(); loadedIopData = isobus::IOPFileInterface::read_iop_file(selectedFileToRead); if (loadedIopData.empty()) { if (importingTaskData) { - logger.logHistory.clear(); LOG_ERROR("[DDOP]: Could not read \"%s\", or it is empty.", selectedFileToRead.c_str()); ImGui::OpenPopup("Error Loading XML File"); } } else if (importingTaskData) { - logger.logHistory.clear(); pendingTaskData.assign(loadedIopData.begin(), loadedIopData.end()); taskDataDevices = list_task_data_devices(pendingTaskData); selectedTaskDataDevice = 0; @@ -640,21 +637,17 @@ void DDOPGeneratorGUI::render_open_file_menu() } else { - selectedObjectID = 0xFFFF; - logger.logHistory.clear(); - currentObjectPool = make_pool(); + auto openedPool = make_pool(); - if (true == currentObjectPool->deserialize_binary_object_pool(loadedIopData, isobus::NAME(0))) + if (true == openedPool->deserialize_binary_object_pool(loadedIopData, isobus::NAME(0))) { - // Valid pool? + currentObjectPool = std::move(openedPool); + selectedObjectID = 0xFFFF; currentPoolValid = true; lastFileName = selectedFileToRead; } else { - currentObjectPool.reset(); - currentPoolValid = false; - ImGui::OpenPopup("Error Loading DDOP"); } } @@ -690,7 +683,7 @@ void DDOPGeneratorGUI::render_open_file_menu() ImGui::EndPopup(); } - // Outside the Select Device modal, so the error popup opens with the ID it is rendered under below. + // After EndPopup, because a popup opened inside another modal never shows. if (importSelectedDevice) { auto importedPool = make_pool(); diff --git a/src/task_data_import.cpp b/src/task_data_import.cpp index 7f8989b..4e84353 100644 --- a/src/task_data_import.cpp +++ b/src/task_data_import.cpp @@ -23,10 +23,12 @@ #include #include -static constexpr std::int64_t MAX_ELEMENT_NUMBER = 4095; -static constexpr std::int64_t MAX_TRIGGER_METHODS = 31; -static constexpr std::int64_t MAX_DECIMALS = 7; -static constexpr std::size_t MAX_SOFTWARE_VERSION_LENGTH = std::numeric_limits::max(); // The serialized length is one byte +constexpr std::int64_t MAX_ELEMENT_NUMBER = 4095; +constexpr std::int64_t MAX_TRIGGER_METHODS = 31; +constexpr std::int64_t MAX_DECIMALS = 7; +constexpr std::size_t MAX_SOFTWARE_VERSION_LENGTH = std::numeric_limits::max(); // The serialized length is one byte +constexpr std::uint8_t LONG_DESIGNATOR_TC_VERSION = 4; +constexpr std::size_t MAX_DESIGNATOR_CHARACTERS = 32; static bool log_invalid_attribute(const tinyxml2::XMLElement &element, const char *attribute) { @@ -42,6 +44,17 @@ static std::string read_text(const tinyxml2::XMLElement &element, const char *at return (nullptr != text) ? text : ""; } +// The stack silently truncates an over-long designator. From TC version 4 the limit is 32 characters, up to 128 UTF-8 bytes. +static bool read_designator(const tinyxml2::XMLElement &element, const char *attribute, const isobus::DeviceDescriptorObjectPool &pool, std::string &text) +{ + text = read_text(element, attribute); + const bool longDesignators = LONG_DESIGNATOR_TC_VERSION <= pool.get_task_controller_compatibility_level(); + const std::size_t maxBytes = longDesignators ? isobus::task_controller_object::Object::MAX_DESIGNATOR_LENGTH : isobus::task_controller_object::Object::MAX_DESIGNATOR_LEGACY_LENGTH; + const std::size_t characters = static_cast(std::count_if(text.begin(), text.end(), [](char byte) { return 0x80 != (static_cast(byte) & 0xC0); })); + + return ((text.size() <= maxBytes) && (!longDesignators || (characters <= MAX_DESIGNATOR_CHARACTERS))) || log_invalid_attribute(element, attribute); +} + static bool read_hex(const tinyxml2::XMLElement &element, const char *attribute, std::size_t digits, std::uint64_t &value) { const char *text = element.Attribute(attribute); @@ -52,14 +65,10 @@ static bool read_hex(const tinyxml2::XMLElement &element, const char *attribute, { value = std::strtoull(text, nullptr, 16); } - else - { - log_invalid_attribute(element, attribute); - } - return valid; + return valid || log_invalid_attribute(element, attribute); } -// ISOXML writes a label's last byte first, so the first hex pair is byte 6 and the lowest byte of the number is byte 0. +// ISOXML writes a label last byte first, so the number's lowest byte is label byte 0. static std::array to_label(std::uint64_t value) { std::array label = {}; @@ -95,11 +104,7 @@ static bool read_integer(const tinyxml2::XMLElement &element, { value = static_cast(number); } - else - { - log_invalid_attribute(element, attribute); - } - return valid; + return valid || log_invalid_attribute(element, attribute); } static bool import_object(const tinyxml2::XMLElement &element, isobus::DeviceDescriptorObjectPool &pool) @@ -115,12 +120,14 @@ static bool import_object(const tinyxml2::XMLElement &element, isobus::DeviceDes std::uint8_t type = 0; std::uint16_t elementNumber = 0; std::uint16_t parentID = 0; + std::string designator; success = read_integer(element, "B", objectID) && read_integer(element, "C", type, 1, 7) && read_integer(element, "E", elementNumber, 0, MAX_ELEMENT_NUMBER) && read_integer(element, "F", parentID) && - pool.add_device_element(read_text(element, "D"), elementNumber, parentID, static_cast(type), objectID); + read_designator(element, "D", pool, designator) && + pool.add_device_element(designator, elementNumber, parentID, static_cast(type), objectID); for (auto reference = element.FirstChildElement("DOR"); success && (nullptr != reference); reference = reference->NextSiblingElement("DOR")) { @@ -137,36 +144,42 @@ static bool import_object(const tinyxml2::XMLElement &element, isobus::DeviceDes { std::uint8_t properties = 0; std::uint8_t triggers = 0; + std::string designator; success = read_integer(element, "A", objectID) && read_hex(element, "B", 4, ddi) && read_integer(element, "C", properties) && read_integer(element, "D", triggers, 0, MAX_TRIGGER_METHODS) && ((nullptr == element.Attribute("F")) || read_integer(element, "F", presentationID)) && - pool.add_device_process_data(read_text(element, "E"), static_cast(ddi), presentationID, properties, triggers, objectID); + read_designator(element, "E", pool, designator) && + pool.add_device_process_data(designator, static_cast(ddi), presentationID, properties, triggers, objectID); } else if ("DPT" == name) { std::int32_t value = 0; + std::string designator; success = read_integer(element, "A", objectID) && read_hex(element, "B", 4, ddi) && read_integer(element, "C", value) && ((nullptr == element.Attribute("E")) || read_integer(element, "E", presentationID)) && - pool.add_device_property(read_text(element, "D"), value, static_cast(ddi), presentationID, objectID); + read_designator(element, "D", pool, designator) && + pool.add_device_property(designator, value, static_cast(ddi), presentationID, objectID); } else if ("DVP" == name) { std::int32_t offset = 0; float scale = 0.0f; std::uint8_t decimals = 0; + std::string designator; success = read_integer(element, "A", objectID) && read_integer(element, "B", offset) && - // Zero is accepted because the exporter writes a scale below 1e-6 as 0.000000 and must be able to read its own output. + // Zero is allowed: the exporter rounds a scale below 1e-6 to 0.000000. ((parse_number(element.Attribute("C"), scale) && (0.0f <= scale)) || log_invalid_attribute(element, "C")) && read_integer(element, "D", decimals, 0, MAX_DECIMALS) && - pool.add_device_value_presentation(read_text(element, "E"), offset, scale, decimals, objectID); + read_designator(element, "E", pool, designator) && + pool.add_device_value_presentation(designator, offset, scale, decimals, objectID); } return success; } @@ -225,16 +238,20 @@ bool import_task_data_device(const std::string &taskData, std::size_t deviceInde std::uint64_t isoName = 0; std::uint64_t structureLabel = 0; std::uint64_t localizationLabel = 0; + std::string designator; + std::string serialNumber; bool success = (nullptr != device) && ((read_text(*device, "C").size() <= MAX_SOFTWARE_VERSION_LENGTH) || log_invalid_attribute(*device, "C")) && read_hex(*device, "D", 16, isoName) && read_hex(*device, "F", 14, structureLabel) && - read_hex(*device, "G", 14, localizationLabel); + read_hex(*device, "G", 14, localizationLabel) && + read_designator(*device, "B", pool, designator) && + read_designator(*device, "E", pool, serialNumber); if (success) { const std::array structureBytes = to_label(structureLabel); - success = pool.add_device(read_text(*device, "B"), read_text(*device, "C"), read_text(*device, "E"), std::string(structureBytes.begin(), structureBytes.end()), to_label(localizationLabel), {}, isoName); + success = pool.add_device(designator, read_text(*device, "C"), serialNumber, std::string(structureBytes.begin(), structureBytes.end()), to_label(localizationLabel), {}, isoName); } for (auto child = success ? device->FirstChildElement() : nullptr; success && (nullptr != child); child = child->NextSiblingElement()) diff --git a/test/task_data_import_test.cpp b/test/task_data_import_test.cpp index 66102e4..75c7d68 100644 --- a/test/task_data_import_test.cpp +++ b/test/task_data_import_test.cpp @@ -30,8 +30,8 @@ using isobus::task_controller_object::DeviceProcessDataObject; using isobus::task_controller_object::DevicePropertyObject; using isobus::task_controller_object::DeviceValuePresentationObject; -static const std::string A_UMLAUT = "\xC3\xA4"; -static const std::uint16_t NO_PRESENTATION = isobus::NULL_OBJECT_ID; +const std::string A_UMLAUT = "\xC3\xA4"; +const std::uint16_t NO_PRESENTATION = isobus::NULL_OBJECT_ID; static int failureCount = 0; @@ -216,48 +216,27 @@ static void check_second_device(const std::string &taskData) return; } - const auto device = get_object(pool, 0); - const auto rootElement = get_object(pool, 5001); - const auto section = get_object(pool, 5003); - const auto workState = get_object(pool, 5006); - const auto offsetX = get_object(pool, 5005); - const auto hours = get_object(pool, 6001); - const auto hectares = get_object(pool, 6002); - - if ((nullptr == device) || (nullptr == rootElement) || (nullptr == section) || (nullptr == workState) || - (nullptr == offsetX) || (nullptr == hours) || (nullptr == hectares)) - { - fail("DVC-2: an imported object is missing or has the wrong type"); - return; - } + check(9 == pool.size(), "DVC-2 holds the device and its 8 objects"); + check_device(pool, "DVC-2", "Sprayer & Boom", "2.3.4", "", 0xA00086000CE01234, "7654321", { 0x65, 0x6E, 0, 0, 0, 0, 0xFF }); + check_element(pool, 5001, DeviceElementObject::Type::Device, 0, 0, "Sprayer", { 5002, 5004 }); + check_element(pool, 5003, DeviceElementObject::Type::Section, 1, 5001, "", { 5005, 5006 }); + check_process_data(pool, 5002, 0x0074, 1, 8, "Total Area", 6002); + check_process_data(pool, 5004, 0x0077, 1, 8, "Total Time", 6001); + check_process_data(pool, 5006, 0x008D, 1, 31, "Work State", NO_PRESENTATION); + check_property(pool, 5005, 0x0086, -1200, "Offset X", NO_PRESENTATION); + check_presentation(pool, 6001, 0, 0.0000012345f, 1, "h"); + check_presentation(pool, 6002, 0, 0.000099999997474f, 2, "ha "); +} - check(9 == pool.size(), "the pool holds the device and its 8 objects"); - check(0xA00086000CE01234 == device->get_iso_name(), "D is read as a big-endian NAME"); - check("Sprayer & Boom" == device->get_designator(), "the designator is entity-decoded"); - check(device->get_serial_number().empty(), "a DVC without E has an empty serial number"); - check("7654321" == device->get_structure_label(), "F is read in reversed byte order"); - check((0x65 == device->get_localization_label().at(0)) && (0x6E == device->get_localization_label().at(1)) && - (0xFF == device->get_localization_label().at(6)), - "G is read in reversed byte order"); - - check(DeviceElementObject::Type::Device == rootElement->get_type(), "the root DET is a Device element"); - check(5001 == section->get_parent_object(), "the section DET has the root DET as its parent"); - check(DeviceElementObject::Type::Section == section->get_type(), "the section DET has type 4"); - check(section->get_designator().empty(), "a DET without D has an empty designator"); - check((2 == section->get_number_child_objects()) && (5005 == section->get_child_object_id(0)) && - (5006 == section->get_child_object_id(1)), - "the section DET keeps its DOR children in order"); - - check(0x008D == workState->get_ddi(), "a DPD DDI is read as hex"); - check(isobus::NULL_OBJECT_ID == workState->get_device_value_presentation_object_id(), "a DPD without F has no DVP"); - check(-1200 == offsetX->get_value(), "a DPT value can be negative"); - check(isobus::NULL_OBJECT_ID == offsetX->get_device_value_presentation_object_id(), "a DPT without E has no DVP"); - check(std::fabs(hours->get_scale() - 0.0000012345) < 1e-9, "the hours scale keeps its precision"); - check(std::fabs(hectares->get_scale() - 0.000099999997474) < 1e-9, "the hectares scale keeps its precision"); - check("ha " == hectares->get_designator(), "a space-padded unit is kept as is"); +static std::string repeat(const std::string &unit, std::size_t count) +{ + std::string repeated; - std::vector binaryPool; - check(pool.generate_binary_object_pool(binaryPool), "the imported pool serializes"); + for (std::size_t i = 0; i < count; i++) + { + repeated += unit; + } + return repeated; } static void check_third_device(const std::string &taskData) @@ -269,12 +248,6 @@ static void check_third_device(const std::string &taskData) return; } - std::string sixteenUmlauts; - for (std::size_t i = 0; i < 16; i++) - { - sixteenUmlauts += A_UMLAUT; - } - check(23 == pool.size(), "DVC-3 holds the device and its 22 objects, all listed out of order"); check_device(pool, "DVC-3", "Sprayer Plus", "3.1", "SN-0003", 0xA00086000CE05678, "GFEDCBA", { 0x72, 0x73, 0, 0, 0, 0, 0xFF }); @@ -283,9 +256,8 @@ static void check_third_device(const std::string &taskData) check_element(pool, 7003, DeviceElementObject::Type::Bin, 2, 7002, "Tank " + A_UMLAUT, { 7113, 7111 }); check_element(pool, 7004, DeviceElementObject::Type::Section, 3, 7002, "Sec" + A_UMLAUT + "tion", { 7101, 7106 }); check_element(pool, 7005, DeviceElementObject::Type::Unit, 4, 7004, "Unit Hi", { 7103, 7105 }); - check_element(pool, 7006, DeviceElementObject::Type::Connector, 5, 7001, sixteenUmlauts, { 7112 }); + check_element(pool, 7006, DeviceElementObject::Type::Connector, 5, 7001, repeat(A_UMLAUT, 16), { 7112 }); check_element(pool, 7007, DeviceElementObject::Type::NavigationReference, 6, 7001, "Nav", { 7115 }); - check(32 == sixteenUmlauts.size(), "the connector designator is 32 bytes"); check_process_data(pool, 7101, 0x0074, 1, 8, "Total Area", 7201); check_process_data(pool, 7102, 0x0077, 3, 9, "Total Time", 7202); @@ -293,8 +265,6 @@ static void check_third_device(const std::string &taskData) check_process_data(pool, 7104, 0x8001, 5, 31, "Proprietary Rate", NO_PRESENTATION); check_process_data(pool, 7105, 0x00AF, 1, 2, "Distance Driven", 7203); check_process_data(pool, 7106, 0x9ABC, 2, 4, "ABCDEFGHIJKLMNOPQRSTUVWXYZ012345", NO_PRESENTATION); - const auto thirtyTwoBytes = get_object(pool, 7106); - check((nullptr != thirtyTwoBytes) && (32 == thirtyTwoBytes->get_designator().size()), "the DPD designator is 32 bytes"); check_property(pool, 7111, 0x0086, 0, "Zero Offset", NO_PRESENTATION); check_property(pool, 7112, 0x0087, -1200, "Negative Offset", 7202); @@ -326,6 +296,38 @@ static void check_fourth_device(const std::string &taskData) check_presentation(pool, 8201, 0, 1.0f, 0, "<&>"); } +static bool imports_with_replacement(const std::string &taskData, const std::string &original, const std::string &replacement, std::uint8_t tcVersion) +{ + const std::size_t position = taskData.find(original); + + if (std::string::npos == position) + { + fail(original + " is not in the file, so its edit changes nothing"); + return false; + } + + std::string editedTaskData = taskData; + editedTaskData.replace(position, original.size(), replacement); + isobus::DeviceDescriptorObjectPool editedPool(tcVersion); + return import_task_data_device(editedTaskData, 1, editedPool); +} + +static void check_text_limits(const std::string &taskData) +{ + const std::string designator = R"(B="Sprayer & Boom")"; + const auto text = [](const std::string &unit, std::size_t count) { return "B=\"" + repeat(unit, count) + "\""; }; + + check(imports_with_replacement(taskData, designator, text("x", 32), 3), "a 32 byte designator imports at TC version 3"); + check(!imports_with_replacement(taskData, designator, text("x", 33), 3), "a 33 byte designator is rejected at TC version 3"); + check(!imports_with_replacement(taskData, designator, text(A_UMLAUT, 17), 3), "a 34 byte designator of 17 characters is rejected at TC version 3"); + check(imports_with_replacement(taskData, designator, text("x", 32), 4), "a 32 character designator imports at TC version 4"); + check(!imports_with_replacement(taskData, designator, text("x", 33), 4), "a 33 character designator is rejected at TC version 4"); + check(imports_with_replacement(taskData, designator, text(A_UMLAUT, 32), 4), "a 32 character, 64 byte designator imports at TC version 4"); + check(!imports_with_replacement(taskData, designator, text(A_UMLAUT, 33), 4), "a 33 character, 66 byte designator is rejected at TC version 4"); + check(imports_with_replacement(taskData, R"(C="0.0000012345")", R"(C="0")", 4), "a scale of 0 imports, because the exporter writes a tiny scale that way"); + check(imports_with_replacement(taskData, R"(C="2.3.4")", "C=\"" + repeat("x", 255) + "\"", 4), "a 255 byte software version imports"); +} + static void check_rejects(const std::string &taskData) { const std::vector> rejects = { @@ -342,19 +344,7 @@ static void check_rejects(const std::string &taskData) }; for (const auto &reject : rejects) { - const std::size_t position = taskData.find(reject.first); - const std::string description = reject.first + " -> " + reject.second + " is rejected"; - - if (std::string::npos == position) - { - fail(reject.first + " is not in the file, so its reject case edits nothing"); - continue; - } - - std::string badTaskData = taskData; - badTaskData.replace(position, reject.first.size(), reject.second); - isobus::DeviceDescriptorObjectPool badPool(4); - check(!import_task_data_device(badTaskData, 1, badPool), description); + check(!imports_with_replacement(taskData, reject.first, reject.second, 4), reject.first + " -> " + reject.second + " is rejected"); } for (const std::string ¬TaskData : { std::string(), std::string("DVC\x01\x02"), std::string("") }) @@ -449,6 +439,7 @@ int main(int argc, char **argv) check_third_device(taskData); check_fourth_device(taskData); check_rejects(taskData); + check_text_limits(taskData); const std::size_t deviceCount = list_task_data_devices(taskData).size(); for (std::size_t i = 0; i < deviceCount; i++)