M3badchannels (#526)

* badchannels for m3 and modify for g2 (file from single and multi)

* m3: invert polarity of bit 7 and 11 signals from setmodule, allow commas in bad channel file

* badchannel file can take commas, colons and comments (also taking care of spaces at the end of channel numbers)

* tests 'badchannels' and 'Channel file reading' added, removing duplicates in badchannel list, defining macro for num counters in client side

* fix segfault when list from file is empty, 

* fix tests assertion for ctbconfig (adding message) for c++11

* fixed badchannels in m3server (clocking in trimming) 

* badchannel tests can be run from any folder (finds the file)

RELEASE.txt

@@ -92,6 +92,9 @@ This document describes the differences between v7.0.0 and v6.x.x
 - jungfrau master
 - g2 parallel command
 - jungfrau sync
+- m3 bad channels (badchannel file also for g2 extended to include commas and colons, remove duplicates)
+- m3 fix for gain caps to invert where needed when loading from trimbit file (fix for feature might have been added only in developer branch)
+- pat loop and wait address default
 - ctb and moench Fw fixed (to work with pattern commdand) )addreess length
 - jungfrau reset core and usleep removed (fix for 6.1.1 is now fixed in firmware)
 

slsDetectorServers/gotthard2DetectorServer/slsDetectorFunctionList.c

@@ -3024,8 +3024,8 @@ void setVetoAlgorithm(enum vetoAlgorithm alg,
     bus_w(addr, value);
 }
 
-void setBadChannels(int nch, int *channels) {
+int setBadChannels(int numChannels, int *channelList) {
-    LOG(logINFO, ("Setting %d bad channels\n", nch));
+    LOG(logINFO, ("Setting %d bad channels\n", numChannels));
 
     int numAddr = MASK_STRIP_NUM_REGS;
     int startAddr = MASK_STRIP_START_REG;
@@ -3037,42 +3037,44 @@ void setBadChannels(int nch, int *channels) {
     }
 
     // setting badchannels, loop through list
-    for (int i = 0; i < nch; ++i) {
+    for (int i = 0; i != numChannels; ++i) {
-        LOG(logINFO, ("\t[%d]: %d\n", i, channels[i]));
+        LOG(logINFO, ("\t[%d]: %d\n", i, channelList[i]));
-        int iaddr = channels[i] / 32;
+        int iaddr = channelList[i] / 32;
-        int iBit = channels[i] % 32;
+        int iBit = channelList[i] % 32;
         uint32_t addr = startAddr + iaddr * REG_OFFSET;
         LOG(logDEBUG1,
             ("val:%d iaddr:%d iBit:%d, addr:0x%x old:0x%x val:0x%x\n",
-             channels[i], iaddr, iBit, addr, bus_r(addr), (1 << iBit)));
+             channelList[i], iaddr, iBit, addr, bus_r(addr), (1 << iBit)));
         bus_w(addr, bus_r(addr) | (1 << iBit));
     }
+    return OK;
 }
 
