diff --git a/src/CanKit.Pro.CANopen/CanOpenEvents.cs b/src/CanKit.Pro.CANopen/CanOpenEvents.cs
index dab32a7..d88b9d8 100644
--- a/src/CanKit.Pro.CANopen/CanOpenEvents.cs
+++ b/src/CanKit.Pro.CANopen/CanOpenEvents.cs
@@ -14,7 +14,8 @@ public sealed class HeartbeatReceivedEventArgs : EventArgs
public byte ProducerNodeId { get; }
/// Reported NMT state of the producer. is used
- /// for the CiA 301 §7.2.8.3.2 bootup frame (data[0] == 0x00).
+ /// for the CiA 301 §7.2.8.3.2 bootup frame (data[0] == 0x00). A frame whose state
+ /// byte is one CiA 301 reserves is not reported.
public NmtState State { get; }
/// UTC timestamp captured when the frame was processed on the actor loop.
diff --git a/src/CanKit.Pro.CANopen/CanOpenNode.NodeGuarding.cs b/src/CanKit.Pro.CANopen/CanOpenNode.NodeGuarding.cs
index b61bf04..8922aa8 100644
--- a/src/CanKit.Pro.CANopen/CanOpenNode.NodeGuarding.cs
+++ b/src/CanKit.Pro.CANopen/CanOpenNode.NodeGuarding.cs
@@ -206,14 +206,10 @@ private void HandleNodeGuardingResponse(byte producerNodeId, byte[] data)
// (Bugbot, plus two adjacent findings from Codex on the same mechanism).
bool toggle = (b & 0x80) != 0;
byte stateByte = (byte)(b & 0x7F);
- NmtState state = stateByte switch
- {
- 0x00 => NmtState.Initializing, // Bootup / freshly reset.
- 0x04 => NmtState.Stopped,
- 0x05 => NmtState.Operational,
- 0x7F => NmtState.PreOperational,
- _ => NmtState.Initializing,
- };
+ // A reserved state byte is a reply all the same, but it is not reported (#255). Neither is
+ // state 0 with the toggle bit set: a node that answers guarding is never Initializing
+ // (see the boot-up branch above), so 0x80 would read as a boot-up that is not one.
+ bool reportable = TryDecodeHeartbeatState(stateByte, out var state) && stateByte != 0;
// CiA 301 §7.2.8.3.3: a reply that does not alternate the toggle bit is invalid for
// resetting the life-time window (stale/repeated frames must not keep the consumer
@@ -235,7 +231,7 @@ private void HandleNodeGuardingResponse(byte producerNodeId, byte[] data)
() => OnNodeGuardingTimeout(producerNodeId));
}
- RaiseNodeGuardingReceived(producerNodeId, state, toggle, DateTime.UtcNow);
+ if (reportable) RaiseNodeGuardingReceived(producerNodeId, state, toggle, DateTime.UtcNow);
}
// =========================================================================================
diff --git a/src/CanKit.Pro.CANopen/CanOpenNode.cs b/src/CanKit.Pro.CANopen/CanOpenNode.cs
index 282b15a..45cddc4 100644
--- a/src/CanKit.Pro.CANopen/CanOpenNode.cs
+++ b/src/CanKit.Pro.CANopen/CanOpenNode.cs
@@ -1335,6 +1335,21 @@ private void HandleEmcy(uint cobId, byte[] data)
// =========================================================================================
// Heartbeat (FR-CO-008)
// =========================================================================================
+
+ /// The NMT state a heartbeat or guarding state byte (bit 7 already masked) reports;
+ /// false for the values CiA 301 §7.2.8.3.2 reserves.
+ internal static bool TryDecodeHeartbeatState(byte stateByte, out NmtState state)
+ {
+ switch (stateByte)
+ {
+ case 0x00: state = NmtState.Initializing; return true; // Bootup frame.
+ case 0x04: state = NmtState.Stopped; return true;
+ case 0x05: state = NmtState.Operational; return true;
+ case 0x7F: state = NmtState.PreOperational; return true;
+ default: state = default; return false;
+ }
+ }
+
private void HandleHeartbeat(uint cobId, byte[] data)
{
if (data.Length < 1) return;
@@ -1343,15 +1358,13 @@ private void HandleHeartbeat(uint cobId, byte[] data)
// reserved (always 0)"), and a guarding reply is routed to HandleNodeGuardingResponse
// before it gets here, so the bit is masked rather than interpreted.
byte stateByte = (byte)(data[0] & 0x7F);
- NmtState state = stateByte switch
- {
- 0x00 => NmtState.Initializing, // Bootup frame.
- 0x04 => NmtState.Stopped,
- 0x05 => NmtState.Operational,
- 0x7F => NmtState.PreOperational,
- _ => NmtState.Initializing,
- };
- RaiseHeartbeatReceived(producer, state, DateTime.UtcNow);
+ // A reserved state byte still shows the producer alive and is still recorded for the NMT
+ // master, but it is not reported: as Initializing it would be indistinguishable from a
+ // boot-up and read as a restart that did not happen (#255).
+ // The boot-up is the byte 0x00; 0x80 is state 0 with the guarding toggle set, which no
+ // producer sends as a heartbeat.
+ if (TryDecodeHeartbeatState(stateByte, out var state) && (stateByte != 0 || data[0] == 0))
+ RaiseHeartbeatReceived(producer, state, DateTime.UtcNow);
NoteSlaveNmtState(producer, stateByte);
_heartbeatConsumer.NoteReceived(producer);
}
@@ -2067,6 +2080,8 @@ private void HandleSdoClientResponse(byte serverNodeId, byte[] data)
RearmSdoClientDeadline(session);
session.InSegmentPhase = true;
session.Payload = declared > 0 ? new byte[declared] : Array.Empty();
+ session.DeclaredTotalSize = declared;
+ session.SizeIndicated = (cs & 0x01) != 0;
session.Offset = 0;
session.Toggle = false;
SendNextClientUploadSegmentRequest(session);
@@ -2120,6 +2135,16 @@ private void HandleSdoClientResponse(byte serverNodeId, byte[] data)
// enforce MaxSdoTransferBytes so a zero-size init cannot bypass the cap by
// streaming unbounded segments.
int needed = session.Offset + payload.Length;
+ // More than the server announced is a protocol error, as it is for the server's own
+ // receive path and for the block-upload client (#253). It is judged before the cap,
+ // so that a server announcing exactly the cap and sending more is told what it did
+ // wrong. Without an indicated size the buffer grows, which is what that case is for;
+ // an indicated size of zero is a size.
+ if (session.SizeIndicated && needed > session.DeclaredTotalSize)
+ {
+ AbortClient(session, SdoAbortCode.LengthTooHigh);
+ return;
+ }
if (needed > _options.MaxSdoTransferBytes)
{
AbortClient(session, SdoAbortCode.OutOfMemory);
@@ -2139,13 +2164,22 @@ private void HandleSdoClientResponse(byte serverNodeId, byte[] data)
var final = session.Payload;
if (session.Offset != final.Length)
{
- // Server declared a size but sent less. Trim.
+ // The buffer is larger than the data: slack from geometric growth when no
+ // size was announced, or a server that announced more than it sent.
var trimmed = new byte[session.Offset];
Buffer.BlockCopy(final, 0, trimmed, 0, session.Offset);
final = trimmed;
}
session.Deadline?.Dispose();
_sdoClients.Remove(serverNodeId);
+ // Fewer bytes than announced (a device that announces the maximum length of a
+ // VISIBLE_STRING and sends what it holds) is accepted, but no longer silently:
+ // the caller gets the data, and the shortfall is reported (#253).
+ if (session.SizeIndicated && session.Offset < session.DeclaredTotalSize)
+ {
+ RaiseBackgroundException(new CanOpenTransportException(
+ $"SDO upload of 0x{session.Index:X4}:{session.Subindex:X2} from node {serverNodeId} announced {session.DeclaredTotalSize} byte(s) and delivered {session.Offset}; the shorter data was returned."));
+ }
session.Tcs.TrySetResult(final);
return;
}
@@ -2577,6 +2611,8 @@ public SdoClientSession(byte serverNodeId, ushort index, byte subindex, bool isD
/// transfer.
public byte[]? Payload { get; set; }
public int Offset { get; set; }
+ public uint DeclaredTotalSize { get; set; }
+ public bool SizeIndicated { get; set; }
public bool Toggle { get; set; }
/// Numbers this transfer's sends; only the latest can still decide it (#197).
diff --git a/src/CanKit.Pro.CANopen/ICanOpenNode.cs b/src/CanKit.Pro.CANopen/ICanOpenNode.cs
index 32f3df6..91a7a3b 100644
--- a/src/CanKit.Pro.CANopen/ICanOpenNode.cs
+++ b/src/CanKit.Pro.CANopen/ICanOpenNode.cs
@@ -251,6 +251,10 @@ Task SendNmtCommandAsync(NmtCommand command, byte targetNodeId,
/// Transmits a single SYNC frame (payload-less) on the SYNC COB-ID configured in
/// 1005h.
+ /// This is a raw send, not the producer: it is not gated by the NMT state and not by
+ /// bit 30 of 1005h ("device generates SYNC"), so a node in Stopped, or one that is not
+ /// configured to produce SYNC, transmits when asked. The periodic producer
+ /// () is the one that follows CiA 301 Table 37.
Task SendSyncAsync(CancellationToken cancellationToken = default);
// -----------------------------------------------------------------------------------------
diff --git a/src/CanKit.Pro.CANopen/README.md b/src/CanKit.Pro.CANopen/README.md
index ab7dd66..83327cf 100644
--- a/src/CanKit.Pro.CANopen/README.md
+++ b/src/CanKit.Pro.CANopen/README.md
@@ -445,7 +445,7 @@ Table 37 of CiA 301, as the node applies it:
| --- | --- | --- | --- |
| PDO | — | transmitted and received | — |
| SDO | served | served | requests ignored; an open server session is aborted with `0800 0022h` |
-| SYNC | consumed and produced | consumed and produced | neither; the producer keeps its cycle and resumes afterwards |
+| SYNC | consumed and produced | consumed and produced | neither; the producer keeps its cycle and resumes afterwards (`SendSyncAsync` is a raw send and transmits regardless of the state and of bit 30 of `1005h`) |
| EMCY | transmitted | transmitted | held; the most recent one is transmitted when the node leaves Stopped |
| Heartbeat, guarding | active | active | active |
diff --git a/tests/CanKit.Pro.Tests/TestCases/CANopen/CanOpenSdoCorrectnessTests.cs b/tests/CanKit.Pro.Tests/TestCases/CANopen/CanOpenSdoCorrectnessTests.cs
index c42fabd..e1c1242 100644
--- a/tests/CanKit.Pro.Tests/TestCases/CANopen/CanOpenSdoCorrectnessTests.cs
+++ b/tests/CanKit.Pro.Tests/TestCases/CANopen/CanOpenSdoCorrectnessTests.cs
@@ -13,6 +13,7 @@
using CanKit.Abstractions.API.Common.Definitions;
using CanKit.Core;
using CanKit.Pro.CANopen;
+using CanKit.Pro.CANopen.Nmt;
using CanKit.Pro.CANopen.Sdo;
using CanKit.Pro.RawCan;
using CanKit.Pro.Tests.Infrastructure;
@@ -1274,6 +1275,206 @@ public async Task Sdo_BlockUpload_Client_Sizeless_Grows_Geometrically()
#endif
+ // -----------------------------------------------------------------------------------------
+ // #253: the classic upload client against the length the server announced. More than that is
+ // refused as the server's receive path and the block-upload client already do; fewer is
+ // accepted (a device that announces the OD maximum of a VISIBLE_STRING and sends what it
+ // holds) but reported through BackgroundExceptionOccurred instead of passing silently.
+ // -----------------------------------------------------------------------------------------
+ private static byte[] AnnouncedUploadInit(ushort index, byte subindex, uint length) => new byte[]
+ {
+ SdoFrames.ScsUploadInitSegmented,
+ (byte)(index & 0xFF), (byte)((index >> 8) & 0xFF), subindex,
+ (byte)length, (byte)(length >> 8), (byte)(length >> 16), (byte)(length >> 24),
+ };
+
+ [Fact]
+ public async Task Sdo_Client_Upload_Aborts_When_The_Server_Sends_More_Than_It_Announced()
+ {
+ var session = NewSession();
+ using var busA = Open(session, 1);
+ using var rawBus = Open(session, 2);
+ using var client = CanOpen.OpenNode(busA, nodeId: 0x01);
+ PeerSdoLaboratory.Bind(client, 0x02);
+ using var tap = new FrameTap(rawBus, CanOpenCobId.SdoRx(0x02));
+
+ var upload = client.SdoUploadAsync(0x02, 0x2100, 0x00);
+ tap.Next(ShortTimeout)[0].Should().Be(SdoFrames.CcsUploadInit);
+ Send(rawBus, CanOpenCobId.SdoTx(0x02), AnnouncedUploadInit(0x2100, 0x00, 10));
+ tap.Next(ShortTimeout)[0].Should().Be(SdoFrames.CcsUploadSegmentBase);
+ Send(rawBus, CanOpenCobId.SdoTx(0x02), SdoFrames.BuildSegment(
+ SdoFrames.ScsUploadSegmentBase, toggle: false, lastSegment: false, new byte[] { 1, 2, 3, 4, 5, 6, 7 }));
+ tap.Next(ShortTimeout)[0].Should().Be((byte)(SdoFrames.CcsUploadSegmentBase | SdoFrames.ToggleBit));
+ // Seven bytes were delivered and ten announced; seven more make fourteen.
+ Send(rawBus, CanOpenCobId.SdoTx(0x02), SdoFrames.BuildSegment(
+ SdoFrames.ScsUploadSegmentBase, toggle: true, lastSegment: true, new byte[] { 8, 9, 10, 11, 12, 13, 14 }));
+
+ var abort = tap.Next(ShortTimeout);
+ abort[0].Should().Be(SdoFrames.CsAbort);
+ SdoFrames.ReadAbortCode(abort).Should().Be((uint)SdoAbortCode.LengthTooHigh);
+ var ex = await Assert.ThrowsAsync(() => upload.WithTimeoutAsync(ShortTimeout));
+ ex.AbortCode.Should().Be((uint)SdoAbortCode.LengthTooHigh);
+ }
+
+ [Fact]
+ public async Task Sdo_Client_Upload_Over_An_Announced_Cap_Sized_Length_Is_Length_Too_High_Not_Out_Of_Memory()
+ {
+ var session = NewSession();
+ using var busA = Open(session, 1);
+ using var rawBus = Open(session, 2);
+ using var client = CanOpen.OpenNode(busA, nodeId: 0x01,
+ new CanOpenNodeOptions().With(maxSdoTransferBytes: 10));
+ PeerSdoLaboratory.Bind(client, 0x02);
+ using var tap = new FrameTap(rawBus, CanOpenCobId.SdoRx(0x02));
+
+ var upload = client.SdoUploadAsync(0x02, 0x2100, 0x00);
+ tap.Next(ShortTimeout)[0].Should().Be(SdoFrames.CcsUploadInit);
+ Send(rawBus, CanOpenCobId.SdoTx(0x02), AnnouncedUploadInit(0x2100, 0x00, 10));
+ tap.Next(ShortTimeout)[0].Should().Be(SdoFrames.CcsUploadSegmentBase);
+ Send(rawBus, CanOpenCobId.SdoTx(0x02), SdoFrames.BuildSegment(
+ SdoFrames.ScsUploadSegmentBase, toggle: false, lastSegment: false, new byte[] { 1, 2, 3, 4, 5, 6, 7 }));
+ tap.Next(ShortTimeout);
+ // Fourteen bytes against ten announced, and ten is also the cap: the announcement is what
+ // was broken, so that is what the abort says.
+ Send(rawBus, CanOpenCobId.SdoTx(0x02), SdoFrames.BuildSegment(
+ SdoFrames.ScsUploadSegmentBase, toggle: true, lastSegment: true, new byte[] { 8, 9, 10, 11, 12, 13, 14 }));
+
+ SdoFrames.ReadAbortCode(tap.Next(ShortTimeout)).Should().Be((uint)SdoAbortCode.LengthTooHigh);
+ var ex = await Assert.ThrowsAsync(() => upload.WithTimeoutAsync(ShortTimeout));
+ ex.AbortCode.Should().Be((uint)SdoAbortCode.LengthTooHigh);
+ }
+
+ [Fact]
+ public async Task Sdo_Client_Upload_Aborts_When_The_Server_Announces_Zero_And_Sends_Data()
+ {
+ var session = NewSession();
+ using var busA = Open(session, 1);
+ using var rawBus = Open(session, 2);
+ using var client = CanOpen.OpenNode(busA, nodeId: 0x01);
+ PeerSdoLaboratory.Bind(client, 0x02);
+ using var tap = new FrameTap(rawBus, CanOpenCobId.SdoRx(0x02));
+
+ var upload = client.SdoUploadAsync(0x02, 0x2100, 0x00);
+ tap.Next(ShortTimeout)[0].Should().Be(SdoFrames.CcsUploadInit);
+ // The size indicator is set and the size is zero: that is an announced length of nothing,
+ // not a missing one.
+ Send(rawBus, CanOpenCobId.SdoTx(0x02), AnnouncedUploadInit(0x2100, 0x00, 0));
+ tap.Next(ShortTimeout)[0].Should().Be(SdoFrames.CcsUploadSegmentBase);
+ Send(rawBus, CanOpenCobId.SdoTx(0x02), SdoFrames.BuildSegment(
+ SdoFrames.ScsUploadSegmentBase, toggle: false, lastSegment: true, new byte[] { 1, 2, 3 }));
+
+ var abort = tap.Next(ShortTimeout);
+ SdoFrames.ReadAbortCode(abort).Should().Be((uint)SdoAbortCode.LengthTooHigh);
+ await Assert.ThrowsAsync(() => upload.WithTimeoutAsync(ShortTimeout));
+ }
+
+ [Fact]
+ public async Task Sdo_Client_Upload_Returns_Short_Data_And_Reports_The_Shortfall()
+ {
+ var session = NewSession();
+ using var busA = Open(session, 1);
+ using var rawBus = Open(session, 2);
+ using var client = CanOpen.OpenNode(busA, nodeId: 0x01);
+ PeerSdoLaboratory.Bind(client, 0x02);
+ using var tap = new FrameTap(rawBus, CanOpenCobId.SdoRx(0x02));
+ var reports = new System.Collections.Concurrent.ConcurrentQueue();
+ client.BackgroundExceptionOccurred += (_, e) => reports.Enqueue(e);
+
+ var upload = client.SdoUploadAsync(0x02, 0x2100, 0x00);
+ tap.Next(ShortTimeout)[0].Should().Be(SdoFrames.CcsUploadInit);
+ Send(rawBus, CanOpenCobId.SdoTx(0x02), AnnouncedUploadInit(0x2100, 0x00, 20));
+ tap.Next(ShortTimeout)[0].Should().Be(SdoFrames.CcsUploadSegmentBase);
+ Send(rawBus, CanOpenCobId.SdoTx(0x02), SdoFrames.BuildSegment(
+ SdoFrames.ScsUploadSegmentBase, toggle: false, lastSegment: true, new byte[] { 1, 2, 3 }));
+
+ (await upload.WithTimeoutAsync(ShortTimeout)).Should().Equal(1, 2, 3);
+ reports.Should().ContainSingle().Which.Should().BeOfType()
+ .Which.Message.Should().Contain("announced 20").And.Contain("delivered 3");
+ }
+
+ [Fact]
+ public async Task Sdo_Client_Upload_With_The_Announced_Length_Reports_Nothing()
+ {
+ var session = NewSession();
+ using var busA = Open(session, 1);
+ using var rawBus = Open(session, 2);
+ using var client = CanOpen.OpenNode(busA, nodeId: 0x01);
+ PeerSdoLaboratory.Bind(client, 0x02);
+ using var tap = new FrameTap(rawBus, CanOpenCobId.SdoRx(0x02));
+ var reports = new System.Collections.Concurrent.ConcurrentQueue();
+ client.BackgroundExceptionOccurred += (_, e) => reports.Enqueue(e);
+
+ var upload = client.SdoUploadAsync(0x02, 0x2100, 0x00);
+ tap.Next(ShortTimeout);
+ Send(rawBus, CanOpenCobId.SdoTx(0x02), AnnouncedUploadInit(0x2100, 0x00, 3));
+ tap.Next(ShortTimeout);
+ Send(rawBus, CanOpenCobId.SdoTx(0x02), SdoFrames.BuildSegment(
+ SdoFrames.ScsUploadSegmentBase, toggle: false, lastSegment: true, new byte[] { 1, 2, 3 }));
+
+ (await upload.WithTimeoutAsync(ShortTimeout)).Should().Equal(1, 2, 3);
+ reports.Should().BeEmpty();
+ }
+
+ // -----------------------------------------------------------------------------------------
+ // #255: a heartbeat or guarding state byte that CiA 301 reserves is not reported. It used to be
+ // reported as Initializing, the value of a boot-up. Events of one producer may be coalesced
+ // by the queue, so "the first event" is not a measuring point; the valid frame sent last is
+ // always delivered, and nothing delivered by then may have been Initializing.
+ // -----------------------------------------------------------------------------------------
+ [Theory]
+ [InlineData(0x01)]
+ [InlineData(0x03)]
+ [InlineData(0x80)] // state 0 with the toggle bit set is not the boot-up byte
+ public async Task Heartbeat_With_A_Reserved_State_Byte_Is_Not_Reported_As_Bootup(byte reserved)
+ {
+ var session = NewSession();
+ using var busA = Open(session, 1);
+ using var rawBus = Open(session, 2);
+ using var node = CanOpen.OpenNode(busA, nodeId: 0x01);
+ var seen = new System.Collections.Concurrent.ConcurrentQueue();
+ var last = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
+ node.HeartbeatReceived += (_, e) =>
+ {
+ if (e.ProducerNodeId != 0x11) return;
+ seen.Enqueue(e.State);
+ if (e.State == NmtState.Operational) last.TrySetResult(true);
+ };
+
+ Send(rawBus, CanOpenCobId.HeartbeatBase + 0x11, new byte[] { reserved });
+ Send(rawBus, CanOpenCobId.HeartbeatBase + 0x11, new byte[] { 0x05 });
+ await last.Task.WithTimeoutAsync(ShortTimeout);
+
+ seen.Should().NotContain(NmtState.Initializing);
+ }
+
+ [Theory]
+ [InlineData(0x01, 0xFF)] // a reserved state, toggle clear; the valid reply flips the toggle
+ [InlineData(0x80, 0x7F)] // state 0 with the toggle set: no guarding producer is Initializing
+ public async Task Guarding_Reply_With_A_Reserved_State_Byte_Is_Not_Reported(byte reserved, byte valid)
+ {
+ var session = NewSession();
+ using var busA = Open(session, 1);
+ using var rawBus = Open(session, 2);
+ using var node = CanOpen.OpenNode(busA, nodeId: 0x01);
+ var seen = new System.Collections.Concurrent.ConcurrentQueue();
+ var last = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
+ node.NodeGuardingReceived += (_, e) =>
+ {
+ if (e.ProducerNodeId != 0x11) return;
+ seen.Enqueue(e.State);
+ if (e.State == NmtState.PreOperational) last.TrySetResult(true);
+ };
+ node.StartNodeGuardingConsumer(producerNodeId: 0x11, guardTime: TimeSpan.FromSeconds(30), lifeTimeFactor: 3);
+
+ // The reserved reply is still a reply and takes the toggle baseline, so the valid one
+ // carries the opposite toggle.
+ Send(rawBus, CanOpenCobId.HeartbeatBase + 0x11, new byte[] { reserved });
+ Send(rawBus, CanOpenCobId.HeartbeatBase + 0x11, new byte[] { valid });
+ await last.Task.WithTimeoutAsync(ShortTimeout);
+
+ seen.Should().NotContain(NmtState.Initializing);
+ }
+
[Fact]
public void Sdo_Server_Sizeless_Download_Aborts_OutOfMemory_Before_Passing_The_Cap()
{