diff --git a/docs/protocol.md b/docs/protocol.md index 4c63d52..9d28cc1 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -118,7 +118,9 @@ dead commander is indistinguishable from an idle one. Therefore: Watching continuously rather than in discrete 1 s samples lets a successor attach ≤ 1.1 s after the incumbent's last beat. "Advisory" is stated plainly: Modbus has no compare-and-swap, so two drivers starting in the same window can both pass; the watchdog bounds the consequence. On a clean disconnect the driver writes `LeaseOwner = 0` if it - still holds it. + still holds it. A tick that reads `LeaseOwner = 0` while attached (a PLC restart or a clean release, never another + commander) makes the driver re-take the lease in place along the attach path: map and limits checked, own id + written, `WatchdogFault = 0` written, sequence continued from `CommandAck`; only a foreign non-zero owner is `LeaseHeld`. - **At attach** the driver writes `WatchdogFault = 0` after winning the lease: a latched trip then belongs to a dead predecessor, and leaving it latched would leave the network disarmed for the new commander. `FaultCode = 4` and ErrorStop stay until a Reset, so the trip remains visible and distinguishable from a drive fault. diff --git a/src/ModelingEvolution.GenericAxis.Tests/DriverIntegrationTests.cs b/src/ModelingEvolution.GenericAxis.Tests/DriverIntegrationTests.cs index 1ab4b93..22abca2 100644 --- a/src/ModelingEvolution.GenericAxis.Tests/DriverIntegrationTests.cs +++ b/src/ModelingEvolution.GenericAxis.Tests/DriverIntegrationTests.cs @@ -691,4 +691,42 @@ public async Task StatusChanged_IdleAndCruising_AtLeast5PerSecondAndPublishedVal positions.Should().NotBeEmpty(); positions.Should().OnlyContain(p => rig.Plc.WasPublished((int)Math.Round(p * 1000, MidpointRounding.AwayFromZero))); } + + [Fact(DisplayName = "GA-I-25 A PLC restart under a live session: the unowned lease is re-taken in place (issue #7)")] + public async Task PlcRestart_LeaseOwnerZero_LeaseRetakenAndAxisReportsThePlcState() + { + await using var rig = new LiveRig(); + var track = await rig.ConnectedTrack(power: true); + await track.Carriage.MoveAbsoluteAsync(new Mm(1000), new MmPerS(500)).WaitAsync(T); + rig.Plc.Truth.Homed.Should().BeTrue("anchor: homed and moving before the restart"); + rig.Plc.Truth.CommandAck.Should().BeGreaterThan(0, "anchor: commands were acknowledged"); + var warningsBefore = rig.Logs.GetSnapshot().Count(r => r.Level == LogLevel.Warning); + + rig.Plc.PowerCycle(); + await rig.Plc.WaitFor(t => t is { LeaseOwner: 0, State: 0, Homed: false }, T, "the PLC restarted"); + + await rig.Plc.WaitFor(t => t.LeaseOwner == 1, T, "the driver re-takes the unowned lease"); + await rig.Plc.WaitFor(t => t.WatchdogArmed, T, "the next beat re-arms the watchdog"); + await DriverRig.Until(() => track.LastSnapshot is { LeaseOwner: 1 }, "the driver sees its lease again"); + await Task.Delay(300); // three more ticks: a latched LeaseHeld would show by now + + track.IsConnected.Should().BeTrue(); + rig.Plc.Truth.WatchdogFault.Should().Be(0); + track.Carriage.Status.Error.Should().BeNull("an unowned lease is not another commander"); + track.Carriage.State.Should().Be(AxisState.Disabled, "the state is whatever the restarted PLC reports"); + track.Carriage.Status.Position.Should().BeNull("not homed after the restart"); + var logs = rig.Logs.GetSnapshot(); + logs.Where(r => r.Level == LogLevel.Warning && r.Message.Contains("LeaseOwner (C+9 = holding 9) = 0")) + .Should().ContainSingle().Which.Message.Should().Contain("PLC restarted or lease released; lease re-taken"); + logs.Should().NotContain(r => r.Message.Contains("LeaseHeld")); + logs.Count(r => r.Level == LogLevel.Warning).Should().Be(warningsBefore + 1, "one Warning for the re-take"); + + await track.Carriage.PowerAsync(true).WaitAsync(T); + var move = await Throws(() => track.Carriage.MoveAbsoluteAsync(new Mm(2000), new MmPerS(500))); + move.Error.Should().Be(MotionError.NotHomed, "the restarted PLC cleared Homed; the host sees it honestly"); + + await track.Carriage.HomeAsync().WaitAsync(T); + await track.Carriage.MoveAbsoluteAsync(new Mm(2000), new MmPerS(500)).WaitAsync(T); + rig.Plc.Truth.Position.Should().BeApproximately(2000, 0.005); + } } diff --git a/src/ModelingEvolution.GenericAxis.Tests/LeaseAndHeartbeatTests.cs b/src/ModelingEvolution.GenericAxis.Tests/LeaseAndHeartbeatTests.cs index 22dc86d..1a646d0 100644 --- a/src/ModelingEvolution.GenericAxis.Tests/LeaseAndHeartbeatTests.cs +++ b/src/ModelingEvolution.GenericAxis.Tests/LeaseAndHeartbeatTests.cs @@ -192,6 +192,68 @@ public async Task Tick_ForeignLeaseOwner_LatchesLeaseHeld() rig.Plc.CommandWritesSince(0).Count.Should().Be(writes, "a refusal writes nothing"); } + [Fact(DisplayName = "GA-U-12b An unowned lease seen in a tick is re-taken in place, along the attach path (issue #7)")] + public async Task Tick_LeaseOwnerZero_RetakesLeaseClearsWatchdogContinuesFromAck() + { + await using var rig = await new DriverRig().ConnectAsync(); + rig.BehaveLikePlc(); + await rig.TickAsync(); + rig.Axis.State.Should().Be(AxisState.Standstill, "anchor: healthy before the restart"); + + // The PLC restarts: command block cleared, Disabled, not homed, CommandAck 0. + rig.Plc.Set(p => + { + p.LeaseOwner = 0; + p.WatchdogFault = 0; + p.State = 0; + p.Flags = StatusFlags.DriveReady; + p.CommandAck = 0; + }); + var from = rig.Plc.OpCount; + await rig.TickAsync(); + + var writes = rig.Plc.Ops.Skip(from).Where(o => o.IsWrite && o.Address != rig.Plc.Map.Heartbeat).ToArray(); + writes.Select(w => (w.Address, w.Values[0])).Should().Equal( + [(rig.Plc.Map.LeaseOwner, DriverRig.Owner), (rig.Plc.Map.WatchdogFault, 0), (rig.Plc.Map.Command, 0)], + "own id, then WatchdogFault 0, then the attach's [Command = 0 (Disabled), CommandSeq = CommandAck]"); + writes[2].Values.Should().Equal([(ushort)0, (ushort)0]); + rig.Plc.LeaseOwner.Should().Be(DriverRig.Owner); + rig.Device.IsConnected.Should().BeTrue(); + rig.Axis.State.Should().Be(AxisState.Disabled); + rig.Axis.Status.Error.Should().BeNull(); + rig.Device.Engine.Sequence.Should().Be(0, "the sequence continues from the restarted PLC's CommandAck"); + rig.LogsAt(LogLevel.Warning).Should().ContainSingle(r => r.Message.Contains("LeaseOwner (C+9 = holding 9) = 0")); + rig.LogsAt(LogLevel.Error).Should().BeEmpty(); + + await rig.TickAsync(3); + rig.Plc.Writes.Count(w => w.Address == rig.Plc.Map.LeaseOwner).Should().Be(2, "taken at attach, re-taken once"); + } + + [Fact(DisplayName = "GA-U-12c A re-take onto a PLC with another map version writes nothing and latches ProtocolMismatch")] + public async Task Tick_LeaseOwnerZeroWrongMapVersion_RefusedNothingWritten() + { + await using var rig = await new DriverRig().ConnectAsync(); + rig.BehaveLikePlc(); + await rig.TickAsync(); + var failures = 0; + rig.Device.Heartbeat.TickFailed += (_, _) => Interlocked.Increment(ref failures); + + rig.Plc.Set(p => + { + p.LeaseOwner = 0; + p.MapVersion = 2; + }); + var from = rig.Plc.OpCount; + rig.Time.Advance(rig.Options.HeartbeatInterval); + await DriverRig.Until(() => Volatile.Read(ref failures) == 1); + await DriverRig.Settle(); + + rig.Plc.Ops.Skip(from).Where(o => o.IsWrite && o.Address != rig.Plc.Map.Heartbeat).Should().BeEmpty(); + rig.Plc.LeaseOwner.Should().Be(0); + rig.Axis.Status.Error.Should().Be(MotionError.ProtocolMismatch); + rig.LogsAt(LogLevel.Error).Should().Contain(r => r.Message.Contains("lease re-take refused; nothing was written")); + } + /// Advances fake time one interval at a time, waiting for the loop's next read before the next step. private static async Task Pump(FakeTimeProvider time, FakePlcChannel plc, Func done) { diff --git a/src/ModelingEvolution.GenericAxis.Tests/Support/MiniPlc.cs b/src/ModelingEvolution.GenericAxis.Tests/Support/MiniPlc.cs index 663ed32..c859564 100644 --- a/src/ModelingEvolution.GenericAxis.Tests/Support/MiniPlc.cs +++ b/src/ModelingEvolution.GenericAxis.Tests/Support/MiniPlc.cs @@ -157,6 +157,27 @@ public MiniPlc(MiniPlcOptions? options = null) public void Teleport(double position) => _actions.Enqueue(() => { _p = position; _v = 0; }); /// Runs an action on the scan thread before the next scan. + /// + /// A PLC restart (issue #7): the PLC comes back with its command block cleared (LeaseOwner = 0, + /// WatchdogFault = 0, Heartbeat = 0), State Disabled, Homed off, CommandAck = 0 and the + /// trip counter at 0 (it counts since power-up). The TCP server stays up, as when the link reconnects + /// transparently. Applied at the start of the next scan. + /// + public void PowerCycle() => _actions.Enqueue(() => + { + for (var offset = 0; offset <= 11; offset++) Set(C(offset), 0); + _state = 0; + _fault = 0; + _ack = 0; + _v = 0; + _homed = false; + _inPosition = false; + _armed = false; + _needFreshEnable = false; + _lastBeat = 0; + _trips = 0; + }); + public void OnScan(Action action) => _actions.Enqueue(action); /// diff --git a/src/ModelingEvolution.GenericAxis/AxisEngine.cs b/src/ModelingEvolution.GenericAxis/AxisEngine.cs index 1b8e9ac..3ed4723 100644 --- a/src/ModelingEvolution.GenericAxis/AxisEngine.cs +++ b/src/ModelingEvolution.GenericAxis/AxisEngine.cs @@ -289,6 +289,34 @@ await _channel.WriteRegistersAsync(_unit, _map.Command, [word, s.CommandAck], "c RaiseStatus(status); } + /// Attached and not disconnecting: the device may re-attach in place (issue #7). + public bool IsCommanding + { + get { lock (_sync) return _attached && !_detaching; } + } + + /// + /// Issue #7: re-attach in place after the PLC dropped the lease (LeaseOwner = 0, a PLC restart or a clean + /// release). A verb in flight fails with CommunicationLost naming and writes nothing + /// more; then runs on — sequence from CommandAck, Enable + /// from the PLC state, stale edges cleared, overlays dropped (the axis is attached anew). + /// + /// when the engine is detached or detaching — nothing was written. + public async Task ReattachAsync(PlcSnapshot fresh, string reason, CancellationToken ct) + { + Running? running; + lock (_sync) + { + if (!_attached || _detaching) return false; + running = _running; + running?.MarkDetached(reason); + } + + running?.Cancel(); + await AttachAsync(fresh, ct).ConfigureAwait(false); + return true; + } + /// /// The start of a clean disconnect (review #8): the verb in flight — which the disconnect's Stop is about to /// cancel — fails with CommunicationLost "detached during <verb>", not a bare cancellation, and no new verb @@ -984,7 +1012,7 @@ private async Task RunVerbAsync(string verb, Action? gua { // Review #8: from the verb's point of view the link is gone — the caller did not cancel. var lost = Error(MotionError.CommunicationLost, - $"the device was detached during {verb}; {verb} did not complete"); + $"{running.DetachReason ?? "the device was detached"} during {verb}; {verb} did not complete"); _logger?.LogWarning("{Message}", lost.Message); throw lost; } @@ -1248,7 +1276,14 @@ private sealed class Running(string verb, CancellationTokenSource cts) : IDispos private int _detached; - public void MarkDetached() => Volatile.Write(ref _detached, 1); + /// Why the verb was cut off, when it was not a disconnect (issue #7: the PLC dropped the lease). + public string? DetachReason { get; private set; } + + public void MarkDetached(string? reason = null) + { + DetachReason ??= reason; + Volatile.Write(ref _detached, 1); + } public void Cancel() { diff --git a/src/ModelingEvolution.GenericAxis/AxisHeartbeat.cs b/src/ModelingEvolution.GenericAxis/AxisHeartbeat.cs index 347fe1c..8b354d7 100644 --- a/src/ModelingEvolution.GenericAxis/AxisHeartbeat.cs +++ b/src/ModelingEvolution.GenericAxis/AxisHeartbeat.cs @@ -76,6 +76,14 @@ public AxisHeartbeat(string axis, IModbusChannel channel, byte unit, RegisterMap /// Raised when a tick fails after the channel's retry. public event EventHandler? TickFailed; + /// + /// Issue #7: run inside the tick when it reads LeaseOwner = 0 while this driver holds the lease — a PLC + /// restart or a clean release, never another commander (protocol § FR-11 "Advisory lease"). It re-attaches in + /// place and returns the snapshot the tick then reports, or when the device is not attached + /// for commanding (the tick then reports what it read). A throw fails the tick like a channel failure. + /// + internal Func>? UnownedLease { get; set; } + // ═══════════════════════ lease ═══════════════════════ /// @@ -142,6 +150,15 @@ await _channel.WriteRegisterAsync(_unit, _map.LeaseOwner, OwnerId, "take lease", return (words[0], words[1]); } + /// Re-takes an unowned lease while attached (issue #7): writes own id into LeaseOwner, no + /// watching — 0 is "take it" in the advisory-lease rule. + internal async Task RetakeAsync(CancellationToken ct) + { + await _channel.WriteRegisterAsync(_unit, _map.LeaseOwner, OwnerId, "re-take unowned lease", + ChannelPriority.Heartbeat, ct).ConfigureAwait(false); + _leaseHeld = true; + } + /// Writes WatchdogFault = 0 on the stop lane (design § Device lifecycle step 4). public async Task ClearWatchdogFaultAsync(CancellationToken ct) { @@ -227,6 +244,10 @@ internal async Task TickOnceAsync(CancellationToken ct) _axis, snapshot.LeaseOwner, snapshot.WatchdogFault, snapshot.WatchdogTrips, snapshot.Status.State, (ushort)snapshot.Status.Flags, snapshot.Status.ActualPosition, snapshot.Status.CommandAck); + if (_leaseHeld && snapshot.LeaseOwner == AdvisoryLease.Unowned && UnownedLease is { } retake + && await retake(snapshot, ct).ConfigureAwait(false) is { } reattached) + snapshot = reattached; + if (snapshot.WatchdogFault != 0 && _lastWatchdogFault == 0) _logger?.LogError("{Message}", AxisErrors.Message(_axis, MotionError.WatchdogTripped, $"WATCHDOG TRIPPED on {_channel.Host}:{_channel.Port}; recovery is Reset, then re-command (no re-home)", diff --git a/src/ModelingEvolution.GenericAxis/ModbusAxisDevice.cs b/src/ModelingEvolution.GenericAxis/ModbusAxisDevice.cs index 691f67c..85af5a1 100644 --- a/src/ModelingEvolution.GenericAxis/ModbusAxisDevice.cs +++ b/src/ModelingEvolution.GenericAxis/ModbusAxisDevice.cs @@ -43,6 +43,7 @@ private protected ModbusAxisDevice(DeviceId id, GenericAxisOptions options, usho _engine = new AxisEngine(options, _channel, ownerId, _logger, clock); _heartbeat.Ticked += (_, snapshot) => _engine.OnTick(snapshot); _heartbeat.TickFailed += (_, error) => _engine.OnTickFailed(error); + _heartbeat.UnownedLease = RetakeLeaseAsync; Axis = leafFactory(_engine); Axes = [Axis]; @@ -177,6 +178,51 @@ private async Task AttachAsync(CancellationToken ct) Connected?.Invoke(this, EventArgs.Empty); } + /// + /// Issue #7, protocol § FR-11 "Advisory lease": a tick read LeaseOwner = 0 while attached. Unowned is a PLC + /// restart or a clean release by someone else, never another commander, so the lease is re-taken in place along + /// the initial attach path: the status block the tick read is checked before any write (map version, limit + /// publication, reading range) → LeaseOwner = own id → WatchdogFault = 0 → a fresh read checked + /// again → engine re-attach (sequence continues from CommandAck, Enable from the PLC state, a verb in + /// flight fails CommunicationLost). The axis then shows whatever the PLC reports — typically Disabled, not Homed. + /// A failure throws into the tick: Transport is CommunicationLost, a refusal is ProtocolMismatch / OutOfRange. + /// + /// The fresh snapshot, or when the device is not attached for commanding (attach + /// still running, or a disconnect under way) — then nothing is written. + private async Task RetakeLeaseAsync(PlcSnapshot seen, CancellationToken ct) + { + if (!_connected || !_engine.IsCommanding) return null; + + var o = Options; + ThrowIfRefused(seen.Status, null); + await _heartbeat.RetakeAsync(ct).ConfigureAwait(false); + await _heartbeat.ClearWatchdogFaultAsync(ct).ConfigureAwait(false); + var fresh = await _heartbeat.ReadSnapshotAsync(ChannelPriority.Heartbeat, ct).ConfigureAwait(false); + ThrowIfRefused(fresh.Status, $"LeaseOwner = {OwnerId} and WatchdogFault = 0"); + var reason = $"the PLC dropped the lease (LeaseOwner ({o.Map.Describe(RegisterField.LeaseOwner)}) = 0)"; + if (!await _engine.ReattachAsync(fresh, reason, ct).ConfigureAwait(false)) return null; + + _lastRetakeRefusal = null; + _logger?.LogWarning( + "{Axis}: LeaseOwner ({Register}) = 0 while attached as {Owner} — PLC restarted or lease released; lease " + + "re-taken. The axis now reports the PLC's state {State}{Homed}", + o.Name, o.Map.Describe(RegisterField.LeaseOwner), OwnerId, AxisEngine.MapState(fresh.Status.State), + fresh.Status.Homed ? "" : ", not homed"); + return fresh; + } + + /// The re-take's refusal, logged at Error once per distinct cause (the tick retries every interval). + private void ThrowIfRefused(StatusBlock s, string? written) + { + if (MapRefusal(s, "lease re-take", written) is not { } refusal) return; + if (refusal.Message != _lastRetakeRefusal) + _logger?.LogError("{Message}", refusal.Message); + _lastRetakeRefusal = refusal.Message; + throw refusal; + } + + private string? _lastRetakeRefusal; // tick loop only + /// /// Map version (S+14) and limit publication (S+8…S+13) before any write — both ProtocolMismatch, the PLC /// answering outside the protocol (ADR-35) — and the configured reading range against the effective travel @@ -184,6 +230,19 @@ private async Task AttachAsync(CancellationToken ct) /// private void CheckMap(StatusBlock s) { + if (MapRefusal(s) is not { } refusal) return; + _logger?.LogError("{Message}", refusal.Message); + _refusalLogged = refusal; + throw refusal; + } + + /// The checks of , returning the refusal instead of logging and throwing it. + /// The status block read. + /// What is refused: "attach" or "lease re-take". + /// What was already written, or for nothing. + private MotionException? MapRefusal(StatusBlock s, string act = "attach", string? written = null) + { + var tail = written is null ? "nothing was written" : $"{written} already written"; var o = Options; var m = o.Map; MotionException? refusal = null; @@ -191,8 +250,8 @@ private void CheckMap(StatusBlock s) { refusal = AxisErrors.Create(o.Name, MotionError.ProtocolMismatch, mapVersion - ? "attach refused; nothing was written" - : "attach refused: the limit publication is partial or not sane; nothing was written", + ? $"{act} refused; {tail}" + : $"{act} refused: the limit publication is partial or not sane; {tail}", violation); } else @@ -203,21 +262,18 @@ private void CheckMap(StatusBlock s) var plc = limits.Source == LimitSource.Plc; if (o.ReadMin is { } rmin && rmin > limits.TravelMin) refusal = AxisErrors.Create(o.Name, MotionError.OutOfRange, - $"attach refused: configured ReadMin {Fmt(rmin)} is above TravelMin {Fmt(limits.TravelMin)} " + $"{act} refused: configured ReadMin {Fmt(rmin)} is above TravelMin {Fmt(limits.TravelMin)} " + $"({limits.SourceText}), expected ReadMin ≤ TravelMin", plc ? [AxisErrors.Read(m, RegisterField.TravelMin, s.TravelMin)] : []); else if (o.ReadMax is { } rmax && rmax < limits.TravelMax) refusal = AxisErrors.Create(o.Name, MotionError.OutOfRange, - $"attach refused: configured ReadMax {Fmt(rmax)} is below TravelMax {Fmt(limits.TravelMax)} " + $"{act} refused: configured ReadMax {Fmt(rmax)} is below TravelMax {Fmt(limits.TravelMax)} " + $"({limits.SourceText}), expected ReadMax ≥ TravelMax", plc ? [AxisErrors.Read(m, RegisterField.TravelMax, s.TravelMax)] : []); } } - if (refusal is null) return; - _logger?.LogError("{Message}", refusal.Message); - _refusalLogged = refusal; - throw refusal; + return refusal; } /// The attach refusal already logged, so the attach path does not log it twice.