Skip to content

Commit

Permalink
Fix wrong queue size while reading multiple frames. (#15)
Browse files Browse the repository at this point in the history 
* Fixed wrong documentation comments.
Made strings ref-to-const.
Removed useless const qualifier for return values.
Fixed return type mismatches.
Added in-class-initialisers.

* Static integers are now constexpr

* Fixed bug where wrong queue size is returned.
waitForMessages() now uses ioctl(FIONREAD) to return the size of the queue.

* Fixed missing lib prefix. Thanks to @sikmir!
  • Loading branch information
@SimonCahill
SimonCahill authored Mar 6, 2024
1 parent 621383e commit f2463f9
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 63 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ install(EXPORT ${PROJECT_NAME}Targets

include(CMakePackageConfigHelpers)
write_basic_package_version_file(
"${PROJECT_NAME}ConfigVersion.cmake"
"lib${PROJECT_NAME}ConfigVersion.cmake"
VERSION ${${PROJECT_NAME}_VERSION}
COMPATIBILITY AnyNewerVersion
)
Expand Down
64 changes: 32 additions & 32 deletions include/CanDriver.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,56 +61,56 @@ namespace sockcanpp {
*/
class CanDriver {
public: // +++ Static +++
static const int32_t CAN_MAX_DATA_LENGTH; ///!< The maximum amount of bytes allowed in a single CAN frame
static const int32_t CAN_SOCK_RAW; ///!< The raw CAN protocol
static const int32_t CAN_SOCK_SEVEN; ///!< A separate CAN protocol, used by certain embedded device OEMs.
static constexpr int32_t CAN_MAX_DATA_LENGTH = 8; //!< The maximum amount of bytes allowed in a single CAN frame
static constexpr int32_t CAN_SOCK_RAW = CAN_RAW; //!< The raw CAN protocol
static constexpr int32_t CAN_SOCK_SEVEN = 7; //!< A separate CAN protocol, used by certain embedded device OEMs.

public: // +++ Constructor / Destructor +++
CanDriver(const string canInterface, const int32_t canProtocol, const CanId defaultSenderId = 0); ///!< Constructor
CanDriver(const string canInterface, const int32_t canProtocol, const int32_t filterMask, const CanId defaultSenderId = 0);
CanDriver() {}
virtual ~CanDriver() { uninitialiseSocketCan(); } ///!< Destructor
CanDriver(const string& canInterface, const int32_t canProtocol, const CanId defaultSenderId = 0); //!< Constructor
CanDriver(const string& canInterface, const int32_t canProtocol, const int32_t filterMask, const CanId defaultSenderId = 0);
CanDriver() = default;
virtual ~CanDriver() { uninitialiseSocketCan(); } //!< Destructor

public: // +++ Getter / Setter +++
CanDriver& setDefaultSenderId(const CanId id) { this->_defaultSenderId = id; return *this; } ///!< Sets the default sender ID
CanDriver& setDefaultSenderId(const CanId id) { this->_defaultSenderId = id; return *this; } //!< Sets the default sender ID

const CanId getDefaultSenderId() const { return this->_defaultSenderId; } ///!< Gets the default sender ID
CanId getDefaultSenderId() const { return this->_defaultSenderId; } //!< Gets the default sender ID

const int32_t getFilterMask() const { return this->_canFilterMask; } ///!< Gets the filter mask used by this instance
const int32_t getMessageQueueSize() const { return this->_queueSize; } ///!< Gets the amount of CAN messages found after last calling waitForMessages()
const int32_t getSocketFd() const { return this->_socketFd; } ///!< The socket file descriptor used by this instance.
int32_t getFilterMask() const { return this->_canFilterMask; } //!< Gets the filter mask used by this instance
int32_t getMessageQueueSize() const { return this->_queueSize; } //!< Gets the amount of CAN messages found after last calling waitForMessages()
int32_t getSocketFd() const { return this->_socketFd; } //!< The socket file descriptor used by this instance.

public: // +++ I/O +++
virtual bool waitForMessages(milliseconds timeout = milliseconds(3000)); ///!< Waits for CAN messages to appear
virtual bool waitForMessages(milliseconds timeout = milliseconds(3000)); //!< Waits for CAN messages to appear

virtual CanMessage readMessage(); ///!< Attempts to read a single message from the bus
virtual CanMessage readMessage(); //!< Attempts to read a single message from the bus

virtual int32_t sendMessage(const CanMessage message, bool forceExtended = false); ///!< Attempts to send a single CAN message
virtual int32_t sendMessageQueue(queue<CanMessage> messages,
milliseconds delay = milliseconds(20), bool forceExtended = false); ///!< Attempts to send a queue of messages
virtual ssize_t sendMessage(const CanMessage& message, bool forceExtended = false); //!< Attempts to send a single CAN message
virtual ssize_t sendMessageQueue(queue<CanMessage> messages,
milliseconds delay = milliseconds(20), bool forceExtended = false); //!< Attempts to send a queue of messages

virtual queue<CanMessage> readQueuedMessages(); ///!< Attempts to read all queued messages from the bus
virtual queue<CanMessage> readQueuedMessages(); //!< Attempts to read all queued messages from the bus

virtual void setCanFilterMask(const int32_t mask); ///!< Attempts to set a new CAN filter mask to the BIOS
virtual void setCanFilterMask(const int32_t mask); //!< Attempts to set a new CAN filter mask to the BIOS

protected: // +++ Socket Management +++
virtual void initialiseSocketCan(); ///!< Initialises socketcan
virtual void uninitialiseSocketCan(); ///!< Uninitialises socketcan
virtual void initialiseSocketCan(); //!< Initialises socketcan
virtual void uninitialiseSocketCan(); //!< Uninitialises socketcan

private:
virtual CanMessage readMessageLock(bool const lock = true); ///!< readMessage deadlock guard
CanId _defaultSenderId; ///!< The ID to send messages with if no other ID was set.
virtual CanMessage readMessageLock(bool const lock = true); //!< readMessage deadlock guard
CanId _defaultSenderId; //!< The ID to send messages with if no other ID was set.

int32_t _canFilterMask; ///!< The bit mask used to filter CAN messages
int32_t _canProtocol; ///!< The protocol used when communicating via CAN
int32_t _socketFd; ///!< The CAN socket file descriptor
int32_t _queueSize; ////!< The size of the message queue read by waitForMessages()
int32_t _canFilterMask; //!< The bit mask used to filter CAN messages
int32_t _canProtocol; //!< The protocol used when communicating via CAN
int32_t _socketFd{-1}; //!< The CAN socket file descriptor
int32_t _queueSize{0}; ///!< The size of the message queue read by waitForMessages()

///!< Mutex for thread-safety.
mutex _lock;
mutex _lockSend;
//!< Mutex for thread-safety.
mutex _lock{};
mutex _lockSend{};

string _canInterface; ///!< The CAN interface used for communication (e.g. can0, can1, ...)
string _canInterface; //!< The CAN interface used for communication (e.g. can0, can1, ...)

};

Expand All @@ -130,7 +130,7 @@ namespace sockcanpp {
template<typename... Args>
string formatString(const string& format, Args... args) {
using std::unique_ptr;
auto stringSize = snprintf(NULL, 0, format.c_str(), args...) + 1; // +1 for \0
auto stringSize = snprintf(nullptr, 0, format.c_str(), args...) + 1; // +1 for \0
unique_ptr<char[]> buffer(new char[stringSize]);

snprintf(buffer.get(), stringSize, format.c_str(), args...);
Expand Down
76 changes: 46 additions & 30 deletions src/CanDriver.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include <sys/types.h>
#include <unistd.h>

#include <cmath>
#include <cstdlib>
#include <cstring>
#include <mutex>
Expand Down Expand Up @@ -66,23 +67,21 @@ namespace sockcanpp {
using std::string;
using std::strncpy;
using std::unique_lock;
using std::unique_ptr;
using std::chrono::milliseconds;
using std::this_thread::sleep_for;

//////////////////////////////////////
// PUBLIC IMPLEMENTATION //
//////////////////////////////////////

const int32_t CanDriver::CAN_MAX_DATA_LENGTH = 8;
const int32_t CanDriver::CAN_SOCK_RAW = CAN_RAW;
const int32_t CanDriver::CAN_SOCK_SEVEN = 7;

#pragma region Object Construction
CanDriver::CanDriver(string canInterface, int32_t canProtocol, const CanId defaultSenderId):
CanDriver::CanDriver(const string& canInterface, int32_t canProtocol, const CanId defaultSenderId):
CanDriver(canInterface, canProtocol, 0 /* match all */, defaultSenderId) {}

CanDriver::CanDriver(const string canInterface, const int32_t canProtocol, const int32_t filterMask, const CanId defaultSenderId):
_defaultSenderId(defaultSenderId), _canProtocol(canProtocol), _canInterface(canInterface), _canFilterMask(filterMask), _socketFd(-1) {
CanDriver::CanDriver(const string& canInterface, const int32_t canProtocol, const int32_t filterMask, const CanId defaultSenderId):
_defaultSenderId(defaultSenderId), _canFilterMask(filterMask), _canProtocol(canProtocol), _canInterface(canInterface) {
initialiseSocketCan();
}
#pragma endregion
Expand All @@ -108,9 +107,17 @@ namespace sockcanpp {

FD_ZERO(&readFileDescriptors);
FD_SET(_socketFd, &readFileDescriptors);
_queueSize = select(_socketFd + 1, &readFileDescriptors, 0, 0, &waitTime);
const auto fdsAvailable = select(_socketFd + 1, &readFileDescriptors, nullptr, nullptr, &waitTime);

int32_t bytesAvailable{0};
const auto retCode = ioctl(_socketFd, FIONREAD, &bytesAvailable);
if (retCode == 0) {
_queueSize = static_cast<int32_t>(std::ceil(bytesAvailable / sizeof(can_frame)));
} else {
_queueSize = 0;
}

return _queueSize > 0;
return fdsAvailable > 0;
}

/**
Expand All @@ -128,17 +135,23 @@ namespace sockcanpp {
* @return CanMessage The message read from the bus.
*/
CanMessage CanDriver::readMessageLock(bool const lock) {
std::unique_ptr<std::unique_lock<std::mutex>> _lockLck{nullptr};
if (lock)
_lockLck = std::unique_ptr<std::unique_lock<std::mutex>>{new std::unique_lock<std::mutex>{_lock}};
if (0 > _socketFd)
throw InvalidSocketException("Invalid socket!", _socketFd);
int32_t readBytes{0};
can_frame canFrame;
unique_ptr<unique_lock<mutex>> locky{nullptr};

if (lock) {
locky = unique_ptr<unique_lock<mutex>>{new unique_lock<mutex>{_lock}};
}

if (0 > _socketFd) { throw InvalidSocketException("Invalid socket!", _socketFd); }

ssize_t readBytes{0};
can_frame canFrame{};

memset(&canFrame, 0, sizeof(can_frame));

readBytes = read(_socketFd, &canFrame, sizeof(can_frame));
if (0 > readBytes)
throw CanException(formatString("FAILED to read from CAN! Error: %d => %s", errno, strerror(errno)), _socketFd);

if (0 > readBytes) { throw CanException(formatString("FAILED to read from CAN! Error: %d => %s", errno, strerror(errno)), _socketFd); }

return CanMessage{canFrame};
}

Expand All @@ -148,14 +161,14 @@ namespace sockcanpp {
* @param message The message to be sent.
* @param forceExtended Whether or not to force use of an extended ID.
*
* @return int32_t The amount of bytes sent on the bus.
* @return ssize_t The amount of bytes sent on the bus.
*/
int32_t CanDriver::sendMessage(const CanMessage message, bool forceExtended) {
ssize_t CanDriver::sendMessage(const CanMessage& message, bool forceExtended) {
if (_socketFd < 0) { throw InvalidSocketException("Invalid socket!", _socketFd); }

unique_lock<mutex> locky(_lockSend);

int32_t bytesWritten = 0;
ssize_t bytesWritten = 0;

if (message.getFrameData().size() > CAN_MAX_DATA_LENGTH) {
throw CanException(formatString("INVALID data length! Message must be smaller than %d bytes!", CAN_MAX_DATA_LENGTH), _socketFd);
Expand All @@ -181,10 +194,10 @@ namespace sockcanpp {
*
* @return int32_t The total amount of bytes sent.
*/
int32_t CanDriver::sendMessageQueue(queue<CanMessage> messages, milliseconds delay, bool forceExtended) {
ssize_t CanDriver::sendMessageQueue(queue<CanMessage> messages, milliseconds delay, bool forceExtended) {
if (_socketFd < 0) { throw InvalidSocketException("Invalid socket!", _socketFd); }

int32_t totalBytesWritten = 0;
ssize_t totalBytesWritten = 0;

while (!messages.empty()) {
totalBytesWritten += sendMessage(messages.front(), forceExtended);
Expand All @@ -200,12 +213,15 @@ namespace sockcanpp {
* @return queue<CanMessage> A queue containing the messages read from the bus buffer.
*/
queue<CanMessage> CanDriver::readQueuedMessages() {
if (_socketFd < 0)
throw InvalidSocketException("Invalid socket!", _socketFd);
unique_lock<mutex> locky(_lock);
queue<CanMessage> messages;
for (int32_t i = _queueSize; 0 < i; --i)
messages.emplace(readMessageLock(false));
if (_socketFd < 0) { throw InvalidSocketException("Invalid socket!", _socketFd); }

unique_lock<mutex> locky{_lock};
queue<CanMessage> messages{};

for (int32_t i = _queueSize; 0 < i; --i) {
messages.emplace(readMessageLock(false));
}

return messages;
}

Expand Down Expand Up @@ -248,8 +264,8 @@ namespace sockcanpp {
int64_t fdOptions = 0;
int32_t tmpReturn;

memset(&address, 0, sizeof(sizeof(struct sockaddr_can)));
memset(&ifaceRequest, 0, sizeof(sizeof(struct ifreq)));
memset(&address, 0, sizeof(struct sockaddr_can));
memset(&ifaceRequest, 0, sizeof(struct ifreq));

_socketFd = socket(PF_CAN, SOCK_RAW, _canProtocol);

Expand Down

0 comments on commit f2463f9

Please sign in to comment.