diff --git a/modules/core_geocaching/include/geocaching/usecase/query_client.h b/modules/core_geocaching/include/geocaching/usecase/query_client.h index 80d868ed..ba35d9e5 100644 --- a/modules/core_geocaching/include/geocaching/usecase/query_client.h +++ b/modules/core_geocaching/include/geocaching/usecase/query_client.h @@ -152,6 +152,14 @@ class QueryClient } bool hasMore() const { return phase_ == QueryClientPhase::PageReady && cursor_size_ != 0; } + bool pendingRequest(Destination& destination, RequestId& id) const + { + if (!selected_ || (phase_ != QueryClientPhase::CheckingCapabilities && phase_ != QueryClientPhase::Querying)) return false; + destination = selected_->delivery; + id = request_; + return true; + } + // A live request is already identified in memory. Historical receipt // lookups are only needed for retries after it has left the waiting state. bool expectsResponse(const Destination& source, const RequestId& id) const diff --git a/modules/core_geocaching/tests/test_query_client.cpp b/modules/core_geocaching/tests/test_query_client.cpp index ebc0792b..f48325fe 100644 --- a/modules/core_geocaching/tests/test_query_client.cpp +++ b/modules/core_geocaching/tests/test_query_client.cpp @@ -44,7 +44,12 @@ int main(int argc, char** argv) app.insert(app.end(), {0, 0xa1, 'x'}); assert(client->observe(discovery, delivery, {key.data(), 64}, {app.data(), app.size()}, 0)); assert(client->query({300000000, 1200000000, 310000000, 1210000000})); + geocaching::Destination pending_destination; + geocaching::RequestId pending_id; + assert(!client->pendingRequest(pending_destination, pending_id)); assert(client->tick(0) && port.requests == 1); + assert(client->pendingRequest(pending_destination, pending_id) && pending_destination.bytes == delivery.bytes); + assert(pending_id.bytes[0] == 1); geocaching::RequestId active; active.bytes.fill(1); assert(client->expectsResponse(delivery, active)); @@ -64,4 +69,5 @@ int main(int argc, char** argv) assert(port.requests == 2 && port.pages == 1 && client->phase() == geocaching::QueryClientPhase::PageReady); active.bytes.fill(3); assert(!client->expectsResponse(delivery, active)); + assert(!client->pendingRequest(pending_destination, pending_id)); } diff --git a/modules/core_geocaching/tests/test_sd_journal.cpp b/modules/core_geocaching/tests/test_sd_journal.cpp index c6834099..9e190d7d 100644 --- a/modules/core_geocaching/tests/test_sd_journal.cpp +++ b/modules/core_geocaching/tests/test_sd_journal.cpp @@ -274,15 +274,18 @@ int checkIncrementalDispatch() return SdRequestStore::expireOneAttempt(now, started, timeout, expired); } }; - for (unsigned scenario = 0; scenario < 8; ++scenario) + for (unsigned scenario = 0; scenario < 16; ++scenario) { const auto mode = scenario % 4; + const bool foreground = scenario >= 8; + const std::array preferred{}; fixture::files.clear(); fixture::flush_ok = true; uint8_t first[2048]{}, second[2048]{}, request[128]{}; ::geocaching::storage::LogicalState state(first, second, sizeof(first)); auto store = std::make_unique(fixture::volume, 0, state); - store->selection_wait = store->send_wait = scenario >= 4 ? 3 : 0; + store->send_wait = scenario % 8 >= 4 ? 3 : 0; + store->selection_wait = foreground ? 0 : store->send_wait; ::geocaching::RequestId id; size_t size = 0; if (!::geocaching::protocol::encodeCapabilitiesRequest(id, request, sizeof(request), size) || @@ -304,7 +307,8 @@ int checkIncrementalDispatch() fixture::step_io_calls = fixture::step_data_calls = 0; fixture::step_data_bytes = 0; const auto old_sends = router.sends; - const auto result = dispatcher->dispatchOne({}); + const auto result = dispatcher->dispatchOne({}, foreground ? ::geocaching::ByteView{preferred.data(), preferred.size()} : ::geocaching::ByteView{}); + if (foreground && store->expiration_calls) return 242; if (store->selection_wait && (router.sends || store->expiration_calls != 1)) return 240; if (store->send_wait && router.sends) return 241; if (!stepBudgetOk()) return 112; @@ -313,6 +317,13 @@ int checkIncrementalDispatch() { if (router.sends != 1 || store->committedSequence() != 3 || router.sent_bytes.size() != size || std::memcmp(router.sent_bytes.data(), request, size)) return 114; + // A repeated hint after submission must not start another + // attempt against the already in-flight durable request. + if (foreground) + { + const auto repeated = dispatcher->dispatchOne({}, {preferred.data(), preferred.size()}); + if (repeated.status == DispatchStatus::StorageBlocked || router.sends != 1 || store->committedSequence() != 3) return 243; + } done = true; break; } diff --git a/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/request_dispatcher.h b/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/request_dispatcher.h index 9986a97a..3578c1d9 100644 --- a/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/request_dispatcher.h +++ b/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/request_dispatcher.h @@ -25,7 +25,9 @@ class RequestDispatcher public: RequestDispatcher(chat::MeshAdapterRouter& router, RequestDispatchStore& store, uint32_t retry_delay_ms, uint64_t attempt_timeout_ms); - DispatchResult dispatchOne(const ::geocaching::storage::StoredTime& now); + // Optional key of a newly committed foreground request. It bypasses only + // history selection; beginAttempt/readForSend still validate durable state. + DispatchResult dispatchOne(const ::geocaching::storage::StoredTime& now, ::geocaching::ByteView preferred = {}); private: enum class Phase : uint8_t @@ -51,6 +53,8 @@ class RequestDispatcher std::array boot_{}; std::array cursor_{}; bool has_cursor_ = false; + std::array preferred_{}; + bool has_preferred_ = false; }; -static_assert(sizeof(RequestDispatcher) <= 192, "Dispatcher retains metadata only, not a request-sized buffer"); +static_assert(sizeof(RequestDispatcher) <= 256, "Dispatcher retains metadata only, not a request-sized buffer"); } // namespace platform::esp::arduino_common::geocaching diff --git a/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/sd_index_lookup.h b/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/sd_index_lookup.h index 856904db..61c54c59 100644 --- a/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/sd_index_lookup.h +++ b/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/sd_index_lookup.h @@ -53,7 +53,7 @@ class SdIndexLookup if (inspectSdVolume(current) != SdVolumeResult::Ready) return fail(IndexLookupStep::IoError); if (current != volume_) return fail(IndexLookupStep::VolumeChanged); if (phase_ == Phase::VerifyVolume) return result_ = completion_; - phase_ = length_ ? Phase::Probe : Phase::VerifyVolume; + phase_ = length_ ? Phase::Open : Phase::VerifyVolume; return result_; } if (phase_ == Phase::Probe || phase_ == Phase::Open) @@ -64,12 +64,18 @@ class SdIndexLookup { const auto probe = storage::sd_read_file(path, bytes_.data(), 1); if (probe.status == storage::SdFileReadStatus::Missing) return fail(IndexLookupStep::Invalid); - if (probe.status != storage::SdFileReadStatus::Ready && probe.status != storage::SdFileReadStatus::Invalid) return fail(IndexLookupStep::IoError); - if (probe.file_size < length_) return fail(IndexLookupStep::Invalid); - phase_ = Phase::Open; - return result_; + // Only classify an unsuccessful open. A present file that + // could not be opened is an I/O error, never an empty index. + return fail(IndexLookupStep::IoError); } - if (!file_.open(path, "r")) return fail(IndexLookupStep::IoError); + // Avoid the old one-byte whole-file probe: normal shards always + // exceeded it, logged Invalid, then had to be opened a second time. + phase_ = file_.open(path, "r") ? Phase::CheckLength : Phase::Probe; + return result_; + } + if (phase_ == Phase::CheckLength) + { + if (file_.size() < length_) return fail(IndexLookupStep::Invalid); phase_ = Phase::Seek; return result_; } @@ -129,6 +135,7 @@ class SdIndexLookup Volume, Probe, Open, + CheckLength, Seek, Read, CheckSize, diff --git a/platform/esp/arduino_common/src/geocaching/browse_runtime.cpp b/platform/esp/arduino_common/src/geocaching/browse_runtime.cpp index cab1e39f..fe33689e 100644 --- a/platform/esp/arduino_common/src/geocaching/browse_runtime.cpp +++ b/platform/esp/arduino_common/src/geocaching/browse_runtime.cpp @@ -2373,7 +2373,17 @@ void step() next_step.store(millis() + 1000); return; } - const auto sent = s.dispatcher->dispatchOne(now(nullptr)); + gc::Destination destination; + gc::RequestId request; + std::array preferred{}; + const bool foreground = s.client->pendingRequest(destination, request); + if (foreground) + { + std::memcpy(preferred.data(), s.local.bytes.data(), 16); + std::memcpy(preferred.data() + 16, destination.bytes.data(), 16); + std::memcpy(preferred.data() + 32, request.bytes.data(), 16); + } + const auto sent = s.dispatcher->dispatchOne(now(nullptr), foreground ? gc::ByteView{preferred.data(), preferred.size()} : gc::ByteView{}); if (sent.status == DispatchStatus::StorageBlocked || sent.status == DispatchStatus::Corrupt) fail("Query storage is blocked"); else if (!s.dispatch_store->busy()) next_step.store(millis() + 250); diff --git a/platform/esp/arduino_common/src/geocaching/request_dispatcher.cpp b/platform/esp/arduino_common/src/geocaching/request_dispatcher.cpp index 02da12ee..f82a77e3 100644 --- a/platform/esp/arduino_common/src/geocaching/request_dispatcher.cpp +++ b/platform/esp/arduino_common/src/geocaching/request_dispatcher.cpp @@ -7,7 +7,7 @@ RequestDispatcher::RequestDispatcher(chat::MeshAdapterRouter& router, RequestDis uint32_t retry_delay_ms, uint64_t attempt_timeout_ms) : router_(router), store_(store), retry_delay_ms_(retry_delay_ms), attempt_timeout_ms_(attempt_timeout_ms) {} -DispatchResult RequestDispatcher::dispatchOne(const ::geocaching::storage::StoredTime& now) +DispatchResult RequestDispatcher::dispatchOne(const ::geocaching::storage::StoredTime& now, ::geocaching::ByteView preferred) { using namespace ::geocaching::storage; if (store_.needsRecovery()) return {DispatchStatus::StorageBlocked}; @@ -15,6 +15,24 @@ DispatchResult RequestDispatcher::dispatchOne(const ::geocaching::storage::Store { not_before_ = now.monotonic_ms > UINT64_MAX - retry_delay_ms_ ? UINT64_MAX : now.monotonic_ms + retry_delay_ms_; }; + const auto begin = [&](const ::geocaching::Destination& local) -> DispatchResult + { + esp_fill_random(attempt_id_.data(), attempt_id_.size()); + const auto begun = store_.beginAttempt(local, {cursor_.data(), cursor_.size()}, attempt_id_, now); + if (begun == JournalWriteResult::InProgress) + { + phase_ = Phase::BeginCommit; + return {DispatchStatus::Deferred}; + } + if (begun == JournalWriteResult::Verified) + { + phase_ = Phase::Send; + return {DispatchStatus::Deferred}; + } + return {begun == JournalWriteResult::StateRejected || begun == JournalWriteResult::Busy + ? DispatchStatus::Deferred + : DispatchStatus::StorageBlocked}; + }; if (phase_ != Phase::Select && phase_ != Phase::SelectRequest && phase_ != Phase::Send) { const auto result = store_.stepCommit(); @@ -82,11 +100,36 @@ DispatchResult RequestDispatcher::dispatchOne(const ::geocaching::storage::Store boot_ = now.boot_id; not_before_ = 0; has_cursor_ = false; + has_preferred_ = false; recovery_started_ms_ = now.monotonic_ms; clock_initialized_ = true; } if (phase_ == Phase::Select) { + // A background scan may already have selected the same request before + // the caller supplied its hint. Do not reserve it a second time. + if (preferred.data && preferred.size == preferred_.size() && has_cursor_ && + !std::memcmp(preferred.data, cursor_.data(), cursor_.size())) + { + preferred_ = cursor_; + has_preferred_ = true; + } + if (preferred.data && preferred.size == preferred_.size() && + (!has_preferred_ || std::memcmp(preferred.data, preferred_.data(), preferred_.size()))) + { + ::geocaching::Destination local; + if (!router_.getGeocachingDispatchDestination(local.bytes.data()) || std::memcmp(preferred.data, local.bytes.data(), 16)) + return {DispatchStatus::Deferred, chat::MeshOperationFailure::NotReady}; + std::memcpy(cursor_.data(), preferred.data, cursor_.size()); + has_cursor_ = true; + const auto result = begin(local); + if (phase_ == Phase::BeginCommit || phase_ == Phase::Send) + { + preferred_ = cursor_; + has_preferred_ = true; + } + return result; + } bool expired = false; const auto expiration = store_.expireOneAttempt(now, recovery_started_ms_, attempt_timeout_ms_, expired); if (expiration == JournalWriteResult::Busy) return {DispatchStatus::Deferred}; @@ -121,20 +164,6 @@ DispatchResult RequestDispatcher::dispatchOne(const ::geocaching::storage::Store if (selected == DispatchReadResult::None) return {DispatchStatus::Idle}; cursor_ = pending.key; has_cursor_ = true; - esp_fill_random(attempt_id_.data(), attempt_id_.size()); - const auto begun = store_.beginAttempt(local, {cursor_.data(), cursor_.size()}, attempt_id_, now); - if (begun == JournalWriteResult::InProgress) - { - phase_ = Phase::BeginCommit; - return {DispatchStatus::Deferred}; - } - if (begun == JournalWriteResult::Verified) - { - phase_ = Phase::Send; - return {DispatchStatus::Deferred}; - } - return {begun == JournalWriteResult::StateRejected || begun == JournalWriteResult::Busy - ? DispatchStatus::Deferred - : DispatchStatus::StorageBlocked}; + return begin(local); } } // namespace platform::esp::arduino_common::geocaching