511 fix crash on api method

AGENTS.md

@@ -93,6 +93,22 @@
  To run tests, go into `<build-dir>` and run `ctest . --progress -j -E "((sodium)|(hydro)|(bcrypt)).*" --output-on-failure`.
  Or run specific test by name from `<build-dir>/tests/run/<test-name>`.
 
+## Dependencies
+
+- Dependencies are managed by CMake through `cmake/CPM.cmake` and `CPMAddPackage` calls in the root `CMakeLists.txt`.
+- Do not add conan, vcpkg, git submodules, or vendored dependency copies unless explicitly requested.
+- The root dependency list is the source of truth. Test-only dependencies live in `tests/CMakeLists.txt`; tool-only dependencies may live in the tool's own `CMakeLists.txt`.
+- CPM downloads dependencies during CMake configure. If `CPM_SOURCE_CACHE` is not set, the project uses `${CMAKE_CURRENT_BINARY_DIR}/cpm.cache`.
+- Prefer setting the `CPM_SOURCE_CACHE` environment variable for repeated local builds to avoid re-downloading dependencies.
+- To use locally checked-out dependencies, pass `-DCPM_<dependency name>_SOURCE=/absolute/path`.
+- `CPM_USE_LOCAL_PACKAGES` may be used to let CPM try `find_package` before downloading from source.
+- Several dependencies require local patches from `third_party/*.patch`. Keep patch files in sync when changing dependency versions or repositories.
+- Root dependencies should use `EXCLUDE_FROM_ALL FALSE` so they can be installed together with `aether`.
+- Install options are propagated through dependency-specific CMake options such as `ENABLE_INSTALL`, `AE_INSTALL`, `AE_NUMERIC_INSTALL`, and `STDEXEC_INSTALL`.
+- Do not casually update dependency tags pinned to branches such as `master` or `main`; check patches, build behavior, and compatibility first. In particular, `libhydrogen` is pinned to `bbca575` because newer versions are noted as incompatible with the Aether server.
+- Desktop builds additionally fetch and link `c-ares`.
+- ESP-IDF builds do not fetch ESP components through CPM; required IDF targets must already exist: `idf::esp_wifi`, `idf::esp_netif`, `idf::nvs_flash`, `idf::spiffs`, and `idf::esp_driver_uart`.
+
 ### Smoke test
 
  The first smoke test is a `<build-dir>/aether-client-cpp-cloud`.

README.md

@@ -1,5 +1,5 @@
 # Aethernet C++ client
-Run git_init script suitable for particular platform. It clones submodules and applies patches.
+Dependencies are managed by CMake through CPM.cmake. Configure the project with CMake to download dependencies and apply local patches from `third_party`.
 **Projects** folder contains cmake files to build axamples for target platforms.
 VSCode + Platformio + ESP-IDF is a toolset that is currently supported.
 

aether/ae_actions/ae_actions_tele.h

@@ -56,7 +56,7 @@ AE_TAG(kGetClientCloudConnectionServerListIsOver, kAeActions)
 AE_TAG(kPing, kAeActions)
 AE_TAG(kPingSend, kAeActions)
 AE_TAG(kPingWriteError, kAeActions)
-AE_TAG(kPingTimeout, kAeActions)
+AE_TAG(kPingResponseError, kAeActions)
 AE_TAG(kPingTimeoutError, kAeActions)
 
 AE_TAG(TelemetryCreated, kAeActions)

aether/ae_actions/ping.cpp

@@ -18,6 +18,9 @@
 #if AE_ENABLE_PING
 
 #  include <cassert>
+#  include <cstdint>
+#  include <limits>
+#  include <utility>
 
 #  include "aether/server.h"
 
