4 series updates#11
Conversation
- still need to add in console commands
- Updated namespace from Essentials.Plugin.CustomValues to UtilitiesCustomValues across all relevant files. - Simplified the CustomValuesJoinMap class by removing unnecessary comments and retaining essential functionality. - Refined the CustomValuesDevice class for better readability and maintainability, including renaming variables for clarity and removing commented-out code. - Enhanced the CustomValuesTemplateFactory to conditionally set the MinimumEssentialsFrameworkVersion based on the series. - Ensured consistent formatting and structure throughout the codebase.
There was a problem hiding this comment.
Pull Request Overview
This PR adds 4 Series support to the epi-utilities-customvalues project by creating a new 4 Series project configuration and restructuring the codebase. The changes modernize the project structure to support both 3 Series and 4 Series Crestron platforms.
- Adds complete 4 Series project structure with modern MSBuild configuration
- Migrates source files to new location and updates namespace structure
- Implements improved CustomValuesController with better error handling and file management
- Updates build configuration and CI/CD workflows for dual-platform support
Reviewed Changes
Copilot reviewed 21 out of 40 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/epi-utilities-customvalues.4Series.csproj | New 4 Series project file with modern MSBuild configuration |
| src/CustomValuesController.cs | New controller implementation with improved file handling and thread safety |
| src/Directory.Build.targets | Build targets for CPLZ file generation |
| src/Directory.Build.props | Shared build properties and package configuration |
| .github/workflows/EssentialsPlugins-builds-caller.yml | Updated CI workflow for dual-platform builds |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <None Remove="../output\epi-utilities-customvalues.4Series.1.0.0-local.cplz" /> |
There was a problem hiding this comment.
There are duplicate entries for removing the same .cplz file. Lines 38, 42, 53, and 57 all remove the same file path. This creates unnecessary redundancy in the project file.
| <Company>PepperDash Technologies</Company> | ||
| <Product>PepperDash Essentials Plugin</Product> | ||
| <Copyright>Copyright © 2025</Copyright> | ||
| <RepositoryUrl>https://github.com/PepperDash/epi-utilities-devicemonitor.git</RepositoryUrl> |
There was a problem hiding this comment.
The repository URL points to 'epi-utilities-devicemonitor' but this project is 'epi-utilities-customvalues'. The URL should be updated to match the correct repository.
| <RepositoryUrl>https://github.com/PepperDash/epi-utilities-devicemonitor.git</RepositoryUrl> | |
| <RepositoryUrl>https://github.com/PepperDash/epi-utilities-customvalues.git</RepositoryUrl> |
| // Clean up stray temp file if it still exists | ||
| if (File.Exists(tempPath)) | ||
| { | ||
| try { File.Delete(tempPath); } catch { } |
There was a problem hiding this comment.
Empty catch blocks should be avoided. Consider logging the exception or using a more specific exception handling approach to understand potential file deletion failures.
| try { File.Delete(tempPath); } catch { } | |
| try { File.Delete(tempPath); } catch (Exception ex) { Debug.Console(0, Debug.ErrorLogLevel.Warning, "Failed to delete temp file '{0}': {1}", tempPath, ex); } |
| trilist.SetStringSigAction(@join, x => | ||
| { | ||
| data.SelectToken(path).Replace(x); | ||
| _saveTimer.Reset(1000); | ||
| }); |
There was a problem hiding this comment.
The Object case for JTokenType is missing the WithLock wrapper that other cases have, creating a potential race condition when updating data concurrently.
| trilist.SetStringSigAction(@join, x => | |
| { | |
| data.SelectToken(path).Replace(x); | |
| _saveTimer.Reset(1000); | |
| }); | |
| trilist.SetStringSigAction(@join, x => WithLock(() => | |
| { | |
| data.SelectToken(path).Replace(x); | |
| _saveTimer.Reset(1000); | |
| })); |
No description provided.