diff --git a/src/PepperDash.Essentials.Core/Routing/RouteDescriptorCollection.cs b/src/PepperDash.Essentials.Core/Routing/RouteDescriptorCollection.cs index 87e03a414..49b3f22cf 100644 --- a/src/PepperDash.Essentials.Core/Routing/RouteDescriptorCollection.cs +++ b/src/PepperDash.Essentials.Core/Routing/RouteDescriptorCollection.cs @@ -164,4 +164,48 @@ public List RemoveRouteDescriptors(IRoutingInputs destination, return removed; } + + /// + /// Records as the route for its destination, input port and signal + /// type, removing whatever was recorded there before - but only once there is something to put in + /// its place. Used to resync the collection with routing feedback without ever leaving a + /// destination with no descriptor, which would leave a later release nothing to tear down. + /// + /// + /// When the existing descriptor already names the same source, nothing changes: that descriptor is + /// the one that was executed, and it holds the output ports' in-use registrations. + /// + /// The descriptor to record. Null is ignored. + /// True if the collection changed. + public bool ReplaceRouteDescriptor(RouteDescriptor replacement) + { + if (replacement == null) + { + return false; + } + + var existing = RouteDescriptors + .Where(rd => rd.Destination == replacement.Destination && + rd.SignalType == replacement.SignalType && + rd.InputPort?.Key == replacement.InputPort?.Key) + .ToList(); + + if (existing.Count == 1 && existing[0].Source == replacement.Source) + { + return false; + } + + foreach (var descriptor in existing) + { + RouteDescriptors.Remove(descriptor); + } + + RouteDescriptors.Add(replacement); + RouteDescriptorCollectionChanged?.Invoke(this, EventArgs.Empty); + + Debug.LogMessage(LogEventLevel.Debug, "Replaced {count} route descriptor(s) for '{destination}':'{inputPortKey}' ({signalType}) with route from {source}", + existing.Count, replacement.Destination?.Key, replacement.InputPort?.Key ?? "auto", replacement.SignalType, replacement.Source?.Key); + + return true; + } } diff --git a/src/PepperDash.Essentials.Core/Routing/RoutingFeedbackManager.cs b/src/PepperDash.Essentials.Core/Routing/RoutingFeedbackManager.cs index 73c995a83..6a8e8ffc3 100644 --- a/src/PepperDash.Essentials.Core/Routing/RoutingFeedbackManager.cs +++ b/src/PepperDash.Essentials.Core/Routing/RoutingFeedbackManager.cs @@ -394,10 +394,12 @@ RoutingInputPort inputPort // Audio and video can come from different sources (e.g. breakaway on a matrix), and a // purely topological search (GetRouteToSource) only proves a source *could* be routed // here, not that it currently is - so it can't be used to report feedback. + // + // The route descriptors for this port are only ever replaced, never just removed: a release + // relies on them to know what to switch off, so a walk that can't name a source (feedback + // still settling, or a midpoint that reports it differently) must leave them in place. try { - RouteDescriptorCollection.DefaultCollection.RemoveRouteDescriptors(destination, inputPort.Key); - foreach (var signalType in new[] { eRoutingSignalType.Audio, eRoutingSignalType.Video }) { if (!firstTieLine.Type.HasFlag(signalType)) @@ -434,7 +436,7 @@ RoutingInputPort inputPort var (route, _) = destination.GetRouteToSource(source, signalType, inputPort, sourcePort); - RouteDescriptorCollection.DefaultCollection.AddRouteDescriptor(route); + RouteDescriptorCollection.DefaultCollection.ReplaceRouteDescriptor(route); } } catch (Exception ex) diff --git a/src/PepperDash.Essentials.Tests/Routing/RouteDescriptorCollectionTests.cs b/src/PepperDash.Essentials.Tests/Routing/RouteDescriptorCollectionTests.cs index a48fec727..eed15734b 100644 --- a/src/PepperDash.Essentials.Tests/Routing/RouteDescriptorCollectionTests.cs +++ b/src/PepperDash.Essentials.Tests/Routing/RouteDescriptorCollectionTests.cs @@ -169,4 +169,85 @@ public void ChangingSourceRepeatedly_NeverAccumulatesDescriptors() previous = source.Key; } } + + // ── ReplaceRouteDescriptor (issue #1496) ──────────────────────────────────────────────────── + + private static RouteDescriptor OnHdmiIn1(FakeSource source, FakeSink sink, eRoutingSignalType type) => + new RouteDescriptor(source, sink, sink.HdmiIn1, source.Out, type); + + [Fact] + public void ReplaceRouteDescriptor_SwapsTheDescriptorForThatPortAndSignalOnly() + { + var collection = new RouteDescriptorCollection(); + var sink = new FakeSink("codec"); + var laptop = new FakeSource("laptop"); + var oldAudio = OnHdmiIn1(laptop, sink, eRoutingSignalType.Audio); + var oldVideo = OnHdmiIn1(laptop, sink, eRoutingSignalType.Video); + var otherPort = new RouteDescriptor(laptop, sink, sink.HdmiIn2, laptop.Out, eRoutingSignalType.Video); + collection.AddRouteDescriptor(oldAudio); + collection.AddRouteDescriptor(oldVideo); + collection.AddRouteDescriptor(otherPort); + + var newVideo = OnHdmiIn1(new FakeSource("bluray"), sink, eRoutingSignalType.Video); + collection.ReplaceRouteDescriptor(newVideo).Should().BeTrue(); + + collection.Descriptors.Should().BeEquivalentTo(new[] { oldAudio, newVideo, otherPort }); + } + + [Fact] + public void ReplaceRouteDescriptor_KeepsTheExecutedDescriptorWhenTheSourceIsUnchanged() + { + // Feedback confirming the route that was just made must not swap out the descriptor that + // was executed, which holds the output ports' in-use registrations. + var collection = new RouteDescriptorCollection(); + var sink = new FakeSink("codec"); + var laptop = new FakeSource("laptop"); + var executed = OnHdmiIn1(laptop, sink, eRoutingSignalType.Video); + collection.AddRouteDescriptor(executed); + var changed = 0; + collection.RouteDescriptorCollectionChanged += (_, _) => changed++; + + collection.ReplaceRouteDescriptor(OnHdmiIn1(laptop, sink, eRoutingSignalType.Video)).Should().BeFalse(); + + collection.Descriptors.Should().ContainSingle().Which.Should().BeSameAs(executed); + changed.Should().Be(0); + } + + [Fact] + public void ReplaceRouteDescriptor_IgnoresNull() + { + var collection = new RouteDescriptorCollection(); + var sink = new FakeSink("codec"); + var executed = OnHdmiIn1(new FakeSource("laptop"), sink, eRoutingSignalType.Video); + collection.AddRouteDescriptor(executed); + + collection.ReplaceRouteDescriptor(null!).Should().BeFalse(); + + collection.Descriptors.Should().ContainSingle().Which.Should().BeSameAs(executed); + } + + [Fact] + public void AFeedbackPassThatCannotRebuildARoute_LeavesItForTheReleaseToTearDown() + { + // The #1496 sequence: a port-keyed AudioVideo route is made, feedback for that port arrives + // but can't name a source (so nothing is replaced), and then the destination is cleared. + var collection = new RouteDescriptorCollection(); + var matrix = new FakeMatrix(); + var sink = new FakeSink("zoomRoom"); + var (audio, video) = AudioVideoRoute(new FakeSource("tx-zoom"), sink, matrix); + foreach (var descriptor in new[] { audio, video }) + { + collection.AddRouteDescriptor(descriptor); + descriptor.ExecuteRoutes(); + } + + // Feedback pass with no source for either signal: no replacement is offered. + + var released = collection.RemoveRouteDescriptors(sink, "hdmiIn1"); + released.Should().BeEquivalentTo(new[] { audio, video }); + foreach (var descriptor in released) + descriptor.ReleaseRoutes(clearRoute: true); + matrix.Out1.InUseTracker.InUseCountFeedback.IntValue.Should().Be(0); + } } + diff --git a/src/PepperDash.Essentials.Tests/Routing/RoutingFeedbackManagerTests.cs b/src/PepperDash.Essentials.Tests/Routing/RoutingFeedbackManagerTests.cs new file mode 100644 index 000000000..64f27fc04 --- /dev/null +++ b/src/PepperDash.Essentials.Tests/Routing/RoutingFeedbackManagerTests.cs @@ -0,0 +1,151 @@ +using System.Reflection; +using FluentAssertions; +using PepperDash.Essentials.Core; +using PepperDash.Essentials.Core.Routing; +using Xunit; + +namespace PepperDash.Essentials.Tests.Routing; + +/// +/// Drives 's per-destination update directly (it is private and +/// normally runs off a debounce timer) against a source -> matrix -> sink topology. The manager works +/// on the global and , +/// so every device here has a key unique to the test instance and is cleaned up afterwards. +/// +/// +/// Only the path where the walk finds no source is tested here. Any path that rebuilds a route calls +/// Extensions.GetRouteToSource, whose static constructor subscribes to the real Crestron SDK's +/// CrestronEnvironment.ProgramStatusEventHandler - which intermittently hangs off a processor. +/// Replacement itself is covered by RouteDescriptorCollectionTests. +/// +public sealed class RoutingFeedbackManagerTests : IDisposable +{ + private sealed class FakeSource : IRoutingSource + { + public FakeSource(string key) + { + Key = key; + Out = new RoutingOutputPort("out", eRoutingSignalType.AudioVideo, eRoutingPortConnectionType.Hdmi, "out", this); + OutputPorts = new RoutingPortCollection { Out }; + } + + public string Key { get; } + public string Name => Key; + public RoutingOutputPort Out { get; } + public RoutingPortCollection OutputPorts { get; } + } + + private sealed class FakeMatrix : IRoutingMidpointWithFeedback + { + public FakeMatrix(string key) + { + Key = key; + In1 = new RoutingInputPort("in1", eRoutingSignalType.AudioVideo, eRoutingPortConnectionType.Hdmi, "in1", this); + Out1 = new RoutingOutputPort("out1", eRoutingSignalType.AudioVideo, eRoutingPortConnectionType.Hdmi, "out1", this); + InputPorts = new RoutingPortCollection { In1 }; + OutputPorts = new RoutingPortCollection { Out1 }; + } + + public string Key { get; } + public RoutingInputPort In1 { get; } + public RoutingOutputPort Out1 { get; } + public RoutingPortCollection InputPorts { get; } + public RoutingPortCollection OutputPorts { get; } + public List CurrentRoutes { get; } = new List(); +#pragma warning disable CS0067 // Required by the interface; these tests never raise it. + public event RouteChangedEventHandler? RouteChanged; +#pragma warning restore CS0067 + + public void ExecuteSwitch(object inputSelector, object outputSelector, eRoutingSignalType signalType) { } + public void ClearRoute(object outputSelector, eRoutingSignalType signalType) { } + } + + /// A sink that, like MockVC, never reports a current input - so routes to it are port-keyed. + private sealed class FakeSink : IRoutingSinkWithFeedback + { + public FakeSink(string key) + { + Key = key; + HdmiIn1 = new RoutingInputPort("hdmiIn1", eRoutingSignalType.AudioVideo, eRoutingPortConnectionType.Hdmi, "hdmiIn1", this); + InputPorts = new RoutingPortCollection { HdmiIn1 }; + } + + public string Key { get; } + public string Name => Key; + public RoutingInputPort HdmiIn1 { get; } + public RoutingPortCollection InputPorts { get; } + public RoutingInputPort? CurrentInputPort => null; + public Dictionary CurrentSources { get; } = new(); + public Dictionary CurrentSourceKeys { get; } = new(); +#pragma warning disable CS0067 + public event InputChangedEventHandler? InputChanged; + public event EventHandler? CurrentSourcesChanged; +#pragma warning restore CS0067 + + public void ExecuteSwitch(object inputSelector) { } + + public void SetCurrentSource(eRoutingSignalType signalType, IRoutingSource sourceDevice) + { + CurrentSources[signalType] = sourceDevice; + CurrentSourceKeys[signalType] = sourceDevice?.Key!; + } + } + + private static readonly MethodInfo UpdateDestinationImmediate = + typeof(RoutingFeedbackManager).GetMethod("UpdateDestinationImmediate", BindingFlags.NonPublic | BindingFlags.Instance) + ?? throw new InvalidOperationException("RoutingFeedbackManager.UpdateDestinationImmediate not found"); + + private readonly string _id = Guid.NewGuid().ToString("N")[..8]; + private readonly FakeSource _zoom; + private readonly FakeMatrix _matrix; + private readonly FakeSink _sink; + private readonly List _tieLines; + private readonly RoutingFeedbackManager _manager; + + public RoutingFeedbackManagerTests() + { + _zoom = new FakeSource($"tx-zoom-{_id}"); + _matrix = new FakeMatrix($"nvx-{_id}"); + _sink = new FakeSink($"zoomRoom-{_id}"); + _tieLines = new List + { + new TieLine(_zoom.Out, _matrix.In1), + new TieLine(_matrix.Out1, _sink.HdmiIn1), + }; + lock (TieLineCollection.Default) TieLineCollection.Default.AddRange(_tieLines); + _manager = new RoutingFeedbackManager($"rfm-{_id}", "Routing Feedback Manager"); + } + + public void Dispose() + { + lock (TieLineCollection.Default) + foreach (var tieLine in _tieLines) TieLineCollection.Default.Remove(tieLine); + RouteDescriptorCollection.DefaultCollection.RemoveRouteDescriptors(_sink); + } + + private void RunFeedbackPass() => UpdateDestinationImmediate.Invoke(_manager, new object[] { _sink, _sink.HdmiIn1 }); + + private (RouteDescriptor audio, RouteDescriptor video) RouteToSink(FakeSource source) + { + var audio = new RouteDescriptor(source, _sink, _sink.HdmiIn1, source.Out, eRoutingSignalType.Audio); + var video = new RouteDescriptor(source, _sink, _sink.HdmiIn1, source.Out, eRoutingSignalType.Video); + RouteDescriptorCollection.DefaultCollection.AddRouteDescriptor(audio); + RouteDescriptorCollection.DefaultCollection.AddRouteDescriptor(video); + return (audio, video); + } + + private List DescriptorsForSink() => + RouteDescriptorCollection.DefaultCollection.Descriptors.Where(d => d.Destination == _sink).ToList(); + + [Fact] + public void WhenTheWalkFindsNoSource_TheRouteDescriptorsSurviveForTheRelease() + { + // Issue #1496: the route was made, but the matrix's feedback doesn't (yet) show it. + var (audio, video) = RouteToSink(_zoom); + + RunFeedbackPass(); + + _sink.CurrentSourceKeys[eRoutingSignalType.Video].Should().BeNull("the walk found nothing routed"); + DescriptorsForSink().Should().BeEquivalentTo(new[] { audio, video }); + } +}