Skip to content

Commit

Permalink
[TP]: Fix bugs such that tests pass again
Browse files Browse the repository at this point in the history
  • Loading branch information
@GwnDaan
GwnDaan committed Nov 7, 2023
1 parent 9477db6 commit 6ad8c55
Show file tree
Hide file tree
Showing 8 changed files with 249 additions and 138 deletions.
17 changes: 13 additions & 4 deletions hardware_integration/include/isobus/hardware_integration/virtual_can_plugin.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,17 @@ namespace isobus
/// @brief Connects to the socket
void open() override;

/// @brief Returns a frame from the hardware (synchronous), or `false` if no frame can be read.
/// @brief Returns a frame from the hardware (synchronous), or `false` if no frame can be read. Times out after 1 second.
/// @param[in, out] canFrame The CAN frame that was read
/// @returns `true` if a CAN frame was read, otherwise `false`
bool read_frame(isobus::CANMessageFrame &canFrame) override;

/// @brief Returns a frame from the hardware (synchronous), or `false` if no frame can be read.
/// @param[in, out] canFrame The CAN frame that was read
/// @param[in] timeout The timeout in milliseconds
/// @returns `true` if a CAN frame was read, otherwise `false`
bool read_frame(isobus::CANMessageFrame &canFrame, std::uint32_t timeout) const;

/// @brief Writes a frame to the bus (synchronous)
/// @param[in] canFrame The frame to write to the bus
/// @returns `true` if the frame was written, otherwise `false`
Expand All @@ -71,10 +77,13 @@ namespace isobus
/// @param[in] canFrame The frame to write to the bus
void write_frame_as_if_received(const isobus::CANMessageFrame &canFrame) const;

/// @brief Returns if the internal message queue is empty or not
/// @returns `true` if the internal message queue is empty, otherwise false
/// @brief Returns if the internal received message queue is empty or not
/// @returns `true` if the internal received message queue is empty, otherwise false
bool get_queue_empty() const;

/// @brief Clear the internal received message queue
void clear_queue() const;

private:
/// @brief A struct holding information about a virtual CAN device
struct VirtualDevice
Expand All @@ -95,4 +104,4 @@ namespace isobus
std::atomic_bool running; ///< If `true`, the driver is running
};
}
#endif // VIRTUAL_CAN_PLUGIN_HPP
#endif // VIRTUAL_CAN_PLUGIN_HPP
13 changes: 12 additions & 1 deletion hardware_integration/src/virtual_can_plugin.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,14 @@ namespace isobus
}

bool VirtualCANPlugin::read_frame(isobus::CANMessageFrame &canFrame)
{
return read_frame(canFrame, 1000);
}

bool VirtualCANPlugin::read_frame(isobus::CANMessageFrame &canFrame, std::uint32_t timeout) const
{
std::unique_lock<std::mutex> lock(mutex);
ourDevice->condition.wait_for(lock, std::chrono::milliseconds(1000), [this] { return !ourDevice->queue.empty() || !running; });
ourDevice->condition.wait_for(lock, std::chrono::milliseconds(timeout), [this] { return !ourDevice->queue.empty() || !running; });
if (!ourDevice->queue.empty())
{
canFrame = ourDevice->queue.front();
Expand All @@ -94,4 +99,10 @@ namespace isobus
const std::lock_guard<std::mutex> lock(mutex);
return ourDevice->queue.empty();
}

void VirtualCANPlugin::clear_queue() const
{
const std::lock_guard<std::mutex> lock(mutex);
ourDevice->queue.clear();
}
}
124 changes: 71 additions & 53 deletions isobus/include/isobus/isobus/can_transport_message.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#ifndef CAN_TRANSPORT_MESSAGE_HPP
#define CAN_TRANSPORT_MESSAGE_HPP

#include "isobus/isobus/can_callbacks.hpp"
#include "isobus/isobus/can_control_function.hpp"
#include "isobus/isobus/can_message.hpp"

