-
Notifications
You must be signed in to change notification settings - Fork 84
Multiple fixes #1373
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 #1373
Conversation
Co-authored-by: erikdred <88980320+erikdred@users.noreply.github.com>
… from latched mode Co-authored-by: erikdred <88980320+erikdred@users.noreply.github.com>
Co-authored-by: erikdred <88980320+erikdred@users.noreply.github.com>
…onlal logic on raise/lower commands
- refactor volume interfaces into separate files - IBasicVolumeControl implements IKeyName
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 addresses multiple issues across the codebase, with the primary focus on fixing threading issues for concurrent WebSocket client connections in the Mobile Control system. Key improvements include implementing thread-safe client registration, refactoring the ScreenLift controller to support timed movements with command banking, and restructuring volume control interfaces for better maintainability.
- Implements thread-safe WebSocket client registration using concurrent collections and query parameter-based client identification
- Adds movement timing and command banking support to ScreenLiftController with .NET Timer replacement
- Refactors volume control interfaces from a single monolithic file into separate, focused interface definitions
- Adds configuration option to control Fusion room name synchronization and improves custom property mapping
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/PepperDash.Essentials/ControlSystem.cs | Adds debug logging when multiple configuration files are detected |
| src/PepperDash.Essentials.MobileControl/WebSocketServer/UiClient.cs | Implements new client registration flow with clientId query parameter validation and legacy fallback support |
| src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs | Adds concurrent dictionaries for thread-safe client registration and pending client tracking |
| src/PepperDash.Essentials.MobileControl/WebSocketServer/JoinResponse.cs | Adds WebSocketUrl property to join response for client connection |
| src/PepperDash.Essentials.MobileControl/Utilities.cs | Makes GetNextClientId thread-safe using Interlocked.Increment |
| src/PepperDash.Essentials.MobileControl/MobileControlSystemController.cs | Updates client filtering to use TokenKey instead of Token string comparison |
| src/PepperDash.Essentials.MobileControl.Messengers/Messengers/IMatrixRoutingMessenger.cs | Subscribes to input slot online status changes for routing updates |
| src/PepperDash.Essentials.MobileControl.Messengers/Messengers/DeviceVolumeMessenger.cs | Removes unnecessary IKeyName cast since IBasicVolumeControls now extends IKeyName |
| src/PepperDash.Essentials.Devices.Common/Displays/ScreenLiftRelaysConfig.cs | Adds MoveTimeInMs configuration property for movement timing |
| src/PepperDash.Essentials.Devices.Common/Displays/ScreenLiftController.cs | Replaces CTimer with System.Timers.Timer, implements movement tracking and command banking |
| src/PepperDash.Essentials.Devices.Common/DSP/DspBase.cs | Implements IKeyName interface for DspControlPoint base class |
| src/PepperDash.Essentials.Core/Routing/RoutingFeedbackManager.cs | Updates log levels and applies code formatting improvements |
| src/PepperDash.Essentials.Core/Fusion/IEssentialsRoomFusionControllerPropertiesConfig.cs | Adds UseFusionRoomName configuration flag to control room name synchronization |
| src/PepperDash.Essentials.Core/Fusion/IEssentialsRoomFusionController.cs | Passes room object instead of room key to custom properties bridge |
| src/PepperDash.Essentials.Core/Fusion/FusionCustomPropertiesBridge.cs | Refactors to apply custom properties only to the associated room, preventing cross-room contamination |
| src/PepperDash.Essentials.Core/Devices/IVolumeAndAudioInterfaces.cs | Deletes monolithic interface file as part of interface refactoring |
| src/PepperDash.Essentials.Core/DeviceTypeInterfaces/*.cs | Splits volume and audio interfaces into separate, focused files for better organization |
| .config/dotnet-tools.json | Adds CSharpier code formatter tool configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This pull request makes significant improvements to the organization and extensibility of the audio and volume control interfaces in the
PepperDash.Essentials.Coreproject. It refactors and modularizes the various audio-related interfaces into individual files, introduces new interfaces for advanced audio control, and enhances the Fusion room configuration process with improved custom property handling and configurability.Audio/Volume Interface Refactoring and Enhancements:
IVolumeAndAudioInterfaces.csinto individual, focused files underDeviceTypeInterfaces, improving code organization and maintainability. These interfaces includeIBasicVolumeControls,IBasicVolumeWithFeedback,IBasicVolumeWithFeedbackAdvanced,IFullAudioSettings,IHasVolumeControl,IHasMuteControl,IHasVolumeDevice,IHasCurrentVolumeControls,IAudioZone,IAudioZones, and theeVolumeLevelUnitsenum. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]IVolumeAndAudioInterfaces.csfile has been removed, with all relevant code now distributed across the new interface files.Fusion Room Custom Property Handling:
FusionCustomPropertiesBridge.EvaluateRoomInfomethod has been refactored to accept theIEssentialsRoomobject, theRoomInformation, and a newuseFusionRoomNameboolean parameter, allowing the option to skip updating the room name from Fusion if desired. This prevents unintended changes when multiple Fusion instances are present. [1] [2]Fusion Room Configuration Improvements:
useFusionRoomNameproperty (defaulting totrue) toIEssentialsRoomFusionControllerPropertiesConfig, giving integrators control over whether the Fusion room name should overwrite the configured room name.EvaluateRoomInfosignature, passing in the room object and theuseFusionRoomNameconfiguration.General Codebase Clean-up:
Tooling:
.config/dotnet-tools.jsonfile specifying the use of thecsharpiercode formatter, supporting code style consistency.