Skip to content

Commit

Permalink
use new CETL cetl::pmr::InterfacePtr #verification #docs #sonar
Browse files Browse the repository at this point in the history
  • Loading branch information
serges147 committed May 15, 2024
1 parent a812139 commit acdf3f9
Show file tree
Hide file tree
Showing 18 changed files with 61 additions and 34 deletions.
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 "63a118441a80078ebfeabf509c342bc0480a50b2")
set(cetl_GIT_TAG "386c1c09967b20a0f0f28df52989bfd704887831")

FetchContent_Declare(
cetl
Expand Down
4 changes: 2 additions & 2 deletions include/libcyphal/runnable.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ class IRunnable
IRunnable(IRunnable&&) noexcept = delete;
IRunnable& operator=(const IRunnable&) = delete;
IRunnable& operator=(IRunnable&&) noexcept = delete;
virtual ~IRunnable() = default;

virtual void run(const TimePoint now) = 0;

protected:
IRunnable() = default;
IRunnable() = default;
~IRunnable() = default;
};

} // namespace libcyphal
Expand Down
4 changes: 2 additions & 2 deletions include/libcyphal/transport/can/msg_rx_session.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ namespace detail
/// NOSONAR cpp:S4963 for below `class MessageRxSession` - we do directly handle resources here;
/// namely: in destructor we have to unsubscribe, as well as let delegate to know this fact.
///
class MessageRxSession final : public IMessageRxSession, private IRxSessionDelegate // NOSONAR cpp:S4963
class MessageRxSession final : private IRxSessionDelegate, public IMessageRxSession // NOSONAR cpp:S4963
{
/// @brief Defines specification for making interface unique ptr.
///
Expand Down Expand Up @@ -97,7 +97,7 @@ class MessageRxSession final : public IMessageRxSession, private IRxSessionDeleg
MessageRxSession& operator=(const MessageRxSession&) = delete;
MessageRxSession& operator=(MessageRxSession&&) noexcept = delete;

~MessageRxSession() override
~MessageRxSession()
{
const int8_t result =
::canardRxUnsubscribe(&delegate_.canard_instance(), CanardTransferKindMessage, params_.subject_id);
Expand Down
4 changes: 2 additions & 2 deletions include/libcyphal/transport/can/svc_rx_sessions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ namespace detail
/// namely: in destructor we have to unsubscribe, as well as let delegate to know this fact.
///
template <typename Interface_, typename Params, CanardTransferKind TransferKind>
class SvcRxSession final : public Interface_, private IRxSessionDelegate // NOSONAR cpp:S4963
class SvcRxSession final : private IRxSessionDelegate, public Interface_ // NOSONAR cpp:S4963
{
/// @brief Defines specification for making interface unique ptr.
///
Expand Down Expand Up @@ -105,7 +105,7 @@ class SvcRxSession final : public Interface_, private IRxSessionDelegate // NOS
SvcRxSession& operator=(const SvcRxSession&) = delete;
SvcRxSession& operator=(SvcRxSession&&) noexcept = delete;

~SvcRxSession() override
~SvcRxSession()
{
const int8_t result = ::canardRxUnsubscribe(&delegate_.canard_instance(), TransferKind, params_.service_id);
(void) result;
Expand Down
9 changes: 6 additions & 3 deletions include/libcyphal/transport/can/transport.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ namespace can
{

class ICanTransport : public ITransport
{};
{
protected:
~ICanTransport() = default;
};

/// Internal implementation details of the CAN transport.
/// Not supposed to be used directly by the users of the library.
Expand All @@ -55,7 +58,7 @@ namespace detail
/// NOSONAR cpp:S4963 for below `class TransportImpl` - we do directly handle resources here;
/// namely: in destructor we have to unsubscribe, as well as let delegate to know this fact.
///
class TransportImpl final : public ICanTransport, private TransportDelegate // NOSONAR cpp:S4963
class TransportImpl final : private TransportDelegate, public ICanTransport // NOSONAR cpp:S4963
{
/// @brief Defines specification for making interface unique ptr.
///
Expand Down Expand Up @@ -155,7 +158,7 @@ class TransportImpl final : public ICanTransport, private TransportDelegate //
TransportImpl& operator=(const TransportImpl&) = delete;
TransportImpl& operator=(TransportImpl&&) noexcept = delete;

~TransportImpl() override
~TransportImpl()
{
for (Media& media : media_array_)
{
Expand Down
7 changes: 7 additions & 0 deletions include/libcyphal/transport/msg_sessions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ class IMessageRxSession : public IRxSession
///
virtual cetl::optional<MessageRxTransfer> receive() = 0;

protected:
~IMessageRxSession() = default;

}; // IMessageRxSession

class IMessageTxSession : public ITxSession
Expand All @@ -58,6 +61,10 @@ class IMessageTxSession : public ITxSession
///
virtual cetl::optional<AnyError> send(const TransferMetadata& metadata,
const PayloadFragments payload_fragments) = 0;

protected:
~IMessageTxSession() = default;

}; // IMessageTxSession

} // namespace transport
Expand Down
9 changes: 8 additions & 1 deletion include/libcyphal/transport/session.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,17 @@ namespace transport

class ISession : public IRunnable
{
public:
protected:
~ISession() = default;
};

class IRxSession : public ISession
{
public:
virtual void setTransferIdTimeout(const Duration timeout) = 0;

protected:
~IRxSession() = default;
};

class ITxSession : public ISession
Expand All @@ -36,6 +40,9 @@ class ITxSession : public ISession
/// @param timeout - Positive duration for transmission timeout. Default value is 1 second.
///
virtual void setSendTimeout(const Duration timeout) = 0;

protected:
~ITxSession() = default;
};

} // namespace transport
Expand Down
15 changes: 15 additions & 0 deletions include/libcyphal/transport/svc_sessions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,18 @@ class ISvcRxSession : public IRxSession
/// @return A service transfer if available; otherwise an empty optional.
///
virtual cetl::optional<ServiceRxTransfer> receive() = 0;

protected:
~ISvcRxSession() = default;
};

class IRequestRxSession : public ISvcRxSession
{
public:
virtual RequestRxParams getParams() const noexcept = 0;

protected:
~IRequestRxSession() = default;
};

class IRequestTxSession : public ITxSession
Expand All @@ -73,12 +79,18 @@ class IRequestTxSession : public ITxSession
///
virtual cetl::optional<AnyError> send(const TransferMetadata& metadata,
const PayloadFragments payload_fragments) = 0;

protected:
~IRequestTxSession() = default;
};

class IResponseRxSession : public ISvcRxSession
{
public:
virtual ResponseRxParams getParams() const noexcept = 0;

protected:
~IResponseRxSession() = default;
};

class IResponseTxSession : public ITxSession
Expand All @@ -94,6 +106,9 @@ class IResponseTxSession : public ITxSession
///
virtual cetl::optional<AnyError> send(const ServiceTransferMetadata& metadata,
const PayloadFragments payload_fragments) = 0;

protected:
~IResponseTxSession() = default;
};

} // namespace transport
Expand Down
3 changes: 3 additions & 0 deletions include/libcyphal/transport/transport.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ class ITransport : public IRunnable
///
virtual Expected<UniquePtr<IResponseTxSession>, AnyError> makeResponseTxSession(const ResponseTxParams& params) = 0;