Expand Down Expand Up @@ -108,7 +109,7 @@ namespace isobus
/// @brief Get the byte at the given index.
/// @param index The index of the byte to get.
/// @return The byte at the given index.
virtual std::uint8_t get_byte(std::size_t index) const = 0;
virtual std::uint8_t get_byte(std::size_t index) = 0;

/// @brief Set the byte at the given index.
/// @param index The index of the byte to set.
Expand All @@ -129,14 +130,30 @@ namespace isobus
, public std::vector<std::uint8_t>
{
public:
/// @brief Construct a new CANTransportDataVector object.
/// @param size The size of the data.
explicit CANTransportDataVector(std::size_t size);

/// @brief Construct a new CANTransportDataVector object.
/// @param data The data to copy.
explicit CANTransportDataVector(const std::vector<std::uint8_t> &data);

/// @brief Construct a new CANTransportDataVector object.
/// @param data A pointer to the data to copy.
/// @param size The size of the data to copy.
CANTransportDataVector(const std::uint8_t *data, std::size_t size);

/// @brief Construct a new CANTransportDataVector object.
/// @param data The data to copy.

/// @brief Get the size of the data.
/// @return The size of the data.
std::size_t size() const override;

/// @brief Get the byte at the given index.
/// @param index The index of the byte to get.
/// @return The byte at the given index.
std::uint8_t get_byte(std::size_t index) const override;
std::uint8_t get_byte(std::size_t index) override;

/// @brief Set the byte at the given index.
/// @param index The index of the byte to set.
Expand Down Expand Up @@ -170,7 +187,7 @@ namespace isobus
/// @brief Get the byte at the given index.
/// @param index The index of the byte to get.
/// @return The byte at the given index.
std::uint8_t get_byte(std::size_t index) const override;
std::uint8_t get_byte(std::size_t index) override;

/// @brief Set the byte at the given index.
/// @param index The index of the byte to set.
Expand All @@ -182,56 +199,50 @@ namespace isobus
DataSpan<uint8_t> data() override;
};

// //================================================================================================
// /// @class CANTransportDataCallback
// ///
// /// @brief A class that represents data of a CAN message by using a callback function.
// //================================================================================================
// class CANTransportDataCallback : public CANTransportData
// {
// public:
// /// @brief Construct a new CANTransportDataCallback object.
// /// @param size_callback The callback function to use for getting the size of the data.
// /// @param get_byte_callback The callback function to use for getting a byte of the data.
// /// @param set_byte_callback The callback function to use for setting a byte of the data.
// CANTransportDataCallback(std::size_t (*size_callback)(),
// std::uint8_t (*get_byte_callback)(int),
// void (*set_byte_callback)(int, std::uint8_t)) :
// size_callback(size_callback),
// get_byte_callback(get_byte_callback),
// set_byte_callback(set_byte_callback)
// {
// }

// /// @brief Get the size of the data.
// /// @return The size of the data.
// std::size_t size() const override
// {
// return size_callback();
// }

// /// @brief Get the byte at the given index.
// /// @param index The index of the byte to get.
// /// @return The byte at the given index.
// std::uint8_t get_byte(int index) const override
// {
// return get_byte_callback(index);
// }

// /// @brief Set the byte at the given index.
// /// @param index The index of the byte to set.
// /// @param value The value to set the byte to.
// void set_byte(int index, std::uint8_t value) override
// {
// set_byte_callback(index, value);
// }

