fix merge formatting and refactoring

python/src/detector.cpp

@@ -109,6 +109,13 @@ void init_det(py::module &m) {
                        (Result<defs::xy>(Detector::*)(sls::Positions) const) &
                            Detector::getModuleSize,
                        py::arg() = Positions{});
+    CppDetectorApi.def("getPortPerModuleGeometry",
+                       (defs::xy(Detector::*)() const) &
+                           Detector::getPortPerModuleGeometry);
+    CppDetectorApi.def("getPortSize",
+                       (Result<defs::xy>(Detector::*)(sls::Positions) const) &
+                           Detector::getPortSize,
+                       py::arg() = Positions{});
     CppDetectorApi.def("getDetectorSize", (defs::xy(Detector::*)() const) &
                                               Detector::getDetectorSize);
     CppDetectorApi.def("setDetectorSize",
@@ -924,6 +931,19 @@ void init_det(py::module &m) {
                        (void (Detector::*)(bool, sls::Positions)) &
                            Detector::setRxArping,
                        py::arg(), py::arg() = Positions{});
+    CppDetectorApi.def("getRxROI",
+                       (std::vector<defs::ROI>(Detector::*)() const) &
+                           Detector::getRxROI);
+    CppDetectorApi.def("getRxROI",
+                       (std::vector<defs::ROI>(Detector::*)(int) const) &
+                           Detector::getRxROI,
+                       py::arg());
+    CppDetectorApi.def("setRxROI",
+                       (void (Detector::*)(const std::vector<defs::ROI> &)) &
+                           Detector::setRxROI,
+                       py::arg());
+    CppDetectorApi.def("clearRxROI",
+                       (void (Detector::*)()) & Detector::clearRxROI);
     CppDetectorApi.def(
         "getFileFormat",
         (Result<defs::fileFormat>(Detector::*)(sls::Positions) const) &

slsDetectorGui/include/qDrawPlot.h

@@ -40,6 +40,7 @@ class qDrawPlot : public QWidget, private Ui::PlotObject {
     void SetDataCallBack(bool enable);
     void SetBinary(bool enable, int from = 0, int to = 0);
     void StartAcquisition();
+    void UpdateROI();
 
   public slots:
     void SetPersistency(int val);
@@ -166,8 +167,8 @@ class qDrawPlot : public QWidget, private Ui::PlotObject {
     int64_t currentFrame{0};
     mutable std::mutex mPlots;
     int64_t currentAcqIndex{0};
-    slsDetectorDefs::ROI rxRoi{};
+    bool hasRoi{false};
-    bool isRxRoiDisplayed{false};
+    bool roiDisplayInitialized{false};
     bool isGapPixels{false};
 
     unsigned int nPixelsX{0};
@@ -176,6 +177,7 @@ class qDrawPlot : public QWidget, private Ui::PlotObject {
     uint32_t gainMask{0};
     int gainOffset{0};
     bool gotthard25;
+    std::vector<slsDetectorDefs::ROI> roi{1};
 };
 
 } // namespace sls

slsDetectorGui/slsDetectorPlotting/include/SlsQt1DPlot.h

@@ -5,6 +5,8 @@
 
 #include "SlsQt1DZoomer.h"
 #include "sls/ansi.h"
+#include "sls/sls_detector_defs.h"
+
 #include <array>
 #include <qwt_plot.h>
 #include <qwt_plot_curve.h>
@@ -141,8 +143,9 @@ class SlsQt1DPlot : public QwtPlot {
     void SetLogX(bool yes = 1);
     void SetLogY(bool yes = 1);
 
-    void EnableRoiBox(std::array<int, 4> roi);
+    void EnableRoiBoxes(std::vector<slsDetectorDefs::ROI> roi, int ymin,
-    void DisableRoiBox();
+                        int ymax);
+    void DisableRoiBoxes();
 
   private:
     bool gainPlot{false};
@@ -169,6 +172,7 @@ class SlsQt1DPlot : public QwtPlot {
     friend void SlsQtH1D::Attach(SlsQt1DPlot *p);
     friend void SlsQtH1D::Detach(SlsQt1DPlot *p);
 
+    std::vector<std::unique_ptr<QwtPlotShapeItem>> roiBoxes{};
     QwtPlotShapeItem *roiBox{nullptr};
 
   signals:

slsDetectorGui/slsDetectorPlotting/include/SlsQt2DPlot.h

@@ -3,6 +3,8 @@
 #pragma once
 #include "SlsQt2DHist.h"
 #include "SlsQt2DZoomer.h"
+#include "sls/sls_detector_defs.h"
+
 #include <array>
 #include <qlist.h>
 #include <qwt_plot.h>
@@ -71,8 +73,8 @@ class SlsQt2DPlot : public QwtPlot {
     void SetLogz(bool enable, bool isMin, bool isMax, double min, double max);
     void SetZRange(bool isMin, bool isMax, double min, double max);
     void LogZ(bool on = 1);
-    void EnableRoiBox(std::array<int, 4> roi);
+    void EnableRoiBoxes(std::vector<slsDetectorDefs::ROI> roi);
-    void DisableRoiBox();
+    void DisableRoiBoxes();
 
   public slots:
     void showSpectrogram(bool on);
@@ -101,7 +103,7 @@ class SlsQt2DPlot : public QwtPlot {
     QList<double> contourLevelsLog;
     bool disableZoom{false};
     int isLog;
-    QwtPlotShapeItem *roiBox{nullptr};
+    std::vector<std::unique_ptr<QwtPlotShapeItem>> roiBoxes{};
 };
 
 } // namespace sls

slsDetectorGui/slsDetectorPlotting/src/SlsQt1DPlot.cpp

@@ -462,24 +462,26 @@ void SlsQt1DPlot::SetLog(int axisId, bool yes) {
     Update();
 }
 
-void SlsQt1DPlot::EnableRoiBox(std::array<int, 4> roi) {
+void SlsQt1DPlot::EnableRoiBoxes(std::vector<slsDetectorDefs::ROI> roi,
-    if (roiBox == nullptr) {
+                                 int ymin, int ymax) {
-        roiBox = new QwtPlotShapeItem();
+    roiBoxes.clear();
-        roiBox->attach(this);
+    for (auto &r : roi) {
-        roiBox->setPen(QColor(Qt::yellow), 2.0, Qt::SolidLine);
+        auto box = std::make_unique<QwtPlotShapeItem>();
-    }
+        box->setPen(QColor(Qt::yellow), 2.0, Qt::SolidLine);
-
         // TopLeft - BottomRight (max points are +1 on graph)
-    QRect myRect(QPoint(roi[0], roi[2]), QPoint(roi[1] - 1, roi[3] - 1));
+        QRectF myRect(QPointF(r.xmin, ymin), QPointF(r.xmax - 1, ymax - 1));
-    roiBox->setRect(QRectF(myRect));
+        box->setRect(myRect);
+        box->attach(this);
+        roiBoxes.push_back(std::move(box));
+    }
     replot();
 }
 
-void SlsQt1DPlot::DisableRoiBox() {
+void SlsQt1DPlot::DisableRoiBoxes() {
-    if (roiBox != nullptr) {
+    for (auto &r : roiBoxes) {
-        roiBox->detach();
+        r->detach();
-        replot();
     }
+    replot();
 }
 
 void SlsQt1DPlot::SetZoomX(const QRectF &rect) {

slsDetectorGui/slsDetectorPlotting/src/SlsQt2DPlot.cpp

@@ -350,25 +350,25 @@ void SlsQt2DPlot::showSpectrogram(bool on) {
     Update();
 }
 
-void SlsQt2DPlot::EnableRoiBox(std::array<int, 4> roi) {
+void SlsQt2DPlot::EnableRoiBoxes(std::vector<slsDetectorDefs::ROI> roi) {
-    if (roiBox == nullptr) {
+    roiBoxes.clear();
-        roiBox = new QwtPlotShapeItem();
+    for (auto &r : roi) {
-    }
+        auto box = std::make_unique<QwtPlotShapeItem>();
-    roiBox->setPen(QColor(Qt::yellow), 2.0, Qt::SolidLine);
+        box->setPen(QColor(Qt::yellow), 2.0, Qt::SolidLine);
-
         // TopLeft - BottomRight (max points are +1 on graph)
-    QRect myRect(QPoint(roi[0], roi[2]), QPoint(roi[1] - 1, roi[3] - 1));
+        QRectF myRect(QPointF(r.xmin, r.ymin), QPointF(r.xmax - 1, r.ymax - 1));
-    roiBox->setRect(QRectF(myRect));
+        box->setRect(myRect);
-
+        box->attach(this);
-    roiBox->attach(this);
+        roiBoxes.push_back(std::move(box));
+    }
     replot();
 }
 
-void SlsQt2DPlot::DisableRoiBox() {
+void SlsQt2DPlot::DisableRoiBoxes() {
-    if (roiBox != nullptr) {
+    for (auto &r : roiBoxes) {
-        roiBox->detach();
+        r->detach();
-        replot();
     }
+    replot();
 }
 
 } // namespace sls

slsDetectorGui/src/qDetectorMain.cpp

@@ -539,6 +539,7 @@ void qDetectorMain::EnableTabs(bool enable) {
             tabDeveloper->Refresh();
 
         tabPlot->Refresh();
+        plot->UpdateROI();
         plot->StartAcquisition();
     } else { // to enable scan box
         tabPlot->Refresh();

slsDetectorGui/src/qDrawPlot.cpp

@@ -608,7 +608,6 @@ void qDrawPlot::StartAcquisition() {
     currentFrame = 0;
     boxPlot->setTitle("Old Plot");
     det->clearAcquiringFlag(); // (from previous exit) or if running
-    isRxRoiDisplayed = false;
 
     // ensure data streaming in receiver (if plot enabled)
     if (isPlot) {
@@ -632,6 +631,34 @@ void qDrawPlot::StartAcquisition() {
     LOG(logDEBUG) << "End of Starting Acquisition in qDrawPlot";
 }
 
+void qDrawPlot::UpdateROI() {
+    try {
+        std::lock_guard<std::mutex> lock(mPlots);
+        roi = det->getRxROI();
+        roiDisplayInitialized = false;
+        // roi enabled
+        if (roi.size() > 1 || !roi[0].completeRoi()) {
+            hasRoi = true;
+            // update gap pixels
+            // hardcoded gap pixels because only for eiger and jungfrau
+            if (isGapPixels) {
+                for (auto &r : roi) {
+                    r.xmin += ((r.xmin / 1024) * 6 + (r.xmin / 256) * 2);
+                    r.xmax += ((r.xmax / 1024) * 6 + (r.xmax / 256) * 2);
+                    r.ymin += ((r.ymin / 512) * 34 + (r.ymin / 256) * 2);
+                    r.ymax += ((r.ymax / 512) * 34 + (r.ymax / 256) * 2);
+                }
+                LOG(logINFO)
+                    << "Roi recalculated with gap pixels: " << ToString(roi);
+            }
+        } else {
+            hasRoi = false;
+        }
+        LOG(logDEBUG) << "Roi: " << ToString(roi);
+    }
+    CATCH_DISPLAY("Could not get reciver ROI.", "qDrawPlot::UpdateRoi")
+}
+
 void qDrawPlot::AcquireThread() {
     LOG(logDEBUG) << "Acquire Thread";
     std::string mess;
@@ -709,7 +736,6 @@ void qDrawPlot::GetData(detectorData *data, uint64_t frameIndex,
                   << "  \t dynamic range: " << data->dynamicRange << std::endl
                   << "  \t file index: " << data->fileIndex << std::endl
                   << "  \t complete image: " << data->completeImage << std::endl
-                  << "  \t rx Roi: " << ToString(data->rxRoi) << std::endl
                   << "  ]";
 
     progress = data->progressIndex;
@@ -717,22 +743,6 @@ void qDrawPlot::GetData(detectorData *data, uint64_t frameIndex,
     currentFrame = frameIndex;
     LOG(logDEBUG) << "[ Progress:" << progress << "%, Frame:" << currentFrame
                   << " ]";
-    if (!isRxRoiDisplayed) {
-        rxRoi.xmin = data->rxRoi[0];
-        rxRoi.xmax = data->rxRoi[1];
-        rxRoi.ymin = data->rxRoi[2];
-        rxRoi.ymax = data->rxRoi[3];
-        // only for 2d anyway
-        if (isGapPixels) {
-            rxRoi.xmin += ((rxRoi.xmin / 1024) * 6 + (rxRoi.xmin / 256) * 2);
-            rxRoi.xmax += ((rxRoi.xmax / 1024) * 6 + (rxRoi.xmax / 256) * 2);
-            rxRoi.ymin += ((rxRoi.ymin / 512) * 34 + (rxRoi.ymin / 256) * 2);
-            rxRoi.ymax += ((rxRoi.ymax / 512) * 34 + (rxRoi.ymax / 256) * 2);
-            LOG(logINFO) << "Rx_roi recalculated with gap pixels: "
-                         << ToString(rxRoi);
-        }
-        LOG(logDEBUG) << "Rx_roi: " << ToString(rxRoi);
-    }
 
     // 1d check if npixelX has changed (m3 for different counters enabled)
     if (is1d && static_cast<int>(nPixelsX) != data->nx) {
@@ -971,30 +981,27 @@ void qDrawPlot::Update1dPlot() {
         xyRangeChanged = false;
     }
     plot1d->DisableZoom(disableZoom);
-    if (!isRxRoiDisplayed) {
+    if (!roiDisplayInitialized) {
-        isRxRoiDisplayed = true;
+        roiDisplayInitialized = true;
-        if (rxRoi.completeRoi()) {
+        if (!hasRoi) {
-            plot1d->DisableRoiBox();
+            plot1d->DisableRoiBoxes();
             if (isGainDataExtracted) {
-                gainplot1d->DisableRoiBox();
+                gainplot1d->DisableRoiBoxes();
             }
             lblRxRoiEnabled->hide();
         } else {
-            plot1d->EnableRoiBox(std::array<int, 4>{
+            plot1d->EnableRoiBoxes(roi, (int)plot1d->GetYMinimum(),
-                rxRoi.xmin, rxRoi.xmax, (int)plot1d->GetYMinimum(),
+                                   (int)plot1d->GetYMaximum());
-                (int)plot1d->GetYMaximum()});
             if (isGainDataExtracted) {
-                gainplot1d->EnableRoiBox(
+                gainplot1d->EnableRoiBoxes(roi, 0, 3);
-                    std::array<int, 4>{rxRoi.xmin, rxRoi.xmax, 0, 3});
             }
             lblRxRoiEnabled->show();
         }
     }
     // ymin and ymax could change (so replot roi every time)
-    if (!rxRoi.completeRoi()) {
+    if (hasRoi) {
-        plot1d->EnableRoiBox(std::array<int, 4>{rxRoi.xmin, rxRoi.xmax,
+        plot1d->EnableRoiBoxes(roi, (int)plot1d->GetYMinimum(),
-                                                (int)plot1d->GetYMinimum(),
+                               (int)plot1d->GetYMaximum());
-                                                (int)plot1d->GetYMaximum()});
     }
 }
 
@@ -1025,18 +1032,18 @@ void qDrawPlot::Update2dPlot() {
     }
     plot2d->DisableZoom(disableZoom);
     plot2d->SetZRange(isZRange[0], isZRange[1], zRange[0], zRange[1]);
-    if (!isRxRoiDisplayed) {
+    if (!roiDisplayInitialized) {
-        isRxRoiDisplayed = true;
+        roiDisplayInitialized = true;
-        if (rxRoi.completeRoi()) {
+        if (!hasRoi) {
-            plot2d->DisableRoiBox();
+            plot2d->DisableRoiBoxes();
             if (isGainDataExtracted) {
-                gainplot2d->DisableRoiBox();
+                gainplot2d->DisableRoiBoxes();
             }
             lblRxRoiEnabled->hide();
         } else {
-            plot2d->EnableRoiBox(rxRoi.getIntArray());
+            plot2d->EnableRoiBoxes(roi);
             if (isGainDataExtracted) {
-                gainplot2d->EnableRoiBox(rxRoi.getIntArray());
+                gainplot2d->EnableRoiBoxes(roi);
             }
             lblRxRoiEnabled->show();
         }

slsDetectorSoftware/generator/Caller.in.h

@@ -22,6 +22,7 @@ class Caller {
     int GetLevelAndInsertIntoArgs(std::string levelSeparatedCommand);
     void WrongNumberOfParameters(size_t expected);
     std::vector<defs::ROI> parseRoiVector(const std::string &input);
+    defs::ROI parseRoi(const std::vector<std::string> &args);
 
     template <typename V> std::string OutStringHex(const V &value) {
         if (value.equal())

slsDetectorSoftware/generator/autocomplete/bash_autocomplete.sh

@@ -2193,20 +2193,6 @@ return 0
 }
 __rx_roi() {
 FCN_RETURN=""
-if [[ ${IS_GET} -eq 0 ]]; then
-if [[ "${cword}" == "2" ]]; then
-FCN_RETURN=""
-fi
-if [[ "${cword}" == "3" ]]; then
-FCN_RETURN=""
-fi
-if [[ "${cword}" == "4" ]]; then
-FCN_RETURN=""
-fi
-if [[ "${cword}" == "5" ]]; then
-FCN_RETURN=""
-fi
-fi
 return 0
 }
 __rx_silent() {

slsDetectorSoftware/generator/autocomplete/zsh_autocomplete.sh

@@ -2117,20 +2117,6 @@ return 0
 }
 __rx_roi() {
 FCN_RETURN=""
-if [[ ${IS_GET} -eq 0 ]]; then
-if [[ "${cword}" == "2" ]]; then
-FCN_RETURN=""
-fi
-if [[ "${cword}" == "3" ]]; then
-FCN_RETURN=""
-fi
-if [[ "${cword}" == "4" ]]; then
-FCN_RETURN=""
-fi
-if [[ "${cword}" == "5" ]]; then
-FCN_RETURN=""
-fi
-fi
 return 0
 }
 __rx_silent() {

slsDetectorSoftware/generator/extended_commands.yaml

@@ -8831,25 +8831,8 @@ rx_roi:
         store_result_in_t: true
     PUT:
       args:
-      - arg_types:
+      - arg_types: []
-        - int
+        argc: -1
-        - int
-        argc: 2
-        cast_input: []
-        check_det_id: false
-        convert_det_id: true
-        function: ''
-        input: []
-        input_types: []
-        output: []
-        require_det_id: false
-        store_result_in_t: false
-      - arg_types:
-        - int
-        - int
-        - int
-        - int
-        argc: 4
         cast_input: []
         check_det_id: false
         convert_det_id: true

slsDetectorSoftware/include/sls/Detector.h

@@ -989,14 +989,13 @@ class Detector {
      * every minute. Useful in 10G mode. */
     void setRxArping(bool value, Positions pos = {});
 
-    /** Returns multi level ROIs */
+    /** If module_id  is -1, returns multi level ROIs. Else it returns port
-    std::vector<defs::ROI> getRxROI() const;
+     * level ROIs. Max 2 ports and hence max 2 elements per readout */
+    std::vector<defs::ROI> getRxROI(int module_id = -1) const;
 
-    /** Returns port level ROIs. Max 2 ports and hence max 2 elements per readout */
+    /** only at multi module level without gap pixels. If more than 1 ROI per
-    std::vector<defs::ROI> getRxROI(int module_id) const;
+     * UDP port, it will throw. Setting number of udp interfaces will clear the
-
+     * roi. Cannot be set for CTB or Xilinx CTB */
-    /** only at multi module level without gap pixels. At most, 1 ROI per UDP
-     * port. Setting number of udp interfaces will clear the roi */
     void setRxROI(const std::vector<defs::ROI> &args);
 
     void clearRxROI();

slsDetectorSoftware/include/sls/detectorData.h

@@ -19,14 +19,6 @@ class detectorData {
           databytes(databytes), dynamicRange(dynamicRange),
           completeImage(completeImage){};
 
-    detectorData(double progressIndex, std::string fileName, int nx, int ny,
-                 char *data, int databytes, int dynamicRange,
-                 uint64_t fileIndex, bool completeImage,
-                 std::array<int, 4> rxRoi)
-        : progressIndex(progressIndex), fileName(fileName),
-          fileIndex(fileIndex), nx(nx), ny(ny), data(data),
-          databytes(databytes), dynamicRange(dynamicRange),
-          completeImage(completeImage), rxRoi(rxRoi){};
     /**
      * data has to be deleted by caller
      */
@@ -62,7 +54,6 @@ class detectorData {
     int databytes;
     int dynamicRange;
     bool completeImage;
-    std::array<int, 4> rxRoi{{-1, -1, -1, -1}};
 };
 
 } // namespace sls

slsDetectorSoftware/src/Caller.h

@@ -22,6 +22,7 @@ class Caller {
     int GetLevelAndInsertIntoArgs(std::string levelSeparatedCommand);
     void WrongNumberOfParameters(size_t expected);
     std::vector<defs::ROI> parseRoiVector(const std::string &input);
+    defs::ROI parseRoi(const std::vector<std::string> &args);
 
     template <typename V> std::string OutStringHex(const V &value) {
         if (value.equal())

slsDetectorSoftware/src/CallerSpecial.cpp

@@ -723,70 +723,70 @@ std::string Caller::rx_zmqip(int action) {
 std::string Caller::rx_roi(int action) {
     std::ostringstream os;
     std::string helpMessage =
-        std::string("[xmin] [xmax] [ymin] [ymax]\n\tRegion of interest in "
+        std::string("[xmin] [xmax] [ymin] [ymax]\n") +
-                    "receiver.\n\t") +
+        "\tDefines a single region of interest (ROI) in the receiver.\n"
-        "For a list of rois, use '[' and ']; ' to distinguish between "
+        "\tFor example, to set a single ROI: 0 100 20 30\n\n"
-        "rois and use comma inside the square brackets.\n\t If one fails to "
+
-        "use space after semicolon, please use quotes" +
+        "\tTo specify multiple ROIs, use square brackets between ROIs and "
-        "For example: [0,100,0,100]; [200,300,0,100] will set two "
+        "commas inside for each ROI. \n"
-        "rois.or '[0,100,0,100];[200,300,0,100]' when the vector is a single "
+        "\tInside each bracket, no spaces allowed.\n\n"
-        "string\n\n\t" +
+
-        "Only allowed to set at multi module level and without gap "
+        "\tIf you use semicolon (along with '['and ']' to separate rois), \n"
-        "ixels.\n\n\t" +
+        "\tenclose the entire list in quotes.\n"
-        "One can get rx_roi also at port level, by specifying the module id "
+        "\tExamples:\n"
-        "and it will return the roi for each port.\n"
+        "\t  [0,100,0,100] [200,300,0,100]\n"
-        "Setting number of udp interfaces will clear the rx_roi\n";
+        "\t  \"[0,100,0,100];[200,300,0,100]\"\n\n"
+
+        "\tNotes:\n"
+        "\t- ROIs can only be set at the multi-module level.\n"
+        "\t- ROIs coordinates assume no gap pixels, even if they are enabled "
+        "in gui.\n"
+        "\t- To retrieve ROIs per port, specify the module ID when using the "
+        "get command.\n"
+        "\t- Use the command 'rx_clearroi' to clear all ROIs.\n"
+        "\t- Changing the number of UDP interfaces will automatically clear "
+        "the current ROIs.\n\n"
+        "\t- Cannot be set for CTB or Xilinx CTB.\n";
+
     if (action == defs::HELP_ACTION) {
         os << helpMessage;
     } else if (action == defs::GET_ACTION) {
         if (!args.empty()) {
             WrongNumberOfParameters(0);
         }
-        if (det_id != -1) {
         auto t = det->getRxROI(det_id);
         os << ToString(t) << '\n';
-        } else {
-            auto t = det->getRxROI();
-            os << ToString(t) << '\n';
-        }
     } else if (action == defs::PUT_ACTION) {
-        std::vector<defs::ROI> rois;
+        if (det_id != -1) {
-
+            throw RuntimeError("Cannot set receiver ROI at module level");
+        }
         // Support multiple args with bracketed ROIs, or single arg with
-        // semicolon-separated vector
+        // semicolon-separated vector in quotes
         bool isVectorInput =
             std::all_of(args.begin(), args.end(), [](const std::string &a) {
                 return a.find('[') != std::string::npos &&
                        a.find(']') != std::string::npos;
             });
+        std::vector<defs::ROI> rois;
         try {
-            // previous format: 2 or 4 separate args
+            // single roi in previous format: [xmin,xmax,ymin,ymax]
-            if ((args.size() == 2 || args.size() == 4) && !isVectorInput) {
+            if (!isVectorInput) {
-                defs::ROI t;
+                auto t = parseRoi(args);
-                t.xmin = StringTo<int>(args[0]);
-                t.xmax = StringTo<int>(args[1]);
-                if (args.size() == 4) {
-                    t.ymin = StringTo<int>(args[2]);
-                    t.ymax = StringTo<int>(args[3]);
-                }
                 rois.emplace_back(t);
-            } else {
+            }
-                if (!isVectorInput)
+            // multiple roi or single roi with brackets
-                    WrongNumberOfParameters(2);
+            // multiple roi: multiple args with bracketed ROIs, or single arg
+            // with semicolon-bracketed Rois in quotes
             else {
                 for (const auto &arg : args) {
                     auto subRois = parseRoiVector(arg);
                     rois.insert(rois.end(), subRois.begin(), subRois.end());
                 }
             }
-            }
         } catch (const std::exception &e) {
-            throw RuntimeError("Could not parse ROI: " + helpMessage);
+            throw RuntimeError("Could not parse ROI: Did you use spaces inside "
-        }
+                               "the brackets? Use sls_detector_help " +
-
+                               cmd + " to get the right syntax expected.");
-        // only multi level
-        if (det_id != -1) {
-            throw RuntimeError("Cannot execute receiver ROI at module level");
         }
 
         det->setRxROI(rois);
@@ -802,16 +802,19 @@ std::vector<defs::ROI> Caller::parseRoiVector(const std::string &input) {
     std::stringstream ss(input);
     std::string token;
 
-    while (std::getline(ss, token, ';')) {
+    while (std::getline(ss, token, ']')) {
-        token.erase(std::remove_if(token.begin(), token.end(), ::isspace),
+        // remove spaces and semicolons
+        token.erase(
+            std::remove_if(token.begin(), token.end(),
+                           [](char c) { return std::isspace(c) || c == ';'; }),
             token.end());
         if (token.empty())
             continue;
-        if (token.front() != '[' || token.back() != ']') {
+        if (token.front() != '[') {
             throw RuntimeError("Each ROI must be enclosed in square brackets: "
                                "[xmin,xmax,ymin,ymax]");
         }
-        token = token.substr(1, token.size() - 2); // remove brackets
+        token = token.substr(1, token.size() - 1); // remove brackets
         std::vector<std::string> parts;
         std::stringstream inner(token);
         std::string num;
@@ -819,8 +822,16 @@ std::vector<defs::ROI> Caller::parseRoiVector(const std::string &input) {
             parts.push_back(num);
         }
 
+        auto roi = parseRoi(parts);
+        rois.emplace_back(roi);
+    }
+    return rois;
+}
+
+defs::ROI Caller::parseRoi(const std::vector<std::string> &parts) {
     if (parts.size() != 2 && parts.size() != 4) {
-            throw RuntimeError("ROI must have 2 or 4 comma-separated integers");
+        throw RuntimeError(
+            "Could not parse ROI. A ROI must have 2 or 4 integers");
     }
 
     defs::ROI roi;
@@ -830,9 +841,7 @@ std::vector<defs::ROI> Caller::parseRoiVector(const std::string &input) {
         roi.ymin = StringTo<int>(parts[2]);
         roi.ymax = StringTo<int>(parts[3]);
     }
-        rois.emplace_back(roi);
+    return roi;
-    }
-    return rois;
 }
 
 std::string Caller::ratecorr(int action) {

slsDetectorSoftware/src/Detector.cpp

@@ -1384,22 +1384,16 @@ void Detector::setRxArping(bool value, Positions pos) {
     pimpl->Parallel(&Module::setRxArping, pos, value);
 }
 
-std::vector<defs::ROI> Detector::getRxROI() const { 
-    return pimpl->getRxROI(); 
-}
-
 std::vector<defs::ROI> Detector::getRxROI(int module_id) const {
     return pimpl->getRxROI(module_id);
 }
 
-// RxROIs can be set for all types except CTB. At multi level without gap pixels
 void Detector::setRxROI(const std::vector<defs::ROI> &args) {
     pimpl->setRxROI(args);
 }
 
 void Detector::clearRxROI() { pimpl->clearRxROI(); }
 
-
 // File
 
 Result<defs::fileFormat> Detector::getFileFormat(Positions pos) const {

slsDetectorSoftware/src/DetectorImpl.cpp

@@ -535,7 +535,6 @@ void DetectorImpl::readFrameFromReceiver() {
     bool quadEnable = false;
     // to flip image
     bool eiger = false;
-    std::array<int, 4> rxRoi{}; // TODO: get roi from json header
 
     std::vector<bool> runningList(zmqSocket.size());
     std::vector<bool> connectList(zmqSocket.size());
@@ -732,7 +731,7 @@ void DetectorImpl::readFrameFromReceiver() {
             thisData = new detectorData(currentProgress, currentFileName,
                                         nDetActualPixelsX, nDetActualPixelsY,
                                         callbackImage, imagesize, dynamicRange,
-                                        currentFileIndex, completeImage, rxRoi);
+                                        currentFileIndex, completeImage);
             try {
                 dataReady(
                     thisData, currentFrameIndex,
@@ -1813,16 +1812,17 @@ void DetectorImpl::convertGlobalRoiToPortLevel(
             clipped.xmin = std::max(userRoi.xmin, portRoi.xmin) - portRoi.xmin;
             clipped.xmax = std::min(userRoi.xmax, portRoi.xmax) - portRoi.xmin;
             if (modSize.y > 1) {
-                clipped.ymin = std::max(userRoi.ymin, portRoi.ymin) - portRoi.ymin;
+                clipped.ymin =
-                clipped.ymax = std::min(userRoi.ymax, portRoi.ymax) - portRoi.ymin;
+                    std::max(userRoi.ymin, portRoi.ymin) - portRoi.ymin;
+                clipped.ymax =
+                    std::min(userRoi.ymax, portRoi.ymax) - portRoi.ymin;
             }
 
             // Check if port ROI already exists for this port (from another user roi)
             if (!portRois[port].completeRoi() && !portRois[port].noRoi()) {
                 throw RuntimeError(
                     "Multiple ROIs specified for the same port " +
-                    std::to_string(port) +
+                    std::to_string(port) + " with ROI: " + ToString(userRoi));
-                    " with ROI: " + ToString(userRoi));
             }
             portRois[port] = clipped;
         }
@@ -1843,7 +1843,9 @@ void DetectorImpl::setRxROI(const std::vector<defs::ROI> &args) {
     }
 
     validateROIs(args);
-    int nPortsPerModule = Parallel(&Module::getNumberofUDPInterfacesFromShm, {}).tsquash("Inconsistent number of udp ports set up per module");
+    int nPortsPerModule =
+        Parallel(&Module::getNumberofUDPInterfacesFromShm, {})
+            .tsquash("Inconsistent number of udp ports set up per module");
 
     for (size_t iModule = 0; iModule < modules.size(); ++iModule) {
         auto moduleGlobalRoi = getModuleROI(iModule);
@@ -1856,7 +1858,6 @@ void DetectorImpl::setRxROI(const std::vector<defs::ROI> &args) {
                 convertGlobalRoiToPortLevel(arg, moduleGlobalRoi, portRois);
             }
         }
-
         modules[iModule]->setRxROI(portRois);
     }
     // metadata
@@ -1864,7 +1865,9 @@ void DetectorImpl::setRxROI(const std::vector<defs::ROI> &args) {
 }
 
 void DetectorImpl::clearRxROI() {
-    int nPortsPerModule = Parallel(&Module::getNumberofUDPInterfacesFromShm, {}).tsquash("Inconsistent number of udp ports set up per module");
+    int nPortsPerModule =
+        Parallel(&Module::getNumberofUDPInterfacesFromShm, {})
+            .tsquash("Inconsistent number of udp ports set up per module");
     for (size_t iModule = 0; iModule < modules.size(); ++iModule) {
         modules[iModule]->setRxROI(std::vector<defs::ROI>(nPortsPerModule));
     }

slsDetectorSoftware/src/DetectorImpl.h

@@ -428,8 +428,8 @@ class DetectorImpl : public virtual slsDetectorDefs {
     void validateROIs(const std::vector<defs::ROI> &rois);
     defs::xy calculatePosition(int moduleIndex) const;
     defs::ROI getModuleROI(int moduleIndex) const;
-    void convertGlobalRoiToPortLevel(
+    void convertGlobalRoiToPortLevel(const defs::ROI &userRoi,
-        const defs::ROI &userRoi, const defs::ROI &moduleRoi,
+                                     const defs::ROI &moduleRoi,
                                      std::vector<defs::ROI> &portRois) const;
 
     const int detectorIndex{0};

slsDetectorSoftware/src/Module.cpp

@@ -1587,8 +1587,7 @@ std::vector<slsDetectorDefs::ROI> Module::getRxROIMetadata() const {
         throw RuntimeError("Invalid number of ROI metadata: " +
                            std::to_string(size) + ". Min: 1.");
     }
-    LOG(logDEBUG1) << "ROI metadata of Receiver: "
+    LOG(logDEBUG1) << "ROI metadata of Receiver: " << ToString(retval);
-                   << ToString(retval);
     return retval;
 }
 

slsDetectorSoftware/src/inferAction.cpp

@@ -2769,22 +2769,8 @@ int InferAction::rx_realudpsocksize() {
 
 int InferAction::rx_roi() {
 
-    if (args.size() == 0) {
+    throw RuntimeError("sls_detector is disabled for command: rx_roi. Use "
-        return slsDetectorDefs::GET_ACTION;
+                       "sls_detector_get or sls_detector_put");
-    }
-
-    if (args.size() == 2) {
-        return slsDetectorDefs::PUT_ACTION;
-    }
-
-    if (args.size() == 4) {
-        return slsDetectorDefs::PUT_ACTION;
-    }
-
-    else {
-
-        throw RuntimeError("Could not infer action: Wrong number of arguments");
-    }
 }
 
 int InferAction::rx_silent() {

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

@@ -7,8 +7,8 @@
 #include "sls/sls_detector_defs.h"
 #include "test-Caller-global.h"
 
-#include <sstream>
 #include <filesystem>
+#include <sstream>
 
 #include "sls/versionAPI.h"
 #include "tests/globals.h"
@@ -502,19 +502,20 @@ TEST_CASE("rx_roi", "[.cmdcall]") {
                 {"[95," + std::to_string(detsize.x + 5) + ", -1, -1]"}, -1,
                 PUT));
             // module level not allowed
-            REQUIRE_THROWS(caller.call(
+            REQUIRE_THROWS(caller.call("rx_roi", {"[5, 10, -1, -1]"}, 0, PUT));
-                "rx_roi", {"[5, 10, -1, -1]"}, 0, PUT));
 
             // vector of rois
             // square brackets missing
             REQUIRE_THROWS(caller.call(
-                "rx_roi", {"[5, 20, -1, -1; 25, 30, -1, -1]"}, -1, PUT));
+                "rx_roi", {"[5, 20, -1, -1]; 25, 30, -1, -1]"}, -1, PUT));
+            REQUIRE_THROWS(caller.call(
+                "rx_roi", {"[5, 20, -1, -1]; [25, 30, -1, -1"}, -1, PUT));
             // invalid roi, 4 parts expected
             REQUIRE_THROWS(caller.call(
-                "rx_roi", {"[5, 20, -1]; [25, 30, -1, -1]"}, -1, PUT));
+                "rx_roi", {"[5, 20, -1] [25, 30, -1, -1]"}, -1, PUT));
             // overlapping rois
             REQUIRE_THROWS(caller.call(
-                "rx_roi", {"[0, 10,-1, -1];[5, 15, -1, -1]"}, -1, PUT));
+                "rx_roi", {"[0, 10,-1, -1] [5, 15, -1, -1]"}, -1, PUT));
 
             if (det.size() == 2) {
                 auto moduleSize = det.getModuleSize()[0];
@@ -523,13 +524,19 @@ TEST_CASE("rx_roi", "[.cmdcall]") {
 
                 // separated by space is allowed
                 REQUIRE_NOTHROW(caller.call(
-                    "rx_roi", {"[5, 10, -1, -1]", "[" + stringMin + ", " + stringMax + ", -1, -1]"}, -1, PUT));
+                    "rx_roi",
+                    {"[5, 10, -1, -1]",
+                     "[" + stringMin + ", " + stringMax + ", -1, -1]"},
+                    -1, PUT));
                 std::ostringstream oss;
-                // separated by semicolon is allowed
+                // separated by semicolon with quotes is allowed (skips
-                REQUIRE_NOTHROW(caller.call(
+                // cmdParser)
-                    "rx_roi", {"[5, 10, -1, -1];[" + stringMin + ", " + stringMax + ", -1, -1]"}, -1, PUT, oss));
+                REQUIRE_NOTHROW(caller.call("rx_roi",
-                REQUIRE(oss.str() ==
+                                            {"[5, 10, -1, -1];[" + stringMin +
-                        "rx_roi [[5, 10], [" + stringMin + ", " + stringMax + "]]\n");
+                                             ", " + stringMax + ", -1, -1]"},
+                                            -1, PUT, oss));
+                REQUIRE(oss.str() == "rx_roi [[5, 10], [" + stringMin + ", " +
+                                         stringMax + "]]\n");
 
                 // verify individual roi
                 {
@@ -537,12 +544,14 @@ TEST_CASE("rx_roi", "[.cmdcall]") {
                     stringMax = std::to_string(moduleSize.x + 50);
                     std::ostringstream oss, oss1;
                     REQUIRE_NOTHROW(caller.call(
-                        "rx_roi", {"[" + stringMin + ", " + stringMax + "]"}, -1, PUT, oss));
+                        "rx_roi", {"[" + stringMin + ", " + stringMax + "]"},
-                    REQUIRE(oss.str() ==
+                        -1, PUT, oss));
-                        "rx_roi [[" + stringMin + ", " + stringMax + "]]\n");
+                    REQUIRE(oss.str() == "rx_roi [[" + stringMin + ", " +
-                    REQUIRE_NOTHROW(
+                                             stringMax + "]]\n");
-                        caller.call("rx_roi", {}, 0, GET, oss1));
+                    REQUIRE_NOTHROW(caller.call("rx_roi", {}, 0, GET, oss1));
-                    REQUIRE(oss1.str() == "rx_roi [[" + stringMin + ", " + std::to_string(moduleSize.x - 1) + "]]\n");
+                    REQUIRE(oss1.str() == "rx_roi [[" + stringMin + ", " +
+                                              std::to_string(moduleSize.x - 1) +
+                                              "]]\n");
                 }
             }
         }
@@ -587,40 +596,47 @@ TEST_CASE("rx_roi", "[.cmdcall]") {
                 {"[95, 100, 0, " + std::to_string(detsize.y + 5) + "]"}, -1,
                 PUT));
             // module level not allowed
-            REQUIRE_THROWS(caller.call(
+            REQUIRE_THROWS(caller.call("rx_roi", {"[5, 10, 20, 30]"}, 0, PUT));
-                "rx_roi", {"[5, 10, 20, 30]"}, 0, PUT));
 
             // vector of rois
             // square brackets missing
             REQUIRE_THROWS(caller.call(
-                "rx_roi", {"[5, 20, 20, 30; 25, 30, 14, 15]"}, -1, PUT));
+                "rx_roi", {"[5, 20, 20, 30]; 25, 30, 14, 15]"}, -1, PUT));
+            REQUIRE_THROWS(caller.call(
+                "rx_roi", {"[5, 20, 20, 30]; [25, 30, 14, 15"}, -1, PUT));
             // invalid roi, 4 parts expected
             REQUIRE_THROWS(caller.call(
-                "rx_roi", {"[5, 20, 20]; [25, 30, 14, 15]"}, -1, PUT));
+                "rx_roi", {"[5, 20, 20] [25, 30, 14, 15]"}, -1, PUT));
             // overlapping rois
             REQUIRE_THROWS(caller.call(
-                "rx_roi", {"[0, 10, 0, 10];[5, 15, 0, 10]"}, -1, PUT));
+                "rx_roi", {"[0, 10, 0, 10] [5, 15, 0, 10]"}, -1, PUT));
             REQUIRE_THROWS(caller.call(
-                "rx_roi", {"[0, 10, 0, 10];[0, 10, 9, 11]"}, -1, PUT));
+                "rx_roi", {"[0, 10, 0, 10] [0, 10, 9, 11]"}, -1, PUT));
-
 
             int numinterfaces = det.getNumberofUDPInterfaces().tsquash(
                 "inconsistent number of interfaces");
 
             // multiple ports horizontally
-            if (det_type == defs::EIGER || (det.size() == 2 && det.getModuleGeometry().x  > 1)) {
+            if (det_type == defs::EIGER ||
+                (det.size() == 2 && det.getModuleGeometry().x > 1)) {
                 std::string stringMin = std::to_string(portSize.x);
                 std::string stringMax = std::to_string(portSize.x + 1);
 
                 // separated by space is allowed
                 REQUIRE_NOTHROW(caller.call(
-                    "rx_roi", {"[5, 10, 20, 30]", "[" + stringMin + ", " + stringMax + ", 20, 30]"}, -1, PUT));
+                    "rx_roi",
+                    {"[5, 10, 20, 30]",
+                     "[" + stringMin + ", " + stringMax + ", 20, 30]"},
+                    -1, PUT));
                 std::ostringstream oss;
-                // separated by semicolon is allowed
+                // separated by semicolon with quotes is allowed (skips
-                REQUIRE_NOTHROW(caller.call(
+                // cmdParser)
-                    "rx_roi", {"[5, 10, 20, 30];[" + stringMin + ", " + stringMax + ", 20, 30]"}, -1, PUT, oss));
+                REQUIRE_NOTHROW(caller.call("rx_roi",
-                REQUIRE(oss.str() ==
+                                            {"[5, 10, 20, 30];[" + stringMin +
-                    "rx_roi [[5, 10, 20, 30], [" + stringMin + ", " + stringMax + ", 20, 30]]\n");
+                                             ", " + stringMax + ", 20, 30]"},
+                                            -1, PUT, oss));
+                REQUIRE(oss.str() == "rx_roi [[5, 10, 20, 30], [" + stringMin +
+                                         ", " + stringMax + ", 20, 30]]\n");
 
                 // verify individual roi
                 {
@@ -628,36 +644,53 @@ TEST_CASE("rx_roi", "[.cmdcall]") {
                     stringMax = std::to_string(portSize.x + delta);
                     std::ostringstream oss, oss1;
                     REQUIRE_NOTHROW(caller.call(
-                        "rx_roi", {"[" + stringMin + ", " + stringMax + ", 20, 30]"}, -1, PUT, oss));
+                        "rx_roi",
-                    REQUIRE(oss.str() == "rx_roi [[" + stringMin + ", " + stringMax + ", 20, 30]]\n");
+                        {"[" + stringMin + ", " + stringMax + ", 20, 30]"}, -1,
-                    REQUIRE_NOTHROW(
+                        PUT, oss));
-                        caller.call("rx_roi", {}, 0, GET, oss1));
+                    REQUIRE(oss.str() == "rx_roi [[" + stringMin + ", " +
+                                             stringMax + ", 20, 30]]\n");
+                    REQUIRE_NOTHROW(caller.call("rx_roi", {}, 0, GET, oss1));
                     // eiger returns 2 values for 2 ports per module
                     if (det_type == defs::EIGER) {
-                        REQUIRE(oss1.str() == "rx_roi [[" + stringMin + ", " + std::to_string(portSize.x - 1) + ", 20, 30], [0, " + std::to_string(delta) + ", 20, 30]]\n");
+                        REQUIRE(oss1.str() ==
+                                "rx_roi [[" + stringMin + ", " +
+                                    std::to_string(portSize.x - 1) +
+                                    ", 20, 30], [0, " + std::to_string(delta) +
+                                    ", 20, 30]]\n");
                     }
                     // others return only 1 roi per module (1 port per module)
                     else {
-                        REQUIRE(oss1.str() == "rx_roi [[" + stringMin + ", " + std::to_string(portSize.x - 1) + ", 20, 30]]\n");
+                        REQUIRE(oss1.str() ==
+                                "rx_roi [[" + stringMin + ", " +
+                                    std::to_string(portSize.x - 1) +
+                                    ", 20, 30]]\n");
                     }
                 }
             }
 
             // multiple ports vertically
-            if (((det_type == defs::JUNGFRAU || det_type == defs::MOENCH) && (numinterfaces == 2)) || 
+            if (((det_type == defs::JUNGFRAU || det_type == defs::MOENCH) &&
+                 (numinterfaces == 2)) ||
                 (det.size() == 2 && det.getModuleGeometry().y > 1)) {
                 std::string stringMin = std::to_string(portSize.y);
                 std::string stringMax = std::to_string(portSize.y + 1);
 
                 // separated by space is allowed
-                REQUIRE_NOTHROW(caller.call(
+                REQUIRE_NOTHROW(
-                    "rx_roi", {"[5, 10, 20, 30]", "[25, 28, " + stringMin + ", " + stringMax + "]"}, -1, PUT));
+                    caller.call("rx_roi",
+                                {"[5, 10, 20, 30]", "[25, 28, " + stringMin +
+                                                        ", " + stringMax + "]"},
+                                -1, PUT));
                 std::ostringstream oss;
-                // separated by semicolon is allowed
+                // separated by semicolon is allowed with quotes (skips
-                REQUIRE_NOTHROW(caller.call(
+                // cmdParser)
-                    "rx_roi", {"[5, 10, 20, 30];[25, 28, " + stringMin + ", " +  stringMax + "]"}, -1, PUT, oss));
+                REQUIRE_NOTHROW(
-                REQUIRE(oss.str() ==
+                    caller.call("rx_roi",
-                    "rx_roi [[5, 10, 20, 30], [25, 28, " + stringMin + ", " +  stringMax + "]]\n");
+                                {"[5, 10, 20, 30];[25, 28, " + stringMin +
+                                 ", " + stringMax + "]"},
+                                -1, PUT, oss));
+                REQUIRE(oss.str() == "rx_roi [[5, 10, 20, 30], [25, 28, " +
+                                         stringMin + ", " + stringMax + "]]\n");
 
                 // verify individual roi
                 {
@@ -665,22 +698,37 @@ TEST_CASE("rx_roi", "[.cmdcall]") {
                     stringMax = std::to_string(portSize.y + delta);
                     std::ostringstream oss, oss1;
                     REQUIRE_NOTHROW(caller.call(
-                        "rx_roi", {"[ 20, 30, " + stringMin + ", " + stringMax + "]"}, -1, PUT, oss));
+                        "rx_roi",
-                    REQUIRE(oss.str() == "rx_roi [[20, 30, " + stringMin + ", " + stringMax + "]]\n");
+                        {"[ 20, 30, " + stringMin + ", " + stringMax + "]"}, -1,
-                    REQUIRE_NOTHROW(
+                        PUT, oss));
-                        caller.call("rx_roi", {}, 0, GET, oss1));
+                    REQUIRE(oss.str() == "rx_roi [[20, 30, " + stringMin +
+                                             ", " + stringMax + "]]\n");
+                    REQUIRE_NOTHROW(caller.call("rx_roi", {}, 0, GET, oss1));
 
-                    // non-eiger with 2 interfaces returns 2 values for 2 ports per module
+                    // non-eiger with 2 interfaces returns 2 values for 2 ports
-                    if ((det_type == defs::JUNGFRAU || det_type == defs::MOENCH) && (numinterfaces == 2)) {
+                    // per module
-                        REQUIRE(oss1.str() == "rx_roi [[20, 30, " + stringMin + ", " + std::to_string(portSize.y - 1) + "], [20, 30, 0, " + std::to_string(delta) + "]]\n");
+                    if ((det_type == defs::JUNGFRAU ||
+                         det_type == defs::MOENCH) &&
+                        (numinterfaces == 2)) {
+                        REQUIRE(oss1.str() ==
+                                "rx_roi [[20, 30, " + stringMin + ", " +
+                                    std::to_string(portSize.y - 1) +
+                                    "], [20, 30, 0, " + std::to_string(delta) +
+                                    "]]\n");
                     }
                     // others return only 1 roi per module (1 port per module)
                     else {
                         // (eiger 2 ports)
                         if (det_type == defs::EIGER) {
-                            REQUIRE(oss1.str() == "rx_roi [[20, 30, " + stringMin + ", " + std::to_string(portSize.y - 1) + "], [-1, -1]]\n");
+                            REQUIRE(oss1.str() ==
+                                    "rx_roi [[20, 30, " + stringMin + ", " +
+                                        std::to_string(portSize.y - 1) +
+                                        "], [-1, -1]]\n");
                         } else {
-                            REQUIRE(oss1.str() == "rx_roi [[20, 30, " + stringMin + ", " + std::to_string(portSize.y - 1) + "]]\n");
+                            REQUIRE(oss1.str() ==
+                                    "rx_roi [[20, 30, " + stringMin + ", " +
+                                        std::to_string(portSize.y - 1) +
+                                        "]]\n");
                         }
                     }
                 }
@@ -736,7 +784,6 @@ TEST_CASE("rx_roi", "[.cmdcall]") {
     }
 }
 
-
 TEST_CASE("rx_clearroi", "[.cmdcall]") {
     Detector det;
     Caller caller(&det);

slsReceiverSoftware/src/ClientInterface.cpp

@@ -1843,7 +1843,7 @@ int ClientInterface::get_roi_metadata(Interface &socket) {
     if (detType == CHIPTESTBOARD || detType == XILINX_CHIPTESTBOARD)
         functionNotImplemented();
     auto retvals = impl()->getMultiROIMetadata();
-    LOG(logINFORED) << "Receiver ROI metadata retval:" << ToString(retvals);
+    LOG(logDEBUG1) << "Receiver ROI metadata retval:" << ToString(retvals);
     auto size = static_cast<int>(retvals.size());
     socket.Send(size);
     if (size > 0)

slsReceiverSoftware/src/DataProcessor.cpp

@@ -227,8 +227,8 @@ std::string DataProcessor::CreateVirtualFile(
     return masterFileUtility::CreateVirtualHDF5File(
         filePath, fileNamePrefix, fileIndex, overWriteEnable, silentMode,
         modulePos, generalData->numUDPInterfaces, framesPerFile,
-        generalData->nPixelsX, ny, generalData->dynamicRange,
+        generalData->nPixelsX, ny, generalData->dynamicRange, numFramesCaught,
-        numFramesCaught, numModX, numModY, dataFile->GetPDataType(),
+        numModX, numModY, dataFile->GetPDataType(),
         dataFile->GetParameterNames(), dataFile->GetParameterDataTypes(),
         hdf5LibMutex, gotthard25um, multiRoiMetadata);
 }
@@ -238,18 +238,14 @@ void DataProcessor::LinkFileInMaster(const std::string &masterFileName,
                                      const bool silentMode,
                                      std::mutex *hdf5LibMutex) {
 
-    /*if (!multiRoiMetadata.empty()) {
-        throw std::runtime_error(
-            "Should not be here, roi with hdf5 virtual should throw.");
-    }*/
     std::string fname{virtualFileName}, masterfname{masterFileName};
     // if no virtual file, link data file
     if (virtualFileName.empty()) {
         fname = dataFile->GetFileName();
     }
-    masterFileUtility::LinkHDF5FileInMaster(masterfname, fname,
+    masterFileUtility::LinkHDF5FileInMaster(
-                                            dataFile->GetParameterNames(),
+        masterfname, fname, dataFile->GetParameterNames(), silentMode,
-                                            silentMode, hdf5LibMutex, multiRoiMetadata.size());
+        hdf5LibMutex, multiRoiMetadata.size());
 }
 #endif
 

slsReceiverSoftware/src/Implementation.cpp

@@ -166,7 +166,6 @@ void Implementation::setDetectorType(const detectorType d) {
     SetLocalNetworkParameters();
     SetupFifoStructure();
 
-
     // create threads
     for (int i = 0; i < generalData->numUDPInterfaces; ++i) {
 
@@ -418,12 +417,11 @@ void Implementation::setPortROIs(const std::vector<defs::ROI> &args) {
         if (it.completeRoi() || it.noRoi()) {
             continue; // valid
         }
-         if (it.xmin < 0 || it.xmax < 0 || 
+        if (it.xmin < 0 || it.xmax < 0 || it.xmin >= nx || it.xmax >= nx) {
-            it.xmin >= nx || it.xmax >= nx) {
             throw RuntimeError("Invalid ROIvx coordinates: " + ToString(it));
         }
-        if (ny > 1 && (it.ymin < 0 || it.ymax < 0 || 
+        if (ny > 1 &&
-            it.ymin >= ny || it.ymax >= ny)) {
+            (it.ymin < 0 || it.ymax < 0 || it.ymin >= ny || it.ymax >= ny)) {
             throw RuntimeError("Invalid ROI y coordinates: " + ToString(it));
         }
     }

slsReceiverSoftware/src/MasterFileUtility.cpp

@@ -49,7 +49,8 @@ std::string CreateMasterBinaryFile(const std::string &filePath,
 void LinkHDF5FileInMaster(std::string &masterFileName,
                           std::string &dataFilename,
                           std::vector<std::string> parameterNames,
-                          const bool silentMode, std::mutex *hdf5LibMutex, size_t multiRoiSize) {
+                          const bool silentMode, std::mutex *hdf5LibMutex,
+                          size_t multiRoiSize) {
 
     std::lock_guard<std::mutex> lock(*hdf5LibMutex);
     std::unique_ptr<H5::H5File> fd{nullptr};
@@ -74,8 +75,7 @@ void LinkHDF5FileInMaster(std::string &masterFileName,
             if (multiRoiSize > 1)
                 datasetname += ('_' + std::to_string(iRoi));
             H5::DataSet dset = fd->openDataSet(datasetname);
-            std::string linkname =
+            std::string linkname = std::string("/entry/data/") + datasetname;
-                std::string("/entry/data/") + datasetname;
             if (H5Lcreate_external(dataFilename.c_str(), datasetname.c_str(),
                                    masterfd.getLocId(), linkname.c_str(),
                                    H5P_DEFAULT, H5P_DEFAULT) < 0) {
@@ -169,7 +169,8 @@ std::string CreateMasterHDF5File(const std::string &filePath,
     return fileName;
 }
 
-defs::ROI GetGlobalPortRoi(const int iPort, const defs::xy portSize, const int numPortsY) {
+defs::ROI GetGlobalPortRoi(const int iPort, const defs::xy portSize,
+                           const int numPortsY) {
     defs::xy portPos = {(iPort / numPortsY), (iPort % numPortsY)};
     const int xmin = portSize.x * portPos.x;
     const int xmax = xmin + portSize.x - 1;
@@ -194,10 +195,10 @@ std::string CreateVirtualHDF5File(
     const std::string &filePath, const std::string &fileNamePrefix,
     const uint64_t fileIndex, const bool overWriteEnable, const bool silentMode,
     const int modulePos, const int numUnitsPerReadout,
-    const uint32_t maxFramesPerFile, const int nPixelsX,
+    const uint32_t maxFramesPerFile, const int nPixelsX, const int nPixelsY,
-    const int nPixelsY, const uint32_t dynamicRange,
+    const uint32_t dynamicRange, const uint64_t numImagesCaught,
-    const uint64_t numImagesCaught, const int numModX, const int numModY,
+    const int numModX, const int numModY, const H5::DataType dataType,
-    const H5::DataType dataType, const std::vector<std::string> parameterNames,
+    const std::vector<std::string> parameterNames,
     const std::vector<H5::DataType> parameterDataTypes,
     std::mutex *hdf5LibMutex, bool gotthard25um,
     std::vector<defs::ROI> multiRoi) {
@@ -210,16 +211,17 @@ std::string CreateVirtualHDF5File(
     // roi not allowed in 4 bit mode and with gotthard 2 mods
     if (!completeRoi) {
         if (dynamicRange == 4) {
-            throw std::runtime_error("Skipping virtual hdf5 file since rx_roi is "
+            throw std::runtime_error(
+                "Skipping virtual hdf5 file since rx_roi is "
                 "enabled and it is in 4 bit mode.");
         }
         if (gotthard25um && (numModX * numModY) == 2) {
-            throw std::runtime_error("Skipping virtual hdf5 file since rx_roi is "
+            throw std::runtime_error(
+                "Skipping virtual hdf5 file since rx_roi is "
                 "enabled and there are 2 Gotthard 25um modules.");
         }
     }
 
-
     // virtual file name
     std::ostringstream osfn;
     osfn << filePath << "/" << fileNamePrefix << "_virtual"
@@ -254,11 +256,12 @@ std::string CreateVirtualHDF5File(
             auto currentRoi = multiRoi[iRoi];
             defs::xy detectorSize = {nPixelsX * numModX, nPixelsY * numModY};
             if (completeRoi) {
-                currentRoi = defs::ROI{0, detectorSize.x - 1, 
+                currentRoi =
-                                        0, detectorSize.y - 1};
+                    defs::ROI{0, detectorSize.x - 1, 0, detectorSize.y - 1};
             }
             if (multiRoi[iRoi].completeRoi() && iRoi != 0)
-                throw RuntimeError("Cannot have complete roi and multiple rois");
+                throw RuntimeError(
+                    "Cannot have complete roi and multiple rois");
 
             // get detector shape and number of ports in roi
             defs::xy portSize{nPixelsX, nPixelsY};
@@ -309,7 +312,9 @@ std::string CreateVirtualHDF5File(
                 hsize_t blockSizePara[VDS_PARA_RANK] = {nSrcFileImages, 1};
 
                 // following recalculated for every readout
-                hsize_t blockSize[DATA_RANK] = {nSrcFileImages, static_cast<hsize_t>(nPixelsY), static_cast<hsize_t>(nPixelsX)};
+                hsize_t blockSize[DATA_RANK] = {nSrcFileImages,
+                                                static_cast<hsize_t>(nPixelsY),
+                                                static_cast<hsize_t>(nPixelsX)};
                 hsize_t startLocation[DATA_RANK] = {framesSaved, 0, 0};
                 hsize_t startLocationPara[VDS_PARA_RANK] = {framesSaved, 0};
 
@@ -318,8 +323,10 @@ std::string CreateVirtualHDF5File(
                     strideBetweenBlocks[2] = 2;
                 }
 
-                for (unsigned int iReadout = 0; iReadout < nTotalPorts; ++iReadout) {
+                for (unsigned int iReadout = 0; iReadout < nTotalPorts;
-                    auto globalPortRoi = GetGlobalPortRoi(iReadout, portSize, numModY);
+                     ++iReadout) {
+                    auto globalPortRoi =
+                        GetGlobalPortRoi(iReadout, portSize, numModY);
                     if (!globalPortRoi.overlap(currentRoi))
                         continue;
 
@@ -338,7 +345,8 @@ std::string CreateVirtualHDF5File(
                         blockSize[1] = portRoiHeight;
                         blockSize[2] = portRoiWidth;
                     }
-                    // interleaving for g2 (startLocation is 0 and 1) (g2 had no roi)
+                    // interleaving for g2 (startLocation is 0 and 1) (g2 had no
+                    // roi)
                     else {
                         ++startLocation[2];
                     }
@@ -369,25 +377,15 @@ std::string CreateVirtualHDF5File(
                     }
 
                     // source dataspace
-                    hsize_t srcDims[DATA_RANK] = {nSrcFileImages, portRoiHeight, portRoiWidth};
+                    hsize_t srcDims[DATA_RANK] = {nSrcFileImages, portRoiHeight,
-                    hsize_t srcDimsMax[DATA_RANK] = {H5S_UNLIMITED, portRoiHeight,
                                                   portRoiWidth};
+                    hsize_t srcDimsMax[DATA_RANK] = {
+                        H5S_UNLIMITED, portRoiHeight, portRoiWidth};
                     H5::DataSpace srcDataSpace(DATA_RANK, srcDims, srcDimsMax);
                     hsize_t srcDimsPara[PARA_RANK] = {nSrcFileImages};
                     hsize_t srcDimsMaxPara[PARA_RANK] = {H5S_UNLIMITED};
-                    H5::DataSpace srcDataSpacePara(PARA_RANK, srcDimsPara, srcDimsMaxPara); 
+                    H5::DataSpace srcDataSpacePara(PARA_RANK, srcDimsPara,
-                    // temporary fixfor corner case bug:
+                                                   srcDimsMaxPara);
-                    // (framescaught not multiple of framesperfile,
-                    // virtual parameter datasets error loading (bad scalar
-                    // value))
-                    // TODO WHY????
-                    /*if (nDimz != maxFramesPerFile) {
-                        hsize_t count[1] = {nDimz};
-                        hsize_t start[1] = {0};
-                        srcDataSpacePara.selectHyperslab(
-                            H5S_SELECT_SET, count, start,
-                    strideBetweenBlocksPara, blockSizePara);
-                    }*/
 
                     // mapping of property list
                     plist.setVirtual(vdsDataSpace, relative_srcFileName.c_str(),

slsReceiverSoftware/src/MasterFileUtility.h

@@ -21,7 +21,8 @@ std::string CreateMasterBinaryFile(const std::string &filePath,
 void LinkHDF5FileInMaster(std::string &masterFileName,
                           std::string &dataFilename,
                           std::vector<std::string> parameterNames,
-                          const bool silentMode, std::mutex *hdf5LibMutex, size_t multiRoiSize);
+                          const bool silentMode, std::mutex *hdf5LibMutex,
+                          size_t multiRoiSize);
 
 std::string CreateMasterHDF5File(const std::string &filePath,
                                  const std::string &fileNamePrefix,
@@ -29,17 +30,18 @@ std::string CreateMasterHDF5File(const std::string &filePath,
                                  const bool overWriteEnable,
                                  const bool silentMode, MasterAttributes *attr,
                                  std::mutex *hdf5LibMutex);
-defs::ROI GetGlobalPortRoi(const int iPort, const defs::xy portSize, const int numPortsY);
+defs::ROI GetGlobalPortRoi(const int iPort, const defs::xy portSize,
+                           const int numPortsY);
 int GetNumPortsInRoi(const defs::ROI roi, const defs::xy portSize);
 
 std::string CreateVirtualHDF5File(
     const std::string &filePath, const std::string &fileNamePrefix,
     const uint64_t fileIndex, const bool overWriteEnable, const bool silentMode,
     const int modulePos, const int numUnitsPerReadout,
-    const uint32_t maxFramesPerFile, const int nPixelsX,
+    const uint32_t maxFramesPerFile, const int nPixelsX, const int nPixelsY,
-    const int nPixelsY, const uint32_t dynamicRange,
+    const uint32_t dynamicRange, const uint64_t numImagesCaught,
-    const uint64_t numImagesCaught, const int numModX, const int numModY,
+    const int numModX, const int numModY, const H5::DataType dataType,
-    const H5::DataType dataType, const std::vector<std::string> parameterNames,
+    const std::vector<std::string> parameterNames,
     const std::vector<H5::DataType> parameterDataTypes,
     std::mutex *hdf5LibMutex, bool gotthard25um,
     std::vector<defs::ROI> multiRoi);

slsReceiverSoftware/src/receiver_defs.h

@@ -19,8 +19,8 @@ namespace sls {
 // files
 
 // versions
-#define HDF5_WRITER_VERSION   (6.7) // 1 decimal places
+#define HDF5_WRITER_VERSION   (7.0) // 1 decimal places
-#define BINARY_WRITER_VERSION (7.3) // 1 decimal places
+#define BINARY_WRITER_VERSION (8.0) // 1 decimal places
 
 #define MAX_FRAMES_PER_FILE            20000
 #define SHORT_MAX_FRAMES_PER_FILE      100000

slsSupportLib/include/sls/ToString.h

@@ -349,5 +349,4 @@ std::vector<T> StringTo(const std::vector<std::string> &strings) {
     return result;
 }
 
-
 } // namespace sls

slsSupportLib/include/sls/sls_detector_defs.h

@@ -230,12 +230,8 @@ class slsDetectorDefs {
         ROI(int xmin, int xmax) : xmin(xmin), xmax(xmax){};
         ROI(int xmin, int xmax, int ymin, int ymax)
             : xmin(xmin), xmax(xmax), ymin(ymin), ymax(ymax){};
-        constexpr int width() const {
+        constexpr int width() const { return (xmax - xmin + 1); }
-            return (xmax - xmin + 1);
+        constexpr int height() const { return (ymax - ymin + 1); }
-        }   
-        constexpr int height() const {
-            return (ymax - ymin + 1);
-        }
         constexpr std::array<int, 4> getIntArray() const {
             return std::array<int, 4>({xmin, xmax, ymin, ymax});
         }