Logger mods#136
Conversation
Removed _logger.LogAntResponse(LogLevel.Trace, e) calls from MessageHandler methods in AntCollection.cs and AntDeviceCollection.cs to reduce unnecessary trace-level logging of ANT responses.
Refactored all device Parse methods to wrap logic in a logging scope using device number for better traceability. Replaced explicit object initializations with target-typed new expressions. Updated logger extension methods to remove unused method name parameters. Cleaned up and clarified code structure in several device profile classes. No functional changes to parsing logic.
Removed obsolete commented-out Serilog initialization from MainWindowViewModel constructor for clarity. Updated the Serilog output template to use {Scope} instead of {Scope:lj} in the logger configuration.
Bump AntPlus to 6.2.1, AntUsbStick to 4.1.4, and HostingExtensions to 2.0.7. Update NuGet dependencies across projects, improve and clean up logging, and add new version history documentation and release notes.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates NuGet dependencies, simplifies object instantiation using target-typed new() expressions, and introduces structured logging scopes using DeviceNumber across various device profile Parse methods. Feedback on these changes suggests declaring lock objects as readonly to prevent synchronization issues, conditionally creating log scopes to reduce GC allocation overhead when logging is disabled, and adding argument validation to public logging extension methods. Additionally, it is recommended to move the IHost bootstrapping logic out of the MainWindowViewModel constructor to the application entry point.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| using (_logger.BeginScope("DeviceNumber={DeviceNumber}", ChannelId.DeviceNumber)) | ||
| { | ||
| case DataPage.AssetLocation1: | ||
| Asset asset = GetAsset(dataPage); | ||
| asset.ParseLocation1(dataPage); | ||
| if (asset.Status.HasFlag(Asset.AssetStatus.RemoveAsset)) | ||
| { | ||
| lock (CollectionLock) | ||
| base.Parse(dataPage); | ||
| switch ((DataPage)dataPage[0]) | ||
| { |
There was a problem hiding this comment.
Creating a log scope (BeginScope) on every single message parsed (which happens multiple times per second per device) introduces significant allocation overhead and GC pressure, especially when logging is disabled (which is the default in production). Since a log scope only has an effect if a log message is actually written during its lifetime, you should conditionally create the scope only when logging is actually enabled. This applies to all other Parse overrides in the device profiles as well.
IDisposable? scope = _logger.IsEnabled(LogLevel.Trace)
? _logger.BeginScope("DeviceNumber={DeviceNumber}", ChannelId.DeviceNumber)
: null;
using (scope)
{
base.Parse(dataPage);
switch ((DataPage)dataPage[0])
{
|
Abandon this pull request. |
1 participant
Improved logging and dependency updates.