-int *getBadChannels(int *nch) {
+int *getBadChannels(int *numChannels) {
     int *retvals = NULL;
     // count number of bad channels
-    *nch = 0;
+    *numChannels = 0;
-    for (int i = 0; i < MASK_STRIP_NUM_REGS; ++i) {
+    for (int i = 0; i != MASK_STRIP_NUM_REGS; ++i) {
         uint32_t addr = MASK_STRIP_START_REG + i * REG_OFFSET;
-        *nch += __builtin_popcount(bus_r(addr));
+        *numChannels += __builtin_popcount(bus_r(addr));
     }
-    if (*nch > 0) {
+    if (*numChannels > 0) {
         // get list of bad channels
-        retvals = malloc(*nch * sizeof(int));
+        retvals = malloc(*numChannels * sizeof(int));
+        memset(retvals, 0, *numChannels * sizeof(int));
         if (retvals == NULL) {
-            *nch = -1;
+            *numChannels = -1;
             return NULL;
         }
         int chIndex = 0;
         int numAddr = MASK_STRIP_NUM_REGS;
         // loop through registers
-        for (int iaddr = 0; iaddr < numAddr; ++iaddr) {
+        for (int iaddr = 0; iaddr != numAddr; ++iaddr) {
             // calculate address and get value
             uint32_t addr = MASK_STRIP_START_REG + iaddr * REG_OFFSET;
             uint32_t val = bus_r(addr);
             // loop through 32 bits
-            for (int iBit = 0; iBit < 32; ++iBit) {
+            for (int iBit = 0; iBit != 32; ++iBit) {
                 // masked, add to list
                 if ((val >> iBit) & 0x1) {
                     LOG(logDEBUG1, ("iaddr:%d iBit:%d val:0x%x, ch:%d\n", iaddr,
@@ -3084,7 +3086,7 @@ int *getBadChannels(int *nch) {
     }
     // debugging
     LOG(logDEBUG1, ("Reading Bad channel list\n"));
-    for (int i = 0; i < (*nch); ++i) {
+    for (int i = 0; i != (*numChannels); ++i) {
         LOG(logDEBUG1, ("[%d]: %d\n", i, retvals[i]));
     }
     return retvals;

slsDetectorServers/mythen3DetectorServer/mythen3.c

@@ -97,6 +97,10 @@ patternParameters *setChipStatusRegisterPattern(int csr) {
     return pat;
 }
 
+void flipNegativePolarityBits(int *csr) {
+    (*csr) ^= ((1 << _CSR_C10pre) | (1 << _CSR_C15pre));
+}
+
 int getGainCaps() {
     int csr = chipStatusRegister;
     // Translates bit representation
@@ -209,7 +213,8 @@ int M3SetNegativePolarity(int enable) {
     return csr;
 }
 
-patternParameters *setChannelRegisterChip(int ichip, int *mask, int *trimbits) {
+patternParameters *setChannelRegisterChip(int ichip, char *mask,
+                                          int *trimbits) {
 
     patternParameters *pat = malloc(sizeof(patternParameters));
     memset(pat, 0, sizeof(patternParameters));
@@ -279,44 +284,44 @@ patternParameters *setChannelRegisterChip(int ichip, int *mask, int *trimbits) {
     // for each channel (all chips)
     for (int ich = 0; ich < NCHAN_1_COUNTER; ich++) {
         LOG(logDEBUG1, (" Chip %d, Channel %d\n", ichip, ich));
-        int val =
+        int chanReg =
-            trimbits[ichip * NCHAN_1_COUNTER * NCOUNTERS + NCOUNTERS * ich] +
+            64 *
+            (trimbits[ichip * NCHAN_1_COUNTER * NCOUNTERS + NCOUNTERS * ich] +
              trimbits[ichip * NCHAN_1_COUNTER * NCOUNTERS + NCOUNTERS * ich +
                       1] *
                  64 +
              trimbits[ichip * NCHAN_1_COUNTER * NCOUNTERS + NCOUNTERS * ich +
                       2] *
-                64 * 64;
+                 64 * 64);
 
-        // push 6 0 bits
+        for (int icounter = 0; icounter != 3; ++icounter) {
-        for (int i = 0; i < 3; i++) {
+            if (mask[ichip * NCHAN + ich * NCOUNTERS + icounter]) {
-            patword = clearBit(SIGNAL_serialIN, patword);
+                LOG(logDEBUG1,
-            patword = clearBit(SIGNAL_clk, patword);
+                    ("badchannel [modCounter:%d, modChan:%d, ichip:%d, ich:%d, "
-            pat->word[iaddr++] = patword;
+                     "icounter:%d]\n",
-            patword = setBit(SIGNAL_clk, patword);
+                     ichip * NCHAN + ich * NCOUNTERS + icounter,
-            pat->word[iaddr++] = patword;
+                     ichip * NCHAN_1_COUNTER + ich, ichip, ich, icounter));
+                chanReg |= (0x1 << (3 + icounter));
             }
-
-        for (int i = 0; i < 3; i++) {
-            if (mask[i])
-                patword = setBit(SIGNAL_serialIN, patword);
-            else
-                patword = clearBit(SIGNAL_serialIN, patword);
-            patword = clearBit(SIGNAL_clk, patword);
-            pat->word[iaddr++] = patword;
-            patword = setBit(SIGNAL_clk, patword);
-            pat->word[iaddr++] = patword;
         }
 
         // deserialize
-        for (int i = 0; i < 18; i++) {
+        if (chanReg & CHAN_REG_BAD_CHANNEL_MSK) {
-            if (val & (1 << i)) {
+            LOG(logINFOBLUE,
+                ("badchannel [chanReg:0x%x modCounter:%d, modChan:%d, "
+                 "ichip:%d, ich:%d]\n",
+                 chanReg, ichip * NCHAN + ich * NCOUNTERS,
+                 ichip * NCHAN_1_COUNTER + ich, ichip, ich));
+        }
+        for (int i = 0; i < 24; i++) {
+            patword = clearBit(SIGNAL_clk, patword);
+            pat->word[iaddr++] = patword;
+
+            if (chanReg & (1 << (i + 1))) {
                 patword = setBit(SIGNAL_serialIN, patword);
             } else {
                 patword = clearBit(SIGNAL_serialIN, patword);
             }
-            patword = clearBit(SIGNAL_clk, patword);
-            pat->word[iaddr++] = patword;
 
             patword = setBit(SIGNAL_clk, patword);
             pat->word[iaddr++] = patword;

slsDetectorServers/mythen3DetectorServer/mythen3.h

@@ -61,12 +61,15 @@
     ((1 << _CSR_C10pre) | (1 << CSR_C15sh) | (1 << CSR_C30sh) |                \
      (1 << CSR_C50sh) | (1 << CSR_C225ACsh) | (1 << _CSR_C15pre))
 
+#define CHAN_REG_BAD_CHANNEL_MSK (0x38)
+
 int setBit(int ibit, int patword);
 int clearBit(int ibit, int patword);
 int getChipStatusRegister();
 
 patternParameters *setChipStatusRegisterPattern(int csr);
-patternParameters *setChannelRegisterChip(int ichip, int *mask, int *trimbits);
+patternParameters *setChannelRegisterChip(int ichip, char *mask, int *trimbits);
+void flipNegativePolarityBits(int *csr);
 int getGainCaps();
 int M3SetGainCaps(int caps);
 int getInterpolation();

slsDetectorServers/mythen3DetectorServer/slsDetectorFunctionList.c

@@ -52,7 +52,7 @@ enum detectorSettings thisSettings = UNINITIALIZED;
 sls_detector_module *detectorModules = NULL;
 int *detectorChans = NULL;
 int *detectorDacs = NULL;
-int *channelMask = NULL;
+char *badChannelMask = NULL;
 int defaultDacValues[NDAC] = DEFAULT_DAC_VALS;
 int defaultDacValue_standard[] = SPECIAL_DEFAULT_STANDARD_DAC_VALS;
 int defaultDacValue_fast[] = SPECIAL_DEFAULT_FAST_DAC_VALS;
@@ -391,9 +391,9 @@ void initStopServer() {
 void allocateDetectorStructureMemory() {
     // Allocation of memory
     detectorModules = malloc(sizeof(sls_detector_module));
-    detectorChans = malloc(NCHIP * NCHAN * sizeof(int));
+    detectorChans = malloc(NCHAN_PER_MODULE * sizeof(int));
-    channelMask = malloc(NCHIP * NCHAN * sizeof(char));
+    badChannelMask = malloc(NCHAN_PER_MODULE * sizeof(char));
-    memset(channelMask, 0, NCHIP * NCHAN * sizeof(char));
+    memset(badChannelMask, 0, NCHAN_PER_MODULE * sizeof(char));
     detectorDacs = malloc(NDAC * sizeof(int));
 
     LOG(logDEBUG1,
@@ -404,7 +404,7 @@ void allocateDetectorStructureMemory() {
     (detectorModules)->chanregs = detectorChans;
     (detectorModules)->ndac = NDAC;
     (detectorModules)->nchip = NCHIP;
-    (detectorModules)->nchan = NCHIP * NCHAN;
+    (detectorModules)->nchan = NCHAN_PER_MODULE;
     (detectorModules)->reg = UNINITIALIZED;
     (detectorModules)->iodelay = 0;
     (detectorModules)->tau = 0;
@@ -1291,6 +1291,7 @@ int setModule(sls_detector_module myMod, char *mess) {
     detectorModules->serialnumber = myMod.serialnumber;
 
     // csr reg
+    flipNegativePolarityBits(&myMod.reg);
     if (setChipStatusRegister(myMod.reg)) {
         sprintf(mess, "Could not CSR from module\n");
         LOG(logERROR, (mess));
@@ -1349,9 +1350,8 @@ int setTrimbits(int *trimbits) {
     int error = 0;
     char cmess[MAX_STR_LENGTH];
     for (int ichip = 0; ichip < NCHIP; ichip++) {
-        patternParameters *pat = setChannelRegisterChip(
+        patternParameters *pat =
-            ichip, channelMask,
+            setChannelRegisterChip(ichip, badChannelMask, trimbits);
-            trimbits); // change here!!! @who: Change what?
         if (pat == NULL) {
             error = 1;
         } else {
@@ -2340,6 +2340,55 @@ int getClockDivider(enum CLKINDEX ind) {
     return clkDivider[ind];
 }
 
+int setBadChannels(int numChannels, int *channelList) {
+    LOG(logINFO, ("Setting %d bad channels\n", numChannels));
+    memset(badChannelMask, 0, NCHAN_PER_MODULE * sizeof(char));
+    for (int i = 0; i != numChannels; ++i) {
+        LOG(logINFO, ("\t[%d]: %d\n", i, channelList[i]));
+        for (int ich = channelList[i] * NCOUNTERS;
+             ich != channelList[i] * NCOUNTERS + NCOUNTERS; ++ich) {
+            badChannelMask[ich] = 1;
+        }
+    }
+    for (int i = 0; i != NCHAN_PER_MODULE; ++i) {
+        if (badChannelMask[i]) {
+            LOG(logDEBUG1, ("[%d]:0x%02x\n", i, badChannelMask[i]));
+        }
+    }
+    return setTrimbits(detectorChans);
+}
+
+int *getBadChannels(int *numChannels) {
+    int *retvals = NULL;
+    *numChannels = 0;
+    for (int i = 0; i != NCHAN_PER_MODULE; i = i + NCOUNTERS) {
+        if (badChannelMask[i]) {
+            *numChannels += 1;
+        }
+    }
+    if (*numChannels > 0) {
+        retvals = malloc(*numChannels * sizeof(int));
+        memset(retvals, 0, *numChannels * sizeof(int));
+        if (retvals == NULL) {
+            *numChannels = -1;
+            return NULL;
+        }
+        // return only 1 channel for all counters
+        int ich = 0;
+        for (int i = 0; i != NCHAN_PER_MODULE; i = i + NCOUNTERS) {
+            if (badChannelMask[i]) {
+                retvals[ich++] = i / NCOUNTERS;
+            }
+        }
+    }
+    // debugging
+    LOG(logDEBUG1, ("Reading Bad channel list: %d\n", *numChannels));
+    for (int i = 0; i != (*numChannels); ++i) {
+        LOG(logDEBUG1, ("[%d]: %d\n", i, retvals[i]));
+    }
+    return retvals;
+}
+
 int getTransmissionDelayFrame() {
     return ((bus_r(FMT_CONFIG_REG) & FMT_CONFIG_TXN_DELAY_MSK) >>
             FMT_CONFIG_TXN_DELAY_OFST);

slsDetectorServers/mythen3DetectorServer/slsDetectorServer_defs.h

@@ -17,6 +17,7 @@
 #define NCHAN_1_COUNTER             (128)
 #define NCHAN                       (128 * NCOUNTERS)
 #define NCHIP                       (10)
+#define NCHAN_PER_MODULE            (NCHAN * NCHIP)
 #define NDAC                        (16)
 #define HV_SOFT_MAX_VOLTAGE         (500)
 #define HV_HARD_MAX_VOLTAGE         (530)

slsDetectorServers/slsDetectorServer/include/slsDetectorFunctionList.h

@@ -624,8 +624,11 @@ int getVetoStream();
 enum vetoAlgorithm getVetoAlgorithm(enum streamingInterface interface);
 void setVetoAlgorithm(enum vetoAlgorithm alg,
                       enum streamingInterface interface);
-void setBadChannels(int nch, int *channels);
+#endif
-int *getBadChannels(int *nch);
+
+#if defined(GOTTHARD2D) || defined(MYTHEN3D)
+int setBadChannels(int numChannels, int *channelList);
+int *getBadChannels(int *numChannels);
 #endif
 
 #if defined(JUNGFRAUD) || defined(EIGERD)

slsDetectorServers/slsDetectorServer/src/loadPattern.c

@@ -31,7 +31,7 @@ extern int64_t set64BitReg(int64_t value, int aLSB, int aMSB);
 #endif
 
 void initializePatternAddresses() {
-    LOG(logINFO, ("Setting default Loop and Wait Addresses(0x%x)\n",
+    LOG(logDEBUG1, ("Setting default Loop and Wait Addresses(0x%x)\n",
                     MAX_PATTERN_LENGTH - 1));
     for (int i = 0; i != MAX_LEVELS; ++i) {
         setPatternLoopAddresses(i, MAX_PATTERN_LENGTH - 1,

slsDetectorServers/slsDetectorServer/src/slsDetectorServer_funcs.c

@@ -1897,10 +1897,10 @@ int acquire(int blocking, int file_des) {
             uint32_t sourceip = getDetectorIP();
             char src_ip[INET_ADDRSTRLEN];
             getIpAddressinString(src_ip, sourceip);
-            sprintf(mess,
+            sprintf(
+                mess,
                 "Invalid udp source ip address for this detector. Must be "
-                    "same "
+                "same as hardware detector ip address %s in 1G readout mode \n",
-                    "as hardware detector ip address %s in 1G readout mode \n",
                 src_ip);
             LOG(logERROR, (mess));
         } else
@@ -7981,7 +7981,7 @@ int get_bad_channels(int file_des) {
 
     LOG(logDEBUG1, ("Getting bad channels\n"));
 
-#ifndef GOTTHARD2D
+#if !defined(GOTTHARD2D) && !defined(MYTHEN3D)
     functionNotImplemented();
 #else
     // get only
@@ -8022,38 +8022,46 @@ int set_bad_channels(int file_des) {
 
     LOG(logDEBUG1, ("Setting %d bad channels\n", nargs));
 
-#ifndef GOTTHARD2D
+#if !defined(GOTTHARD2D) && !defined(MYTHEN3D)
     functionNotImplemented();
 #else
     // only set
     if (Server_VerifyLock() == OK) {
         // validate bad channel number
+        int maxChannel = NCHAN * NCHIP;
+#ifdef MYTHEN3D
+        maxChannel = NCHAN_1_COUNTER * NCHIP;
+#endif
         for (int i = 0; i < nargs; ++i) {
             LOG(logDEBUG1, ("\t[%d]:%d\n", i, args[i]));
-            if (args[i] < 0 || args[i] >= (NCHAN * NCHIP)) {
+            if (args[i] < 0 || args[i] >= maxChannel) {
                 ret = FAIL;
                 sprintf(mess,
                         "Could not set bad channels. Invalid bad channel "
                         "number %d. Options [0-%d]\n",
-                        args[i], NCHIP * NCHAN - 1);
+                        args[i], maxChannel - 1);
                 LOG(logERROR, (mess));
                 break;
             }
         }
         if (ret == OK) {
-            setBadChannels(nargs, args);
+            ret = setBadChannels(nargs, args);
+            if (ret == FAIL) {
+                strcpy(mess, "Could not set bad channels.\n");
+                LOG(logERROR, (mess));
+            } else {
                 int nretvals = 0;
                 int *retvals = getBadChannels(&nretvals);
                 if (nretvals == -1) {
                     ret = FAIL;
-                strcpy(mess,
+                    strcpy(mess, "Could not get bad channels. Memory "
-                       "Could not get bad channels. Memory allcoation error\n");
+                                 "allcoation error\n");
                     LOG(logERROR, (mess));
                 } else if (nretvals != nargs) {
                     ret = FAIL;
-                sprintf(
+                    sprintf(mess,
-                    mess,
+                            "Could not set bad channels. Set %d channels, but "
-                    "Could not set bad channels. Set %d channels, but read %d "
+                            "read %d "
                             "channels\n",
                             nargs, nretvals);
                     LOG(logERROR, (mess));
@@ -8063,6 +8071,7 @@ int set_bad_channels(int file_des) {
                 }
             }
         }
+    }
     if (args != NULL) {
         free(args);
     }

slsDetectorSoftware/include/sls/Detector.h

@@ -210,6 +210,14 @@ class Detector {
     /** [Jungfrau]  */
     void setSynchronization(bool value);
 
+    /** [Gotthard2][Mythen3] */
+    void getBadChannels(const std::string &fname, Positions pos = {}) const;
+
+    /** [Gotthard2][Mythen3]
+     * [Mythen3] Also does trimming
+     */
+    void setBadChannels(const std::string &fname, Positions pos = {});
+
     Result<bool> isVirtualDetectorServer(Positions pos = {}) const;
     ///@}
 
@@ -1439,11 +1447,6 @@ class Detector {
     void setADCConfiguration(const int chipIndex, const int adcIndex,
                              const int value, Positions pos = {});
 
-    /** [Gotthard2] */
-    void getBadChannels(const std::string &fname, Positions pos = {}) const;
-
-    /** [Gotthard2] */
-    void setBadChannels(const std::string &fname, Positions pos = {});
     ///@}
 
     /** @name Mythen3 Specific */

slsDetectorSoftware/src/CmdProxy.cpp

@@ -540,6 +540,32 @@ std::string CmdProxy::GapPixels(int action) {
     return os.str();
 }
 
+std::string CmdProxy::BadChannels(int action) {
+    std::ostringstream os;
+    os << cmd << ' ';
+    if (action == defs::HELP_ACTION) {
+        os << "[fname]\n\t[Gotthard2][Mythen3] Sets the bad channels (from "
+              "file of bad channel numbers) to be masked out."
+              "\n\t[Mythen3] Also does trimming"
+           << '\n';
+    } else if (action == defs::GET_ACTION) {
+        if (args.size() != 1) {
+            WrongNumberOfParameters(1);
+        }
+        det->getBadChannels(args[0], std::vector<int>{det_id});
+        os << "successfully retrieved" << '\n';
+    } else if (action == defs::PUT_ACTION) {
+        if (args.size() != 1) {
+            WrongNumberOfParameters(1);
+        }
+        det->setBadChannels(args[0], std::vector<int>{det_id});
+        os << "successfully loaded" << '\n';
+    } else {
+        throw RuntimeError("Unknown action");
+    }
+    return os.str();
+}
+
 /* acquisition parameters */
 
 std::string CmdProxy::Exptime(int action) {
@@ -2320,31 +2346,6 @@ std::string CmdProxy::ConfigureADC(int action) {
     return os.str();
 }
 
-std::string CmdProxy::BadChannels(int action) {
-    std::ostringstream os;
-    os << cmd << ' ';
-    if (action == defs::HELP_ACTION) {
-        os << "[fname]\n\t[Gotthard2] Sets the bad channels (from file of bad "
-              "channel numbers) to be masked out."
-           << '\n';
-    } else if (action == defs::GET_ACTION) {
-        if (args.size() != 1) {
-            WrongNumberOfParameters(1);
-        }
-        det->getBadChannels(args[0], std::vector<int>{det_id});
-        os << "successfully retrieved" << '\n';
-    } else if (action == defs::PUT_ACTION) {
-        if (args.size() != 1) {
-            WrongNumberOfParameters(1);
-        }
-        det->setBadChannels(args[0], std::vector<int>{det_id});
-        os << "successfully loaded" << '\n';
-    } else {
-        throw RuntimeError("Unknown action");
-    }
-    return os.str();
-}
-
 /* Mythen3 Specific */
 
 std::string CmdProxy::Counters(int action) {

slsDetectorSoftware/src/CmdProxy.h

@@ -788,6 +788,7 @@ class CmdProxy {
         {"fliprows", &CmdProxy::fliprows},
         {"master", &CmdProxy::master},
         {"sync", &CmdProxy::sync},
+        {"badchannels", &CmdProxy::BadChannels},
 
         /* acquisition parameters */
         {"acquire", &CmdProxy::Acquire},
@@ -984,7 +985,6 @@ class CmdProxy {
         {"vetostream", &CmdProxy::VetoStreaming},
         {"vetoalg", &CmdProxy::VetoAlgorithm},
         {"confadc", &CmdProxy::ConfigureADC},
-        {"badchannels", &CmdProxy::BadChannels},
 
         /* Mythen3 Specific */
         {"counters", &CmdProxy::Counters},
@@ -1122,6 +1122,7 @@ class CmdProxy {
     std::string Trimbits(int action);
     std::string TrimEnergies(int action);
     std::string GapPixels(int action);
+    std::string BadChannels(int action);
     /* acquisition parameters */
     std::string Acquire(int action);
     std::string Exptime(int action);
@@ -1182,7 +1183,6 @@ class CmdProxy {
     std::string VetoStreaming(int action);
     std::string VetoAlgorithm(int action);
     std::string ConfigureADC(int action);
-    std::string BadChannels(int action);
     /* Mythen3 Specific */
     std::string Counters(int action);
     std::string GateDelay(int action);

slsDetectorSoftware/src/Detector.cpp

@@ -333,6 +333,14 @@ void Detector::setSynchronization(bool value) {
     pimpl->Parallel(&Module::setSynchronization, {}, value);
 }
 
+void Detector::getBadChannels(const std::string &fname, Positions pos) const {
+    pimpl->getBadChannels(fname, pos);
+}
+
+void Detector::setBadChannels(const std::string &fname, Positions pos) {
+    pimpl->setBadChannels(fname, pos);
+}
+
 Result<bool> Detector::isVirtualDetectorServer(Positions pos) const {
     return pimpl->Parallel(&Module::isVirtualDetectorServer, pos);
 }
@@ -1819,14 +1827,6 @@ void Detector::setADCConfiguration(const int chipIndex, const int adcIndex,
                     value);
 }
 
-void Detector::getBadChannels(const std::string &fname, Positions pos) const {
-    pimpl->Parallel(&Module::getBadChannels, pos, fname);
-}
-
-void Detector::setBadChannels(const std::string &fname, Positions pos) {
-    pimpl->Parallel(&Module::setBadChannels, pos, fname);
-}
-
 // Mythen3 Specific
 
 Result<uint32_t> Detector::getCounterMask(Positions pos) const {

slsDetectorSoftware/src/DetectorImpl.cpp

@@ -50,6 +50,11 @@ void DetectorImpl::setupDetector(bool verify, bool update) {
         ctb_shm.openSharedMemory(verify);
 }
 
+bool DetectorImpl::isAllPositions(const Positions pos) const {
+    return (pos.empty() || (pos.size() == 1 && pos[0] == -1) ||
+            (pos.size() == modules.size()));
+}
+
 void DetectorImpl::setAcquiringFlag(bool flag) { shm()->acquiringFlag = flag; }
 
 int DetectorImpl::getDetectorIndex() const { return detectorIndex; }
@@ -1645,6 +1650,85 @@ void DetectorImpl::clearRxROI() {
     shm()->rx_roi.ymax = -1;
 }
 
+void DetectorImpl::getBadChannels(const std::string &fname,
+                                  Positions pos) const {
+    auto res = Parallel(&Module::getBadChannels, pos);
+    std::vector<int> badchannels(res[0]);
+
+    // update to multi values if multi modules
+    if (isAllPositions(pos)) {
+        badchannels.clear();
+        int nchan = modules[0]->getNumberOfChannels().x;
+        if (shm()->detType == MYTHEN3) {
+            // assuming single counter
+            nchan /= MAX_NUM_COUNTERS;
+        }
+        int imod = 0;
+        for (auto vec : res) {
+            for (auto badch : vec) {
+                badchannels.push_back(imod * nchan + badch);
+            }
+            ++imod;
+        }
+    } else if (pos.size() != 1) {
+        throw RuntimeError("Can get bad channels only for 1 or all modules.\n");
+    }
+
+    // save to file
+    LOG(logDEBUG1) << "Getting bad channels to " << fname;
+    std::ofstream outfile(fname);
+    if (!outfile) {
+        throw RuntimeError("Could not create file to save bad channels");
+    }
+    for (auto ch : badchannels)
+        outfile << ch << '\n';
+    LOG(logDEBUG1) << badchannels.size() << " bad channels saved to file";
+}
+
+void DetectorImpl::setBadChannels(const std::string &fname, Positions pos) {
+    std::vector<int> list = sls::getChannelsFromFile(fname);
+    if (list.empty()) {
+        throw RuntimeError("Bad channel file is empty.");
+    }
+
+    // update to multi values if multi modules
+    if (isAllPositions(pos)) {
+        std::vector<std::vector<int>> badchannels;
+        int nchan = modules[0]->getNumberOfChannels().x;
+        if (shm()->detType == MYTHEN3) {
+            // assuming single counter
+            nchan /= MAX_NUM_COUNTERS;
+        }
+        for (auto badchannel : list) {
+            if (badchannel < 0) {
+                throw RuntimeError("Invalid bad channel list. " +
+                                   std::to_string(badchannel) +
+                                   " out of bounds.");
+            }
+            int ch = badchannel % nchan;
+            int imod = badchannel / nchan;
+            if (imod >= (int)modules.size()) {
+                throw RuntimeError("Invalid bad channel list. " +
+                                   std::to_string(badchannel) +
+                                   " out of bounds.");
+            }
+
+            if ((int)badchannels.size() != imod + 1) {
+                badchannels.push_back(std::vector<int>{});
+            }
+            badchannels[imod].push_back(ch);
+        }
+        for (int imod = 0; imod != (int)modules.size(); ++imod) {
+            Parallel(&Module::setBadChannels, {imod}, badchannels[imod]);
+        }
+
+    } else if (pos.size() != 1) {
+        throw RuntimeError("Can set bad channels only for 1 or all modules.\n");
+    } else {
+        Parallel(&Module::setBadChannels, pos, list);
+    }
+}
+
 std::vector<std::string> DetectorImpl::getCtbDacNames() const {
     return ctb_shm()->getDacNames();
 }

slsDetectorSoftware/src/DetectorImpl.h

@@ -190,6 +190,8 @@ class DetectorImpl : public virtual slsDetectorDefs {
         }
     }
 
+    bool isAllPositions(const Positions pos) const;
+
     /** set acquiring flag in shared memory */
     void setAcquiringFlag(bool flag);
 
@@ -303,6 +305,9 @@ class DetectorImpl : public virtual slsDetectorDefs {
     void setRxROI(const defs::ROI arg);
     void clearRxROI();
 
+    void getBadChannels(const std::string &fname, Positions pos) const;
+    void setBadChannels(const std::string &fname, Positions pos);
+
     std::vector<std::string> getCtbDacNames() const;
     std::string getCtbDacName(defs::dacIndex i) const;
     void setCtbDacNames(const std::vector<std::string> &names);

slsDetectorSoftware/src/Module.cpp

@@ -498,6 +498,40 @@ void Module::setSynchronization(const bool value) {
     sendToDetector(F_SET_SYNCHRONIZATION, static_cast<int>(value), nullptr);
 }
 
+std::vector<int> Module::getBadChannels() const {
+    auto client = DetectorSocket(shm()->hostname, shm()->controlPort);
+    client.Send(F_GET_BAD_CHANNELS);
+    if (client.Receive<int>() == FAIL) {
+        throw DetectorError("Detector " + std::to_string(moduleIndex) +
+                            " returned error: " + client.readErrorMessage());
+    }
+    // receive badchannels
+    auto nch = client.Receive<int>();
+    std::vector<int> badchannels(nch);
+    if (nch > 0) {
+        client.Receive(badchannels);
+        for (size_t i = 0; i < badchannels.size(); ++i) {
+            LOG(logDEBUG1) << i << ":" << badchannels[i];
+        }
+    }
+    return badchannels;
+}
+
+void Module::setBadChannels(std::vector<int> list) {
+    auto nch = static_cast<int>(list.size());
+    LOG(logDEBUG1) << "Sending bad channels to detector, nch:" << nch;
+    auto client = DetectorSocket(shm()->hostname, shm()->controlPort);
+    client.Send(F_SET_BAD_CHANNELS);
+    client.Send(nch);
+    if (nch > 0) {
+        client.Send(list);
+    }
+    if (client.Receive<int>() == FAIL) {
+        throw DetectorError("Detector " + std::to_string(moduleIndex) +
+                            " returned error: " + client.readErrorMessage());
+    }
+}
+
 bool Module::isVirtualDetectorServer() const {
     return sendToDetector<int>(F_IS_VIRTUAL);
 }
@@ -2136,73 +2170,6 @@ void Module::setADCConfiguration(const int chipIndex, const int adcIndex,
     sendToDetector(F_SET_ADC_CONFIGURATION, args, nullptr);
 }
 
-void Module::getBadChannels(const std::string &fname) const {
-    LOG(logDEBUG1) << "Getting bad channels to " << fname;
-    auto client = DetectorSocket(shm()->hostname, shm()->controlPort);
-    client.Send(F_GET_BAD_CHANNELS);
-    if (client.Receive<int>() == FAIL) {
-        throw DetectorError("Detector " + std::to_string(moduleIndex) +
-                            " returned error: " + client.readErrorMessage());
-    }
-    // receive badchannels
-    auto nch = client.Receive<int>();
-    std::vector<int> badchannels(nch);
-    if (nch > 0) {
-        client.Receive(badchannels);
-        for (size_t i = 0; i < badchannels.size(); ++i) {
-            LOG(logDEBUG1) << i << ":" << badchannels[i];
-        }
-    }
-
-    // save to file
-    std::ofstream outfile(fname);
-    if (!outfile) {
-        throw RuntimeError("Could not create file to save bad channels");
-    }
-    for (auto ch : badchannels)
-        outfile << ch << '\n';
-    LOG(logDEBUG1) << nch << " bad channels saved to file";
-}
-
-void Module::setBadChannels(const std::string &fname) {
-    // read bad channels file
-    std::ifstream input_file(fname);
-    if (!input_file) {
-        throw RuntimeError("Could not open bad channels file " + fname +
-                           " for reading");
-    }
-    std::vector<int> badchannels;
-    for (std::string line; std::getline(input_file, line);) {
-        line.erase(std::remove_if(begin(line), end(line), isspace),
-                   end(line)); // remove space
-        if (!line.empty()) {
-            std::istringstream iss(line);
-            int ival = 0;
-            iss >> ival;
-            if (iss.fail()) {
-                throw RuntimeError("Could not load bad channels file. Invalid "
-                                   "channel number at position " +
-                                   std::to_string(badchannels.size()));
-            }
-            badchannels.push_back(ival);
-        }
-    }
-
-    // send bad channels to module
-    auto nch = static_cast<int>(badchannels.size());
-    LOG(logDEBUG1) << "Sending bad channels to detector, nch:" << nch;
-    auto client = DetectorSocket(shm()->hostname, shm()->controlPort);
-    client.Send(F_SET_BAD_CHANNELS);
-    client.Send(nch);
-    if (nch > 0) {
-        client.Send(badchannels);
-    }
-    if (client.Receive<int>() == FAIL) {
-        throw DetectorError("Detector " + std::to_string(moduleIndex) +
-                            " returned error: " + client.readErrorMessage());
-    }
-}
-
 // Mythen3 Specific
 
 uint32_t Module::getCounterMask() const {

slsDetectorSoftware/src/Module.h

@@ -125,6 +125,8 @@ class Module : public virtual slsDetectorDefs {
     void setMaster(const bool master);
     bool getSynchronization() const;
     void setSynchronization(const bool value);
+    std::vector<int> getBadChannels() const;
+    void setBadChannels(std::vector<int> list);
 
     bool isVirtualDetectorServer() const;
 
@@ -453,8 +455,6 @@ class Module : public virtual slsDetectorDefs {
     int getADCConfiguration(const int chipIndex, const int adcIndex) const;
     void setADCConfiguration(const int chipIndex, const int adcIndex,
                              int value);
-    void getBadChannels(const std::string &fname) const;
-    void setBadChannels(const std::string &fname);
 
     /**************************************************
      *                                                *

slsDetectorSoftware/tests/test-CmdProxy-gotthard2.cpp

@@ -731,18 +731,4 @@ TEST_CASE("confadc", "[.cmd]") {
     }
 }
 
-TEST_CASE("badchannels", "[.cmd]") {
-    Detector det;
-    CmdProxy proxy(&det);
-    auto det_type = det.getDetectorType().squash();
-
-    if (det_type == defs::GOTTHARD2) {
-        REQUIRE_THROWS(proxy.Call("badchannels", {}, -1, GET));
-        REQUIRE_NOTHROW(proxy.Call("badchannels", {"/tmp/bla.txt"}, -1, GET));
-        REQUIRE_NOTHROW(proxy.Call("badchannels", {"/tmp/bla.txt"}, -1, PUT));
-    } else {
-        REQUIRE_THROWS(proxy.Call("badchannels", {}, -1, GET));
-    }
-}
-
 } // namespace sls

slsDetectorSoftware/tests/test-CmdProxy-jungfrau.cpp

@@ -561,7 +561,7 @@ TEST_CASE("sync", "[.cmd]") {
             proxy.Call("sync", {}, -1, GET, oss);
             REQUIRE(oss.str() == "sync 1\n");
         }
-        det.getSynchronization(prev_val);
+        det.setSynchronization(prev_val);
     } else {
         REQUIRE_THROWS(proxy.Call("sync", {}, -1, GET));
         REQUIRE_THROWS(proxy.Call("sync", {"0"}, -1, PUT));

slsDetectorSoftware/tests/test-CmdProxy.cpp

@@ -3,6 +3,7 @@
 #include "CmdProxy.h"
 #include "catch.hpp"
 #include "sls/Detector.h"
+#include "sls/file_utils.h"
 #include "sls/sls_detector_defs.h"
 
 #include <chrono>
@@ -622,6 +623,29 @@ TEST_CASE("master", "[.cmd]") {
     }
 }
 
+TEST_CASE("badchannels", "[.cmd]") {
+    Detector det;
+    CmdProxy proxy(&det);
+    auto det_type = det.getDetectorType().squash();
+
+    if (det_type == defs::GOTTHARD2 || det_type == defs::MYTHEN3) {
+        REQUIRE_THROWS(proxy.Call("badchannels", {}, -1, GET));
+
+        std::string fname_put =
+            getAbsolutePathFromCurrentProcess(TEST_FILE_NAME_BAD_CHANNELS);
+        std::string fname_get = "/tmp/sls_test_channels.txt";
+
+        REQUIRE_NOTHROW(proxy.Call("badchannels", {fname_put}, 0, PUT));
+        REQUIRE_NOTHROW(proxy.Call("badchannels", {fname_get}, 0, GET));
+        auto list = getChannelsFromFile(fname_get);
+        std::vector<int> expected = {0, 12, 15, 40, 41, 42, 43, 44, 1279};
+        REQUIRE(list == expected);
+
+    } else {
+        REQUIRE_THROWS(proxy.Call("badchannels", {}, -1, GET));
+    }
+}
+
 /* acquisition parameters */
 
 // acquire: not testing

slsDetectorSoftware/tests/test-CtbConfig.cpp

@@ -10,7 +10,8 @@
 namespace sls {
 
 TEST_CASE("Default construction") {
-    static_assert(sizeof(CtbConfig) == 360); // 18*20
+    static_assert(sizeof(CtbConfig) == 360,
+                  "Size of CtbConfig does not match"); // 18*20
 
     CtbConfig c;
     auto names = c.getDacNames();

slsSupportLib/include/sls/file_utils.h

@@ -7,6 +7,7 @@
 #include <cstdio>
 #include <fstream>
 #include <string>
+#include <vector>
 
 namespace sls {
 
@@ -48,4 +49,13 @@ int getFileSize(std::ifstream &ifs);
 ssize_t getFileSize(FILE *fd, const std::string &prependErrorString);
 
 std::string getFileNameFromFilePath(const std::string &fpath);
+
+/** File can have # for comments.
+ * Channels can be separated by spaces, commas
+ * and ranges provided using ':', eg. 23:29
+ * */
+std::vector<int> getChannelsFromFile(const std::string &fname);
+
+std::string getAbsolutePathFromCurrentProcess(const std::string &fname);
+
 } // namespace sls

slsSupportLib/include/sls/sls_detector_defs.h

@@ -73,6 +73,8 @@
 #define MAX_PATTERN_LEVELS    6
 #define M3_MAX_PATTERN_LEVELS 3
 
+#define MAX_NUM_COUNTERS 3
+
 #define DEFAULT_STREAMING_TIMER_IN_MS 500
 
 #define NUM_RX_THREAD_IDS 9

slsSupportLib/include/sls/versionAPI.h

@@ -5,10 +5,10 @@
 #define APILIB       0x220609
 #define APIRECEIVER  0x220609
 #define APIGUI       0x220609
-#define APIGOTTHARD  0x220816
 #define APIEIGER     0x220816
-#define APIMYTHEN3   0x220822
+#define APICTB       0x220831
-#define APICTB       0x220825
+#define APIGOTTHARD  0x220831
-#define APIMOENCH    0x220825
-#define APIGOTTHARD2 0x220830
 #define APIJUNGFRAU  0x220831
+#define APIMOENCH    0x220831
+#define APIMYTHEN3 0x220901
+#define APIGOTTHARD2 0x220901

slsSupportLib/src/file_utils.cpp

@@ -1,15 +1,18 @@
 // SPDX-License-Identifier: LGPL-3.0-or-other
 // Copyright (C) 2021 Contributors to the SLS Detector Package
 #include "sls/file_utils.h"
+#include "sls/ToString.h"
 #include "sls/logger.h"
 #include "sls/sls_detector_exceptions.h"
 
 #include <errno.h>
 #include <ios>
 #include <iostream>
+#include <libgen.h> // dirname
 #include <sstream>
 #include <sys/stat.h>
 #include <sys/types.h>
+#include <unistd.h> //readlink
 
 namespace sls {
 
@@ -162,4 +165,99 @@ ssize_t getFileSize(FILE *fd, const std::string &prependErrorString) {
     return fileSize;
 }
 
+std::vector<int> getChannelsFromFile(const std::string &fname) {
+    // read bad channels file
+    std::ifstream input_file(fname);
+    if (!input_file) {
+        throw RuntimeError("Could not open bad channels file " + fname +
+                           " for reading");
+    }
+    std::vector<int> list;
+    for (std::string line; std::getline(input_file, line);) {
+        // ignore comments
+        if (line.find('#') != std::string::npos) {
+            line.erase(line.find('#'));
+        }
+
+        // replace comma with space
+        std::replace_if(
+            begin(line), end(line), [](char c) { return (c == ','); }, ' ');
+
+        // replace x:y with a sequence of x to y
+        auto result = line.find(':');
+        while (result != std::string::npos) {
+            auto start = line.rfind(' ', result);
+            if (start == std::string::npos) {
+                start = 0;
+            } else
+                ++start;
+            int istart = StringTo<int>(line.substr(start, result - start));
+
+            auto stop = line.find(' ', result);
+            if (stop == std::string::npos) {
+                stop = line.length();
+            }
+            int istop =
+                StringTo<int>(line.substr(result + 1, stop - result - 1));
+
+            std::vector<int> v(istop - istart);
+            std::generate(v.begin(), v.end(),
+                          [n = istart]() mutable { return n++; });
+            line.replace(start, stop - start, ToString(v));
+
+            LOG(logDEBUG1) << line;
+            result = line.find(':');
+        }
+
+        // remove punctuations including [ and ]
+        line.erase(std::remove_if(begin(line), end(line), ispunct), end(line));
+
+        LOG(logDEBUG) << "\nline: [" << line << ']';
+
+        // split line (delim space) and push to list
+        std::vector<std::string> vec = split(line, ' ');
+        for (auto it : vec) {
+            int ival = 0;
+            try {
+                ival = StringTo<int>(it);
+            } catch (std::exception &e) {
+                throw RuntimeError("Could not load channels from file. Invalid "
+                                   "channel number: " +
+                                   it);
+            }
+            list.push_back(ival);
+        }
+    }
+
+    // remove duplicates from list
+    auto listSize = list.size();
+    std::sort(list.begin(), list.end());
+    list.erase(unique(list.begin(), list.end()), list.end());
+    if (list.size() != listSize) {
+        LOG(logWARNING) << "Removed duplicates from channel file";
+    }
+
+    LOG(logDEBUG1) << "list:" << ToString(list);
+    return list;
+}
+
+std::string getAbsolutePathFromCurrentProcess(const std::string &fname) {
+    if (fname[0] == '/') {
+        return fname;
+    }
+
+    // get path of current binary
+    char path[MAX_STR_LENGTH];
+    memset(path, 0, MAX_STR_LENGTH);
+    ssize_t len = readlink("/proc/self/exe", path, MAX_STR_LENGTH - 1);
+    if (len < 0) {
+        throw RuntimeError("Could not get absolute path for " + fname);
+    }
+    path[len] = '\0';
+
+    // get dir path and attach file name
+    std::string absPath = (std::string(dirname(path)) + '/' + fname);
+    return absPath;
+}
+
 } // namespace sls

slsSupportLib/src/string_utils.cpp

@@ -14,8 +14,10 @@ std::vector<std::string> split(const std::string &strToSplit, char delimeter) {
     std::string item;
     std::vector<std::string> splittedStrings;
     while (std::getline(ss, item, delimeter)) {
+        if (item.length() > 0) {
             splittedStrings.push_back(item);
         }
+    }
     return splittedStrings;
 }
 

slsSupportLib/tests/CMakeLists.txt

@@ -16,3 +16,5 @@ target_sources(tests PRIVATE
                 ${CMAKE_CURRENT_SOURCE_DIR}/test-logger.cpp
                 ${CMAKE_CURRENT_SOURCE_DIR}/test-ZmqSocket.cpp
                 )
+
+configure_file(${CMAKE_CURRENT_SOURCE_DIR}/test-file_utils-channels.txt ${CMAKE_BINARY_DIR}/bin/test-file_utils-channels.txt COPYONLY)               

slsSupportLib/tests/test-file_utils-channels.txt

@@ -0,0 +1,5 @@
+0
+#should remove duplicates
+12, 15, 43
+40:45 #sequence from 40 to 44
+1279

slsSupportLib/tests/test-file_utils.cpp

@@ -7,6 +7,8 @@
 #include <unistd.h>
 #include <vector>
 
+#include "tests/globals.h"
+
 namespace sls {
 
 TEST_CASE("Get size of empty file") {
@@ -29,4 +31,13 @@ TEST_CASE("Get size of file with data") {
     REQUIRE(ifs.tellg() == 0); // getting size resets pos!
 }
 
+TEST_CASE("Channel file reading") {
+    std::string fname =
+        getAbsolutePathFromCurrentProcess(TEST_FILE_NAME_BAD_CHANNELS);
+    std::vector<int> list;
+    REQUIRE_NOTHROW(list = getChannelsFromFile(fname));
+    std::vector<int> expected = {0, 12, 15, 40, 41, 42, 43, 44, 1279};
+    REQUIRE(list == expected);
+}
+
 } // namespace sls

tests/include/tests/globals.h

@@ -3,6 +3,8 @@
 #pragma once
 #include "sls/sls_detector_defs.h"
 
+#define TEST_FILE_NAME_BAD_CHANNELS ("test-file_utils-channels.txt")
+
 namespace sls {
 
 using dt = slsDetectorDefs::detectorType;