Hex viewer

[efargas]

User description

/describe

---

PR Type

Enhancement, Tests

---

Description

• Major refactoring to facade and service specialization patterns: Refactored SerialPortService, SocatService, and TaskManagerViewModel to use facade pattern with specialized services for improved separation of concerns and maintainability

• New bootloader orchestration framework: Introduced BaseBootloaderService with unified 13-stage bootloader workflow and streaming support, with BootloaderService and EnhancedBootloaderService now inheriting from it

• Enhanced task scheduling system: Consolidated multiple timers into single dynamic scheduler with dual persistence (active and history tasks), explicit task removal, and on-demand processing via TriggerScheduler()

• High-performance memory dump ingestion: Added DumperService using System.IO.Pipelines for zero-allocation async processing with greeting detection and multi-segment dump support

• Improved logging architecture: Replaced synchronous file logging with async-safe AsyncFileLogger using channels, removed protocol logging, and updated FileLogSink to use IOptions dependency injection pattern

• Dynamic UI task management: Enhanced BottomPanelViewModel with dynamic task-specific log tabs and integrated TaskManagerViewModel with new TaskCommandManager and TaskStatisticsViewModel for centralized command handling

• New specialized services: Added SerialPortDiscoveryService, SerialPortConfigurationService, SerialPortMonitoringService, SocatProcessManager, SocatCommandBuilder, SocatPortManager, SocatConfigurationService, and ShellCommandExecutor

• Hex viewer infrastructure: Added PlcTransportAdapter.GetStream() accessor and new hex viewer view models and services for binary data inspection and searching

• Comprehensive DI container updates: Extended service registration with all new specialized services and view models

• Test updates: Disabled BootloaderServiceTests due to signature mismatches; added new tests for serial port and hex viewer functionality

---

Diagram Walkthrough

flowchart LR
  A["Monolithic Services<br/>SerialPortService<br/>SocatService<br/>TaskManagerViewModel"] -->|"Refactor to Facade"| B["Facade Services<br/>with Specialized<br/>Components"]
  C["Bootloader Logic<br/>Scattered"] -->|"Consolidate"| D["BaseBootloaderService<br/>Orchestration"]
  E["Multiple Timers<br/>Fixed Intervals"] -->|"Unify & Dynamize"| F["Single Dynamic<br/>Task Scheduler"]
  G["Sync File Logging<br/>Protocol Logging"] -->|"Modernize"| H["Async File Logger<br/>Channel-based"]
  I["Raw Byte Arrays<br/>from Dumps"] -->|"Add Pipeline"| J["DumperService<br/>High-Performance<br/>Ingestion"]
  B -->|"Enable"| K["Hex Viewer<br/>Infrastructure"]
  D -->|"Return"| L["BootloaderResult<br/>with Metadata"]
  F -->|"Persist"| M["Dual Persistence<br/>Active + History"]

Loading

File Walkthrough

	Relevant files
