rc.10 regression: RoutingFeedbackManager deletes route descriptors it cannot rebuild, so destinations with a destinationPortKey can never be cleared

[ediablo80]

Summary

RoutingFeedbackManager.UpdateDestinationImmediate removes a destination's route descriptors before re-deriving them, and does not restore them when the re-derivation yields no source. The destination's routing state is then gone, so a subsequent release finds nothing to release and the hardware is never torn down.

User-visible symptom: routing an "off"/clear source to an affected destination silently does nothing. The route stays up.

Regression in v3.0.0-rc.10. Not present in rc.5.

Why it only hits some destinations

RemoveRouteDescriptors matches on input port key:

.Where(rd => rd.Destination == destination &&
    (string.IsNullOrEmpty(inputPortKey) || (rd.InputPort != null && rd.InputPort.Key == inputPortKey)))

The feedback manager always passes a concrete inputPort.Key. So:

• A destination configured without destinationPortKey builds descriptors with a null InputPort (logged as dest:auto). They never match, and survive.
• A destination configured with destinationPortKey builds descriptors carrying that port. They match, and are removed.

That is the whole difference between a destination that works and one that doesn't.

Observed

Three destinations, same room, same NVX fabric. display has no destinationPortKey; content and audio do.

Routing a source to audio, then clearing it:

11:03:31.003  Adding route descriptor: tx-zoom -> dsp-audio-in:anyVideoIn (Audio)
11:03:31.003  Adding route descriptor: tx-zoom -> dsp-audio-in:anyVideoIn (Video)
              ... route executes, NVX stream starts ...
11:03:31.611  [routingFeedbackManager] Updating destination dsp-audio-in with inputPort anyVideoIn
11:03:31.611  Removed 2 route descriptor(s) for 'dsp-audio-in':'anyVideoIn'
11:03:31.612  [routingFeedbackManager] Setting dsp-audio-in current Audio source to none
11:03:31.614  [routingFeedbackManager] Setting dsp-audio-in current Video source to none

11:03:33.689  Release route for 'dsp-audio-in':'anyVideoIn'
11:03:33.689  Removed 0 route descriptor(s) for 'dsp-audio-in':'anyVideoIn'

No switch is executed, no stream cleared. The decoder stays tuned.

The same clear on display works, because its descriptors were never removed:

11:03:23.146  Removed 2 route descriptor(s) for 'display':'auto'
11:03:23.146  [display] Releasing current route: tx-pc (Audio)
11:03:23.147  *** Executing switch: null to rx-display with signal type Audio ***
11:03:23.148  [rx-display] Clearing stream
11:03:23.250  activeRoutes: {"display":"none"}

Cause

RouteDescriptorCollection.DefaultCollection.RemoveRouteDescriptors(destination, inputPort.Key);

foreach (var signalType in new[] { eRoutingSignalType.Audio, eRoutingSignalType.Video })
{
    if (!TryGetActiveSourcePort(firstTieLine, signalType, new HashSet<string>(), out var sourcePort))
        continue;                               // bails - descriptors stay removed

    var source = sourcePort?.ParentDevice as IRoutingOutputs;
    destination.SetCurrentSource(signalType, source as IRoutingSource);

    if (source == null)
        continue;                               // bails - descriptors stay removed

    var (route, _) = destination.GetRouteToSource(source, signalType, inputPort, sourcePort);
    RouteDescriptorCollection.DefaultCollection.AddRouteDescriptor(route);
}

The removal is unconditional; both re-add paths are conditional. Any upstream walk that fails to resolve a source destroys state it cannot rebuild. In our case TryGetActiveSourcePort resolves to a null source while walking back through the NVX virtual matrix, roughly 600 ms after the route executed.

Suggested fix

Do not remove before you can replace. Either:

• derive the replacement descriptors first, and only remove the old ones once a replacement exists for that signal type; or
• remove per signal type, inside the loop, after TryGetActiveSourcePort has succeeded.

