Skip to content

Commit f66b8a6

Browse files
authored
Use canardTxPoll; MediaPayload::Ownership (#409)
1 parent 914d85d commit f66b8a6

File tree

14 files changed

+179
-159
lines changed

14 files changed

+179
-159
lines changed

include/libcyphal/application/registry/register.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ enum class SetError : std::uint8_t
5151

5252
/// Defines interface for a register.
5353
///
54-
class IRegister : public cavl::Node<IRegister>
54+
class IRegister : public common::cavl::Node<IRegister>
5555
{
5656
// 1AD1885B-954B-48CF-BAC4-FA0A251D3FC0
5757
// clang-format off

include/libcyphal/application/registry/registry_impl.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,8 @@ class Registry final : public IIntrospectableRegistry
159159
[key = IRegister::Key{name}](const IRegister& other) { return other.compareBy(key); });
160160
}
161161

162-
cetl::pmr::memory_resource& memory_;
163-
cavl::Tree<IRegister> registers_tree_;
162+
cetl::pmr::memory_resource& memory_;
163+
common::cavl::Tree<IRegister> registers_tree_;
164164

165165
}; // Registry
166166

include/libcyphal/common/cavl/cavl.hpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@
4646

4747
// NOLINTBEGIN(cppcoreguidelines-pro-bounds-constant-array-index)
4848

49+
namespace libcyphal
50+
{
51+
namespace common
52+
{
4953
namespace cavl
5054
{
5155
template <typename Derived>
@@ -788,7 +792,7 @@ class Tree final // NOSONAR cpp:S3624
788792
{
789793
public:
790794
/// Helper alias of the compatible node type.
791-
using NodeType = ::cavl::Node<Derived>;
795+
using NodeType = Node<Derived>;
792796
using DerivedType = Derived;
793797

794798
Tree() = default;
@@ -998,5 +1002,7 @@ class Tree final // NOSONAR cpp:S3624
9981002
};
9991003

10001004
} // namespace cavl
1005+
} // namespace common
1006+
} // namespace libcyphal
10011007

10021008
// NOLINTEND(cppcoreguidelines-pro-bounds-constant-array-index)

include/libcyphal/platform/single_threaded_executor.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ class SingleThreadedExecutor : public IExecutor
111111
}
112112

113113
protected:
114-
class CallbackNode : public cavl::Node<CallbackNode>, public Callback::Interface
114+
class CallbackNode : public libcyphal::common::cavl::Node<CallbackNode>, public Callback::Interface
115115
{
116116
public:
117117
CallbackNode(SingleThreadedExecutor& executor, Callback::Function&& function)
@@ -272,7 +272,7 @@ class SingleThreadedExecutor : public IExecutor
272272
// MARK: - Data members:
273273

274274
/// Holds AVL tree of registered callback node, sorted by the next execution time.
275-
cavl::Tree<CallbackNode> callback_nodes_;
275+
common::cavl::Tree<CallbackNode> callback_nodes_;
276276

277277
}; // SingleThreadedExecutor
278278

include/libcyphal/presentation/client_impl.hpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ namespace presentation
3434
namespace detail
3535
{
3636

37-
class SharedClient : public cavl::Node<SharedClient>, public SharedObject
37+
class SharedClient : public common::cavl::Node<SharedClient>, public SharedObject
3838
{
3939
public:
4040
using Node::remove;
@@ -388,9 +388,9 @@ class SharedClient : public cavl::Node<SharedClient>, public SharedObject
388388
const UniquePtr<transport::IRequestTxSession> svc_request_tx_session_;
389389
const UniquePtr<transport::IResponseRxSession> svc_response_rx_session_;
390390
const transport::ResponseRxParams response_rx_params_;
391-
cavl::Tree<CallbackNode> cb_nodes_by_transfer_id_;
391+
common::cavl::Tree<CallbackNode> cb_nodes_by_transfer_id_;
392392
TimePoint nearest_deadline_;
393-
cavl::Tree<TimeoutNode> timeout_nodes_by_deadline_;
393+
common::cavl::Tree<TimeoutNode> timeout_nodes_by_deadline_;
394394
IExecutor::Callback::Any nearest_deadline_callback_;
395395

396396
}; // SharedClient

include/libcyphal/presentation/presentation.hpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -727,14 +727,14 @@ class Presentation final : private detail::IPresentationDelegate
727727

728728
// MARK: Data members:
729729

730-
cetl::pmr::memory_resource& memory_;
731-
IExecutor& executor_;
732-
transport::ITransport& transport_;
733-
cavl::Tree<detail::SharedClient> shared_client_nodes_;
734-
cavl::Tree<detail::PublisherImpl> publisher_impl_nodes_;
735-
cavl::Tree<detail::SubscriberImpl> subscriber_impl_nodes_;
736-
detail::UnRefNode unreferenced_nodes_;
737-
IExecutor::Callback::Any unref_nodes_deleter_callback_;
730+
cetl::pmr::memory_resource& memory_;
731+
IExecutor& executor_;
732+
transport::ITransport& transport_;
733+
common::cavl::Tree<detail::SharedClient> shared_client_nodes_;
734+
common::cavl::Tree<detail::PublisherImpl> publisher_impl_nodes_;
735+
common::cavl::Tree<detail::SubscriberImpl> subscriber_impl_nodes_;
736+
detail::UnRefNode unreferenced_nodes_;
737+
IExecutor::Callback::Any unref_nodes_deleter_callback_;
738738

739739
}; // Presentation
740740

