Merge pull request #461 from dborgards/fix/srdo-disabled-emcy-434

fix(canopen): preserve SRDO errors while EMCY is disabled

Author: dborgards

src/CanKit.Pro.CANopen/CanOpenNode.Safety.cs

@@ -756,6 +756,8 @@ private async Task SendSrdoPairAsync(SrdoPair pair, CancellationToken period)
         }
     }
 
+    bool ISrdoEngineHost.EmcyEnabled => _emcyValid;
+
     void ISrdoEngineHost.EmitEmcy(int srdoNumber, ushort errorCode)
         => RaiseCommunicationError(CommunicationError.SrdoLength, srdoNumber, errorCode);
 

src/CanKit.Pro.CANopen/README.md

@@ -658,7 +658,9 @@ while a communication error is active still carries bit 4. An EMCY held in Stopp
 register of the moment it is sent. Reset Node and Reset Communication end the communication
 errors without an EMCY (transitions 5 and 0) and leave the application's bits in `1001h`. With
 the EMCY switched off (bit 31 of `1014h`) no error is raised, and an error that ends meanwhile
-still clears the bits. Heartbeat, node-guarding and life-guarding timeouts and the CAN
+still clears the bits. A run of short RPDO frames or SRDO pairs is not consumed while disabled:
+if it persists after enabling, the next short frame/pair reports `8210h` once. Toggling the
+producer after that report does not start a new error event (#434). Heartbeat, node-guarding and life-guarding timeouts and the CAN
 controller state raise no EMCY (`8110h`–`8140h`); they are events only.
 
 **Heartbeat.** The producer and the consumer are separate modules. `HeartbeatProducer` sends

src/CanKit.Pro.CANopen/Safety/ISrdoEngineHost.cs

@@ -6,6 +6,10 @@ namespace CanKit.Pro.CANopen.Safety;
 /// events. All calls arrive on the actor loop. The node implements it; tests record it.</summary>
 internal interface ISrdoEngineHost
 {
+    /// <summary>Whether the host's EMCY producer is enabled. An error run is not consumed
+    /// while disabled, matching the node's RPDO reporting rule (#434). Actor loop only.</summary>
+    bool EmcyEnabled { get; }
+
     /// <summary>A single frame, sent at once: the global failsafe command.</summary>
     void Send(uint cobId, byte[] payload);
 

src/CanKit.Pro.CANopen/Safety/SrdoEngine.Consumer.cs

@@ -150,7 +150,9 @@ private void OnSecondFrame(SrdoRuntime rt, byte[] data)
         // Emergency message with error code 8210h shall be produced" — once per run, like RPDOs.
         if (first.Length < rt.TotalBytes)
         {
-            if (!rt.ShortFrameReported)
+            // The node does not consume an RPDO error event while EMCY is disabled.
+            // Keep the SRDO event available too, so enabling mid-run reports the next pair.
+            if (!rt.ShortFrameReported && _host.EmcyEnabled)
             {
                 rt.ShortFrameReported = true;
                 _host.EmitEmcy(rt.Number, 0x8210);

tests/CanKit.Pro.Tests/TestCases/CANopen/CanOpenErrorRegisterTests.cs

@@ -173,6 +173,59 @@ public async Task Srdo_Length_Error_Sets_The_Communication_Bit_Until_The_Engine_
         await ExpectEmcyAsync(wire, 3, 0xFF00, 0x00);
     }
 
+    // #434: the SRDO engine must not latch a disabled error before the node can report it.
+    // RPDO is the existing contract; application EMCYs witness the exact wire sequence.
+    [Theory]
+    [InlineData(false)]
+    [InlineData(true)]
+    public async Task A_Length_Error_While_Emcy_Is_Disabled_Is_Reported_After_Enabling_For_Pdo_And_Srdo(bool srdo)
+    {
+        var session = NewSession();
+        using var bus = Open(session, 0);
+        using var wire = new Wire(session, 2);
+        using var device = OpenClocked(bus, Device, new ManualTimeSource(),
+            new CanOpenNodeOptions { SrdoCount = 1, WritableCommunicationParameters = true });
+        await WaitForStateAsync(device, NmtState.PreOperational);
+        var od = device.ObjectDictionary;
+        od.AddU16(0x2000, 0, 0); od.AddU16(0x2001, 0, 0);
+        if (srdo)
+        {
+            device.Safety().ConfigureSrdoConsumer(1, new SrdoMapping().Add(0x2000, 0, 16).Add(0x2001, 0, 16),
+                TimeSpan.FromMilliseconds(100), TimeSpan.FromMilliseconds(20), 0x123, 0x124);
+            device.Safety().CommitSafetyConfiguration();
+        }
+        else device.ConfigureRpdo(1, new PdoMapping().Add(0x2000, 0, 16).Add(0x2001, 0, 16));
+        await StartAsync(wire, device);
+        async Task Receive(bool shortFrame)
+        {
+            byte[] normal = shortFrame ? new byte[] { 1, 2 } : new byte[] { 1, 2, 3, 4 };
+            await Inject(device, srdo ? 0x123 : Rpdo1, normal);
+            if (srdo) await Inject(device, 0x124, SrdoFrames.Invert(normal));
+        }
+        od.WriteUnsigned(0x1014, 0, EmcyId | CanOpenCobId.InvalidBit);
+        await Receive(shortFrame: true);
+        await Receive(shortFrame: true);
+        od.WriteUnsigned(0x1014, 0, EmcyId);
+        await device.SendEmcyAsync(0xFF00, 0);
+        await ExpectEmcyAsync(wire, 1, 0xFF00, 0);
+        await Receive(shortFrame: true);
+        await device.SendEmcyAsync(0xFF01, 0);
+        await ExpectEmcyAsync(wire, 2, 0x8210, Communication);
+        await ExpectEmcyAsync(wire, 3, 0xFF01, Communication);
+        od.ReadUnsigned(0x2000, 0).Should().Be(0, "short frames remain unprocessed");
+        od.WriteUnsigned(0x1014, 0, EmcyId | CanOpenCobId.InvalidBit);
+        await Receive(shortFrame: true);
+        od.WriteUnsigned(0x1014, 0, EmcyId);
+        await Receive(shortFrame: true);
+        await device.SendEmcyAsync(0xFF02, 0);
+        await ExpectEmcyAsync(wire, 4, 0xFF02, Communication);
+        await Receive(shortFrame: false);
+        await device.SendEmcyAsync(0xFF03, 0);
+        await ExpectEmcyAsync(wire, 5, ErrorReset, 0);
+        await ExpectEmcyAsync(wire, 6, 0xFF03, 0);
+        wire.Count(EmcyId).Should().Be(6, "enabling an already reported run does not report it twice");
+    }
+
     // The rule across sources and the application: each new error is its own EMCY, the register
     // keeps bit 4 while any one is active, the application's bits are ORed in and never cleared by
     // the node, and the 0000h comes when the last communication error ends — with the

tests/CanKit.Pro.Tests/TestCases/CANopen/Safety/SrdoEngineConsumerTests.cs

@@ -386,6 +386,35 @@ public void A_Short_Pair_Raises_Emcy_8210h_Once_And_Is_Not_Processed()
         _host.EmcyEnded.Should().Equal(new[] { 1, 1 }, "a rebuild ends the run too");
     }
 
+    [Fact]
+    public void Disabled_Emcy_Does_Not_Consume_A_Short_Run_And_Enabling_Reports_Exactly_Once()
+    {
+        StartConsumer();
+        _host.EmcyEnabled = false;
+        Frame(0x123, 0x78, 0x56); Frame(0x124, 0x87, 0xA9);
+        Frame(0x123, 0x78, 0x56); Frame(0x124, 0x87, 0xA9);
+        _host.Emcys.Should().BeEmpty();
+        _od.ReadUnsigned(0x2000, 0).Should().Be(0x1234u, "a disabled diagnostic never actuates a short pair");
+        _engine.GetState(1).IsValid.Should().BeFalse();
+        _host.EmcyEnabled = true;
+        Frame(0x123, 0x78, 0x56); Frame(0x124, 0x87, 0xA9);
+        _host.Emcys.Should().Equal((ushort)0x8210);
+        _host.EmcyEnabled = false;
+        Frame(0x123, 0x78, 0x56); Frame(0x124, 0x87, 0xA9);
+        _host.EmcyEnabled = true;
+        Frame(0x123, 0x78, 0x56); Frame(0x124, 0x87, 0xA9);
+        _host.Emcys.Should().HaveCount(1, "toggling EMCY does not start a new already-reported error run");
+        Frame(0x123, Plain); Frame(0x124, Inverse);
+        _host.EmcyEnded.Should().Equal(1);
+        _host.EmcyEnabled = false;
+        Frame(0x123, 0x78, 0x56); Frame(0x124, 0x87, 0xA9);
+        Frame(0x123, Plain); Frame(0x124, Inverse);
+        _host.EmcyEnded.Should().Equal(new[] { 1 }, "an unreported disabled run has no reported cause to end");
+        _host.EmcyEnabled = true;
+        Frame(0x123, 0x78, 0x56); Frame(0x124, 0x87, 0xA9);
+        _host.Emcys.Should().HaveCount(2, "the completed run allows a fresh error");
+    }
+
     /// <summary>Bugbot on #417: entering Operational re-reads every SRDO without a change of its
     /// own, so it does not end a run of short pairs — the error goes on until a pair fits, as an
     /// RPDO's does — and a short pair after it is the same run.</summary>

tests/CanKit.Pro.Tests/TestCases/CANopen/Safety/SrdoEngineProducerTests.cs

@@ -79,6 +79,7 @@ private void Record(uint cobId, byte[] payload)
             Sent.Add((cobId, payload));
             Log.Add($"send:{cobId:X3}");
         }
+        public bool EmcyEnabled { get; set; } = true;
         public void EmitEmcy(int srdoNumber, ushort errorCode) { Emcys.Add(errorCode); Log.Add($"emcy:{errorCode:X4}"); }
         /// <summary>The SRDOs whose reported error ended, in order (<c>emcy-ended:1</c> in <see cref="Log"/>).</summary>
         public readonly List<int> EmcyEnded = new();