code review

etc/generate_registerdefs.py

@@ -77,23 +77,6 @@ constexpr RegisterField ModuleIndex{
 // clang-format on
 """
 
-postpend = r""" 
-constexpr RegisterField ModuleRow{
-     Frame_HDR_ModCoord_LSB_Reg, 0, 0xffff}; 
-
-constexpr RegisterField ModuleCol{
-     Frame_HDR_ModCoord_LSB_Reg, 16, 0xffff};
-
-constexpr RegisterField ModuleCoordz{
-     Frame_HDR_ModCoord_MSB_Reg, 0, 0xffff};
-
-constexpr RegisterField ModuleIndex{
-     Frame_HDR_ModCoord_MSB_Reg, 16, 0xffff};
-} // namespace Reg
-} // namespace sls
-// clang-format on
-"""
-
 def main():
 
     args = argument_parser()

slsDetectorServers/matterhornServer/CMakeLists.txt

@@ -6,6 +6,11 @@ set(MATTERHORN_SOURCES
     ${CMAKE_CURRENT_SOURCE_DIR}/src/HelperFunctions.cpp
 )
 
+set(MATTERHORN_INCLUDE_DIRS
+    ${CMAKE_CURRENT_SOURCE_DIR}/include
+    ${CMAKE_CURRENT_SOURCE_DIR}/../../slsSupportLib/include
+    ${CMAKE_CURRENT_SOURCE_DIR}/../slsDetectorServer_cpp/include
+)
 
 if(SLS_USE_SIMULATOR)
     list(APPEND MATTERHORN_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/src/VirtualMatterhornServer.cpp)
@@ -20,10 +25,7 @@ if(SLS_USE_SIMULATOR)
     )
 
     target_include_directories(matterhornDetectorServer_virtual
-        PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include
-        ${CMAKE_CURRENT_SOURCE_DIR}/../../slsSupportLib/include
-        ${CMAKE_CURRENT_SOURCE_DIR}/../slsDetectorServer_cpp/include
-        ${CMAKE_SOURCE_DIR}/slsDetectorSoftware/src) # because of SharedMemory TODO: should be in slsSupportLib?
+        PRIVATE ${MATTERHORN_INCLUDE_DIRS})
 
     target_link_libraries(matterhornDetectorServer_virtual
         PUBLIC 
@@ -39,67 +41,60 @@ if(SLS_USE_SIMULATOR)
     )
 endif() # maybe better to have a else if build with simulators on one always uses the virtual server in MatterhornApp 
 
-list(APPEND MATTERHORN_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/src/MatterhornServer.cpp)
+if(SLS_USE_MATTERHORN) 
+    list(APPEND MATTERHORN_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/src/MatterhornServer.cpp)
 
-#mmh should slsSupportLib be considered as well when updating version? 
-if(CMAKE_TOOLCHAIN_FILE AND NOT CMAKE_TOOLCHAIN_FILE STREQUAL "") # only update version if cross compiling binaries 
-    find_package(Python3 REQUIRED COMPONENTS Interpreter REQUIRED)
-    add_custom_target(update_server_version ALL
-        COMMAND ${Python3_EXECUTABLE} ${CMAKE_SOURCE_DIR}/etc/updateAPIVersion.py APIMATTERHORN ${CMAKE_SOURCE_DIR}/slsDetectorServers/matterhornServer ${CMAKE_SOURCE_DIR}/slsDetectorServers/slsDetectorServer_cpp ${CMAKE_SOURCE_DIR}/slsSupportLib  
-        WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
-        COMMENT "Updating APIMATTERHORN version"
-        VERBATIM)
-endif() 
+    if(CMAKE_TOOLCHAIN_FILE AND NOT CMAKE_TOOLCHAIN_FILE STREQUAL "") 
+        set(CROSS_COMPILE TRUE)
+    endif()
 
-add_executable(matterhornDetectorServer ${MATTERHORN_SOURCES})
+    #mmh should slsSupportLib be considered as well when updating version? 
+    if(${CROSS_COMPILE}) # only update version if cross compiling binaries 
+        find_package(Python3 REQUIRED COMPONENTS Interpreter REQUIRED)
+        add_custom_target(update_server_version ALL
+            COMMAND ${Python3_EXECUTABLE} ${CMAKE_SOURCE_DIR}/etc/updateAPIVersion.py APIMATTERHORN ${CMAKE_SOURCE_DIR}/slsDetectorServers/matterhornServer ${CMAKE_SOURCE_DIR}/slsDetectorServers/slsDetectorServer_cpp   
+            WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
+            COMMENT "Updating APIMATTERHORN version"
+            VERBATIM)
+    endif() 
 
-if(CMAKE_TOOLCHAIN_FILE AND NOT CMAKE_TOOLCHAIN_FILE STREQUAL "") # only update version if cross compiling binaries 
-    add_dependencies(matterhornDetectorServer update_server_version)
-endif()
+    add_executable(matterhornDetectorServer ${MATTERHORN_SOURCES})
 
-target_compile_definitions(matterhornDetectorServer PRIVATE
-    MATTERHORN_SERVER_HEADER=<MatterhornServer.h>
-    MATTERHORN_SERVER_CLASS=MatterhornServer
-)
+    if(${CROSS_COMPILE}) # only update version if cross compiling binaries 
+        add_dependencies(matterhornDetectorServer update_server_version)
+    endif()
 
-target_include_directories(matterhornDetectorServer
-        PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include
-        ${CMAKE_CURRENT_SOURCE_DIR}/../../slsSupportLib/include
-        ${CMAKE_CURRENT_SOURCE_DIR}/../slsDetectorServer_cpp/include)
-    
-target_link_libraries(matterhornDetectorServer
-    PUBLIC
-    slsSupportStatic
-    slsServerStatic)
+    target_compile_definitions(matterhornDetectorServer PRIVATE
+        MATTERHORN_SERVER_HEADER=<MatterhornServer.h>
+        MATTERHORN_SERVER_CLASS=MatterhornServer
+    )
 
-set_target_properties(matterhornDetectorServer PROPERTIES
-    RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin
-)
+    target_include_directories(matterhornDetectorServer
+            PRIVATE ${MATTERHORN_INCLUDE_DIRS})
+        
+    target_link_libraries(matterhornDetectorServer
+        PUBLIC
+        slsSupportStatic
+        slsServerStatic)
 
-install(TARGETS matterhornDetectorServer
-    RUNTIME  DESTINATION ${CMAKE_INSTALL_BINDIR}
-)
+    if(${CROSS_COMPILE}) # change output directory to stay consitent with c server binaries when cross compiling
+        set(RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/bin)
+    else()
+        set(RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
+    endif()
 
-if(CMAKE_TOOLCHAIN_FILE AND NOT CMAKE_TOOLCHAIN_FILE STREQUAL "") # copy the server binary  
-    add_custom_command(TARGET matterhornDetectorServer POST_BUILD
-        COMMAND ${CMAKE_COMMAND} -E copy_if_different
-                $<TARGET_FILE:matterhornDetectorServer>
-                ${CMAKE_CURRENT_SOURCE_DIR}/bin/matterhornDetectorServer
-        DEPENDS matterhornDetectorServer
-        COMMENT "Copying matterhornDetectorServer binary to bin for cross compilation"
+    set_target_properties(matterhornDetectorServer PROPERTIES
+        RUNTIME_OUTPUT_DIRECTORY ${RUNTIME_OUTPUT_DIRECTORY}
+    )
+
+    install(TARGETS matterhornDetectorServer
+        RUNTIME  DESTINATION ${CMAKE_INSTALL_BINDIR}
     )
 endif()
 
-
 if(SLS_USE_TESTS)
     add_subdirectory(tests)
 endif()
 
 
-#target_compile_definitions(matterhornDetectorServer_virtual
-#    PUBLIC VIRTUAL STOP_SERVER #what is this stop server should we really have a generic ServerAPP and pass compile options to create server e.g. MatterHorn? 
-#)
-
-
-
 

slsDetectorServers/matterhornServer/tests/test-HelperFunctions.cpp

@@ -6,9 +6,17 @@ namespace sls {
 
 auto get_test_parameters_countermaskspiconversion() {
     return GENERATE(std::make_tuple(uint32_t{0x1}, uint32_t{0b0000}),
+                    std::make_tuple(uint32_t{0x2}, uint32_t{0b0001}),
+                    std::make_tuple(uint32_t{0x4}, uint32_t{0b0010}),
                     std::make_tuple(uint32_t{0x3}, uint32_t{0b0100}),
                     std::make_tuple(uint32_t{0x6}, uint32_t{0b0101}),
+                    std::make_tuple(uint32_t{0x7}, uint32_t{0b1000}),
                     std::make_tuple(uint32_t{0x8}, uint32_t{0b0011}),
+                    std::make_tuple(uint32_t{0x9}, uint32_t{0b0111}),
+                    std::make_tuple(uint32_t{0xB}, uint32_t{0b1011}),
+                    std::make_tuple(uint32_t{0xC}, uint32_t{0b0110}),
+                    std::make_tuple(uint32_t{0xD}, uint32_t{0b1010}),
+                    std::make_tuple(uint32_t{0xE}, uint32_t{0b1001}),
                     std::make_tuple(uint32_t{0xF}, uint32_t{0b1100}),
                     std::make_tuple(uint32_t{0xB}, uint32_t{0b1011}));
 }
@@ -23,6 +31,9 @@ TEST_CASE("Convert Counter Mask to SPI Counter Mask",
 
     REQUIRE_THROWS(
         convertCounterMaskToSPICounterMask(0xA)); // invalid counter mask
+
+    REQUIRE_THROWS(
+        convertCounterMaskToSPICounterMask(0x5)); // invalid counter mask
 }
 
 TEST_CASE("Convert SPI Counter Mask to Counter Mask",

slsReceiverSoftware/src/ClientInterface.cpp

@@ -671,12 +671,20 @@ int ClientInterface::set_dynamic_range(Interface &socket) {
             break;
         */
         case 4:
+            if (detType == MATTERHORN) {
+                exists = true;
+            }
+            break;
         case 12:
             if (detType == EIGER) {
                 exists = true;
             }
             break;
         case 8:
+            if (detType == MATTERHORN) {
+                exists = true;
+            }
+            break;
         case 32:
             if (detType == EIGER || detType == MYTHEN3) {
                 exists = true;

slsReceiverSoftware/src/GeneralData.h

@@ -477,12 +477,13 @@ class MatterhornData : public GeneralData {
         framesPerFile = MATTERHORN_MAX_FRAMES_PER_FILE; // TODO: dummy value
         fifoDepth = 50000;                              // TODO: dummy value
         standardheader = true;
+        dataSize = 8192;
+        packetSize = headerSizeinPacket + dataSize;
         // udpSocketBufferSize = 0; // TODO: dummy value
-        dynamicRange = 16;    // default
-        tengigaEnable = true; // TODO: not sure should also be 100g
-        SetCounterMask(0xf);  // default all 4 counters enabled
+        dynamicRange = 16;   // default
+        SetCounterMask(0xf); // default all 4 counters enabled
         UpdateImageSize();
-        calculatefifoDepth();
+        CalculatefifoDepth();
     };
 
     void SetDynamicRange(int dr) {
@@ -501,38 +502,29 @@ class MatterhornData : public GeneralData {
         UpdateImageSize();
     };
 
-    void calculatefifoDepth() { fifoDepth = max_fifo_depth / imageSize; }
+    void CalculatefifoDepth() { fifoDepth = max_memory_allocated / imageSize; }
 
   private:
     void UpdateImageSize() {
-        nPixelsX = (nChannelsX * ncounters);
-        nPixelsY = (nChannelsY * ncounters);
+        nPixelsX = nChannelsX;
+        nPixelsY = nChannelsY * ncounters;
 
         imageSize = nPixelsX * nPixelsY * GetPixelDepth();
         LOG(logINFO) << "imageSize: " << imageSize;
         actualImageSize = imageSize;
 
-        // 10g
-        // TODO: dont know what to do here
-        /*
-        if (tengigaEnable) {
-        }
-        // 1g
-        else {
-        }
-        */
+        packetsPerFrame = imageSize / dataSize;
 
         LOG(logINFO) << "Packets Per Frame: " << packetsPerFrame;
-        packetSize = headerSizeinPacket + dataSize;
-        LOG(logINFO) << "PacketSize: " << packetSize;
     };
 
   private:
     int ncounters{0};
-    int nChannelsX{1024};
-    int nChannelsY{512};
+    static constexpr int nChannelsX{1024};
+    static constexpr int nChannelsY{512};
 
-    constexpr static int max_fifo_depth = 100000; // TODO: dummy value for now
+    constexpr static int max_memory_allocated =
+        100000; // TODO: dummy value for now
 };
 
 class Gotthard2Data : public GeneralData {

slsReceiverSoftware/src/Implementation.cpp

@@ -269,8 +269,13 @@ void Implementation::setModulePositionId(const int id) {
     xy portGeometry = GetPortGeometry();
     streamingPort = DEFAULT_ZMQ_RX_PORTNO + modulePos * portGeometry.x;
 
-    assert(numModules.y !=
-           0); // TODO why an assert here? should we throw an exception instead?
+    if (numModules.y == 0) {
+        LOG(logERROR) << "Number of modules in y direction is 0. Cannot set "
+                         "module position.";
+        throw RuntimeError("Number of modules in y direction is 0. Cannot set "
+                           "module position.");
+    }
+
     for (unsigned int i = 0; i < listener.size(); ++i) {
         uint16_t row = 0, col = 0;
         row = (modulePos % numModules.y) * portGeometry.y;
@@ -1602,7 +1607,8 @@ uint32_t Implementation::getDynamicRange() const {
 
 void Implementation::setDynamicRange(const uint32_t i) {
     if (generalData->dynamicRange != i) {
-        if (generalData->detType == EIGER || generalData->detType == MYTHEN3) {
+        if (generalData->detType == EIGER || generalData->detType == MYTHEN3 ||
+            generalData->detType == MATTERHORN) {
             generalData->SetDynamicRange(i);
             SetupFifoStructure();
         }

slsSupportLib/include/sls/SharedMemory.h

@@ -193,8 +193,6 @@ template <typename T> class SharedMemory {
         if (verifySize)
             checkSize(fd);
         shared_struct = mapSharedMemory(fd);
-
-        LOG(logINFO) << "Shared memory " << name << " opened";
     }
 
     void unmapSharedMemory() {