Skip to content

Commit

Permalink
Implements #2723: Adds Os::Console to replace Os::Log (#2831)
Browse files Browse the repository at this point in the history
* Initial logging refactor - WIP

* Removing LogAssert as it is not used

* Working on Fw::Logger rewrite

* Initial Os::Console refactor

* Fixing Fw::Logger unit tests

* Fixing up UT build

* Adding Os::Stub::Console implementation

* Added interface UTs

* Adding posix console tests

* Fixing Linux errors

* Spelling

* Initial review requests

* Fixing RPI build issues

* Fixing codeql errors

* Fixing code QL

* Validating 'message' parameter
  • Loading branch information
LeStarch authored Aug 16, 2024
1 parent 63d5bf0 commit 7159e7e
Show file tree
Hide file tree
Showing 84 changed files with 1,350 additions and 869 deletions.
3 changes: 2 additions & 1 deletion Drv/Ip/IpSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,12 @@ IpSocket::IpSocket() : m_fd(-1), m_timeoutSeconds(0), m_timeoutMicroseconds(0),
SocketIpStatus IpSocket::configure(const char* const hostname, const U16 port, const U32 timeout_seconds, const U32 timeout_microseconds) {
FW_ASSERT(timeout_microseconds < 1000000, static_cast<FwAssertArgType>(timeout_microseconds));
FW_ASSERT(this->isValidPort(port));
FW_ASSERT(hostname != nullptr);
this->m_lock.lock();
this->m_timeoutSeconds = timeout_seconds;
this->m_timeoutMicroseconds = timeout_microseconds;
this->m_port = port;
(void) Fw::StringUtils::string_copy(this->m_hostname, hostname, SOCKET_MAX_HOSTNAME_SIZE);
(void) Fw::StringUtils::string_copy(this->m_hostname, hostname, static_cast<FwSizeType>(SOCKET_MAX_HOSTNAME_SIZE));
this->m_lock.unlock();
return SOCK_SUCCESS;
}
Expand Down
6 changes: 3 additions & 3 deletions Drv/Ip/SocketReadTask.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ void SocketReadTask::readTask(void* pointer) {
// Open a network connection if it has not already been open
if ((not self->getSocketHandler().isStarted()) and (not self->m_stop) and
((status = self->startup()) != SOCK_SUCCESS)) {
Fw::Logger::logMsg(
Fw::Logger::log(
"[WARNING] Failed to open port with status %d and errno %d\n",
static_cast<POINTER_CAST>(status),
static_cast<POINTER_CAST>(errno));
Expand All @@ -86,7 +86,7 @@ void SocketReadTask::readTask(void* pointer) {
// Open a network connection if it has not already been open
if ((not self->getSocketHandler().isOpened()) and (not self->m_stop) and
((status = self->open()) != SOCK_SUCCESS)) {
Fw::Logger::logMsg(
Fw::Logger::log(
"[WARNING] Failed to open port with status %d and errno %d\n",
static_cast<POINTER_CAST>(status),
static_cast<POINTER_CAST>(errno));
Expand All @@ -102,7 +102,7 @@ void SocketReadTask::readTask(void* pointer) {
U32 size = buffer.getSize();
status = self->getSocketHandler().recv(data, size);
if ((status != SOCK_SUCCESS) && (status != SOCK_INTERRUPTED_TRY_AGAIN)) {
Fw::Logger::logMsg("[WARNING] Failed to recv from port with status %d and errno %d\n",
Fw::Logger::log("[WARNING] Failed to recv from port with status %d and errno %d\n",
static_cast<POINTER_CAST>(status),
static_cast<POINTER_CAST>(errno));
self->getSocketHandler().close();
Expand Down
2 changes: 1 addition & 1 deletion Drv/Ip/TcpClientSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ SocketIpStatus TcpClientSocket::openProtocol(NATIVE_INT_TYPE& fd) {
return SOCK_FAILED_TO_CONNECT;
}
fd = socketFd;
Fw::Logger::logMsg("Connected to %s:%hu as a tcp client\n", reinterpret_cast<POINTER_CAST>(m_hostname), m_port);
Fw::Logger::log("Connected to %s:%hu as a tcp client\n", m_hostname, m_port);
return SOCK_SUCCESS;
}

Expand Down
4 changes: 2 additions & 2 deletions Drv/Ip/TcpServerSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ SocketIpStatus TcpServerSocket::startup() {
return SOCK_FAILED_TO_READ_BACK_PORT;
}
U16 port = ntohs(address.sin_port);
Fw::Logger::logMsg("Listening for single client at %s:%hu\n", reinterpret_cast<POINTER_CAST>(m_hostname), port);
Fw::Logger::log("Listening for single client at %s:%hu\n", m_hostname, port);
// TCP requires listening on the socket. Since we only expect a single client, set the TCP backlog (second argument) to 1 to prevent queuing of multiple clients.
if (::listen(serverFd, 1) < 0) {
::close(serverFd);
Expand Down Expand Up @@ -133,7 +133,7 @@ SocketIpStatus TcpServerSocket::openProtocol(NATIVE_INT_TYPE& fd) {
return SOCK_FAILED_TO_SET_SOCKET_OPTIONS;
}

Fw::Logger::logMsg("Accepted client at %s:%hu\n", reinterpret_cast<POINTER_CAST>(m_hostname), m_port);
Fw::Logger::log("Accepted client at %s:%hu\n", m_hostname, m_port);
fd = clientFd;
return SOCK_SUCCESS;
}
Expand Down
12 changes: 7 additions & 5 deletions Drv/Ip/UdpSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,16 @@ UdpSocket::~UdpSocket() {
SocketIpStatus UdpSocket::configureSend(const char* const hostname, const U16 port, const U32 timeout_seconds, const U32 timeout_microseconds) {
//Timeout is for the send, so configure send will work with the base class
FW_ASSERT(port != 0, port); // Send cannot be on port 0
FW_ASSERT(hostname != nullptr);
return this->IpSocket::configure(hostname, port, timeout_seconds, timeout_microseconds);
}

SocketIpStatus UdpSocket::configureRecv(const char* hostname, const U16 port) {
FW_ASSERT(this->isValidPort(port));
FW_ASSERT(hostname != nullptr);
this->m_lock.lock();
this->m_recv_port = port;
(void) Fw::StringUtils::string_copy(this->m_recv_hostname, hostname, SOCKET_MAX_HOSTNAME_SIZE);
(void) Fw::StringUtils::string_copy(this->m_recv_hostname, hostname, static_cast<FwSizeType>(SOCKET_MAX_HOSTNAME_SIZE));
this->m_lock.unlock();
return SOCK_SUCCESS;
}
Expand Down Expand Up @@ -175,13 +177,13 @@ SocketIpStatus UdpSocket::openProtocol(NATIVE_INT_TYPE& fd) {
this->m_lock.unlock();
// Log message for UDP
if (port == 0) {
Fw::Logger::logMsg("Setup to receive udp at %s:%hu\n", reinterpret_cast<POINTER_CAST>(m_recv_hostname),
Fw::Logger::log("Setup to receive udp at %s:%hu\n", m_recv_hostname,
recv_port);
} else {
Fw::Logger::logMsg("Setup to receive udp at %s:%hu and send to %s:%hu\n",
reinterpret_cast<POINTER_CAST>(m_recv_hostname),
Fw::Logger::log("Setup to receive udp at %s:%hu and send to %s:%hu\n",
m_recv_hostname,
recv_port,
reinterpret_cast<POINTER_CAST>(m_hostname),
m_hostname,
port);
}
FW_ASSERT(status == SOCK_SUCCESS, status);
Expand Down
4 changes: 2 additions & 2 deletions Drv/Ip/test/ut/TestTcp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
#include <Drv/Ip/TcpClientSocket.hpp>
#include <Drv/Ip/TcpServerSocket.hpp>
#include <Drv/Ip/IpSocket.hpp>
#include <Os/Log.hpp>
#include <Os/Console.hpp>
#include <Fw/Logger/Logger.hpp>
#include <Drv/Ip/test/ut/SocketTestHelper.hpp>

Os::Log logger;
Os::Console logger;


void test_with_loop(U32 iterations) {
Expand Down
4 changes: 2 additions & 2 deletions Drv/Ip/test/ut/TestUdp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
#include <gtest/gtest.h>
#include <Drv/Ip/UdpSocket.hpp>
#include <Drv/Ip/IpSocket.hpp>
#include <Os/Log.hpp>
#include <Os/Console.hpp>
#include <Fw/Logger/Logger.hpp>
#include <Drv/Ip/test/ut/PortSelector.hpp>
#include <Drv/Ip/test/ut/SocketTestHelper.hpp>

Os::Log logger;
Os::Console logger;

void test_with_loop(U32 iterations, bool duplex) {
Drv::SocketIpStatus status1 = Drv::SOCK_SUCCESS;
Expand Down
36 changes: 18 additions & 18 deletions Drv/LinuxI2cDriver/LinuxI2cDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,18 +81,18 @@ namespace Drv {
}

#if DEBUG_PRINT
Fw::Logger::logMsg("I2c addr: 0x%02X\n",addr);
Fw::Logger::log("I2c addr: 0x%02X\n",addr);
for (U32 byte = 0; byte < serBuffer.getSize(); byte++) {
Fw::Logger::logMsg("0x%02X ",serBuffer.getData()[byte]);
Fw::Logger::log("0x%02X ",serBuffer.getData()[byte]);

}
Fw::Logger::logMsg("\n");
Fw::Logger::log("\n");
#endif
// select slave address
int stat = ioctl(this->m_fd, I2C_SLAVE, addr);
if (stat == -1) {
#if DEBUG_PRINT
Fw::Logger::logMsg("Status: %d Errno: %d\n", stat, errno);
Fw::Logger::log("Status: %d Errno: %d\n", stat, errno);
#endif
return I2cStatus::I2C_ADDRESS_ERR;
}
Expand All @@ -102,7 +102,7 @@ namespace Drv {
stat = static_cast<int>(write(this->m_fd, serBuffer.getData(), serBuffer.getSize()));
if (stat == -1) {
#if DEBUG_PRINT
Fw::Logger::logMsg("Status: %d Errno: %d\n", stat, errno);
Fw::Logger::log("Status: %d Errno: %d\n", stat, errno);
#endif
return I2cStatus::I2C_WRITE_ERR;
}
Expand All @@ -122,13 +122,13 @@ namespace Drv {
}

#if DEBUG_PRINT
Fw::Logger::logMsg("I2c addr: 0x%02X\n",addr);
Fw::Logger::log("I2c addr: 0x%02X\n",addr);
#endif
// select slave address
int stat = ioctl(this->m_fd, I2C_SLAVE, addr);
if (stat == -1) {
#if DEBUG_PRINT
Fw::Logger::logMsg("Status: %d Errno: %d\n", stat, errno);
Fw::Logger::log("Status: %d Errno: %d\n", stat, errno);
#endif
return I2cStatus::I2C_ADDRESS_ERR;
}
Expand All @@ -138,16 +138,16 @@ namespace Drv {
stat = static_cast<int>(read(this->m_fd, serBuffer.getData(), serBuffer.getSize()));
if (stat == -1) {
#if DEBUG_PRINT
Fw::Logger::logMsg("Status: %d Errno: %d\n", stat, errno);
Fw::Logger::log("Status: %d Errno: %d\n", stat, errno);
#endif
return I2cStatus::I2C_READ_ERR;
}
#if DEBUG_PRINT
for (U32 byte = 0; byte < serBuffer.getSize(); byte++) {
Fw::Logger::logMsg("0x%02X ",serBuffer.getData()[byte]);
Fw::Logger::log("0x%02X ",serBuffer.getData()[byte]);

}
Fw::Logger::logMsg("\n");
Fw::Logger::log("\n");
#endif
return I2cStatus::I2C_OK;
}
Expand All @@ -171,7 +171,7 @@ namespace Drv {
FW_ASSERT(readBuffer.getData());

#if DEBUG_PRINT
Fw::Logger::logMsg("I2c addr: 0x%02X\n",addr);
Fw::Logger::log("I2c addr: 0x%02X\n",addr);
#endif

struct i2c_msg rdwr_msgs[2];
Expand All @@ -197,25 +197,25 @@ namespace Drv {

if(stat == -1){
#if DEBUG_PRINT
Fw::Logger::logMsg("Status: %d Errno: %d\n", stat, errno);
Fw::Logger::log("Status: %d Errno: %d\n", stat, errno);
#endif
//Because we're using ioctl to perform the transaction we dont know exactly the type of error that occurred
return I2cStatus::I2C_OTHER_ERR;
}

#if DEBUG_PRINT
Fw::Logger::logMsg("Wrote:\n");
Fw::Logger::log("Wrote:\n");
for (U32 byte = 0; byte < writeBuffer.getSize(); byte++) {
Fw::Logger::logMsg("0x%02X ",writeBuffer.getData()[byte]);
Fw::Logger::log("0x%02X ",writeBuffer.getData()[byte]);

}
Fw::Logger::logMsg("\n");
Fw::Logger::logMsg("Read:\n");
Fw::Logger::log("\n");
Fw::Logger::log("Read:\n");
for (U32 byte = 0; byte < readBuffer.getSize(); byte++) {
Fw::Logger::logMsg("0x%02X ",readBuffer.getData()[byte]);
Fw::Logger::log("0x%02X ",readBuffer.getData()[byte]);

}
Fw::Logger::logMsg("\n");
Fw::Logger::log("\n");
#endif

return I2cStatus::I2C_OK;
Expand Down
5 changes: 3 additions & 2 deletions Drv/TcpClient/test/ut/TcpClientTester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@
// ======================================================================
#include "TcpClientTester.hpp"
#include "STest/Pick/Pick.hpp"
#include <Os/Log.hpp>
#include <Os/Console.hpp>
#include <Drv/Ip/test/ut/PortSelector.hpp>
#include <Drv/Ip/test/ut/SocketTestHelper.hpp>

Os::Log logger;
Os::Console logger;

namespace Drv {

Expand Down
5 changes: 3 additions & 2 deletions Drv/TcpServer/test/ut/TcpServerTester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@
// ======================================================================
#include "TcpServerTester.hpp"
#include "STest/Pick/Pick.hpp"
#include "Os/Log.hpp"
#include "Os/Console.hpp"
#include <Drv/Ip/test/ut/PortSelector.hpp>
#include <Drv/Ip/test/ut/SocketTestHelper.hpp>

Os::Log logger;
Os::Console logger;

namespace Drv {

Expand Down
4 changes: 2 additions & 2 deletions Drv/Udp/test/ut/UdpTester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@
#include "STest/Pick/Pick.hpp"
#include <Drv/Ip/test/ut/PortSelector.hpp>
#include <Drv/Ip/test/ut/SocketTestHelper.hpp>
#include "Os/Log.hpp"
#include "Os/Console.hpp"
#include <sys/socket.h>

Os::Log logger;
Os::Console logger;

namespace Drv {

Expand Down
2 changes: 1 addition & 1 deletion Fw/FilePacket/PathName.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace Fw {
void FilePacket::PathName ::
initialize(const char *const value)
{
const U8 length = static_cast<U8>(StringUtils::string_length(value, MAX_LENGTH));
const U8 length = static_cast<U8>(StringUtils::string_length(value, static_cast<FwSizeType>(MAX_LENGTH)));
this->m_length = length;
this->m_value = value;
}
Expand Down
3 changes: 1 addition & 2 deletions Fw/Logger/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ set(MOD_DEPS
)
set(SOURCE_FILES
"${CMAKE_CURRENT_LIST_DIR}/Logger.cpp"
"${CMAKE_CURRENT_LIST_DIR}/LogAssert.cpp"
)
register_fprime_module()

Expand All @@ -26,4 +25,4 @@ set(UT_SOURCE_FILES
"${CMAKE_CURRENT_LIST_DIR}/test/ut/LoggerMain.cpp"
)
# STest Includes for this UT
register_fprime_ut("Logger_Rules_Testing")
register_fprime_ut()
Loading

0 comments on commit 7159e7e

Please sign in to comment.