diff --git a/.clang-tidy b/.clang-tidy index 6731b54..49f9778 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -26,6 +26,9 @@ Checks: >- -*-easily-swappable-parameters, -*-owning-memory, -*-malloc, +CheckOptions: + - key: readability-magic-numbers.IgnoredIntegerValues + value: '1;2;3;4;5;10;20;60;64;100;128;256;500;512;1000' WarningsAsErrors: '*' HeaderFilterRegex: '.*' AnalyzeTemporaryDtors: false diff --git a/README.md b/README.md index 0fcf4df..460985b 100644 --- a/README.md +++ b/README.md @@ -25,9 +25,9 @@ A standard-compliant implementation of the software update server is provided in Kochergá's own codebase features extensive test coverage. - **Multiple supported transports:** - - **Cyphal/CAN** + **DroneCAN** -- the protocol version is auto-detected at runtime. - - **Cyphal/serial** - - More may appear in the future -- new transports are easy to add. + - **Cyphal/CAN** + **DroneCAN** -- the protocol version is auto-detected at runtime. + - **Cyphal/serial** + - More may appear in the future -- new transports are easy to add. ## Usage @@ -311,9 +311,6 @@ static_assert(std::is_trivial_v); #include // Pick the transports you need. #include // In this example we are using Cyphal/serial + Cyphal/CAN. -/// Maximum possible size of the application image for your platform. -static constexpr std::size_t MaxAppSize = 1024 * 1024; - int main() { // Check if the application has passed any arguments to the bootloader via shared RAM. @@ -325,7 +322,8 @@ int main() // Initialize the bootloader core. MyROMBackend rom_backend; kocherga::SystemInfo system_info = GET_SYSTEM_INFO(); - kocherga::Bootloader boot(rom_backend, system_info, MaxAppSize, bool(args)); + kocherga::Bootloader::Params params{.linger = args.has_value()}; // Read the docs on the available params. + kocherga::Bootloader boot(rom_backend, system_info, params); // It's a good idea to check if the app is valid and safe to boot before adding the nodes. // This way you can skip the potentially slow or disturbing interface initialization on the happy path. // You can do it by calling poll() here once. @@ -450,6 +448,11 @@ It is recommended to copy-paste relevant pieces from Kochergá instead; specific ## Revisions +### v2.0 + +- Provide dedicated parameter struct to minimize usage errors. +- Retry timed out requests up to a configurable number of times (https://github.com/Zubax/kocherga/issues/17). + ### v1.0 The first stable revision is virtually identical to v0.2. diff --git a/kocherga/kocherga.hpp b/kocherga/kocherga.hpp index a2ac0eb..abe8106 100644 --- a/kocherga/kocherga.hpp +++ b/kocherga/kocherga.hpp @@ -12,7 +12,7 @@ #include #include -#define KOCHERGA_VERSION_MAJOR 1 // NOLINT NOSONAR +#define KOCHERGA_VERSION_MAJOR 2 // NOLINT NOSONAR #define KOCHERGA_VERSION_MINOR 0 // NOLINT NOSONAR #ifndef KOCHERGA_ASSERT @@ -543,9 +543,30 @@ class IController /// Unifies multiple INode and performs DSDL serialization. Manages the network at the presentation layer. class Presenter final : public IReactor { + struct FileLocationSpecifier final + { + std::uint8_t local_node_index{}; + NodeID server_node_id{}; + std::size_t path_length{}; + std::array path{}; + + struct Pending final + { + std::uint64_t offset; + std::chrono::microseconds response_deadline; + std::uint8_t remaining_attempts; + }; + std::optional pending; + }; + public: - Presenter(const SystemInfo& system_info, IController& controller) : - system_info_(system_info), controller_(controller) + struct Params final + { + std::uint8_t request_retry_limit; + }; + + Presenter(const SystemInfo& system_info, IController& controller, const Params& param) : + par_(param), system_info_(system_info), controller_(controller) {} [[nodiscard]] auto addNode(INode* const node) -> bool @@ -614,12 +635,20 @@ class Presenter final : public IReactor if (file_loc_spec_) { FileLocationSpecifier& fls = *file_loc_spec_; - if (fls.response_deadline && (uptime > *fls.response_deadline)) + if (fls.pending && (uptime > fls.pending->response_deadline)) { - INode* const nd = nodes_.at(fls.local_node_index); - nd->cancelRequest(); - fls.response_deadline.reset(); - controller_.handleFileReadResult({}); + if (fls.pending->remaining_attempts > 0) + { + fls.pending->remaining_attempts--; + fls.pending->response_deadline = uptime + ServiceResponseTimeout; + (void) sendFileReadRequest(fls); // Ignore the result, we will retry later if possible. + } + else + { + nodes_.at(fls.local_node_index)->cancelRequest(); + fls.pending.reset(); + controller_.handleFileReadResult({}); + } } } @@ -633,39 +662,21 @@ class Presenter final : public IReactor void setNodeHealth(const dsdl::Heartbeat::Health value) { node_health_ = value; } void setNodeVSSC(const std::uint8_t value) { node_vssc_ = value; } - /// The timeout will be managed by the presenter automatically. + /// The timeout and retries will be managed by the presenter automatically. + /// No timeout error will be reported until all retries are exhausted. [[nodiscard]] auto requestFileRead(const std::uint64_t offset) -> bool { if (file_loc_spec_) { - std::array buf{}; - - auto of = offset; - buf[0] = static_cast(of); - of >>= BitsPerByte; - buf[1] = static_cast(of); - of >>= BitsPerByte; - buf[2] = static_cast(of); - of >>= BitsPerByte; - buf[3] = static_cast(of); - of >>= BitsPerByte; - buf[4] = static_cast(of); - - static constexpr auto length_minus_path = 6U; - FileLocationSpecifier& fls = *file_loc_spec_; - buf.at(length_minus_path - 1U) = static_cast(fls.path_length); - (void) std::memmove(&buf.at(length_minus_path), fls.path.data(), fls.path_length); - INode* const node = nodes_.at(fls.local_node_index); - - read_transfer_id_++; - const bool out = node->sendRequest(ServiceID::FileRead, - fls.server_node_id, - read_transfer_id_, - fls.path_length + length_minus_path, - buf.data()); - if (out) + FileLocationSpecifier::Pending pend{}; + pend.offset = offset; + pend.response_deadline = last_poll_at_ + ServiceResponseTimeout; + pend.remaining_attempts = par_.request_retry_limit; + file_loc_spec_->pending.emplace(pend); + const bool out = sendFileReadRequest(*file_loc_spec_); + if (!out) { - fls.response_deadline = last_poll_at_ + ServiceResponseTimeout; + file_loc_spec_->pending.reset(); } return out; } @@ -836,12 +847,12 @@ class Presenter final : public IReactor if (file_loc_spec_ && (response_length >= dsdl::File::ReadResponseSizeMin)) { FileLocationSpecifier& fls = *file_loc_spec_; - if (fls.response_deadline && (fls.local_node_index == current_node_index_)) + if (fls.pending && (fls.local_node_index == current_node_index_)) { - fls.response_deadline.reset(); + fls.pending.reset(); static const std::array zero_error{}; std::optional argument; - if (std::equal(std::begin(zero_error), std::end(zero_error), response)) // Error = OK + if (std::equal(zero_error.begin(), zero_error.end(), response)) // Error = OK { argument = dsdl::File::ReadResponse{ static_cast( @@ -882,14 +893,33 @@ class Presenter final : public IReactor ++tid_heartbeat_; } - struct FileLocationSpecifier + [[nodiscard]] auto sendFileReadRequest(FileLocationSpecifier& fls) -> bool { - std::uint8_t local_node_index{}; - NodeID server_node_id{}; - std::size_t path_length{}; - std::array path{}; - std::optional response_deadline{}; - }; + std::array buf{}; + + auto of = fls.pending.value().offset; + buf[0] = static_cast(of); + of >>= BitsPerByte; + buf[1] = static_cast(of); + of >>= BitsPerByte; + buf[2] = static_cast(of); + of >>= BitsPerByte; + buf[3] = static_cast(of); + of >>= BitsPerByte; + buf[4] = static_cast(of); + + static constexpr auto length_minus_path = 6U; + buf.at(length_minus_path - 1U) = static_cast(fls.path_length); + (void) std::memmove(&buf.at(length_minus_path), fls.path.data(), fls.path_length); + INode* const node = nodes_.at(fls.local_node_index); + + read_transfer_id_++; + return node->sendRequest(ServiceID::FileRead, + fls.server_node_id, + read_transfer_id_, + fls.path_length + length_minus_path, + buf.data()); + } void beginUpdate(const std::uint8_t local_node_index, const NodeID file_server_node_id, @@ -902,7 +932,7 @@ class Presenter final : public IReactor fls.path_length = std::min(app_image_file_path_length, std::size(fls.path)); (void) std::memmove(fls.path.data(), app_image_file_path, fls.path_length); - if (file_loc_spec_ && file_loc_spec_->response_deadline) + if (file_loc_spec_ && file_loc_spec_->pending) { nodes_.at(file_loc_spec_->local_node_index)->cancelRequest(); } @@ -913,6 +943,7 @@ class Presenter final : public IReactor static constexpr std::uint8_t MaxNodes = 8; + const Params par_; const SystemInfo system_info_; std::array nodes_{}; std::uint8_t num_nodes_ = 0; @@ -998,39 +1029,53 @@ enum class Final class Bootloader : public detail::IController { public: - /// The max application image size parameter is very important for performance reasons. - /// Without it, the bootloader may encounter an unrelated data structure in the ROM that looks like a - /// valid app descriptor (by virtue of having the same magic, which is only 64 bit long), - /// and it may spend a considerable amount of time trying to check the CRC that is certainly invalid. - /// Having an upper size limit for the application image allows the bootloader to weed out too large - /// values early, greatly improving the worst case boot time. - /// + struct Params final + { + /// The max application image size parameter is very important for performance reasons. + /// Without it, the bootloader may encounter an unrelated data structure in the ROM that looks like a + /// valid app descriptor (by virtue of having the same magic, which is only 64 bit long), + /// and it may spend a considerable amount of time trying to check the CRC that is certainly invalid. + /// Having an upper size limit for the application image allows the bootloader to weed out too large + /// values early, improving the worst case boot time. + std::size_t max_app_size = std::numeric_limits::max(); + + /// If the linger flag is set, the bootloader will not boot the application after the initial verification. + /// If the application is valid, then the initial state will be BootCanceled instead of BootDelay. + /// If the application is invalid, the flag will have no effect. + /// It is designed to support the common use case where the application commands the bootloader to start and + /// sit idle until instructed otherwise, or if the application itself commands the bootloader to begin the + /// update. The flag affects only the initial verification and has no effect on all subsequent checks; for + /// example, after the application is updated and validated, it will be booted after BootDelay regardless of + /// this flag. + bool linger = false; + + /// Wait this much time before booting the application. Keep zero if not sure. + std::chrono::seconds boot_delay = std::chrono::seconds::zero(); + + /// If the allow_legacy_app_descriptors option is set, the bootloader will also accept legacy descriptors + /// alongside the new format. This option should be set only if the bootloader is introduced to a product that + /// was using the old app descriptor format in the past; refer to the PX4 Brickproof Bootloader for details. If + /// you are not sure, leave the default value. + bool allow_legacy_app_descriptors = false; + + /// The total maximum number of network service requests is this value plus one. + /// The counter is reset after every successful request. + std::uint8_t request_retry_limit = 5; + }; + /// SystemInfo is used for responding to uavcan.node.GetInfo requests. - /// - /// If the linger flag is set, the bootloader will not boot the application after the initial verification. - /// If the application is valid, then the initial state will be BootCanceled instead of BootDelay. - /// If the application is invalid, the flag will have no effect. - /// It is designed to support the common use case where the application commands the bootloader to start and - /// sit idle until instructed otherwise, or if the application itself commands the bootloader to begin the update. - /// The flag affects only the initial verification and has no effect on all subsequent checks; for example, - /// after the application is updated and validated, it will be booted after BootDelay regardless of this flag. - /// - /// If the allow_legacy_app_descriptors option is set, the bootloader will also accept legacy descriptors alongside - /// the new format. This option should be set only if the bootloader is introduced to a product that was using - /// the old app descriptor format in the past; refer to the PX4 Brickproof Bootloader for details. If you are not - /// sure, leave the default value. - Bootloader(IROMBackend& rom_backend, - const SystemInfo& system_info, - const std::size_t max_app_size, - const bool linger, - const std::chrono::seconds boot_delay = std::chrono::seconds(0), - const bool allow_legacy_app_descriptors = false) : - max_app_size_(max_app_size), - boot_delay_(boot_delay), + /// The lifetime of params is unrestricted as the contents are copied. + Bootloader(IROMBackend& rom_backend, const SystemInfo& system_info, const Params& param) : + max_app_size_(param.max_app_size), + boot_delay_(param.boot_delay), backend_(rom_backend), - presentation_(system_info, *this), - linger_(linger), - allow_legacy_app_descriptors_(allow_legacy_app_descriptors) + presentation_{ + system_info, + *this, + detail::Presenter::Params{param.request_retry_limit}, + }, + linger_(param.linger), + allow_legacy_app_descriptors_(param.allow_legacy_app_descriptors) {} /// Nodes shall be registered using this method after the instance is constructed. diff --git a/tests/integration/bootloader/main.cpp b/tests/integration/bootloader/main.cpp index 310670c..67703f7 100644 --- a/tests/integration/bootloader/main.cpp +++ b/tests/integration/bootloader/main.cpp @@ -274,8 +274,12 @@ auto main(const int argc, char* const argv[]) -> int util::FileROMBackend rom(rom_file, rom_size); - const auto system_info = getSystemInfo(); - kocherga::Bootloader boot(rom, system_info, max_app_size, linger, boot_delay); + const auto system_info = getSystemInfo(); + kocherga::Bootloader::Params params; + params.max_app_size = max_app_size; + params.linger = linger; + params.boot_delay = boot_delay; + kocherga::Bootloader boot(rom, system_info, params); // Configure the serial port node. auto serial_port = initSerialPort(); diff --git a/tests/unit/test_core.cpp b/tests/unit/test_core.cpp index b4310da..3ceedf6 100644 --- a/tests/unit/test_core.cpp +++ b/tests/unit/test_core.cpp @@ -64,9 +64,12 @@ TEST_CASE("Bootloader-fast-boot") const auto sys = getSysInfo(); const auto img = util::getImagePath("good-le-simple-3.1.badc0ffee0ddf00d.452a4267971a3928.app.release.bin"); - util::FileROMBackend rom(img); - std::array nodes; - kocherga::Bootloader bl(rom, sys, static_cast(std::filesystem::file_size(img)), false); + util::FileROMBackend rom(img); + std::array nodes; + kocherga::Bootloader::Params params; + params.max_app_size = static_cast(std::filesystem::file_size(img)); + params.linger = false; + kocherga::Bootloader bl(rom, sys, params); REQUIRE(bl.addNode(&nodes.at(0))); REQUIRE(bl.addNode(&nodes.at(1))); REQUIRE(bl.addNode(&nodes.at(2))); @@ -92,9 +95,13 @@ TEST_CASE("Bootloader-boot-delay") const auto sys = getSysInfo(); const auto img = util::getImagePath("good-le-3rd-entry-5.6.3333333333333333.8b61938ee5f90b1f.app.dirty.bin"); - util::FileROMBackend rom(img); - mock::Node node; - kocherga::Bootloader bl(rom, sys, static_cast(std::filesystem::file_size(img)), false, 1s); + util::FileROMBackend rom(img); + mock::Node node; + kocherga::Bootloader::Params params; + params.max_app_size = static_cast(std::filesystem::file_size(img)); + params.linger = false; + params.boot_delay = 1s; + kocherga::Bootloader bl(rom, sys, params); REQUIRE(bl.addNode(&node)); REQUIRE(!bl.poll(500ms)); @@ -123,9 +130,12 @@ TEST_CASE("Bootloader-linger-reboot") const auto sys = getSysInfo(); const auto img = util::getImagePath("good-le-simple-3.1.badc0ffee0ddf00d.452a4267971a3928.app.release.bin"); - util::FileROMBackend rom(img); - std::array nodes; - kocherga::Bootloader bl(rom, sys, static_cast(std::filesystem::file_size(img)), true); + util::FileROMBackend rom(img); + std::array nodes; + kocherga::Bootloader::Params params; + params.max_app_size = static_cast(std::filesystem::file_size(img)); + params.linger = true; + kocherga::Bootloader bl(rom, sys, params); REQUIRE(bl.addNode(&nodes.at(0))); REQUIRE(bl.addNode(&nodes.at(1))); @@ -157,9 +167,12 @@ TEST_CASE("Bootloader-update-valid") const auto sys = getSysInfo(); const auto img = util::getImagePath("good-le-3rd-entry-5.6.3333333333333333.8b61938ee5f90b1f.app.dirty.bin"); REQUIRE(std::filesystem::copy_file(img, "rom.img.tmp", std::filesystem::copy_options::overwrite_existing)); - util::FileROMBackend rom("rom.img.tmp"); - std::array nodes; - kocherga::Bootloader bl(rom, sys, static_cast(std::filesystem::file_size(img)), true); + util::FileROMBackend rom("rom.img.tmp"); + std::array nodes; + kocherga::Bootloader::Params params; + params.max_app_size = static_cast(std::filesystem::file_size(img)); + params.linger = true; + kocherga::Bootloader bl(rom, sys, params); REQUIRE(bl.addNode(&nodes.at(0))); REQUIRE(bl.addNode(&nodes.at(1))); @@ -251,9 +264,14 @@ TEST_CASE("Bootloader-update-invalid") // NOLINT NOSONAR complexity threshold const auto sys = getSysInfo(); const auto img = util::getImagePath("good-le-3rd-entry-5.6.3333333333333333.8b61938ee5f90b1f.app.dirty.bin"); REQUIRE(std::filesystem::copy_file(img, "rom.img.tmp", std::filesystem::copy_options::overwrite_existing)); - util::FileROMBackend rom("rom.img.tmp"); - std::array nodes; - kocherga::Bootloader bl(rom, sys, static_cast(std::filesystem::file_size(img)), false, 2s); + util::FileROMBackend rom("rom.img.tmp"); + std::array nodes; + kocherga::Bootloader::Params params; + params.max_app_size = static_cast(std::filesystem::file_size(img)); + params.linger = false; + params.boot_delay = 2s; + params.request_retry_limit = 0; + kocherga::Bootloader bl(rom, sys, params); REQUIRE(bl.addNode(&nodes.at(0))); REQUIRE(!bl.poll(1'100ms)); @@ -331,7 +349,7 @@ TEST_CASE("Bootloader-update-invalid") // NOLINT NOSONAR complexity threshold // SECOND READ REQUEST REQUIRE(!bl.poll(3'300ms)); - received = *nodes.at(0).popOutput(Node::Output::FileReadRequest); + received = nodes.at(0).popOutput(Node::Output::FileReadRequest).value(); reference = Transfer(2, {0, 1, 0, 0, 0, 17, 98, 97, 100, 45, 108, 101, 45, 99, 114, 99, 45, 120, 51, 46, 98, 105, 110}, 1111); @@ -399,7 +417,7 @@ TEST_CASE("Bootloader-update-invalid") // NOLINT NOSONAR complexity threshold (void) nodes.at(0).popOutput(Node::Output::LogRecordMessage); REQUIRE(nodes.at(0).popOutput(Node::Output::FileReadRequest)); nodes.at(0).pushInput(Node::Input::FileReadResponse, - Transfer(3, + Transfer(4, {0x00, 0x00, 0x09, 0x00, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa}, 3210)); REQUIRE(!bl.poll(10'800ms)); @@ -422,9 +440,13 @@ TEST_CASE("Bootloader-trigger") const auto sys = getSysInfo(); const auto img = util::getImagePath("good-le-3rd-entry-5.6.3333333333333333.8b61938ee5f90b1f.app.dirty.bin"); REQUIRE(std::filesystem::copy_file(img, "rom.img.tmp", std::filesystem::copy_options::overwrite_existing)); - util::FileROMBackend rom("rom.img.tmp"); - std::array nodes; - kocherga::Bootloader bl(rom, sys, static_cast(std::filesystem::file_size(img)), false, 1s); + util::FileROMBackend rom("rom.img.tmp"); + std::array nodes; + kocherga::Bootloader::Params params; + params.max_app_size = static_cast(std::filesystem::file_size(img)); + params.linger = false; + params.boot_delay = 1s; + kocherga::Bootloader bl(rom, sys, params); REQUIRE(bl.addNode(&nodes.at(0))); REQUIRE(bl.addNode(&nodes.at(1))); REQUIRE(bl.addNode(&nodes.at(2))); @@ -450,7 +472,7 @@ TEST_CASE("Bootloader-trigger") REQUIRE(!bl.poll(1'100ms)); REQUIRE(checkHeartbeat(nodes, 0, 1, Heartbeat::Health::Nominal, 1)); - // FIRST READ REQUEST + // FIRST READ REQUEST, FIRST ATTEMPT REQUIRE(!bl.poll(1'200ms)); REQUIRE(nodes.at(0).popOutput(Node::Output::LogRecordMessage)); REQUIRE(nodes.at(1).popOutput(Node::Output::LogRecordMessage)); @@ -459,8 +481,8 @@ TEST_CASE("Bootloader-trigger") REQUIRE(!nodes.at(1).popOutput(Node::Output::FileReadRequest)); // list(b''.join(pycyphal.dsdl.serialize(uavcan.file.Read_1_1.Request(0, // uavcan.file.Path_2_0('good-le-3rd-entry-5.6.3333333333333333.8b61938ee5f90b1f.app.dirty.bin'))))) - const auto received = *nodes.at(2).popOutput(Node::Output::FileReadRequest); - const auto reference = + auto received = nodes.at(2).popOutput(Node::Output::FileReadRequest).value(); + auto reference = Transfer(1, {0, 0, 0, 0, 0, 69, 103, 111, 111, 100, 45, 108, 101, 45, 51, 114, 100, 45, 101, 110, 116, 114, 121, 45, 53, 46, 54, 46, 51, 51, 51, 51, 51, 51, 51, 51, 51, 51, @@ -470,10 +492,38 @@ TEST_CASE("Bootloader-trigger") std::cout << received.toString() << reference.toString() << std::endl; REQUIRE(received == reference); - // READ RESPONSE + // FIRST READ REQUEST, SECOND ATTEMPT + REQUIRE(!bl.poll(6'000ms)); + REQUIRE(nodes.at(0).popOutput(Node::Output::HeartbeatMessage)); + REQUIRE(nodes.at(1).popOutput(Node::Output::HeartbeatMessage)); + REQUIRE(nodes.at(2).popOutput(Node::Output::HeartbeatMessage)); + REQUIRE(!nodes.at(0).popOutput(Node::Output::LogRecordMessage)); + REQUIRE(!nodes.at(1).popOutput(Node::Output::LogRecordMessage)); + REQUIRE(!nodes.at(2).popOutput(Node::Output::LogRecordMessage)); + REQUIRE(!nodes.at(0).popOutput(Node::Output::FileReadRequest)); + REQUIRE(!nodes.at(1).popOutput(Node::Output::FileReadRequest)); + received = nodes.at(2).popOutput(Node::Output::FileReadRequest).value(); + reference.transfer_id = 2; // transfer-ID incremented, the rest is the same + REQUIRE(received == reference); + + // FIRST READ REQUEST, THIRD ATTEMPT + REQUIRE(!bl.poll(12'000ms)); + REQUIRE(nodes.at(0).popOutput(Node::Output::HeartbeatMessage)); + REQUIRE(nodes.at(1).popOutput(Node::Output::HeartbeatMessage)); + REQUIRE(nodes.at(2).popOutput(Node::Output::HeartbeatMessage)); + REQUIRE(!nodes.at(0).popOutput(Node::Output::LogRecordMessage)); + REQUIRE(!nodes.at(1).popOutput(Node::Output::LogRecordMessage)); + REQUIRE(!nodes.at(2).popOutput(Node::Output::LogRecordMessage)); + REQUIRE(!nodes.at(0).popOutput(Node::Output::FileReadRequest)); + REQUIRE(!nodes.at(1).popOutput(Node::Output::FileReadRequest)); + received = nodes.at(2).popOutput(Node::Output::FileReadRequest).value(); + reference.transfer_id = 3; // transfer-ID incremented, the rest is the same + REQUIRE(received == reference); + + // READ RESPONSE TO THE THIRD ATTEMPT // The serialized representation was constructed manually from the binary file nodes.at(2).pushInput(Node::Input::FileReadResponse, - Transfer(0, + Transfer(3, {0, 0, 128, 0, 72, 101, 108, 108, 111, 32, 119, 111, 114, 108, 100, 63, 32, 32, 32, 32, 199, 196, 192, 111, 20, 21, 68, 94, 65, 80, 68, 101, 115, 99, 48, 48, 124, 194, 145, 71, 22, 198, 82, 134, 64, 2, 0, 0, 0, 0, 0, @@ -483,12 +533,14 @@ TEST_CASE("Bootloader-trigger") 126, 126, 126, 126, 126, 126, 126, 126, 126, 126, 126, 126, 126, 126, 126, 126, 126, 126, 126, 126, 126, 126, 126, 126, 126, 126, 126, 126, 126, 126}, 2222)); - (void) bl.poll(1'300ms); // Results will appear on the SECOND poll. + (void) bl.poll(12'100ms); // Results will appear on the SECOND poll. + REQUIRE(nodes.at(0).popOutput(Node::Output::HeartbeatMessage)); + REQUIRE(nodes.at(1).popOutput(Node::Output::HeartbeatMessage)); + REQUIRE(nodes.at(2).popOutput(Node::Output::HeartbeatMessage)); REQUIRE(bl.getState() == kocherga::State::BootDelay); - REQUIRE(kocherga::Final::BootApp == *bl.poll(2'400ms)); // Completed here. - REQUIRE(kocherga::Final::BootApp == *bl.poll(2'500ms)); // All subsequent calls yield the same Final. - REQUIRE(kocherga::Final::BootApp == *bl.poll(2'600ms)); // Yep. + REQUIRE(kocherga::Final::BootApp == bl.poll(15'000ms).value()); + REQUIRE(kocherga::Final::BootApp == bl.poll(15'100ms).value()); // All subsequent calls yield the same Final. // NEW APPLICATION IS NOW AVAILABLE ai = *bl.getAppInfo(); diff --git a/tests/unit/test_presenter.cpp b/tests/unit/test_presenter.cpp index 653159d..1ce5ff3 100644 --- a/tests/unit/test_presenter.cpp +++ b/tests/unit/test_presenter.cpp @@ -87,9 +87,11 @@ TEST_CASE("Presenter") // NOLINT NOSONAR complexity threshold coa.data(), }; - MockController controller; - std::array nodes; - kocherga::detail::Presenter pres(sys_info, controller); + MockController controller; + std::array nodes; + kocherga::detail::Presenter::Params params{}; + params.request_retry_limit = 1; + kocherga::detail::Presenter pres(sys_info, controller, params); REQUIRE(pres.addNode(&nodes.at(0))); REQUIRE(pres.addNode(&nodes.at(1))); REQUIRE(!pres.addNode(&nodes.at(1))); // Double registration has no effect. @@ -358,11 +360,12 @@ TEST_CASE("Presenter") // NOLINT NOSONAR complexity threshold } REQUIRE(!*controller.popFileReadResult()); // Empty option. - // Successful request, but the response times out. + // Successful request, but the response times out after the configured number of retries. + REQUIRE(1 == params.request_retry_limit); // Ensure the configuration matches the expectations. nodes.at(1).setFileReadResult(true); REQUIRE(pres.requestFileRead(123'456)); REQUIRE(!nodes.at(0).popOutput(Node::Output::FileReadRequest)); - REQUIRE((*nodes.at(1).popOutput(Node::Output::FileReadRequest)) == + REQUIRE(nodes.at(1).popOutput(Node::Output::FileReadRequest).value() == Transfer(4, {64, 226, 1, 0, 0, 20, 47, 102, 111, 111, 47, 98, 97, 114, 47, 98, 97, 122, 46, 97, 112, 112, 46, 98, 105, 110}, @@ -381,8 +384,23 @@ TEST_CASE("Presenter") // NOLINT NOSONAR complexity threshold REQUIRE(n.popOutput(Node::Output::HeartbeatMessage)); REQUIRE(n.getLastPollTime() == ts); } + REQUIRE(!nodes.at(0).popOutput(Node::Output::FileReadRequest)); + REQUIRE(nodes.at(1).popOutput(Node::Output::FileReadRequest).value() == + Transfer(5, // transfer-ID incremented + {64, 226, 1, 0, 0, 20, 47, 102, 111, 111, 47, 98, 97, + 114, 47, 98, 97, 122, 46, 97, 112, 112, 46, 98, 105, 110}, + 3210)); + REQUIRE(!controller.popFileReadResult()); + REQUIRE(!nodes.at(0).wasRequestCanceled()); + REQUIRE(!nodes.at(1).wasRequestCanceled()); // Not yet! + ts = std::chrono::microseconds{15'000'000}; pres.poll(ts); - REQUIRE(!*controller.popFileReadResult()); // Empty option. + for (auto& n : nodes) + { + REQUIRE(n.popOutput(Node::Output::HeartbeatMessage)); + REQUIRE(n.getLastPollTime() == ts); + } + REQUIRE(!controller.popFileReadResult().value()); // Empty option. REQUIRE(!nodes.at(0).wasRequestCanceled()); - REQUIRE(nodes.at(1).wasRequestCanceled()); + REQUIRE(nodes.at(1).wasRequestCanceled()); // Given up. } diff --git a/tests/util/util.hpp b/tests/util/util.hpp index c429323..3501286 100644 --- a/tests/util/util.hpp +++ b/tests/util/util.hpp @@ -272,10 +272,7 @@ inline auto getImagePath(const std::string& name) -> std::filesystem::path template inline auto getRandomInteger() -> std::enable_if_t, T> { - static std::random_device rd; - static std::mt19937 gen(rd()); - static std::uniform_int_distribution dis(0, std::numeric_limits::max()); - return static_cast(dis(gen)); + return static_cast((static_cast(std::rand()) * std::numeric_limits::max()) / RAND_MAX); } } // namespace util