Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 37 additions & 2 deletions include/libserial/serial.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,16 @@
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <poll.h>
#include <asm/ioctls.h>
#include <asm/termbits.h>
#include <sys/ioctl.h>
#include <sys/types.h>

#include <chrono>
#include <iostream>
#include <memory>
#include <functional>
#include <string>
#include <thread>
#include <vector>
Expand Down Expand Up @@ -227,7 +230,7 @@
* @param parity The desired parity setting (ENABLE or DISABLE)
* @throws SerialException if parity cannot be set
*/
void setParity([[maybe_unused]] Parity parity);
void setParity(Parity parity);

/**
* @brief Sets the stop bits configuration
Expand All @@ -237,7 +240,7 @@
* @param stop_bits The desired stop bits setting (ONE, ONE_AND_HALF, or TWO)
* @throws SerialException if stop bits cannot be set
*/
void setStopBits([[maybe_unused]] StopBits stop_bits);
void setStopBits(StopBits stop_bits);

/**
* @brief Sets the flow control configuration
Expand Down Expand Up @@ -315,9 +318,41 @@
void setFdForTest(int fd) {
fd_serial_port_ = fd;
}

// For testing - allow injection of mock functions
void setSystemCallFunctions(
std::function<int(struct pollfd*, nfds_t, int)> poll_func,
std::function<ssize_t(int, void*, size_t)> read_func) {
poll_ = [poll_func](struct pollfd* f, nfds_t n, int t) {
return poll_func(f, n, t);
};
read_ = [read_func](int fd, void* buf, size_t sz) {
return read_func(fd, buf, sz);
};
}
#endif

private:
/**
* @brief Poll system call function wrapper
*
* Allows injection of custom poll function for testing.
*/
std::function<int(struct pollfd*, nfds_t, int)> poll_ =
[](struct pollfd* f, nfds_t n, int t) {
return ::poll(f, n, t);
};

/**
* @brief Read system call function wrapper
*
* Allows injection of custom read function for testing.
*/
std::function<ssize_t(int, void*, size_t)> read_ =
[](int fd, void* buf, size_t sz) {
return ::read(fd, buf, sz);

Check failure on line 353 in include/libserial/serial.hpp

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

include/libserial/serial.hpp#L353

Check buffer boundaries if used in a loop including recursive loops (CWE-120, CWE-20).
};

