diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 54b4250..2b75c8c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -22,7 +22,7 @@ jobs: - name: Compile run: | mkdir build - cmake -S . -B build -DBUILD_EXAMPLES=OFF -DBUILD_TESTING=OFF -DAOG_TC_VALIDATE_IOP=OFF -Wno-dev + cmake -S . -B build -DBUILD_EXAMPLES=OFF -DBUILD_TESTING=OFF -DAOG_TC_VALIDATE_IOP=OFF -DAOG_TC_BUILD_TESTS=OFF -Wno-dev cmake --build build --config Release --target package - name: 'Upload Windows Installer' uses: actions/upload-artifact@v4 @@ -52,7 +52,7 @@ jobs: sudo apt-get install -y --no-install-recommends \ build-essential cmake ninja-build - name: Configure (CMake) - run: cmake -S . -B build -G Ninja -DBUILD_EXAMPLES=OFF -DBUILD_TESTING=OFF -DAOG_TC_VALIDATE_IOP=OFF -DCMAKE_BUILD_TYPE=Release -Wno-dev + run: cmake -S . -B build -G Ninja -DBUILD_EXAMPLES=OFF -DBUILD_TESTING=OFF -DAOG_TC_VALIDATE_IOP=OFF -DAOG_TC_BUILD_TESTS=OFF -DCMAKE_BUILD_TYPE=Release -Wno-dev - name: Build run: cmake --build build --config Release - name: Stage tarball diff --git a/.github/workflows/validate-iop.yml b/.github/workflows/validate-iop.yml index 13fd36f..6b018fa 100644 --- a/.github/workflows/validate-iop.yml +++ b/.github/workflows/validate-iop.yml @@ -34,7 +34,7 @@ jobs: -Wno-dev - name: Build - run: cmake --build build --config Release --target iop_validator + run: cmake --build build --config Release --target iop_validator ddop_hydration_test - - name: Validate object pool + - name: Validate object pool and run unit tests run: ctest --test-dir build --output-on-failure diff --git a/CMakeLists.txt b/CMakeLists.txt index 6785346..b681570 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -146,6 +146,28 @@ endif() install(TARGETS ${PROJECT_NAME} RUNTIME DESTINATION bin COMPONENT applications) +option(AOG_TC_BUILD_TESTS "Build and register the unit tests" ON) +if(AOG_TC_BUILD_TESTS) + enable_testing() + add_executable( + ddop_hydration_test + ${CMAKE_CURRENT_LIST_DIR}/tools/ddop_hydration_test.cpp + ${CMAKE_CURRENT_LIST_DIR}/src/ddop_hydration.cpp + ${CMAKE_CURRENT_LIST_DIR}/src/settings.cpp + ${CMAKE_CURRENT_LIST_DIR}/src/logging.cpp) + target_compile_features(ddop_hydration_test PRIVATE cxx_std_20) + set_target_properties(ddop_hydration_test PROPERTIES CXX_EXTENSIONS OFF) + target_include_directories(ddop_hydration_test + PRIVATE ${CMAKE_CURRENT_LIST_DIR}/include) + target_compile_definitions( + ddop_hydration_test PRIVATE PROJECT_VERSION="${PROJECT_VERSION}" + PROJECT_NAME="${PROJECT_NAME}") + target_link_libraries( + ddop_hydration_test PRIVATE isobus::Isobus isobus::Utility + nlohmann_json::nlohmann_json) + add_test(NAME ddop_hydration_snapshot COMMAND ddop_hydration_test) +endif() + option(AOG_TC_VALIDATE_IOP "Build and register the object pool validator test" ON) if(AOG_TC_VALIDATE_IOP) diff --git a/docs/CONCURRENCY.md b/docs/CONCURRENCY.md index 8df81de..396097e 100644 --- a/docs/CONCURRENCY.md +++ b/docs/CONCURRENCY.md @@ -57,6 +57,12 @@ registration point itself defers to an `update()` call, or invokes the listener/ take `clientsMutex` at its top, even though (per the note above) the `TaskControllerServer` overrides may turn out to already be main-thread-only — the lock is cheap insurance and keeps every entry point consistent regardless of how the library's internals might change. +- **Hydrated DDOP snapshot state** (`ClientState`'s shadow values, `MyTCServer::pendingHydration`) — + also guarded by `clientsMutex`. `on_value_command()` records values; `begin_hydration_snapshot()` + and `poll_hydration_snapshot()` run from `Application::update()`. The snapshot wait is polled, + never blocked on: `tcServer->update()` runs in the same main loop, and blocking it would stall the + responses being waited for. `poll_hydration_snapshot()` copies what it needs and writes files + after releasing the lock. The VT button listener only sets an `std::atomic`. ## What's *not* protected yet — known gap diff --git a/include/app.hpp b/include/app.hpp index d74de11..8d80e15 100644 --- a/include/app.hpp +++ b/include/app.hpp @@ -8,6 +8,7 @@ */ #pragma once +#include #include #include #include @@ -68,6 +69,7 @@ class Application void sync_vt_config_once(); void update_vt_section_map(); void update_vt_status_strings(bool aogConnected); + void update_hydration_snapshot(); void send_vt_string_if_changed(std::uint16_t objectID, const std::string &value); void send_hardware_message(const std::string &text, std::uint8_t duration, std::uint8_t color); @@ -96,6 +98,7 @@ class Application bool vtWasConnected = false; bool vtConnectionWarningLogged = false; bool vtCapabilitiesLogged = false; + std::atomic hydrationSnapshotRequested{ false }; ///< Set by the VT button listener, handled in update() std::uint8_t nmea2000SequenceIdentifier = 0; std::uint32_t lastJ1939SpeedTransmit = 0; std::uint32_t lastTCStatusTransmit = 0; diff --git a/include/ddop_hydration.hpp b/include/ddop_hydration.hpp new file mode 100644 index 0000000..828c2d1 --- /dev/null +++ b/include/ddop_hydration.hpp @@ -0,0 +1,123 @@ +/** + * @brief On-demand "hydrated" DDOP snapshots for debugging and visualization + * + * A client's canonical DDOP describes structure only: DeviceProcessData objects carry no value, + * those arrive later as process data value commands. A hydrated snapshot is a derived copy of the + * canonical pool in which hydratable DeviceProcessData objects are replaced by DeviceProperty + * objects (same object ID, DDI, designator and presentation) holding the latest known value, so + * external tools such as AgIsoDDOPGenerator can show those values. + * + * Snapshots are never uploaded to a TC and never replace or modify the canonical pool. + */ + +#pragma once + +#include "isobus/isobus/can_NAME.hpp" +#include "isobus/isobus/isobus_device_descriptor_object_pool.hpp" + +#include +#include +#include +#include +#include +#include + +namespace ddop_hydration +{ + /// @brief How long a snapshot waits for answers to its value requests + constexpr std::uint32_t REQUEST_WAIT_MS = 10000; + + /// @brief DDIs that are never hydrated: totals, setpoints and transient state such as work states, + /// section control state and actual rates. + bool is_always_excluded(std::uint16_t ddi); + + /// @brief Whether a pool object is part of a snapshot, decided from the pool alone: every DeviceProperty, + /// and every DeviceProcessData that reports on change, is not a total and is not always excluded. + bool is_hydratable(const isobus::task_controller_object::Object &object); + + /// @brief The latest process data value reported by a client for one object + struct ShadowValue + { + std::int32_t value = 0; + std::uint32_t timestamp_ms = 0; + }; + + /// @brief Latest reported process data values per object ID for one client. + /// Not thread-safe on its own; MyTCServer guards it with clientsMutex. + class ShadowValueStore + { + public: + void record(std::uint16_t objectID, std::int32_t value); + bool try_get(std::uint16_t objectID, ShadowValue &shadowValue) const; + + private: + std::map values; + }; + + /// @brief Maps the (DDI, element number) addressing of process data messages to DDOP object IDs + class ProcessDataIndex + { + public: + void build(isobus::DeviceDescriptorObjectPool &pool); + bool try_get_object_id(std::uint16_t ddi, std::uint16_t elementNumber, std::uint16_t &objectID) const; + bool try_get_element_number(std::uint16_t objectID, std::uint16_t &elementNumber) const; + + private: + std::map, std::uint16_t> ddiAndElementToObjectID; + std::map objectIDToElementNumber; + }; + + /// @brief Where a snapshot object's value came from + enum class ValueSource : std::uint8_t + { + Pool, ///< DeviceProperty, value taken from the canonical pool as-is + Live, ///< Shadow value that was already known when the snapshot was requested + Requested, ///< Value arrived after an on-demand value request + NoResponse, ///< Value was requested but did not arrive in time + NotRequestable ///< DeviceProcessData that no device element references, so it cannot be addressed + }; + + const char *to_string(ValueSource source); + + struct SnapshotEntry + { + static constexpr std::uint16_t NO_ELEMENT = 0xFFFF; + + std::uint16_t objectID = 0; + std::uint16_t ddi = 0; + std::uint16_t elementNumber = NO_ELEMENT; + ValueSource source = ValueSource::NoResponse; + std::int32_t value = 0; ///< Only meaningful for Pool, Live and Requested + }; + + struct SnapshotInput + { + std::vector> canonicalPoolChunks; ///< The client's DDOP exactly as uploaded + std::uint8_t taskControllerCompatibilityLevel = 0; + std::uint64_t clientName = 0; + std::string fileStem; ///< Relative to the settings directory, e.g. "/