include/libcyphal/presentation/publisher_impl.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ namespace presentation
3232
namespace detail
3333
{
3434

35-
class PublisherImpl final : public cavl::Node<PublisherImpl>, public SharedObject
35+
class PublisherImpl final : public common::cavl::Node<PublisherImpl>, public SharedObject
3636
{
3737
public:
3838
using Node::remove;

include/libcyphal/presentation/subscriber_impl.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ namespace presentation
3333
namespace detail
3434
{
3535

36-
class SubscriberImpl final : public cavl::Node<SubscriberImpl>, public SharedObject
36+
class SubscriberImpl final : public common::cavl::Node<SubscriberImpl>, public SharedObject
3737
{
3838
public:
3939
using Node::remove;
@@ -304,7 +304,7 @@ class SubscriberImpl final : public cavl::Node<SubscriberImpl>, public SharedObj
304304
ITimeProvider& time_provider_;
305305
const UniquePtr<transport::IMessageRxSession> msg_rx_session_;
306306
const transport::PortId subject_id_;
307-
cavl::Tree<CallbackNode> callback_nodes_;
307+
common::cavl::Tree<CallbackNode> callback_nodes_;
308308
CallbackNode* next_cb_node_;
309309

310310
}; // SubscriberImpl

include/libcyphal/transport/can/can_transport_impl.hpp

Lines changed: 73 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,7 @@ class TransportImpl final : private TransportDelegate, public ICanTransport
324324
return MemoryError{};
325325
}
326326

327+
const auto now_us = std::chrono::duration_cast<std::chrono::microseconds>(executor_.now().time_since_epoch());
327328
const auto deadline_us = std::chrono::duration_cast<std::chrono::microseconds>(deadline.time_since_epoch());
328329

329330
for (Media& media : media_array_)
@@ -335,7 +336,8 @@ class TransportImpl final : private TransportDelegate, public ICanTransport
335336
&canardInstance(),
336337
static_cast<CanardMicrosecond>(deadline_us.count()),
337338
&metadata,
338-
{payload.size(), payload.data()}); // NOSONAR cpp:S5356
339+
{payload.size(), payload.data()}, // NOSONAR cpp:S5356
340+
static_cast<CanardMicrosecond>(now_us.count()));
339341

340342
cetl::optional<AnyFailure> failure =
341343
tryHandleTransientCanardResult<TransientErrorReport::CanardTxPush>(media, result);
@@ -572,77 +574,84 @@ class TransportImpl final : private TransportDelegate, public ICanTransport
572574
}
573575
}
574576

