1010/check overlap rxroi deactivate (#1579)

* wip

* fixed

* tests work for both roi and port disable, but only in 10g mode

* minor

* updating tests, but going to fail

* refactoring from dev

* renamed function to reflect rx roi

slsDetectorSoftware/include/sls/Detector.h

@@ -1004,6 +1004,9 @@ class Detector {
 
     void clearRxROI();
 
+    /** Returns true if the current ROI covers the complete detector (default)*/
+    bool isCompleteRxRoi() const;
+
     ///@}
 
     /** @name File */

slsDetectorSoftware/src/Detector.cpp

@@ -1463,6 +1463,8 @@ void Detector::setRxROI(const std::vector<defs::ROI> &args) {
 
 void Detector::clearRxROI() { pimpl->clearRxROI(); }
 
+bool Detector::isCompleteRxRoi() const { return pimpl->isCompleteRxRoi(); }
+
 // File
 
 Result<defs::fileFormat> Detector::getFileFormat(Positions pos) const {
@@ -1769,6 +1771,7 @@ Result<bool> Detector::getDataStream(const defs::portPosition port,
 
 void Detector::setDataStream(const defs::portPosition port, const bool enable,
                              Positions pos) {
+    pimpl->validatePortEnable(port, enable, pos);
     pimpl->Parallel(&Module::setDataStream, pos, port, enable);
 }
 

slsDetectorSoftware/src/DetectorImpl.cpp

@@ -48,6 +48,15 @@ bool DetectorImpl::isAllPositions(Positions pos) const {
             (pos.size() == modules.size()));
 }
 
+std::vector<int> DetectorImpl::fillInPositions(Positions pos) const {
+    if (isAllPositions(pos)) {
+        std::vector<int> positions(modules.size());
+        std::iota(begin(positions), end(positions), 0);
+        return positions;
+    }
+    return pos;
+}
+
 void DetectorImpl::setAcquiringFlag(bool flag) { shm()->acquiringFlag = flag; }
 
 int DetectorImpl::getDetectorIndex() const { return detectorIndex; }
@@ -1784,6 +1793,48 @@ void DetectorImpl::convertGlobalRoiToPortLevel(
     }
 }
 
+void DetectorImpl::validatePortEnable(const defs::portPosition port,
+                                      const bool enable,
+                                      std::vector<int> pos) const {
+    if (shm()->detType != EIGER)
+        return;
+    if (isCompleteRxRoi())
+        return;
+    auto positions = fillInPositions(pos);
+    for (int i : positions) {
+        if (i < 0 || static_cast<size_t>(i) >= modules.size()) {
+            throw RuntimeError("Invalid module index: " + std::to_string(i));
+        }
+        auto modRois = modules[i]->getRxROI();
+        auto portIndex =
+            static_cast<int>(port); // only eiger (no top, bottom for port)
+        validatePortEnableRoiState(i, port, modRois[portIndex], enable);
+    }
+}
+
+/** Assumption:
+ * 1. Check only for Eiger because port disable is only for Eiger now
+ * 2. This function called only when an Roi being set (not complete detector)
+ * ie. complete ROI enabled and some ports disabled is allowed
+ */
+void DetectorImpl::validatePortEnableRoiState(const int moduleIndex,
+                                              defs::portPosition changedPort,
+                                              const defs::ROI &portRoi,
+                                              bool enabled) const {
+    if (shm()->detType != EIGER)
+        return;
+    // enabled or disabled and no roi
+    if (enabled || portRoi.noRoi())
+        return;
+    // diabled but ROI specified
+    std::ostringstream oss;
+    oss << ToString(changedPort) << " port of module " << moduleIndex << " ("
+        << modules[moduleIndex]->getHostname()
+        << ") is not active but ROI is specified for it: " << ToString(portRoi)
+        << ". Please disable the ROI for this port or enable the port.";
+    throw RuntimeError(oss.str());
+}
+
 void DetectorImpl::setRxROI(const std::vector<defs::ROI> &args) {
     if (shm()->detType == CHIPTESTBOARD ||
         shm()->detType == defs::XILINX_CHIPTESTBOARD) {
@@ -1798,10 +1849,13 @@ 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");
+    std::vector<std::vector<defs::ROI>> moduleRois;
 
+    // further validate and create port level rois for each module
     for (size_t iModule = 0; iModule < modules.size(); ++iModule) {
         auto moduleGlobalRoi = getModuleROI(iModule);
         // at most 2 rois per module (for each port)
@@ -1813,15 +1867,29 @@ void DetectorImpl::setRxROI(const std::vector<defs::ROI> &args) {
                 roi.ymax = -1;
             }
         }
-
-        // check overlap with module
+        // check overlap of each roi with module
         for (const auto &arg : args) {
             if (arg.overlap(moduleGlobalRoi)) {
                 convertGlobalRoiToPortLevel(arg, moduleGlobalRoi, portRois);
             }
         }
-        modules[iModule]->setRxROI(portRois);
+        // validate roi against port enables
+        if (shm()->detType == EIGER) {
+            for (int iport = 0; iport != nPortsPerModule; ++iport) {
+                auto port = static_cast<defs::portPosition>(iport);
+                auto enable = modules[iModule]->getDataStream(port);
+                validatePortEnableRoiState(iModule, port, portRois[iport],
+                                           enable);
+            }
+        }
+        moduleRois.push_back(portRois);
     }
+
+    // set rois for each module
+    for (size_t iModule = 0; iModule < modules.size(); ++iModule) {
+        modules[iModule]->setRxROI(moduleRois[iModule]);
+    }
+
     // metadata
     modules[0]->setRxROIMetadata(args);
 }
@@ -1836,6 +1904,14 @@ void DetectorImpl::clearRxROI() {
     modules[0]->setRxROIMetadata(std::vector<defs::ROI>(1));
 }
 
+bool DetectorImpl::isCompleteRxRoi() const {
+    const auto rois = getRxROI();
+    if (rois.empty() || (rois.size() == 1 && rois[0].completeRoi())) {
+        return true;
+    }
+    return false;
+}
+
 void DetectorImpl::getBadChannels(const std::string &fname,
                                   Positions pos) const {
     auto res = Parallel(&Module::getBadChannels, pos);
@@ -1999,4 +2075,4 @@ void DetectorImpl::setCtbSlowADCName(const defs::dacIndex index,
     ctb_shm()->setSlowADCName(static_cast<int>(index - defs::SLOW_ADC0), name);
 }
 
-} // namespace sls
+} // namespace sls

slsDetectorSoftware/src/DetectorImpl.h

@@ -183,6 +183,8 @@ class DetectorImpl : public virtual slsDetectorDefs {
 
     bool isAllPositions(Positions pos) const;
 
+    std::vector<int> fillInPositions(Positions pos) const;
+
     /** set acquiring flag in shared memory */
     void setAcquiringFlag(bool flag);
 
@@ -296,8 +298,16 @@ class DetectorImpl : public virtual slsDetectorDefs {
 
     defs::xy getPortGeometry() const;
     std::vector<defs::ROI> getRxROI(int module_id = -1) const;
+    void validatePortEnable(const defs::portPosition port, const bool enable,
+                            std::vector<int> pos) const;
+    void validatePortEnableRoiState(const int moduleIndex,
+                                    defs::portPosition changedPort,
+                                    const defs::ROI &portRoi,
+                                    bool enabled) const;
+
     void setRxROI(const std::vector<defs::ROI> &args);
     void clearRxROI();
+    bool isCompleteRxRoi() const;
 
     void getBadChannels(const std::string &fname, Positions pos) const;
     void setBadChannels(const std::string &fname, Positions pos);
@@ -448,4 +458,4 @@ class DetectorImpl : public virtual slsDetectorDefs {
     void *pCallbackArg{nullptr};
 };
 
-} // namespace sls
+} // namespace sls

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

@@ -855,6 +855,66 @@ TEST_CASE("rx_roi", "[.detectorintegration][.disable_check_data_file]") {
     }
 }
 
+TEST_CASE("rx_roi_port_disabled", "[.detectorintegration]") {
+    Detector det;
+    auto det_type = det.getDetectorType().squash();
+    if (det_type == defs::EIGER) {
+        auto prev_roi = det.getRxROI();
+        std::vector<defs::portPosition> ports = {defs::LEFT, defs::RIGHT};
+        auto prev_port0 = det.getDataStream(ports[0], {0})[0];
+        auto prev_port1 = det.getDataStream(ports[1], {0})[0];
+        bool prev_tengiga = det.getTenGiga().squash(false);
+
+        std::vector<defs::ROI> rois = {defs::ROI{0, 10, 0, 20},
+                                       defs::ROI{600, 610, 0, 30}};
+        det.setTenGiga(true);
+
+        det.clearRxROI();
+        det.setDataStream(ports[0], true, {0});
+        det.setDataStream(ports[1], true, {0});
+        det.setRxROI(rois);
+        REQUIRE_THROWS(det.setDataStream(ports[0], false));
+        REQUIRE_THROWS(det.setDataStream(ports[1], false));
+
+        det.clearRxROI();
+        det.setDataStream(ports[0], true, {0});
+        det.setDataStream(ports[1], true, {0});
+        det.setRxROI({rois[0]});
+        REQUIRE_THROWS(det.setDataStream(ports[0], false));
+        REQUIRE_NOTHROW(det.setDataStream(ports[1], false));
+
+        det.clearRxROI();
+        det.setDataStream(ports[0], true, {0});
+        det.setDataStream(ports[1], true, {0});
+        det.setRxROI({rois[1]});
+        REQUIRE_NOTHROW(det.setDataStream(ports[0], false));
+        REQUIRE_THROWS(det.setDataStream(ports[1], false));
+
+        det.clearRxROI();
+        det.setDataStream(ports[0], false, {0});
+        det.setDataStream(ports[1], true, {0});
+        REQUIRE_THROWS(det.setRxROI({rois[0]}));
+        REQUIRE_NOTHROW(det.setRxROI({rois[1]}));
+
+        det.clearRxROI();
+        det.setDataStream(ports[0], true, {0});
+        det.setDataStream(ports[1], false, {0});
+        REQUIRE_NOTHROW(det.setRxROI({rois[0]}));
+        REQUIRE_THROWS(det.setRxROI({rois[1]}));
+
+        /* eiger virtual detector doesnt understand it cant disable in 1g mode,
+         * so cant test in 1g mode */
+
+        if (prev_roi.size() == 1 && prev_roi[0].completeRoi())
+            det.clearRxROI();
+        else
+            det.setRxROI(prev_roi);
+        det.setDataStream(ports[0], prev_port0, {0});
+        det.setDataStream(ports[1], prev_port1, {0});
+        det.setTenGiga(prev_tengiga);
+    }
+}
+
 TEST_CASE("rx_clearroi", "[.detectorintegration]") {
     Detector det;
     Caller caller(&det);