From ff333664170228020ea5df1d4927aea43cf95a74 Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Wed, 31 May 2023 20:49:21 +0200 Subject: [PATCH 1/3] [Feature] Allow passing of function pointer for feeding the watchdog during lengthy file IO operations. This prevents the watchdog from biting and resetting the application before all values have been loaded from/stored to the persistent storage medium. This fixes #233. --- src/util/storage/register_storage.hpp | 30 ++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/src/util/storage/register_storage.hpp b/src/util/storage/register_storage.hpp index d77b9435..d6cda413 100644 --- a/src/util/storage/register_storage.hpp +++ b/src/util/storage/register_storage.hpp @@ -24,10 +24,16 @@ namespace cyphal::support /// The serialization format is simply the Cyphal DSDL. /// In case of error, only part of the registers may be loaded and the registry will be left in an inconsistent state. [[nodiscard]] inline std::optional load(const platform::storage::interface::KeyValueStorage& kv, - registry::IIntrospectableRegistry& rgy) + registry::IIntrospectableRegistry& rgy, + std::function const feed_watchdog_func) { for (std::size_t index = 0; index < rgy.size(); index++) { + // Prevent timeout during slow persistent storage IO access + if (feed_watchdog_func) { + feed_watchdog_func(); + } + // Find the next register in the registry. const auto reg_name_storage = rgy.index(index); // This is a little suboptimal but we don't care. const auto reg_name = std::string_view(reinterpret_cast(reg_name_storage.name.cbegin()), reg_name_storage.name.size()); @@ -67,6 +73,11 @@ namespace cyphal::support } return std::nullopt; } +[[nodiscard]] inline std::optional load(const platform::storage::interface::KeyValueStorage& kv, + registry::IIntrospectableRegistry& rgy) +{ + return load(kv, rgy, nullptr); +} /// The register savior is the counterpart of load(). /// Saves all persistent mutable registers from the registry to the storage. @@ -84,10 +95,16 @@ namespace cyphal::support template [[nodiscard]] std::optional save(platform::storage::interface::KeyValueStorage& kv, const registry::IIntrospectableRegistry& rgy, + std::function const feed_watchdog_func, ResetPredicate const reset_predicate) { for (std::size_t index = 0; index < rgy.size(); index++) { + // Prevent timeout during slow persistent storage IO access + if (feed_watchdog_func) { + feed_watchdog_func(); + } + const auto reg_name_storage = rgy.index(index); // This is a little suboptimal but we don't care. const auto reg_name = std::string_view(reinterpret_cast(reg_name_storage.name.cbegin()), reg_name_storage.name.size()); if (reg_name.empty()) @@ -127,10 +144,17 @@ template } return std::nullopt; } -[[nodiscard]] inline std::optional save(platform::storage::interface::KeyValueStorage& kv, +template +[[nodiscard]] std::optional save(platform::storage::interface::KeyValueStorage& kv, + const registry::IIntrospectableRegistry& rgy, + std::function const feed_watchdog_func) +{ + return save(kv, rgy, feed_watchdog_func, [](std::string_view) { return false; }); +} +[[nodiscard]] inline std::optional save(platform::storage::interface::KeyValueStorage& kv, const registry::IIntrospectableRegistry& rgy) { - return save(kv, rgy, [](std::string_view) { return false; }); + return save(kv, rgy, nullptr, [](std::string_view) { return false; }); } } /* namespace cyphal::support */ From c2abc0357b02c3cb70f204005d1212fd9dcc8855 Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Thu, 1 Jun 2023 20:11:29 +0200 Subject: [PATCH 2/3] Cleaning up formatting and fixing a bug. --- src/util/storage/register_storage.hpp | 39 ++++++++++++++++----------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/src/util/storage/register_storage.hpp b/src/util/storage/register_storage.hpp index d6cda413..744483fb 100644 --- a/src/util/storage/register_storage.hpp +++ b/src/util/storage/register_storage.hpp @@ -23,9 +23,10 @@ namespace cyphal::support /// Stored registers that are not present in the registry will not be loaded. /// The serialization format is simply the Cyphal DSDL. /// In case of error, only part of the registers may be loaded and the registry will be left in an inconsistent state. -[[nodiscard]] inline std::optional load(const platform::storage::interface::KeyValueStorage& kv, - registry::IIntrospectableRegistry& rgy, - std::function const feed_watchdog_func) +[[nodiscard]] inline std::optional load( + const platform::storage::interface::KeyValueStorage & kv, + registry::IIntrospectableRegistry & rgy, + std::function const feed_watchdog_func) { for (std::size_t index = 0; index < rgy.size(); index++) { @@ -73,8 +74,10 @@ namespace cyphal::support } return std::nullopt; } -[[nodiscard]] inline std::optional load(const platform::storage::interface::KeyValueStorage& kv, - registry::IIntrospectableRegistry& rgy) + +[[nodiscard]] inline std::optional load( + const platform::storage::interface::KeyValueStorage & kv, + registry::IIntrospectableRegistry & rgy) { return load(kv, rgy, nullptr); } @@ -93,10 +96,11 @@ namespace cyphal::support /// The removal predicate, if provided, allows the caller to specify which registers need to be removed from the /// storage instead of being saved. This is useful for implementing the "factory reset" feature. template -[[nodiscard]] std::optional save(platform::storage::interface::KeyValueStorage& kv, - const registry::IIntrospectableRegistry& rgy, - std::function const feed_watchdog_func, - ResetPredicate const reset_predicate) +[[nodiscard]] inline std::optional save( + platform::storage::interface::KeyValueStorage & kv, + const registry::IIntrospectableRegistry & rgy, + std::function const feed_watchdog_func, + ResetPredicate const reset_predicate) { for (std::size_t index = 0; index < rgy.size(); index++) { @@ -144,19 +148,22 @@ template } return std::nullopt; } -template -[[nodiscard]] std::optional save(platform::storage::interface::KeyValueStorage& kv, - const registry::IIntrospectableRegistry& rgy, - std::function const feed_watchdog_func) + +[[nodiscard]] inline std::optional save( + platform::storage::interface::KeyValueStorage & kv, + const registry::IIntrospectableRegistry & rgy, + std::function const feed_watchdog_func) { return save(kv, rgy, feed_watchdog_func, [](std::string_view) { return false; }); } -[[nodiscard]] inline std::optional save(platform::storage::interface::KeyValueStorage& kv, - const registry::IIntrospectableRegistry& rgy) + +[[nodiscard]] inline std::optional save( + platform::storage::interface::KeyValueStorage & kv, + const registry::IIntrospectableRegistry & rgy) { return save(kv, rgy, nullptr, [](std::string_view) { return false; }); } -} /* namespace cyphal::support */ +} /* cyphal::support */ #endif /* !defined(__GNUC__) || (__GNUC__ >= 11) */ From 8d4059bc0f079895e1316e9852cc09dad38320ec Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Thu, 1 Jun 2023 20:19:43 +0200 Subject: [PATCH 3/3] Replace std::function with template parameter. --- src/util/storage/register_storage.hpp | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/util/storage/register_storage.hpp b/src/util/storage/register_storage.hpp index 744483fb..afc1d94a 100644 --- a/src/util/storage/register_storage.hpp +++ b/src/util/storage/register_storage.hpp @@ -23,17 +23,16 @@ namespace cyphal::support /// Stored registers that are not present in the registry will not be loaded. /// The serialization format is simply the Cyphal DSDL. /// In case of error, only part of the registers may be loaded and the registry will be left in an inconsistent state. +template [[nodiscard]] inline std::optional load( const platform::storage::interface::KeyValueStorage & kv, registry::IIntrospectableRegistry & rgy, - std::function const feed_watchdog_func) + FeedWatchdogFunc const & feed_watchdog_func) { for (std::size_t index = 0; index < rgy.size(); index++) { // Prevent timeout during slow persistent storage IO access - if (feed_watchdog_func) { - feed_watchdog_func(); - } + feed_watchdog_func(); // Find the next register in the registry. const auto reg_name_storage = rgy.index(index); // This is a little suboptimal but we don't care. @@ -79,7 +78,7 @@ namespace cyphal::support const platform::storage::interface::KeyValueStorage & kv, registry::IIntrospectableRegistry & rgy) { - return load(kv, rgy, nullptr); + return load(kv, rgy, []() { }); } /// The register savior is the counterpart of load(). @@ -95,19 +94,17 @@ namespace cyphal::support /// /// The removal predicate, if provided, allows the caller to specify which registers need to be removed from the /// storage instead of being saved. This is useful for implementing the "factory reset" feature. -template +template [[nodiscard]] inline std::optional save( platform::storage::interface::KeyValueStorage & kv, const registry::IIntrospectableRegistry & rgy, - std::function const feed_watchdog_func, + FeedWatchdogFunc const & feed_watchdog_func, ResetPredicate const reset_predicate) { for (std::size_t index = 0; index < rgy.size(); index++) { // Prevent timeout during slow persistent storage IO access - if (feed_watchdog_func) { - feed_watchdog_func(); - } + feed_watchdog_func(); const auto reg_name_storage = rgy.index(index); // This is a little suboptimal but we don't care. const auto reg_name = std::string_view(reinterpret_cast(reg_name_storage.name.cbegin()), reg_name_storage.name.size()); @@ -149,10 +146,11 @@ template return std::nullopt; } +template [[nodiscard]] inline std::optional save( platform::storage::interface::KeyValueStorage & kv, const registry::IIntrospectableRegistry & rgy, - std::function const feed_watchdog_func) + FeedWatchdogFunc const & feed_watchdog_func) { return save(kv, rgy, feed_watchdog_func, [](std::string_view) { return false; }); } @@ -161,7 +159,7 @@ template platform::storage::interface::KeyValueStorage & kv, const registry::IIntrospectableRegistry & rgy) { - return save(kv, rgy, nullptr, [](std::string_view) { return false; }); + return save(kv, rgy, []() { }, [](std::string_view) { return false; }); } } /* cyphal::support */