refactoring copied from developer, missing to pass tests in 10.1.0 (#1570)

slsDetectorSoftware/tests/CMakeLists.txt

@@ -6,6 +6,7 @@ target_sources(tests PRIVATE
     ${CMAKE_CURRENT_SOURCE_DIR}/test-SharedMemory.cpp
 
     ${CMAKE_CURRENT_SOURCE_DIR}/acquire/Acquire.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/acquire/CTBState.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/acquire/ExpectedState.cpp
 
     ${CMAKE_CURRENT_SOURCE_DIR}/Caller/test-Caller.cpp

slsDetectorSoftware/tests/Caller/test-Caller-global.cpp

@@ -36,28 +36,34 @@ void test_valid_port_caller(const std::string &command,
 }
 
 void test_dac_caller(defs::dacIndex index, const std::string &dacname,
-                     int dacvalue) {
+                     int dacvalue, bool mV) {
     Detector det;
     Caller caller(&det);
-    std::ostringstream oss_set, oss_get;
-    auto dacstr = std::to_string(dacvalue);
+    std::string dac = dacname;
+    auto value = std::to_string(dacvalue);
     auto previous = det.getDAC(index, false);
     // chip test board
     if (dacname == "dac") {
-        auto dacIndexstr = std::to_string(static_cast<int>(index));
-        caller.call(dacname, {dacIndexstr, dacstr}, -1, PUT, oss_set);
-        REQUIRE(oss_set.str() ==
-                dacname + " " + dacIndexstr + " " + dacstr + "\n");
-        caller.call(dacname, {dacIndexstr}, -1, GET, oss_get);
-        REQUIRE(oss_get.str() ==
-                dacname + " " + dacIndexstr + " " + dacstr + "\n");
+        dac = std::to_string(static_cast<int>(index));
     }
-    // other detectors
-    else {
-        caller.call("dac", {dacname, dacstr}, -1, PUT, oss_set);
-        REQUIRE(oss_set.str() == "dac " + dacname + " " + dacstr + "\n");
-        caller.call("dac", {dacname}, -1, GET, oss_get);
-        REQUIRE(oss_get.str() == "dac " + dacname + " " + dacstr + "\n");
+    {
+        std::ostringstream oss;
+        std::vector<std::string> args = {dac, value};
+        if (mV)
+            args.push_back("mV");
+        std::cout << "args:" << ToString(args) << std::endl;
+        caller.call("dac", args, -1, PUT, oss);
+        REQUIRE(oss.str() == std::string("dac ") + dac + " " + value +
+                                 (mV ? " mV\n" : "\n"));
+    }
+    {
+        std::ostringstream oss;
+        std::vector<std::string> args = {dac};
+        if (mV)
+            args.push_back("mV");
+        caller.call("dac", args, -1, GET, oss);
+        REQUIRE(oss.str() ==
+                "dac " + dac + " " + value + (mV ? " mV\n" : "\n"));
     }
     // Reset all dacs to previous value
     for (int i = 0; i != det.size(); ++i) {
@@ -93,31 +99,4 @@ void test_onchip_dac_caller(defs::dacIndex index, const std::string &dacname,
     }
 }
 
-std::pair<uint64_t, int>
-calculate_ctb_image_size(const acq::CTBState &test_info, bool isXilinxCtb) {
-
-    LOG(logDEBUG1) << test_info;
-    sls::CtbImageInputs inputs{};
-    inputs.mode = test_info.readout_mode;
-    inputs.nAnalogSamples = test_info.num_adc_samples;
-    inputs.adcMask = test_info.adc_enable_10g;
-    if (!isXilinxCtb && !test_info.ten_giga) {
-        inputs.adcMask = test_info.adc_enable_1g;
-    }
-    inputs.nTransceiverSamples = test_info.num_trans_samples;
-    inputs.transceiverMask = test_info.transceiver_mask;
-    inputs.nDigitalSamples = test_info.num_dbit_samples;
-    inputs.dbitOffset = test_info.dbit_offset;
-    inputs.dbitReorder = test_info.dbit_reorder;
-    inputs.dbitList = test_info.dbit_list;
-
-    auto out = computeCtbImageSize(inputs);
-    uint64_t image_size =
-        out.nAnalogBytes + out.nDigitalBytes + out.nTransceiverBytes;
-    LOG(logDEBUG1) << "Expected image size: " << image_size;
-    int npixelx = out.nPixelsX;
-    LOG(logDEBUG1) << "Expected number of pixels in x: " << npixelx;
-    return std::make_pair(image_size, npixelx);
-}
-
 } // namespace sls

slsDetectorSoftware/tests/Caller/test-Caller-global.h

@@ -2,28 +2,66 @@
 // Copyright (C) 2021 Contributors to the SLS Detector Package
 #pragma once
 
-#include "acquire/CTBState.h"
+#include "checks/MasterFileChecks.h"
 #include "sls/sls_detector_defs.h"
 
 #include <chrono>
 #include <filesystem>
 #include <optional>
+#include <string>
 #include <thread>
 
 namespace sls {
 
 namespace acq = sls::test::acquire;
+namespace mf = sls::test::master_file;
+namespace checks = sls::test::checks;
 
 void test_valid_port_caller(const std::string &command,
                             const std::vector<std::string> &arguments,
                             int detector_id, int action);
 
 void test_dac_caller(slsDetectorDefs::dacIndex index,
-                     const std::string &dacname, int dacvalue);
+                     const std::string &dacname, int dacvalue, bool mV = false);
 void test_onchip_dac_caller(slsDetectorDefs::dacIndex index,
                             const std::string &dacname, int dacvalue);
 
-std::pair<uint64_t, int>
-calculate_ctb_image_size(const acq::CTBState &test_info, bool isXilinxCtb);
+/**
+ * Helper function to run an acquisition and check the master file (both binary
+ * and hdf5) for expected values. The function takes in a lambda that is called
+ * with the master filechecker object to perform checks on the master file. The
+ * acquisition is run with default acquisition and file states, but these can be
+ * modified within the lambda if needed. This version has the master file
+ * checker object created within the function instead of using a helper
+ * function, to allow for more flexibility in handling exceptions (especially
+ * HDF5 exceptions) and logging.
+ */
+template <typename F>
+void test_run_with_master_file_checker(Detector &det, F f) {
+
+    auto acq_state = acq::default_acquisition_state();
+    auto file_state = acq::default_file_state();
+    std::array<defs::fileFormat, 2> formats = {defs::BINARY, defs::HDF5};
+
+    for (const auto &format : formats) {
+        file_state.file_format = format;
+        acq::run(det, acq_state, file_state);
+        std::string fname = acq::get_master_file_name(file_state);
+        if (format == defs::HDF5) {
+#ifdef HDF5C
+            try {
+                mf::Checker<mf::H5Context> checker(fname);
+                f(det, acq_state, file_state, checker);
+            } catch (H5::Exception &e) {
+                LOG(logERROR) << "HDF5 error: " << e.getDetailMsg();
+                throw;
+            }
+#endif
+        } else {
+            mf::Checker<mf::JsonContext> checker(fname);
+            f(det, acq_state, file_state, checker);
+        }
+    }
+}
 
 } // namespace sls

slsDetectorSoftware/tests/Caller/test-Caller-master-attributes.cpp

@@ -1,29 +1,16 @@
 // SPDX-License-Identifier: LGPL-3.0-or-other
 // Copyright (C) 2021 Contributors to the SLS Detector Package
-#include "acquire/ExpectedState.h"
-#include "checks/MasterFileChecks.h"
 
 #include "sls/Detector.h"
 #include "sls/ToString.h"
-#include "sls/logger.h"
+#include "test-Caller-global.h"
 
 #include "catch.hpp"
-#include <filesystem>
-#include <fstream>
-#include <rapidjson/document.h>
-#include <rapidjson/error/en.h>
-#include <sstream>
 #include <string>
 
-#ifdef HDF5C
-#include "H5Cpp.h"
-#endif
-
 namespace sls {
 
-namespace mf = sls::test::master_file;
 namespace acq = sls::test::acquire;
-namespace checks = sls::test::checks;
 
 TEST_CASE("check_master_file_attributes",
           "[.detectorintegration][.disable_check_data_file]") {
@@ -32,10 +19,6 @@ TEST_CASE("check_master_file_attributes",
     auto detType = det.getDetectorType().squash(defs::GENERIC);
     INFO("Testing master file attributes with " << ToString(detType));
 
-    // currently num frame = 1 (default)
-    auto acq_state = acq::default_acquisition_state();
-    auto file_state = acq::default_file_state();
-
     // if ctb, set to default and restore after test
     std::optional<acq::CTBState> ctb_state = std::nullopt;
     if (detType == defs::CHIPTESTBOARD ||
@@ -44,36 +27,13 @@ TEST_CASE("check_master_file_attributes",
     }
     acq::CTBStateGuard ctb_guard(det, ctb_state);
 
-    // binary => /tmp/sls_test_master_0.json
-    file_state.file_format = defs::BINARY;
-    acq::run(det, acq_state, file_state);
-
-    std::string fname = acq::get_master_file_name(file_state);
-    mf::Checker<mf::JsonContext> checker(fname);
-
-    // get expected state of parameters and check against master file
-    acq::ExpectedState expected_state =
-        acq::build_expected_state(det, acq_state, file_state, ctb_state);
-    checks::check_metadata(checker, expected_state);
-
-#ifdef HDF5C
-    try {
-        // hdf5 => /tmp/sls_test_master_0.h5
-        file_state.file_format = defs::HDF5;
-        acq::run(det, acq_state, file_state);
-
-        std::string fname = acq::get_master_file_name(file_state);
-        mf::Checker<mf::H5Context> checker(fname);
-
-        // get expected state of parameters and check against master file
-        acq::ExpectedState expected_state =
-            acq::build_expected_state(det, acq_state, file_state, ctb_state);
-        checks::check_metadata(checker, expected_state);
-    } catch (H5::Exception &e) {
-        LOG(logERROR) << "HDF5 error: " << e.getDetailMsg();
-        throw;
-    }
-#endif
+    test_run_with_master_file_checker(
+        det, [&](auto &det, auto &acq_state, auto &file_state, auto &checker) {
+            // get expected state of parameters and check against master file
+            auto expected_state = acq::build_expected_state(
+                det, acq_state, file_state, ctb_state);
+            checks::check_metadata(checker, expected_state);
+        });
 }
 
 } // namespace sls

slsDetectorSoftware/tests/acquire/CTBState.cpp

@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: LGPL-3.0-or-other
+// Copyright (C) 2021 Contributors to the SLS Detector Package
+
+#include "CTBState.h"
+#include "GeneralData.h"
+
+namespace sls::test::acquire {
+
+std::pair<uint64_t, int> calculate_ctb_image_size(const CTBState &test_info,
+                                                  bool isXilinxCtb) {
+    LOG(logDEBUG1) << test_info;
+    CtbImageInputs inputs{};
+    inputs.mode = test_info.readout_mode;
+    inputs.nAnalogSamples = test_info.num_adc_samples;
+    inputs.adcMask = test_info.adc_enable_10g;
+    if (!isXilinxCtb && !test_info.ten_giga) {
+        inputs.adcMask = test_info.adc_enable_1g;
+    }
+    inputs.nTransceiverSamples = test_info.num_trans_samples;
+    inputs.transceiverMask = test_info.transceiver_mask;
+    inputs.nDigitalSamples = test_info.num_dbit_samples;
+    inputs.dbitOffset = test_info.dbit_offset;
+    inputs.dbitReorder = test_info.dbit_reorder;
+    inputs.dbitList = test_info.dbit_list;
+
+    auto out = computeCtbImageSize(inputs);
+    uint64_t image_size =
+        out.nAnalogBytes + out.nDigitalBytes + out.nTransceiverBytes;
+    LOG(logDEBUG1) << "Expected image size: " << image_size;
+    int npixelx = out.nPixelsX;
+    LOG(logDEBUG1) << "Expected number of pixels in x: " << npixelx;
+    return std::make_pair(image_size, npixelx);
+}
+
+} // namespace sls::test::acquire

slsDetectorSoftware/tests/acquire/CTBState.h

@@ -135,4 +135,14 @@ class CTBStateGuard {
     CTBState saved_;
 };
 
+/**
+ * @brief
+ * @param test_info current CTB state
+ * @param isXilinxCtb if the detector type is Xilinx CTB
+ * @return std::pair<uint64_t, int>  pair of image size in bytes and number of
+ * channels in dimension X (Currently only analog channels)
+ */
+std::pair<uint64_t, int> calculate_ctb_image_size(const CTBState &test_info,
+                                                  bool isXilinxCtb);
+
 } // namespace sls::test::acquire

slsDetectorSoftware/tests/acquire/ExpectedState.cpp

@@ -2,7 +2,6 @@
 // Copyright (C) 2021 Contributors to the SLS Detector Package
 
 #include "ExpectedState.h"
-#include "Caller/test-Caller-global.h"
 #include "receiver_defs.h"
 
 // unnamed namespace for internal linkage
@@ -47,7 +46,7 @@ defs::xy get_port_shape(const Detector &det,
                 "CTB state must be provided to calculate expected port shape");
         }
         portSize.x =
-            sls::calculate_ctb_image_size(
+            acq::calculate_ctb_image_size(
                 ctb_state.value(), det_type == defs::XILINX_CHIPTESTBOARD)
                 .second;
         portSize.y = 1;
@@ -342,7 +341,7 @@ int get_expected_image_size(const Detector &det,
         }
         LOG(logINFORED) << ctb_state.value();
         image_size =
-            sls::calculate_ctb_image_size(
+            acq::calculate_ctb_image_size(
                 ctb_state.value(), (det_type == defs::XILINX_CHIPTESTBOARD))
                 .first;
         break;