Setting the current source to none when the walk fails seems reasonable on its own — the feedback genuinely is unknown. Deleting the route descriptors is the part that breaks teardown, because ReleaseRouteInternal depends on them to know what to switch off.

Worth noting this interacts with #1494: before that fix this call removed at most one descriptor, so the blast radius was smaller. The #1494 fix is correct; this call site just needs to stop assuming removal is free.

Workaround

Omit destinationPortKey from the destination so its descriptors are keyed auto and escape the match. Only viable when a single tie line feeds the sink, so the port still resolves unambiguously.

Environment

• Essentials v3.0.0-rc.10 on a CP4N
• PepperDash.Essentials.Plugins.Crestron.Nvx 4.0.0-feat-matrix-routing-videosync-status.4
• Destinations: LG display (no port key), Zoom codec hdmiIn1, genericsink anyVideoIn

[ediablo80]

Follow-up after testing the destinationPortKey workaround on three destinations in one room. It is not generally available, because whether a destination needs that property depends on its sink type — and the destinations that need it are exactly the ones this bug breaks.

The route trace asks a sink for CurrentInputPort first, and only falls back to the configured port when the sink reports nothing:

if (device is IRoutingSinkWithFeedback reporting && reporting.CurrentInputPort != null)
    return configuredPort == null || current.Key == configuredPort ? current : null;

if (configuredPort != null)
    return device.InputPorts?.FirstOrDefault(p => p.Key == configuredPort);

• GenericSink.CurrentInputPort is _currentInputPort ?? InputPorts[0] — never null. Works without destinationPortKey, so it can take the workaround.
• MockVC.CurrentInputPort is { get; private set; } and is never assigned; ExecuteSwitch only sets _SharingSource. Without destinationPortKey the trace cannot resolve it and the destination reports nothing routed.

Measured on the same room, same NVX fabric:

Destination	sink type	destinationPortKey	feedback	clear
display	lgDisplay	none	works	works
audio	genericsink	none	works	works
content	mockvc	hdmiIn1	works	fails

Removing destinationPortKey from the content destination restored teardown but broke its feedback; putting it back restored feedback and broke teardown again. Both directions reproduced.

Clear on the two unaffected destinations:

Removed 2 route descriptor(s) for 'display':'auto'
[display] Releasing current route: tx-zoom (Audio)
*** Executing switch: null to rx-display with signal type Audio ***
[rx-display] Clearing stream
activeRoutes: {"display":"none"}

Clear on content:

Release route for 'zoomRoom':'hdmiIn1'
Removed 0 route descriptor(s) for 'zoomRoom':'hdmiIn1'

No switch executed, no stream cleared, decoder stays tuned.

So for any destination whose sink does not report CurrentInputPort — mock codecs here, but equally any sink that leaves it unset — there is no configuration that yields both working feedback and working teardown. That seems worth weighing when prioritising.

[ndorin]

Thanks for the analysis, and for the follow-up table. It showed clearly that there's no configuration that works for a sink like MockVC.

Your diagnosis is right, and the regression came from our own changes. The feedback manager removed a port's descriptors before it knew whether it could rebuild them, and #1494 made that removal take all of them. The fix is in #1497. It takes your first suggestion:

• A new RouteDescriptorCollection.ReplaceRouteDescriptor replaces the descriptor for one destination, input port and signal type, and only when there is a replacement. If the existing descriptor already names the same source, it's kept as is, because that's the one that was executed and it holds the ports' in-use registrations.
• The feedback manager no longer removes anything up front. A walk that finds no source still sets the current source to none, as you suggested, but leaves the descriptors so the release can tear the route down.

The PR adds a test that reproduces your sequence with a sink that doesn't report CurrentInputPort. It fails on rc.10 with an empty collection and passes with the fix.

Two related things are deliberately out of scope here:

• The walk resolving no source through the NVX virtual matrix about 600 ms after the route executes. That's probably in how the plugin reports CurrentRoutes. If you can share what that matrix reports in CurrentRoutes for the affected output at that point, it would help track down.
• MockVC.CurrentInputPort never being assigned.

[ndorin]

Merged into release