protected:
~ITransport() = default;

}; // ITransport

} // namespace transport
Expand Down
5 changes: 4 additions & 1 deletion include/libcyphal/transport/udp/transport.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ namespace udp
{

class IUdpTransport : public ITransport
{};
{
protected:
~IUdpTransport() = default;
};

/// Internal implementation details of the UDP transport.
/// Not supposed to be used directly by the users of the library.
Expand Down
13 changes: 4 additions & 9 deletions include/libcyphal/types.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

#include <cetl/cetl.hpp>
#include <cetl/pf17/cetlpf.hpp>
#include <cetl/pmr/memory.hpp>
#include <cetl/pmr/interface_ptr.hpp>
#include <cetl/variable_length_array.hpp>

#include <chrono>
Expand Down Expand Up @@ -50,7 +50,7 @@ using TimePoint = MonotonicClock::time_point;
using Duration = MonotonicClock::duration;

template <typename T>
using UniquePtr = cetl::pmr::Factory::unique_ptr_t<cetl::pmr::polymorphic_allocator<T>>;
using UniquePtr = cetl::pmr::InterfacePtr<T>;

// TODO: Maybe introduce `cetl::expected` at CETL repo.
template <typename Success, typename Failure>
Expand All @@ -68,13 +68,8 @@ using VarArray = cetl::VariableLengthArray<T, PmrAllocator<T>>;
template <typename Spec, typename... Args>
CETL_NODISCARD UniquePtr<typename Spec::Interface> makeUniquePtr(cetl::pmr::memory_resource& memory, Args&&... args)
{
PmrAllocator<typename Spec::Concrete> allocator{&memory};
auto interface_deleter = typename UniquePtr<typename Spec::Interface>::deleter_type{allocator, 1};

auto concrete = cetl::pmr::Factory::make_unique(allocator, std::forward<Args>(args)...);
auto interface = UniquePtr<typename Spec::Interface>{concrete.release(), interface_deleter};

return interface;
return cetl::pmr::InterfaceFactory::make_unique<
typename Spec::Interface>(PmrAllocator<typename Spec::Concrete>{&memory}, std::forward<Args>(args)...);
}

} // namespace detail
Expand Down
3 changes: 1 addition & 2 deletions test/unittest/transport/can/test_can_msg_rx_session.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ class TestCanMsgRxSession : public testing::Test
void TearDown() override
{
EXPECT_THAT(mr_.allocations, IsEmpty());
// TODO: Uncomment this when PMR deleter is fixed.
// EXPECT_EQ(mr_.total_allocated_bytes, mr_.total_deallocated_bytes);
EXPECT_THAT(mr_.total_allocated_bytes, mr_.total_deallocated_bytes);
}

