fix(canopen): report a short classic upload, refuse a long one, stop reporting reserved heartbeat states as boot-up

src/CanKit.Pro.CANopen/CanOpenEvents.cs

@@ -14,7 +14,8 @@ public sealed class HeartbeatReceivedEventArgs : EventArgs
     public byte ProducerNodeId { get; }
 
     /// <summary>Reported NMT state of the producer. <see cref="NmtState.Initializing"/> is used
-    /// for the CiA 301 §7.2.8.3.2 bootup frame (<c>data[0] == 0x00</c>).</summary>
+    /// for the CiA 301 §7.2.8.3.2 bootup frame (<c>data[0] == 0x00</c>). A frame whose state
+    /// byte is one CiA 301 reserves is not reported.</summary>
     public NmtState State { get; }
 
     /// <summary>UTC timestamp captured when the frame was processed on the actor loop.</summary>

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);
     }
 
     // =========================================================================================

src/CanKit.Pro.CANopen/CanOpenNode.cs

@@ -1335,6 +1335,21 @@ private void HandleEmcy(uint cobId, byte[] data)
     // =========================================================================================
     // Heartbeat (FR-CO-008)
     // =========================================================================================
+
+    /// <summary>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.</summary>
+    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<byte>();
+                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.</summary>
         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; }
 
         /// <summary>Numbers this transfer's sends; only the latest can still decide it (#197).</summary>

src/CanKit.Pro.CANopen/ICanOpenNode.cs

@@ -251,6 +251,10 @@ Task SendNmtCommandAsync(NmtCommand command, byte targetNodeId,
 
     /// <summary>Transmits a single SYNC frame (payload-less) on the SYNC COB-ID configured in
     /// <c>1005h</c>.</summary>
+    /// <remarks>This is a raw send, not the producer: it is not gated by the NMT state and not by
+    /// bit 30 of <c>1005h</c> ("device generates SYNC"), so a node in Stopped, or one that is not
+    /// configured to produce SYNC, transmits when asked. The periodic producer
+    /// (<see cref="StartSyncProducer(TimeSpan)"/>) is the one that follows CiA 301 Table 37.</remarks>
     Task SendSyncAsync(CancellationToken cancellationToken = default);
 
     // -----------------------------------------------------------------------------------------

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 |