@@ -27,6 +30,35 @@
 #  include "aether/ae_actions/ae_actions_tele.h"
 
 namespace ae {
+namespace {
+
+constexpr int kPingPromiseEvictedError{-1};
+
+constexpr int PromiseErrorCodeToPingErrorCode(
+    std::uint32_t error_code) noexcept {
+  if (error_code == std::numeric_limits<std::uint32_t>::max()) {
+    return kPingPromiseEvictedError;
+  }
+  if (error_code <=
+      static_cast<std::uint32_t>(std::numeric_limits<int>::max())) {
+    return static_cast<int>(error_code);
+  }
+  return kPingPromiseEvictedError;
+}
+
+static_assert(PromiseErrorCodeToPingErrorCode(
+                  std::numeric_limits<std::uint32_t>::max()) ==
+              kPingPromiseEvictedError);
+static_assert(PromiseErrorCodeToPingErrorCode(0) == 0);
+static_assert(PromiseErrorCodeToPingErrorCode(static_cast<std::uint32_t>(
+                  std::numeric_limits<int>::max())) ==
+              std::numeric_limits<int>::max());
+static_assert(PromiseErrorCodeToPingErrorCode(
+                  static_cast<std::uint32_t>(std::numeric_limits<int>::max()) +
+                  1U) == kPingPromiseEvictedError);
+
+}  // namespace
+
 Ping::Ping(AeContext const& ae_context,
            CloudServerConnection& cloud_server_connection,
            Duration next_ping_hint, Duration rx_window, Duration timeout)
@@ -56,8 +88,6 @@ void Ping::Start(TimePoint current_time) {
   }
   started_ = true;
 
-  AE_TELE_DEBUG(kPingSend, "Send ping");
-
   auto& write_action = cc->AuthorizedApiCall(
       SubApi{[this, current_time](ApiContext<AuthorizedApi>& auth_api) {
         auto next_ping_hint_ms = static_cast<std::uint64_t>(
@@ -68,23 +98,28 @@ void Ping::Start(TimePoint current_time) {
             std::chrono::duration_cast<std::chrono::milliseconds>(rx_window_)
                 .count());
 
-        auto& pong_promise = auth_api->ping(next_ping_hint_ms, rx_window_ms);
+        auto pong_promise = auth_api->ping(next_ping_hint_ms, rx_window_ms);
         auto req_id = pong_promise.request_id();
 
-        AE_TELED_DEBUG("Ping server id {}, request {} expected time {:%S}s",
-                       server_id_, req_id, timeout_);
+        AE_TELE_DEBUG(kPingSend,
+                      "Ping server id {}, request {} expected time {}",
+                      server_id_, req_id, timeout_);
 
         request_start_ = current_time;
-        request_id_ = req_id;
 
-        auto wait_result_sub = pong_promise.Subscribe(
-            [this, req_id](auto&&...) { PingResponse(req_id); });
+        auto wait_result_sub =
+            pong_promise.Subscribe([this, req_id](auto&& res) {
+              if (res) {
+                PingResponse(req_id);
+              } else {
+                PingResponseError(req_id, res.error());
+              }
+            });
         wait_result_sub_ = std::move(wait_result_sub);
 
-        auto timeout_sub = ae_context_.scheduler().DelayedTask(
+        timeout_sub_ = ae_context_.scheduler().DelayedTask(
             [this, req_id]() { PingResponseTimeout(req_id); },
             current_time + timeout_);
-        timeout_sub_ = std::move(timeout_sub);
       }});
 
   write_sub_ = write_action.status_event().Subscribe([this](auto status) {
@@ -112,28 +147,29 @@ void Ping::Start(TimePoint current_time) {
 }
 
 void Ping::PingResponse(RequestId request_id) {
-  if (!HasActiveRequest() || request_id_ != request_id) {
-    AE_TELED_WARNING("Got lost, or not our pong response");
-    return;
-  }
-
   auto current_time = Now();
   auto ping_duration =
       std::chrono::duration_cast<Duration>(current_time - request_start_);
 
-  AE_TELED_DEBUG("Ping server id {} request {} received by {:%S} s", server_id_,
+  AE_TELED_DEBUG("Ping server id {} request {} received by {}", server_id_,
                  request_id, ping_duration);
   ResetRequestSubscriptions();
   result_event_.Emit(Ok{ping_duration});
 }
 
-void Ping::PingResponseTimeout(RequestId request_id) {
-  if (!HasActiveRequest() || request_id_ != request_id) {
-    AE_TELED_WARNING("Timeout for lost, or not our pong response");
-    return;
+void Ping::PingResponseError(RequestId request_id, std::int32_t error_code) {
+  AE_TELE_ERROR(kPingResponseError, "Ping server id {} request {} error {}",
+                server_id_, request_id, error_code);
+  ResetRequestSubscriptions();
+  if (error_code == -1) {
+    result_event_.Emit(Error{4});
+  } else {
+    result_event_.Emit(Error{3});
   }
+}
 
-  AE_TELE_ERROR(kPingTimeout, "Ping server id {} request {} timeout",
+void Ping::PingResponseTimeout(RequestId request_id) {
+  AE_TELE_ERROR(kPingTimeoutError, "Ping server id {} request {} timeout",
                 server_id_, request_id);
   ResetRequestSubscriptions();
   result_event_.Emit(Error{2});
@@ -145,9 +181,5 @@ void Ping::ResetRequestSubscriptions() {
   write_sub_.Reset();
 }
 
-bool Ping::HasActiveRequest() const noexcept {
-  return static_cast<bool>(wait_result_sub_);
-}
-
 }  // namespace ae
 #endif  // AE_ENABLE_PING

aether/ae_actions/ping.h

@@ -50,9 +50,9 @@ class Ping {
 
  private:
   void PingResponse(RequestId request_id);
+  void PingResponseError(RequestId request_id, std::int32_t error_code);
   void PingResponseTimeout(RequestId request_id);
   void ResetRequestSubscriptions();
-  bool HasActiveRequest() const noexcept;
 
   AeContext ae_context_;
   CloudServerConnection* cloud_server_connection_;
@@ -61,8 +61,7 @@ class Ping {
   Duration timeout_;
   ServerId server_id_;
 
-  TimePoint request_start_{};
-  RequestId request_id_{};
+  TimePoint request_start_;
   Subscription wait_result_sub_;
   TaskSubscription timeout_sub_;
   Subscription write_sub_;

aether/api_protocol/api_method.h

@@ -17,13 +17,6 @@
 #ifndef AETHER_API_PROTOCOL_API_METHOD_H_
 #define AETHER_API_PROTOCOL_API_METHOD_H_
 
-#include "aether/warning_disable.h"
-
-DISABLE_WARNING_PUSH()
-IGNORE_IMPLICIT_CONVERSION()
-#include <etl/pool.h>
-DISABLE_WARNING_POP()
-
 #include "aether/api_protocol/api_message.h"
 #include "aether/api_protocol/api_context.h"
 #include "aether/api_protocol/api_promise.h"
@@ -60,7 +53,8 @@ struct Method<MessageCode, void(Args...), ArgProc> {
 
   void operator()(Args... args) {
     auto* packet_stack = protocol_context_->packet_stack();
-    assert(packet_stack);
+    assert(packet_stack != nullptr &&
+           "Method::operator() requires active ApiContext");
     packet_stack->Push(*this, arg_proc_(std::forward<Args>(args)...));
   }
 
@@ -77,27 +71,21 @@ struct Method<MessageCode, void(Args...), ArgProc> {
 /**
  * \brief Specialization for method with return value.
  * A GenericMessage generated with request id and list of args.
- * return PromiseView<R> for waiting the result or error.
+ * return ApiPromise<R> for waiting the result or error.
  */
 template <MessageId MessageCode, typename R, typename... Args, typename ArgProc>
 struct Method<MessageCode, ApiPromise<R>(Args...), ArgProc> {
   explicit Method(ProtocolContext& protocol_context, ArgProc arg_proc = {})
       : protocol_context_{&protocol_context}, arg_proc_{std::move(arg_proc)} {}
 
-  ~Method() noexcept {
-    for (auto i = api_promise_pool_.begin(); i != api_promise_pool_.end();
-         ++i) {
-      api_promise_pool_.destroy(&i.template get<ApiPromise<R>>());
-    }
-  }
-
-  ApiPromise<R>& operator()(Args... args) {
+  ApiPromise<R> operator()(Args... args) {
     auto request_id = RequestId::GenRequestId();
     auto* packet_stack = protocol_context_->packet_stack();
-    assert(packet_stack);
+    assert(packet_stack != nullptr &&
+           "Method::operator() requires active ApiContext");
     packet_stack->Push(*this,
                        arg_proc_(request_id, std::forward<Args>(args)...));
-    return UpdateRequestCb(request_id);
+    return ApiPromise<R>{*protocol_context_, request_id};
   }
 
   template <typename... Ts>
@@ -106,37 +94,7 @@ struct Method<MessageCode, ApiPromise<R>(Args...), ArgProc> {
   }
 
  private:
-  ApiPromise<R>& UpdateRequestCb(RequestId request_id) {
-    auto* api_promise = api_promise_pool_.create(request_id);
-    assert(api_promise != nullptr);
-
-    if constexpr (!std::is_same_v<void, R>) {
-      protocol_context_->AddSendResultCallback(
-          request_id, [this, api_promise]() {
-            api_promise->SetValue(
-                protocol_context_->parser()->template Extract<R>());
-
-            api_promise_pool_.destroy(api_promise);
-          });
-    } else {
-      protocol_context_->AddSendResultCallback(
-          request_id, [this, api_promise]() {
-            api_promise->SetValue();
-            api_promise_pool_.destroy(api_promise);
-          });
-    }
-    protocol_context_->AddSendErrorCallback(
-        request_id, [this, api_promise](auto, std::uint32_t err) {
-          api_promise->SetError(std::move(err));
-          api_promise_pool_.destroy(api_promise);
-        });
-
-    return *api_promise;
-  }
-
   ProtocolContext* protocol_context_;
-  // TODO: configure pool size
-  etl::pool<ApiPromise<R>, 10> api_promise_pool_;
   ArgProc arg_proc_;
 };
 
@@ -159,7 +117,8 @@ struct Method<MessageCode, SubContext<Api>(Args...), ArgProc> {
     SubContext context{*api_, *child_stack};
 
     auto* packet_stack = protocol_context_->packet_stack();
-    assert(packet_stack);
+    assert(packet_stack != nullptr &&
+           "Method::operator() requires active ApiContext");
     packet_stack->Push(
         *this, arg_proc_(std::forward<Args>(args)..., std::move(child_stack)));
     return context;