Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Service RX/TX Request/Response sessions #356

Merged
merged 82 commits into from
May 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
82 commits
Select commit Hold shift + click to select a range
08a337d
#verification #docs
serges147 Apr 11, 2024
88c4478
Added `MessageRxSession` implementation.
serges147 Apr 16, 2024
74d030c
Implemented `CanardMemory` for `DynamicBuffer`.
serges147 Apr 17, 2024
bff389e
Merge branch 'issue/336_transport_interfaces' into sshirokov/msg_rx_s…
serges147 Apr 17, 2024
a90c2fe
set proper type id
serges147 Apr 17, 2024
47c4218
minor fixes
serges147 Apr 17, 2024
e9c5133
introduce `TestCanTransport` test fixture
serges147 Apr 17, 2024
ec91d2a
Added `TrackingMemoryResource` test helper
serges147 Apr 17, 2024
dd2e74c
temporary disable memory tracking #verification
serges147 Apr 17, 2024
ec53ae0
implemented memory copying #verification
serges147 Apr 17, 2024
a34dd90
more unit tests #verification
serges147 Apr 17, 2024
fdb7480
try fix gcc build #verification
serges147 Apr 17, 2024
f56d006
Merge branch 'sshirokov/msg_rx_session' into sshirokov/try_ts22.4.7
serges147 Apr 18, 2024
478e36d
#verification
serges147 Apr 18, 2024
34b6422
#verification
serges147 Apr 18, 2024
9a4703b
Merge branch 'sshirokov/try_ts22.4.7' into sshirokov/msg_rx_session
serges147 Apr 18, 2024
d318b1e
cover delegate with unit tests #verification
serges147 Apr 18, 2024
7b729e1
more unit tests to cover memory errors #verification
serges147 Apr 18, 2024
b92db32
fix unit tests on cpp17+ #verification
serges147 Apr 18, 2024
4952f81
try to fix seg fault on release #verification
serges147 Apr 19, 2024
6e1c79b
first draft of can msg tx session
serges147 Apr 19, 2024
a68fadb
unit test for no memory for tx session #verification
serges147 Apr 19, 2024
a705deb
buffer
serges147 Apr 19, 2024
96b65ae
Added `ContiguousPayload` helper. #verification
serges147 Apr 20, 2024
c4e478b
try fix build #verification
serges147 Apr 20, 2024
27a04f5
disable/ignore exceptions during coverage #verification
serges147 Apr 20, 2024
b0c2876
Merge branch 'issue/346_can_msg_sessions' into sshirokov/346_msg_rx_s…
serges147 Apr 22, 2024
11bdbea
PR fixes
serges147 Apr 22, 2024
9e2a394
Merge branch 'sshirokov/346_msg_rx_session' into sshirokov/346_msg_tx…
serges147 Apr 22, 2024
784665a
minor fixes #verification
serges147 Apr 22, 2024
7c0a538
Fix for issue #349 #verification
serges147 Apr 22, 2024
cdeedd4
mention issue # 223
serges147 Apr 23, 2024
7847d92
Merge branch 'issue/346_can_msg_sessions' into sshirokov/346_msg_rx_s…
serges147 Apr 23, 2024
2ccfbf9
Merge branch 'sshirokov/346_msg_rx_session' into sshirokov/346_msg_tx…
serges147 Apr 23, 2024
8d9ff46
rename `defines.hpp` → `types.hpp`
serges147 Apr 23, 2024
de03c56
Merge branch 'sshirokov/346_msg_rx_session' into sshirokov/346_msg_tx…
serges147 Apr 23, 2024
56d77e5
minor fix after merge
serges147 Apr 23, 2024
445c3d0
Merge branch 'issue/346_can_msg_sessions' into sshirokov/346_msg_rx_s…
serges147 Apr 23, 2024
0f9ad7c
Merge branch 'sshirokov/346_msg_rx_session' into sshirokov/346_msg_tx…
serges147 Apr 23, 2024
e2d38fa
implemented msg tx push
serges147 Apr 23, 2024
01cfe9d
minor fix #verification
serges147 Apr 23, 2024
b8696a6
added `setSendTimeout(const Duration timeout)`
serges147 Apr 23, 2024
362dafb
cover tx send & run #verification
serges147 Apr 24, 2024
769f23f
Added `ITransport::setLocalNodeId`; unit tests for multiframe msg tx;…
serges147 Apr 25, 2024
15a950d
Cover `can::TransportImpl::setLocalNodeId` #verification
serges147 Apr 25, 2024
1d62193
Allow setting the same node ID multiple times #verification
serges147 Apr 25, 2024
090b34f
added test for CAN tx to redundant media with "temporary not ready" c…
serges147 Apr 25, 2024
05388d9
Merge branch 'issue/346_can_msg_sessions' into sshirokov/346_msg_rx_s…
sshirokov-mwb Apr 25, 2024
9404519
Merge branch 'sshirokov/346_msg_rx_session' into sshirokov/346_msg_tx…
sshirokov-mwb Apr 25, 2024
9baad4b
Merge branch 'issue/346_can_msg_sessions' into sshirokov/346_msg_tx_s…
serges147 Apr 26, 2024
865fe6e
adding `std::` and removing `inline`
serges147 Apr 26, 2024
2b12f90
`SessionAlreadyExistsError` → `AlreadyExistsError`
serges147 Apr 26, 2024
e69f8af
`memcpy` → `memmove` #verification
serges147 Apr 26, 2024
f104f7d
better coverage
serges147 Apr 26, 2024
98a3934
better coverage
serges147 Apr 26, 2024
3a0a6b9
Update doxygen ini file (by `doxyge -u` command) - no warnings now. #…
serges147 Apr 26, 2024
50f26be
build fix #verification
serges147 Apr 26, 2024
960dc20
minor fix
serges147 Apr 26, 2024
dfe95f1
introduced svc tx sessions
serges147 Apr 26, 2024
99a4bb7
more coverage
serges147 Apr 26, 2024
08f036c
coverage for svc tx sessions #verification
serges147 Apr 26, 2024
3aa2aa1
added implementation of svc rx sessions #verification
serges147 Apr 28, 2024
296f699
more coverage #verification
sshirokov-mwb Apr 28, 2024
1f2d744
Autosar A12-1-2 fixes
sshirokov-mwb Apr 28, 2024
b94e0c8
PR fixes: less `auto` in production when result type is not obvious
sshirokov-mwb Apr 28, 2024
85aa367
PR fixes: rename `TestScheduler` → `VirtualTimeScheduler`.
sshirokov-mwb Apr 28, 2024
1786912
Merge branch 'sshirokov/346_msg_tx_session' into sshirokov/xxx_svc_tx
sshirokov-mwb Apr 28, 2024
6d66d69
minor changes
sshirokov-mwb Apr 28, 2024
9c711df
fix OOM for CAN transport
sshirokov-mwb Apr 28, 2024
b41fac8
Merge branch 'sshirokov/346_msg_tx_session' into sshirokov/xxx_svc_tx
sshirokov-mwb Apr 28, 2024
05c4ad5
rename `Tag` -> `Spec` #verification
serges147 Apr 28, 2024
db0c796
Rename `ScatteredBuffer::Interface` → `Storage`.
serges147 Apr 29, 2024
6065693
switch to the latest `cetl::unbounded_variant` #verification
serges147 Apr 29, 2024
e366c98
docs improvements
serges147 Apr 30, 2024
ffa98c5
added `.clang-tidy` file #verification
serges147 Apr 30, 2024
814de68
Merge branch 'sshirokov/xxx_svc_tx' into sshirokov/xxx1_svc_tx_rx
serges147 Apr 30, 2024
03a1e60
Merge branch 'sshirokov/xxx1_svc_tx_rx' into sshirokov/xxx2_docs
serges147 Apr 30, 2024
377f0e0
RxSessionDelegate → IRxSessionDelegate; Storage → IStorage
serges147 Apr 30, 2024
a4c8ff1
switch to CETL's `main` branch commit #verification
serges147 May 1, 2024
03e1da1
Merge branch 'issue/346_can_msg_sessions' into sshirokov/xxx1_svc_tx_rx
serges147 May 2, 2024
975e6a7
minor change
serges147 May 2, 2024
2b06c32
PR fixes
serges147 May 3, 2024
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
31 changes: 31 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
Checks: >-
boost-*,
bugprone-*,
cert-*,
clang-analyzer-*,
cppcoreguidelines-*,
google-*,
hicpp-*,
llvm-*,
misc-*,
modernize-*,
performance-*,
portability-*,
readability-*,
-cppcoreguidelines-avoid-const-or-ref-data-members,
-google-readability-todo,
-readability-avoid-const-params-in-decls,
-readability-identifier-length,
-llvm-header-guard,
-llvm-include-order,
-*-use-trailing-return-type,
-*-named-parameter,
CheckOptions:
- key: readability-function-cognitive-complexity.Threshold
value: '90'
- key: readability-magic-numbers.IgnoredIntegerValues
value: '1;2;3;4;5;8;10;16;20;32;60;64;100;128;256;500;512;1000'
WarningsAsErrors: '*'
HeaderFilterRegex: '.*\.hpp'
AnalyzeTemporaryDtors: false
FormatStyle: file
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet integrated, but just initial version of the config according issue #226

