Skip to content

Commit 03f278e

Browse files
Allow CAN unicast to remote node-ID 0
Review finding #12 (Medium). Node-ID 0 is a valid regular node in Cyphal/CAN v1 (only v0/DroneCAN treated 0 as anonymous; libcanard permits a remote 0 and a service destination 0), and the wire encoder already accepts it, but unicast() guarded 1 <= remote_id, so the ACK path could not answer a node-0 peer and reliable delivery from such a peer never completed. The destination range is now 0..127. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CabiCDQ5DNBYq8WDKRzG4Z
1 parent d5e3e5f commit 03f278e

2 files changed

Lines changed: 16 additions & 5 deletions

File tree

src/pycyphal2/can/_transport.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,9 @@ def unicast_listen(self, handler: Callable[[TransportArrival], None]) -> None:
250250
async def unicast(self, deadline: Instant, priority: Priority, remote_id: int, message: bytes | memoryview) -> None:
251251
if self._closed:
252252
raise ClosedError("CAN transport closed")
253-
if not (1 <= remote_id <= NODE_ID_MAX):
253+
# Node-ID 0 is a valid regular node in Cyphal/CAN v1 (only v0 treated it as anonymous), so it is
254+
# a legal unicast destination; rejecting it would make the ACK path unable to answer a node-0 peer.
255+
if not (0 <= remote_id <= NODE_ID_MAX):
254256
raise ValueError(f"Invalid remote node-ID: {remote_id}")
255257
transfer_id = self._unicast_tid[remote_id]
256258
self._unicast_tid[remote_id] = (transfer_id + 1) % TRANSFER_ID_MODULO

tests/can/test_transport_internal.py

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
from pycyphal2._transport import SUBJECT_ID_MODULUS_16bit, TransportArrival
1111
from pycyphal2.can import CANTransport, TimestampedFrame
1212
from pycyphal2.can._transport import _CANTransportImpl, _PinnedSubjectState
13-
from pycyphal2.can._wire import NODE_ID_ANONYMOUS, TransferKind, serialize_transfer
13+
from pycyphal2.can._wire import NODE_ID_ANONYMOUS, TransferKind, parse_frame, serialize_transfer
1414
from tests.can._support import MockCANBus, MockCANInterface, wait_for
1515

1616

@@ -99,9 +99,18 @@ async def test_writer_unicast_and_send_transfer_error_paths() -> None:
9999
with pytest.raises(ClosedError, match="CAN transport closed"):
100100
await writer2(Instant.now() + 1.0, Priority.NOMINAL, b"x")
101101

102-
live = CANTransport.new(MockCANInterface(bus, "if1"))
103-
with pytest.raises(ValueError, match="Invalid remote node-ID"):
104-
await live.unicast(Instant.now() + 1.0, Priority.NOMINAL, 0, b"x")
102+
live_if = MockCANInterface(bus, "if1")
103+
live = CANTransport.new(live_if)
104+
# Node-ID 0 is a valid regular Cyphal/CAN v1 node, hence a legal unicast destination;
105+
# the wire encoding must carry destination 0.
106+
await live.unicast(Instant.now() + 1.0, Priority.NOMINAL, 0, b"x")
107+
uni_id, uni_frames, _ = live_if.enqueue_history[-1]
108+
parsed = parse_frame(uni_id, uni_frames[0])
109+
assert parsed is not None
110+
assert parsed.destination_id == 0
111+
for bad_remote in (-1, 128):
112+
with pytest.raises(ValueError, match="Invalid remote node-ID"):
113+
await live.unicast(Instant.now() + 1.0, Priority.NOMINAL, bad_remote, b"x")
105114

106115
live_impl = cast(_CANTransportImpl, live)
107116
with pytest.raises(SendError, match="Deadline exceeded"):

0 commit comments

Comments
 (0)