Skip to content

Commit e3f6889

Browse files
pavel-kirienkopavel-kirienko
committed
Migrating to 16-bit subject-ID in v1.1, was 17 bits
1 parent 2012295 commit e3f6889

File tree

7 files changed

+108
-54
lines changed

7 files changed

+108
-54
lines changed

‎libcanard/canard.c‎

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -915,18 +915,32 @@ bool canard_publish(canard_t* const self,
915915
const canard_us_t deadline,
916916
const uint_least8_t iface_bitmap,
917917
const canard_prio_t priority,
918-
const uint32_t subject_id,
918+
const uint16_t subject_id,
919919
const uint_least8_t transfer_id,
920920
const canard_bytes_chain_t payload,
921921
const canard_user_context_t context)
922922
{
923923
bool ok =
924924
(self != NULL) && (priority < CANARD_PRIO_COUNT) && bytes_chain_valid(payload) &&
925-
(((iface_bitmap & CANARD_IFACE_BITMAP_ALL) != 0) && ((iface_bitmap & CANARD_IFACE_BITMAP_ALL) == iface_bitmap)) &&
926-
(subject_id <= CANARD_SUBJECT_ID_MAX);
925+
(((iface_bitmap & CANARD_IFACE_BITMAP_ALL) != 0) && ((iface_bitmap & CANARD_IFACE_BITMAP_ALL) == iface_bitmap));
927926
if (ok) {
928-
const uint32_t can_id = (((uint32_t)priority) << PRIO_SHIFT) | (subject_id << 8U) | (UINT32_C(1) << 7U);
929-
tx_transfer_t* const tr = tx_transfer_new(self, deadline, can_id, self->tx.fd, context);
927+
// v1.1 message format extends the subject-ID to 16 bits, using bit 7 as 1 to discriminate from v1.0,
928+
// and designating the anonymous bit as reserved=0. ID bit layout:
929+
//
930+
// 28 27 26 25 24 23 22 21 20 19 18 17 16 15 14 13 12 11 10 9 8 7 6 5 4 3 2 1 0
931+
// |prio[3] |sv| 0| subject_id[16] | 1| source_node_id[7] |
932+
//
933+
// In DSDL notation:
934+
//
935+
// uint3 priority
936+
// bool service_not_message # =0
937+
// bool reserved_24 # =0, was anonymous
938+
// uint16 subject_id
939+
// bool reserved_7 # =1, version discrimination
940+
// uint7 source_node_id
941+
const uint32_t can_id =
942+
(((uint32_t)priority) << PRIO_SHIFT) | ((uint32_t)subject_id << 8U) | (UINT32_C(1) << 7U);
943+
tx_transfer_t* const tr = tx_transfer_new(self, deadline, can_id, self->tx.fd, context);
930944
ok = (tr != NULL) && tx_push(self, tr, false, iface_bitmap, transfer_id, payload, CRC_INITIAL);
931945
}
932946
return ok;
@@ -1078,7 +1092,7 @@ typedef struct
10781092
canard_prio_t priority;
10791093
canard_kind_t kind;
10801094

1081-
uint32_t port_id; // in v0 this stores the data type ID.
1095+
uint16_t port_id; // in v0 this stores the data type ID.
10821096

10831097
byte_t dst;
10841098
byte_t src;
@@ -1153,8 +1167,10 @@ static byte_t rx_parse(const uint32_t can_id,
11531167
out_v1->dst = CANARD_NODE_ID_ANONYMOUS;
11541168
const bool is_1v1 = (can_id & (UINT32_C(1) << 7U)) != 0U;
11551169
if (is_1v1) {
1156-
out_v1->port_id = (can_id >> 8U) & CANARD_SUBJECT_ID_MAX;
1157-
out_v1->kind = canard_kind_1v1_message;
1170+
const bool bit_24 = (can_id & (UINT32_C(1) << 24U)) != 0U;
1171+
is_v1 = is_v1 && !bit_24; // was anonymous
1172+
out_v1->port_id = (can_id >> 8U) & CANARD_SUBJECT_ID_MAX;
1173+
out_v1->kind = canard_kind_1v1_message;
11581174
} else {
11591175
is_v1 = is_v1 && !bit_23;
11601176
out_v1->port_id = (can_id >> 8U) & CANARD_SUBJECT_ID_MAX_1v0;
@@ -1572,7 +1588,7 @@ static void rx_session_update(canard_subscription_t* const sub,
15721588

15731589
static int32_t rx_subscription_cavl_compare(const void* const user, const canard_tree_t* const node)
15741590
{
1575-
return ((int32_t)(*(const uint32_t*)user)) - ((int32_t)((const canard_subscription_t*)(const void*)node)->port_id);
1591+
return ((int32_t)(*(const uint16_t*)user)) - ((int32_t)((const canard_subscription_t*)(const void*)node)->port_id);
15761592
}
15771593

15781594
// Locates the appropriate subscription if the destination is matching and there is a subscription.
@@ -1594,26 +1610,24 @@ static canard_filter_t rx_filter_for_subscription(const canard_t* const self, co
15941610
const uint32_t id = self->node_id & CANARD_NODE_ID_MAX;
15951611
switch (sub->kind) {
15961612
case canard_kind_1v1_message:
1597-
CANARD_ASSERT(sub->port_id <= CANARD_SUBJECT_ID_MAX);
1598-
f.extended_can_id = (sub->port_id << 8U) | (UINT32_C(1) << 7U);
1613+
f.extended_can_id = ((uint32_t)sub->port_id << 8U) | (UINT32_C(1) << 7U);
15991614
f.extended_mask = 0x03FFFF80U;
16001615
break;
16011616
case canard_kind_1v0_message:
16021617
CANARD_ASSERT(sub->port_id <= CANARD_SUBJECT_ID_MAX_1v0);
1603-
f.extended_can_id = sub->port_id << 8U;
1618+
f.extended_can_id = (uint32_t)sub->port_id << 8U;
16041619
f.extended_mask = 0x029fff80U;
16051620
break;
16061621
case canard_kind_1v0_response:
16071622
case canard_kind_1v0_request: {
16081623
CANARD_ASSERT(sub->port_id <= CANARD_SERVICE_ID_MAX);
16091624
const uint32_t rnr = (sub->kind == canard_kind_1v0_request) ? (UINT32_C(1) << 24U) : 0U;
1610-
f.extended_can_id = (UINT32_C(1) << 25U) | rnr | (sub->port_id << 14U) | (id << 7U);
1625+
f.extended_can_id = (UINT32_C(1) << 25U) | rnr | ((uint32_t)sub->port_id << 14U) | (id << 7U);
16111626
f.extended_mask = 0x03FFFF80U;
16121627
break;
16131628
}
16141629
case canard_kind_0v1_message:
1615-
CANARD_ASSERT(sub->port_id <= 0xFFFFU);
1616-
f.extended_can_id = (sub->port_id & 0xFFFFU) << 8U;
1630+
f.extended_can_id = (uint32_t)sub->port_id << 8U;
16171631
f.extended_mask = 0x00FFFF80;
16181632
break;
16191633
case canard_kind_0v1_response:

‎libcanard/canard.h‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ extern "C"
5353
#define CANARD_IFACE_BITMAP_ALL ((1U << CANARD_IFACE_COUNT) - 1U)
5454

5555
/// Parameter ranges are inclusive; the lower bound is zero for all.
56-
#define CANARD_SUBJECT_ID_MAX 0x1FFFFUL
57-
#define CANARD_SUBJECT_ID_MAX_1v0 8191U // Cyphal v1.0 supports only 13-bit subject-IDs.
56+
#define CANARD_SUBJECT_ID_MAX 0xFFFFU // Applies to Cyphal v1.1 and UAVCAN v0/DroneCAN message data type IDs.
57+
#define CANARD_SUBJECT_ID_MAX_1v0 8191U // Cyphal v1.0 supports only 13-bit subject-IDs.
5858
#define CANARD_SERVICE_ID_MAX 511U
5959
#define CANARD_NODE_ID_MAX 127U
6060
#define CANARD_NODE_ID_CAPACITY (CANARD_NODE_ID_MAX + 1U)
@@ -251,8 +251,8 @@ struct canard_subscription_t
251251
canard_tree_t index_port_id; ///< Must be the first member.
252252

253253
canard_us_t transfer_id_timeout;
254-
uint32_t port_id; ///< Represents subjects, services, and legacy message- and service type IDs.
255254
size_t extent; ///< Must not be altered after initialization! In v0 includes the CRC.
255+
uint16_t port_id; ///< Represents subjects, services, and legacy message- and service type IDs.
256256
uint16_t crc_seed; ///< For v0 this is set at subscription time, for v1 this is always 0xFFFF.
257257
canard_kind_t kind;
258258

@@ -440,7 +440,7 @@ bool canard_publish(canard_t* const self,
440440
const canard_us_t deadline,
441441
const uint_least8_t iface_bitmap,
442442
const canard_prio_t priority,
443-
const uint32_t subject_id,
443+
const uint16_t subject_id,
444444
const uint_least8_t transfer_id,
445445
const canard_bytes_chain_t payload,
446446
const canard_user_context_t context);
@@ -454,7 +454,7 @@ bool canard_unicast(canard_t* const self,
454454

455455
bool canard_subscribe(canard_t* const self,
456456
canard_subscription_t* const subscription,
457-
const uint32_t subject_id,
457+
const uint16_t subject_id,
458458
const size_t extent,
459459
const canard_us_t transfer_id_timeout,
460460
const canard_subscription_vtable_t* const vtable);
@@ -493,7 +493,7 @@ bool canard_1v0_respond(canard_t* const self,
493493

494494
bool canard_1v0_subscribe(canard_t* const self,
495495
canard_subscription_t* const subscription,
496-
const uint16_t subject_id, // Narrower than in v1.1
496+
const uint16_t subject_id,
497497
const size_t extent,
498498
const canard_us_t transfer_id_timeout,
499499
const canard_subscription_vtable_t* const vtable);

‎tests/src/test_api_tx.cpp‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -216,12 +216,27 @@ static void test_canard_0v1_publish_requires_node_id()
216216
canard_0v1_publish(&self, 0, 1, canard_prio_nominal, 1, 0xFFFF, 0, payload, CANARD_USER_CONTEXT_NULL));
217217
}
218218

219-
static void test_canard_publish_subject_id_out_of_range()
219+
static void test_canard_publish_max_subject_id_encoding()
220220
{
221+
canard_t self = {};
222+
tx_capture_t cap = {};
223+
init_with_capture(&self, &cap);
224+
221225
const canard_bytes_chain_t payload = { .bytes = { .size = 0, .data = nullptr }, .next = nullptr };
222-
canard_t self = {};
223-
TEST_ASSERT_FALSE(canard_publish(
224-
&self, 0, 1, canard_prio_nominal, CANARD_SUBJECT_ID_MAX + 1U, 0, payload, CANARD_USER_CONTEXT_NULL));
226+
TEST_ASSERT_TRUE(canard_publish(
227+
&self, 1000, 1U, canard_prio_nominal, CANARD_SUBJECT_ID_MAX, 0U, payload, CANARD_USER_CONTEXT_NULL));
228+
229+
canard_poll(&self, 1U);
230+
TEST_ASSERT_EQUAL_size_t(1U, cap.count);
231+
const uint32_t can_id = cap.records[0].can_id;
232+
TEST_ASSERT_EQUAL_UINT8((uint8_t)canard_prio_nominal, (uint8_t)((can_id >> 26U) & 7U));
233+
TEST_ASSERT_EQUAL_UINT8(0U, (uint8_t)((can_id >> 25U) & 1U)); // service=0
234+
TEST_ASSERT_EQUAL_UINT8(0U, (uint8_t)((can_id >> 24U) & 1U)); // reserved=0
235+
TEST_ASSERT_EQUAL_UINT32(CANARD_SUBJECT_ID_MAX, (can_id >> 8U) & 0xFFFFU);
236+
TEST_ASSERT_EQUAL_UINT8(1U, (uint8_t)((can_id >> 7U) & 1U)); // v1.1 message marker
237+
TEST_ASSERT_EQUAL_UINT8(42U, (uint8_t)(can_id & CANARD_NODE_ID_MAX));
238+
239+
canard_destroy(&self);
225240
}
226241

227242
// Poll only drives interfaces marked writable in the provided bitmap.
@@ -434,7 +449,7 @@ int main()
434449
RUN_TEST(test_canard_publish_validation);
435450
RUN_TEST(test_canard_publish_oom);
436451
RUN_TEST(test_canard_0v1_publish_requires_node_id);
437-
RUN_TEST(test_canard_publish_subject_id_out_of_range);
452+
RUN_TEST(test_canard_publish_max_subject_id_encoding);
438453
RUN_TEST(test_canard_poll_ready_bitmap);
439454
RUN_TEST(test_canard_poll_backpressure);
440455
RUN_TEST(test_canard_poll_expiration);

‎tests/src/test_intrusive_rx.c‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -67,13 +67,13 @@ static void test_rx_parse_v1_1_message_golden(void)
6767
TEST_ASSERT_EQUAL_INT(canard_prio_exceptional, v1.priority);
6868
TEST_ASSERT_EQUAL_UINT8(0, v1.transfer_id);
6969
}
70-
// Max: prio=7, subject=131071 (0x1FFFF), src=127. CAN ID = 0x1DFFFFFF.
70+
// Max: prio=7, subject=65535 (0xFFFF), src=127. CAN ID = 0x1CFFFFFF.
7171
{
7272
const byte_t d[] = { 0xFF }; // 0xE0|31 → tid=31
7373
const canard_bytes_t pl = { sizeof(d), d };
74-
TEST_ASSERT_EQUAL_UINT8(2, rx_parse(0x1DFFFFFFUL, pl, &v0, &v1));
74+
TEST_ASSERT_EQUAL_UINT8(2, rx_parse(0x1CFFFFFFUL, pl, &v0, &v1));
7575
TEST_ASSERT_EQUAL_INT(canard_kind_1v1_message, v1.kind);
76-
TEST_ASSERT_EQUAL_UINT32(131071UL, v1.port_id);
76+
TEST_ASSERT_EQUAL_UINT32(0xFFFFUL, v1.port_id);
7777
TEST_ASSERT_EQUAL_HEX8(0xFF, v1.dst);
7878
TEST_ASSERT_EQUAL_HEX8(127, v1.src);
7979
TEST_ASSERT_EQUAL_INT(canard_prio_optional, v1.priority);
@@ -548,25 +548,25 @@ static void test_rx_parse_v1_1_accepts_bit23(void)
548548
{
549549
frame_t v0;
550550
frame_t v1;
551-
// Subject 0x18000: bit 23 of CAN ID is set because (0x18000 << 8) sets bit 23.
552-
// CAN ID: (0<<26) | (0x18000<<8) | (1<<7) | 0 = 0x01800080
551+
// Subject 0x8000: bit 23 of CAN ID is set because (0x8000 << 8) sets bit 23.
552+
// CAN ID: (0<<26) | (0x8000<<8) | (1<<7) | 0 = 0x00800080
553553
{
554554
const byte_t d[] = { 0xE5 }; // v1 single, tid=5
555555
const canard_bytes_t pl = { sizeof(d), d };
556-
const byte_t ret = rx_parse(0x01800080UL, pl, &v0, &v1);
556+
const byte_t ret = rx_parse(0x00800080UL, pl, &v0, &v1);
557557
TEST_ASSERT_EQUAL_UINT8(2, ret); // accepted despite bit 23
558558
TEST_ASSERT_EQUAL_INT(canard_kind_1v1_message, v1.kind);
559-
TEST_ASSERT_EQUAL_UINT32(0x18000UL, v1.port_id);
559+
TEST_ASSERT_EQUAL_UINT32(0x8000UL, v1.port_id);
560560
}
561-
// Max subject 0x1FFFF also sets bit 23.
562-
// CAN ID: (0<<26) | (0x1FFFF<<8) | (1<<7) | 0 = 0x01FFFF80
561+
// Max subject 0xFFFF also sets bit 23.
562+
// CAN ID: (0<<26) | (0xFFFF<<8) | (1<<7) | 0 = 0x00FFFF80
563563
{
564564
const byte_t d[] = { 0xE0 };
565565
const canard_bytes_t pl = { sizeof(d), d };
566-
const byte_t ret = rx_parse(0x01FFFF80UL, pl, &v0, &v1);
566+
const byte_t ret = rx_parse(0x00FFFF80UL, pl, &v0, &v1);
567567
TEST_ASSERT_EQUAL_UINT8(2, ret);
568568
TEST_ASSERT_EQUAL_INT(canard_kind_1v1_message, v1.kind);
569-
TEST_ASSERT_EQUAL_UINT32(0x1FFFFUL, v1.port_id);
569+
TEST_ASSERT_EQUAL_UINT32(0xFFFFUL, v1.port_id);
570570
}
571571
}
572572

‎tests/src/test_intrusive_rx_filter.c‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
void setUp(void) {}
99
void tearDown(void) {}
1010

11-
static canard_filter_t make_filter(const canard_kind_t kind, const uint32_t port_id, const byte_t node_id)
11+
static canard_filter_t make_filter(const canard_kind_t kind, const uint16_t port_id, const byte_t node_id)
1212
{
1313
canard_t self;
1414
canard_subscription_t sub;
@@ -30,10 +30,10 @@ static bool filter_accepts(const canard_filter_t filter, const uint32_t can_id)
3030

3131
static void test_rx_filter_for_subscription_golden_vectors(void)
3232
{
33-
// v1.1 message: subject=0x1ABCD
33+
// v1.1 message: subject=0xABCD
3434
{
35-
const canard_filter_t f = make_filter(canard_kind_1v1_message, 0x1ABCDUL, 42U);
36-
TEST_ASSERT_EQUAL_HEX32(0x01ABCD80UL, f.extended_can_id);
35+
const canard_filter_t f = make_filter(canard_kind_1v1_message, 0xABCDU, 42U);
36+
TEST_ASSERT_EQUAL_HEX32(0x00ABCD80UL, f.extended_can_id);
3737
TEST_ASSERT_EQUAL_HEX32(0x03FFFF80UL, f.extended_mask);
3838
}
3939

@@ -85,12 +85,13 @@ static void test_rx_filter_for_subscription_golden_vectors(void)
8585

8686
static void test_rx_filter_for_subscription_v1_1_message_semantics(void)
8787
{
88-
const canard_filter_t f = make_filter(canard_kind_1v1_message, 0x18001UL, 55U);
89-
TEST_ASSERT_TRUE(filter_accepts(f, 0x018001AAUL)); // same subject, prio=0, src=42
90-
TEST_ASSERT_TRUE(filter_accepts(f, 0x1D8001FFUL)); // same subject, prio=7, src=127
91-
TEST_ASSERT_FALSE(filter_accepts(f, 0x038001AAUL)); // service bit (25) must be zero
92-
TEST_ASSERT_FALSE(filter_accepts(f, 0x0180012AUL)); // message selector bit (7) must be one
93-
TEST_ASSERT_FALSE(filter_accepts(f, 0x018002AAUL)); // subject mismatch
88+
const canard_filter_t f = make_filter(canard_kind_1v1_message, 0x8001U, 55U);
89+
TEST_ASSERT_TRUE(filter_accepts(f, 0x008001AAUL)); // same subject, prio=0, src=42
90+
TEST_ASSERT_TRUE(filter_accepts(f, 0x1C8001FFUL)); // same subject, prio=7, src=127
91+
TEST_ASSERT_FALSE(filter_accepts(f, 0x028001AAUL)); // service bit (25) must be zero
92+
TEST_ASSERT_FALSE(filter_accepts(f, 0x018001AAUL)); // bit 24 must be zero for v1.1
93+
TEST_ASSERT_FALSE(filter_accepts(f, 0x0080012AUL)); // message selector bit (7) must be one
94+
TEST_ASSERT_FALSE(filter_accepts(f, 0x008002AAUL)); // subject mismatch
9495
}
9596

9697
static void test_rx_filter_for_subscription_v1_0_message_semantics(void)

‎tests/src/test_intrusive_rx_session.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ static void on_message_capture(canard_subscription_t* const self,
7474

7575
static void fixture_init(session_fixture_t* const fx,
7676
const canard_kind_t kind,
77-
const uint32_t port_id,
77+
const uint16_t port_id,
7878
const size_t extent,
7979
const canard_us_t tid_timeout,
8080
const uint16_t crc_seed)
@@ -98,7 +98,7 @@ static void fixture_init(session_fixture_t* const fx,
9898

9999
static void fixture_init_v1(session_fixture_t* const fx,
100100
const canard_kind_t kind,
101-
const uint32_t port_id,
101+
const uint16_t port_id,
102102
const size_t extent)
103103
{
104104
fixture_init(fx, kind, port_id, extent, 2 * MEGA, CRC_INITIAL);
@@ -115,7 +115,7 @@ static bool feed(session_fixture_t* const fx, const canard_us_t ts, const frame_
115115
/// Construct a frame_t for a single-frame transfer (start=true, end=true).
116116
static frame_t make_single_frame(const canard_prio_t priority,
117117
const canard_kind_t kind,
118-
const uint32_t port_id,
118+
const uint16_t port_id,
119119
const byte_t dst,
120120
const byte_t src,
121121
const byte_t transfer_id,
@@ -140,7 +140,7 @@ static frame_t make_single_frame(const canard_prio_t priority,
140140
/// Construct a frame_t for a multi-frame start frame.
141141
static frame_t make_start_frame(const canard_prio_t priority,
142142
const canard_kind_t kind,
143-
const uint32_t port_id,
143+
const uint16_t port_id,
144144
const byte_t dst,
145145
const byte_t src,
146146
const byte_t transfer_id,
@@ -165,7 +165,7 @@ static frame_t make_start_frame(const canard_prio_t priority,
165165
/// Construct a continuation frame.
166166
static frame_t make_cont_frame(const canard_prio_t priority,
167167
const canard_kind_t kind,
168-
const uint32_t port_id,
168+
const uint16_t port_id,
169169
const byte_t dst,
170170
const byte_t src,
171171
const byte_t transfer_id,

0 commit comments

Comments
 (0)