Enhancement	15 files BaseBootloaderService.cs New BaseBootloaderService with unified bootloader orchestration src/S7Tools/Services/Bootloader/BaseBootloaderService.cs New abstract base class for bootloader services containing shared memory dumping logic Implements 13-stage bootloader orchestration workflow with streaming support Provides three dump process methods: standard, streaming, and simplified streaming approaches Includes progress reporting, payload installation simulation, and comprehensive error handling +1174/-0 SocatService.cs SocatService refactored to facade pattern with specialized services src/S7Tools/Services/SocatService.cs Refactored to use facade pattern with four specialized services (CommandBuilder, ProcessManager, PortManager, ConfigurationService) Simplified public methods by delegating to specialized services Removed protocol logger parameter and hex dump routing logic Improved process lifecycle management with event-driven architecture Streamlined port checking and process discovery logic +189/-691 FileLogSink.cs FileLogSink updated to use IOptions dependency injection pattern src/S7Tools.Infrastructure.Logging/Sinks/FileLogSink.cs Updated constructor to use IOptions pattern instead of direct configuration Added null checks with ArgumentNullException for both options and pathService parameters Improved dependency injection alignment with Microsoft.Extensions.Options conventions +6/-4      SerialPortService.cs Refactor SerialPortService to Facade Pattern with Specialized Services src/S7Tools/Services/SerialPortService.cs Refactored SerialPortService from a monolithic implementation into a facade pattern delegating to specialized services (SerialPortDiscoveryService, SerialPortConfigurationService, SerialPortMonitoringService) Removed 700+ lines of implementation code including port scanning, stty command execution, USB device detection, and monitoring logic Simplified constructor to accept three specialized services instead of ITimeProvider and internal state management Wired up event forwarding from monitoring service to maintain backward compatibility with ISerialPortService interface Removed internal timers, semaphores, and complex monitoring state; delegated to SerialPortMonitoringService +103/-848 TaskManagerViewModel.cs Refactor TaskManagerViewModel with Command Manager and Statistics Separation src/S7Tools/ViewModels/Tasks/TaskManagerViewModel.cs Introduced TaskStatisticsViewModel and TaskCommandManager dependencies for separation of concerns Changed SelectedTask from nullable to non-nullable using TaskExecution.Empty sentinel value Removed inline statistics properties (TotalTasksCount, RunningTasksCount, FailedTasksCount, AverageExecutionTime, ResourceUtilization) and delegated to TaskStatisticsViewModel Implemented throttled task state change handling using Reactive subjects to prevent UI freezing Refactored all task command execution methods to use TaskCommandManager for centralized command handling and result processing Added helper methods UpdateCommandResult() and HandleCommandException() for consistent error handling +135/-348 EnhancedBootloaderService.cs Refactor EnhancedBootloaderService to Use Base Orchestration src/S7Tools/Services/Bootloader/EnhancedBootloaderService.cs Changed DumpAsync() return type from byte[] to BootloaderResult to provide structured result with file paths and metadata Removed 400+ lines of orchestration logic from DumpAsync() and delegated to PerformBootloaderOrchestrationAsync() base method Updated DumpWithTaskTrackingAsync() to work with BootloaderResult instead of raw byte arrays Removed protocolLogger parameter from method signatures Simplified error handling and resource cleanup by delegating to base orchestration +45/-552 EnhancedTaskScheduler.cs Refactor Task Scheduler with Dynamic Timer and Dual Persistence src/S7Tools/Services/Tasking/EnhancedTaskScheduler.cs Consolidated three separate timers (_processingTimer, _cleanupTimer, _persistenceTimer) into single _scheduleTimer with dynamic rescheduling Implemented TriggerScheduler() method for on-demand task processing instead of fixed intervals Added RemoveTaskAsync() and ClearFinishedTasksAsync() methods for explicit task removal Modified task persistence to separate active and finished tasks into two files (Tasks.json and Tasks_History.json) Enhanced task loading to restore both active and history tasks from persistence Added explicit SaveTasksAsync() calls after state changes instead of relying on periodic timer Improved scheduled task promotion logic with dynamic timer rescheduling based on next scheduled time Disabled automatic cleanup in favor of manual-only cleanup as per user request +257/-78 BottomPanelViewModel.cs Add Dynamic Task Log Tabs to Bottom Panel                                src/S7Tools/ViewModels/Layout/BottomPanelViewModel.cs Added TaskManagerViewModel dependency and ICentralizedTaskLogService for task log integration Refactored tab management to dynamically create task-specific log tabs when tasks are added to ActiveTasks Implemented AddNewTaskTab() and RemoveTaskTab() methods to handle task lifecycle in UI Added CloseTabCommand and CloseTab() method for closable tab support Simplified tab initialization to only include log viewer tab by default Changed SelectTabCommand parameter type from PanelTabItem to PanelTabItem? to handle null selections Updated constructor to accept optional TaskManagerViewModel and ICentralizedTaskLogService parameters +118/-33 PlcTransportAdapter.cs Add Stream Accessor to PlcTransportAdapter                              src/S7Tools/Services/Adapters/PlcTransportAdapter.cs Added public GetStream() method to expose the underlying Stream for external access +2/-0      SerialPortConfigurationService.cs New serial port configuration management service                  src/S7Tools/Services/SerialPort/SerialPortConfigurationService.cs New service for serial port configuration management using stty commands Provides methods to read, apply, backup, and restore port configurations Generates stty commands with security validation and dangerous pattern detection Parses stty output to create SerialPortConfiguration objects Uses regex patterns for command validation and output parsing +475/-0  SocatProcessManager.cs New socat process lifecycle management service                      src/S7Tools/Services/Socat/SocatProcessManager.cs New service for socat process lifecycle management (start, stop, monitor) Handles process creation with output/error capture and logging routing Implements graceful shutdown with SIGTERM followed by SIGKILL Manages child process cleanup and process exit event handling Uses semaphore for thread-safe process tracking +542/-0  SerialPortDiscoveryService.cs New serial port discovery and scanning service                      src/S7Tools/Services/SerialPort/SerialPortDiscoveryService.cs New service for serial port discovery and information retrieval Scans for USB, ACM, and standard serial ports with configurable limits Retrieves USB device information from sysfs (vendor ID, product ID, serial number) Tests port accessibility and checks if ports are in use Provides port type classification and descriptions +422/-0  DumperService.cs New high-performance PLC memory dump service                          src/S7Tools/Services/Adapters/Plc/DumperService.cs New high-performance memory dump ingestion service using System.IO.Pipelines Implements zero-allocation reading via Pipe and async processing with Channels Handles greeting detection (both framed and legacy formats) with junk byte skipping Supports multi-segment dumps with session reset and data flushing Manages socket connections to socat TCP bridge with EOF retry logic +469/-0  TaskLoggerFactory.cs Remove protocol logging and implement async file logger    src/S7Tools/Services/Logging/TaskLoggerFactory.cs Removed captureProtocol parameter from CreateTaskLoggerAsync method Removed protocol logger provider and protocol data store creation Replaced synchronous FileLogger with async-safe AsyncFileLogger using Channel Implemented background writer task for non-blocking log writes Updated disposal logic to handle both IAsyncDisposable and IDisposable interfaces Removed protocol-related fields from TaskLoggerContext class +127/-75 TaskCommandManager.cs New task command manager for operation handling                    src/S7Tools/ViewModels/Tasks/TaskCommandManager.cs New command manager class encapsulating task execution logic Provides methods for task lifecycle operations (start, stop, pause, resume, restart, delete) Handles task scheduling with time parsing (minutes offset or specific time) Implements confirmation dialogs and error handling for all operations Returns CommandResult struct with success/failure/cancelled states +406/-0
Tests	1 files BootloaderServiceTests.cs BootloaderServiceTests temporarily disabled for refactoring tests/S7Tools.Tests/Services/Bootloader/BootloaderServiceTests.cs Entire test file commented out due to signature mismatches with BootloaderService Tests for 7-stage workflow, progress reporting, and resource failure handling disabled Placeholder comment indicates tests need updating to match new service signatures +4/-533
Refactoring	1 files BootloaderService.cs Refactor bootloader dump orchestration to base class          src/S7Tools/Services/Bootloader/BootloaderService.cs Changed inheritance from IBootloaderService to BaseBootloaderService(timeProvider), IBootloaderService Modified DumpAsync return type from byte[] to BootloaderResult Removed protocolLogger parameter from method signature Extracted 400+ lines of orchestration logic into base class method PerformBootloaderOrchestrationAsync Removed private fields _timeProvider and constant InitialPowerOffWaitMs Removed helper method WaitWithProgressAsync +15/-442
Configuration changes	1 files ServiceCollectionExtensions.cs Extend DI container with new services and view models        src/S7Tools/Extensions/ServiceCollectionExtensions.cs Added IShellCommandExecutor singleton registration Registered new serial port specialized services (SerialPortDiscoveryService, SerialPortConfigurationService, SerialPortMonitoringService) Registered new socat specialized services (SocatCommandBuilder, SocatProcessManager, SocatPortManager, SocatConfigurationService) Updated SocatService registration to inject new specialized services Added FileLogSink as ILogSink for unified logger provider Registered new memory dump and hex viewer view models Added TaskCommandManager and TaskStatisticsViewModel registrations Updated logging configuration to use absolute paths from PathService +71/-17
Additional files	101 files copilot-instructions.md +2/-6      launch.json +9/-3      README.md +1/-1      graph.json +5892/-0 migration-log.json +7/-0      migration-tracking.json +7/-0      schema.json +1/-1      ATTRIBUTE_BASED_DISPLAY.md +1/-1      DEPRECATED_PROPERTY_MIGRATION.md +1/-1      INDEX.md +11/-1    JOB_WIZARD_SEGMENT_PERSISTENCE.md +4/-8      MEMORY_REGION_PROFILES.md +4/-6      Project_Architecture_Blueprint.md +1/-1      Project_Folders_Structure_Blueprint.md +1/-1      README.md +18/-0    SETTINGS_SCHEMA.md +1/-1      TASK_LOGGING_SYSTEM.md +19/-11  UI_INTEGRATION_WORKFLOW.md +3/-1      _index.md +3/-1      clean-architecture.md +2/-1      0001-ui-framework.md +3/-1      0002-logging-provider.md +2/-1      _index.md +4/-1      _template.md +1/-1      dependency-injection.md +7/-6      diagrams.md +1/-1      mvvm-patterns.md +3/-1      overview.md +10/-1    product-context.md +9/-0      project-brief.md +9/-0      technology-stack.md +10/-0    ATTRIBUTE_BASED_DISPLAY.md +2/-1      Project_Architecture_Blueprint.md +2/-1      Project_Folders_Structure_Blueprint.md +2/-1      _index.md +6/-5      codeproject-logviewer.markitdown.full.md +8/-0      codeproject-logviewer.markitdown.md +9/-0      VALIDATION_QUICK_REFERENCE.md +8/-0      _index.md +23/-0    ai-agent-guide.md +4/-1      archive-management.md +14/-6    code-style.md +5/-1      contributing-to-docs.md +17/-6    development-standards.md +12/-0    development-workflow.md +8/-1      documentation-templates.md +14/-7    frontmatter-schema.md +7/-5      memory-bank-usage.md +3/-1      _index.md +1/-1      breaking-changes.md +1/-1      deprecated-patterns.md +4/-1      onboarding.md +5/-1      testing-guide.md +2/-1      versioning-guide.md +6/-1      _index.md +6/-1      custom-exceptions.md +1/-1      profile-manager-example.md +2/-1      resource-coordinator-example.md +1/-1      semaphore-pattern-example.md +2/-1      internal-method.md +1/-1      profile-management.md +5/-2      resource-coordination.md +1/-1      reusable-controls.md +1/-1      system-patterns.md +11/-1    2025-11-07-comprehensive-review.md +1/-1      2025-11-10-quality-improvements.md +2/-1      _index.md +1/-1      _index.md +6/-1      adr-template.md +2/-1      guide-template.md +4/-13    pattern-template.md +4/-1      service-template.md +4/-1      test-template.md +5/-1      INTEGRATION_CHECKLIST.md +3/-1      README.md +1/-1      viewmodel-template.md +5/-1      dotnet-install.sh +1887/-0 global.json +6/-0      README.md +4/-1      generate-cross-references.py +21/-6    ITaskLoggerFactory.cs +0/-2      BootloaderResult.cs +10/-0    JobProfile.cs +34/-2    JobProfileSet.cs +3/-1      TaskExecution.cs +49/-39  TaskLogger.cs +5/-20    MemoryBlock.cs +134/-0  ProtocolParser.cs +115/-0  S7Tools.Core.csproj +4/-2      IBootloaderService.cs +3/-4      IEnhancedBootloaderService.cs +3/-2      IPlcClient.cs +37/-9    IPlcProtocol.cs +13/-0    IPlcTransport.cs +6/-0      ISocatService.cs +2/-4      ITaskScheduler.cs +16/-0    IShellCommandExecutor.cs +41/-0    S7Tools.Diagnostics.csproj +2/-2      S7Tools.Infrastructure.Logging.csproj +3/-2      App.axaml.cs +80/-191 Additional files not shown

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Arbitrary file write Description: The new streaming dump workflow writes dump output to a directory taken from profiles.OutputPath (or ./dumps) and then writes files via File.WriteAllBytesAsync, which can enable arbitrary file write/overwrite (including to sensitive locations) if profiles.OutputPath is user-controlled or can be influenced externally. BaseBootloaderService.cs [383-394] Referred Code string dumpsDir = !string.IsNullOrWhiteSpace(profiles.OutputPath) ? profiles.OutputPath : "./dumps"; if (!System.IO.Directory.Exists(dumpsDir)) { System.IO.Directory.CreateDirectory(dumpsDir); } string dumpFilePath = System.IO.Path.Combine(dumpsDir, dumpFileName); await System.IO.File.WriteAllBytesAsync(dumpFilePath, dumpData, cancellationToken).ConfigureAwait(false); savedFiles.Add(dumpFilePath);
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Missing input validation: SimulateProgressAsync divides by baudRate without validating it is > 0, which can cause divide-by-zero/invalid timing behavior and an unhandled exception. Referred Code protected async Task SimulateProgressAsync( long payloadSize, int baudRate, IProgress<(string stage, double percent, long? bytesRead, long? totalBytes)> progress, double startPercent, double targetPercent, string stage, bool isStagerInstall, CancellationToken cancellationToken) { // UART: 10 bits per byte (8 data + 1 start + 1 stop) const int bitsPerByte = 10; long totalWireBytes; int totalDelayMs; if (isStagerInstall) { // Stager uses WriteToIramAsync with 16-byte chunks // Each chunk requires 2 packets (mask + write) // Protocol overhead: length(1) + payload(N) + checksum(1) = N+2 bytes per packet ... (clipped 52 lines) Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing user context: New bootloader orchestration logs critical actions but does not include any user identifier, so it is unclear if audit trail requirements are satisfied in the broader app context. Referred Code // Stage 0: Configure serial port (1% progress) progress.Report(("serial_config", 1.0, null, null)); effectiveTaskLogger.LogDebug("Configuring serial port {Device} with profile configuration", profiles.Serial.Device); bool serialConfigured = await serialPort.ApplyConfigurationAsync( profiles.Serial.Device, profiles.Serial.Configuration, effectiveTaskLogger, cancellationToken).ConfigureAwait(false); if (!serialConfigured) { throw new InvalidOperationException($"Failed to configure serial port {profiles.Serial.Device}"); } effectiveTaskLogger.LogInformation("✓ Serial port {Device} configured successfully", profiles.Serial.Device); // Stage 1: Setup socat bridge (3% progress) progress.Report(("socat_setup", 3.0, null, null)); effectiveTaskLogger.LogDebug("Setting up socat bridge on port {Port}", profiles.Socat.Port); ... (clipped 196 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: UI error exposure: The bootloader orchestration logs ex.Message via effectiveTaskLogger.LogError, but it is unclear whether this logger is user-visible, which could expose internal error details to end-users. Referred Code catch (Exception ex) { // Log full exception in process logger if available processLogger?.LogError(ex, "Dump failed: {ErrorMessage}", ex.Message); effectiveTaskLogger.LogError("DUMP OPERATION FAILED: {ErrorMessage}", ex.Message); throw; Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Potential sensitive paths: New logs include device paths and output file paths, and without seeing data classification rules it is unclear whether these values are considered sensitive and need redaction/structuring guarantees. Referred Code effectiveTaskLogger.LogDebug("Configuring serial port {Device} with profile configuration", profiles.Serial.Device); bool serialConfigured = await serialPort.ApplyConfigurationAsync( profiles.Serial.Device, profiles.Serial.Configuration, effectiveTaskLogger, cancellationToken).ConfigureAwait(false); if (!serialConfigured) { throw new InvalidOperationException($"Failed to configure serial port {profiles.Serial.Device}"); } effectiveTaskLogger.LogInformation("✓ Serial port {Device} configured successfully", profiles.Serial.Device); // Stage 1: Setup socat bridge (3% progress) progress.Report(("socat_setup", 3.0, null, null)); effectiveTaskLogger.LogDebug("Setting up socat bridge on port {Port}", profiles.Socat.Port); if (profiles.Socat.Configuration == null) { ... (clipped 33 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Unvalidated output path: The dump file destination uses profiles.OutputPath to create directories and write files without validating/normalizing the path, so it is unclear if path safety constraints are enforced elsewhere. Referred Code string dumpsDir = !string.IsNullOrWhiteSpace(profiles.OutputPath) ? profiles.OutputPath : "./dumps"; if (!System.IO.Directory.Exists(dumpsDir)) { System.IO.Directory.CreateDirectory(dumpsDir); } string dumpFilePath = System.IO.Path.Combine(dumpsDir, dumpFileName); await System.IO.File.WriteAllBytesAsync(dumpFilePath, dumpData, cancellationToken).ConfigureAwait(false); savedFiles.Add(dumpFilePath); Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Sanitize the entire generated filename Sanitize the entire dumpFileName after construction, as the timestamp component may contain invalid characters like colons, which could cause file write errors. src/S7Tools/Services/Bootloader/BaseBootloaderService.cs [376-382] // Sanitize job name jobName = string.Join("_", jobName.Split(Path.GetInvalidFileNameChars())); // Format: {SegmentName}_iter{i+1}_of_{Count}_{TaskId}_{Timestamp}.bin // Use TaskId if available, otherwise omit it (or use empty) -- but user asked for TaskId. string taskIdStr = taskId.HasValue ? $"_{taskId.Value:N}" : ""; -string dumpFileName = $"{jobName}_iter{iter + 1}_of_{iterationCount}{taskIdStr}_{timestamp}.bin"; +string unsafeDumpFileName = $"{jobName}_iter{iter + 1}_of_{iterationCount}{taskIdStr}_{timestamp}.bin"; +string dumpFileName = string.Join("_", unsafeDumpFileName.Split(Path.GetInvalidFileNameChars())); [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a potential IOException because the timestamp format includes colons, which are invalid in Windows filenames. This is a critical bug that would prevent file creation on Windows systems.	Medium
	Fix race condition in progress reporting Add a lock around the access to lastLoggedPercent within the Progress delegate to prevent a race condition in log throttling. src/S7Tools/Services/Bootloader/EnhancedBootloaderService.cs [76-125] public async Task<BootloaderResult> DumpWithTaskTrackingAsync( TaskExecution taskExecution, JobProfileSet profiles, CancellationToken cancellationToken = default) { double lastLoggedPercent = -1.0; + object progressLock = new(); var progressReporter = new Progress<(string stage, double percent, long? bytesRead, long? totalBytes)>( (progressData) => { var (operation, percent, bytesRead, totalBytes) = progressData; var extraData = new Dictionary<string, object> { { "BytesRead", bytesRead ?? 0 }, { "TotalBytes", totalBytes ?? 0 } }; taskExecution.UpdateProgress(percent, operation, extraData); // Throttle logging to avoid spam (log every 0.1% change or if bytes are involved/important stages) - if (Math.Abs(percent - lastLoggedPercent) >= 0.1 || percent >= 100.0 || percent <= 0.0) + lock (progressLock) { - lastLoggedPercent = percent; - if (bytesRead.HasValue && totalBytes.HasValue) + if (Math.Abs(percent - lastLoggedPercent) >= 0.1 || percent >= 100.0 || percent <= 0.0) { - _logger.LogTrace("Task {TaskId} progress: {Percent:F1}% ({Operation}) - {BytesRead}/{TotalBytes} bytes", - taskExecution.TaskId, percent, operation, bytesRead, totalBytes); - } - else - { - _logger.LogTrace("Task {TaskId} progress: {Percent:F1}% ({Operation})", - taskExecution.TaskId, percent, operation); + lastLoggedPercent = percent; + if (bytesRead.HasValue && totalBytes.HasValue) + { + _logger.LogTrace("Task {TaskId} progress: {Percent:F1}% ({Operation}) - {BytesRead}/{TotalBytes} bytes", + taskExecution.TaskId, percent, operation, bytesRead, totalBytes); + } + else + { + _logger.LogTrace("Task {TaskId} progress: {Percent:F1}% ({Operation})", + taskExecution.TaskId, percent, operation); + } } } }); ... [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a potential race condition on the lastLoggedPercent variable and provides a valid fix using a lock, which is critical for correctness in a concurrent environment.	Medium
	Fix incorrect awaiting of async method Fix an incorrect use of Task.Run which causes a race condition. Directly await SaveTasksAsync() to ensure the operation completes before the method returns. src/S7Tools/Services/Tasking/EnhancedTaskScheduler.cs [733-738] if (_tasks.TryRemove(taskId, out _)) { _logger.LogInformation("Removed task {TaskId} ({JobName})", taskId, task.JobName); - await Task.Run(() => SaveTasksAsync(), CancellationToken.None); + await SaveTasksAsync().ConfigureAwait(false); // Persist removal return true; } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a bug where SaveTasksAsync is not properly awaited, which could lead to data loss or inconsistent state.	Medium
	Dispose all disposable services properly Dispose _discoveryService and _configService in the Dispose method if they implement IDisposable to prevent potential resource leaks. src/S7Tools/Services/SerialPortService.cs [216-228] public void Dispose() { if (_monitoringService != null) { _monitoringService.PortAdded -= OnPortAdded; _monitoringService.PortRemoved -= OnPortRemoved; _monitoringService.PortStatusChanged -= OnPortStatusChanged; } + // Assuming the other services are also IDisposable, they should be disposed. + (_discoveryService as IDisposable)?.Dispose(); + (_configService as IDisposable)?.Dispose(); _monitoringService?.Dispose(); GC.SuppressFinalize(this); } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential resource leak by pointing out that only one of the three injected services is being disposed, which is an important correctness issue.	Medium
	Pass logger to base constructor Pass the logger instance to the BaseBootloaderService constructor instead of null to enable logging in the base class. src/S7Tools/Services/Bootloader/EnhancedBootloaderService.cs [26] -public sealed class EnhancedBootloaderService : BaseBootloaderService(null), IEnhancedBootloaderService, IDisposable +public sealed class EnhancedBootloaderService : BaseBootloaderService(logger), IEnhancedBootloaderService, IDisposable [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 7 __ Why: This is a valid and important suggestion to enable logging in the base class logic, which is crucial for debugging and monitoring the new orchestration flow.	Medium
General	Unsubscribe scheduler events in Dispose Unsubscribe from _taskScheduler.TaskStateChanged and _taskScheduler.TaskProgressUpdated in the Dispose method to prevent memory leaks. src/S7Tools/ViewModels/Tasks/TaskManagerViewModel.cs [1004-1007] public void Dispose() { + _taskScheduler.TaskStateChanged -= OnTaskStateChanged; + _taskScheduler.TaskProgressUpdated -= OnTaskProgressUpdated; _disposables.Dispose(); } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a memory leak due to not unsubscribing from events and provides the correct fix, which is critical for application stability.	Medium
	Log socat stderr at Information level In StartSocatProcessAsync, change the logging level for socat's stderr output from LogDebug to LogInformation to ensure important operational messages and errors are not missed. src/S7Tools/Services/SocatService.cs [790-804] private async Task<SocatProcessInfo> StartSocatProcessAsync( string command, SocatConfiguration configuration, string serialDevice, SocatProfile? profile, Microsoft.Extensions.Logging.ILogger? processLogger, CancellationToken cancellationToken) { // ... if (processLogger != null) { process.ErrorDataReceived += (sender, e) => { if (!string.IsNullOrEmpty(e.Data)) { errorBuilder!.AppendLine(e.Data); // ... string cleanMessage = SocatLogTimestampRegex().Replace(e.Data, string.Empty); // Only log to the task-specific logger, NOT the main application logger - processLogger?.LogDebug("socat[{ProcessId}] {Message}", processId, cleanMessage); + // Socat often prints important status/error messages to stderr. + processLogger?.LogInformation("socat[{ProcessId}] {Message}", processId, cleanMessage); } }; } // ... } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out that socat's stderr may contain important information that would be missed if logged at the Debug level. Raising the log level to Information improves observability and debugging.	Low
	Improve stream reading for better performance Improve stream reading performance by using stream.ReadAtLeastAsync instead of stream.ReadAsync. This will fill the pipe buffer more efficiently by ensuring a minimum number of bytes are read. src/S7Tools/Services/Adapters/Plc/DumperService.cs [157-183] private async Task FillPipeAsync(Stream stream, PipeWriter writer, CancellationToken token) { const int MinBufferSize = 4096; while (!token.IsCancellationRequested) { Memory<byte> memory = writer.GetMemory(MinBufferSize); try { - int bytesRead = await stream.ReadAsync(memory, token); + // Use ReadAtLeastAsync to ensure we get a meaningful chunk of data + int bytesRead = await stream.ReadAtLeastAsync(memory, 1, throwOnEndOfStream: false, token); if (bytesRead == 0) { // EOF received - but don't break immediately, PLC might send more data // This is critical for multi-iteration dumps _consecutiveEofCount++; if (_consecutiveEofCount >= MaxEofRetries) { Logger.LogWarning("Max EOF retries reached ({Count}), stopping reader", MaxEofRetries); break; } Logger.LogDebug("EOF detected ({Count}/{Max}), waiting for more data...", _consecutiveEofCount, MaxEofRetries); await Task.Delay(50, token); continue; } ... [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: The suggestion offers a valid performance improvement by replacing ReadAsync with the more efficient ReadAtLeastAsync for this pipeline-based implementation.	Low
Update

[gemini-code-assist]

Summary of Changes

Hello @efargas, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request delivers significant architectural enhancements and new features, primarily focusing on advanced memory analysis and improved system stability. The introduction of a real-time hex viewer and high-performance streaming for PLC memory dumps provides powerful new debugging and analysis capabilities. Concurrently, a substantial refactoring effort has modularized core services, making the codebase more robust and easier to extend. These changes, coupled with a .NET 9.0 upgrade and comprehensive documentation improvements, lay a stronger foundation for future development and enhance the overall developer and user experience.

Highlights

• Hex Viewer Integration: Introduced a new interactive Hex Viewer UI component, enabling real-time visualization of PLC memory dumps and static binary files. This includes features like data inspection (various formats, endianness), binary search, and configurable display options.
• Enhanced PLC Memory Dumping with Streaming: Implemented high-performance streaming capabilities for PLC memory dumps, significantly reducing memory footprint during large dumps by writing directly to temporary files. The dumping process now supports multi-iteration dumps and improved progress reporting, with a new BootloaderResult record to encapsulate dumped data and saved file paths.
• Core Service Refactoring and Modularity: Undertook a major refactoring of core services like SerialPortService and SocatService. These monolithic services have been decomposed into smaller, more focused, and testable components (e.g., SerialPortDiscoveryService, SocatProcessManager, SocatCommandBuilder), adhering to the Single Responsibility Principle and improving maintainability.
• .NET 9.0 Upgrade and Dependency Updates: Upgraded the project's target framework from .NET 8.0 to .NET 9.0, incorporating new features and performance improvements from the latest framework. Key UI and utility libraries, including Avalonia and AvaloniaHex, have also been updated to their latest stable versions.
• Improved Task Management and Logging: Enhanced the task scheduling system with new functionalities for removing specific tasks and clearing finished tasks. The logging infrastructure for tasks has been revamped, removing protocol-specific loggers and introducing AsyncFileLogger for non-blocking file writes, along with a new TaskLogsPanelViewModel for better log visualization.
• Comprehensive Documentation Updates: Performed extensive updates across the documentation, including adding YAML frontmatter to key documents, introducing a new duplicates.csv metadata file, and updating numerous 'Related Documentation' links to ensure accuracy and improve navigation within the project's knowledge base.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This is an impressive and extensive pull request that introduces major refactorings and a new Hex Viewer feature. The architectural improvements are significant, including:

• Refactoring SerialPortService and SocatService into facades with specialized services, which greatly improves separation of concerns.
• Introducing a BaseBootloaderService to centralize the bootloader orchestration workflow, effectively reducing code duplication.
• Modernizing the task scheduler with a single dynamic timer and improved persistence.
• Implementing a high-performance, zero-allocation memory dump ingestion service (DumperService) using System.IO.Pipelines.
• Replacing synchronous file logging with an async-safe logger using System.Threading.Channels.

The overall code quality of the new and refactored components is very high.

However, there are a few critical issues that need to be addressed:

1. Disabled Tests: The entire test suite for BootloaderService has been commented out. A refactoring of this scale must be accompanied by updated tests to ensure correctness and prevent regressions. Please update the tests to match the new service implementation.
2. Invalid JSON Files: Two metadata files (migration-log.json and migration-tracking.json) have been modified in a way that makes them invalid JSON by appending markdown content. This will break any tools that rely on parsing these files.

[gemini-code-assist]

The file migration-log.json appears to be a JSON file, but this change appends markdown content to it, which makes it invalid JSON. If this file is intended to be parsed by any tools, this will cause a parsing error. Please ensure the file remains valid JSON.

[gemini-code-assist]

Similar to migration-log.json, this change appends markdown content to migration-tracking.json, which invalidates the JSON format. This will likely break any tool that tries to parse this file. Please correct the file to be valid JSON.