diff --git a/detector_control/DectrisSimplonClient.cpp b/detector_control/DectrisSimplonClient.cpp index 1bf811fd9..f079b0073 100644 --- a/detector_control/DectrisSimplonClient.cpp +++ b/detector_control/DectrisSimplonClient.cpp @@ -21,8 +21,18 @@ std::string to_string(SimplonState state) { } DectrisSimplonClient::DectrisSimplonClient(const std::string &addr, uint16_t in_port) -: hostname(addr), port(in_port), http(std::make_unique(addr, in_port)) { - http->set_keep_alive(true); +: hostname(addr), port(in_port), + cmd_http(std::make_unique(addr, in_port)), + status_http(std::make_unique(addr, in_port)) { + cmd_http->set_keep_alive(true); + cmd_http->set_connection_timeout(std::chrono::seconds(1)); + cmd_http->set_read_timeout(std::chrono::minutes(20)); // Initialize or disarm might take a lot of time + cmd_http->set_write_timeout(std::chrono::minutes(20)); // Initialize or disarm might take a lot of time + + status_http->set_keep_alive(true); + status_http->set_connection_timeout(std::chrono::milliseconds(500)); + status_http->set_read_timeout(std::chrono::seconds(1)); + status_http->set_write_timeout(std::chrono::seconds(1)); } DectrisSimplonClient::~DectrisSimplonClient() = default; @@ -48,12 +58,8 @@ void DectrisSimplonClient::SendDetectorCommand(SimplonDetectorCommand cmd) { } auto addr = GenAddr(SimplonModule::Detector, SimplonTask::Command, key); - std::lock_guard lg(http_mutex); - http->set_connection_timeout(std::chrono::milliseconds(500)); - http->set_read_timeout(std::chrono::minutes(20)); // Initialize or disarm might take a lot of time - http->set_write_timeout(std::chrono::minutes(20)); // Initialize or disarm might take a lot of time - - auto res = http->Post(addr); + std::lock_guard lg(cmd_mutex); + auto res = cmd_http->Post(addr); if (!res || res->status != httplib::StatusCode::OK_200) { std::string err_msg = "Command failed " + key; @@ -74,12 +80,8 @@ void DectrisSimplonClient::SetConfig(SimplonModule element, const std::string &k nlohmann::json j; j["value"] = value; - std::lock_guard lg(http_mutex); - http->set_connection_timeout(std::chrono::seconds(1)); - http->set_read_timeout(std::chrono::minutes(20)); // Initialize or disarm might take a lot of time - http->set_write_timeout(std::chrono::minutes(20)); // Initialize or disarm might take a lot of time - - auto res = http->Post(addr, j.dump(), "application/json"); + std::lock_guard lg(cmd_mutex); + auto res = cmd_http->Post(addr, j.dump(), "application/json"); if (!res || res->status != httplib::StatusCode::OK_200) { std::string err_msg = "Configure failed " + key; @@ -142,11 +144,8 @@ std::string DectrisSimplonClient::GenAddr(SimplonModule element, SimplonTask tas SimplonConfig DectrisSimplonClient::GetConfig(SimplonModule element, SimplonTask task, const std::string &key) { - std::lock_guard lg(http_mutex); - http->set_connection_timeout(std::chrono::milliseconds(500)); - http->set_read_timeout(std::chrono::seconds(1)); - http->set_write_timeout(std::chrono::seconds(1)); - auto res = http->Get(GenAddr(element, task, key)); + std::lock_guard lg(status_mutex); + auto res = status_http->Get(GenAddr(element, task, key)); if (!res) { throw JFJochException(JFJochExceptionCategory::SimplonError, diff --git a/detector_control/DectrisSimplonClient.h b/detector_control/DectrisSimplonClient.h index 15e50dffb..4fabee085 100644 --- a/detector_control/DectrisSimplonClient.h +++ b/detector_control/DectrisSimplonClient.h @@ -29,11 +29,15 @@ class DectrisSimplonClient { std::string hostname; uint16_t port; - // One connection, kept alive and reused for every request. httplib::Client cannot run requests + // Two connections, each kept alive and reused: one for commands and configuration, which can take + // minutes (initialize, disarm, threshold change), and one for reading status and configuration, + // so that status polls are never stuck behind a long command. httplib::Client cannot run requests // from several threads at once (status polling, cancel and the HTTP status endpoint can overlap), - // so each request holds http_mutex. - std::mutex http_mutex; - std::unique_ptr http; + // so each request holds the mutex of its connection. + std::mutex cmd_mutex; + std::unique_ptr cmd_http; + std::mutex status_mutex; + std::unique_ptr status_http; enum class SimplonModule {Detector, Stream, Filewriter, Monitor}; enum class SimplonTask {Config, Command, Status}; diff --git a/tests/DectrisSimplonClientTest.cpp b/tests/DectrisSimplonClientTest.cpp index 9e4c21869..d4c473681 100644 --- a/tests/DectrisSimplonClientTest.cpp +++ b/tests/DectrisSimplonClientTest.cpp @@ -4,13 +4,16 @@ #include #include #include +#include +#include #include #include #include #include "../detector_control/DectrisSimplonClient.h" -// A minimal SIMPLON server: the detector state and error list, and a disarm command that fails. +// A minimal SIMPLON server: the detector state and error list, a disarm command that fails and a +// cancel command that takes 2 s. TEST_CASE("DectrisSimplonClient_MockServer", "[detector]") { httplib::Server server; std::mutex m; @@ -34,6 +37,13 @@ TEST_CASE("DectrisSimplonClient_MockServer", "[detector]") { res.set_content("disarm refused", "text/plain"); }); + std::atomic cancel_running = false; + server.Post("/detector/api/1.8.0/command/cancel", [&](const httplib::Request &, httplib::Response &res) { + cancel_running = true; + std::this_thread::sleep_for(std::chrono::seconds(2)); + res.set_content("", "text/plain"); + }); + const int port = server.bind_to_any_port("127.0.0.1"); REQUIRE(port > 0); struct ServerThread { @@ -53,6 +63,15 @@ TEST_CASE("DectrisSimplonClient_MockServer", "[detector]") { CHECK(client.GetErrorStatus() == R"(["overheating"])"); + // A long command does not hold up status polls + std::thread cancel([&client] { client.CancelAcquisition(); }); + while (!cancel_running) + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + auto t0 = std::chrono::steady_clock::now(); + CHECK(client.GetState() == SimplonState::Idle); + CHECK(std::chrono::steady_clock::now() - t0 < std::chrono::milliseconds(500)); + cancel.join(); + // Failures carry the HTTP status and the body the DCU sent { std::lock_guard lg(m);