TimePoint now() const
Expand Down
3 changes: 1 addition & 2 deletions test/unittest/transport/can/test_can_msg_tx_session.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,7 @@ class TestCanMsgTxSession : public testing::Test
void TearDown() override
{
EXPECT_THAT(mr_.allocations, IsEmpty());
// TODO: Uncomment this when PMR deleter is fixed.
// EXPECT_EQ(mr_.total_allocated_bytes, mr_.total_deallocated_bytes);
EXPECT_THAT(mr_.total_allocated_bytes, mr_.total_deallocated_bytes);
}

TimePoint now() const
Expand Down
3 changes: 1 addition & 2 deletions test/unittest/transport/can/test_can_svc_rx_sessions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,7 @@ class TestCanSvcRxSessions : public testing::Test
void TearDown() override
{
EXPECT_THAT(mr_.allocations, IsEmpty());
// TODO: Uncomment this when PMR deleter is fixed.
// EXPECT_EQ(mr_.total_allocated_bytes, mr_.total_deallocated_bytes);
EXPECT_THAT(mr_.total_allocated_bytes, mr_.total_deallocated_bytes);
}

TimePoint now() const
Expand Down
3 changes: 1 addition & 2 deletions test/unittest/transport/can/test_can_svc_tx_sessions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ class TestCanSvcTxSessions : public testing::Test
void TearDown() override
{
EXPECT_THAT(mr_.allocations, IsEmpty());
// TODO: Uncomment this when PMR deleter is fixed.
// EXPECT_EQ(mr_.total_allocated_bytes, mr_.total_deallocated_bytes);
EXPECT_THAT(mr_.total_allocated_bytes, mr_.total_deallocated_bytes);
}

TimePoint now() const
Expand Down
3 changes: 1 addition & 2 deletions test/unittest/transport/can/test_can_transport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,7 @@ class TestCanTransport : public testing::Test
void TearDown() override
{
EXPECT_THAT(mr_.allocations, IsEmpty());
// TODO: Uncomment this when PMR deleter is fixed.
// EXPECT_EQ(mr_.total_allocated_bytes, mr_.total_deallocated_bytes);
EXPECT_THAT(mr_.total_allocated_bytes, mr_.total_deallocated_bytes);
}

TimePoint now() const
Expand Down
2 changes: 1 addition & 1 deletion test/unittest/transport/test_contiguous_payload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class TestContiguousPayload : public testing::Test
void TearDown() override
{
EXPECT_THAT(mr_.allocations, IsEmpty());
EXPECT_EQ(mr_.total_allocated_bytes, mr_.total_deallocated_bytes);
EXPECT_THAT(mr_.total_allocated_bytes, mr_.total_deallocated_bytes);
}

// MARK: Data members:
Expand Down
3 changes: 1 addition & 2 deletions test/unittest/transport/udp/test_udp_transport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ class TestUpdTransport : public testing::Test
void TearDown() override
{
EXPECT_THAT(mr_.allocations, IsEmpty());
// TODO: Uncomment this when PMR deleter is fixed.
// EXPECT_EQ(mr_.total_allocated_bytes, mr_.total_deallocated_bytes);
EXPECT_THAT(mr_.total_allocated_bytes, mr_.total_deallocated_bytes);
}

// MARK: Data members:
Expand Down

0 comments on commit acdf3f9

Please sign in to comment.