From e67ff6a937c155659ee49118bf80ad238093cc3b Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 8 Mar 2021 23:00:53 +0100 Subject: [PATCH 01/37] cleaner wiretapping handling --- osal/windows/CMakeLists.txt | 15 +++++---- osal/windows/TcWinTcpServer.cpp | 46 ++++++++++++++++++++++++++++ osal/windows/TcWinTcpServer.h | 30 ++++++++++++++++++ osal/windows/TcWinUdpPollingTask.cpp | 33 ++++++++++---------- osal/windows/TcWinUdpPollingTask.h | 6 ++-- osal/windows/TmTcWinUdpBridge.cpp | 40 ++++++++++++------------ osal/windows/TmTcWinUdpBridge.h | 5 ++- 7 files changed, 127 insertions(+), 48 deletions(-) create mode 100644 osal/windows/TcWinTcpServer.cpp create mode 100644 osal/windows/TcWinTcpServer.h diff --git a/osal/windows/CMakeLists.txt b/osal/windows/CMakeLists.txt index b6e76d6a..889ea339 100644 --- a/osal/windows/CMakeLists.txt +++ b/osal/windows/CMakeLists.txt @@ -1,11 +1,10 @@ -target_sources(${LIB_FSFW_NAME} - PRIVATE - TcWinUdpPollingTask.cpp - TmTcWinUdpBridge.cpp +target_sources(${LIB_FSFW_NAME} PRIVATE + TcWinUdpPollingTask.cpp + TmTcWinUdpBridge.cpp + TcWinTcpServer.cpp ) -target_link_libraries(${LIB_FSFW_NAME} - PRIVATE - wsock32 - ws2_32 +target_link_libraries(${LIB_FSFW_NAME} PRIVATE + wsock32 + ws2_32 ) \ No newline at end of file diff --git a/osal/windows/TcWinTcpServer.cpp b/osal/windows/TcWinTcpServer.cpp new file mode 100644 index 00000000..aebc0bee --- /dev/null +++ b/osal/windows/TcWinTcpServer.cpp @@ -0,0 +1,46 @@ +#include "TcWinTcpServer.h" +#include "../../serviceinterface/ServiceInterface.h" +#include + +TcWinTcpServer::TcWinTcpServer(object_id_t objectId, object_id_t tmtcUnixUdpBridge): + SystemObject(objectId) { + /* Open TCP socket */ + serverTcpSocket = socket(AF_INET, SOCK_STREAM, 0); + if(serverTcpSocket == 0) { +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "TcWinTcpServer::TcWinTcpServer: Socket creation failed!" << std::endl; + handleSocketError(); +#endif + } + + setsockopt(serverTcpSocket, SOL_SOCKET, SO_REUSEADDR | SO_BROADCAST, + &tcpSockOpt, sizeof(tcpSockOpt)); +} + +TcWinTcpServer::~TcWinTcpServer() { +} + +void TcWinTcpServer::handleSocketError() { + int errCode = WSAGetLastError(); + switch(errCode) { + case(WSANOTINITIALISED): { +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "TmTcWinUdpBridge::handleSocketError: WSANOTINITIALISED: WSAStartup" + " call necessary" << std::endl; +#endif + break; + } + default: { + /* + https://docs.microsoft.com/en-us/windows/win32/winsock/ + windows-sockets-error-codes-2 + */ +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "TmTcWinUdpBridge::handleSocketError: Error code: " << errCode << std::endl; +#endif + break; + } + } +} + + diff --git a/osal/windows/TcWinTcpServer.h b/osal/windows/TcWinTcpServer.h new file mode 100644 index 00000000..3a17336a --- /dev/null +++ b/osal/windows/TcWinTcpServer.h @@ -0,0 +1,30 @@ +#ifndef FSFW_OSAL_WINDOWS_TCWINTCPSERVER_H_ +#define FSFW_OSAL_WINDOWS_TCWINTCPSERVER_H_ + +#include "../../objectmanager/SystemObject.h" +#include "../../tasks/ExecutableObjectIF.h" + +#include + +class TcWinTcpServer: + public SystemObject, + public ExecutableObjectIF { +public: + /* The ports chosen here should not be used by any other process. */ + static constexpr uint16_t DEFAULT_TCP_SERVER_PORT = 7301; + static constexpr uint16_t DEFAULT_TCP_CLIENT_PORT = 7302; + + TcWinTcpServer(object_id_t objectId, object_id_t tmtcUnixUdpBridge); + virtual~ TcWinTcpServer(); + +private: + + SOCKET serverTcpSocket = 0; + + std::vector receptionBuffer; + int tcpSockOpt = 0; + + void handleSocketError(); +}; + +#endif /* FSFW_OSAL_WINDOWS_TCWINTCPSERVER_H_ */ diff --git a/osal/windows/TcWinUdpPollingTask.cpp b/osal/windows/TcWinUdpPollingTask.cpp index f9b3753e..32c21700 100644 --- a/osal/windows/TcWinUdpPollingTask.cpp +++ b/osal/windows/TcWinUdpPollingTask.cpp @@ -20,8 +20,8 @@ TcWinUdpPollingTask::TcWinUdpPollingTask(object_id_t objectId, this->frameSize = DEFAULT_MAX_FRAME_SIZE; } - // Set up reception buffer with specified frame size. - // For now, it is assumed that only one frame is held in the buffer! + /* Set up reception buffer with specified frame size. + For now, it is assumed that only one frame is held in the buffer! */ receptionBuffer.reserve(this->frameSize); receptionBuffer.resize(this->frameSize); @@ -36,7 +36,7 @@ TcWinUdpPollingTask::TcWinUdpPollingTask(object_id_t objectId, TcWinUdpPollingTask::~TcWinUdpPollingTask() {} ReturnValue_t TcWinUdpPollingTask::performOperation(uint8_t opCode) { - // Poll for new UDP datagrams in permanent loop. + /* Poll for new UDP datagrams in permanent loop. */ while(true) { //! Sender Address is cached here. struct sockaddr_in senderAddress; @@ -46,7 +46,7 @@ ReturnValue_t TcWinUdpPollingTask::performOperation(uint8_t opCode) { receptionFlags, reinterpret_cast(&senderAddress), &senderAddressSize); if(bytesReceived == SOCKET_ERROR) { - // handle error + /* Handle error */ #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::error << "TcWinUdpPollingTask::performOperation: Reception" " error." << std::endl; @@ -54,9 +54,9 @@ ReturnValue_t TcWinUdpPollingTask::performOperation(uint8_t opCode) { handleReadError(); continue; } -#if FSFW_CPP_OSTREAM_ENABLED == 1 - //sif::debug << "TcWinUdpPollingTask::performOperation: " << bytesReceived - // << " bytes received" << std::endl; +#if FSFW_CPP_OSTREAM_ENABLED == 1 && FSFW_UDP_WIRETAPPING_ENABLED == 1 + sif::debug << "TcWinUdpPollingTask::performOperation: " << bytesReceived << + " bytes received" << std::endl; #endif ReturnValue_t result = handleSuccessfullTcRead(bytesReceived); @@ -74,12 +74,14 @@ ReturnValue_t TcWinUdpPollingTask::handleSuccessfullTcRead(size_t bytesRead) { store_address_t storeId; ReturnValue_t result = tcStore->addData(&storeId, receptionBuffer.data(), bytesRead); - // arrayprinter::print(receptionBuffer.data(), bytesRead); +#if FSFW_UDP_WIRETAPPING_ENABLED == 1 + arrayprinter::print(receptionBuffer.data(), bytesRead);# +#endif if (result != HasReturnvaluesIF::RETURN_OK) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "TcSerialPollingTask::transferPusToSoftwareBus: Data " - "storage failed" << std::endl; - sif::error << "Packet size: " << bytesRead << std::endl; + sif::warning<< "TcSerialPollingTask::transferPusToSoftwareBus: Data " + "storage failed" << std::endl; + sif::warning << "Packet size: " << bytesRead << std::endl; #endif return HasReturnvaluesIF::RETURN_FAILED; } @@ -89,8 +91,7 @@ ReturnValue_t TcWinUdpPollingTask::handleSuccessfullTcRead(size_t bytesRead) { result = MessageQueueSenderIF::sendMessage(targetTcDestination, &message); if (result != HasReturnvaluesIF::RETURN_OK) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "Serial Polling: Sending message to queue failed" - << std::endl; + sif::warning << "Serial Polling: Sending message to queue failed" << std::endl; #endif tcStore->deleteData(storeId); } @@ -117,9 +118,9 @@ ReturnValue_t TcWinUdpPollingTask::initialize() { } serverUdpSocket = tmtcBridge->serverSocket; -#if FSFW_CPP_OSTREAM_ENABLED == 1 - //sif::info << "TcWinUdpPollingTask::initialize: Server UDP socket " - // << serverUdpSocket << std::endl; +#if FSFW_CPP_OSTREAM_ENABLED == 1 && FSFW_UDP_WIRETAPPING_ENABLED == 1 + sif::info << "TcWinUdpPollingTask::initialize: Server UDP socket " << serverUdpSocket << + std::endl; #endif return HasReturnvaluesIF::RETURN_OK; diff --git a/osal/windows/TcWinUdpPollingTask.h b/osal/windows/TcWinUdpPollingTask.h index 063a783e..8a9cf51f 100644 --- a/osal/windows/TcWinUdpPollingTask.h +++ b/osal/windows/TcWinUdpPollingTask.h @@ -8,6 +8,9 @@ #include +//! Debugging preprocessor define. +#define FSFW_UDP_RCV_WIRETAPPING_ENABLED 0 + /** * @brief This class can be used to implement the polling of a Unix socket, * using UDP for now. @@ -51,8 +54,7 @@ private: //! Reception flags: https://linux.die.net/man/2/recvfrom. int receptionFlags = 0; - //! Server socket, which is member of TMTC bridge and is assigned in - //! constructor + //! Server socket, which is member of TMTC bridge and is assigned in constructor SOCKET serverUdpSocket = 0; std::vector receptionBuffer; diff --git a/osal/windows/TmTcWinUdpBridge.cpp b/osal/windows/TmTcWinUdpBridge.cpp index ac890198..dce88bb2 100644 --- a/osal/windows/TmTcWinUdpBridge.cpp +++ b/osal/windows/TmTcWinUdpBridge.cpp @@ -85,13 +85,12 @@ TmTcWinUdpBridge::~TmTcWinUdpBridge() { ReturnValue_t TmTcWinUdpBridge::sendTm(const uint8_t *data, size_t dataLen) { int flags = 0; - //clientAddress.sin_addr.s_addr = htons(INADDR_ANY); - //clientAddressLen = sizeof(serverAddress); - -// char ipAddress [15]; -#if FSFW_CPP_OSTREAM_ENABLED == 1 -// sif::debug << "IP Address Sender: "<< inet_ntop(AF_INET, -// &clientAddress.sin_addr.s_addr, ipAddress, 15) << std::endl; +#if FSFW_CPP_OSTREAM_ENABLED == 1 && FSFW_UDP_SEND_WIRETAPPING_ENABLED == 1 + clientAddress.sin_addr.s_addr = htons(INADDR_ANY); + clientAddressLen = sizeof(serverAddress); + char ipAddress [15]; + sif::debug << "IP Address Sender: "<< inet_ntop(AF_INET, + &clientAddress.sin_addr.s_addr, ipAddress, 15) << std::endl; #endif ssize_t bytesSent = sendto(serverSocket, @@ -104,9 +103,9 @@ ReturnValue_t TmTcWinUdpBridge::sendTm(const uint8_t *data, size_t dataLen) { #endif handleSendError(); } -#if FSFW_CPP_OSTREAM_ENABLED == 1 -// sif::debug << "TmTcUnixUdpBridge::sendTm: " << bytesSent << " bytes were" -// " sent." << std::endl; +#if FSFW_CPP_OSTREAM_ENABLED == 1 && FSFW_UDP_SEND_WIRETAPPING_ENABLED == 1 + sif::debug << "TmTcUnixUdpBridge::sendTm: " << bytesSent << " bytes were" + " sent." << std::endl; #endif return HasReturnvaluesIF::RETURN_OK; } @@ -114,16 +113,16 @@ ReturnValue_t TmTcWinUdpBridge::sendTm(const uint8_t *data, size_t dataLen) { void TmTcWinUdpBridge::checkAndSetClientAddress(sockaddr_in newAddress) { MutexHelper lock(mutex, MutexIF::TimeoutType::WAITING, 10); -// char ipAddress [15]; -#if FSFW_CPP_OSTREAM_ENABLED == 1 -// sif::debug << "IP Address Sender: "<< inet_ntop(AF_INET, -// &newAddress.sin_addr.s_addr, ipAddress, 15) << std::endl; -// sif::debug << "IP Address Old: " << inet_ntop(AF_INET, -// &clientAddress.sin_addr.s_addr, ipAddress, 15) << std::endl; +#if FSFW_CPP_OSTREAM_ENABLED == 1 && FSFW_UDP_SEND_WIRETAPPING_ENABLED == 1 + char ipAddress [15]; + sif::debug << "IP Address Sender: "<< inet_ntop(AF_INET, + &newAddress.sin_addr.s_addr, ipAddress, 15) << std::endl; + sif::debug << "IP Address Old: " << inet_ntop(AF_INET, + &clientAddress.sin_addr.s_addr, ipAddress, 15) << std::endl; #endif registerCommConnect(); - // Set new IP address if it has changed. + /* Set new IP address if it has changed. */ if(clientAddress.sin_addr.s_addr != newAddress.sin_addr.s_addr) { clientAddress.sin_addr.s_addr = newAddress.sin_addr.s_addr; clientAddressLen = sizeof(clientAddress); @@ -135,8 +134,8 @@ void TmTcWinUdpBridge::handleSocketError() { switch(errCode) { case(WSANOTINITIALISED): { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::info << "TmTcWinUdpBridge::handleSocketError: WSANOTINITIALISED: " - << "WSAStartup(...) call necessary" << std::endl; + sif::warning << "TmTcWinUdpBridge::handleSocketError: WSANOTINITIALISED: WSAStartup" + " call necessary" << std::endl; #endif break; } @@ -146,8 +145,7 @@ void TmTcWinUdpBridge::handleSocketError() { windows-sockets-error-codes-2 */ #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::info << "TmTcWinUdpBridge::handleSocketError: Error code: " - << errCode << std::endl; + sif::warning << "TmTcWinUdpBridge::handleSocketError: Error code: " << errCode << std::endl; #endif break; } diff --git a/osal/windows/TmTcWinUdpBridge.h b/osal/windows/TmTcWinUdpBridge.h index 8188039c..b51c8c7a 100644 --- a/osal/windows/TmTcWinUdpBridge.h +++ b/osal/windows/TmTcWinUdpBridge.h @@ -6,10 +6,13 @@ #include #include +//! Debugging preprocessor define. +#define FSFW_UDP_SEND_WIRETAPPING_ENABLED 0 + class TmTcWinUdpBridge: public TmTcBridge { friend class TcWinUdpPollingTask; public: - // The ports chosen here should not be used by any other process. + /* The ports chosen here should not be used by any other process. */ static constexpr uint16_t DEFAULT_UDP_SERVER_PORT = 7301; static constexpr uint16_t DEFAULT_UDP_CLIENT_PORT = 7302; From 4e5e6e145ecfe866b34a2ed1ef94de1e192a1053 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 8 Mar 2021 23:02:06 +0100 Subject: [PATCH 02/37] added win sock --- osal/windows/TmTcWinUdpBridge.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/osal/windows/TmTcWinUdpBridge.cpp b/osal/windows/TmTcWinUdpBridge.cpp index dce88bb2..07fedd02 100644 --- a/osal/windows/TmTcWinUdpBridge.cpp +++ b/osal/windows/TmTcWinUdpBridge.cpp @@ -38,8 +38,9 @@ TmTcWinUdpBridge::TmTcWinUdpBridge(object_id_t objectId, setClientPort = clientPort; } - // Set up UDP socket: https://man7.org/linux/man-pages/man7/ip.7.html - //clientSocket = socket(AF_INET, SOCK_DGRAM, 0); + /* Set up UDP socket: + https://docs.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-socket + */ serverSocket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); if(serverSocket == INVALID_SOCKET) { #if FSFW_CPP_OSTREAM_ENABLED == 1 @@ -52,7 +53,7 @@ TmTcWinUdpBridge::TmTcWinUdpBridge(object_id_t objectId, serverAddress.sin_family = AF_INET; - // Accept packets from any interface. (potentially insecure). + /* Accept packets from any interface. (potentially insecure). */ serverAddress.sin_addr.s_addr = htonl(INADDR_ANY); serverAddress.sin_port = htons(setServerPort); serverAddressLen = sizeof(serverAddress); From 8be4f45969ea46b73f10973dde548af84d9a3621 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 8 Mar 2021 23:14:10 +0100 Subject: [PATCH 03/37] added generic error handler --- osal/windows/TcWinTcpServer.cpp | 35 ++++++++++++++++++++----------- osal/windows/TcWinTcpServer.h | 7 ++++++- osal/windows/TmTcWinUdpBridge.cpp | 19 +++++++++++------ osal/windows/TmTcWinUdpBridge.h | 5 +++++ 4 files changed, 47 insertions(+), 19 deletions(-) diff --git a/osal/windows/TcWinTcpServer.cpp b/osal/windows/TcWinTcpServer.cpp index aebc0bee..756e7776 100644 --- a/osal/windows/TcWinTcpServer.cpp +++ b/osal/windows/TcWinTcpServer.cpp @@ -9,25 +9,39 @@ TcWinTcpServer::TcWinTcpServer(object_id_t objectId, object_id_t tmtcUnixUdpBrid if(serverTcpSocket == 0) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "TcWinTcpServer::TcWinTcpServer: Socket creation failed!" << std::endl; - handleSocketError(); + handleError(ErrorSources::SOCKET_CALL); #endif } - setsockopt(serverTcpSocket, SOL_SOCKET, SO_REUSEADDR | SO_BROADCAST, - &tcpSockOpt, sizeof(tcpSockOpt)); + int retval = setsockopt(serverTcpSocket, SOL_SOCKET, SO_REUSEADDR | SO_BROADCAST, + reinterpret_cast(&tcpSockOpt), sizeof(tcpSockOpt)); + if(retval != 0) { + + } } TcWinTcpServer::~TcWinTcpServer() { } -void TcWinTcpServer::handleSocketError() { +void TcWinTcpServer::handleError(ErrorSources errorSrc) { +#if FSFW_CPP_OSTREAM_ENABLED == 1 int errCode = WSAGetLastError(); + std::string errorSrcString; + if(errorSrc == ErrorSources::SETSOCKOPT_CALL) { + errorSrcString = "setsockopt call"; + } + else if(errorSrc == ErrorSources::SOCKET_CALL) { + errorSrcString = "socket call"; + } switch(errCode) { case(WSANOTINITIALISED): { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "TmTcWinUdpBridge::handleSocketError: WSANOTINITIALISED: WSAStartup" - " call necessary" << std::endl; -#endif + sif::warning << "TmTcWinUdpBridge::handleError: " << errorSrcString << " | " + "WSANOTINITIALISED: WSAStartup call necessary" << std::endl; + break; + } + case(WSAEINVAL): { + sif::warning << "TmTcWinUdpBridge::handleError: " << errorSrcString << " | " + "WSAEINVAL: Invalid parameters" << std::endl; break; } default: { @@ -35,12 +49,9 @@ void TcWinTcpServer::handleSocketError() { https://docs.microsoft.com/en-us/windows/win32/winsock/ windows-sockets-error-codes-2 */ -#if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "TmTcWinUdpBridge::handleSocketError: Error code: " << errCode << std::endl; -#endif break; } } +#endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */ } - - diff --git a/osal/windows/TcWinTcpServer.h b/osal/windows/TcWinTcpServer.h index 3a17336a..4f53dda4 100644 --- a/osal/windows/TcWinTcpServer.h +++ b/osal/windows/TcWinTcpServer.h @@ -24,7 +24,12 @@ private: std::vector receptionBuffer; int tcpSockOpt = 0; - void handleSocketError(); + enum class ErrorSources { + SOCKET_CALL, + SETSOCKOPT_CALL + }; + + void handleError(ErrorSources errorSrc); }; #endif /* FSFW_OSAL_WINDOWS_TCWINTCPSERVER_H_ */ diff --git a/osal/windows/TmTcWinUdpBridge.cpp b/osal/windows/TmTcWinUdpBridge.cpp index 07fedd02..375e45e7 100644 --- a/osal/windows/TmTcWinUdpBridge.cpp +++ b/osal/windows/TmTcWinUdpBridge.cpp @@ -44,8 +44,8 @@ TmTcWinUdpBridge::TmTcWinUdpBridge(object_id_t objectId, serverSocket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); if(serverSocket == INVALID_SOCKET) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "TmTcWinUdpBridge::TmTcWinUdpBridge: Could not open" - " UDP socket!" << std::endl; + sif::warning << "TmTcWinUdpBridge::TmTcWinUdpBridge: Could not open UDP socket!" << + std::endl; #endif handleSocketError(); return; @@ -57,16 +57,23 @@ TmTcWinUdpBridge::TmTcWinUdpBridge(object_id_t objectId, serverAddress.sin_addr.s_addr = htonl(INADDR_ANY); serverAddress.sin_port = htons(setServerPort); serverAddressLen = sizeof(serverAddress); - setsockopt(serverSocket, SOL_SOCKET, SO_REUSEADDR, + int result = setsockopt(serverSocket, SOL_SOCKET, SO_REUSEADDR, reinterpret_cast(&serverSocketOptions), sizeof(serverSocketOptions)); + if(result != 0) { +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "TmTcWinUdpBridge::TmTcWinUdpBridge: Could not set socket options!" << + std::endl; +#endif + handleSocketError(); + } clientAddress.sin_family = AF_INET; clientAddress.sin_addr.s_addr = htonl(INADDR_ANY); clientAddress.sin_port = htons(setClientPort); clientAddressLen = sizeof(clientAddress); - int result = bind(serverSocket, + result = bind(serverSocket, reinterpret_cast(&serverAddress), serverAddressLen); if(result != 0) { @@ -159,14 +166,14 @@ void TmTcWinUdpBridge::handleBindError() { case(WSANOTINITIALISED): { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::info << "TmTcWinUdpBridge::handleBindError: WSANOTINITIALISED: " - << "WSAStartup(...) call " << "necessary" << std::endl; + << "WSAStartup call necessary" << std::endl; #endif break; } case(WSAEADDRINUSE): { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "TmTcWinUdpBridge::handleBindError: WSAEADDRINUSE: " - << "Port is already in use!" << std::endl; + "Port is already in use!" << std::endl; #endif break; } diff --git a/osal/windows/TmTcWinUdpBridge.h b/osal/windows/TmTcWinUdpBridge.h index b51c8c7a..14354139 100644 --- a/osal/windows/TmTcWinUdpBridge.h +++ b/osal/windows/TmTcWinUdpBridge.h @@ -41,6 +41,11 @@ private: //! by another task. MutexIF* mutex; + enum class ErrorSources { + SOCKET_CALL, + SETSOCKOPT_CALL + }; + void handleSocketError(); void handleBindError(); void handleSendError(); From 494dd0db324af343e1596a295506bc1748ed584f Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 8 Mar 2021 23:55:58 +0100 Subject: [PATCH 04/37] continued tcp server --- osal/windows/TcWinTcpServer.cpp | 76 +++++++++++++++++++++++++++++-- osal/windows/TcWinTcpServer.h | 14 +++++- osal/windows/TmTcWinUdpBridge.cpp | 3 +- 3 files changed, 87 insertions(+), 6 deletions(-) diff --git a/osal/windows/TcWinTcpServer.cpp b/osal/windows/TcWinTcpServer.cpp index 756e7776..d090df45 100644 --- a/osal/windows/TcWinTcpServer.cpp +++ b/osal/windows/TcWinTcpServer.cpp @@ -2,10 +2,32 @@ #include "../../serviceinterface/ServiceInterface.h" #include -TcWinTcpServer::TcWinTcpServer(object_id_t objectId, object_id_t tmtcUnixUdpBridge): + +TcWinTcpServer::TcWinTcpServer(object_id_t objectId, object_id_t tmtcUnixUdpBridge, + uint16_t customTcpServerPort): SystemObject(objectId) { - /* Open TCP socket */ + /* Initiates Winsock DLL. */ + WSAData wsaData; + WORD wVersionRequested = MAKEWORD(2, 2); + int err = WSAStartup(wVersionRequested, &wsaData); + if (err != 0) { + /* Tell the user that we could not find a usable */ + /* Winsock DLL. */ +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::error << "TmTcWinUdpBridge::TmTcWinUdpBridge:" + "WSAStartup failed with error: " << err << std::endl; +#endif + return; + } + + /* Open TCP (stream) socket */ serverTcpSocket = socket(AF_INET, SOCK_STREAM, 0); + uint16_t tcpPort = customTcpServerPort; + + if(customTcpServerPort == 0xffff) { + tcpPort = DEFAULT_TCP_SERVER_PORT; + } + if(serverTcpSocket == 0) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "TcWinTcpServer::TcWinTcpServer: Socket creation failed!" << std::endl; @@ -16,11 +38,52 @@ TcWinTcpServer::TcWinTcpServer(object_id_t objectId, object_id_t tmtcUnixUdpBrid int retval = setsockopt(serverTcpSocket, SOL_SOCKET, SO_REUSEADDR | SO_BROADCAST, reinterpret_cast(&tcpSockOpt), sizeof(tcpSockOpt)); if(retval != 0) { - + sif::warning << "TcWinTcpServer::TcWinTcpServer: Setting socket options failed!" << + std::endl; + handleError(ErrorSources::SETSOCKOPT_CALL); } + tcpAddress.sin_family = AF_INET; + tcpAddress.sin_addr.s_addr = htonl(INADDR_ANY); + tcpAddress.sin_port = htons(tcpPort); + + retval = bind(serverTcpSocket, reinterpret_cast(&tcpAddress), + tcpAddrLen); + if(retval != 0) { + sif::warning << "TcWinTcpServer::TcWinTcpServer: Binding socket failed!" << + std::endl; + handleError(ErrorSources::BIND_CALL); + } + } TcWinTcpServer::~TcWinTcpServer() { + closesocket(serverTcpSocket); + WSACleanup(); +} + +ReturnValue_t TcWinTcpServer::initialize() { + return HasReturnvaluesIF::RETURN_OK; +} + +ReturnValue_t TcWinTcpServer::performOperation(uint8_t opCode) { + /* If a connection is accepted, the corresponding scoket will be assigned to the new socket */ + SOCKET connectorSocket; + sockaddr_in connectorSockAddr; + int connectorSockAddrLen = 0; + /* Listen for connection requests permanently for lifetime of program */ + while(true) { + int retval = listen(serverTcpSocket, backlog); + if(retval != 0) { + handleError(ErrorSources::LISTEN_CALL); + } + + connectorSocket = accept(serverTcpSocket, reinterpret_cast(&connectorSockAddr), + &connectorSockAddrLen); + + if(connectorSocket) {}; + + } + return HasReturnvaluesIF::RETURN_OK; } void TcWinTcpServer::handleError(ErrorSources errorSrc) { @@ -33,6 +96,13 @@ void TcWinTcpServer::handleError(ErrorSources errorSrc) { else if(errorSrc == ErrorSources::SOCKET_CALL) { errorSrcString = "socket call"; } + else if(errorSrc == ErrorSources::LISTEN_CALL) { + errorSrcString = "listen call"; + } + else if(errorSrc == ErrorSources::ACCEPT_CALL) { + errorSrcString = "accept call"; + } + switch(errCode) { case(WSANOTINITIALISED): { sif::warning << "TmTcWinUdpBridge::handleError: " << errorSrcString << " | " diff --git a/osal/windows/TcWinTcpServer.h b/osal/windows/TcWinTcpServer.h index 4f53dda4..45d92ff8 100644 --- a/osal/windows/TcWinTcpServer.h +++ b/osal/windows/TcWinTcpServer.h @@ -14,19 +14,29 @@ public: static constexpr uint16_t DEFAULT_TCP_SERVER_PORT = 7301; static constexpr uint16_t DEFAULT_TCP_CLIENT_PORT = 7302; - TcWinTcpServer(object_id_t objectId, object_id_t tmtcUnixUdpBridge); + TcWinTcpServer(object_id_t objectId, object_id_t tmtcUnixUdpBridge, + uint16_t customTcpServerPort = 0xffff); virtual~ TcWinTcpServer(); + ReturnValue_t initialize() override; + ReturnValue_t performOperation(uint8_t opCode) override; + private: SOCKET serverTcpSocket = 0; + struct sockaddr_in tcpAddress; + int tcpAddrLen = sizeof(tcpAddress); + int backlog = 3; std::vector receptionBuffer; int tcpSockOpt = 0; enum class ErrorSources { SOCKET_CALL, - SETSOCKOPT_CALL + SETSOCKOPT_CALL, + BIND_CALL, + LISTEN_CALL, + ACCEPT_CALL }; void handleError(ErrorSources errorSrc); diff --git a/osal/windows/TmTcWinUdpBridge.cpp b/osal/windows/TmTcWinUdpBridge.cpp index 375e45e7..d4467e44 100644 --- a/osal/windows/TmTcWinUdpBridge.cpp +++ b/osal/windows/TmTcWinUdpBridge.cpp @@ -14,7 +14,7 @@ TmTcWinUdpBridge::TmTcWinUdpBridge(object_id_t objectId, mutex = MutexFactory::instance()->createMutex(); communicationLinkUp = false; - // Initiates Winsock DLL. + /* Initiates Winsock DLL. */ WSAData wsaData; WORD wVersionRequested = MAKEWORD(2, 2); int err = WSAStartup(wVersionRequested, &wsaData); @@ -87,6 +87,7 @@ TmTcWinUdpBridge::TmTcWinUdpBridge(object_id_t objectId, } TmTcWinUdpBridge::~TmTcWinUdpBridge() { + closesocket(serverSocket); WSACleanup(); } From b6952424205a5a656294450ae365c54d7c5079a0 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 9 Mar 2021 00:37:42 +0100 Subject: [PATCH 05/37] contiued tcp and improved udp task --- osal/windows/TcWinTcpServer.cpp | 115 ++++++++++++++++++--------- osal/windows/TcWinTcpServer.h | 16 ++-- osal/windows/TcWinUdpPollingTask.cpp | 7 +- 3 files changed, 88 insertions(+), 50 deletions(-) diff --git a/osal/windows/TcWinTcpServer.cpp b/osal/windows/TcWinTcpServer.cpp index d090df45..1cf7ed1d 100644 --- a/osal/windows/TcWinTcpServer.cpp +++ b/osal/windows/TcWinTcpServer.cpp @@ -1,86 +1,120 @@ #include "TcWinTcpServer.h" #include "../../serviceinterface/ServiceInterface.h" #include +#include +const std::string TcWinTcpServer::DEFAULT_TCP_SERVER_PORT = "7301"; +const std::string TcWinTcpServer::DEFAULT_TCP_CLIENT_PORT = "7302"; TcWinTcpServer::TcWinTcpServer(object_id_t objectId, object_id_t tmtcUnixUdpBridge, - uint16_t customTcpServerPort): - SystemObject(objectId) { + std::string customTcpServerPort): + SystemObject(objectId), tcpPort(customTcpServerPort) { + if(tcpPort == "") { + tcpPort = DEFAULT_TCP_SERVER_PORT; + } +} + +ReturnValue_t TcWinTcpServer::initialize() { + int retval = 0; + struct addrinfo *addrResult = nullptr; + struct addrinfo hints; /* Initiates Winsock DLL. */ WSAData wsaData; WORD wVersionRequested = MAKEWORD(2, 2); int err = WSAStartup(wVersionRequested, &wsaData); if (err != 0) { - /* Tell the user that we could not find a usable */ - /* Winsock DLL. */ + /* Tell the user that we could not find a usable Winsock DLL. */ #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "TmTcWinUdpBridge::TmTcWinUdpBridge:" - "WSAStartup failed with error: " << err << std::endl; + sif::error << "TmTcWinUdpBridge::TmTcWinUdpBridge: WSAStartup failed with error: " << + err << std::endl; #endif - return; + return HasReturnvaluesIF::RETURN_FAILED; + } + + ZeroMemory(&hints, sizeof (hints)); + hints.ai_family = AF_INET; + hints.ai_socktype = SOCK_STREAM; + hints.ai_protocol = IPPROTO_TCP; + hints.ai_flags = AI_PASSIVE; + + retval = getaddrinfo(nullptr, tcpPort.c_str(), &hints, &addrResult); + if (retval != 0) { +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "TcWinTcpServer::TcWinTcpServer: Retrieving address info failed!" << + std::endl; +#endif + handleError(ErrorSources::GETADDRINFO_CALL); + return HasReturnvaluesIF::RETURN_FAILED; } /* Open TCP (stream) socket */ - serverTcpSocket = socket(AF_INET, SOCK_STREAM, 0); - uint16_t tcpPort = customTcpServerPort; - - if(customTcpServerPort == 0xffff) { - tcpPort = DEFAULT_TCP_SERVER_PORT; - } - - if(serverTcpSocket == 0) { + listenerTcpSocket = socket(addrResult->ai_family, addrResult->ai_socktype, + addrResult->ai_protocol); + if(listenerTcpSocket == INVALID_SOCKET) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "TcWinTcpServer::TcWinTcpServer: Socket creation failed!" << std::endl; - handleError(ErrorSources::SOCKET_CALL); #endif + freeaddrinfo(addrResult); + handleError(ErrorSources::SOCKET_CALL); + return HasReturnvaluesIF::RETURN_FAILED; } - int retval = setsockopt(serverTcpSocket, SOL_SOCKET, SO_REUSEADDR | SO_BROADCAST, - reinterpret_cast(&tcpSockOpt), sizeof(tcpSockOpt)); - if(retval != 0) { - sif::warning << "TcWinTcpServer::TcWinTcpServer: Setting socket options failed!" << - std::endl; - handleError(ErrorSources::SETSOCKOPT_CALL); - } - tcpAddress.sin_family = AF_INET; - tcpAddress.sin_addr.s_addr = htonl(INADDR_ANY); - tcpAddress.sin_port = htons(tcpPort); +// retval = setsockopt(listenerTcpSocket, SOL_SOCKET, SO_REUSEADDR | SO_BROADCAST, +// reinterpret_cast(&tcpSockOpt), sizeof(tcpSockOpt)); +// if(retval != 0) { +// sif::warning << "TcWinTcpServer::TcWinTcpServer: Setting socket options failed!" << +// std::endl; +// handleError(ErrorSources::SETSOCKOPT_CALL); +// return HasReturnvaluesIF::RETURN_FAILED; +// } +// tcpAddress.sin_family = AF_INET; +// tcpAddress.sin_addr.s_addr = htonl(INADDR_ANY); - retval = bind(serverTcpSocket, reinterpret_cast(&tcpAddress), + retval = bind(listenerTcpSocket, reinterpret_cast(&tcpAddress), tcpAddrLen); - if(retval != 0) { + if(retval == SOCKET_ERROR) { sif::warning << "TcWinTcpServer::TcWinTcpServer: Binding socket failed!" << std::endl; + freeaddrinfo(addrResult); handleError(ErrorSources::BIND_CALL); } + freeaddrinfo(addrResult); + return HasReturnvaluesIF::RETURN_OK; } + TcWinTcpServer::~TcWinTcpServer() { - closesocket(serverTcpSocket); + closesocket(listenerTcpSocket); WSACleanup(); } -ReturnValue_t TcWinTcpServer::initialize() { - return HasReturnvaluesIF::RETURN_OK; -} - ReturnValue_t TcWinTcpServer::performOperation(uint8_t opCode) { /* If a connection is accepted, the corresponding scoket will be assigned to the new socket */ - SOCKET connectorSocket; - sockaddr_in connectorSockAddr; + SOCKET clientSocket; + sockaddr_in clientSockAddr; int connectorSockAddrLen = 0; + int retval = 0; /* Listen for connection requests permanently for lifetime of program */ while(true) { - int retval = listen(serverTcpSocket, backlog); - if(retval != 0) { + retval = listen(listenerTcpSocket, currentBacklog); + if(retval == SOCKET_ERROR) { handleError(ErrorSources::LISTEN_CALL); + continue; } - connectorSocket = accept(serverTcpSocket, reinterpret_cast(&connectorSockAddr), + clientSocket = accept(listenerTcpSocket, reinterpret_cast(&clientSockAddr), &connectorSockAddrLen); - if(connectorSocket) {}; + if(clientSocket == INVALID_SOCKET) { + handleError(ErrorSources::ACCEPT_CALL); + continue; + }; + + retval = recv(clientSocket, reinterpret_cast(receptionBuffer.data()), + receptionBuffer.size(), 0); +#if FSFW_TCP_SERVER_WIRETAPPING_ENABLED == 1 +#endif } return HasReturnvaluesIF::RETURN_OK; @@ -102,6 +136,9 @@ void TcWinTcpServer::handleError(ErrorSources errorSrc) { else if(errorSrc == ErrorSources::ACCEPT_CALL) { errorSrcString = "accept call"; } + else if(errorSrc == ErrorSources::GETADDRINFO_CALL) { + errorSrcString = "getaddrinfo call"; + } switch(errCode) { case(WSANOTINITIALISED): { diff --git a/osal/windows/TcWinTcpServer.h b/osal/windows/TcWinTcpServer.h index 45d92ff8..7b679106 100644 --- a/osal/windows/TcWinTcpServer.h +++ b/osal/windows/TcWinTcpServer.h @@ -4,18 +4,22 @@ #include "../../objectmanager/SystemObject.h" #include "../../tasks/ExecutableObjectIF.h" +#include #include +//! Debugging preprocessor define. +#define FSFW_TCP_SERVER_WIRETAPPING_ENABLED 0 + class TcWinTcpServer: public SystemObject, public ExecutableObjectIF { public: /* The ports chosen here should not be used by any other process. */ - static constexpr uint16_t DEFAULT_TCP_SERVER_PORT = 7301; - static constexpr uint16_t DEFAULT_TCP_CLIENT_PORT = 7302; + static const std::string DEFAULT_TCP_SERVER_PORT; + static const std::string DEFAULT_TCP_CLIENT_PORT; TcWinTcpServer(object_id_t objectId, object_id_t tmtcUnixUdpBridge, - uint16_t customTcpServerPort = 0xffff); + std::string customTcpServerPort = ""); virtual~ TcWinTcpServer(); ReturnValue_t initialize() override; @@ -23,15 +27,17 @@ public: private: - SOCKET serverTcpSocket = 0; + std::string tcpPort; + SOCKET listenerTcpSocket = 0; struct sockaddr_in tcpAddress; int tcpAddrLen = sizeof(tcpAddress); - int backlog = 3; + int currentBacklog = 3; std::vector receptionBuffer; int tcpSockOpt = 0; enum class ErrorSources { + GETADDRINFO_CALL, SOCKET_CALL, SETSOCKOPT_CALL, BIND_CALL, diff --git a/osal/windows/TcWinUdpPollingTask.cpp b/osal/windows/TcWinUdpPollingTask.cpp index 32c21700..06e75bd5 100644 --- a/osal/windows/TcWinUdpPollingTask.cpp +++ b/osal/windows/TcWinUdpPollingTask.cpp @@ -3,11 +3,6 @@ #include "../../serviceinterface/ServiceInterfaceStream.h" #include -#include -#if defined(_MSC_VER) -#include -typedef SSIZE_T ssize_t; -#endif TcWinUdpPollingTask::TcWinUdpPollingTask(object_id_t objectId, object_id_t tmtcUnixUdpBridge, size_t frameSize, @@ -41,7 +36,7 @@ ReturnValue_t TcWinUdpPollingTask::performOperation(uint8_t opCode) { //! Sender Address is cached here. struct sockaddr_in senderAddress; int senderAddressSize = sizeof(senderAddress); - ssize_t bytesReceived = recvfrom(serverUdpSocket, + int bytesReceived = recvfrom(serverUdpSocket, reinterpret_cast(receptionBuffer.data()), frameSize, receptionFlags, reinterpret_cast(&senderAddress), &senderAddressSize); From d5a065eaa8c5e5e9472201c9914c8c932bcfdf36 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 9 Mar 2021 00:42:50 +0100 Subject: [PATCH 06/37] continued tcp server --- osal/windows/TcWinTcpServer.cpp | 15 ++++++++++++++- osal/windows/TcWinTcpServer.h | 5 +++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/osal/windows/TcWinTcpServer.cpp b/osal/windows/TcWinTcpServer.cpp index 1cf7ed1d..f85147f0 100644 --- a/osal/windows/TcWinTcpServer.cpp +++ b/osal/windows/TcWinTcpServer.cpp @@ -1,5 +1,6 @@ #include "TcWinTcpServer.h" #include "../../serviceinterface/ServiceInterface.h" + #include #include @@ -90,7 +91,7 @@ TcWinTcpServer::~TcWinTcpServer() { } ReturnValue_t TcWinTcpServer::performOperation(uint8_t opCode) { - /* If a connection is accepted, the corresponding scoket will be assigned to the new socket */ + /* If a connection is accepted, the corresponding socket will be assigned to the new socket */ SOCKET clientSocket; sockaddr_in clientSockAddr; int connectorSockAddrLen = 0; @@ -113,9 +114,21 @@ ReturnValue_t TcWinTcpServer::performOperation(uint8_t opCode) { retval = recv(clientSocket, reinterpret_cast(receptionBuffer.data()), receptionBuffer.size(), 0); + if(retval > 0) { #if FSFW_TCP_SERVER_WIRETAPPING_ENABLED == 1 + sif::info << "TcWinTcpServer::performOperation: Received " << retval << " bytes." + std::endl; #endif + } + else if(retval == 0) { + } + else { + + } + + /* Done, shut down connection */ + retval = shutdown(clientSocket, SD_SEND); } return HasReturnvaluesIF::RETURN_OK; } diff --git a/osal/windows/TcWinTcpServer.h b/osal/windows/TcWinTcpServer.h index 7b679106..f8aebc53 100644 --- a/osal/windows/TcWinTcpServer.h +++ b/osal/windows/TcWinTcpServer.h @@ -10,6 +10,11 @@ //! Debugging preprocessor define. #define FSFW_TCP_SERVER_WIRETAPPING_ENABLED 0 +/** + * @brief Windows TCP server used to receive telecommands on a Windows Host + * @details + * Based on: https://docs.microsoft.com/en-us/windows/win32/winsock/complete-server-code + */ class TcWinTcpServer: public SystemObject, public ExecutableObjectIF { From 82d7b7ed6f62d0f3ca64b7bdc89781a6c242fd3c Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 9 Mar 2021 21:21:01 +0100 Subject: [PATCH 07/37] renamed mutex helper --- ipc/MutexGuard.h | 2 -- osal/linux/TmTcUnixUdpBridge.cpp | 6 +++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/ipc/MutexGuard.h b/ipc/MutexGuard.h index 7eee47b5..4ad23012 100644 --- a/ipc/MutexGuard.h +++ b/ipc/MutexGuard.h @@ -40,8 +40,6 @@ public: #endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */ } #else - /* To avoid unused variable warning */ - static_cast(status); #endif /* FSFW_VERBOSE_LEVEL >= 1 */ } diff --git a/osal/linux/TmTcUnixUdpBridge.cpp b/osal/linux/TmTcUnixUdpBridge.cpp index 0af1fc68..7f110114 100644 --- a/osal/linux/TmTcUnixUdpBridge.cpp +++ b/osal/linux/TmTcUnixUdpBridge.cpp @@ -1,6 +1,6 @@ #include "TmTcUnixUdpBridge.h" #include "../../serviceinterface/ServiceInterface.h" -#include "../../ipc/MutexHelper.h" +#include "../../ipc/MutexGuard.h" #include #include @@ -69,7 +69,7 @@ TmTcUnixUdpBridge::~TmTcUnixUdpBridge() { ReturnValue_t TmTcUnixUdpBridge::sendTm(const uint8_t *data, size_t dataLen) { int flags = 0; - MutexHelper lock(mutex, MutexIF::TimeoutType::WAITING, 10); + MutexGuard lock(mutex, MutexIF::TimeoutType::WAITING, 10); if(ipAddrAnySet){ clientAddress.sin_addr.s_addr = htons(INADDR_ANY); @@ -100,7 +100,7 @@ ReturnValue_t TmTcUnixUdpBridge::sendTm(const uint8_t *data, size_t dataLen) { } void TmTcUnixUdpBridge::checkAndSetClientAddress(sockaddr_in& newAddress) { - MutexHelper lock(mutex, MutexIF::TimeoutType::WAITING, 10); + MutexGuard lock(mutex, MutexIF::TimeoutType::WAITING, 10); // char ipAddress [15]; #if FSFW_CPP_OSTREAM_ENABLED == 1 From a7bf9a6734d29f59a74024934886969c6572e9c6 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 9 Mar 2021 21:25:22 +0100 Subject: [PATCH 08/37] important replacements --- ipc/MutexGuard.h | 18 +++++++++--------- osal/host/MessageQueue.cpp | 6 +++--- osal/host/QueueMapManager.cpp | 4 ++-- osal/rtems/Clock.cpp | 6 +++--- osal/windows/TmTcWinUdpBridge.cpp | 2 +- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/ipc/MutexGuard.h b/ipc/MutexGuard.h index 4ad23012..9ee68c81 100644 --- a/ipc/MutexGuard.h +++ b/ipc/MutexGuard.h @@ -1,5 +1,5 @@ -#ifndef FRAMEWORK_IPC_MUTEXHELPER_H_ -#define FRAMEWORK_IPC_MUTEXHELPER_H_ +#ifndef FRAMEWORK_IPC_MUTEXGUARD_H_ +#define FRAMEWORK_IPC_MUTEXGUARD_H_ #include "MutexFactory.h" #include "../serviceinterface/ServiceInterface.h" @@ -12,9 +12,9 @@ public: if(mutex == nullptr) { #if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "MutexHelper: Passed mutex is invalid!" << std::endl; + sif::error << "MutexGuard: Passed mutex is invalid!" << std::endl; #else - sif::printError("MutexHelper: Passed mutex is invalid!\n"); + sif::printError("MutexGuard: Passed mutex is invalid!\n"); #endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */ #endif /* FSFW_VERBOSE_LEVEL >= 1 */ return; @@ -24,19 +24,19 @@ public: #if FSFW_VERBOSE_LEVEL >= 1 if(result == MutexIF::MUTEX_TIMEOUT) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "MutexHelper: Lock of mutex failed with timeout of " + sif::error << "MutexGuard: Lock of mutex failed with timeout of " << timeoutMs << " milliseconds!" << std::endl; #else - sif::printError("MutexHelper: Lock of mutex failed with timeout of %lu milliseconds\n", + sif::printError("MutexGuard: Lock of mutex failed with timeout of %lu milliseconds\n", timeoutMs); #endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */ } else if(result != HasReturnvaluesIF::RETURN_OK) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "MutexHelper: Lock of Mutex failed with code " << result << std::endl; + sif::error << "MutexGuard: Lock of Mutex failed with code " << result << std::endl; #else - sif::printError("MutexHelper: Lock of Mutex failed with code %d\n", result); + sif::printError("MutexGuard: Lock of Mutex failed with code %d\n", result); #endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */ } #else @@ -57,4 +57,4 @@ private: ReturnValue_t result = HasReturnvaluesIF::RETURN_FAILED; }; -#endif /* FRAMEWORK_IPC_MUTEXHELPER_H_ */ +#endif /* FRAMEWORK_IPC_MUTEXGUARD_H_ */ diff --git a/osal/host/MessageQueue.cpp b/osal/host/MessageQueue.cpp index dfc045e8..fe7b8c08 100644 --- a/osal/host/MessageQueue.cpp +++ b/osal/host/MessageQueue.cpp @@ -3,7 +3,7 @@ #include "../../serviceinterface/ServiceInterfaceStream.h" #include "../../ipc/MutexFactory.h" -#include "../../ipc/MutexHelper.h" +#include "../../ipc/MutexGuard.h" MessageQueue::MessageQueue(size_t messageDepth, size_t maxMessageSize): messageSize(maxMessageSize), messageDepth(messageDepth) { @@ -65,7 +65,7 @@ ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message) { } // not sure this will work.. //*message = std::move(messageQueue.front()); - MutexHelper mutexLock(queueLock, MutexIF::TimeoutType::WAITING, 20); + MutexGuard mutexLock(queueLock, MutexIF::TimeoutType::WAITING, 20); MessageQueueMessage* currentMessage = &messageQueue.front(); std::copy(currentMessage->getBuffer(), currentMessage->getBuffer() + messageSize, message->getBuffer()); @@ -130,7 +130,7 @@ ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, return HasReturnvaluesIF::RETURN_FAILED; } if(targetQueue->messageQueue.size() < targetQueue->messageDepth) { - MutexHelper mutexLock(targetQueue->queueLock, + MutexGuard mutexLock(targetQueue->queueLock, MutexIF::TimeoutType::WAITING, 20); // not ideal, works for now though. MessageQueueMessage* mqmMessage = diff --git a/osal/host/QueueMapManager.cpp b/osal/host/QueueMapManager.cpp index 2a54f813..b50d62dc 100644 --- a/osal/host/QueueMapManager.cpp +++ b/osal/host/QueueMapManager.cpp @@ -2,7 +2,7 @@ #include "../../serviceinterface/ServiceInterface.h" #include "../../ipc/MutexFactory.h" -#include "../../ipc/MutexHelper.h" +#include "../../ipc/MutexGuard.h" QueueMapManager* QueueMapManager::mqManagerInstance = nullptr; @@ -43,7 +43,7 @@ ReturnValue_t QueueMapManager::addMessageQueue( MessageQueueIF* QueueMapManager::getMessageQueue( MessageQueueId_t messageQueueId) const { - MutexHelper(mapLock, MutexIF::TimeoutType::WAITING, 50); + MutexGuard(mapLock, MutexIF::TimeoutType::WAITING, 50); auto queueIter = queueMap.find(messageQueueId); if(queueIter != queueMap.end()) { return queueIter->second; diff --git a/osal/rtems/Clock.cpp b/osal/rtems/Clock.cpp index aef71fe1..b80786f7 100644 --- a/osal/rtems/Clock.cpp +++ b/osal/rtems/Clock.cpp @@ -1,7 +1,7 @@ #include "RtemsBasic.h" #include "../../timemanager/Clock.h" -#include "../../ipc/MutexHelper.h" +#include "../../ipc/MutexGuard.h" #include #include @@ -183,7 +183,7 @@ ReturnValue_t Clock::setLeapSeconds(const uint16_t leapSeconds_) { if(checkOrCreateClockMutex()!=HasReturnvaluesIF::RETURN_OK){ return HasReturnvaluesIF::RETURN_FAILED; } - MutexHelper helper(timeMutex); + MutexGuard helper(timeMutex); leapSeconds = leapSeconds_; @@ -196,7 +196,7 @@ ReturnValue_t Clock::getLeapSeconds(uint16_t* leapSeconds_) { if(timeMutex==nullptr){ return HasReturnvaluesIF::RETURN_FAILED; } - MutexHelper helper(timeMutex); + MutexGuard helper(timeMutex); *leapSeconds_ = leapSeconds; diff --git a/osal/windows/TmTcWinUdpBridge.cpp b/osal/windows/TmTcWinUdpBridge.cpp index 36bb9ad3..d0de33d3 100644 --- a/osal/windows/TmTcWinUdpBridge.cpp +++ b/osal/windows/TmTcWinUdpBridge.cpp @@ -1,6 +1,6 @@ #include "TmTcWinUdpBridge.h" -#include +#include #if defined(_MSC_VER) #include From 61affafecdd4777b84cd46c711e97aa696e951dc Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 9 Mar 2021 21:58:29 +0100 Subject: [PATCH 09/37] and now some test broke.. --- osal/linux/TaskFactory.cpp | 1 + unittest/tests/datapoollocal/DataSetTest.cpp | 6 +++--- unittest/tests/datapoollocal/LocalPoolManagerTest.cpp | 4 ++-- unittest/tests/datapoollocal/LocalPoolOwnerBase.h | 5 +++-- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/osal/linux/TaskFactory.cpp b/osal/linux/TaskFactory.cpp index 93564647..80bf47b7 100644 --- a/osal/linux/TaskFactory.cpp +++ b/osal/linux/TaskFactory.cpp @@ -2,6 +2,7 @@ #include "PeriodicPosixTask.h" #include "../../tasks/TaskFactory.h" +#include "../../serviceinterface/ServiceInterface.h" #include "../../returnvalues/HasReturnvaluesIF.h" //TODO: Different variant than the lazy loading in QueueFactory. What's better and why? diff --git a/unittest/tests/datapoollocal/DataSetTest.cpp b/unittest/tests/datapoollocal/DataSetTest.cpp index 56134595..920bbda2 100644 --- a/unittest/tests/datapoollocal/DataSetTest.cpp +++ b/unittest/tests/datapoollocal/DataSetTest.cpp @@ -5,7 +5,7 @@ #include #include -#include +#include #include #include @@ -54,7 +54,7 @@ TEST_CASE("LocalDataSet" , "[LocDataSetTest]") { { /* Test read operation. Values should be all zeros */ - PoolReadHelper readHelper(&localSet); + PoolReadGuard readHelper(&localSet); REQUIRE(readHelper.getReadResult() == retval::CATCH_OK); CHECK(not localSet.isValid()); CHECK(localSet.localPoolVarUint8.value == 0); @@ -82,7 +82,7 @@ TEST_CASE("LocalDataSet" , "[LocDataSetTest]") { { /* Now we read again and check whether our zeroed values were overwritten with the values in the pool */ - PoolReadHelper readHelper(&localSet); + PoolReadGuard readHelper(&localSet); REQUIRE(readHelper.getReadResult() == retval::CATCH_OK); CHECK(localSet.isValid()); CHECK(localSet.localPoolVarUint8.value == 232); diff --git a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp index a10b4499..cd3be942 100644 --- a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp +++ b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp @@ -3,7 +3,7 @@ #include #include -#include +#include #include #include #include @@ -75,7 +75,7 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { SECTION("SnapshotUpdateTests") { /* Set the variables in the set to certain values. These are checked later. */ { - PoolReadHelper readHelper(&poolOwner->dataset); + PoolReadGuard readHelper(&poolOwner->dataset); REQUIRE(readHelper.getReadResult() == retval::CATCH_OK); poolOwner->dataset.localPoolVarUint8.value = 5; poolOwner->dataset.localPoolVarFloat.value = -12.242; diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h index 5c277850..8e6b07b0 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h @@ -1,16 +1,17 @@ #ifndef FSFW_UNITTEST_TESTS_DATAPOOLLOCAL_LOCALPOOLOWNERBASE_H_ #define FSFW_UNITTEST_TESTS_DATAPOOLLOCAL_LOCALPOOLOWNERBASE_H_ +#include + #include #include #include #include #include #include -#include #include #include -#include "../../../datapool/PoolReadHelper.h" +#include namespace lpool { static constexpr lp_id_t uint8VarId = 0; From e5b3b6d75eaaa424f21aaf0120768ba0f94b18d3 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 9 Mar 2021 22:21:27 +0100 Subject: [PATCH 10/37] fixed unit test --- unittest/tests/action/TestActionHelper.cpp | 2 +- unittest/tests/datapoollocal/LocalPoolVectorTest.cpp | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/unittest/tests/action/TestActionHelper.cpp b/unittest/tests/action/TestActionHelper.cpp index a7adfc82..d8bd58c9 100644 --- a/unittest/tests/action/TestActionHelper.cpp +++ b/unittest/tests/action/TestActionHelper.cpp @@ -70,7 +70,7 @@ TEST_CASE( "Action Helper" , "[ActionHelper]") { SECTION("Handle finish"){ CHECK(not testMqMock.wasMessageSent()); ReturnValue_t status = 0x9876; - actionHelper.finish(true, testMqMock.getId(), testActionId, status); + actionHelper.finish(false, testMqMock.getId(), testActionId, status); CHECK(testMqMock.wasMessageSent()); CommandMessage testMessage; REQUIRE(testMqMock.receiveMessage(&testMessage) == static_cast(HasReturnvaluesIF::RETURN_OK)); diff --git a/unittest/tests/datapoollocal/LocalPoolVectorTest.cpp b/unittest/tests/datapoollocal/LocalPoolVectorTest.cpp index 2bc47568..db76fc00 100644 --- a/unittest/tests/datapoollocal/LocalPoolVectorTest.cpp +++ b/unittest/tests/datapoollocal/LocalPoolVectorTest.cpp @@ -115,6 +115,7 @@ TEST_CASE("LocalPoolVector" , "[LocPoolVecTest]") { REQUIRE(readOnlyVec.commit() == static_cast(PoolVariableIF::INVALID_READ_WRITE_MODE)); } + poolOwner->reset(); } From bb5b7bed40eaf86293ebc9f631dc39ee2b844469 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 9 Mar 2021 23:18:53 +0100 Subject: [PATCH 11/37] made getter public --- datapoollocal/LocalPoolDataSetBase.h | 3 ++- unittest/tests/datapoollocal/DataSetTest.cpp | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/datapoollocal/LocalPoolDataSetBase.h b/datapoollocal/LocalPoolDataSetBase.h index 404509ae..b9946aaf 100644 --- a/datapoollocal/LocalPoolDataSetBase.h +++ b/datapoollocal/LocalPoolDataSetBase.h @@ -166,6 +166,8 @@ public: object_id_t getCreatorObjectId(); + bool getReportingEnabled() const; + protected: sid_t sid; //! This mutex is used if the data is created by one object only. @@ -180,7 +182,6 @@ protected: */ bool reportingEnabled = false; void setReportingEnabled(bool enabled); - bool getReportingEnabled() const; void initializePeriodicHelper(float collectionInterval, dur_millis_t minimumPeriodicInterval, diff --git a/unittest/tests/datapoollocal/DataSetTest.cpp b/unittest/tests/datapoollocal/DataSetTest.cpp index 920bbda2..101116cb 100644 --- a/unittest/tests/datapoollocal/DataSetTest.cpp +++ b/unittest/tests/datapoollocal/DataSetTest.cpp @@ -21,6 +21,7 @@ TEST_CASE("LocalDataSet" , "[LocDataSetTest]") { SECTION("BasicTest") { /* Test some basic functions */ + CHECK(localSet.getReportingEnabled() == false); CHECK(localSet.getLocalPoolIdsSerializedSize(false) == 3 * sizeof(lp_id_t)); CHECK(localSet.getLocalPoolIdsSerializedSize(true) == 3 * sizeof(lp_id_t) + sizeof(uint8_t)); From 676c9ffcf39c648be8c5cb7fc7f630b585088365 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 10 Mar 2021 16:32:24 +0100 Subject: [PATCH 12/37] added header amalagation --- datapoollocal/datapoollocal.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 datapoollocal/datapoollocal.h diff --git a/datapoollocal/datapoollocal.h b/datapoollocal/datapoollocal.h new file mode 100644 index 00000000..c5c47078 --- /dev/null +++ b/datapoollocal/datapoollocal.h @@ -0,0 +1,12 @@ +#ifndef FSFW_DATAPOOLLOCAL_DATAPOOLLOCAL_H_ +#define FSFW_DATAPOOLLOCAL_DATAPOOLLOCAL_H_ + +/* Collected related headers */ +#include "LocalPoolVariable.h" +#include "LocalPoolVector.h" +#include "StaticLocalDataSet.h" +#include "LocalDataSet.h" +#include "SharedLocalDataSet.h" + + +#endif /* FSFW_DATAPOOLLOCAL_DATAPOOLLOCAL_H_ */ From 9ba7fabdeab72b71f325a8cf57d11ea089455656 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 10 Mar 2021 16:38:54 +0100 Subject: [PATCH 13/37] removed commented out code --- unittest/tests/mocks/MessageQueueMockBase.h | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/unittest/tests/mocks/MessageQueueMockBase.h b/unittest/tests/mocks/MessageQueueMockBase.h index 7b810b41..31146d34 100644 --- a/unittest/tests/mocks/MessageQueueMockBase.h +++ b/unittest/tests/mocks/MessageQueueMockBase.h @@ -30,10 +30,7 @@ public: } virtual ReturnValue_t reply( MessageQueueMessageIF* message ) { - //messageSent = true; - //lastMessage = *(dynamic_cast(message)); return sendMessage(myQueueId, message); - return HasReturnvaluesIF::RETURN_OK; }; virtual ReturnValue_t receiveMessage(MessageQueueMessageIF* message, MessageQueueId_t *receivedFrom) { @@ -61,21 +58,13 @@ public: virtual ReturnValue_t sendMessageFrom( MessageQueueId_t sendTo, MessageQueueMessageIF* message, MessageQueueId_t sentFrom, bool ignoreFault = false ) { - //messageSent = true; - //lastMessage = *(dynamic_cast(message)); - //return HasReturnvaluesIF::RETURN_OK; return sendMessage(sendTo, message); } virtual ReturnValue_t sendToDefaultFrom( MessageQueueMessageIF* message, MessageQueueId_t sentFrom, bool ignoreFault = false ) { - //messageSent = true; - //lastMessage = *(dynamic_cast(message)); - //return HasReturnvaluesIF::RETURN_OK; return sendMessage(myQueueId, message); } virtual ReturnValue_t sendToDefault( MessageQueueMessageIF* message ) { - //messageSent = true; - //lastMessage = *(dynamic_cast(message)); return sendMessage(myQueueId, message); } virtual ReturnValue_t sendMessage( MessageQueueId_t sendTo, @@ -114,7 +103,6 @@ public: private: std::queue messagesSentQueue; - //MessageQueueMessage lastMessage; }; From 1e73302ba27f4a9beb1a5084cd3c495b48481c7c Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Mar 2021 17:45:22 +0100 Subject: [PATCH 14/37] corrected include --- unittest/tests/datapoollocal/LocalPoolOwnerBase.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h index 5c277850..03b0d190 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h @@ -10,7 +10,7 @@ #include #include #include -#include "../../../datapool/PoolReadHelper.h" +#include namespace lpool { static constexpr lp_id_t uint8VarId = 0; From 30910034f0af710d49f5f5d3746c4b41ef948137 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 10 Mar 2021 17:46:36 +0100 Subject: [PATCH 15/37] tiny improvementst --- unittest/tests/datapoollocal/DataSetTest.cpp | 2 +- unittest/tests/datapoollocal/LocalPoolVariableTest.cpp | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/unittest/tests/datapoollocal/DataSetTest.cpp b/unittest/tests/datapoollocal/DataSetTest.cpp index 101116cb..eb587e3f 100644 --- a/unittest/tests/datapoollocal/DataSetTest.cpp +++ b/unittest/tests/datapoollocal/DataSetTest.cpp @@ -10,7 +10,7 @@ #include -TEST_CASE("LocalDataSet" , "[LocDataSetTest]") { +TEST_CASE("DataSetTest" , "[DataSetTest]") { LocalPoolOwnerBase* poolOwner = objectManager-> get(objects::TEST_LOCAL_POOL_OWNER_BASE); REQUIRE(poolOwner != nullptr); diff --git a/unittest/tests/datapoollocal/LocalPoolVariableTest.cpp b/unittest/tests/datapoollocal/LocalPoolVariableTest.cpp index e5a5d364..980ffda1 100644 --- a/unittest/tests/datapoollocal/LocalPoolVariableTest.cpp +++ b/unittest/tests/datapoollocal/LocalPoolVariableTest.cpp @@ -10,8 +10,7 @@ TEST_CASE("LocalPoolVariable" , "[LocPoolVarTest]") { get(objects::TEST_LOCAL_POOL_OWNER_BASE); REQUIRE(poolOwner != nullptr); REQUIRE(poolOwner->initializeHkManager() == retval::CATCH_OK); - REQUIRE(poolOwner->initializeHkManagerAfterTaskCreation() - == retval::CATCH_OK); + REQUIRE(poolOwner->initializeHkManagerAfterTaskCreation() == retval::CATCH_OK); SECTION("Basic Tests") { /* very basic test. */ From 943495117b416d357fd7afa02fc02608b655f4af Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Mar 2021 17:50:10 +0100 Subject: [PATCH 16/37] moved preproc block --- datapoollocal/LocalDataPoolManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index 0c44ab7e..f572dc02 100644 --- a/datapoollocal/LocalDataPoolManager.cpp +++ b/datapoollocal/LocalDataPoolManager.cpp @@ -843,6 +843,7 @@ object_id_t LocalDataPoolManager::getCreatorObjectId() const { void LocalDataPoolManager::printWarningOrError(sif::OutputTypes outputType, const char* functionName, ReturnValue_t error, const char* errorPrint) { +#if FSFW_VERBOSE_LEVEL >= 1 if(errorPrint == nullptr) { if(error == DATASET_NOT_FOUND) { errorPrint = "Dataset not found"; @@ -873,7 +874,6 @@ void LocalDataPoolManager::printWarningOrError(sif::OutputTypes outputType, } if(outputType == sif::OutputTypes::OUT_WARNING) { -#if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "LocalDataPoolManager::" << functionName << ": Object ID 0x" << std::setw(8) << std::setfill('0') From 6501c16fd796e9fd2bb2d67dae60319603ca3b53 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Mar 2021 17:51:53 +0100 Subject: [PATCH 17/37] fixed preproc block --- datapoollocal/LocalDataPoolManager.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index f572dc02..5c702f1b 100644 --- a/datapoollocal/LocalDataPoolManager.cpp +++ b/datapoollocal/LocalDataPoolManager.cpp @@ -883,10 +883,8 @@ void LocalDataPoolManager::printWarningOrError(sif::OutputTypes outputType, sif::printWarning("LocalDataPoolManager::%s: Object ID 0x%08x | %s\n", functionName, owner->getObjectId(), errorPrint); #endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */ -#endif /* FSFW_VERBOSE_LEVEL >= 1 */ } else if(outputType == sif::OutputTypes::OUT_ERROR) { -#if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::error << "LocalDataPoolManager::" << functionName << ": Object ID 0x" << std::setw(8) << std::setfill('0') @@ -896,8 +894,8 @@ void LocalDataPoolManager::printWarningOrError(sif::OutputTypes outputType, sif::printError("LocalDataPoolManager::%s: Object ID 0x%08x | %s\n", functionName, owner->getObjectId(), errorPrint); #endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */ -#endif /* FSFW_VERBOSE_LEVEL >= 1 */ } +#endif /* #if FSFW_VERBOSE_LEVEL >= 1 */ } LocalDataPoolManager* LocalDataPoolManager::getPoolManagerHandle() { From b2e4438811f19c72aa34cbead972bd8d33282ffa Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Mar 2021 18:03:39 +0100 Subject: [PATCH 18/37] added some test, initial tick counter higher now --- housekeeping/PeriodicHousekeepingHelper.cpp | 3 +++ unittest/tests/datapoollocal/DataSetTest.cpp | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/housekeeping/PeriodicHousekeepingHelper.cpp b/housekeeping/PeriodicHousekeepingHelper.cpp index 365f0004..e7345677 100644 --- a/housekeeping/PeriodicHousekeepingHelper.cpp +++ b/housekeeping/PeriodicHousekeepingHelper.cpp @@ -16,6 +16,9 @@ void PeriodicHousekeepingHelper::initialize(float collectionInterval, nonDiagIntervalFactor; } collectionIntervalTicks = intervalSecondsToInterval(collectionInterval); + /* This will cause a checkOpNecessary call to be true immediately. I think it's okay + if a HK packet is generated immediately instead of waiting one generation cycle. */ + internalTickCounter = collectionIntervalTicks; } float PeriodicHousekeepingHelper::getCollectionIntervalInSeconds() { diff --git a/unittest/tests/datapoollocal/DataSetTest.cpp b/unittest/tests/datapoollocal/DataSetTest.cpp index eb587e3f..9b4509e9 100644 --- a/unittest/tests/datapoollocal/DataSetTest.cpp +++ b/unittest/tests/datapoollocal/DataSetTest.cpp @@ -80,6 +80,13 @@ TEST_CASE("DataSetTest" , "[DataSetTest]") { localSet.localPoolVarUint8 = 0; localSet.localPoolVarFloat = 0; + localSet.setAllVariablesReadOnly(); + CHECK(localSet.localPoolUint16Vec.getReadWriteMode() == pool_rwm_t::VAR_READ); + CHECK(localSet.localPoolVarUint8.getReadWriteMode() == pool_rwm_t::VAR_READ); + CHECK(localSet.localPoolVarFloat.getReadWriteMode() == pool_rwm_t::VAR_READ); + /* For code coverage */ + localSet.initializePeriodicHelper(0.0, 0.4, false); + { /* Now we read again and check whether our zeroed values were overwritten with the values in the pool */ From 3789663db7dac886393b2ad50437f63a86fba56e Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Mar 2021 18:26:07 +0100 Subject: [PATCH 19/37] added some tests --- unittest/tests/datapoollocal/DataSetTest.cpp | 2 -- .../tests/datapoollocal/LocalPoolManagerTest.cpp | 16 ++++++++++++++++ .../tests/datapoollocal/LocalPoolOwnerBase.h | 6 +++++- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/unittest/tests/datapoollocal/DataSetTest.cpp b/unittest/tests/datapoollocal/DataSetTest.cpp index 9b4509e9..4def46be 100644 --- a/unittest/tests/datapoollocal/DataSetTest.cpp +++ b/unittest/tests/datapoollocal/DataSetTest.cpp @@ -84,8 +84,6 @@ TEST_CASE("DataSetTest" , "[DataSetTest]") { CHECK(localSet.localPoolUint16Vec.getReadWriteMode() == pool_rwm_t::VAR_READ); CHECK(localSet.localPoolVarUint8.getReadWriteMode() == pool_rwm_t::VAR_READ); CHECK(localSet.localPoolVarFloat.getReadWriteMode() == pool_rwm_t::VAR_READ); - /* For code coverage */ - localSet.initializePeriodicHelper(0.0, 0.4, false); { /* Now we read again and check whether our zeroed values were overwritten with diff --git a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp index cd3be942..fd2c78d7 100644 --- a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp +++ b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp @@ -28,6 +28,10 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { SECTION("BasicTest") { + auto owner = poolOwner->poolManager.getOwner(); + REQUIRE(owner != nullptr); + CHECK(owner->getObjectId() == objects::TEST_LOCAL_POOL_OWNER_BASE); + /* Subscribe for message generation on update. */ REQUIRE(poolOwner->subscribeWrapperSetUpdate() == retval::CATCH_OK); /* Subscribe for an update message. */ @@ -190,6 +194,18 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { REQUIRE(mqMock->receiveMessage(&messageSent) == static_cast(MessageQueueIF::EMPTY)); } + SECTION("Periodic HK") { + /* Now we subcribe for a HK periodic generation. Even when it's difficult to simulate + the temporal behaviour correctly the HK manager should generate a HK packet + immediately and the periodic helper depends on HK op function calls anyway instead of + using the clock, so we could also just call performHkOperation multiple times */ + REQUIRE(poolOwner->subscribePeriodicHk() == retval::CATCH_OK); + REQUIRE(poolOwner->poolManager.performHkOperation() == retval::CATCH_OK); + /* Now HK packet should be sent as message. */ + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); + } + /* we need to reset the subscription list because the pool owner is a global object. */ CHECK(poolOwner->reset() == retval::CATCH_OK); diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h index 8e6b07b0..58ca445c 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h @@ -130,7 +130,7 @@ public: } uint32_t getPeriodicOperationFrequency() const override { - return 0; + return 0.2; } /** @@ -169,6 +169,10 @@ public: return dynamic_cast(messageQueue); } + ReturnValue_t subscribePeriodicHk() { + return poolManager.subscribeForPeriodicPacket(lpool::testSid, true, 0.2, false); + } + ReturnValue_t subscribeWrapperSetUpdate() { return poolManager.subscribeForSetUpdateMessage(lpool::testSetId, objects::NO_OBJECT, objects::HK_RECEIVER_MOCK, false); From 03936fc5c1929dfc152fecb5d06949e1b84a7564 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Mar 2021 19:15:05 +0100 Subject: [PATCH 20/37] snapshot test added, bugfix --- datapoollocal/LocalDataPoolManager.cpp | 21 +++++---- .../datapoollocal/LocalPoolManagerTest.cpp | 45 ++++++++++++++++++- .../tests/datapoollocal/LocalPoolOwnerBase.h | 5 +++ 3 files changed, 61 insertions(+), 10 deletions(-) diff --git a/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index 5c702f1b..a9668a32 100644 --- a/datapoollocal/LocalDataPoolManager.cpp +++ b/datapoollocal/LocalDataPoolManager.cpp @@ -132,13 +132,16 @@ ReturnValue_t LocalDataPoolManager::performHkOperation() { ReturnValue_t LocalDataPoolManager::handleHkUpdate(HkReceiver& receiver, ReturnValue_t& status) { if(receiver.dataType == DataType::LOCAL_POOL_VARIABLE) { - // Update packets shall only be generated from datasets. + /* Update packets shall only be generated from datasets. */ return HasReturnvaluesIF::RETURN_FAILED; } LocalPoolDataSetBase* dataSet = HasLocalDpIFManagerAttorney::getDataSetHandle(owner, receiver.dataId.sid); + if(dataSet == nullptr) { + return DATASET_NOT_FOUND; + } if(dataSet->hasChanged()) { - // prepare and send update notification + /* Prepare and send update notification */ ReturnValue_t result = generateHousekeepingPacket( receiver.dataId.sid, dataSet, true); if(result != HasReturnvaluesIF::RETURN_OK) { @@ -328,7 +331,7 @@ void LocalDataPoolManager::handleChangeResetLogic( toReset->setChanged(false); } /* All recipients have been notified, reset the changed flag */ - if(changeInfo.currentUpdateCounter <= 1) { + else if(changeInfo.currentUpdateCounter <= 1) { toReset->setChanged(false); changeInfo.currentUpdateCounter = 0; } @@ -398,7 +401,6 @@ ReturnValue_t LocalDataPoolManager::subscribeForUpdatePacket(sid_t sid, hkReceiver.destinationQueue = hkReceiverObject->getHkQueue(); LocalPoolDataSetBase* dataSet = HasLocalDpIFManagerAttorney::getDataSetHandle(owner, sid); - //LocalPoolDataSetBase* dataSet = owner->getDataSetHandle(sid); if(dataSet != nullptr) { LocalPoolDataSetAttorney::setReportingEnabled(*dataSet, true); LocalPoolDataSetAttorney::setDiagnostic(*dataSet, isDiagnostics); @@ -616,7 +618,7 @@ ReturnValue_t LocalDataPoolManager::generateHousekeepingPacket(sid_t sid, LocalPoolDataSetBase* dataSet, bool forDownlink, MessageQueueId_t destination) { if(dataSet == nullptr) { - // Configuration error. + /* Configuration error. */ printWarningOrError(sif::OutputTypes::OUT_WARNING, "generateHousekeepingPacket", DATASET_NOT_FOUND); @@ -632,7 +634,7 @@ ReturnValue_t LocalDataPoolManager::generateHousekeepingPacket(sid_t sid, return result; } - // and now we set a HK message and send it the HK packet destination. + /* Now we set a HK message and send it the HK packet destination. */ CommandMessage hkMessage; if(LocalPoolDataSetAttorney::isDiagnostics(*dataSet)) { HousekeepingMessage::setHkDiagnosticsReply(&hkMessage, sid, storeId); @@ -642,7 +644,7 @@ ReturnValue_t LocalDataPoolManager::generateHousekeepingPacket(sid_t sid, } if(hkQueue == nullptr) { - // error, no queue available to send packet with. + /* Error, no queue available to send packet with. */ printWarningOrError(sif::OutputTypes::OUT_WARNING, "generateHousekeepingPacket", QUEUE_OR_DESTINATION_INVALID); @@ -650,7 +652,7 @@ ReturnValue_t LocalDataPoolManager::generateHousekeepingPacket(sid_t sid, } if(destination == MessageQueueIF::NO_QUEUE) { if(hkDestinationId == MessageQueueIF::NO_QUEUE) { - // error, all destinations invalid + /* Error, all destinations invalid */ printWarningOrError(sif::OutputTypes::OUT_WARNING, "generateHousekeepingPacket", QUEUE_OR_DESTINATION_INVALID); @@ -831,6 +833,9 @@ ReturnValue_t LocalDataPoolManager::generateSetStructurePacket(sid_t sid, void LocalDataPoolManager::clearReceiversList() { /* Clear the vector completely and releases allocated memory. */ HkReceivers().swap(hkReceivers); + if(hkUpdateResetList != nullptr) { + HkUpdateResetList().swap(*hkUpdateResetList); + } } MutexIF* LocalDataPoolManager::getLocalPoolMutex() { diff --git a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp index fd2c78d7..685016ed 100644 --- a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp +++ b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp @@ -76,7 +76,7 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { } - SECTION("SnapshotUpdateTests") { + SECTION("SetSnapshotUpdateTest") { /* Set the variables in the set to certain values. These are checked later. */ { PoolReadGuard readHelper(&poolOwner->dataset); @@ -141,7 +141,31 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { CHECK(cdsShort.msDay_ll == Catch::Approx(timeCdsNow.msDay_ll).margin(1)); } - SECTION("AdvancedTests") { + SECTION("VariableSnapshotTest") { + /* Acquire subscription interface */ + ProvidesDataPoolSubscriptionIF* subscriptionIF = poolOwner->getSubscriptionInterface(); + REQUIRE(subscriptionIF != nullptr); + + /* Subscribe for variable snapshot */ + REQUIRE(poolOwner->subscribeWrapperVariableSnapshot(lpool::uint8VarId) == retval::CATCH_OK); + auto poolVar = dynamic_cast*>( + poolOwner->getPoolObjectHandle(lpool::uint8VarId)); + REQUIRE(poolVar != nullptr); + poolVar->setChanged(true); + REQUIRE(poolOwner->poolManager.performHkOperation() == retval::CATCH_OK); + + /* Check update snapshot was sent. */ + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); + + /* Should have been reset. */ + CHECK(poolVar->hasChanged() == false); + REQUIRE(mqMock->receiveMessage(&messageSent) == retval::CATCH_OK); + CHECK(messageSent.getCommand() == static_cast( + HousekeepingMessage::UPDATE_SNAPSHOT_VARIABLE)); + } + + SECTION("VariableUpdateTest") { /* Acquire subscription interface */ ProvidesDataPoolSubscriptionIF* subscriptionIF = poolOwner->getSubscriptionInterface(); REQUIRE(subscriptionIF != nullptr); @@ -153,6 +177,7 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { poolOwner->getPoolObjectHandle(lpool::uint8VarId)); REQUIRE(poolVar != nullptr); poolVar->setChanged(true); + REQUIRE(poolVar->hasChanged() == true); REQUIRE(poolOwner->poolManager.performHkOperation() == retval::CATCH_OK); /* Check update notification was sent. */ @@ -204,6 +229,22 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { /* Now HK packet should be sent as message. */ REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); CHECK(messagesSent == 1); + + LocalPoolDataSetBase* setHandle = poolOwner->getDataSetHandle(lpool::testSid); + REQUIRE(setHandle != nullptr); + CHECK(poolOwner->poolManager.generateHousekeepingPacket(lpool::testSid, + setHandle, false) == retval::CATCH_OK); + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); + + CHECK(setHandle->getReportingEnabled() == true); + CommandMessage hkCmd; + HousekeepingMessage::setToggleReportingCommand(&hkCmd, lpool::testSid, false, false); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); + CHECK(setHandle->getReportingEnabled() == false); + HousekeepingMessage::setToggleReportingCommand(&hkCmd, lpool::testSid, true, false); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); + CHECK(setHandle->getReportingEnabled() == true); } /* we need to reset the subscription list because the pool owner diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h index 58ca445c..f74c47b5 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h @@ -193,6 +193,11 @@ public: MessageQueueIF::NO_QUEUE, objects::HK_RECEIVER_MOCK, false); } + ReturnValue_t subscribeWrapperVariableSnapshot(lp_id_t localPoolId) { + return poolManager.subscribeForVariableUpdateMessage(localPoolId, + MessageQueueIF::NO_QUEUE, objects::HK_RECEIVER_MOCK, true); + } + ReturnValue_t reset() { resetSubscriptionList(); ReturnValue_t status = HasReturnvaluesIF::RETURN_OK; From 7ad8763b14f200768438d6118de85b7a7de38882 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Mar 2021 19:18:52 +0100 Subject: [PATCH 21/37] added more nullptr checks --- datapoollocal/LocalDataPoolManager.cpp | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index a9668a32..9f0eb779 100644 --- a/datapoollocal/LocalDataPoolManager.cpp +++ b/datapoollocal/LocalDataPoolManager.cpp @@ -731,6 +731,12 @@ void LocalDataPoolManager::performPeriodicHkGeneration(HkReceiver& receiver) { ReturnValue_t LocalDataPoolManager::togglePeriodicGeneration(sid_t sid, bool enable, bool isDiagnostics) { LocalPoolDataSetBase* dataSet = HasLocalDpIFManagerAttorney::getDataSetHandle(owner, sid); + if(dataSet == nullptr) { + printWarningOrError(sif::OutputTypes::OUT_WARNING, "togglePeriodicGeneration", + DATASET_NOT_FOUND); + return DATASET_NOT_FOUND; + } + if((LocalPoolDataSetAttorney::isDiagnostics(*dataSet) and not isDiagnostics) or (not LocalPoolDataSetAttorney::isDiagnostics(*dataSet) and isDiagnostics)) { return WRONG_HK_PACKET_TYPE; @@ -748,6 +754,12 @@ ReturnValue_t LocalDataPoolManager::togglePeriodicGeneration(sid_t sid, ReturnValue_t LocalDataPoolManager::changeCollectionInterval(sid_t sid, float newCollectionInterval, bool isDiagnostics) { LocalPoolDataSetBase* dataSet = HasLocalDpIFManagerAttorney::getDataSetHandle(owner, sid); + if(dataSet == nullptr) { + printWarningOrError(sif::OutputTypes::OUT_WARNING, "changeCollectionInterval", + DATASET_NOT_FOUND); + return DATASET_NOT_FOUND; + } + bool targetIsDiagnostics = LocalPoolDataSetAttorney::isDiagnostics(*dataSet); if((targetIsDiagnostics and not isDiagnostics) or (not targetIsDiagnostics and isDiagnostics)) { @@ -768,13 +780,11 @@ ReturnValue_t LocalDataPoolManager::changeCollectionInterval(sid_t sid, ReturnValue_t LocalDataPoolManager::generateSetStructurePacket(sid_t sid, bool isDiagnostics) { - // Get and check dataset first. - //LocalPoolDataSetBase* dataSet = owner->getDataSetHandle(sid); + /* Get and check dataset first. */ LocalPoolDataSetBase* dataSet = HasLocalDpIFManagerAttorney::getDataSetHandle(owner, sid); if(dataSet == nullptr) { printWarningOrError(sif::OutputTypes::OUT_WARNING, - "performPeriodicHkGeneration", - DATASET_NOT_FOUND); + "performPeriodicHkGeneration", DATASET_NOT_FOUND); return DATASET_NOT_FOUND; } @@ -833,6 +843,7 @@ ReturnValue_t LocalDataPoolManager::generateSetStructurePacket(sid_t sid, void LocalDataPoolManager::clearReceiversList() { /* Clear the vector completely and releases allocated memory. */ HkReceivers().swap(hkReceivers); + /* Also clear the reset helper if it exists */ if(hkUpdateResetList != nullptr) { HkUpdateResetList().swap(*hkUpdateResetList); } From 6c0972b2d5770d066f2b0666ad40a33caed364a8 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Mar 2021 21:17:08 +0100 Subject: [PATCH 22/37] several bugfixes --- datapoollocal/HasLocalDataPoolIF.h | 2 +- datapoollocal/LocalDataPoolManager.cpp | 2 +- datapoollocal/LocalPoolDataSetBase.cpp | 9 ++ datapoollocal/LocalPoolDataSetBase.h | 8 ++ housekeeping/HousekeepingMessage.cpp | 10 +- housekeeping/PeriodicHousekeepingHelper.cpp | 94 +++++++++++++------ housekeeping/PeriodicHousekeepingHelper.h | 8 +- .../datapoollocal/LocalPoolManagerTest.cpp | 7 ++ .../tests/datapoollocal/LocalPoolOwnerBase.h | 4 +- 9 files changed, 106 insertions(+), 38 deletions(-) diff --git a/datapoollocal/HasLocalDataPoolIF.h b/datapoollocal/HasLocalDataPoolIF.h index d52c72b6..ec25f082 100644 --- a/datapoollocal/HasLocalDataPoolIF.h +++ b/datapoollocal/HasLocalDataPoolIF.h @@ -65,7 +65,7 @@ public: * usually be the period the pool owner performs its periodic operation. * @return */ - virtual uint32_t getPeriodicOperationFrequency() const = 0; + virtual dur_millis_t getPeriodicOperationFrequency() const = 0; /** * @brief This function will be called by the manager if an update diff --git a/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index 9f0eb779..83e30e82 100644 --- a/datapoollocal/LocalDataPoolManager.cpp +++ b/datapoollocal/LocalDataPoolManager.cpp @@ -770,7 +770,7 @@ ReturnValue_t LocalDataPoolManager::changeCollectionInterval(sid_t sid, LocalPoolDataSetAttorney::getPeriodicHelper(*dataSet); if(periodicHelper == nullptr) { - // config error + /* Configuration error, set might not have a corresponding pool manager */ return PERIODIC_HELPER_INVALID; } diff --git a/datapoollocal/LocalPoolDataSetBase.cpp b/datapoollocal/LocalPoolDataSetBase.cpp index 2d70712b..3abbf0dd 100644 --- a/datapoollocal/LocalPoolDataSetBase.cpp +++ b/datapoollocal/LocalPoolDataSetBase.cpp @@ -308,3 +308,12 @@ void LocalPoolDataSetBase::setAllVariablesReadOnly() { registeredVariables[idx]->setReadWriteMode(pool_rwm_t::VAR_READ); } } + +float LocalPoolDataSetBase::getCollectionInterval() const { + if(periodicHelper != nullptr) { + return periodicHelper->getCollectionIntervalInSeconds(); + } + else { + return 0.0; + } +} diff --git a/datapoollocal/LocalPoolDataSetBase.h b/datapoollocal/LocalPoolDataSetBase.h index b9946aaf..39ea9f1a 100644 --- a/datapoollocal/LocalPoolDataSetBase.h +++ b/datapoollocal/LocalPoolDataSetBase.h @@ -168,6 +168,14 @@ public: bool getReportingEnabled() const; + /** + * Returns the current periodic HK generation interval this set + * belongs to a HK manager and the interval is not 0. Otherwise, + * returns 0.0 + * @return + */ + float getCollectionInterval() const; + protected: sid_t sid; //! This mutex is used if the data is created by one object only. diff --git a/housekeeping/HousekeepingMessage.cpp b/housekeeping/HousekeepingMessage.cpp index d9803ef6..58f8551d 100644 --- a/housekeeping/HousekeepingMessage.cpp +++ b/housekeeping/HousekeepingMessage.cpp @@ -84,15 +84,21 @@ void HousekeepingMessage::setCollectionIntervalModificationCommand( else { command->setCommand(MODIFY_PARAMETER_REPORT_COLLECTION_INTERVAL); } - command->setParameter3(collectionInterval); + + /* Raw storage of the float in the message. Do not use setParameter3, does + implicit conversion to integer type! */ + std::memcpy(command->getData() + 2 * sizeof(uint32_t), &collectionInterval, + sizeof(collectionInterval)); setSid(command, sid); } sid_t HousekeepingMessage::getCollectionIntervalModificationCommand( const CommandMessage* command, float* newCollectionInterval) { + if(newCollectionInterval != nullptr) { - *newCollectionInterval = command->getParameter3(); + std::memcpy(newCollectionInterval, command->getData() + 2 * sizeof(uint32_t), + sizeof(*newCollectionInterval)); } return getSid(command); diff --git a/housekeeping/PeriodicHousekeepingHelper.cpp b/housekeeping/PeriodicHousekeepingHelper.cpp index e7345677..b0dd8522 100644 --- a/housekeeping/PeriodicHousekeepingHelper.cpp +++ b/housekeeping/PeriodicHousekeepingHelper.cpp @@ -4,49 +4,85 @@ #include PeriodicHousekeepingHelper::PeriodicHousekeepingHelper( - LocalPoolDataSetBase* owner): owner(owner) {} + LocalPoolDataSetBase* owner): owner(owner) {} void PeriodicHousekeepingHelper::initialize(float collectionInterval, - dur_millis_t minimumPeriodicInterval, bool isDiagnostics, - uint8_t nonDiagIntervalFactor) { - this->minimumPeriodicInterval = minimumPeriodicInterval; - if(not isDiagnostics) { - this->minimumPeriodicInterval = this->minimumPeriodicInterval * - nonDiagIntervalFactor; - } - collectionIntervalTicks = intervalSecondsToInterval(collectionInterval); - /* This will cause a checkOpNecessary call to be true immediately. I think it's okay + dur_millis_t minimumPeriodicInterval, bool isDiagnostics, + uint8_t nonDiagIntervalFactor) { + this->isDiagnostics = isDiagnostics; + this->minimumPeriodicInterval = minimumPeriodicInterval; + this->nonDiagIntervalFactor = nonDiagIntervalFactor; + collectionIntervalTicks = intervalSecondsToIntervalTicks(collectionInterval); + /* This will cause a checkOpNecessary call to be true immediately. I think it's okay if a HK packet is generated immediately instead of waiting one generation cycle. */ - internalTickCounter = collectionIntervalTicks; + internalTickCounter = collectionIntervalTicks; } -float PeriodicHousekeepingHelper::getCollectionIntervalInSeconds() { - return intervalToIntervalSeconds(collectionIntervalTicks); +float PeriodicHousekeepingHelper::getCollectionIntervalInSeconds() const { + return intervalTicksToSeconds(collectionIntervalTicks); } bool PeriodicHousekeepingHelper::checkOpNecessary() { - if(internalTickCounter >= collectionIntervalTicks) { - internalTickCounter = 1; - return true; - } - internalTickCounter++; - return false; + if(internalTickCounter >= collectionIntervalTicks) { + internalTickCounter = 1; + return true; + } + internalTickCounter++; + return false; } -uint32_t PeriodicHousekeepingHelper::intervalSecondsToInterval( - float collectionIntervalSeconds) { - return std::ceil(collectionIntervalSeconds * 1000 - / minimumPeriodicInterval); +uint32_t PeriodicHousekeepingHelper::intervalSecondsToIntervalTicks( + float collectionIntervalSeconds) { + /* Avoid division by zero */ + if(minimumPeriodicInterval == 0) { + if(isDiagnostics) { + /* Perform operation each cycle */ + return 1; + } + else { + return nonDiagIntervalFactor; + } + } + else { + dur_millis_t intervalInMs = collectionIntervalSeconds * 1000; + uint32_t divisor = minimumPeriodicInterval; + if(not isDiagnostics) { + /* We need to multiply the divisor because non-diagnostics only + allow a multiple of the minimum periodic interval */ + divisor *= nonDiagIntervalFactor; + } + uint32_t ticks = std::ceil(static_cast(intervalInMs) / divisor); + if(not isDiagnostics) { + /* Now we need to multiply the calculated ticks with the factor as as well + because the minimum tick count to generate a non-diagnostic is + the factor. + + Example calculation for non-diagnostic with + 0.4 second interval and 0.2 second task interval. + Resultant tick count of 5 is equal to operation each second. + + Examle calculation for non-diagnostic with 2.0 second interval and 0.2 second + task interval. + Resultant tick count of 10 is equal to operatin every 2 seconds. + + Example calculation for diagnostic with 0.4 second interval and 0.3 + second task interval. Resulting tick count of 2 is equal to operation + every 0.6 seconds. */ + ticks *= nonDiagIntervalFactor; + } + return ticks; + } } -float PeriodicHousekeepingHelper::intervalToIntervalSeconds( - uint32_t collectionInterval) { - return static_cast(collectionInterval * - minimumPeriodicInterval); +float PeriodicHousekeepingHelper::intervalTicksToSeconds( + uint32_t collectionInterval) const { + /* Number of ticks times the minimum interval is in milliseconds, so we divide by 1000 to get + the value in seconds */ + return static_cast(collectionInterval * minimumPeriodicInterval / 1000.0); } void PeriodicHousekeepingHelper::changeCollectionInterval( - float newIntervalSeconds) { - collectionIntervalTicks = intervalSecondsToInterval(newIntervalSeconds); + float newIntervalSeconds) { + collectionIntervalTicks = intervalSecondsToIntervalTicks(newIntervalSeconds); } diff --git a/housekeeping/PeriodicHousekeepingHelper.h b/housekeeping/PeriodicHousekeepingHelper.h index d96eae1d..3e8d030e 100644 --- a/housekeeping/PeriodicHousekeepingHelper.h +++ b/housekeeping/PeriodicHousekeepingHelper.h @@ -15,13 +15,15 @@ public: uint8_t nonDiagIntervalFactor); void changeCollectionInterval(float newInterval); - float getCollectionIntervalInSeconds(); + float getCollectionIntervalInSeconds() const; bool checkOpNecessary(); private: LocalPoolDataSetBase* owner = nullptr; + bool isDiagnostics = true; + uint8_t nonDiagIntervalFactor = 0; - uint32_t intervalSecondsToInterval(float collectionIntervalSeconds); - float intervalToIntervalSeconds(uint32_t collectionInterval); + uint32_t intervalSecondsToIntervalTicks(float collectionIntervalSeconds); + float intervalTicksToSeconds(uint32_t collectionInterval) const; dur_millis_t minimumPeriodicInterval = 0; uint32_t internalTickCounter = 1; diff --git a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp index 685016ed..1ab87151 100644 --- a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp +++ b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp @@ -245,6 +245,13 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { HousekeepingMessage::setToggleReportingCommand(&hkCmd, lpool::testSid, true, false); CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); CHECK(setHandle->getReportingEnabled() == true); + + HousekeepingMessage::setCollectionIntervalModificationCommand(&hkCmd, + lpool::testSid, 0.4, false); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); + /* For non-diagnostics and a specified minimum frequency of 0.2 seconds, the + resulting collection interval should be 1.0 second */ + CHECK(poolOwner->dataset.getCollectionInterval() == 1.0); } /* we need to reset the subscription list because the pool owner diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h index f74c47b5..20c565ff 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h @@ -129,8 +129,8 @@ public: return &poolManager; } - uint32_t getPeriodicOperationFrequency() const override { - return 0.2; + dur_millis_t getPeriodicOperationFrequency() const override { + return 200; } /** From c59fa578c7aa7aabaf5aa58782c4bf425a226cdf Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Mar 2021 21:18:47 +0100 Subject: [PATCH 23/37] format improvements --- housekeeping/PeriodicHousekeepingHelper.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/housekeeping/PeriodicHousekeepingHelper.cpp b/housekeeping/PeriodicHousekeepingHelper.cpp index b0dd8522..5152706f 100644 --- a/housekeeping/PeriodicHousekeepingHelper.cpp +++ b/housekeeping/PeriodicHousekeepingHelper.cpp @@ -55,8 +55,7 @@ uint32_t PeriodicHousekeepingHelper::intervalSecondsToIntervalTicks( uint32_t ticks = std::ceil(static_cast(intervalInMs) / divisor); if(not isDiagnostics) { /* Now we need to multiply the calculated ticks with the factor as as well - because the minimum tick count to generate a non-diagnostic is - the factor. + because the minimum tick count to generate a non-diagnostic is the factor itself. Example calculation for non-diagnostic with 0.4 second interval and 0.2 second task interval. From 4f89fc62abfc5587dd84ebde3e2d040f5cbd0ecf Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Mar 2021 21:38:13 +0100 Subject: [PATCH 24/37] continued tests --- .../datapoollocal/LocalPoolManagerTest.cpp | 30 +++++++++++++++++-- .../tests/datapoollocal/LocalPoolOwnerBase.h | 4 +-- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp index 1ab87151..4457e85b 100644 --- a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp +++ b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp @@ -219,14 +219,14 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { REQUIRE(mqMock->receiveMessage(&messageSent) == static_cast(MessageQueueIF::EMPTY)); } - SECTION("Periodic HK") { + SECTION("PeriodicHKAndMessaging") { /* Now we subcribe for a HK periodic generation. Even when it's difficult to simulate the temporal behaviour correctly the HK manager should generate a HK packet immediately and the periodic helper depends on HK op function calls anyway instead of using the clock, so we could also just call performHkOperation multiple times */ - REQUIRE(poolOwner->subscribePeriodicHk() == retval::CATCH_OK); + REQUIRE(poolOwner->subscribePeriodicHk(true) == retval::CATCH_OK); REQUIRE(poolOwner->poolManager.performHkOperation() == retval::CATCH_OK); - /* Now HK packet should be sent as message. */ + /* Now HK packet should be sent as message immediately. */ REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); CHECK(messagesSent == 1); @@ -242,9 +242,19 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { HousekeepingMessage::setToggleReportingCommand(&hkCmd, lpool::testSid, false, false); CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); CHECK(setHandle->getReportingEnabled() == false); + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); HousekeepingMessage::setToggleReportingCommand(&hkCmd, lpool::testSid, true, false); CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); CHECK(setHandle->getReportingEnabled() == true); + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); + + HousekeepingMessage::setToggleReportingCommand(&hkCmd, lpool::testSid, false, false); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); + CHECK(setHandle->getReportingEnabled() == false); + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); HousekeepingMessage::setCollectionIntervalModificationCommand(&hkCmd, lpool::testSid, 0.4, false); @@ -252,6 +262,20 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { /* For non-diagnostics and a specified minimum frequency of 0.2 seconds, the resulting collection interval should be 1.0 second */ CHECK(poolOwner->dataset.getCollectionInterval() == 1.0); + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); + + HousekeepingMessage::setStructureReportingCommand(&hkCmd, lpool::testSid, false); + REQUIRE(poolOwner->poolManager.performHkOperation() == retval::CATCH_OK); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); + /* Now HK packet should be sent as message. */ + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); + + HousekeepingMessage::setOneShotReportCommand(&hkCmd, lpool::testSid, false); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); } /* we need to reset the subscription list because the pool owner diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h index 20c565ff..eb6d8a5d 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h @@ -169,8 +169,8 @@ public: return dynamic_cast(messageQueue); } - ReturnValue_t subscribePeriodicHk() { - return poolManager.subscribeForPeriodicPacket(lpool::testSid, true, 0.2, false); + ReturnValue_t subscribePeriodicHk(bool enableReporting) { + return poolManager.subscribeForPeriodicPacket(lpool::testSid, enableReporting, 0.2, false); } ReturnValue_t subscribeWrapperSetUpdate() { From dcde177fe38fc29ea6d02cf6024f31b7f8dcf7ba Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Mar 2021 22:08:05 +0100 Subject: [PATCH 25/37] added additional tests for more than 8 variables --- unittest/tests/datapoollocal/DataSetTest.cpp | 45 ++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/unittest/tests/datapoollocal/DataSetTest.cpp b/unittest/tests/datapoollocal/DataSetTest.cpp index 4def46be..c11c66c2 100644 --- a/unittest/tests/datapoollocal/DataSetTest.cpp +++ b/unittest/tests/datapoollocal/DataSetTest.cpp @@ -207,6 +207,51 @@ TEST_CASE("DataSetTest" , "[DataSetTest]") { } + SECTION("MorePoolVariables") { + LocalDataSet set(poolOwner, 2, 10); + + /* Register same variables again to get more than 8 registered variables */ + for(uint8_t idx = 0; idx < 8; idx ++) { + REQUIRE(set.registerVariable(&localSet.localPoolVarUint8) == retval::CATCH_OK); + } + REQUIRE(set.registerVariable(&localSet.localPoolVarUint8) == retval::CATCH_OK); + REQUIRE(set.registerVariable(&localSet.localPoolUint16Vec) == retval::CATCH_OK); + + set.setValidityBufferGeneration(true); + { + PoolReadGuard readHelper(&localSet); + localSet.localPoolVarUint8.value = 42; + localSet.localPoolVarUint8.setValid(true); + localSet.localPoolUint16Vec.setValid(false); + } + + size_t maxSize = set.getSerializedSize(); + CHECK(maxSize == 9 + sizeof(uint16_t) * 3 + 2); + size_t serSize = 0; + /* Already reserve additional space for validity buffer, will be needed later */ + uint8_t buffer[maxSize + 1]; + uint8_t* buffPtr = buffer; + CHECK(set.serialize(&buffPtr, &serSize, maxSize, + SerializeIF::Endianness::MACHINE) == retval::CATCH_OK); + std::array validityBuffer; + std::memcpy(validityBuffer.data(), buffer + 9 + sizeof(uint16_t) * 3, 2); + /* The first 9 variables should be valid */ + CHECK(validityBuffer[0] == 0xff); + CHECK(bitutil::bitGet(validityBuffer.data() + 1, 0) == true); + CHECK(bitutil::bitGet(validityBuffer.data() + 1, 1) == false); + + /* Now we invert the validity */ + validityBuffer[0] = 0; + validityBuffer[1] = 0b0100'0000; + std::memcpy(buffer + 9 + sizeof(uint16_t) * 3, validityBuffer.data(), 2); + const uint8_t* constBuffPtr = buffer; + size_t sizeToDeSerialize = serSize; + CHECK(set.deSerialize(&constBuffPtr, &sizeToDeSerialize, SerializeIF::Endianness::MACHINE) + == retval::CATCH_OK); + CHECK(localSet.localPoolVarUint8.isValid() == false); + CHECK(localSet.localPoolUint16Vec.isValid() == true); + } + /* we need to reset the subscription list because the pool owner is a global object. */ CHECK(poolOwner->reset() == retval::CATCH_OK); From 8d28bc4b6addfb9d48279ad5cf1e44d56fe1cab2 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Mar 2021 22:19:55 +0100 Subject: [PATCH 26/37] added source file --- unittest/tests/datapoollocal/CMakeLists.txt | 1 + .../datapoollocal/LocalPoolManagerTest.cpp | 2 + .../datapoollocal/LocalPoolOwnerBase.cpp | 91 ++++++++++++++++ .../tests/datapoollocal/LocalPoolOwnerBase.h | 101 +++--------------- 4 files changed, 108 insertions(+), 87 deletions(-) create mode 100644 unittest/tests/datapoollocal/LocalPoolOwnerBase.cpp diff --git a/unittest/tests/datapoollocal/CMakeLists.txt b/unittest/tests/datapoollocal/CMakeLists.txt index f196a080..1c98e7dc 100644 --- a/unittest/tests/datapoollocal/CMakeLists.txt +++ b/unittest/tests/datapoollocal/CMakeLists.txt @@ -3,4 +3,5 @@ target_sources(${TARGET_NAME} PRIVATE LocalPoolVectorTest.cpp DataSetTest.cpp LocalPoolManagerTest.cpp + LocalPoolOwnerBase.cpp ) diff --git a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp index 4457e85b..cce55e1a 100644 --- a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp +++ b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp @@ -276,6 +276,8 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); CHECK(messagesSent == 1); + + HousekeepingMessage::setUpdateNotificationSetCommand(&hkCmd, lpool::testSid); } /* we need to reset the subscription list because the pool owner diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.cpp b/unittest/tests/datapoollocal/LocalPoolOwnerBase.cpp new file mode 100644 index 00000000..7abbd454 --- /dev/null +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.cpp @@ -0,0 +1,91 @@ +#include "LocalPoolOwnerBase.h" + +LocalPoolOwnerBase::LocalPoolOwnerBase(object_id_t objectId): + SystemObject(objectId), poolManager(this, messageQueue), + dataset(this, lpool::testSetId) { + messageQueue = new MessageQueueMockBase(); +} + +ReturnValue_t LocalPoolOwnerBase::initializeHkManager() { + if(not initialized) { + initialized = true; + return poolManager.initialize(messageQueue); + } + return HasReturnvaluesIF::RETURN_OK; +} + +ReturnValue_t LocalPoolOwnerBase::initializeLocalDataPool(localpool::DataPool &localDataPoolMap, + LocalDataPoolManager &poolManager) { + + // Default initialization empty for now. + localDataPoolMap.emplace(lpool::uint8VarId, + new PoolEntry({0})); + localDataPoolMap.emplace(lpool::floatVarId, + new PoolEntry({0})); + localDataPoolMap.emplace(lpool::uint32VarId, + new PoolEntry({0})); + + localDataPoolMap.emplace(lpool::uint16Vec3Id, + new PoolEntry({0, 0, 0})); + localDataPoolMap.emplace(lpool::int64Vec2Id, + new PoolEntry({0, 0})); + return HasReturnvaluesIF::RETURN_OK; +} + +LocalPoolObjectBase* LocalPoolOwnerBase::getPoolObjectHandle(lp_id_t localPoolId) { + if(localPoolId == lpool::uint8VarId) { + return &testUint8; + } + else if(localPoolId == lpool::uint16Vec3Id) { + return &testUint16Vec; + } + else if(localPoolId == lpool::floatVarId) { + return &testFloat; + } + else if(localPoolId == lpool::int64Vec2Id) { + return &testInt64Vec; + } + else if(localPoolId == lpool::uint32VarId) { + return &testUint32; + } + else { + return &testUint8; + } +} + +ReturnValue_t LocalPoolOwnerBase::reset() { + resetSubscriptionList(); + ReturnValue_t status = HasReturnvaluesIF::RETURN_OK; + { + PoolReadGuard readHelper(&dataset); + if(readHelper.getReadResult() != HasReturnvaluesIF::RETURN_OK) { + status = readHelper.getReadResult(); + } + dataset.localPoolVarUint8.value = 0; + dataset.localPoolVarFloat.value = 0.0; + dataset.localPoolUint16Vec.value[0] = 0; + dataset.localPoolUint16Vec.value[1] = 0; + dataset.localPoolUint16Vec.value[2] = 0; + dataset.setValidity(false, true); + } + + { + PoolReadGuard readHelper(&testUint32); + if(readHelper.getReadResult() != HasReturnvaluesIF::RETURN_OK) { + status = readHelper.getReadResult(); + } + testUint32.value = 0; + testUint32.setValid(false); + } + + { + PoolReadGuard readHelper(&testInt64Vec); + if(readHelper.getReadResult() != HasReturnvaluesIF::RETURN_OK) { + status = readHelper.getReadResult(); + } + testInt64Vec.value[0] = 0; + testInt64Vec.value[1] = 0; + testInt64Vec.setValid(false); + } + return status; +} diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h index eb6d8a5d..fc009544 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h @@ -70,12 +70,7 @@ private: class LocalPoolOwnerBase: public SystemObject, public HasLocalDataPoolIF { public: - LocalPoolOwnerBase( - object_id_t objectId = objects::TEST_LOCAL_POOL_OWNER_BASE): - SystemObject(objectId), poolManager(this, messageQueue), - dataset(this, lpool::testSetId) { - messageQueue = new MessageQueueMockBase(); - } + LocalPoolOwnerBase(object_id_t objectId = objects::TEST_LOCAL_POOL_OWNER_BASE); ~LocalPoolOwnerBase() { QueueFactory::instance()->deleteMessageQueue(messageQueue); @@ -85,13 +80,7 @@ public: return SystemObject::getObjectId(); } - ReturnValue_t initializeHkManager() { - if(not initialized) { - initialized = true; - return poolManager.initialize(messageQueue); - } - return HasReturnvaluesIF::RETURN_OK; - } + ReturnValue_t initializeHkManager(); ReturnValue_t initializeHkManagerAfterTaskCreation() { if(not initializedAfterTaskCreation) { @@ -107,23 +96,8 @@ public: } // This is called by initializeAfterTaskCreation of the HK manager. - virtual ReturnValue_t initializeLocalDataPool( - localpool::DataPool& localDataPoolMap, - LocalDataPoolManager& poolManager) { - // Default initialization empty for now. - localDataPoolMap.emplace(lpool::uint8VarId, - new PoolEntry({0})); - localDataPoolMap.emplace(lpool::floatVarId, - new PoolEntry({0})); - localDataPoolMap.emplace(lpool::uint32VarId, - new PoolEntry({0})); - - localDataPoolMap.emplace(lpool::uint16Vec3Id, - new PoolEntry({0, 0, 0})); - localDataPoolMap.emplace(lpool::int64Vec2Id, - new PoolEntry({0, 0})); - return HasReturnvaluesIF::RETURN_OK; - } + virtual ReturnValue_t initializeLocalDataPool(localpool::DataPool& localDataPoolMap, + LocalDataPoolManager& poolManager) override; LocalDataPoolManager* getHkManagerHandle() override { return &poolManager; @@ -143,27 +117,7 @@ public: return &dataset; } - virtual LocalPoolObjectBase* getPoolObjectHandle( - lp_id_t localPoolId) override { - if(localPoolId == lpool::uint8VarId) { - return &testUint8; - } - else if(localPoolId == lpool::uint16Vec3Id) { - return &testUint16Vec; - } - else if(localPoolId == lpool::floatVarId) { - return &testFloat; - } - else if(localPoolId == lpool::int64Vec2Id) { - return &testInt64Vec; - } - else if(localPoolId == lpool::uint32VarId) { - return &testUint32; - } - else { - return &testUint8; - } - } + virtual LocalPoolObjectBase* getPoolObjectHandle(lp_id_t localPoolId) override; MessageQueueMockBase* getMockQueueHandle() const { return dynamic_cast(messageQueue); @@ -198,51 +152,24 @@ public: MessageQueueIF::NO_QUEUE, objects::HK_RECEIVER_MOCK, true); } - ReturnValue_t reset() { - resetSubscriptionList(); - ReturnValue_t status = HasReturnvaluesIF::RETURN_OK; - { - PoolReadGuard readHelper(&dataset); - if(readHelper.getReadResult() != HasReturnvaluesIF::RETURN_OK) { - status = readHelper.getReadResult(); - } - dataset.localPoolVarUint8.value = 0; - dataset.localPoolVarFloat.value = 0.0; - dataset.localPoolUint16Vec.value[0] = 0; - dataset.localPoolUint16Vec.value[1] = 0; - dataset.localPoolUint16Vec.value[2] = 0; - dataset.setValidity(false, true); - } - - { - PoolReadGuard readHelper(&testUint32); - if(readHelper.getReadResult() != HasReturnvaluesIF::RETURN_OK) { - status = readHelper.getReadResult(); - } - testUint32.value = 0; - testUint32.setValid(false); - } - - { - PoolReadGuard readHelper(&testInt64Vec); - if(readHelper.getReadResult() != HasReturnvaluesIF::RETURN_OK) { - status = readHelper.getReadResult(); - } - testInt64Vec.value[0] = 0; - testInt64Vec.value[1] = 0; - testInt64Vec.setValid(false); - } - return status; - } + ReturnValue_t reset(); void resetSubscriptionList() { poolManager.clearReceiversList(); } + void handleChangedDataset(sid_t sid, store_address_t storeId) { + this->thisSidHasChanged = sid; + this->storeIdForChangedSid = storeId; + } + LocalDataPoolManager poolManager; LocalPoolTestDataSet dataset; private: + sid_t thisSidHasChanged; + store_address_t storeIdForChangedSid; + lp_var_t testUint8 = lp_var_t(this, lpool::uint8VarId); lp_var_t testFloat = lp_var_t(this, lpool::floatVarId); lp_var_t testUint32 = lp_var_t(this, lpool::uint32VarId); From 620b2ae79e551b110e37f45f8e9123100b326a89 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Mar 2021 23:16:47 +0100 Subject: [PATCH 27/37] weird bug --- datapoollocal/SharedLocalDataSet.cpp | 13 ++++++ datapoollocal/SharedLocalDataSet.h | 9 +++- unittest/tests/datapoollocal/DataSetTest.cpp | 6 +++ .../datapoollocal/LocalPoolManagerTest.cpp | 5 +++ .../datapoollocal/LocalPoolOwnerBase.cpp | 43 +++++++++++++++++++ .../tests/datapoollocal/LocalPoolOwnerBase.h | 23 +++++----- 6 files changed, 84 insertions(+), 15 deletions(-) diff --git a/datapoollocal/SharedLocalDataSet.cpp b/datapoollocal/SharedLocalDataSet.cpp index dd1bdcc4..9915628e 100644 --- a/datapoollocal/SharedLocalDataSet.cpp +++ b/datapoollocal/SharedLocalDataSet.cpp @@ -1,5 +1,6 @@ #include "SharedLocalDataSet.h" + SharedLocalDataSet::SharedLocalDataSet(object_id_t objectId, sid_t sid, const size_t maxSize): SystemObject(objectId), LocalPoolDataSetBase(sid, nullptr, maxSize) { @@ -11,6 +12,18 @@ ReturnValue_t SharedLocalDataSet::lockDataset(dur_millis_t mutexTimeout) { return datasetLock->lockMutex(MutexIF::TimeoutType::WAITING, mutexTimeout); } +SharedLocalDataSet::SharedLocalDataSet(object_id_t objectId, + HasLocalDataPoolIF *owner, uint32_t setId, + const size_t maxSize): SystemObject(objectId), + LocalPoolDataSetBase(owner, setId, nullptr, maxSize) { + this->setContainer(poolVarVector.data()); + datasetLock = MutexFactory::instance()->createMutex(); +} + +SharedLocalDataSet::~SharedLocalDataSet() { + MutexFactory::instance()->deleteMutex(datasetLock); +} + ReturnValue_t SharedLocalDataSet::unlockDataset() { return datasetLock->unlockMutex(); } diff --git a/datapoollocal/SharedLocalDataSet.h b/datapoollocal/SharedLocalDataSet.h index 83f2a72f..bbb47da4 100644 --- a/datapoollocal/SharedLocalDataSet.h +++ b/datapoollocal/SharedLocalDataSet.h @@ -11,15 +11,20 @@ * multiple threads. It provides a lock in addition to all other functionalities provided * by the LocalPoolDataSetBase class. * - * TODO: override and protect read, commit and some other calls used by pool manager. + * The user is completely responsible for lockingand unlocking the dataset when using the + * shared dataset. */ class SharedLocalDataSet: public SystemObject, public LocalPoolDataSetBase, public SharedDataSetIF { public: - SharedLocalDataSet(object_id_t objectId, sid_t sid, + SharedLocalDataSet(object_id_t objectId, HasLocalDataPoolIF* owner, uint32_t setId, const size_t maxSize); + SharedLocalDataSet(object_id_t objectId, sid_t sid, const size_t maxSize); + + virtual~ SharedLocalDataSet(); + ReturnValue_t lockDataset(dur_millis_t mutexTimeout) override; ReturnValue_t unlockDataset() override; private: diff --git a/unittest/tests/datapoollocal/DataSetTest.cpp b/unittest/tests/datapoollocal/DataSetTest.cpp index c11c66c2..30b5bb05 100644 --- a/unittest/tests/datapoollocal/DataSetTest.cpp +++ b/unittest/tests/datapoollocal/DataSetTest.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -252,6 +253,11 @@ TEST_CASE("DataSetTest" , "[DataSetTest]") { CHECK(localSet.localPoolUint16Vec.isValid() == true); } + SECTION("SharedDataSet") { + object_id_t sharedSetId = objects::SHARED_SET_ID; + SharedLocalDataSet sharedSet(sharedSetId, poolOwner, 2, 5); + } + /* we need to reset the subscription list because the pool owner is a global object. */ CHECK(poolOwner->reset() == retval::CATCH_OK); diff --git a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp index cce55e1a..2d7294ef 100644 --- a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp +++ b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp @@ -278,6 +278,11 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { CHECK(messagesSent == 1); HousekeepingMessage::setUpdateNotificationSetCommand(&hkCmd, lpool::testSid); + sid_t sidToCheck; + store_address_t storeId; + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); + CHECK(poolOwner->changedDataSetCallbackWasCalled(sidToCheck, storeId) == true); + CHECK(sidToCheck == lpool::testSid); } /* we need to reset the subscription list because the pool owner diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.cpp b/unittest/tests/datapoollocal/LocalPoolOwnerBase.cpp index 7abbd454..0e2703a7 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.cpp +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.cpp @@ -89,3 +89,46 @@ ReturnValue_t LocalPoolOwnerBase::reset() { } return status; } + +bool LocalPoolOwnerBase::changedDataSetCallbackWasCalled(sid_t &sid, store_address_t &storeId) { + bool condition = false; + if(not this->changedDatasetSid.notSet()) { + condition = true; + } + sid = changedDatasetSid; + storeId = storeIdForChangedSet; + this->changedDatasetSid.raw = sid_t::INVALID_SID; + this->storeIdForChangedSet = storeId::INVALID_STORE_ADDRESS; + return condition; +} + +void LocalPoolOwnerBase::handleChangedDataset(sid_t sid, store_address_t storeId) { + this->changedDatasetSid = sid; + this->storeIdForChangedSet = storeId; +} + +bool LocalPoolOwnerBase::changedVariableCallbackWasCalled(gp_id_t &gpid, store_address_t &storeId) { + bool condition = false; + if(not this->changedPoolVariableGpid.notSet()) { + condition = true; + } + gpid = changedPoolVariableGpid; + storeId = storeIdForChangedVariable; + this->changedPoolVariableGpid.raw = gp_id_t::INVALID_GPID; + this->storeIdForChangedVariable = storeId::INVALID_STORE_ADDRESS; + return condition; +} + +ReturnValue_t LocalPoolOwnerBase::initializeHkManagerAfterTaskCreation() { + if(not initializedAfterTaskCreation) { + initializedAfterTaskCreation = true; + return poolManager.initializeAfterTaskCreation(); + } + return HasReturnvaluesIF::RETURN_OK; + +} + +void LocalPoolOwnerBase::handleChangedPoolVariable(gp_id_t globPoolId, store_address_t storeId) { + this->changedPoolVariableGpid = globPoolId; + this->storeIdForChangedVariable = storeId; +} diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h index fc009544..7915eeb4 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h @@ -82,13 +82,7 @@ public: ReturnValue_t initializeHkManager(); - ReturnValue_t initializeHkManagerAfterTaskCreation() { - if(not initializedAfterTaskCreation) { - initializedAfterTaskCreation = true; - return poolManager.initializeAfterTaskCreation(); - } - return HasReturnvaluesIF::RETURN_OK; - } + ReturnValue_t initializeHkManagerAfterTaskCreation(); /** Command queue for housekeeping messages. */ MessageQueueId_t getCommandQueue() const override { @@ -158,17 +152,20 @@ public: poolManager.clearReceiversList(); } - void handleChangedDataset(sid_t sid, store_address_t storeId) { - this->thisSidHasChanged = sid; - this->storeIdForChangedSid = storeId; - } + bool changedDataSetCallbackWasCalled(sid_t& sid, store_address_t& storeId); + bool changedVariableCallbackWasCalled(gp_id_t& gpid, store_address_t& storeId); LocalDataPoolManager poolManager; LocalPoolTestDataSet dataset; private: - sid_t thisSidHasChanged; - store_address_t storeIdForChangedSid; + void handleChangedDataset(sid_t sid, store_address_t storeId) override; + sid_t changedDatasetSid; + store_address_t storeIdForChangedSet; + + void handleChangedPoolVariable(gp_id_t globPoolId, store_address_t storeId) override; + gp_id_t changedPoolVariableGpid; + store_address_t storeIdForChangedVariable; lp_var_t testUint8 = lp_var_t(this, lpool::uint8VarId); lp_var_t testFloat = lp_var_t(this, lpool::floatVarId); From 78b6a83285df57f1df6354a474cf774d45e30d9b Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 11 Mar 2021 00:18:17 +0100 Subject: [PATCH 28/37] issues with virtual inheritanc3 --- datapool/PoolDataSetBase.cpp | 3 +-- datapool/SharedDataSetIF.h | 7 +++++-- datapoollocal/LocalPoolDataSetBase.cpp | 2 +- datapoollocal/SharedLocalDataSet.cpp | 18 +++++++++++++----- datapoollocal/SharedLocalDataSet.h | 3 ++- unittest/tests/datapoollocal/DataSetTest.cpp | 10 +++++++--- .../tests/datapoollocal/LocalPoolOwnerBase.h | 4 ++-- 7 files changed, 31 insertions(+), 16 deletions(-) diff --git a/datapool/PoolDataSetBase.cpp b/datapool/PoolDataSetBase.cpp index bdca22c3..73ec4bd7 100644 --- a/datapool/PoolDataSetBase.cpp +++ b/datapool/PoolDataSetBase.cpp @@ -8,8 +8,7 @@ PoolDataSetBase::PoolDataSetBase(PoolVariableIF** registeredVariablesArray, const size_t maxFillCount): registeredVariables(registeredVariablesArray), - maxFillCount(maxFillCount) { -} + maxFillCount(maxFillCount) {} PoolDataSetBase::~PoolDataSetBase() {} diff --git a/datapool/SharedDataSetIF.h b/datapool/SharedDataSetIF.h index b8d98794..da21f9cf 100644 --- a/datapool/SharedDataSetIF.h +++ b/datapool/SharedDataSetIF.h @@ -1,13 +1,16 @@ #ifndef FRAMEWORK_DATAPOOL_SHAREDDATASETIF_H_ #define FRAMEWORK_DATAPOOL_SHAREDDATASETIF_H_ + #include "PoolDataSetIF.h" -class SharedDataSetIF: public PoolDataSetIF { +class SharedDataSetIF: + public PoolDataSetIF { public: virtual ~SharedDataSetIF() {}; private: - virtual ReturnValue_t lockDataset(dur_millis_t mutexTimeout) = 0; + virtual ReturnValue_t lockDataset(MutexIF::TimeoutType timeoutType, + dur_millis_t mutexTimeout) = 0; virtual ReturnValue_t unlockDataset() = 0; }; diff --git a/datapoollocal/LocalPoolDataSetBase.cpp b/datapoollocal/LocalPoolDataSetBase.cpp index 3abbf0dd..d93b926d 100644 --- a/datapoollocal/LocalPoolDataSetBase.cpp +++ b/datapoollocal/LocalPoolDataSetBase.cpp @@ -44,7 +44,7 @@ LocalPoolDataSetBase::LocalPoolDataSetBase(HasLocalDataPoolIF *hkOwner, LocalPoolDataSetBase::LocalPoolDataSetBase(sid_t sid, PoolVariableIF** registeredVariablesArray, const size_t maxNumberOfVariables): - PoolDataSetBase(registeredVariablesArray, maxNumberOfVariables) { + PoolDataSetBase(registeredVariablesArray, maxNumberOfVariables) { HasLocalDataPoolIF* hkOwner = objectManager->get( sid.objectId); if(hkOwner != nullptr) { diff --git a/datapoollocal/SharedLocalDataSet.cpp b/datapoollocal/SharedLocalDataSet.cpp index 9915628e..abcf7da9 100644 --- a/datapoollocal/SharedLocalDataSet.cpp +++ b/datapoollocal/SharedLocalDataSet.cpp @@ -8,10 +8,6 @@ SharedLocalDataSet::SharedLocalDataSet(object_id_t objectId, sid_t sid, datasetLock = MutexFactory::instance()->createMutex(); } -ReturnValue_t SharedLocalDataSet::lockDataset(dur_millis_t mutexTimeout) { - return datasetLock->lockMutex(MutexIF::TimeoutType::WAITING, mutexTimeout); -} - SharedLocalDataSet::SharedLocalDataSet(object_id_t objectId, HasLocalDataPoolIF *owner, uint32_t setId, const size_t maxSize): SystemObject(objectId), @@ -20,10 +16,22 @@ SharedLocalDataSet::SharedLocalDataSet(object_id_t objectId, datasetLock = MutexFactory::instance()->createMutex(); } +ReturnValue_t SharedLocalDataSet::lockDataset(MutexIF::TimeoutType timeoutType, + dur_millis_t mutexTimeout) { + if(datasetLock != nullptr) { + return datasetLock->lockMutex(timeoutType, mutexTimeout); + } + return HasReturnvaluesIF::RETURN_FAILED; +} + + SharedLocalDataSet::~SharedLocalDataSet() { MutexFactory::instance()->deleteMutex(datasetLock); } ReturnValue_t SharedLocalDataSet::unlockDataset() { - return datasetLock->unlockMutex(); + if(datasetLock != nullptr) { + return datasetLock->unlockMutex(); + } + return HasReturnvaluesIF::RETURN_FAILED; } diff --git a/datapoollocal/SharedLocalDataSet.h b/datapoollocal/SharedLocalDataSet.h index bbb47da4..8d12610a 100644 --- a/datapoollocal/SharedLocalDataSet.h +++ b/datapoollocal/SharedLocalDataSet.h @@ -25,7 +25,8 @@ public: virtual~ SharedLocalDataSet(); - ReturnValue_t lockDataset(dur_millis_t mutexTimeout) override; + ReturnValue_t lockDataset(MutexIF::TimeoutType timeoutType = MutexIF::TimeoutType::WAITING, + dur_millis_t mutexTimeout = 20) override; ReturnValue_t unlockDataset() override; private: diff --git a/unittest/tests/datapoollocal/DataSetTest.cpp b/unittest/tests/datapoollocal/DataSetTest.cpp index 30b5bb05..ecc1921f 100644 --- a/unittest/tests/datapoollocal/DataSetTest.cpp +++ b/unittest/tests/datapoollocal/DataSetTest.cpp @@ -4,9 +4,9 @@ #include #include +#include #include #include -#include #include #include @@ -254,8 +254,12 @@ TEST_CASE("DataSetTest" , "[DataSetTest]") { } SECTION("SharedDataSet") { - object_id_t sharedSetId = objects::SHARED_SET_ID; - SharedLocalDataSet sharedSet(sharedSetId, poolOwner, 2, 5); +// object_id_t sharedSetId = objects::SHARED_SET_ID; +// SharedLocalDataSet sharedSet(sharedSetId, poolOwner, 2, 5); +// CHECK(sharedSet.initialize() == retval::CATCH_OK); +// CHECK(sharedSet.lockDataset() == retval::CATCH_OK); +// +// CHECK(sharedSet.unlockDataset() == retval::CATCH_OK); } /* we need to reset the subscription list because the pool owner diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h index 7915eeb4..9a7639f8 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h @@ -36,11 +36,11 @@ static const gp_id_t uint64Vec2Id = gp_id_t(objects::TEST_LOCAL_POOL_OWNER_BASE, class LocalPoolStaticTestDataSet: public StaticLocalDataSet<3> { public: LocalPoolStaticTestDataSet(): - StaticLocalDataSet(lpool::testSid) { + StaticLocalDataSet(lpool::testSid) { } LocalPoolStaticTestDataSet(HasLocalDataPoolIF* owner, uint32_t setId): - StaticLocalDataSet(owner, setId) { + StaticLocalDataSet(owner, setId) { } lp_var_t localPoolVarUint8 = lp_var_t(lpool::uint8VarGpid, this); From 824f272432b51fee59e9061f6118e4f0aa2f4e47 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 11 Mar 2021 01:04:39 +0100 Subject: [PATCH 29/37] no virtual inhertience for now --- datapool/PoolDataSetIF.h | 4 ++-- datapool/SharedDataSetIF.h | 3 +-- unittest/tests/datapoollocal/DataSetTest.cpp | 12 ++++++------ unittest/tests/datapoollocal/LocalPoolOwnerBase.h | 10 ++++++++-- 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/datapool/PoolDataSetIF.h b/datapool/PoolDataSetIF.h index 9151f2f8..f905cc4d 100644 --- a/datapool/PoolDataSetIF.h +++ b/datapool/PoolDataSetIF.h @@ -9,8 +9,8 @@ * and unlock a data pool and read/commit semantics. */ class PoolDataSetIF: - public DataSetIF, - public ReadCommitIF { + virtual public DataSetIF, + virtual public ReadCommitIF { public: virtual~ PoolDataSetIF() {}; diff --git a/datapool/SharedDataSetIF.h b/datapool/SharedDataSetIF.h index da21f9cf..4d23f87f 100644 --- a/datapool/SharedDataSetIF.h +++ b/datapool/SharedDataSetIF.h @@ -3,8 +3,7 @@ #include "PoolDataSetIF.h" -class SharedDataSetIF: - public PoolDataSetIF { +class SharedDataSetIF { public: virtual ~SharedDataSetIF() {}; diff --git a/unittest/tests/datapoollocal/DataSetTest.cpp b/unittest/tests/datapoollocal/DataSetTest.cpp index ecc1921f..06964d26 100644 --- a/unittest/tests/datapoollocal/DataSetTest.cpp +++ b/unittest/tests/datapoollocal/DataSetTest.cpp @@ -254,12 +254,12 @@ TEST_CASE("DataSetTest" , "[DataSetTest]") { } SECTION("SharedDataSet") { -// object_id_t sharedSetId = objects::SHARED_SET_ID; -// SharedLocalDataSet sharedSet(sharedSetId, poolOwner, 2, 5); -// CHECK(sharedSet.initialize() == retval::CATCH_OK); -// CHECK(sharedSet.lockDataset() == retval::CATCH_OK); -// -// CHECK(sharedSet.unlockDataset() == retval::CATCH_OK); + object_id_t sharedSetId = objects::SHARED_SET_ID; + SharedLocalDataSet sharedSet(sharedSetId, poolOwner, 2, 5); + CHECK(sharedSet.initialize() == retval::CATCH_OK); + CHECK(sharedSet.lockDataset() == retval::CATCH_OK); + + CHECK(sharedSet.unlockDataset() == retval::CATCH_OK); } /* we need to reset the subscription list because the pool owner diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h index 9a7639f8..f08b9eb9 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h @@ -36,7 +36,8 @@ static const gp_id_t uint64Vec2Id = gp_id_t(objects::TEST_LOCAL_POOL_OWNER_BASE, class LocalPoolStaticTestDataSet: public StaticLocalDataSet<3> { public: LocalPoolStaticTestDataSet(): - StaticLocalDataSet(lpool::testSid) { + StaticLocalDataSet(lpool::testSid) { + } LocalPoolStaticTestDataSet(HasLocalDataPoolIF* owner, uint32_t setId): @@ -53,7 +54,12 @@ private: class LocalPoolTestDataSet: public LocalDataSet { public: LocalPoolTestDataSet(): - LocalDataSet(lpool::testSid, lpool::dataSetMaxVariables) { + LocalDataSet(lpool::testSid, lpool::dataSetMaxVariables), + localPoolVarUint8(lpool::uint8VarGpid, this), + localPoolVarFloat(lpool::floatVarGpid, this), + localPoolUint16Vec(lpool::uint16Vec3Gpid, this) + + { } LocalPoolTestDataSet(HasLocalDataPoolIF* owner, uint32_t setId): From 6f78c13dcfb8343563339c30d90f3cc5134ed38c Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 11 Mar 2021 11:06:23 +0100 Subject: [PATCH 30/37] added addtional nullptr check --- datapoollocal/LocalDataPoolManager.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/datapoollocal/LocalDataPoolManager.h b/datapoollocal/LocalDataPoolManager.h index d06cf2c6..2ec81f1c 100644 --- a/datapoollocal/LocalDataPoolManager.h +++ b/datapoollocal/LocalDataPoolManager.h @@ -391,6 +391,10 @@ protected: template inline ReturnValue_t LocalDataPoolManager::fetchPoolEntry(lp_id_t localPoolId, PoolEntry **poolEntry) { + if(poolEntry == nullptr) { + return HasReturnvaluesIF::RETURN_FAILED; + } + auto poolIter = localPoolMap.find(localPoolId); if (poolIter == localPoolMap.end()) { printWarningOrError(sif::OutputTypes::OUT_WARNING, "fetchPoolEntry", From 3bacc8ec53b1370ce6829af7e3c69ae7a0b21b5d Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 11 Mar 2021 12:04:54 +0100 Subject: [PATCH 31/37] more tests and bugfixes --- datapoollocal/LocalDataPoolManager.cpp | 20 ++++++-- datapoollocal/LocalPoolDataSetBase.cpp | 8 ++- datapoollocal/LocalPoolDataSetBase.h | 5 +- .../internal/LocalPoolDataSetAttorney.h | 5 +- housekeeping/PeriodicHousekeepingHelper.cpp | 11 ++-- housekeeping/PeriodicHousekeepingHelper.h | 7 ++- .../datapoollocal/LocalPoolManagerTest.cpp | 51 ++++++++++++++++++- .../datapoollocal/LocalPoolOwnerBase.cpp | 5 ++ .../tests/datapoollocal/LocalPoolOwnerBase.h | 15 ++---- unittest/tests/mocks/MessageQueueMockBase.h | 11 ++++ 10 files changed, 103 insertions(+), 35 deletions(-) diff --git a/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index 83e30e82..cad54094 100644 --- a/datapoollocal/LocalDataPoolManager.cpp +++ b/datapoollocal/LocalDataPoolManager.cpp @@ -38,7 +38,11 @@ LocalDataPoolManager::LocalDataPoolManager(HasLocalDataPoolIF* owner, MessageQue hkQueue = queueToUse; } -LocalDataPoolManager::~LocalDataPoolManager() {} +LocalDataPoolManager::~LocalDataPoolManager() { + if(mutex != nullptr) { + MutexFactory::instance()->deleteMutex(mutex); + } +} ReturnValue_t LocalDataPoolManager::initialize(MessageQueueIF* queueToUse) { if(queueToUse == nullptr) { @@ -375,7 +379,7 @@ ReturnValue_t LocalDataPoolManager::subscribeForPeriodicPacket(sid_t sid, LocalPoolDataSetAttorney::setReportingEnabled(*dataSet, enableReporting); LocalPoolDataSetAttorney::setDiagnostic(*dataSet, isDiagnostics); LocalPoolDataSetAttorney::initializePeriodicHelper(*dataSet, collectionInterval, - owner->getPeriodicOperationFrequency(), isDiagnostics); + owner->getPeriodicOperationFrequency()); } hkReceivers.push_back(hkReceiver); @@ -518,11 +522,19 @@ ReturnValue_t LocalDataPoolManager::handleHousekeepingMessage( } case(HousekeepingMessage::REPORT_DIAGNOSTICS_REPORT_STRUCTURES): { - return generateSetStructurePacket(sid, true); + result = generateSetStructurePacket(sid, true); + if(result == HasReturnvaluesIF::RETURN_OK) { + return result; + } + break; } case(HousekeepingMessage::REPORT_HK_REPORT_STRUCTURES): { - return generateSetStructurePacket(sid, false); + result = generateSetStructurePacket(sid, false); + if(result == HasReturnvaluesIF::RETURN_OK) { + return result; + } + break; } case(HousekeepingMessage::MODIFY_DIAGNOSTICS_REPORT_COLLECTION_INTERVAL): case(HousekeepingMessage::MODIFY_PARAMETER_REPORT_COLLECTION_INTERVAL): { diff --git a/datapoollocal/LocalPoolDataSetBase.cpp b/datapoollocal/LocalPoolDataSetBase.cpp index d93b926d..2a2444c4 100644 --- a/datapoollocal/LocalPoolDataSetBase.cpp +++ b/datapoollocal/LocalPoolDataSetBase.cpp @@ -264,11 +264,9 @@ bool LocalPoolDataSetBase::getReportingEnabled() const { return reportingEnabled; } -void LocalPoolDataSetBase::initializePeriodicHelper( - float collectionInterval, dur_millis_t minimumPeriodicInterval, - bool isDiagnostics, uint8_t nonDiagIntervalFactor) { - periodicHelper->initialize(collectionInterval, minimumPeriodicInterval, - isDiagnostics, nonDiagIntervalFactor); +void LocalPoolDataSetBase::initializePeriodicHelper(float collectionInterval, + dur_millis_t minimumPeriodicInterval, uint8_t nonDiagIntervalFactor) { + periodicHelper->initialize(collectionInterval, minimumPeriodicInterval, nonDiagIntervalFactor); } void LocalPoolDataSetBase::setChanged(bool changed) { diff --git a/datapoollocal/LocalPoolDataSetBase.h b/datapoollocal/LocalPoolDataSetBase.h index 39ea9f1a..ab67dc3f 100644 --- a/datapoollocal/LocalPoolDataSetBase.h +++ b/datapoollocal/LocalPoolDataSetBase.h @@ -191,9 +191,8 @@ protected: bool reportingEnabled = false; void setReportingEnabled(bool enabled); - void initializePeriodicHelper(float collectionInterval, - dur_millis_t minimumPeriodicInterval, - bool isDiagnostics, uint8_t nonDiagIntervalFactor = 5); + void initializePeriodicHelper(float collectionInterval, dur_millis_t minimumPeriodicInterval, + uint8_t nonDiagIntervalFactor = 5); /** * If the valid state of a dataset is always relevant to the whole diff --git a/datapoollocal/internal/LocalPoolDataSetAttorney.h b/datapoollocal/internal/LocalPoolDataSetAttorney.h index 7a34e9c8..f81428cd 100644 --- a/datapoollocal/internal/LocalPoolDataSetAttorney.h +++ b/datapoollocal/internal/LocalPoolDataSetAttorney.h @@ -14,9 +14,8 @@ private: } static void initializePeriodicHelper(LocalPoolDataSetBase& set, float collectionInterval, - uint32_t minimumPeriodicIntervalMs, - bool isDiagnostics, uint8_t nonDiagIntervalFactor = 5) { - set.initializePeriodicHelper(collectionInterval, minimumPeriodicIntervalMs, isDiagnostics, + uint32_t minimumPeriodicIntervalMs, uint8_t nonDiagIntervalFactor = 5) { + set.initializePeriodicHelper(collectionInterval, minimumPeriodicIntervalMs, nonDiagIntervalFactor); } diff --git a/housekeeping/PeriodicHousekeepingHelper.cpp b/housekeeping/PeriodicHousekeepingHelper.cpp index 5152706f..892eb354 100644 --- a/housekeeping/PeriodicHousekeepingHelper.cpp +++ b/housekeeping/PeriodicHousekeepingHelper.cpp @@ -6,11 +6,8 @@ PeriodicHousekeepingHelper::PeriodicHousekeepingHelper( LocalPoolDataSetBase* owner): owner(owner) {} - void PeriodicHousekeepingHelper::initialize(float collectionInterval, - dur_millis_t minimumPeriodicInterval, bool isDiagnostics, - uint8_t nonDiagIntervalFactor) { - this->isDiagnostics = isDiagnostics; + dur_millis_t minimumPeriodicInterval, uint8_t nonDiagIntervalFactor) { this->minimumPeriodicInterval = minimumPeriodicInterval; this->nonDiagIntervalFactor = nonDiagIntervalFactor; collectionIntervalTicks = intervalSecondsToIntervalTicks(collectionInterval); @@ -34,6 +31,11 @@ bool PeriodicHousekeepingHelper::checkOpNecessary() { uint32_t PeriodicHousekeepingHelper::intervalSecondsToIntervalTicks( float collectionIntervalSeconds) { + if(owner == nullptr) { + return 0; + } + bool isDiagnostics = owner->isDiagnostics(); + /* Avoid division by zero */ if(minimumPeriodicInterval == 0) { if(isDiagnostics) { @@ -85,3 +87,4 @@ void PeriodicHousekeepingHelper::changeCollectionInterval( float newIntervalSeconds) { collectionIntervalTicks = intervalSecondsToIntervalTicks(newIntervalSeconds); } + diff --git a/housekeeping/PeriodicHousekeepingHelper.h b/housekeeping/PeriodicHousekeepingHelper.h index 3e8d030e..3256fbff 100644 --- a/housekeeping/PeriodicHousekeepingHelper.h +++ b/housekeeping/PeriodicHousekeepingHelper.h @@ -10,16 +10,15 @@ class PeriodicHousekeepingHelper { public: PeriodicHousekeepingHelper(LocalPoolDataSetBase* owner); - void initialize(float collectionInterval, - dur_millis_t minimumPeriodicInterval, bool isDiagnostics, - uint8_t nonDiagIntervalFactor); + void initialize(float collectionInterval, dur_millis_t minimumPeriodicInterval, + uint8_t nonDiagIntervalFactor); void changeCollectionInterval(float newInterval); float getCollectionIntervalInSeconds() const; bool checkOpNecessary(); + private: LocalPoolDataSetBase* owner = nullptr; - bool isDiagnostics = true; uint8_t nonDiagIntervalFactor = 0; uint32_t intervalSecondsToIntervalTicks(float collectionIntervalSeconds); diff --git a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp index 2d7294ef..c152078d 100644 --- a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp +++ b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp @@ -229,6 +229,7 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { /* Now HK packet should be sent as message immediately. */ REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); LocalPoolDataSetBase* setHandle = poolOwner->getDataSetHandle(lpool::testSid); REQUIRE(setHandle != nullptr); @@ -236,6 +237,7 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { setHandle, false) == retval::CATCH_OK); REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); CHECK(setHandle->getReportingEnabled() == true); CommandMessage hkCmd; @@ -244,17 +246,19 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { CHECK(setHandle->getReportingEnabled() == false); REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); + HousekeepingMessage::setToggleReportingCommand(&hkCmd, lpool::testSid, true, false); CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); CHECK(setHandle->getReportingEnabled() == true); REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); - CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); HousekeepingMessage::setToggleReportingCommand(&hkCmd, lpool::testSid, false, false); CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); CHECK(setHandle->getReportingEnabled() == false); REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); - CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); HousekeepingMessage::setCollectionIntervalModificationCommand(&hkCmd, lpool::testSid, 0.4, false); @@ -264,6 +268,7 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { CHECK(poolOwner->dataset.getCollectionInterval() == 1.0); REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); HousekeepingMessage::setStructureReportingCommand(&hkCmd, lpool::testSid, false); REQUIRE(poolOwner->poolManager.performHkOperation() == retval::CATCH_OK); @@ -271,11 +276,13 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { /* Now HK packet should be sent as message. */ REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); HousekeepingMessage::setOneShotReportCommand(&hkCmd, lpool::testSid, false); CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); HousekeepingMessage::setUpdateNotificationSetCommand(&hkCmd, lpool::testSid); sid_t sidToCheck; @@ -283,6 +290,46 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); CHECK(poolOwner->changedDataSetCallbackWasCalled(sidToCheck, storeId) == true); CHECK(sidToCheck == lpool::testSid); + + /* Now we test the handling is the dataset is set to diagnostic */ + poolOwner->dataset.setDiagnostic(true); + + HousekeepingMessage::setStructureReportingCommand(&hkCmd, lpool::testSid, false); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == + static_cast(LocalDataPoolManager::WRONG_HK_PACKET_TYPE)); + /* We still expect a failure message being sent */ + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); + + HousekeepingMessage::setCollectionIntervalModificationCommand(&hkCmd, + lpool::testSid, 0.4, false); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == + static_cast(LocalDataPoolManager::WRONG_HK_PACKET_TYPE)); + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); + + HousekeepingMessage::setStructureReportingCommand(&hkCmd, lpool::testSid, false); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == + static_cast(LocalDataPoolManager::WRONG_HK_PACKET_TYPE)); + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); + + HousekeepingMessage::setStructureReportingCommand(&hkCmd, lpool::testSid, true); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); + + HousekeepingMessage::setCollectionIntervalModificationCommand(&hkCmd, lpool::testSid, 0.4, + true); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); + } /* we need to reset the subscription list because the pool owner diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.cpp b/unittest/tests/datapoollocal/LocalPoolOwnerBase.cpp index 0e2703a7..7ccceb06 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.cpp +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.cpp @@ -6,6 +6,10 @@ LocalPoolOwnerBase::LocalPoolOwnerBase(object_id_t objectId): messageQueue = new MessageQueueMockBase(); } +LocalPoolOwnerBase::~LocalPoolOwnerBase() { + QueueFactory::instance()->deleteMessageQueue(messageQueue); +} + ReturnValue_t LocalPoolOwnerBase::initializeHkManager() { if(not initialized) { initialized = true; @@ -132,3 +136,4 @@ void LocalPoolOwnerBase::handleChangedPoolVariable(gp_id_t globPoolId, store_add this->changedPoolVariableGpid = globPoolId; this->storeIdForChangedVariable = storeId; } + diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h index f08b9eb9..c045b6d3 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h @@ -54,13 +54,7 @@ private: class LocalPoolTestDataSet: public LocalDataSet { public: LocalPoolTestDataSet(): - LocalDataSet(lpool::testSid, lpool::dataSetMaxVariables), - localPoolVarUint8(lpool::uint8VarGpid, this), - localPoolVarFloat(lpool::floatVarGpid, this), - localPoolUint16Vec(lpool::uint16Vec3Gpid, this) - - { - } + LocalDataSet(lpool::testSid, lpool::dataSetMaxVariables) {} LocalPoolTestDataSet(HasLocalDataPoolIF* owner, uint32_t setId): LocalDataSet(owner, setId, lpool::dataSetMaxVariables) { @@ -70,6 +64,9 @@ public: lp_var_t localPoolVarFloat = lp_var_t(lpool::floatVarGpid, this); lp_vec_t localPoolUint16Vec = lp_vec_t(lpool::uint16Vec3Gpid, this); + void setDiagnostic(bool isDiagnostic) { + LocalPoolDataSetBase::setDiagnostic(isDiagnostic); + } private: }; @@ -78,9 +75,7 @@ class LocalPoolOwnerBase: public SystemObject, public HasLocalDataPoolIF { public: LocalPoolOwnerBase(object_id_t objectId = objects::TEST_LOCAL_POOL_OWNER_BASE); - ~LocalPoolOwnerBase() { - QueueFactory::instance()->deleteMessageQueue(messageQueue); - } + ~LocalPoolOwnerBase(); object_id_t getObjectId() const override { return SystemObject::getObjectId(); diff --git a/unittest/tests/mocks/MessageQueueMockBase.h b/unittest/tests/mocks/MessageQueueMockBase.h index 31146d34..3000f7fb 100644 --- a/unittest/tests/mocks/MessageQueueMockBase.h +++ b/unittest/tests/mocks/MessageQueueMockBase.h @@ -29,6 +29,16 @@ public: return tempMessageSent; } + /** + * Pop a message, clearing it in the process. + * @return + */ + ReturnValue_t popMessage() { + CommandMessage message; + message.clear(); + return receiveMessage(&message); + } + virtual ReturnValue_t reply( MessageQueueMessageIF* message ) { return sendMessage(myQueueId, message); }; @@ -36,6 +46,7 @@ public: MessageQueueId_t *receivedFrom) { return receiveMessage(message); } + virtual ReturnValue_t receiveMessage(MessageQueueMessageIF* message) { if(messagesSentQueue.empty()) { return MessageQueueIF::EMPTY; From 8b83de6ca9fbd9b574a5a178a4b884267527bacc Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 11 Mar 2021 12:33:26 +0100 Subject: [PATCH 32/37] added way to automatically clear unhandled messages --- datapoollocal/HasLocalDataPoolIF.h | 23 +++++++----- datapoollocal/LocalDataPoolManager.cpp | 19 +++++++--- housekeeping/HousekeepingMessage.cpp | 3 +- .../datapoollocal/LocalPoolManagerTest.cpp | 36 ++++++++++++++++++- .../datapoollocal/LocalPoolOwnerBase.cpp | 6 ++-- .../tests/datapoollocal/LocalPoolOwnerBase.h | 5 +-- 6 files changed, 73 insertions(+), 19 deletions(-) diff --git a/datapoollocal/HasLocalDataPoolIF.h b/datapoollocal/HasLocalDataPoolIF.h index ec25f082..54856fac 100644 --- a/datapoollocal/HasLocalDataPoolIF.h +++ b/datapoollocal/HasLocalDataPoolIF.h @@ -72,12 +72,15 @@ public: * notification is received. * @details HasLocalDataPoolIF * Can be overriden by the child class to handle changed datasets. - * @param sid - * @param storeId If a snapshot was requested, data will be located inside + * @param sid SID of the updated set + * @param storeId If a snapshot was requested, data will be located inside * the IPC store with this store ID. + * @param clearMessage If this is set to true, the pool manager will take care of + * clearing the store automatically */ virtual void handleChangedDataset(sid_t sid, - store_address_t storeId = storeId::INVALID_STORE_ADDRESS) { + store_address_t storeId = storeId::INVALID_STORE_ADDRESS, + bool* clearMessage = nullptr) { return; } @@ -85,13 +88,17 @@ public: * @brief This function will be called by the manager if an update * notification is received. * @details - * Can be overriden by the child class to handle changed pool IDs. - * @param sid - * @param storeId If a snapshot was requested, data will be located inside + * Can be overriden by the child class to handle changed pool variables. + * @param gpid GPID of the updated variable. + * @param storeId If a snapshot was requested, data will be located inside * the IPC store with this store ID. + * @param clearMessage Relevant for snapshots. If the boolean this points to is set to true, + * the pool manager will take care of clearing the store automatically + * after the callback. */ - virtual void handleChangedPoolVariable(gp_id_t globPoolId, - store_address_t storeId = storeId::INVALID_STORE_ADDRESS) { + virtual void handleChangedPoolVariable(gp_id_t gpid, + store_address_t storeId = storeId::INVALID_STORE_ADDRESS, + bool* clearMessage = nullptr) { return; } diff --git a/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index cad54094..e9bb57c4 100644 --- a/datapoollocal/LocalDataPoolManager.cpp +++ b/datapoollocal/LocalDataPoolManager.cpp @@ -554,14 +554,15 @@ ReturnValue_t LocalDataPoolManager::handleHousekeepingMessage( case(HousekeepingMessage::GENERATE_ONE_PARAMETER_REPORT): case(HousekeepingMessage::GENERATE_ONE_DIAGNOSTICS_REPORT): { LocalPoolDataSetBase* dataSet =HasLocalDpIFManagerAttorney::getDataSetHandle(owner, sid); - //LocalPoolDataSetBase* dataSet = owner->getDataSetHandle(sid); if(command == HousekeepingMessage::GENERATE_ONE_PARAMETER_REPORT and LocalPoolDataSetAttorney::isDiagnostics(*dataSet)) { - return WRONG_HK_PACKET_TYPE; + result = WRONG_HK_PACKET_TYPE; + break; } else if(command == HousekeepingMessage::GENERATE_ONE_DIAGNOSTICS_REPORT and not LocalPoolDataSetAttorney::isDiagnostics(*dataSet)) { - return WRONG_HK_PACKET_TYPE; + result = WRONG_HK_PACKET_TYPE; + break; } return generateHousekeepingPacket(HousekeepingMessage::getSid(message), dataSet, true); @@ -580,14 +581,22 @@ ReturnValue_t LocalDataPoolManager::handleHousekeepingMessage( case(HousekeepingMessage::UPDATE_SNAPSHOT_SET): { store_address_t storeId; HousekeepingMessage::getUpdateSnapshotSetCommand(message, &storeId); - owner->handleChangedDataset(sid, storeId); + bool clearMessage = true; + owner->handleChangedDataset(sid, storeId, &clearMessage); + if(clearMessage) { + message->clear(); + } return HasReturnvaluesIF::RETURN_OK; } case(HousekeepingMessage::UPDATE_SNAPSHOT_VARIABLE): { store_address_t storeId; gp_id_t globPoolId = HousekeepingMessage::getUpdateSnapshotVariableCommand(message, &storeId); - owner->handleChangedPoolVariable(globPoolId, storeId); + bool clearMessage = true; + owner->handleChangedPoolVariable(globPoolId, storeId, &clearMessage); + if(clearMessage) { + message->clear(); + } return HasReturnvaluesIF::RETURN_OK; } diff --git a/housekeeping/HousekeepingMessage.cpp b/housekeeping/HousekeepingMessage.cpp index 58f8551d..90ca73c8 100644 --- a/housekeeping/HousekeepingMessage.cpp +++ b/housekeeping/HousekeepingMessage.cpp @@ -157,7 +157,8 @@ void HousekeepingMessage::clear(CommandMessage* message) { case(DIAGNOSTICS_REPORT): case(HK_DEFINITIONS_REPORT): case(DIAGNOSTICS_DEFINITION_REPORT): - case(UPDATE_SNAPSHOT_SET): { + case(UPDATE_SNAPSHOT_SET): + case(UPDATE_SNAPSHOT_VARIABLE): { store_address_t storeId; getHkDataReply(message, &storeId); StorageManagerIF *ipcStore = objectManager->get( diff --git a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp index c152078d..88332719 100644 --- a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp +++ b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp @@ -26,8 +26,11 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { CommandMessage messageSent; uint8_t messagesSent = 0; - SECTION("BasicTest") { + { + /* For code coverage, should not crash */ + LocalDataPoolManager manager(nullptr, nullptr); + } auto owner = poolOwner->poolManager.getOwner(); REQUIRE(owner != nullptr); CHECK(owner->getObjectId() == objects::TEST_LOCAL_POOL_OWNER_BASE); @@ -330,6 +333,37 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { CHECK(messagesSent == 1); CHECK(mqMock->popMessage() == retval::CATCH_OK); + HousekeepingMessage::setToggleReportingCommand(&hkCmd, lpool::testSid, true, true); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); + + HousekeepingMessage::setToggleReportingCommand(&hkCmd, lpool::testSid, false, true); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); + + HousekeepingMessage::setOneShotReportCommand(&hkCmd, lpool::testSid, false); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == + static_cast(LocalDataPoolManager::WRONG_HK_PACKET_TYPE)); + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); + + HousekeepingMessage::setOneShotReportCommand(&hkCmd, lpool::testSid, true); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); + + HousekeepingMessage::setUpdateNotificationVariableCommand(&hkCmd, lpool::uint8VarGpid); + gp_id_t gpidToCheck; + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); + CHECK(poolOwner->changedVariableCallbackWasCalled(gpidToCheck, storeId) == true); + CHECK(gpidToCheck == lpool::testSid); + } /* we need to reset the subscription list because the pool owner diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.cpp b/unittest/tests/datapoollocal/LocalPoolOwnerBase.cpp index 7ccceb06..991314a9 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.cpp +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.cpp @@ -106,7 +106,8 @@ bool LocalPoolOwnerBase::changedDataSetCallbackWasCalled(sid_t &sid, store_addre return condition; } -void LocalPoolOwnerBase::handleChangedDataset(sid_t sid, store_address_t storeId) { +void LocalPoolOwnerBase::handleChangedDataset(sid_t sid, store_address_t storeId, + bool* clearMessage) { this->changedDatasetSid = sid; this->storeIdForChangedSet = storeId; } @@ -132,7 +133,8 @@ ReturnValue_t LocalPoolOwnerBase::initializeHkManagerAfterTaskCreation() { } -void LocalPoolOwnerBase::handleChangedPoolVariable(gp_id_t globPoolId, store_address_t storeId) { +void LocalPoolOwnerBase::handleChangedPoolVariable(gp_id_t globPoolId, store_address_t storeId, + bool* clearMessage) { this->changedPoolVariableGpid = globPoolId; this->storeIdForChangedVariable = storeId; } diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h index c045b6d3..8d2073b0 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h @@ -160,11 +160,12 @@ public: LocalPoolTestDataSet dataset; private: - void handleChangedDataset(sid_t sid, store_address_t storeId) override; + void handleChangedDataset(sid_t sid, store_address_t storeId, bool* clearMessage) override; sid_t changedDatasetSid; store_address_t storeIdForChangedSet; - void handleChangedPoolVariable(gp_id_t globPoolId, store_address_t storeId) override; + void handleChangedPoolVariable(gp_id_t globPoolId, store_address_t storeId, + bool* clearMessage) override; gp_id_t changedPoolVariableGpid; store_address_t storeIdForChangedVariable; From e55f74a00ef51047f4a450d1e826ef0e081615e7 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 11 Mar 2021 12:34:25 +0100 Subject: [PATCH 33/37] default auto clearance --- datapoollocal/HasLocalDataPoolIF.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/datapoollocal/HasLocalDataPoolIF.h b/datapoollocal/HasLocalDataPoolIF.h index 54856fac..74e372c9 100644 --- a/datapoollocal/HasLocalDataPoolIF.h +++ b/datapoollocal/HasLocalDataPoolIF.h @@ -81,7 +81,9 @@ public: virtual void handleChangedDataset(sid_t sid, store_address_t storeId = storeId::INVALID_STORE_ADDRESS, bool* clearMessage = nullptr) { - return; + if(clearMessage != nullptr) { + *clearMessage = true; + } } /** @@ -99,7 +101,9 @@ public: virtual void handleChangedPoolVariable(gp_id_t gpid, store_address_t storeId = storeId::INVALID_STORE_ADDRESS, bool* clearMessage = nullptr) { - return; + if(clearMessage != nullptr) { + *clearMessage = true; + } } /** From 33823b445c79c8e273527642f412eba650545b25 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 11 Mar 2021 12:44:35 +0100 Subject: [PATCH 34/37] some more tests added --- datapoollocal/LocalDataPoolManager.cpp | 2 +- datapoollocal/localPoolDefinitions.h | 4 ++-- globalfunctions/arrayprinter.cpp | 4 ++++ .../tests/datapoollocal/LocalPoolManagerTest.cpp | 16 +++++++++++++++- 4 files changed, 22 insertions(+), 4 deletions(-) diff --git a/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index e9bb57c4..94ee9c26 100644 --- a/datapoollocal/LocalDataPoolManager.cpp +++ b/datapoollocal/LocalDataPoolManager.cpp @@ -592,7 +592,7 @@ ReturnValue_t LocalDataPoolManager::handleHousekeepingMessage( store_address_t storeId; gp_id_t globPoolId = HousekeepingMessage::getUpdateSnapshotVariableCommand(message, &storeId); - bool clearMessage = true; + bool clearMessage = true; owner->handleChangedPoolVariable(globPoolId, storeId, &clearMessage); if(clearMessage) { message->clear(); diff --git a/datapoollocal/localPoolDefinitions.h b/datapoollocal/localPoolDefinitions.h index af8ce711..daab8b9c 100644 --- a/datapoollocal/localPoolDefinitions.h +++ b/datapoollocal/localPoolDefinitions.h @@ -96,11 +96,11 @@ union gp_id_t { return raw == INVALID_GPID; } - bool operator==(const sid_t& other) const { + bool operator==(const gp_id_t& other) const { return raw == other.raw; } - bool operator!=(const sid_t& other) const { + bool operator!=(const gp_id_t& other) const { return not (raw == other.raw); } }; diff --git a/globalfunctions/arrayprinter.cpp b/globalfunctions/arrayprinter.cpp index 7dc056b0..20a64f5b 100644 --- a/globalfunctions/arrayprinter.cpp +++ b/globalfunctions/arrayprinter.cpp @@ -67,7 +67,9 @@ void arrayprinter::printHex(const uint8_t *data, size_t size, } } } +#if FSFW_DISABLE_PRINTOUT == 0 printf("[%s]\n", printBuffer); +#endif /* FSFW_DISABLE_PRINTOUT == 0 */ #endif } @@ -108,7 +110,9 @@ void arrayprinter::printDec(const uint8_t *data, size_t size, } } } +#if FSFW_DISABLE_PRINTOUT == 0 printf("[%s]\n", printBuffer); +#endif /* FSFW_DISABLE_PRINTOUT == 0 */ #endif } diff --git a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp index 88332719..a98db610 100644 --- a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp +++ b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp @@ -362,7 +362,21 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { gp_id_t gpidToCheck; CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); CHECK(poolOwner->changedVariableCallbackWasCalled(gpidToCheck, storeId) == true); - CHECK(gpidToCheck == lpool::testSid); + CHECK(gpidToCheck == lpool::uint8VarGpid); + + HousekeepingMessage::setUpdateSnapshotSetCommand(&hkCmd, lpool::testSid, + storeId::INVALID_STORE_ADDRESS); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); + CHECK(poolOwner->changedDataSetCallbackWasCalled(sidToCheck, storeId) == true); + CHECK(sidToCheck == lpool::testSid); + + HousekeepingMessage::setUpdateSnapshotVariableCommand(&hkCmd, lpool::uint8VarGpid, + storeId::INVALID_STORE_ADDRESS); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); + CHECK(poolOwner->changedVariableCallbackWasCalled(gpidToCheck, storeId) == true); + CHECK(gpidToCheck == lpool::uint8VarGpid); + + poolOwner->poolManager.printPoolEntry(lpool::uint8VarId); } From 9602a3ed6a9b451ae9b704ba8a3fbbd6e85c9afe Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 11 Mar 2021 13:02:10 +0100 Subject: [PATCH 35/37] bugfix and a few more tests --- datapool/PoolDataSetBase.cpp | 4 ++++ datapoollocal/SharedLocalDataSet.cpp | 4 ++-- unittest/tests/datapoollocal/DataSetTest.cpp | 20 ++++++++++++++++++-- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/datapool/PoolDataSetBase.cpp b/datapool/PoolDataSetBase.cpp index 73ec4bd7..60f08b88 100644 --- a/datapool/PoolDataSetBase.cpp +++ b/datapool/PoolDataSetBase.cpp @@ -14,6 +14,10 @@ PoolDataSetBase::~PoolDataSetBase() {} ReturnValue_t PoolDataSetBase::registerVariable(PoolVariableIF *variable) { + if(registeredVariables == nullptr) { + /* Underlying container invalid */ + return HasReturnvaluesIF::RETURN_FAILED; + } if (state != States::STATE_SET_UNINITIALISED) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::error << "DataSet::registerVariable: Call made in wrong position." << std::endl; diff --git a/datapoollocal/SharedLocalDataSet.cpp b/datapoollocal/SharedLocalDataSet.cpp index abcf7da9..84c2d1c3 100644 --- a/datapoollocal/SharedLocalDataSet.cpp +++ b/datapoollocal/SharedLocalDataSet.cpp @@ -3,7 +3,7 @@ SharedLocalDataSet::SharedLocalDataSet(object_id_t objectId, sid_t sid, const size_t maxSize): SystemObject(objectId), - LocalPoolDataSetBase(sid, nullptr, maxSize) { + LocalPoolDataSetBase(sid, nullptr, maxSize), poolVarVector(maxSize) { this->setContainer(poolVarVector.data()); datasetLock = MutexFactory::instance()->createMutex(); } @@ -11,7 +11,7 @@ SharedLocalDataSet::SharedLocalDataSet(object_id_t objectId, sid_t sid, SharedLocalDataSet::SharedLocalDataSet(object_id_t objectId, HasLocalDataPoolIF *owner, uint32_t setId, const size_t maxSize): SystemObject(objectId), - LocalPoolDataSetBase(owner, setId, nullptr, maxSize) { + LocalPoolDataSetBase(owner, setId, nullptr, maxSize), poolVarVector(maxSize) { this->setContainer(poolVarVector.data()); datasetLock = MutexFactory::instance()->createMutex(); } diff --git a/unittest/tests/datapoollocal/DataSetTest.cpp b/unittest/tests/datapoollocal/DataSetTest.cpp index 06964d26..ce4d9d43 100644 --- a/unittest/tests/datapoollocal/DataSetTest.cpp +++ b/unittest/tests/datapoollocal/DataSetTest.cpp @@ -255,11 +255,27 @@ TEST_CASE("DataSetTest" , "[DataSetTest]") { SECTION("SharedDataSet") { object_id_t sharedSetId = objects::SHARED_SET_ID; - SharedLocalDataSet sharedSet(sharedSetId, poolOwner, 2, 5); + SharedLocalDataSet sharedSet(sharedSetId, poolOwner, lpool::testSetId, 5); + localSet.localPoolVarUint8.setReadWriteMode(pool_rwm_t::VAR_WRITE); + localSet.localPoolUint16Vec.setReadWriteMode(pool_rwm_t::VAR_WRITE); + CHECK(sharedSet.registerVariable(&localSet.localPoolVarUint8) == retval::CATCH_OK); + CHECK(sharedSet.registerVariable(&localSet.localPoolUint16Vec) == retval::CATCH_OK); CHECK(sharedSet.initialize() == retval::CATCH_OK); CHECK(sharedSet.lockDataset() == retval::CATCH_OK); - CHECK(sharedSet.unlockDataset() == retval::CATCH_OK); + + { + //PoolReadGuard rg(&sharedSet); + //CHECK(rg.getReadResult() == retval::CATCH_OK); + localSet.localPoolVarUint8.value = 5; + localSet.localPoolUint16Vec.value[0] = 1; + localSet.localPoolUint16Vec.value[1] = 2; + localSet.localPoolUint16Vec.value[2] = 3; + CHECK(sharedSet.commit() == retval::CATCH_OK); + } + + sharedSet.setReadCommitProtectionBehaviour(true); + } /* we need to reset the subscription list because the pool owner From c527391b106d49b0d4d72590fb6c88fb0ac03e34 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 11 Mar 2021 13:14:49 +0100 Subject: [PATCH 36/37] removed unfisnihed stuff from PR --- osal/windows/CMakeLists.txt | 1 - osal/windows/TcWinTcpServer.cpp | 177 -------------------------------- osal/windows/TcWinTcpServer.h | 56 ---------- 3 files changed, 234 deletions(-) delete mode 100644 osal/windows/TcWinTcpServer.cpp delete mode 100644 osal/windows/TcWinTcpServer.h diff --git a/osal/windows/CMakeLists.txt b/osal/windows/CMakeLists.txt index 889ea339..a3a719e0 100644 --- a/osal/windows/CMakeLists.txt +++ b/osal/windows/CMakeLists.txt @@ -1,7 +1,6 @@ target_sources(${LIB_FSFW_NAME} PRIVATE TcWinUdpPollingTask.cpp TmTcWinUdpBridge.cpp - TcWinTcpServer.cpp ) target_link_libraries(${LIB_FSFW_NAME} PRIVATE diff --git a/osal/windows/TcWinTcpServer.cpp b/osal/windows/TcWinTcpServer.cpp deleted file mode 100644 index f85147f0..00000000 --- a/osal/windows/TcWinTcpServer.cpp +++ /dev/null @@ -1,177 +0,0 @@ -#include "TcWinTcpServer.h" -#include "../../serviceinterface/ServiceInterface.h" - -#include -#include - -const std::string TcWinTcpServer::DEFAULT_TCP_SERVER_PORT = "7301"; -const std::string TcWinTcpServer::DEFAULT_TCP_CLIENT_PORT = "7302"; - -TcWinTcpServer::TcWinTcpServer(object_id_t objectId, object_id_t tmtcUnixUdpBridge, - std::string customTcpServerPort): - SystemObject(objectId), tcpPort(customTcpServerPort) { - if(tcpPort == "") { - tcpPort = DEFAULT_TCP_SERVER_PORT; - } -} - -ReturnValue_t TcWinTcpServer::initialize() { - int retval = 0; - struct addrinfo *addrResult = nullptr; - struct addrinfo hints; - /* Initiates Winsock DLL. */ - WSAData wsaData; - WORD wVersionRequested = MAKEWORD(2, 2); - int err = WSAStartup(wVersionRequested, &wsaData); - if (err != 0) { - /* Tell the user that we could not find a usable Winsock DLL. */ -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "TmTcWinUdpBridge::TmTcWinUdpBridge: WSAStartup failed with error: " << - err << std::endl; -#endif - return HasReturnvaluesIF::RETURN_FAILED; - } - - ZeroMemory(&hints, sizeof (hints)); - hints.ai_family = AF_INET; - hints.ai_socktype = SOCK_STREAM; - hints.ai_protocol = IPPROTO_TCP; - hints.ai_flags = AI_PASSIVE; - - retval = getaddrinfo(nullptr, tcpPort.c_str(), &hints, &addrResult); - if (retval != 0) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "TcWinTcpServer::TcWinTcpServer: Retrieving address info failed!" << - std::endl; -#endif - handleError(ErrorSources::GETADDRINFO_CALL); - return HasReturnvaluesIF::RETURN_FAILED; - } - - /* Open TCP (stream) socket */ - listenerTcpSocket = socket(addrResult->ai_family, addrResult->ai_socktype, - addrResult->ai_protocol); - if(listenerTcpSocket == INVALID_SOCKET) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "TcWinTcpServer::TcWinTcpServer: Socket creation failed!" << std::endl; -#endif - freeaddrinfo(addrResult); - handleError(ErrorSources::SOCKET_CALL); - return HasReturnvaluesIF::RETURN_FAILED; - } - -// retval = setsockopt(listenerTcpSocket, SOL_SOCKET, SO_REUSEADDR | SO_BROADCAST, -// reinterpret_cast(&tcpSockOpt), sizeof(tcpSockOpt)); -// if(retval != 0) { -// sif::warning << "TcWinTcpServer::TcWinTcpServer: Setting socket options failed!" << -// std::endl; -// handleError(ErrorSources::SETSOCKOPT_CALL); -// return HasReturnvaluesIF::RETURN_FAILED; -// } -// tcpAddress.sin_family = AF_INET; -// tcpAddress.sin_addr.s_addr = htonl(INADDR_ANY); - - retval = bind(listenerTcpSocket, reinterpret_cast(&tcpAddress), - tcpAddrLen); - if(retval == SOCKET_ERROR) { - sif::warning << "TcWinTcpServer::TcWinTcpServer: Binding socket failed!" << - std::endl; - freeaddrinfo(addrResult); - handleError(ErrorSources::BIND_CALL); - } - - freeaddrinfo(addrResult); - return HasReturnvaluesIF::RETURN_OK; -} - - -TcWinTcpServer::~TcWinTcpServer() { - closesocket(listenerTcpSocket); - WSACleanup(); -} - -ReturnValue_t TcWinTcpServer::performOperation(uint8_t opCode) { - /* If a connection is accepted, the corresponding socket will be assigned to the new socket */ - SOCKET clientSocket; - sockaddr_in clientSockAddr; - int connectorSockAddrLen = 0; - int retval = 0; - /* Listen for connection requests permanently for lifetime of program */ - while(true) { - retval = listen(listenerTcpSocket, currentBacklog); - if(retval == SOCKET_ERROR) { - handleError(ErrorSources::LISTEN_CALL); - continue; - } - - clientSocket = accept(listenerTcpSocket, reinterpret_cast(&clientSockAddr), - &connectorSockAddrLen); - - if(clientSocket == INVALID_SOCKET) { - handleError(ErrorSources::ACCEPT_CALL); - continue; - }; - - retval = recv(clientSocket, reinterpret_cast(receptionBuffer.data()), - receptionBuffer.size(), 0); - if(retval > 0) { -#if FSFW_TCP_SERVER_WIRETAPPING_ENABLED == 1 - sif::info << "TcWinTcpServer::performOperation: Received " << retval << " bytes." - std::endl; -#endif - } - else if(retval == 0) { - - } - else { - - } - - /* Done, shut down connection */ - retval = shutdown(clientSocket, SD_SEND); - } - return HasReturnvaluesIF::RETURN_OK; -} - -void TcWinTcpServer::handleError(ErrorSources errorSrc) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - int errCode = WSAGetLastError(); - std::string errorSrcString; - if(errorSrc == ErrorSources::SETSOCKOPT_CALL) { - errorSrcString = "setsockopt call"; - } - else if(errorSrc == ErrorSources::SOCKET_CALL) { - errorSrcString = "socket call"; - } - else if(errorSrc == ErrorSources::LISTEN_CALL) { - errorSrcString = "listen call"; - } - else if(errorSrc == ErrorSources::ACCEPT_CALL) { - errorSrcString = "accept call"; - } - else if(errorSrc == ErrorSources::GETADDRINFO_CALL) { - errorSrcString = "getaddrinfo call"; - } - - switch(errCode) { - case(WSANOTINITIALISED): { - sif::warning << "TmTcWinUdpBridge::handleError: " << errorSrcString << " | " - "WSANOTINITIALISED: WSAStartup call necessary" << std::endl; - break; - } - case(WSAEINVAL): { - sif::warning << "TmTcWinUdpBridge::handleError: " << errorSrcString << " | " - "WSAEINVAL: Invalid parameters" << std::endl; - break; - } - default: { - /* - https://docs.microsoft.com/en-us/windows/win32/winsock/ - windows-sockets-error-codes-2 - */ - sif::warning << "TmTcWinUdpBridge::handleSocketError: Error code: " << errCode << std::endl; - break; - } - } -#endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */ -} diff --git a/osal/windows/TcWinTcpServer.h b/osal/windows/TcWinTcpServer.h deleted file mode 100644 index f8aebc53..00000000 --- a/osal/windows/TcWinTcpServer.h +++ /dev/null @@ -1,56 +0,0 @@ -#ifndef FSFW_OSAL_WINDOWS_TCWINTCPSERVER_H_ -#define FSFW_OSAL_WINDOWS_TCWINTCPSERVER_H_ - -#include "../../objectmanager/SystemObject.h" -#include "../../tasks/ExecutableObjectIF.h" - -#include -#include - -//! Debugging preprocessor define. -#define FSFW_TCP_SERVER_WIRETAPPING_ENABLED 0 - -/** - * @brief Windows TCP server used to receive telecommands on a Windows Host - * @details - * Based on: https://docs.microsoft.com/en-us/windows/win32/winsock/complete-server-code - */ -class TcWinTcpServer: - public SystemObject, - public ExecutableObjectIF { -public: - /* The ports chosen here should not be used by any other process. */ - static const std::string DEFAULT_TCP_SERVER_PORT; - static const std::string DEFAULT_TCP_CLIENT_PORT; - - TcWinTcpServer(object_id_t objectId, object_id_t tmtcUnixUdpBridge, - std::string customTcpServerPort = ""); - virtual~ TcWinTcpServer(); - - ReturnValue_t initialize() override; - ReturnValue_t performOperation(uint8_t opCode) override; - -private: - - std::string tcpPort; - SOCKET listenerTcpSocket = 0; - struct sockaddr_in tcpAddress; - int tcpAddrLen = sizeof(tcpAddress); - int currentBacklog = 3; - - std::vector receptionBuffer; - int tcpSockOpt = 0; - - enum class ErrorSources { - GETADDRINFO_CALL, - SOCKET_CALL, - SETSOCKOPT_CALL, - BIND_CALL, - LISTEN_CALL, - ACCEPT_CALL - }; - - void handleError(ErrorSources errorSrc); -}; - -#endif /* FSFW_OSAL_WINDOWS_TCWINTCPSERVER_H_ */ From 36e524abe3217e7f2a31986d3f59dfb98448ecfa Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 11 Mar 2021 14:46:22 +0100 Subject: [PATCH 37/37] more tests --- housekeeping/HousekeepingSnapshot.h | 18 +++++++-- .../datapoollocal/LocalPoolManagerTest.cpp | 40 ++++++++++++++++++- 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/housekeeping/HousekeepingSnapshot.h b/housekeeping/HousekeepingSnapshot.h index 50afd4af..45b40d37 100644 --- a/housekeeping/HousekeepingSnapshot.h +++ b/housekeeping/HousekeepingSnapshot.h @@ -11,7 +11,8 @@ * @brief This helper class will be used to serialize and deserialize update housekeeping packets * into the store. */ -class HousekeepingSnapshot: public SerializeIF { +class HousekeepingSnapshot: + public SerializeIF { public: /** @@ -36,6 +37,17 @@ public: timeStamp(timeStamp), timeStampSize(timeStampSize), updateData(dataSetPtr) {}; + /** + * Update packet constructor for pool variables. + * @param timeStamp + * @param timeStampSize + * @param dataSetPtr + */ + HousekeepingSnapshot(CCSDSTime::CDS_short* cdsShort, LocalPoolObjectBase* dataSetPtr): + timeStamp(reinterpret_cast(cdsShort)), + timeStampSize(sizeof(CCSDSTime::CDS_short)), updateData(dataSetPtr) {}; + + /** * Update packet constructor for pool variables. * @param timeStamp @@ -47,8 +59,8 @@ public: timeStamp(timeStamp), timeStampSize(timeStampSize), updateData(dataSetPtr) {}; - virtual ReturnValue_t serialize(uint8_t **buffer, size_t *size, - size_t maxSize, Endianness streamEndianness) const { + virtual ReturnValue_t serialize(uint8_t **buffer, size_t *size, size_t maxSize, + Endianness streamEndianness) const { if(timeStamp != nullptr) { /* Endianness will always be MACHINE, so we can simply use memcpy here. */ diff --git a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp index a98db610..4df735cd 100644 --- a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp +++ b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp @@ -20,7 +20,7 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { REQUIRE(poolOwner->initializeHkManager() == retval::CATCH_OK); REQUIRE(poolOwner->initializeHkManagerAfterTaskCreation() == retval::CATCH_OK); - //REQUIRE(poolOwner->dataset.assignPointers() == retval::CATCH_OK); + MessageQueueMockBase* mqMock = poolOwner->getMockQueueHandle(); REQUIRE(mqMock != nullptr); CommandMessage messageSent; @@ -154,7 +154,21 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { auto poolVar = dynamic_cast*>( poolOwner->getPoolObjectHandle(lpool::uint8VarId)); REQUIRE(poolVar != nullptr); + + { + PoolReadGuard rg(poolVar); + CHECK(rg.getReadResult() == retval::CATCH_OK); + poolVar->value = 25; + } + poolVar->setChanged(true); + + /* Store current time, we are going to check the (approximate) time equality later */ + CCSDSTime::CDS_short timeCdsNow; + timeval now; + Clock::getClock_timeval(&now); + CCSDSTime::convertToCcsds(&timeCdsNow, &now); + REQUIRE(poolOwner->poolManager.performHkOperation() == retval::CATCH_OK); /* Check update snapshot was sent. */ @@ -166,6 +180,30 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { REQUIRE(mqMock->receiveMessage(&messageSent) == retval::CATCH_OK); CHECK(messageSent.getCommand() == static_cast( HousekeepingMessage::UPDATE_SNAPSHOT_VARIABLE)); + /* Now we deserialize the snapshot into a new dataset instance */ + CCSDSTime::CDS_short cdsShort; + lp_var_t varCopy = lp_var_t(lpool::uint8VarGpid); + HousekeepingSnapshot snapshot(&cdsShort, &varCopy); + store_address_t storeId; + HousekeepingMessage::getUpdateSnapshotVariableCommand(&messageSent, &storeId); + ConstAccessorPair accessorPair = tglob::getIpcStoreHandle()->getData(storeId); + REQUIRE(accessorPair.first == retval::CATCH_OK); + const uint8_t* readOnlyPtr = accessorPair.second.data(); + size_t sizeToDeserialize = accessorPair.second.size(); + CHECK(varCopy.value == 0); + /* Fill the dataset and timestamp */ + REQUIRE(snapshot.deSerialize(&readOnlyPtr, &sizeToDeserialize, + SerializeIF::Endianness::MACHINE) == retval::CATCH_OK); + CHECK(varCopy.value == 25); + + /* Now we check that both times are equal */ + CHECK(cdsShort.pField == timeCdsNow.pField); + CHECK(cdsShort.dayLSB == Catch::Approx(timeCdsNow.dayLSB).margin(1)); + CHECK(cdsShort.dayMSB == Catch::Approx(timeCdsNow.dayMSB).margin(1)); + CHECK(cdsShort.msDay_h == Catch::Approx(timeCdsNow.msDay_h).margin(1)); + CHECK(cdsShort.msDay_hh == Catch::Approx(timeCdsNow.msDay_hh).margin(1)); + CHECK(cdsShort.msDay_l == Catch::Approx(timeCdsNow.msDay_l).margin(1)); + CHECK(cdsShort.msDay_ll == Catch::Approx(timeCdsNow.msDay_ll).margin(1)); } SECTION("VariableUpdateTest") {