-
Notifications
You must be signed in to change notification settings - Fork 84
Multiple Fixes #1356
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Multiple Fixes #1356
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements multiple fixes focused on improving logging behavior and updating Portal URL parsing logic. The changes reduce console traffic by adjusting log levels for subscription events, enhance error message clarity, and update URL regex patterns to correctly parse system and template UUIDs from current Portal URL formats.
Key Changes
- Moved messenger subscription state logging from Information/Warning levels to Debug level to reduce console noise
- Updated URL parsing regex patterns in EssentialsConfig to handle current Portal URL structure with
/detail/segments - Enhanced GenericSink class to implement
IRoutingSinkWithSwitchingWithInputPortinterface for improved routing support
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| MobileControlSystemController.cs | Added warning log when messenger subscriptions are disabled to help with configuration troubleshooting |
| MessengerBase.cs | Reduced logging verbosity by removing redundant warnings and changing subscription event logs to Debug level; improved exception logging with separated error message and stack trace |
| GenericSink.cs | Updated interface implementation to support input switching capabilities and added SecondaryAudio signal type support |
| RoutingFeedbackManager.cs | Enabled verbose debug logging for destination updates to aid in routing troubleshooting |
| EssentialsConfig.cs | Fixed regex patterns for SystemUuid and TemplateUuid extraction to handle current Portal URL format with /detail/ path segments; normalized indentation from tabs to spaces |
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.
| { | ||
| if (!Config.EnableMessengerSubscriptions) | ||
| { | ||
| this.LogWarning("Messenger subscriptions disabled. add \"enableMessengerSubscriptions\": true to config for {key} to enable.", Key); |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The log message formatting should have a space after the closing brace and before "to enable". The current format reads: "...for {key} to enable." but should be "...for {key} to enable."
Consider updating to:
this.LogWarning("Messenger subscriptions disabled. Add \"enableMessengerSubscriptions\": true to config for {key} to enable.", Key);Also note the capitalization of "Add" for consistency.
| this.LogWarning("Messenger subscriptions disabled. add \"enableMessengerSubscriptions\": true to config for {key} to enable.", Key); | |
| this.LogWarning("Messenger subscriptions disabled. Add \"enableMessengerSubscriptions\": true to config for {key} to enable.", Key); |
| /// <inheritdoc /> | ||
| public void ExecuteSwitch(object inputSelector) | ||
| { | ||
| throw new System.NotImplementedException(); |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ExecuteSwitch method throws NotImplementedException, which will cause runtime errors when code attempts to switch inputs on a GenericSink device. Since the class now implements IRoutingSinkWithSwitchingWithInputPort, it should provide a proper implementation or at minimum log the attempt.
Consider implementing the method to handle input switching appropriately, or if input switching is intentionally not supported, provide a no-op implementation with appropriate logging:
public void ExecuteSwitch(object inputSelector)
{
this.LogWarning("Input switching is not supported on GenericSink devices");
}| throw new System.NotImplementedException(); | |
| this.LogWarning("Input switching is not supported on GenericSink devices"); |
This pull request introduces several improvements and bug fixes across the codebase, focusing on device registration logic, configuration parsing, interface implementation, and logging enhancements. The main themes are improved device registration for COM ports, more accurate parsing of UUIDs from URLs, better interface alignment for generic sinks, and refined logging for messenger subscription and status message handling.
Device Registration and Configuration:
RegisterAndConfigureComPortinComPortController.csto only register COM ports if the parent is aCrestronControlSystemorCenIoCom102, improving device compatibility and reliability. Also, removed obsolete debug logging related to property changes and extended information events. [1] [2]Configuration Parsing:
EssentialsConfig.csforSystemUuidandTemplateUuidto more accurately extract UUIDs from system and template URLs, handling new URL formats. [1] [2]Device Interface Implementation:
GenericSinkto implementIRoutingSinkWithSwitchingWithInputPortinstead ofIRoutingSinkWithInputPort, added support for secondary audio signals, and implemented required members for input switching, ensuring correct routing behavior. [1] [2] [3]Logging Improvements:
MessengerBase.csby downgrading unnecessary warnings, improving debug information for client unsubscribes, and logging detailed error messages and stack traces for status message posting exceptions. [1] [2] [3] [4] [5]Configuration Feedback:
MobileControlSystemController.csto alert when messenger subscriptions are disabled, guiding users to enable the feature via configuration.