// /// @brief Get the data span.
// /// @return The data span.
// DataSpan<uint8_t> data() override
// {
// return DataSpan<uint8_t>(nullptr, 0);
// }
//================================================================================================
/// @class CANTransportDataCallback
///
/// @brief A class that represents data of a CAN message by using a callback function.
//================================================================================================
class CANTransportDataCallback : public CANTransportData
{
public:
/// @brief Constructor for transport data that uses a callback function.
/// @param size The size of the data.
/// @param callback The callback function to be called for each data chunk.
/// @param parentPointer The parent object that owns this callback (optional).
/// @param chunkSize The size of each data chunk (optional, default is 7).
CANTransportDataCallback(std::size_t size,
DataChunkCallback callback,
void *parentPointer = nullptr,
std::size_t chunkSize = 7);

/// @brief Get the size of the data.
/// @return The size of the data.
std::size_t size() const override;

/// @brief Get the byte at the given index.
/// @param index The index of the byte to get.
/// @return The byte at the given index.
std::uint8_t get_byte(std::size_t index) override;

/// @brief Set the byte at the given index.
/// @param index The index of the byte to set.
/// @param value The value to set the byte to.
void set_byte(std::size_t index, std::uint8_t value) override;

/// @brief Get the data span.
/// @return The data span.
DataSpan<uint8_t> data() override;

private:
std::size_t totalSize;
DataChunkCallback callback;
void *parentPointer;
std::vector<std::uint8_t> buffer;
std::size_t bufferSize;
std::size_t dataOffset = 0;
};

//================================================================================================
/// @class CANTransportMessage
Expand Down Expand Up @@ -288,6 +299,13 @@ namespace isobus
/// @return true if the message can continue to be transported.
bool can_continue() const;

/// @brief Check if this message is from a specific source and destination.
/// @param source The source control function.
/// @param destination The destination control function.
/// @return true if this message is from the given source and destination.
bool matches(std::shared_ptr<ControlFunction> source,
std::shared_ptr<ControlFunction> destination) const;

private:
std::uint32_t identifier;
std::weak_ptr<ControlFunction> source;
Expand Down
44 changes: 25 additions & 19 deletions isobus/src/can_network_manager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,11 @@ namespace isobus
std::unique_ptr<CANTransportData> data;
if (nullptr != frameChunkCallback)
{
//! @todo use the frameChunkCallback to send the message
data = std::make_unique<CANTransportDataCallback>(dataLength, frameChunkCallback, parentPointer);
}
else
{
data = std::make_unique<CANTransportDataView>(const_cast<std::uint8_t *>(dataBuffer), static_cast<size_t>(dataLength));
data = std::make_unique<CANTransportDataVector>(dataBuffer, dataLength);
}

if (transportProtocol.protocol_transmit_message(parameterGroupNumber,
Expand All @@ -151,27 +151,30 @@ namespace isobus
parentPointer))
{
// Successfully sent via the transport protocol
retVal = true;
}

//! @todo convert the other protocols to stop using the abstract protocollib class
CANLibProtocol *currentProtocol;
// See if any transport layer protocol can handle this message
for (std::uint32_t i = 0; i < CANLibProtocol::get_number_protocols(); i++)
else
{
if (CANLibProtocol::get_protocol(i, currentProtocol))
//! @todo convert the other protocols to stop using the abstract protocollib class
CANLibProtocol *currentProtocol;
// See if any transport layer protocol can handle this message
for (std::uint32_t i = 0; i < CANLibProtocol::get_number_protocols(); i++)
{
retVal = currentProtocol->protocol_transmit_message(parameterGroupNumber,
dataBuffer,
dataLength,
sourceControlFunction,
destinationControlFunction,
transmitCompleteCallback,
parentPointer,
frameChunkCallback);

if (retVal)
if (CANLibProtocol::get_protocol(i, currentProtocol))
{
break;
retVal = currentProtocol->protocol_transmit_message(parameterGroupNumber,
dataBuffer,
dataLength,
sourceControlFunction,
destinationControlFunction,
transmitCompleteCallback,
parentPointer,
frameChunkCallback);

if (retVal)
{
break;
}
}
}
}
Expand Down Expand Up @@ -232,6 +235,9 @@ namespace isobus

prune_inactive_control_functions();

// Update transport protocols
transportProtocol.update({});

for (std::size_t i = 0; i < CANLibProtocol::get_number_protocols(); i++)
{
CANLibProtocol *currentProtocol = nullptr;
Expand Down
Loading

0 comments on commit 6ad8c55

Please sign in to comment.