diff --git a/src/CanKit.Pro.CANopen/CanOpenNode.Safety.cs b/src/CanKit.Pro.CANopen/CanOpenNode.Safety.cs index 1319e1ad..f3988623 100644 --- a/src/CanKit.Pro.CANopen/CanOpenNode.Safety.cs +++ b/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); diff --git a/src/CanKit.Pro.CANopen/README.md b/src/CanKit.Pro.CANopen/README.md index 24c41f0e..ae07012a 100644 --- a/src/CanKit.Pro.CANopen/README.md +++ b/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 diff --git a/src/CanKit.Pro.CANopen/Safety/ISrdoEngineHost.cs b/src/CanKit.Pro.CANopen/Safety/ISrdoEngineHost.cs index 3d8b8cbc..6a4f2b80 100644 --- a/src/CanKit.Pro.CANopen/Safety/ISrdoEngineHost.cs +++ b/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. internal interface ISrdoEngineHost { + /// 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. + bool EmcyEnabled { get; } + /// A single frame, sent at once: the global failsafe command. void Send(uint cobId, byte[] payload); diff --git a/src/CanKit.Pro.CANopen/Safety/SrdoEngine.Consumer.cs b/src/CanKit.Pro.CANopen/Safety/SrdoEngine.Consumer.cs index 20c54633..ebedc27a 100644 --- a/src/CanKit.Pro.CANopen/Safety/SrdoEngine.Consumer.cs +++ b/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); diff --git a/tests/CanKit.Pro.Tests/TestCases/CANopen/CanOpenErrorRegisterTests.cs b/tests/CanKit.Pro.Tests/TestCases/CANopen/CanOpenErrorRegisterTests.cs index e00c2e0b..6d0f6790 100644 --- a/tests/CanKit.Pro.Tests/TestCases/CANopen/CanOpenErrorRegisterTests.cs +++ b/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 diff --git a/tests/CanKit.Pro.Tests/TestCases/CANopen/Safety/SrdoEngineConsumerTests.cs b/tests/CanKit.Pro.Tests/TestCases/CANopen/Safety/SrdoEngineConsumerTests.cs index 584ba606..838be77b 100644 --- a/tests/CanKit.Pro.Tests/TestCases/CANopen/Safety/SrdoEngineConsumerTests.cs +++ b/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"); + } + /// 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. diff --git a/tests/CanKit.Pro.Tests/TestCases/CANopen/Safety/SrdoEngineProducerTests.cs b/tests/CanKit.Pro.Tests/TestCases/CANopen/Safety/SrdoEngineProducerTests.cs index 872f84ec..8c808f37 100644 --- a/tests/CanKit.Pro.Tests/TestCases/CANopen/Safety/SrdoEngineProducerTests.cs +++ b/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}"); } /// The SRDOs whose reported error ended, in order (emcy-ended:1 in ). public readonly List EmcyEnded = new();