Fix for issue #412 Only one single RPC client is possible per port #415
Conversation
…ponse rx transfer
serges147
changed the title
thirtytwobits
left a comment
thirtytwobits
left a comment
There was a problem hiding this comment.
Looking good. Mostly cosmetic comments.
include/libcyphal/types.hpp
Outdated
| } | ||
|
|
||
| template <typename Action> | ||
| bool performWithoutThrowing(Action&& action) noexcept |
include/libcyphal/types.hpp
Outdated
| std::forward<Action>(action)(); | ||
| return true; | ||
| #if defined(__cpp_exceptions) | ||
| } catch (...) |
There was a problem hiding this comment.
catch all isn't acceptable. Let's templatize this with base exceptions that are thrown.
There was a problem hiding this comment.
we should also add a debug macro to allow inserting asserts, breakpoints, or logging for any squelched exception.
| return rx_rpc_dispatcher_; | ||
| } | ||
|
|
||
| void listenForRxRpcPort(UdpardRxRPCPort& rpc_port, const RequestRxParams& params) |
There was a problem hiding this comment.
you could avoid the code duplication here by specializing on a private template method with a "is_request" parameter.
include/libcyphal/types.hpp
Outdated
| #if defined(__cpp_exceptions) | ||
| } catch (const Exception& ex) | ||
| { | ||
| CETL_DEBUG_ASSERT(false, ex.what()); |
There was a problem hiding this comment.
Sorry but I was thinking this was distinct from the debug assert. Failures may be recoverable and this would lower the utility of debug assert which should be used for "this should never happen" cases only. Uh...just remove this line and ignore my previous request. Sorry.
|
) (#417) 1. Response Rx Session now doesn't own lizard's rpc port subscription - instead now session references the shared subscription (with reference counting). 2. Transport delegates now manage subscriptions. 3. CAN Response Rx Sessions now stored in cavl tree (for UDP it was already like this) - matching now by both port and node ids (previously it was by the port only). 4. CAN filters now also are made by the delegate (b/c it knows about subscriptions (see bullet # 2). 5. Extended unit tests to cover multiple response Rx sessions (both CAN and UDP). Also covered unsolicited responses. Github's Hide whitespaces is recommended.
Github's Hide whitespaces is recommended.