From 62418c13167864cf5b552994a99d59cfc7fa9e71 Mon Sep 17 00:00:00 2001 From: Dhanya Thattil <33750417+thattil@users.noreply.github.com> Date: Thu, 7 Apr 2022 10:19:47 +0200 Subject: [PATCH 1/3] sls_receiver_header* in callbacks (#425) * char* to sls_receiver_header* in receiver data call backs * uint32_t to size_t in callbacks * string to const std::string & for callbacks --- RELEASE.txt | 3 + slsReceiverSoftware/include/sls/Receiver.h | 54 ++++++++--------- slsReceiverSoftware/src/ClientInterface.cpp | 6 +- slsReceiverSoftware/src/ClientInterface.h | 30 +++++----- slsReceiverSoftware/src/DataProcessor.cpp | 16 ++--- slsReceiverSoftware/src/DataProcessor.h | 33 ++++------ slsReceiverSoftware/src/Implementation.cpp | 14 ++--- slsReceiverSoftware/src/Implementation.h | 27 +++++---- slsReceiverSoftware/src/MultiReceiverApp.cpp | 63 +++++++------------- slsReceiverSoftware/src/Receiver.cpp | 9 ++- 10 files changed, 114 insertions(+), 141 deletions(-) diff --git a/RELEASE.txt b/RELEASE.txt index 1f0efd8a2..f69f3eb86 100755 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -59,6 +59,9 @@ This document describes the differences between v7.0.0 and v6.x.x - gotthard 25 um image reconstructed in gui and virtual hdf5 (firmware updated for slave to reverse channels) - master binary file in json format now - fixed bug introduced in 6.0.0: hdf5 files created 1 file per frame after the initial file which had maxframesperfile +- registerCallBackRawDataReady and registerCallBackRawDataModifyReady now gives a sls_receiver_header* instead of a char*, and uint32_t to size_t +- registerCallBackStartAcquisition gave incorrect imagesize (+120 bytes). corrected. +- registerCallBackStartAcquisition parameter is a const string reference 2. Resolved Issues ================== diff --git a/slsReceiverSoftware/include/sls/Receiver.h b/slsReceiverSoftware/include/sls/Receiver.h index 0cae1327c..02901c2d3 100644 --- a/slsReceiverSoftware/include/sls/Receiver.h +++ b/slsReceiverSoftware/include/sls/Receiver.h @@ -39,52 +39,50 @@ class Receiver : private virtual slsDetectorDefs { int64_t getReceiverVersion(); /** - * Call back for start acquisition - * callback arguments are - * filepath - * filename - * fileindex - * datasize - * - * return value is undefined at the moment - * we write depending on file write enable - * users get data to write depending on call backs registered + * Start Acquisition Call back (slsMultiReceiver writes data if file write enabled) + * if registerCallBackRawDataReady or registerCallBackRawDataModifyReady registered, + * users get data + * callback arguments are: + * - file path + * - file name prefix + * - file index + * - image size in bytes */ - void registerCallBackStartAcquisition(int (*func)(std::string, std::string, - uint64_t, uint32_t, - void *), + void registerCallBackStartAcquisition(int (*func)(const std::string &, const std::string &, + uint64_t, size_t, void *), void *arg); /** * Call back for acquisition finished - * callback argument is - * total frames caught + * callback argument is: + * - total frames caught */ void registerCallBackAcquisitionFinished(void (*func)(uint64_t, void *), void *arg); /** * Call back for raw data - * args to raw data ready callback are - * sls_receiver_header frame metadata, - * dataPointer is the pointer to the data, - * dataSize in bytes is the size of the data in bytes. + * args to raw data ready callback are: + * - sls_receiver_header frame metadata, + * - pointer to data + * - image size in bytes */ - void registerCallBackRawDataReady(void (*func)(char *, char *, uint32_t, - void *), + void registerCallBackRawDataReady(void (*func)(sls_receiver_header *, + char *, size_t, void *), void *arg); /** * Call back for raw data (modified) - * args to raw data ready callback are - * sls_receiver_header frame metadata, - * dataPointer is the pointer to the data, - * revDatasize is the reference of data size in bytes. + * args to raw data ready callback are: + * - sls_receiver_header frame metadata, + * - pointer to data + * - revDatasize is the reference of data size in bytes. * Can be modified to the new size to be written/streamed. (only smaller - * value). + * value allowed). */ - void registerCallBackRawDataModifyReady(void (*func)(char *, char *, - uint32_t &, void *), + void registerCallBackRawDataModifyReady(void (*func)(sls_receiver_header *, + char *, size_t &, + void *), void *arg); private: diff --git a/slsReceiverSoftware/src/ClientInterface.cpp b/slsReceiverSoftware/src/ClientInterface.cpp index cd055d1f5..e352cbd98 100644 --- a/slsReceiverSoftware/src/ClientInterface.cpp +++ b/slsReceiverSoftware/src/ClientInterface.cpp @@ -55,7 +55,7 @@ int64_t ClientInterface::getReceiverVersion() { return APIRECEIVER; } /***callback functions***/ void ClientInterface::registerCallBackStartAcquisition( - int (*func)(std::string, std::string, uint64_t, uint32_t, void *), + int (*func)(const std::string &, const std::string &, uint64_t, size_t, void *), void *arg) { startAcquisitionCallBack = func; pStartAcquisition = arg; @@ -69,13 +69,13 @@ void ClientInterface::registerCallBackAcquisitionFinished(void (*func)(uint64_t, } void ClientInterface::registerCallBackRawDataReady( - void (*func)(char *, char *, uint32_t, void *), void *arg) { + void (*func)(sls_receiver_header *, char *, size_t, void *), void *arg) { rawDataReadyCallBack = func; pRawDataReady = arg; } void ClientInterface::registerCallBackRawDataModifyReady( - void (*func)(char *, char *, uint32_t &, void *), void *arg) { + void (*func)(sls_receiver_header *, char *, size_t &, void *), void *arg) { rawDataModifyReadyCallBack = func; pRawDataReady = arg; } diff --git a/slsReceiverSoftware/src/ClientInterface.h b/slsReceiverSoftware/src/ClientInterface.h index 6671c4592..ef0a36529 100644 --- a/slsReceiverSoftware/src/ClientInterface.h +++ b/slsReceiverSoftware/src/ClientInterface.h @@ -30,25 +30,24 @@ class ClientInterface : private virtual slsDetectorDefs { int64_t getReceiverVersion(); //***callback functions*** - /** params: filepath, filename, fileindex, datasize */ - void registerCallBackStartAcquisition(int (*func)(std::string, std::string, - uint64_t, uint32_t, - void *), + /** params: file path, file name, file index, image size */ + void registerCallBackStartAcquisition(int (*func)(const std::string &, const std::string &, + uint64_t, size_t, void *), void *arg); /** params: total frames caught */ void registerCallBackAcquisitionFinished(void (*func)(uint64_t, void *), void *arg); - /** params: sls_receiver_header frame metadata, dataPointer, dataSize */ - void registerCallBackRawDataReady(void (*func)(char *, char *, uint32_t, - void *), + /** params: sls_receiver_header pointer, pointer to data, image size */ + void registerCallBackRawDataReady(void (*func)(sls_receiver_header *, + char *, size_t, void *), void *arg); - /** params: sls_receiver_header frame metadata, dataPointer, modified size - */ - void registerCallBackRawDataModifyReady(void (*func)(char *, char *, - uint32_t &, void *), + /** params: sls_receiver_header pointer, pointer to data, reference to image size */ + void registerCallBackRawDataModifyReady(void (*func)(sls_receiver_header *, + char *, size_t &, + void *), void *arg); private: @@ -180,13 +179,14 @@ class ClientInterface : private virtual slsDetectorDefs { //***callback parameters*** - int (*startAcquisitionCallBack)(std::string, std::string, uint64_t, - uint32_t, void *) = nullptr; + int (*startAcquisitionCallBack)(const std::string &, const std::string &, uint64_t, size_t, + void *) = nullptr; void *pStartAcquisition{nullptr}; void (*acquisitionFinishedCallBack)(uint64_t, void *) = nullptr; void *pAcquisitionFinished{nullptr}; - void (*rawDataReadyCallBack)(char *, char *, uint32_t, void *) = nullptr; - void (*rawDataModifyReadyCallBack)(char *, char *, uint32_t &, + void (*rawDataReadyCallBack)(sls_receiver_header *, char *, size_t, + void *) = nullptr; + void (*rawDataModifyReadyCallBack)(sls_receiver_header *, char *, size_t &, void *) = nullptr; void *pRawDataReady{nullptr}; diff --git a/slsReceiverSoftware/src/DataProcessor.cpp b/slsReceiverSoftware/src/DataProcessor.cpp index d6b801e3f..ede68129c 100644 --- a/slsReceiverSoftware/src/DataProcessor.cpp +++ b/slsReceiverSoftware/src/DataProcessor.cpp @@ -316,16 +316,17 @@ uint64_t DataProcessor::ProcessAnImage(char *buf) { try { // normal call back if (rawDataReadyCallBack != nullptr) { - rawDataReadyCallBack((char *)rheader, + std::size_t dsize = *reinterpret_cast(buf); + rawDataReadyCallBack(rheader, buf + FIFO_HEADER_NUMBYTES + sizeof(sls_receiver_header), - (uint32_t)(*((uint32_t *)buf)), pRawDataReady); + dsize, pRawDataReady); } // call back with modified size else if (rawDataModifyReadyCallBack != nullptr) { - auto revsize = (uint32_t)(*((uint32_t *)buf)); - rawDataModifyReadyCallBack((char *)rheader, + std::size_t revsize = *reinterpret_cast(buf); + rawDataModifyReadyCallBack(rheader, buf + FIFO_HEADER_NUMBYTES + sizeof(sls_receiver_header), revsize, pRawDataReady); @@ -393,15 +394,14 @@ bool DataProcessor::CheckCount() { return false; } -void DataProcessor::registerCallBackRawDataReady(void (*func)(char *, char *, - uint32_t, void *), - void *arg) { +void DataProcessor::registerCallBackRawDataReady( + void (*func)(sls_receiver_header *, char *, size_t, void *), void *arg) { rawDataReadyCallBack = func; pRawDataReady = arg; } void DataProcessor::registerCallBackRawDataModifyReady( - void (*func)(char *, char *, uint32_t &, void *), void *arg) { + void (*func)(sls_receiver_header *, char *, size_t &, void *), void *arg) { rawDataModifyReadyCallBack = func; pRawDataReady = arg; } diff --git a/slsReceiverSoftware/src/DataProcessor.h b/slsReceiverSoftware/src/DataProcessor.h index b62bb9d1f..9756b6896 100644 --- a/slsReceiverSoftware/src/DataProcessor.h +++ b/slsReceiverSoftware/src/DataProcessor.h @@ -79,28 +79,16 @@ class DataProcessor : private virtual slsDetectorDefs, public ThreadObject { const fileFormat fileFormatType, MasterAttributes *attr, std::mutex *hdf5LibMutex); - /** - * Call back for raw data - * args to raw data ready callback are - * sls_receiver_header frame metadata - * dataPointer is the pointer to the data - * dataSize in bytes is the size of the data in bytes. - */ - void registerCallBackRawDataReady(void (*func)(char *, char *, uint32_t, - void *), + + /** params: sls_receiver_header pointer, pointer to data, image size */ + void registerCallBackRawDataReady(void (*func)(sls_receiver_header *, + char *, size_t, void *), void *arg); - /** - * Call back for raw data (modified) - * args to raw data ready callback are - * sls_receiver_header frame metadata - * dataPointer is the pointer to the data - * revDatasize is the reference of data size in bytes. - * Can be modified to the new size to be written/streamed. (only smaller - * value). - */ - void registerCallBackRawDataModifyReady(void (*func)(char *, char *, - uint32_t &, void *), + /** params: sls_receiver_header pointer, pointer to data, reference to image size */ + void registerCallBackRawDataModifyReady(void (*func)(sls_receiver_header *, + char *, size_t &, + void *), void *arg); private: @@ -195,7 +183,8 @@ class DataProcessor : private virtual slsDetectorDefs, public ThreadObject { * dataPointer is the pointer to the data * dataSize in bytes is the size of the data in bytes. */ - void (*rawDataReadyCallBack)(char *, char *, uint32_t, void *) = nullptr; + void (*rawDataReadyCallBack)(sls_receiver_header *, char *, size_t, + void *) = nullptr; /** * Call back for raw data (modified) @@ -205,7 +194,7 @@ class DataProcessor : private virtual slsDetectorDefs, public ThreadObject { * revDatasize is the reference of data size in bytes. Can be modified to * the new size to be written/streamed. (only smaller value). */ - void (*rawDataModifyReadyCallBack)(char *, char *, uint32_t &, + void (*rawDataModifyReadyCallBack)(sls_receiver_header *, char *, size_t &, void *) = nullptr; void *pRawDataReady{nullptr}; diff --git a/slsReceiverSoftware/src/Implementation.cpp b/slsReceiverSoftware/src/Implementation.cpp index 3ec1831d7..30ff2854c 100644 --- a/slsReceiverSoftware/src/Implementation.cpp +++ b/slsReceiverSoftware/src/Implementation.cpp @@ -523,10 +523,10 @@ void Implementation::startReceiver() { // callbacks if (startAcquisitionCallBack) { try { - startAcquisitionCallBack(filePath, fileName, fileIndex, - (generalData->imageSize) + - (generalData->fifoBufferHeaderSize), - pStartAcquisition); + std::size_t imageSize = static_cast(generalData->imageSize); + startAcquisitionCallBack( + filePath, fileName, fileIndex, imageSize, + pStartAcquisition); } catch (const std::exception &e) { throw sls::RuntimeError("Start Acquisition Callback Error: " + std::string(e.what())); @@ -1627,7 +1627,7 @@ void Implementation::setDbitOffset(const int s) { ctbDbitOffset = s; } * * * ************************************************/ void Implementation::registerCallBackStartAcquisition( - int (*func)(std::string, std::string, uint64_t, uint32_t, void *), + int (*func)(const std::string &, const std::string &, uint64_t, size_t, void *), void *arg) { startAcquisitionCallBack = func; pStartAcquisition = arg; @@ -1641,7 +1641,7 @@ void Implementation::registerCallBackAcquisitionFinished(void (*func)(uint64_t, } void Implementation::registerCallBackRawDataReady( - void (*func)(char *, char *, uint32_t, void *), void *arg) { + void (*func)(sls_receiver_header *, char *, size_t, void *), void *arg) { rawDataReadyCallBack = func; pRawDataReady = arg; for (const auto &it : dataProcessor) @@ -1649,7 +1649,7 @@ void Implementation::registerCallBackRawDataReady( } void Implementation::registerCallBackRawDataModifyReady( - void (*func)(char *, char *, uint32_t &, void *), void *arg) { + void (*func)(sls_receiver_header *, char *, size_t &, void *), void *arg) { rawDataModifyReadyCallBack = func; pRawDataReady = arg; for (const auto &it : dataProcessor) diff --git a/slsReceiverSoftware/src/Implementation.h b/slsReceiverSoftware/src/Implementation.h index c792430e5..a9080b1f5 100644 --- a/slsReceiverSoftware/src/Implementation.h +++ b/slsReceiverSoftware/src/Implementation.h @@ -252,17 +252,21 @@ class Implementation : private virtual slsDetectorDefs { * Callbacks * * * * ************************************************/ - void registerCallBackStartAcquisition(int (*func)(std::string, std::string, - uint64_t, uint32_t, - void *), + /** params: file path, file name, file index, image size */ + void registerCallBackStartAcquisition(int (*func)(const std::string &, const std::string &, + uint64_t, size_t, void *), void *arg); + /** params: total frames caught */ void registerCallBackAcquisitionFinished(void (*func)(uint64_t, void *), void *arg); - void registerCallBackRawDataReady(void (*func)(char *, char *, uint32_t, - void *), + /** params: sls_receiver_header pointer, pointer to data, image size */ + void registerCallBackRawDataReady(void (*func)(sls_receiver_header *, + char *, size_t, void *), void *arg); - void registerCallBackRawDataModifyReady(void (*func)(char *, char *, - uint32_t &, void *), + /** params: sls_receiver_header pointer, pointer to data, reference to image size */ + void registerCallBackRawDataModifyReady(void (*func)(sls_receiver_header *, + char *, size_t &, + void *), void *arg); private: @@ -369,13 +373,14 @@ class Implementation : private virtual slsDetectorDefs { int ctbDbitOffset{0}; // callbacks - int (*startAcquisitionCallBack)(std::string, std::string, uint64_t, - uint32_t, void *){nullptr}; + int (*startAcquisitionCallBack)(const std::string &, const std::string &, uint64_t, size_t, + void *){nullptr}; void *pStartAcquisition{nullptr}; void (*acquisitionFinishedCallBack)(uint64_t, void *){nullptr}; void *pAcquisitionFinished{nullptr}; - void (*rawDataReadyCallBack)(char *, char *, uint32_t, void *){nullptr}; - void (*rawDataModifyReadyCallBack)(char *, char *, uint32_t &, + void (*rawDataReadyCallBack)(sls_receiver_header *, char *, size_t, + void *){nullptr}; + void (*rawDataModifyReadyCallBack)(sls_receiver_header *, char *, size_t &, void *){nullptr}; void *pRawDataReady{nullptr}; diff --git a/slsReceiverSoftware/src/MultiReceiverApp.cpp b/slsReceiverSoftware/src/MultiReceiverApp.cpp index f3828435f..5cb6cc8de 100644 --- a/slsReceiverSoftware/src/MultiReceiverApp.cpp +++ b/slsReceiverSoftware/src/MultiReceiverApp.cpp @@ -47,32 +47,21 @@ void printHelp() { } /** - * Start Acquisition Call back - * slsReceiver writes data if file write enabled. - * Users get data to write using call back if registerCallBackRawDataReady is - * registered. - * @param filepath file path - * @param filename file name - * @param fileindex file index - * @param datasize data size in bytes - * @param p pointer to object - * \returns ignored + * Start Acquisition Call back (slsMultiReceiver writes data if file write enabled) + * if registerCallBackRawDataReady or registerCallBackRawDataModifyReady registered, + * users get data */ -int StartAcq(std::string filepath, std::string filename, uint64_t fileindex, - uint32_t datasize, void *p) { - LOG(logINFOBLUE) << "#### StartAcq: filepath:" << filepath - << " filename:" << filename << " fileindex:" << fileindex - << " datasize:" << datasize << " ####"; +int StartAcq(const std::string & filePath, const std::string & fileName, uint64_t fileIndex, + size_t imageSize, void *objectPointer) { + LOG(logINFOBLUE) << "#### StartAcq: filePath:" << filePath + << " fileName:" << fileName << " fileIndex:" << fileIndex + << " imageSize:" << imageSize << " ####"; return 0; } -/** - * Acquisition Finished Call back - * @param frames Number of frames caught - * @param p pointer to object - */ -void AcquisitionFinished(uint64_t frames, void *p) { - LOG(logINFOBLUE) << "#### AcquisitionFinished: frames:" << frames +/** Acquisition Finished Call back */ +void AcquisitionFinished(uint64_t framesCaught, void *objectPointer) { + LOG(logINFOBLUE) << "#### AcquisitionFinished: framesCaught:" << framesCaught << " ####"; } @@ -80,14 +69,9 @@ void AcquisitionFinished(uint64_t frames, void *p) { * Get Receiver Data Call back * Prints in different colors(for each receiver process) the different headers * for each image call back. - * @param metadata sls_receiver_header metadata - * @param datapointer pointer to data - * @param datasize data size in bytes. - * @param p pointer to object */ -void GetData(char *metadata, char *datapointer, uint32_t datasize, void *p) { - slsDetectorDefs::sls_receiver_header *header = - (slsDetectorDefs::sls_receiver_header *)metadata; +void GetData(slsDetectorDefs::sls_receiver_header *header, char *dataPointer, + size_t imageSize, void *objectPointer) { slsDetectorDefs::sls_detector_header detectorHeader = header->detHeader; PRINT_IN_COLOR( @@ -98,7 +82,7 @@ void GetData(char *metadata, char *datapointer, uint32_t datasize, void *p) { "row: %u\t\tcolumn: %u\t\treserved: %u\t\tdebug: %u" "\t\troundRNumber: %u\t\tdetType: %u\t\tversion: %u" //"\t\tpacketsMask:%s" - "\t\tfirstbytedata: 0x%x\t\tdatsize: %u\n\n", + "\t\tfirstbytedata: 0x%x\t\tdatsize: %zu\n\n", detectorHeader.row, (long unsigned int)detectorHeader.frameNumber, detectorHeader.expLength, detectorHeader.packetNumber, (long unsigned int)detectorHeader.bunchId, @@ -107,23 +91,18 @@ void GetData(char *metadata, char *datapointer, uint32_t datasize, void *p) { detectorHeader.debug, detectorHeader.roundRNumber, detectorHeader.detType, detectorHeader.version, // header->packetsMask.to_string().c_str(), - ((uint8_t)(*((uint8_t *)(datapointer)))), datasize); + ((uint8_t)(*((uint8_t *)(dataPointer)))), imageSize); } /** * Get Receiver Data Call back (modified) * Prints in different colors(for each receiver process) the different headers * for each image call back. - * @param metadata sls_receiver_header metadata - * @param datapointer pointer to data - * @param revDatasize new data size in bytes after the callback. + * @param modifiedImageSize new data size in bytes after the callback. * This will be the size written/streamed. (only smaller value is allowed). - * @param p pointer to object */ -void GetData(char *metadata, char *datapointer, uint32_t &revDatasize, - void *p) { - slsDetectorDefs::sls_receiver_header *header = - (slsDetectorDefs::sls_receiver_header *)metadata; +void GetData(slsDetectorDefs::sls_receiver_header *header, char *dataPointer, + size_t &modifiedImageSize, void *objectPointer) { slsDetectorDefs::sls_detector_header detectorHeader = header->detHeader; PRINT_IN_COLOR( @@ -135,7 +114,7 @@ void GetData(char *metadata, char *datapointer, uint32_t &revDatasize, "row: %u\t\tcolumn: %u\t\treserved: %u\t\tdebug: %u" "\t\troundRNumber: %u\t\tdetType: %u\t\tversion: %u" //"\t\tpacketsMask:%s" - "\t\tfirstbytedata: 0x%x\t\tdatsize: %u\n\n", + "\t\tfirstbytedata: 0x%x\t\tdatsize: %zu\n\n", detectorHeader.row, (long long unsigned int)detectorHeader.frameNumber, detectorHeader.expLength, detectorHeader.packetNumber, (long long unsigned int)detectorHeader.bunchId, @@ -144,10 +123,10 @@ void GetData(char *metadata, char *datapointer, uint32_t &revDatasize, detectorHeader.debug, detectorHeader.roundRNumber, detectorHeader.detType, detectorHeader.version, // header->packetsMask.to_string().c_str(), - ((uint8_t)(*((uint8_t *)(datapointer)))), revDatasize); + ((uint8_t)(*((uint8_t *)(dataPointer)))), modifiedImageSize); // if data is modified, eg ROI and size is reduced - revDatasize = 26000; + modifiedImageSize = 26000; } /** diff --git a/slsReceiverSoftware/src/Receiver.cpp b/slsReceiverSoftware/src/Receiver.cpp index 2701e8b17..79c1fa266 100644 --- a/slsReceiverSoftware/src/Receiver.cpp +++ b/slsReceiverSoftware/src/Receiver.cpp @@ -129,7 +129,7 @@ int64_t Receiver::getReceiverVersion() { } void Receiver::registerCallBackStartAcquisition( - int (*func)(std::string, std::string, uint64_t, uint32_t, void *), + int (*func)(const std::string &, const std::string &, uint64_t, size_t, void *), void *arg) { tcpipInterface->registerCallBackStartAcquisition(func, arg); } @@ -140,14 +140,13 @@ void Receiver::registerCallBackAcquisitionFinished(void (*func)(uint64_t, tcpipInterface->registerCallBackAcquisitionFinished(func, arg); } -void Receiver::registerCallBackRawDataReady(void (*func)(char *, char *, - uint32_t, void *), - void *arg) { +void Receiver::registerCallBackRawDataReady( + void (*func)(sls_receiver_header *, char *, size_t, void *), void *arg) { tcpipInterface->registerCallBackRawDataReady(func, arg); } void Receiver::registerCallBackRawDataModifyReady( - void (*func)(char *, char *, uint32_t &, void *), void *arg) { + void (*func)(sls_receiver_header *, char *, size_t &, void *), void *arg) { tcpipInterface->registerCallBackRawDataModifyReady(func, arg); } From e9dc3d8c389eef4fffa7b80ba4e068fa57fd38d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Fr=C3=B6jdh?= Date: Thu, 7 Apr 2022 14:39:26 +0200 Subject: [PATCH 2/3] minor changes (#429) Various small changes to the data processor --- .clang-tidy | 1 + slsReceiverSoftware/src/DataProcessor.cpp | 46 ++++++++++------------- slsReceiverSoftware/src/DataProcessor.h | 2 +- 3 files changed, 21 insertions(+), 28 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index be4381b08..b1dfd03d3 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -19,6 +19,7 @@ Checks: '*, -google-readability-braces-around-statements, -modernize-use-trailing-return-type, -readability-isolate-declaration, + -readability-implicit-bool-conversion, -llvmlibc-*' HeaderFilterRegex: \.h diff --git a/slsReceiverSoftware/src/DataProcessor.cpp b/slsReceiverSoftware/src/DataProcessor.cpp index ede68129c..c38668507 100644 --- a/slsReceiverSoftware/src/DataProcessor.cpp +++ b/slsReceiverSoftware/src/DataProcessor.cpp @@ -42,14 +42,10 @@ DataProcessor::DataProcessor(int index, detectorType detectorType, Fifo *fifo, ctbAnalogDataBytes_(ctbAnalogDataBytes), firstStreamerFrame_(false) { LOG(logDEBUG) << "DataProcessor " << index << " created"; - - memset((void *)&timerbegin_, 0, sizeof(timespec)); } DataProcessor::~DataProcessor() { DeleteFiles(); } -/** getters */ - bool DataProcessor::GetStartedFlag() const { return startedFlag_; } void DataProcessor::SetFifo(Fifo *fifo) { fifo_ = fifo; } @@ -66,10 +62,8 @@ void DataProcessor::ResetParametersforNewAcquisition() { void DataProcessor::RecordFirstIndex(uint64_t fnum) { // listen to this fnum, later +1 currentFrameIndex_ = fnum; - startedFlag_ = true; firstIndex_ = fnum; - LOG(logDEBUG1) << index << " First Index:" << firstIndex_; } @@ -84,10 +78,8 @@ void DataProcessor::CloseFiles() { void DataProcessor::DeleteFiles() { CloseFiles(); - if (dataFile_) { - delete dataFile_; - dataFile_ = nullptr; - } + delete dataFile_; + dataFile_ = nullptr; } void DataProcessor::SetupFileWriter(const bool filewriteEnable, const fileFormat fileFormatType, @@ -231,13 +223,11 @@ std::string DataProcessor::CreateMasterFile( void DataProcessor::ThreadExecution() { char *buffer = nullptr; fifo_->PopAddress(buffer); - LOG(logDEBUG5) << "DataProcessor " << index - << ", " - "pop 0x" - << std::hex << (void *)(buffer) << std::dec << ":" << buffer; + LOG(logDEBUG5) << "DataProcessor " << index << ", " << std::hex + << static_cast(buffer) << std::dec << ":" << buffer; // check dummy - auto numBytes = (uint32_t)(*((uint32_t *)buffer)); + auto numBytes = *reinterpret_cast(buffer); LOG(logDEBUG1) << "DataProcessor " << index << ", Numbytes:" << numBytes; if (numBytes == DUMMY_PACKET_VALUE) { StopProcessing(buffer); @@ -282,7 +272,7 @@ void DataProcessor::StopProcessing(char *buf) { uint64_t DataProcessor::ProcessAnImage(char *buf) { - auto *rheader = (sls_receiver_header *)(buf + FIFO_HEADER_NUMBYTES); + auto *rheader = reinterpret_cast(buf + FIFO_HEADER_NUMBYTES); sls_detector_header header = rheader->detHeader; uint64_t fnum = header.frameNumber; currentFrameIndex_ = fnum; @@ -316,7 +306,7 @@ uint64_t DataProcessor::ProcessAnImage(char *buf) { try { // normal call back if (rawDataReadyCallBack != nullptr) { - std::size_t dsize = *reinterpret_cast(buf); + std::size_t dsize = *reinterpret_cast(buf); rawDataReadyCallBack(rheader, buf + FIFO_HEADER_NUMBYTES + sizeof(sls_receiver_header), @@ -325,7 +315,7 @@ uint64_t DataProcessor::ProcessAnImage(char *buf) { // call back with modified size else if (rawDataModifyReadyCallBack != nullptr) { - std::size_t revsize = *reinterpret_cast(buf); + std::size_t revsize = *reinterpret_cast(buf); rawDataModifyReadyCallBack(rheader, buf + FIFO_HEADER_NUMBYTES + sizeof(sls_receiver_header), @@ -370,14 +360,15 @@ bool DataProcessor::CheckTimer() { struct timespec end; clock_gettime(CLOCK_REALTIME, &end); - LOG(logDEBUG1) << index << " Timer elapsed time:" - << ((end.tv_sec - timerbegin_.tv_sec) + - (end.tv_nsec - timerbegin_.tv_nsec) / 1000000000.0) + auto elapsed_s = (end.tv_sec - timerbegin_.tv_sec) + + (end.tv_nsec - timerbegin_.tv_nsec) / 1e9; + double timer_s = *streamingTimerInMs_ / 1e3; + + LOG(logDEBUG1) << index << " Timer elapsed time:" << elapsed_s << " seconds"; + // still less than streaming timer, keep waiting - if (((end.tv_sec - timerbegin_.tv_sec) + - (end.tv_nsec - timerbegin_.tv_nsec) / 1000000000.0) < - ((double)*streamingTimerInMs_ / 1000.00)) + if (elapsed_s < timer_s) return false; // restart timer @@ -410,7 +401,8 @@ void DataProcessor::PadMissingPackets(char *buf) { LOG(logDEBUG) << index << ": Padding Missing Packets"; uint32_t pperFrame = generalData_->packetsPerFrame; - auto *header = (sls_receiver_header *)(buf + FIFO_HEADER_NUMBYTES); + auto *header = + reinterpret_cast(buf + FIFO_HEADER_NUMBYTES); uint32_t nmissing = pperFrame - header->detHeader.packetNumber; sls_bitset pmask = header->packetsMask; @@ -483,7 +475,7 @@ void DataProcessor::RearrangeDbitData(char *buf) { // ceil as numResult8Bits could be decimal const int numResult8Bits = - ceil((double)(numSamples * (*ctbDbitList_).size()) / 8.00); + ceil((numSamples * (*ctbDbitList_).size()) / 8.00); std::vector result(numResult8Bits); uint8_t *dest = &result[0]; @@ -499,7 +491,7 @@ void DataProcessor::RearrangeDbitData(char *buf) { } // loop through the frame digital data - for (auto ptr = source; ptr < (source + numSamples);) { + for (auto *ptr = source; ptr < (source + numSamples);) { // get selected bit from each 8 bit uint8_t bit = (*ptr++ >> bi) & 1; *dest |= bit << bitoffset; diff --git a/slsReceiverSoftware/src/DataProcessor.h b/slsReceiverSoftware/src/DataProcessor.h index 9756b6896..69cc0a33b 100644 --- a/slsReceiverSoftware/src/DataProcessor.h +++ b/slsReceiverSoftware/src/DataProcessor.h @@ -155,7 +155,7 @@ class DataProcessor : private virtual slsDetectorDefs, public ThreadObject { uint32_t *streamingTimerInMs_; uint32_t *streamingStartFnum_; uint32_t currentFreqCount_{0}; - struct timespec timerbegin_; + struct timespec timerbegin_{}; bool *framePadding_; std::vector *ctbDbitList_; int *ctbDbitOffset_; From d8c6f9141dbdc8dd6cbdb1f2d92cb7dcdda4d5fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Fr=C3=B6jdh?= Date: Thu, 7 Apr 2022 16:20:54 +0200 Subject: [PATCH 3/3] Fixed crash on gendoc (#430) Fixed by checking for help action before using the detector added test that checks that for all helps this doesn't crash Disabled Timer tests by default since they take ~2s --- docs/src/gendoc.cpp | 1 + slsDetectorSoftware/src/CmdProxy.cpp | 19 ++++++++++++------- slsDetectorSoftware/tests/test-CmdProxy.cpp | 11 +++++++++++ slsSupportLib/tests/test-Timer.cpp | 4 ++-- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/docs/src/gendoc.cpp b/docs/src/gendoc.cpp index 9044f563e..78c0520f3 100644 --- a/docs/src/gendoc.cpp +++ b/docs/src/gendoc.cpp @@ -45,6 +45,7 @@ int main() { for (const auto &cmd : commands) { std::ostringstream os; + std::cout << cmd << '\n'; proxy.Call(cmd, {}, -1, slsDetectorDefs::HELP_ACTION, os); auto tmp = os.str().erase(0, cmd.size()); diff --git a/slsDetectorSoftware/src/CmdProxy.cpp b/slsDetectorSoftware/src/CmdProxy.cpp index dc61cc538..ddd97eb41 100644 --- a/slsDetectorSoftware/src/CmdProxy.cpp +++ b/slsDetectorSoftware/src/CmdProxy.cpp @@ -1056,7 +1056,18 @@ std::string CmdProxy::TemperatureValues(int action) { std::string CmdProxy::Dac(int action) { std::ostringstream os; os << cmd << ' '; + + if (action == defs::HELP_ACTION) { + if (args.size() == 0) { + os << GetHelpDac(std::to_string(0)) << '\n'; + } else { + os << args[0] << ' ' << GetHelpDac(args[0]) << '\n'; + } + return os.str(); + } + auto type = det->getDetectorType().squash(); + // dac indices only for ctb if (args.size() > 0 && action != defs::HELP_ACTION) { if (is_int(args[0]) && type != defs::CHIPTESTBOARD) { @@ -1066,13 +1077,7 @@ std::string CmdProxy::Dac(int action) { } } - if (action == defs::HELP_ACTION) { - if (args.size() == 0) { - os << GetHelpDac(std::to_string(0)) << '\n'; - } else { - os << args[0] << ' ' << GetHelpDac(args[0]) << '\n'; - } - } else if (action == defs::GET_ACTION) { + if (action == defs::GET_ACTION) { if (args.empty()) WrongNumberOfParameters(1); // This prints slightly wrong diff --git a/slsDetectorSoftware/tests/test-CmdProxy.cpp b/slsDetectorSoftware/tests/test-CmdProxy.cpp index ff451bff5..f3e631fa4 100644 --- a/slsDetectorSoftware/tests/test-CmdProxy.cpp +++ b/slsDetectorSoftware/tests/test-CmdProxy.cpp @@ -16,6 +16,17 @@ using sls::Detector; using test::GET; using test::PUT; +TEST_CASE("Calling help doesn't throw or cause segfault"){ + //Dont add [.cmd] tag this should run with normal tests + CmdProxy proxy(nullptr); + auto commands = proxy.GetProxyCommands(); + std::ostringstream os; + for (const auto &cmd : commands) + REQUIRE_NOTHROW(proxy.Call(cmd, {}, -1, slsDetectorDefs::HELP_ACTION, os)); + + +} + TEST_CASE("Unknown command", "[.cmd]") { Detector det; diff --git a/slsSupportLib/tests/test-Timer.cpp b/slsSupportLib/tests/test-Timer.cpp index f5733a2de..9d3ae69cb 100644 --- a/slsSupportLib/tests/test-Timer.cpp +++ b/slsSupportLib/tests/test-Timer.cpp @@ -6,7 +6,7 @@ #include #include -TEST_CASE("Time 1s restart then time 2s") { +TEST_CASE("Time 1s restart then time 2s", "[.timer]") { auto sleep_duration = std::chrono::seconds(1); auto t = sls::Timer(); std::this_thread::sleep_for(sleep_duration); @@ -17,7 +17,7 @@ TEST_CASE("Time 1s restart then time 2s") { REQUIRE(t.elapsed_s() == Approx(2).epsilon(0.01)); } -TEST_CASE("Return ms") { +TEST_CASE("Return ms", "[.timer]") { auto sleep_duration = std::chrono::milliseconds(1300); auto t = sls::Timer(); std::this_thread::sleep_for(sleep_duration);