2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ if (${CMAKE_CXX_PLATFORM_ID} STREQUAL "Linux")
endif()

# Don't normalize deviance: if CMAKE_TOOLCHAIN_FILE is not set then provide
# an initalized default to display in the status thus avoiding a warning about
# an initialized default to display in the status thus avoiding a warning about
# using an uninitialized variable.
if (NOT CMAKE_TOOLCHAIN_FILE)
set(LOCAL_CMAKE_TOOLCHAIN_FILE "(not set)")
Expand Down
2 changes: 1 addition & 1 deletion cmake/modules/Findcetl.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

include(FetchContent)
set(cetl_GIT_REPOSITORY "https://github.com/OpenCyphal/cetl.git")
set(cetl_GIT_TAG "886a0d227a043511eed6b252ea0f788590c50e75")
set(cetl_GIT_TAG "10fbb2b7b89473d68e73db7235848b0692169e5a")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CETL's main (with recently merged cetl::unbounded_variant.


FetchContent_Declare(
cetl
Expand Down
2 changes: 1 addition & 1 deletion docs/doxygen.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1071,7 +1071,7 @@ EXCLUDE_PATTERNS =
# wildcard * is used, a substring. Examples: ANamespace, AClass,
# ANamespace::AClass, ANamespace::*Test

EXCLUDE_SYMBOLS =
EXCLUDE_SYMBOLS = *::detail::*
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Internal implementation detail won't go to generated docs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice


# The EXAMPLE_PATH tag can be used to specify one or more files or directories
# that contain example code fragments that are included (see the \include
Expand Down
34 changes: 18 additions & 16 deletions include/libcyphal/transport/can/delegate.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#ifndef LIBCYPHAL_TRANSPORT_CAN_DELEGATE_HPP_INCLUDED
#define LIBCYPHAL_TRANSPORT_CAN_DELEGATE_HPP_INCLUDED

#include <libcyphal/transport/types.hpp>
#include <libcyphal/transport/errors.hpp>
#include <libcyphal/transport/scattered_buffer.hpp>

Expand All @@ -29,7 +30,7 @@ namespace detail
/// This internal transport delegate class serves the following purposes:
/// 1. It provides memory management functions for the Canard library.
/// 2. It provides a way to convert Canard error codes to `AnyError` type.
/// 3. It provides interface to access the transport from various session classes.
/// 3. It provides an interface to access the transport from various session classes.
///
class TransportDelegate
{
Expand All @@ -40,7 +41,7 @@ class TransportDelegate
public:
/// @brief RAII class to manage memory allocated by Canard library.
///
class CanardMemory final : public cetl::rtti_helper<CanardMemoryTypeIdType, ScatteredBuffer::Interface>
class CanardMemory final : public cetl::rtti_helper<CanardMemoryTypeIdType, ScatteredBuffer::IStorage>
{
public:
CanardMemory(TransportDelegate& delegate, void* const buffer, const std::size_t payload_size)
Expand Down Expand Up @@ -70,7 +71,7 @@ class TransportDelegate
CanardMemory& operator=(const CanardMemory&) = delete;
CanardMemory& operator=(CanardMemory&&) noexcept = delete;

// MARK: ScatteredBuffer::Interface
// MARK: ScatteredBuffer::IStorage

CETL_NODISCARD std::size_t size() const noexcept final
{
Expand Down Expand Up @@ -161,10 +162,9 @@ class TransportDelegate
///
/// Internal method which is in use by TX session implementations to delegate actual sending to transport.
///
CETL_NODISCARD virtual cetl::optional<AnyError> sendTransfer(const CanardMicrosecond deadline,
CETL_NODISCARD virtual cetl::optional<AnyError> sendTransfer(const TimePoint deadline,
const CanardTransferMetadata& metadata,
const void* const payload,
const std::size_t payload_size) = 0;
const PayloadFragments payload_fragments) = 0;

protected:
~TransportDelegate() = default;
Expand Down Expand Up @@ -230,26 +230,28 @@ class TransportDelegate

}; // TransportDelegate

/// This internal session delegate class serves the following purpose: it provides interface
/// to access a session from transport (by casting canard's `user_reference` member to this class).
// MARK: -

/// This internal session delegate class serves the following purpose: it provides an interface (aka gateway)
/// to access RX session from transport (by casting canard's `user_reference` member to this class).
///
class SessionDelegate
class IRxSessionDelegate
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding I prefix for anything like an interface (regardless whether it's internal or public stuff) - as it was requested in previous PR.

{
public:
SessionDelegate(const SessionDelegate&) = delete;
SessionDelegate(SessionDelegate&&) noexcept = delete;
SessionDelegate& operator=(const SessionDelegate&) = delete;
SessionDelegate& operator=(SessionDelegate&&) noexcept = delete;
IRxSessionDelegate(const IRxSessionDelegate&) = delete;
IRxSessionDelegate(IRxSessionDelegate&&) noexcept = delete;
IRxSessionDelegate& operator=(const IRxSessionDelegate&) = delete;
IRxSessionDelegate& operator=(IRxSessionDelegate&&) noexcept = delete;

/// @brief Accepts a received transfer from the transport dedicated to this RX session.
///
virtual void acceptRxTransfer(const CanardRxTransfer& transfer) = 0;

protected:
SessionDelegate() = default;
~SessionDelegate() = default;
IRxSessionDelegate() = default;
~IRxSessionDelegate() = default;

}; // SessionDelegate
}; // IRxSessionDelegate

} // namespace detail
} // namespace can
Expand Down
2 changes: 1 addition & 1 deletion include/libcyphal/transport/can/media.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class IMedia
/// This value may change arbitrarily at runtime. The transport implementation will query it before every
/// transmission on the port. This value has no effect on the reception pipeline as it can accept arbitrary MTU.
///
CETL_NODISCARD virtual std::size_t getMtu() const noexcept = 0;
virtual std::size_t getMtu() const noexcept = 0;

/// @brief Set the filters for the CAN bus.
///
Expand Down
43 changes: 28 additions & 15 deletions include/libcyphal/transport/can/msg_rx_session.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,33 @@ namespace can
///
namespace detail
{
class MessageRxSession final : public IMessageRxSession, private SessionDelegate

/// @brief A class to represent a message subscriber RX session.
///
class MessageRxSession final : public IMessageRxSession, private IRxSessionDelegate
{
// In use to disable public construction.
// See https://seanmiddleditch.github.io/enabling-make-unique-with-private-constructors/
struct Tag
/// @brief Defines specification for making interface unique ptr.
///
struct Spec
{
explicit Tag() = default;
using Interface = IMessageRxSession;
using Concrete = MessageRxSession;

// In use to disable public construction.
// See https://seanmiddleditch.github.io/enabling-make-unique-with-private-constructors/
explicit Spec() = default;
};

public:
CETL_NODISCARD static Expected<UniquePtr<IMessageRxSession>, AnyError> make(TransportDelegate& delegate,
const MessageRxParams& params)
{
auto session = libcyphal::detail::makeUniquePtr<Tag>(delegate.memory(), Tag{}, delegate, params);
if (params.subject_id > CANARD_SUBJECT_ID_MAX)
{
return ArgumentError{};
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parameters are const (see params_ below), and their subject_id won't change. So, it makes sense fail early (at make phase) rather than later during transmissions. The same story for other session factories.


auto session = libcyphal::detail::makeUniquePtr<Spec>(delegate.memory(), Spec{}, delegate, params);
if (session == nullptr)
{
return MemoryError{};
Expand All @@ -49,7 +60,7 @@ class MessageRxSession final : public IMessageRxSession, private SessionDelegate
return session;
}

MessageRxSession(Tag, TransportDelegate& delegate, const MessageRxParams& params)
MessageRxSession(Spec, TransportDelegate& delegate, const MessageRxParams& params)
: delegate_{delegate}
, params_{params}
, subscription_{}
Expand All @@ -65,14 +76,17 @@ class MessageRxSession final : public IMessageRxSession, private SessionDelegate
CETL_DEBUG_ASSERT(result >= 0, "There is no way currently to get an error here.");
CETL_DEBUG_ASSERT(result > 0, "New subscription supposed to be made.");

subscription_.user_reference = static_cast<SessionDelegate*>(this);
subscription_.user_reference = static_cast<IRxSessionDelegate*>(this);
}

~MessageRxSession() final
{
::canardRxUnsubscribe(&delegate_.canard_instance(),
CanardTransferKindMessage,
static_cast<CanardPortID>(params_.subject_id));
const int8_t result = ::canardRxUnsubscribe(&delegate_.canard_instance(),
CanardTransferKindMessage,
static_cast<CanardPortID>(params_.subject_id));
(void) result;
CETL_DEBUG_ASSERT(result >= 0, "There is no way currently to get an error here.");
CETL_DEBUG_ASSERT(result > 0, "Subscription supposed to be made at constructor.");
}

private:
Expand Down Expand Up @@ -108,7 +122,7 @@ class MessageRxSession final : public IMessageRxSession, private SessionDelegate
// Nothing to do here currently.
}

// MARK: SessionDelegate
// MARK: IRxSessionDelegate

void acceptRxTransfer(const CanardRxTransfer& transfer) final
{
Expand All @@ -129,9 +143,8 @@ class MessageRxSession final : public IMessageRxSession, private SessionDelegate

// MARK: Data members:

TransportDelegate& delegate_;
const MessageRxParams params_;

TransportDelegate& delegate_;
const MessageRxParams params_;
CanardRxSubscription subscription_;
cetl::optional<MessageRxTransfer> last_rx_transfer_;

Expand Down
39 changes: 16 additions & 23 deletions include/libcyphal/transport/can/msg_tx_session.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,31 @@ namespace can
///
namespace detail
{

class MessageTxSession final : public IMessageTxSession
{
// In use to disable public construction.
// See https://seanmiddleditch.github.io/enabling-make-unique-with-private-constructors/
struct Tag
/// @brief Defines specification for making interface unique ptr.
///
struct Spec
{
explicit Tag() = default;
using Interface = IMessageTxSession;
using Concrete = MessageTxSession;

// In use to disable public construction.
// See https://seanmiddleditch.github.io/enabling-make-unique-with-private-constructors/
explicit Spec() = default;
};

public:
CETL_NODISCARD static Expected<UniquePtr<IMessageTxSession>, AnyError> make(TransportDelegate& delegate,
const MessageTxParams& params)
{
auto session = libcyphal::detail::makeUniquePtr<Tag>(delegate.memory(), Tag{}, delegate, params);
if (params.subject_id > CANARD_SUBJECT_ID_MAX)
{
return ArgumentError{};
}

auto session = libcyphal::detail::makeUniquePtr<Spec>(delegate.memory(), Spec{}, delegate, params);
if (session == nullptr)
{
return MemoryError{};
Expand All @@ -50,7 +59,7 @@ class MessageTxSession final : public IMessageTxSession
return session;
}

MessageTxSession(Tag, TransportDelegate& delegate, const MessageTxParams& params)
MessageTxSession(Spec, TransportDelegate& delegate, const MessageTxParams& params)
: delegate_{delegate}
, params_{params}
, send_timeout_{std::chrono::seconds{1}}
Expand All @@ -75,29 +84,13 @@ class MessageTxSession final : public IMessageTxSession
CETL_NODISCARD cetl::optional<AnyError> send(const TransferMetadata& metadata,
const PayloadFragments payload_fragments) final
{
// libcanard currently does not support fragmented payloads (at `canardTxPush`).
// so we need to concatenate them when there are more than one non-empty fragment.
// See https://github.com/OpenCyphal/libcanard/issues/223
//
const transport::detail::ContiguousPayload contiguous_payload{delegate_.memory(), payload_fragments};
if ((contiguous_payload.data() == nullptr) && (contiguous_payload.size() > 0))
{
return MemoryError{};
}

const TimePoint deadline = metadata.timestamp + send_timeout_;
const auto deadline_us = std::chrono::duration_cast<std::chrono::microseconds>(deadline.time_since_epoch());

const auto canard_metadata = CanardTransferMetadata{static_cast<CanardPriority>(metadata.priority),
CanardTransferKindMessage,
static_cast<CanardPortID>(params_.subject_id),
CANARD_NODE_ID_UNSET,
static_cast<CanardTransferID>(metadata.transfer_id)};

return delegate_.sendTransfer(static_cast<CanardMicrosecond>(deadline_us.count()),
canard_metadata,
contiguous_payload.data(),
contiguous_payload.size());
return delegate_.sendTransfer(metadata.timestamp + send_timeout_, canard_metadata, payload_fragments);
}

// MARK: IRunnable
Expand Down
Loading