diff --git a/broker/JFJochServices.cpp b/broker/JFJochServices.cpp index 0cca548e7..8eb9edb96 100644 --- a/broker/JFJochServices.cpp +++ b/broker/JFJochServices.cpp @@ -140,15 +140,17 @@ JFJochServicesOutput JFJochServices::Stop() { DetectorState state = detector->GetState(); while ((!cannot_stop_detector) && ((state == DetectorState::WAITING) || (state == DetectorState::BUSY))) { - // check detector state every 5 ms - std::this_thread::sleep_for(std::chrono::milliseconds(5)); + // check detector state every 100 ms - every check is an HTTP request for DECTRIS + std::this_thread::sleep_for(std::chrono::milliseconds(100)); state = detector->GetState(); } if (state == DetectorState::IDLE) { logger.Info(" ... detector idle"); receiver->Cancel(true); // cancel silently } else { - logger.Error(" ... detector in error state"); + logger.Error(" ... detector not idle: state {}{}; {}", to_string(state), + cannot_stop_detector ? " (stop command failed)" : "", + detector->GetErrorDescription()); receiver->Cancel(false); detector_error = true; } @@ -207,6 +209,9 @@ void JFJochServices::Cancel() { // Best effort - if detector cannot be stopped, this is OK, important to still stop receiver try { detector->Stop(); + } catch (const std::exception &e) { + logger.Error("Cannot stop detector: {}", e.what()); + cannot_stop_detector = true; } catch (...) { cannot_stop_detector = true; } diff --git a/detector_control/DectrisDetectorWrapper.cpp b/detector_control/DectrisDetectorWrapper.cpp index dff0cb185..3195b2ab4 100644 --- a/detector_control/DectrisDetectorWrapper.cpp +++ b/detector_control/DectrisDetectorWrapper.cpp @@ -11,6 +11,7 @@ DetectorState DectrisDetectorWrapper::GetState() const { try { simplon_state = simplon->GetState(); } catch (const JFJochException &e) { + logger.Warning("Cannot read detector state: {}", e.what()); return DetectorState::NOT_CONNECTED; } @@ -121,3 +122,20 @@ void DectrisDetectorWrapper::LoadPixelMask(PixelMask &mask) { if (simplon) mask.LoadDECTRISBadPixelMask(simplon->GetPixelMask()); } + +std::string DectrisDetectorWrapper::GetErrorDescription() const { + if (!simplon) + return {}; + std::string ret; + try { + ret = "SIMPLON state " + to_string(simplon->GetState()); + } catch (const JFJochException &e) { + ret = std::string("SIMPLON state unreadable (") + e.what() + ")"; + } + try { + ret += ", errors " + simplon->GetErrorStatus(); + } catch (const JFJochException &e) { + ret += std::string(", errors unreadable (") + e.what() + ")"; + } + return ret; +} diff --git a/detector_control/DectrisDetectorWrapper.h b/detector_control/DectrisDetectorWrapper.h index 6217429ba..a9cdfde9d 100644 --- a/detector_control/DectrisDetectorWrapper.h +++ b/detector_control/DectrisDetectorWrapper.h @@ -5,9 +5,11 @@ #include "DetectorWrapper.h" #include "DectrisSimplonClient.h" +#include "../common/Logger.h" class DectrisDetectorWrapper : public DetectorWrapper { std::unique_ptr simplon; + mutable Logger logger{"DectrisDetectorWrapper"}; void CheckBusy(); void CheckBusyOrError(); public: @@ -24,6 +26,7 @@ public: DetectorState GetState() const override; DetectorStatus GetStatus() const override; + std::string GetErrorDescription() const override; }; diff --git a/detector_control/DectrisSimplonClient.cpp b/detector_control/DectrisSimplonClient.cpp index 0da7b7c9d..1bf811fd9 100644 --- a/detector_control/DectrisSimplonClient.cpp +++ b/detector_control/DectrisSimplonClient.cpp @@ -21,10 +21,12 @@ std::string to_string(SimplonState state) { } DectrisSimplonClient::DectrisSimplonClient(const std::string &addr, uint16_t in_port) -: hostname(addr), port(in_port) { - +: hostname(addr), port(in_port), http(std::make_unique(addr, in_port)) { + http->set_keep_alive(true); } +DectrisSimplonClient::~DectrisSimplonClient() = default; + void DectrisSimplonClient::SendDetectorCommand(SimplonDetectorCommand cmd) { std::string key; switch (cmd) { @@ -46,16 +48,18 @@ void DectrisSimplonClient::SendDetectorCommand(SimplonDetectorCommand cmd) { } auto addr = GenAddr(SimplonModule::Detector, SimplonTask::Command, key); - httplib::Client cli_cmd(hostname, port); - cli_cmd.set_connection_timeout(std::chrono::milliseconds(500)); - cli_cmd.set_read_timeout(std::chrono::minutes(20)); // Initialize or disarm might take a lot of time - cli_cmd.set_write_timeout(std::chrono::minutes(20)); // Initialize or disarm might take a lot of time + 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 = cli_cmd.Post(addr); + auto res = http->Post(addr); if (!res || res->status != httplib::StatusCode::OK_200) { std::string err_msg = "Command failed " + key; - if (res) { + if (!res) + err_msg += "; " + to_string(res.error()); + else { err_msg += "; HTTP" + std::to_string(res->status); if (!res->body.empty()) err_msg += "; " + res->body; @@ -70,16 +74,18 @@ void DectrisSimplonClient::SetConfig(SimplonModule element, const std::string &k nlohmann::json j; j["value"] = value; - httplib::Client cli_cmd(hostname, port); - cli_cmd.set_connection_timeout(std::chrono::seconds(1)); - cli_cmd.set_read_timeout(std::chrono::minutes(20)); // Initialize or disarm might take a lot of time - cli_cmd.set_write_timeout(std::chrono::minutes(20)); // Initialize or disarm might take a lot of time + 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 = cli_cmd.Post(addr, j.dump(), "application/json"); + auto res = http->Post(addr, j.dump(), "application/json"); if (!res || res->status != httplib::StatusCode::OK_200) { std::string err_msg = "Configure failed " + key; - if (res) { + if (!res) + err_msg += "; " + to_string(res.error()); + else { err_msg += "; HTTP" + std::to_string(res->status); if (!res->body.empty()) err_msg += "; " + res->body; @@ -136,11 +142,11 @@ std::string DectrisSimplonClient::GenAddr(SimplonModule element, SimplonTask tas SimplonConfig DectrisSimplonClient::GetConfig(SimplonModule element, SimplonTask task, const std::string &key) { - httplib::Client cli_simple(hostname, port); - cli_simple.set_connection_timeout(std::chrono::milliseconds(500)); - cli_simple.set_read_timeout(std::chrono::seconds(1)); - cli_simple.set_write_timeout(std::chrono::seconds(1)); - auto res = cli_simple.Get(GenAddr(element, task, 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)); if (!res) { throw JFJochException(JFJochExceptionCategory::SimplonError, @@ -148,7 +154,8 @@ DectrisSimplonClient::GetConfig(SimplonModule element, SimplonTask task, const s } if (res->status != httplib::StatusCode::OK_200) { throw JFJochException(JFJochExceptionCategory::SimplonError, - "Get key " + key + " failed: " + to_string(res.error())); + "Get key " + key + " failed: HTTP" + std::to_string(res->status) + + (res->body.empty() ? "" : "; " + res->body)); } try { @@ -319,6 +326,11 @@ SimplonState DectrisSimplonClient::GetState() { "State unknown " + res); } +// List of error messages the detector reports (SIMPLON detector status "error"), as JSON text +std::string DectrisSimplonClient::GetErrorStatus() { + return GetStatus(SimplonModule::Detector, "error").dump(); +} + float DectrisSimplonClient::GetHumidity() { return GetStatus(SimplonModule::Detector, "humidity"); } diff --git a/detector_control/DectrisSimplonClient.h b/detector_control/DectrisSimplonClient.h index 68340e90a..15e50dffb 100644 --- a/detector_control/DectrisSimplonClient.h +++ b/detector_control/DectrisSimplonClient.h @@ -4,6 +4,7 @@ #pragma once #include +#include #include #include "../common/DetectorSetup.h" @@ -22,10 +23,18 @@ enum class SimplonDetectorCommand {Initialize, Arm, Disarm, Trigger, Cancel}; std::string to_string(SimplonState state); +namespace httplib { class Client; } + class DectrisSimplonClient { std::string hostname; uint16_t port; + // One connection, kept alive and reused for every request. 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; + enum class SimplonModule {Detector, Stream, Filewriter, Monitor}; enum class SimplonTask {Config, Command, Status}; @@ -44,6 +53,7 @@ class DectrisSimplonClient { void SendDetectorCommand(SimplonDetectorCommand cmd); public: explicit DectrisSimplonClient(const std::string& hostname, uint16_t port = 80); + ~DectrisSimplonClient(); void ReadDetectorConfig(DetectorSetup &setup); void ConfigureDetector(const DiffractionExperiment& experiment); void StartAcquisition(const DiffractionExperiment& experiment); @@ -53,6 +63,7 @@ public: void InitializeDetector(DetectorSetup& setup); SimplonState GetState(); + std::string GetErrorStatus(); float GetHumidity(); float GetTemperature(); diff --git a/detector_control/DetectorWrapper.h b/detector_control/DetectorWrapper.h index d8fb18162..f685e6752 100644 --- a/detector_control/DetectorWrapper.h +++ b/detector_control/DetectorWrapper.h @@ -12,6 +12,17 @@ enum class DetectorState {IDLE, ERROR, BUSY, WAITING, NOT_CONNECTED}; enum class DetectorPowerState {ON, OFF, PARTIAL}; +inline std::string to_string(DetectorState state) { + switch (state) { + case DetectorState::IDLE: return "idle"; + case DetectorState::ERROR: return "error"; + case DetectorState::BUSY: return "busy"; + case DetectorState::WAITING: return "waiting"; + case DetectorState::NOT_CONNECTED: return "not connected"; + default: return "unknown"; + } +} + struct DetectorStatus { std::vector temperature_fpga_degC; std::vector high_voltage_V; @@ -35,6 +46,8 @@ public: virtual void LoadPixelMask(PixelMask &mask) = 0; [[nodiscard]] virtual DetectorStatus GetStatus() const = 0; + // What the detector itself says about a fault, for the log; empty if it has nothing to add + [[nodiscard]] virtual std::string GetErrorDescription() const { return {}; } }; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 2283f4395..a8e0468d1 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -173,6 +173,7 @@ ADD_EXECUTABLE(jfjoch_test BeamCenterFFTGPUTest.cpp BearerTokensTest.cpp BrokerHttpAuthTest.cpp + DectrisSimplonClientTest.cpp ../broker/JFJochBrokerHttp.cpp ) diff --git a/tests/DectrisSimplonClientTest.cpp b/tests/DectrisSimplonClientTest.cpp new file mode 100644 index 000000000..9e4c21869 --- /dev/null +++ b/tests/DectrisSimplonClientTest.cpp @@ -0,0 +1,66 @@ +// SPDX-FileCopyrightText: 2026 Filip Leonarski, Paul Scherrer Institute +// SPDX-License-Identifier: GPL-3.0-only + +#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. +TEST_CASE("DectrisSimplonClient_MockServer", "[detector]") { + httplib::Server server; + std::mutex m; + std::set client_ports; + std::string state = "idle"; + + server.Get("/detector/api/1.8.0/status/state", [&](const httplib::Request &req, httplib::Response &res) { + std::lock_guard lg(m); + client_ports.insert(req.remote_port); + if (state == "unavailable") { + res.status = 503; + res.set_content("DCU busy", "text/plain"); + } else + res.set_content(nlohmann::json{{"value", state}}.dump(), "application/json"); + }); + server.Get("/detector/api/1.8.0/status/error", [&](const httplib::Request &, httplib::Response &res) { + res.set_content(nlohmann::json{{"value", {"overheating"}}}.dump(), "application/json"); + }); + server.Post("/detector/api/1.8.0/command/disarm", [&](const httplib::Request &, httplib::Response &res) { + res.status = 500; + res.set_content("disarm refused", "text/plain"); + }); + + const int port = server.bind_to_any_port("127.0.0.1"); + REQUIRE(port > 0); + struct ServerThread { + httplib::Server &server; + std::thread thread; + explicit ServerThread(httplib::Server &s) : server(s), thread([&s] { s.listen_after_bind(); }) {} + ~ServerThread() { server.stop(); thread.join(); } + } server_thread(server); + server.wait_until_ready(); + + DectrisSimplonClient client("127.0.0.1", port); + + // Repeated polling reuses one connection instead of opening a new one per request + for (int i = 0; i < 10; i++) + REQUIRE(client.GetState() == SimplonState::Idle); + CHECK(client_ports.size() == 1); + + CHECK(client.GetErrorStatus() == R"(["overheating"])"); + + // Failures carry the HTTP status and the body the DCU sent + { + std::lock_guard lg(m); + state = "unavailable"; + } + REQUIRE_THROWS_WITH(client.GetState(), Catch::Matchers::ContainsSubstring("HTTP503") + && Catch::Matchers::ContainsSubstring("DCU busy")); + REQUIRE_THROWS_WITH(client.EndAcquisitionFinished(), Catch::Matchers::ContainsSubstring("HTTP500") + && Catch::Matchers::ContainsSubstring("disarm refused")); +} +