[Feature Request] Retrieve metadata from received msg from a subscription

[rebeccaRossRobotics]

:bug: Bug Report

Describe the bug

In a previous version of 107-Arduino-Cyphal the calback function for a subscription also passed through the CanardRxTransfer which allowed access to the remote node id. I'm looking to do this in the latest version of 107-Arduino-Cyphal but i can't seem to find how I can do this? Is it possible? If so how do I do this?

Minimum example code to replicate the bug Node and Subscription declaration

Node node_hdl(
  node_heap.data(),
  node_heap.size(),
  micros,
  [] (CanardFrame const & frame) { return mcp2515.transmit(frame); },
  OPEN_CYPHAL_NODE_ID);

Subscription heartbeat_subscription = node_hdl.create_subscription<uavcan::node::Heartbeat_1_0>(
  heartbeat::on_heartbeat_received);

Subscription callback code

void on_heartbeat_received(const uavcan::node::Heartbeat_1_0 & msg)
{
  if (DEBUG) {
    char msg_buf[64];
    snprintf(msg_buf, sizeof(msg_buf),
            "Uptime = %d, Health = %d, Mode = %d, VSSC = %d",
            msg.uptime, msg.health.value, msg.mode.value, msg.vendor_specific_status_code);

    Serial.println(msg_buf);
  }

* this is where i'd like to be able to do some filtering based on the remote node id *
}

Minimum configuration to replicate the bug Please provide the configuration data used to build your application:

• Board being used: RPI Pico 2040
• Build Toolchain: Arduino IDE 1.8.19
• Library Version (of all involved libraries): 107-Arduino-Cyphal 3.1.0, 107-Arduino-MCP2515 1.4.0, 107-CriticalSection 1.0.0

[aentinger]

The metadata has been eliminated from the callback from the perspective that Cyphal should be transport-agnostic (and a CAN node ID is very much transport-specific). Isn't that the Cypal design philosophy @pavel-kirienko?

However, I do recognise the need for having access to metadata if one wants to implement i.e. a heartbeat monitor which whould keep track of all nodes within the Cyphal network, very much as yakut does.

I'd be consequently open to accept a well-crafted PR that overloads the callback (one time only providing the deserialized message and the other time providing both deserialized message and metadata). Would you be willing to work on that @rebeccaRossRobotics ?

[pavel-kirienko]

Some metadata is transport-agnostic. A node-ID is one such example. While it is rarely needed, sometimes it is, and the Heartbeat message is one such example.

[aentinger]

A node-ID is one such example.

How do you specify a node ID for Cyphal/UDP? Is it limited to 0-125 as it is for CAN? If so, isn't it transport specific then?

Not looking to bike-shed, only to clarify 😁

[pavel-kirienko]

A node-ID is always some number from 0 (inclusive) up to some upper boundary which is transport-specific but is guaranteed to be not lower than 127. Relevant excerpts:

Section 1.4 Capabilities:

The maximum number of nodes per logical network is dependent on the transport protocol in use, but it is guaranteed to be not less than 128.

A generic implementation like this library would simply represent a node-ID as a 16-bit unsigned integer.

[rebeccaRossRobotics]

@aentinger Yep I can work on that :)

[aentinger]

Thank you, this is illuminating. A extended subscription callback API could then look like this (mocking stuff up a bit):

#if LIBCANARD
#define CanardNodeID CyphalNodeID;
#elif LIBUDPARD
#define UdpardNodeID CyphalNodeID;
#else
# error "We only support CAN or UDP as Cyphal transport layer"
#endif

template <typename T>
using OnReceiveCallback_Default= std::function<void(T const &)>;

template <typename T>
using OnReceiveCallback_Ext = std::function<void(T const &, CyphalNodeId const)>;

What do you think?

[aentinger]

@aentinger Yep I can work on that :)

Terrrific! @pavel-kirienko and I shall help with review and stuff.

[pavel-kirienko]

What do you think?

It's always a good idea to define a new strong type and use that:

struct TransferMetadata final
{
    CyphalNodeId node_id;
    // More stuff may appear here in the future!
};

using OnReceiveCallback_Ext = std::function<void(T const &, TransferMetadata const&)>;

[aentinger]

FIxed by #235.