fix: enhance COM port registration logging and update GenericSink cla…

[jkdevito]

…ss for input switching.

Pending field testing RS232 comms and confirmation with non-CEN-IO-COM devices.

[Copilot]

Pull Request Overview

This PR enhances COM port registration logging for specific device types and updates the GenericSink class to support input switching by implementing the IRoutingSinkWithSwitchingWithInputPort interface. It also enables verbose logging in the RoutingFeedbackManager. The PR description indicates this is pending field testing for RS232 communications and non-CEN-IO-COM devices.

• Enhanced COM port registration to conditionally register ports only for CrestronControlSystem and CenIoCom102 devices with improved logging
• Updated GenericSink to implement input switching interface with SecondaryAudio signal support
• Enabled verbose routing feedback logging

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
src/PepperDash.Essentials.Devices.Common/Generic/GenericSink.cs	Updated interface to support input switching and added SecondaryAudio signal type, though implementation is incomplete
src/PepperDash.Essentials.Core/Routing/RoutingFeedbackManager.cs	Enabled verbose logging for destination updates to aid in debugging routing behavior

Comments suppressed due to low confidence (6)

src/PepperDash.Essentials.Core/Routing/RoutingFeedbackManager.cs:105

• Variable destination may be null at this access as suggested by this null check.

                Debug.LogMessage(Serilog.Events.LogEventLevel.Warning, "Destination {destination} has not reported an input port yet", this,destination.Key);

src/PepperDash.Essentials.Core/Routing/RoutingFeedbackManager.cs:127

• Variable destination may be null at this access as suggested by this null check.

                    destination.CurrentSourceInfo = tempSourceListItem;                        ;

src/PepperDash.Essentials.Core/Routing/RoutingFeedbackManager.cs:154

• Variable destination may be null at this access as suggested by this null check.

                    destination.CurrentSourceInfo = tempSourceListItem;

src/PepperDash.Essentials.Core/Routing/RoutingFeedbackManager.cs:183

• Variable destination may be null at this access as suggested by this null check.

                Debug.LogMessage(Serilog.Events.LogEventLevel.Warning, "No room found for display {destination}", this, destination.Key);

src/PepperDash.Essentials.Core/Routing/RoutingFeedbackManager.cs:223

• Variable destination may be null at this access as suggested by this null check.

                destination.CurrentSourceInfoKey = "$transient";

src/PepperDash.Essentials.Core/Routing/RoutingFeedbackManager.cs:230

• Variable destination may be null at this access as suggested by this null check.

            destination.CurrentSourceInfoKey = sourceKey;

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The ExecuteSwitch method throws NotImplementedException, but this method will be called by the routing system when routes are executed (see RouteDescriptor.ExecuteRoutes line 79). This will cause runtime failures when GenericSink is used in routing scenarios.

Based on similar implementations in GenericSoftCodec and MockDisplay, this method should:

1. Find the input port matching the inputSelector
2. Update the current input port
3. Fire the InputChanged event

However, there's a dependency issue: the CurrentInputPort property (line 63) is currently read-only and always returns InputPorts[0]. To implement switching functionality, CurrentInputPort needs to be made settable with a backing field (similar to GenericSoftCodec._currentInputPort), or the switching logic needs to be implemented differently.

Example implementation:

public void ExecuteSwitch(object inputSelector)
{
    var inputPort = InputPorts.FirstOrDefault(p => p.Selector == inputSelector);
    if (inputPort == null)
    {
        Debug.LogMessage(LogEventLevel.Warning, "No input port found for selector {inputSelector}", this, inputSelector);
        return;
    }
    
    // Update CurrentInputPort (requires making it settable)
    InputChanged?.Invoke(this, inputPort);
}

[Copilot]

The InputChanged event is declared but never invoked. According to the IRoutingSinkWithSwitchingWithInputPort interface requirements and based on implementations in GenericSoftCodec and MockDisplay, this event should be fired when the input selection changes (i.e., in the ExecuteSwitch method when the input port is successfully changed).

The RoutingFeedbackManager subscribes to this event to track routing changes, so failing to invoke it will break routing feedback functionality for GenericSink devices.

[Copilot]

Changing the interface from IRoutingSinkWithInputPort to IRoutingSinkWithSwitchingWithInputPort is a breaking change that adds new requirements (ExecuteSwitch method and InputChanged event). While the new members have been added, they are not properly implemented:

1. ExecuteSwitch throws NotImplementedException
2. InputChanged is never invoked
3. The existing CurrentInputPort property (line 63) always returns the same port, so switching cannot work

This suggests the implementation is incomplete. Consider whether GenericSink should actually support switching, or if a different approach is needed. If switching is not needed for GenericSink's use case, it may be better to keep the original IRoutingSinkWithInputPort interface.

[andrew-welker]

closing in favor of #1356