Codec messenger serialization issues#1240
Merged
Merged
Conversation
added 2 commits
April 2, 2025 10:00
The `RoutingPortCollection` type appears to not be currently serializable. If a class that contains this collection is going to be serialized, the collection should have the `JsonIgnore` attribute added. If the list is needed, use a conversion object and convert it to a regular list.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR addresses codec messenger serialization issues by incorporating plugin modifications, ignoring the CameraBase routing port list, and improving error handling and logging across various messenger modules. Key changes include:
- Registering messengers post-initialization with detailed logging.
- Refactoring handler lookup for improved message matching.
- Adding try/catch blocks with enhanced error logging and updating endpoint naming conventions.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/PepperDash.Essentials.MobileControl/MobileControlSystemController.cs | Added logging after initialization and refined message handler retrieval. |
| src/PepperDash.Essentials.MobileControl.Messengers/Messengers/VideoCodecBaseMessenger.cs | Wrapped several methods in try/catch blocks and improved error logging. |
| src/PepperDash.Essentials.MobileControl.Messengers/Messengers/MessengerBase.cs | Updated error logging implementation with consistent token initialization. |
| src/PepperDash.Essentials.MobileControl.Messengers/Messengers/ISelectableItemsMessenger.cs | Introduced try/catch in status messaging and defined a generic state message class. |
| src/PepperDash.Essentials.MobileControl.Messengers/Messengers/DevicePresetsModelMessenger.cs | Renamed action endpoints to remove the "/presets" prefix. |
| src/PepperDash.Essentials.Devices.Common/Cameras/CameraBase.cs | Added JsonIgnore to the OutputPorts property. |
| src/PepperDash.Essentials.Core/Routing/* | Applied minor changes to using directives and added JsonIgnore to ParentDevice references. |
Comments suppressed due to low confidence (3)
src/PepperDash.Essentials.MobileControl.Messengers/Messengers/DevicePresetsModelMessenger.cs:46
- Changing the endpoint from "/presets/fullStatus" to "/fullStatus" may break existing integrations. Please confirm that this change is intentional and update related documentation or client configurations accordingly.
AddAction("/fullStatus", (id, content) =>
src/PepperDash.Essentials.MobileControl.Messengers/Messengers/ISelectableItemsMessenger.cs:76
- [nitpick] Consider passing the exception object directly to the log method for improved diagnostics, for example: this.LogError(e, "Error sending full status");
this.LogError("Error sending full status: {0}", e.Message);
src/PepperDash.Essentials.MobileControl.Messengers/Messengers/VideoCodecBaseMessenger.cs:779
- [nitpick] Returning null in the GetStatus method could lead to NullReferenceExceptions in downstream consumers. Consider returning a default state object instead of null.
return null;
aknous
approved these changes
Apr 2, 2025
ndorin
approved these changes
Apr 2, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CameraBaserouting port list