Skip to content

Commit f74453f

Browse files
new solver fixes
1 parent de88465 commit f74453f

File tree

1 file changed

+15
-9
lines changed

1 file changed

+15
-9
lines changed

libcanard/canard.c

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1249,20 +1249,24 @@ static rx_seqno_packed_t rx_seqno_pack(const uint64_t v)
12491249
{ (uint16_t)(v & 0xFFFFU), (uint16_t)((v >> 16U) & 0xFFFFU), (uint16_t)((v >> 32U) & 0xFFFFU) }
12501250
};
12511251
}
1252+
1253+
// The linearizer logic results in a behavior that differs compared to v4 significantly: v4 used to admit any
1254+
// transfer-ID except the same and the one older compared to the last transfer; in terms of the linearization logic it
1255+
// means that the reference point was biased.
12521256
static uint64_t rx_seqno_linearize(const uint64_t ref_seqno, const byte_t transfer_id)
12531257
{
12541258
CANARD_ASSERT(transfer_id <= CANARD_TRANSFER_ID_MAX);
12551259
const byte_t ref_tid = (byte_t)(ref_seqno & CANARD_TRANSFER_ID_MAX);
12561260
int16_t delta = ((int16_t)transfer_id) - ((int16_t)ref_tid); // NOLINT(*-narrowing-conversions)
12571261
// Select the nearest congruent seqno in the half-open interval [-16, +16) around the reference.
12581262
if (delta > (int16_t)(CANARD_TRANSFER_ID_MAX / 2U)) {
1259-
delta -= (int16_t)(CANARD_TRANSFER_ID_MAX + 1U);
1263+
delta -= (int16_t)CANARD_TRANSFER_ID_MODULO;
12601264
} else if (delta < -((int16_t)(CANARD_TRANSFER_ID_MAX / 2U) + 1)) {
1261-
delta += (int16_t)(CANARD_TRANSFER_ID_MAX + 1U);
1265+
delta += (int16_t)CANARD_TRANSFER_ID_MODULO;
12621266
}
12631267
// Near the origin the backward representative may underflow, so use the next forward one instead.
12641268
if ((delta < 0) && (ref_seqno < (uint64_t)(-delta))) {
1265-
delta += (int16_t)(CANARD_TRANSFER_ID_MAX + 1U);
1269+
delta += (int16_t)CANARD_TRANSFER_ID_MODULO;
12661270
}
12671271
return (uint64_t)(((int64_t)ref_seqno) + ((int64_t)delta));
12681272
}
@@ -1425,12 +1429,6 @@ static uint64_t rx_session_solve_admission(const rx_session_t* const ses,
14251429
return admit ? RX_SESSION_ADMISSION_CONTINUATION : RX_SESSION_ADMISSION_REJECTED;
14261430
}
14271431

1428-
// This is a start frame, but before we allocate new state for it, we must ensure that it is of the correct version.
1429-
const bool start_toggle = kind_is_v1(ses->owner->kind) ? 1 : 0;
1430-
if (toggle != start_toggle) {
1431-
return RX_SESSION_ADMISSION_REJECTED; // Wrong protocol version.
1432-
}
1433-
14341432
// It is best to postpone seqno derivation until the last moment because it is costly.
14351433
// Life would have been so much easier if we could just use normal non-wrapping IDs like we have in Cyphal/UDP!
14361434
const uint64_t frontier_global = rx_session_seqno_frontier(ses, canard_prio_exceptional);
@@ -1491,6 +1489,14 @@ static bool rx_session_update(canard_subscription_t* const sub,
14911489
CANARD_ASSERT((sub != NULL) && (frame != NULL) && (frame->payload.data != NULL) && (ts >= 0));
14921490
CANARD_ASSERT(frame->end || (frame->payload.size >= 7));
14931491

1492+
// This is a start frame, but before we allocate new state for it, we must ensure that it is of the correct version.
1493+
// Wrong protocol version must be rejected as early as possible to avoid wasting memory on unused states.
1494+
// The protocol version is only acceptable on start-of-transfer frames, but new sessions can only be created
1495+
// on start frames, so this is robust.
1496+
if (frame->start && (frame->toggle != kind_is_v1(sub->kind))) {
1497+
return true; // Wrong protocol version is not a failure.
1498+
}
1499+
14941500
// Only start frames may create new states.
14951501
rx_session_factory_context_t factory_context = { .owner = sub, .iface_index = iface_index, .node_id = frame->src };
14961502
rx_session_t* const ses =

0 commit comments

Comments
 (0)