575-
/// @brief Tries to push next frame from TX queue to media.
576-
///
577-
void pushNextFrameToMedia(Media& media)
577+
std::int8_t handleMediaTxFrame(Media& media, const CanardMicrosecond deadline, CanardMutableFrame& frame)
578578
{
579-
TimePoint tx_deadline;
580-
while (CanardTxQueueItem* const tx_item = peekFirstValidTxItem(media.canard_tx_queue(), tx_deadline))
579+
//
580+
// Move the payload from the frame to the media payload - `media.push` might take ownership of it.
581+
// No Sonar `cpp:S5356` and `cpp:S5357` b/c we integrate here with C libcanard API.
582+
//
583+
MediaPayload payload{frame.payload.size,
584+
static_cast<cetl::byte*>(frame.payload.data), // NOSONAR cpp:S5356 cpp:S5357
585+
frame.payload.allocated_size,
586+
&media.interface().getTxMemoryResource()};
587+
frame.payload = {0, nullptr, 0};
588+
589+
auto push_result = media.interface().push(TimePoint{std::chrono::microseconds{deadline}}, //
590+
frame.extended_can_id,
591+
payload);
592+
593+
if (const auto* const push = cetl::get_if<IMedia::PushResult::Success>(&push_result))
581594
{
582-
// Move the payload from the frame to the media payload - `media.push` might take ownership of it.
583-
// No Sonar `cpp:S5356` and `cpp:S5357` b/c we integrate here with C libcanard API.
584-
//
585-
auto& frame_payload = tx_item->frame.payload;
586-
MediaPayload payload{frame_payload.size,
587-
static_cast<cetl::byte*>(frame_payload.data), // NOSONAR cpp:S5356 cpp:S5357
588-
frame_payload.allocated_size,
589-
&media.interface().getTxMemoryResource()};
590-
frame_payload = {0, nullptr, 0};
591-
592-
auto push_result = media.interface().push(tx_deadline, tx_item->frame.extended_can_id, payload);
593-
594-
// In case of media push error, we are going to drop this problematic frame
595-
// (b/c it looks like media can't handle this frame),
596-
// but we will continue to process with another transfer frame.
597-
// Note that media not being ready/able to push a frame just yet (aka temporary)
598-
// is not reported as an error (see `is_pushed` below).
599-
//
600-
auto* const push_failure = cetl::get_if<IMedia::PushResult::Failure>(&push_result);
601-
if (nullptr == push_failure)
595+
if (!push->is_accepted)
602596
{
603-
const auto push = cetl::get<IMedia::PushResult::Success>(push_result);
604-
if (push.is_accepted)
605-
{
606-
popAndFreeCanardTxQueueItem(media.canard_tx_queue(),
607-
canardInstance(),
608-
tx_item,
609-
false /* single frame */);
610-
}
611-
else
612-
{
613-
// Media has not accepted the frame, so we need return original payload back to the item,
614-
// so that in the future potential retry could try to push it again.
615-
const auto org_payload = payload.release();
616-
frame_payload.size = std::get<0>(org_payload);
617-
frame_payload.data = std::get<1>(org_payload);
618-
frame_payload.allocated_size = std::get<2>(org_payload);
619-
}
620-
621-
// If needed schedule (recursively!) next frame to push.
622-
// Already existing callback will be called by executor when media TX is ready to push more.
623-
//
624-
if (!media.tx_callback())
625-
{
626-
media.tx_callback() = media.interface().registerPushCallback([this, &media](const auto&) {
627-
//
628-
pushNextFrameToMedia(media);
629-
});
630-
}
631-
return;
597+
// Media has not accepted the frame, so we need return original payload back to the item,
598+
// so that in the future potential retry could try to push it again.
599+
// No Sonar `cpp:S5356` b/c we need to pass payload as a raw data to the libcanard.
600+
const auto org_payload = payload.release();
601+
frame.payload.size = org_payload.size;
602+
frame.payload.data = org_payload.data; // NOSONAR cpp:S5356
603+
frame.payload.allocated_size = org_payload.allocated_size;
632604
}
633605

634-
// Release whole problematic transfer from the TX queue,
635-
// so that other transfers in TX queue have their chance.
636-
// Otherwise, we would be stuck in an execution loop trying to send the same frame.
637-
popAndFreeCanardTxQueueItem(media.canard_tx_queue(), canardInstance(), tx_item, true /* whole transfer */);
606+
// If needed schedule (recursively!) next frame to push.
607+
// Already existing callback will be called by executor when media TX is ready to push more.
608+
//
609+
if (!media.tx_callback())
610+
{
611+
media.tx_callback() = media.interface().registerPushCallback([this, &media](const auto&) {
612+
//
613+
pushNextFrameToMedia(media);
614+
});
615+
}
616+
return push->is_accepted ? 1 : 0;
617+
}
638618

639-
using Report = TransientErrorReport::MediaPush;
640-
tryHandleTransientMediaFailure<Report>(media, std::move(*push_failure));
619+
using Report = TransientErrorReport::MediaPush;
620+
tryHandleTransientMediaFailure<Report>(media, cetl::get<IMedia::PushResult::Failure>(std::move(push_result)));
621+
return -1;
622+
}
641623

642-
} // for a valid tx item
624+
/// @brief Tries to push next frame from TX queue to media.
625+
///
626+
void pushNextFrameToMedia(Media& media)
627+
{
628+
auto frame_handler = [this, &media](const CanardMicrosecond deadline,
629+
CanardMutableFrame& frame) -> std::int8_t {
630+
//
631+
return handleMediaTxFrame(media, deadline, frame);
632+
};
643633

644-
// There is nothing to send anymore, so we are done with this media TX - no more callbacks for now.
645-
media.tx_callback().reset();
634+
// In case of a media failure we gonna try to push another frame from the next transfer in the queue, so
635+
// that at least (and at most) one new frame will be succesfully attempted to be pushed in the end.
636+
// Everytime we poll the queue, its size surely decrements (when `result != 0`),
637+
// so there is no risk of infinite loop here.
638+
//
639+
std::int8_t result = -1;
640+
while (result < 0)
641+
{
642+
// No Sonar `cpp:S5356` & `cpp:S5356` b/c we integrate with Canard C api.
643+
result = ::canardTxPoll( //
644+
&media.canard_tx_queue(),
645+
&canardInstance(),
646+
static_cast<CanardMicrosecond>(executor_.now().time_since_epoch().count()),
647+
&frame_handler, // NOSONAR cpp:S5356
648+
[](auto* const user_reference, const auto deadline, auto* frame) {
649+
//
650+
auto* const frame_handler_ptr =
651+
static_cast<decltype(frame_handler)*>(user_reference); // NOSONAR cpp:S5356, cpp:S5357
652+
return (*frame_handler_ptr)(deadline, *frame);
653+
});
654+
}
646655
}
647656

648657
/// @brief Tries to peek the first TX item from the media TX queue which is not expired.

0 commit comments

Comments
 (0)