/**
* @brief Applies terminal settings to the port
*
Expand Down
65 changes: 29 additions & 36 deletions src/serial.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,19 +73,17 @@ size_t Serial::read(std::shared_ptr<std::string> buffer) {

// 0 => no wait (immediate return), -1 => block forever, positive => wait specified milliseconds
int timeout_ms = static_cast<int>(read_timeout_ms_.count());
int pr = poll(&fd_poll, 1, timeout_ms);
int pr = poll_(&fd_poll, 1, timeout_ms);
if (pr < 0) {
if (errno == EINTR) {
throw IOException("Interrupted while polling");
}
throw IOException(std::string("Error in poll(): ") + strerror(errno));
}
if (pr == 0) {
throw IOException("Read operation timed out after " + std::to_string(timeout_ms) + "ms");
throw IOException("Read operation timed out after " + std::to_string(timeout_ms) +
" milliseconds");
}

// Data available: do the read
ssize_t bytes_read = ::read(fd_serial_port_, const_cast<char*>(buffer->data()), kMaxSafeReadSize);
ssize_t bytes_read = read_(fd_serial_port_, const_cast<char*>(buffer->data()), kMaxSafeReadSize);
if (bytes_read < 0) {
throw IOException(std::string("Error reading from serial port: ") + strerror(errno));
}
Expand All @@ -104,7 +102,7 @@ size_t Serial::readBytes(std::shared_ptr<std::string> buffer, size_t num_bytes)
}

if (num_bytes == 0) {
throw IOException("Invalid number of bytes requested");
throw IOException("Number of bytes requested must be greater than zero");
}

buffer->clear();
Expand Down Expand Up @@ -157,7 +155,7 @@ size_t Serial::readUntil(std::shared_ptr<std::string> buffer, char terminator) {
int64_t remaining_timeout = read_timeout_ms_.count() - elapsed;
int timeout_ms = static_cast<int>(remaining_timeout);

int poll_result = poll(&pfd, 1, timeout_ms);
int poll_result = poll_(&pfd, 1, timeout_ms);
if (poll_result < 0) {
throw IOException("Error in poll(): " + std::string(strerror(errno)));
}
Expand All @@ -167,7 +165,7 @@ size_t Serial::readUntil(std::shared_ptr<std::string> buffer, char terminator) {
}

// Data is available, perform the read
ssize_t bytes_read = ::read(fd_serial_port_, &temp_char, 1);
ssize_t bytes_read = read_(fd_serial_port_, &temp_char, 1);

if (bytes_read < 0) {
if (errno == EAGAIN || errno == EWOULDBLOCK) {
Expand Down Expand Up @@ -246,35 +244,30 @@ void Serial::setDataLength(DataLength nbits) {
this->setTermios2();
}

void Serial::setParity([[maybe_unused]] Parity parity) {
// this->getTermios2();
// switch (parity) {
// case Parity::DISABLE:
// options_.c_cflag &= ~PARENB;
// break;
// case Parity::ENABLE:
// options_.c_cflag |= PARENB;
// break;
// default:
// options_.c_cflag &= ~PARENB;
// break;
// }
// this->setTermios2();
void Serial::setParity(Parity parity) {
this->getTermios2();
switch (parity) {
case Parity::DISABLE:
options_.c_cflag &= ~PARENB;
break;
case Parity::ENABLE:
options_.c_cflag |= PARENB;
break;
}
this->setTermios2();
}

void Serial::setStopBits([[maybe_unused]] StopBits stop_bits) {
// this->getTermios2();
// switch (stop_bits) {
// case StopBits::DISABLE:
// options_.c_cflag &= ~CSTOP;
// break;
// case StopBits::ENABLE:
// options_.c_cflag |= CSTOP;
// default:
// options_.c_cflag &= ~CSTOP;
// break;
// }
// this->setTermios2();
void Serial::setStopBits(StopBits stop_bits) {
this->getTermios2();
switch (stop_bits) {
case StopBits::ONE:
options_.c_cflag &= ~CSTOP;
break;
case StopBits::TWO:
options_.c_cflag |= CSTOP;
break;
}
this->setTermios2();
}

void Serial::setFlowControl([[maybe_unused]] FlowControl flow_control) {
Expand Down
1 change: 0 additions & 1 deletion test/test_ports.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ TEST_F(PortsTest, ScanPortsThrowsWhenPathMissing) {
FAIL() << "Expected libserial::SerialException to be thrown";
}
catch (const libserial::SerialException& e) {
GTEST_LOG_(INFO) << "Exception: " << e.what();
// Optionally assert something about the message:
EXPECT_NE(std::string(e.what()).find("Error while reading"), std::string::npos);
}
Expand Down
95 changes: 90 additions & 5 deletions test/test_serial_pty.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -278,21 +278,104 @@ TEST_F(PseudoTerminalTest, ReadTimeout) {
serial_port.setBaudRate(9600);

// Set a short read timeout
serial_port.setReadTimeout(std::chrono::milliseconds(100));
int time_out_ms = 100;
serial_port.setReadTimeout(std::chrono::milliseconds(time_out_ms));

auto read_buffer = std::make_shared<std::string>();

auto expected_what = "Read operation timed out after " + std::to_string(time_out_ms) +
" milliseconds";

EXPECT_THROW({
try {
serial_port.read(read_buffer);
}
catch (const libserial::IOException& e) {
GTEST_LOG_(INFO) << "Exception: " << e.what();
EXPECT_STREQ(expected_what.c_str(), e.what());
throw;
}
}, libserial::IOException);
}

TEST_F(PseudoTerminalTest, ReadWithReadFail) {
libserial::Serial serial_port;
auto read_buffer = std::make_shared<std::string>();

const std::vector<std::pair<int, std::string> > error_scenarios = {
{EBADF, "Bad file descriptor"},
{EIO, "Input/output error"},
{EINTR, "Interrupted system call"},
{EAGAIN, "Resource temporarily unavailable"},
{EISDIR, "Is a directory"}
};

ASSERT_EQ(error_scenarios.size(), 5);
Comment thread
NestorDP marked this conversation as resolved.
Outdated

for (const auto& [error_num, error_msg] : error_scenarios) {
serial_port.setSystemCallFunctions(
[](struct pollfd*, nfds_t, int) -> int {
return 1;
},
[error_num](int, void*, size_t) -> ssize_t {
errno = error_num;
return -1;
});

auto expected_what = "Error reading from serial port: " + error_msg;

EXPECT_THROW({
try {
serial_port.read(read_buffer);
}
catch (const libserial::IOException& e) {
EXPECT_STREQ(expected_what.c_str(), e.what());
throw;
}
}, libserial::IOException);
}
}

TEST_F(PseudoTerminalTest, ReadWithPollFail) {
libserial::Serial serial_port;
auto read_buffer = std::make_shared<std::string>();

const std::vector<std::pair<int, std::string> > error_scenarios = {
{EAGAIN, "Resource temporarily unavailable"},
{ENOMEM, "Cannot allocate memory"},
{EINVAL, "Invalid argument"},
{EPERM, "Operation not permitted"},
{EBADF, "Bad file descriptor"},
{EEXIST, "File exists"},
{ENOENT, "No such file or directory"},
{EINTR, "Interrupted system call"}
};

ASSERT_EQ(error_scenarios.size(), 8);
Comment thread
NestorDP marked this conversation as resolved.
Outdated

for (const auto& [error_num, error_msg] : error_scenarios) {
serial_port.setSystemCallFunctions(
[error_num](struct pollfd*, nfds_t, int) -> int {
errno = error_num;
return -1;
},
[](int, void*, size_t) -> ssize_t {
return 1;
});

auto expected_what = "Error in poll(): " + error_msg;

EXPECT_THROW({
try {
serial_port.read(read_buffer);
}
catch (const libserial::IOException& e) {
EXPECT_STREQ(expected_what.c_str(), e.what());
throw;
}
}, libserial::IOException);
}
}

TEST_F(PseudoTerminalTest, ReadBytesNonCanonicalMode) {
libserial::Serial serial_port;

Expand Down Expand Up @@ -333,7 +416,7 @@ TEST_F(PseudoTerminalTest, ReadBytesWithNullBuffer) {
serial_port.readBytes(null_buffer, 10);
}
catch (const libserial::IOException& e) {
GTEST_LOG_(INFO) << "Exception: " << e.what();
EXPECT_STREQ("Null pointer passed to readBytes function", e.what());
throw;
}
}, libserial::IOException);
Expand All @@ -353,7 +436,7 @@ TEST_F(PseudoTerminalTest, ReadBytesWithInvalidNumBytes) {
serial_port.readBytes(read_buffer, 0);
}
catch (const libserial::IOException& e) {
GTEST_LOG_(INFO) << "Exception: " << e.what();
EXPECT_STREQ("Number of bytes requested must be greater than zero", e.what());
throw;
}
}, libserial::IOException);
Expand All @@ -374,7 +457,9 @@ TEST_F(PseudoTerminalTest, ReadBytesCanonicalMode) {
ADD_FAILURE() << "Expected SerialException but no exception was thrown";
}
catch (const libserial::IOException& e) {
GTEST_LOG_(INFO) << "Exception: " << e.what();
EXPECT_STREQ(
"readBytes() is not supported in canonical mode; use read() or readUntil() instead",
e.what());
throw;
}
}, libserial::IOException);
Expand Down