fallback support

This commit is contained in:
EugeneTes
2026-02-05 09:31:38 +01:00
parent 08da2c1e3e
commit f0ae32c9e5
8 changed files with 165 additions and 721 deletions

View File

@@ -54,6 +54,8 @@ kernel.Bind<EpsonPrintServiceConfiguration>().ToConstant(config);
kernel.Bind<IPrinterFactory>().To<EpsonPrinterFactory>();
kernel.Bind<IPrintJobSource, SignalRPrintJobSource>().To<SignalRPrintJobSource>().InSingletonScope();
//kernel.Bind<IPrintJobSource>().To<HttpPollingPrintJobSource>().InSingletonScope();
kernel.Bind<IPrintService>().To<Inspectron.Epson.PrintServer.PrintServices.EpsonPrintService>();
kernel.Bind<ILogger>().ToMethod(ctx =>
{
@@ -74,6 +76,7 @@ kernel.Bind<IDiscoveredPrintersReceiver>().To<JamesDiscoveredPrintersReceiver>()
kernel.Bind<PrinterDiscoveryBackgroundTask>().ToSelf().InSingletonScope();
kernel.Bind<HeartbeatBackgroundTask>().ToSelf().InSingletonScope();
var printLoop = kernel.Get<PrintLoop>();
var printServer = kernel.Get<PrintServer>();
@@ -81,7 +84,7 @@ var printServer = kernel.Get<PrintServer>();
var discoveryTask = kernel.Get<PrinterDiscoveryBackgroundTask>();
var heartbeatTask = kernel.Get<HeartbeatBackgroundTask>();
var jobSourceTask = kernel.Get<IPrintJobSource>();
//printServer.RegisterPrinter("10.0.20.12");
@@ -100,10 +103,13 @@ Console.CancelKeyPress += (sender, e) =>
_ = discoveryTask.StartAsync(cts.Token);
_ = heartbeatTask.StartAsync(cts.Token);
// delay for a moment to allow discovery to find printers
Console.WriteLine("Waiting for printer discovery...");
await Task.Delay(3000);
_ = jobSourceTask.StartAsync();
await printLoop.StartAsync();
try

View File

@@ -12,7 +12,7 @@ namespace Inspectron.Epson;
/// </summary>
public class EpsonPrinter : IEpsonPrinter
{
private readonly ILogger? _logger;
private readonly ILogger? _logger=null;
private TcpClient? _tcpClient;
private NetworkStream? _stream;
private string? _printerIp;
@@ -29,9 +29,9 @@ public class EpsonPrinter : IEpsonPrinter
/// Initialize a new instance of EpsonPrinter
/// </summary>
/// <param name="logger">Optional logger for diagnostic output</param>
public EpsonPrinter(ILogger? logger = null)
public EpsonPrinter(/*ILogger? logger = null*/)
{
_logger = logger;
//_logger = logger;
RegisterCodepages();
}

View File

@@ -4,5 +4,6 @@ namespace Inspectron.Epson.PrintServer;
public interface IPrintJobSource
{
Task StartAsync();
Task<PrintJob> GetNextJobAsync(CancellationToken cancellationToken);
}

View File

@@ -0,0 +1,143 @@
using System.Net.Http.Json;
using System.Text.Json;
using System.Text.Json.Serialization;
using System.Threading.Channels;
using Inspectron.Epson.PrintServer.ConfigurationSources;
using Inspectron.Epson.Queue;
using Microsoft.Extensions.Logging;
namespace Inspectron.Epson.PrintServer.JobSources;
public class HttpPollingPrintJobSource : IPrintJobSource, IDisposable
{
private readonly EpsonPrintServiceConfiguration _groupConfiguration;
private readonly ILogger _logger;
private readonly HttpClient _httpClient;
private readonly Channel<PrintJob> _printJobChannel = Channel.CreateUnbounded<PrintJob>();
private readonly CancellationTokenSource _pollingCts = new();
private readonly TimeSpan _pollingInterval;
public HttpPollingPrintJobSource(
EpsonPrintServiceConfiguration groupConfiguration,
ILogger logger
)
{
_groupConfiguration = groupConfiguration;
_logger = logger;
_pollingInterval = TimeSpan.FromSeconds(3);
_httpClient = new HttpClient
{
BaseAddress = new Uri(groupConfiguration.ApiUrl?.TrimEnd('/') + "/")
};
_httpClient.DefaultRequestHeaders.Add("X-Printer-Server-Key", groupConfiguration.ApiKey);
}
private async Task StartPollingAsync(CancellationToken cancellationToken)
{
_logger.LogInformation("Starting HTTP polling for print jobs at {BaseUrl}", _httpClient.BaseAddress);
while (!cancellationToken.IsCancellationRequested)
{
try
{
await PollForJobsAsync(cancellationToken);
}
catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
{
break;
}
catch (Exception ex)
{
_logger.LogError(ex, "Error polling for print jobs. Retrying in {Interval}...", _pollingInterval);
}
try
{
await Task.Delay(_pollingInterval, cancellationToken);
}
catch (OperationCanceledException)
{
break;
}
}
_logger.LogInformation("HTTP polling stopped.");
}
private async Task PollForJobsAsync(CancellationToken cancellationToken)
{
var requestUrl = $"/api/printerServer/jobs";
var response = await _httpClient.GetAsync(requestUrl, cancellationToken);
if (!response.IsSuccessStatusCode)
{
_logger.LogWarning("Failed to fetch jobs. Status: {StatusCode}", response.StatusCode);
return;
}
var jobs = await response.Content.ReadFromJsonAsync<List<PrintJobDto>>(cancellationToken: cancellationToken);
if (jobs == null || jobs.Count == 0)
{
return;
}
_logger.LogInformation("Received {Count} print job(s) from HTTP endpoint.", jobs.Count);
foreach (var job in jobs)
{
_logger.LogInformation("Queuing print job: {PrintJob}", JsonSerializer.Serialize(job));
_printJobChannel.Writer.TryWrite(new PrintJob
{
JobId = job.JobId,
IP = job.PrinterIp,
Document = new SignalRPrintJobSource.PrintJobFromSignalR
{
JobId = job.JobId,
PrinterIp = job.PrinterIp,
LogoUrl = job.LogoUrl,
ReceiptType = job.ReceiptType,
Content = job.Content
}
});
}
}
public Task StartAsync()
{
return StartPollingAsync(_pollingCts.Token);
}
public async Task<PrintJob> GetNextJobAsync(CancellationToken cancellationToken)
{
return await _printJobChannel.Reader.ReadAsync(cancellationToken);
}
public void Dispose()
{
_pollingCts.Cancel();
_pollingCts.Dispose();
_httpClient.Dispose();
}
public class PrintJobDto
{
[JsonPropertyName("jobId")]
public string JobId { get; set; } = "";
[JsonPropertyName("printerIp")]
public string PrinterIp { get; set; } = "";
[JsonPropertyName("logoUrl")]
public string? LogoUrl { get; set; }
[JsonPropertyName("receiptType")]
public int ReceiptType { get; set; }
[JsonPropertyName("content")]
public string Content { get; set; } = "";
}
}

View File

@@ -34,7 +34,7 @@ public class SignalRPrintJobSource: IPrintJobSource
_connection.Reconnected += OnReconnected;
_connection.Closed += OnClosed;
_ = InitializeConnectionAsync();
}
private async Task InitializeConnectionAsync()
@@ -106,6 +106,11 @@ public class SignalRPrintJobSource: IPrintJobSource
});
}
public Task StartAsync()
{
return InitializeConnectionAsync();
}
public async Task<PrintJob> GetNextJobAsync(CancellationToken cancellationToken)
{
return await _printJobChannel.Reader.ReadAsync(cancellationToken);

View File

@@ -11,6 +11,11 @@ public class SingleJobSource: IPrintJobSource
}
private Channel<PrintJob> _channel = Channel.CreateUnbounded<PrintJob>();
public Task StartAsync()
{
return Task.CompletedTask;
}
public async Task<PrintJob> GetNextJobAsync(CancellationToken cancellationToken)
{
return await _channel.Reader.ReadAsync(cancellationToken);

View File

@@ -1,454 +0,0 @@
# Junior Engineer Task List - Code Improvements
This document contains actionable tasks derived from the code analysis in `plan.md`. Complete tasks in priority order. After each fix, run:
```bash
dotnet build Inspectron.Epson.slnx
dotnet test EpsonTest/EpsonTest.csproj
```
---
## CRITICAL PRIORITY (Fix First)
### Task 1: Move API Key to Environment Variable
**File:** `EpsonPrintService/config.json:4`
**Problem:** API key stored in plaintext - security risk.
**Steps:**
1. Remove `ApiKey` from `config.json`
2. In `EpsonPrintService/Program.cs`, read from environment:
```csharp
var apiKey = Environment.GetEnvironmentVariable("EPSON_API_KEY")
?? throw new InvalidOperationException("EPSON_API_KEY environment variable not set");
```
3. Update `EpsonPrintServiceConfiguration` class to accept apiKey via constructor or property
4. Add to systemd service file `epson.service`:
```ini
[Service]
Environment="EPSON_API_KEY=your-key-here"
```
5. Document the required environment variable in README
---
### Task 2: Fix HttpClient Anti-Pattern
**Files:**
- `EpsonPrintService/HeartbeatBackgroundTask.cs:24`
- `Inspectron.Epson/PrintServer/PrintServices/EpsonPrintService.cs:68`
**Problem:** Creating new HttpClient per request causes socket exhaustion.
**Steps:**
1. In `HeartbeatBackgroundTask.cs`:
- Change constructor to accept `HttpClient` as parameter
- Remove `_httpClient = new HttpClient();` line
- Keep using `_httpClient` field
2. In `Program.cs` where HeartbeatBackgroundTask is created:
```csharp
// Create ONE HttpClient instance for the app lifetime
var httpClient = new HttpClient();
var heartbeatTask = new HeartbeatBackgroundTask(httpClient, ...);
```
3. In `EpsonPrintService.cs:68`:
- Add `HttpClient _httpClient` field to class
- Inject via constructor
- Replace `using var httpClient = new HttpClient();` with `_httpClient`
---
### Task 3: Add Graceful Shutdown
**File:** `EpsonPrintService/Program.cs:51-82`
**Problem:** PrintLoop and PrintServer never call StopAsync() - jobs lost on Ctrl+C.
**Steps:**
1. Store references to started services:
```csharp
var printLoop = kernel.Get<PrintLoop>();
var printServer = kernel.Get<PrintServer>();
```
2. In the Console.CancelKeyPress handler (or before Environment.Exit):
```csharp
Console.CancelKeyPress += async (sender, e) =>
{
e.Cancel = true; // Prevent immediate exit
Console.WriteLine("Shutting down gracefully...");
await printLoop.StopAsync();
await printServer.StopAsync();
cts.Cancel();
};
```
3. Ensure `PrintLoop` and `PrintServer` have `StopAsync()` methods that properly clean up
---
### Task 4: Add Null Check for PrinterId
**File:** `Inspectron.Epson/PrintServer/PrintServices/EpsonPrintService.cs:35-44`
**Problem:** `GetPrinterIdAsync()` can return null but `.Value` accessed without check.
**Steps:**
1. Find this code block:
```csharp
var printerId = await epsonPrinter.GetPrinterIdAsync();
var printerAdapter = _wrapperPrinterFactory.CreatePrinterFromId(printerId.Value, ...);
```
2. Add null check:
```csharp
var printerId = await epsonPrinter.GetPrinterIdAsync();
if (printerId == null || !printerId.HasValue)
{
_logger.LogError("Failed to get printer ID from {PrinterIp}", printerIp);
return false;
}
var printerAdapter = _wrapperPrinterFactory.CreatePrinterFromId(printerId.Value, ...);
```
---
### Task 5: Fix Race Condition in PrintServer
**File:** `Inspectron.Epson/Queue/PrintServer.cs:19-23`
**Problem:** TOCTOU bug - two threads could create duplicate queues.
**Steps:**
1. Change dictionary type from `Dictionary<string, PrinterQueue>` to:
```csharp
private readonly ConcurrentDictionary<string, PrinterQueue> _printerQueues = new();
```
2. Replace this pattern:
```csharp
if (_printerQueues.ContainsKey(printerIp))
return;
var queue = new PrinterQueue(...);
_printerQueues[printerIp] = queue;
```
With atomic operation:
```csharp
var queue = _printerQueues.GetOrAdd(printerIp, ip =>
{
var newQueue = new PrinterQueue(ip, _printService, _logger);
newQueue.StartAsync(); // Start the queue
Console.WriteLine($"Printer {ip} registered");
return newQueue;
});
```
3. Add `using System.Collections.Concurrent;` at top of file
---
## HIGH PRIORITY
### Task 6: Add CancellationToken Support
**File:** `Inspectron.Epson/EpsonPrinter.cs` - Lines 52, 96, 129, 205, 335, 408, etc.
**Problem:** Async methods can't be cancelled by callers.
**Steps:**
1. Find all public async methods in EpsonPrinter.cs
2. Add optional parameter to each:
```csharp
// Before:
public async Task<bool> ConnectAsync(string host, int port = 9100)
// After:
public async Task<bool> ConnectAsync(string host, int port = 9100, CancellationToken cancellationToken = default)
```
3. Pass cancellationToken to internal async calls:
```csharp
await _stream.WriteAsync(data, cancellationToken);
await _stream.ReadAsync(buffer, cancellationToken);
```
4. Update interface `IEpsonPrinter` if one exists
---
### Task 7: Add Exception Handling to Fire-and-Forget Tasks
**Files:**
- `Inspectron.Epson/PrintServer/PrintLoop.cs:29`
- `EpsonPrintService/Program.cs:71-72`
**Problem:** Unobserved exceptions crash the process.
**Steps:**
1. In `PrintLoop.cs`, replace:
```csharp
_ = Task.Run(Loop);
```
With:
```csharp
_ = Task.Run(async () =>
{
try
{
await Loop();
}
catch (Exception ex)
{
_logger.LogError(ex, "PrintLoop crashed unexpectedly");
}
});
```
2. In `Program.cs`, replace:
```csharp
_ = discoveryTask.StartAsync(cts.Token);
_ = heartbeatTask.StartAsync(cts.Token);
```
With:
```csharp
var discoveryRunning = Task.Run(async () =>
{
try { await discoveryTask.StartAsync(cts.Token); }
catch (Exception ex) { Console.WriteLine($"Discovery error: {ex.Message}"); }
});
var heartbeatRunning = Task.Run(async () =>
{
try { await heartbeatTask.StartAsync(cts.Token); }
catch (Exception ex) { Console.WriteLine($"Heartbeat error: {ex.Message}"); }
});
```
---
### Task 8: Implement IDisposable for Resource Cleanup
**Files:**
- `EpsonPrintService/HeartbeatBackgroundTask.cs`
- `Inspectron.Epson/Queue/PrinterQueue.cs:13-14`
- `Inspectron.Epson/EpsonPrinter.cs:757-761`
**Steps:**
1. **HeartbeatBackgroundTask.cs** - Add IDisposable:
```csharp
public class HeartbeatBackgroundTask : IDisposable
{
private bool _disposed;
public void Dispose()
{
if (_disposed) return;
_httpClient?.Dispose();
_disposed = true;
}
}
```
2. **PrinterQueue.cs** - Add IDisposable:
```csharp
public class PrinterQueue : IDisposable
{
private bool _disposed;
public void Dispose()
{
if (_disposed) return;
_semaphore?.Dispose();
_cts?.Dispose();
_disposed = true;
}
}
```
3. **EpsonPrinter.cs** - Fix DisposeAsync:
```csharp
public async ValueTask DisposeAsync()
{
Disconnect();
GC.SuppressFinalize(this);
await Task.CompletedTask;
}
```
---
### Task 9: Replace .Wait() with await
**File:** `Inspectron.Epson/Queue/PrintServer.cs:32`
**Problem:** `.Wait()` can cause deadlocks.
**Steps:**
1. Find method containing:
```csharp
queue.StopAsync().Wait();
```
2. Make the method async:
```csharp
// Before:
public void StopQueue(string printerIp)
// After:
public async Task StopQueueAsync(string printerIp)
```
3. Replace `.Wait()` with `await`:
```csharp
await queue.StopAsync();
```
4. Update all callers to use `await StopQueueAsync()`
---
### Task 10: Add Configuration Validation at Startup
**File:** `EpsonPrintService/Program.cs:20-21`
**Problem:** No validation after deserializing config.
**Steps:**
1. After config deserialization, add validation:
```csharp
var config = JsonSerializer.Deserialize<EpsonPrintServiceConfiguration>(configJson);
if (config == null)
throw new InvalidOperationException("Failed to parse config.json");
if (string.IsNullOrEmpty(config.GroupId))
throw new InvalidOperationException("GroupId is required in config.json");
if (string.IsNullOrEmpty(config.RestaurantId))
throw new InvalidOperationException("RestaurantId is required in config.json");
if (config.PrinterConfigurations == null || config.PrinterConfigurations.Count == 0)
Console.WriteLine("Warning: No printer configurations found");
```
---
## MEDIUM PRIORITY
### Task 11: Replace Console.WriteLine with ILogger
**File:** `Inspectron.Epson/Queue/PrintServer.cs:25, 34, 64`
Replace all `Console.WriteLine()` calls with proper logging:
```csharp
// Before:
Console.WriteLine($"Printer {printerIp} registered");
// After:
_logger.LogInformation("Printer {PrinterIp} registered", printerIp);
```
---
### Task 12: Fix Logger Factory Singleton
**File:** `EpsonPrintService/Program.cs:33-40`
Create factory once:
```csharp
// Create once at startup
using var loggerFactory = LoggerFactory.Create(builder =>
{
builder.AddConsole();
builder.SetMinimumLevel(LogLevel.Debug);
});
// Bind as singleton for all injections
kernel.Bind<ILoggerFactory>().ToConstant(loggerFactory);
kernel.Bind(typeof(ILogger<>)).ToMethod(ctx =>
loggerFactory.CreateLogger(ctx.Request.Service.GetGenericArguments()[0]));
```
---
### Task 13: Extract Magic Numbers to Constants
**File:** `Inspectron.Epson/Queue/PrinterQueue.cs:87-88`
```csharp
// Add at top of class:
private const int MaxRetryCount = 3;
// Replace:
if (job.RetryCount >= 3)
// With:
if (job.RetryCount >= MaxRetryCount)
```
---
### Task 14: Fix Static Thread Safety
**File:** `Inspectron.Epson/EpsonPrinter.cs:38-43`
```csharp
private static readonly object _initLock = new();
private static bool _initialized;
private static void RegisterCodepages()
{
if (_initialized) return;
lock (_initLock)
{
if (_initialized) return; // Double-check pattern
Encoding.RegisterProvider(CodePagesEncodingProvider.Instance);
_initialized = true;
}
}
```
---
## LOW PRIORITY
### Task 15: Remove Commented-Out Code
- `EpsonPrintService/Program.cs:27-28` - Delete old factory bindings
- `Inspectron.Epson/PrintServer/Printers/TM-T30III.cs:25` - Delete dead code
### Task 16: Resolve TODO Comments
**File:** `Inspectron.Epson/Queue/PrintJob.cs:5`
- Either change `IP` property to proper type or remove the TODO comment
### Task 17: Remove Unused Serilog Packages
**File:** `EpsonPrintService/EpsonPrintService.csproj:14-16`
```bash
dotnet remove EpsonPrintService/EpsonPrintService.csproj package Serilog
dotnet remove EpsonPrintService/EpsonPrintService.csproj package Serilog.Extensions.Logging
dotnet remove EpsonPrintService/EpsonPrintService.csproj package Serilog.Sinks.Console
```
### Task 18: Fix Inconsistent Logging Style
Change string interpolation to structured logging:
```csharp
// Before:
_logger.LogWarning($"Printer {printerIp} not found");
// After:
_logger.LogWarning("Printer {PrinterIp} not found", printerIp);
```
---
## Verification Checklist
After completing all tasks:
- [ ] `dotnet build Inspectron.Epson.slnx` - No errors
- [ ] `dotnet test EpsonTest/EpsonTest.csproj` - All tests pass
- [ ] Test printer connectivity manually
- [ ] Verify systemd service starts/stops cleanly
- [ ] Verify graceful shutdown with Ctrl+C
- [ ] Check no Console.WriteLine in library code (only logging)
---
## Questions?
If you're unsure about any task:
1. Read the original issue in `plan.md` for more context
2. Look at existing patterns in the codebase
3. Ask a senior engineer before making architectural changes

262
plan.md
View File

@@ -1,262 +0,0 @@
# Code Improvement Analysis: EpsonPrintService & Inspectron.Epson
## Executive Summary
Analysis of both projects revealed **50+ distinct issues** across security, reliability, performance, and maintainability. The most critical issues involve resource management, thread safety, and missing cancellation support.
---
## Critical Issues (Fix Immediately)
### 1. Security: Plaintext API Key
**File:** `EpsonPrintService/config.json:4`
```json
"ApiKey": "Pd8X/VtXLWgl5Djy4ksjdHC3xboeoh5Jig3Qo4277GQ="
```
- API key stored in plaintext JSON file
- **Fix:** Move to environment variables or secure configuration provider
### 2. HttpClient Anti-Pattern (Socket Exhaustion Risk)
**File:** `EpsonPrintService/HeartbeatBackgroundTask.cs:24`
```csharp
_httpClient = new HttpClient(); // Created per instance
```
**File:** `Inspectron.Epson/PrintServer/PrintServices/EpsonPrintService.cs:68`
```csharp
using var httpClient = new HttpClient(); // Created per request
```
- **Fix:** Inject singleton HttpClient or use IHttpClientFactory
### 3. Missing Graceful Shutdown
**File:** `EpsonPrintService/Program.cs:51-82`
- `PrintLoop` and `PrintServer` never call `StopAsync()` on exit
- Queued print jobs are lost on Ctrl+C
- **Fix:** Store references and call shutdown methods before exiting
### 4. NullReferenceException Risk
**File:** `Inspectron.Epson/PrintServer/PrintServices/EpsonPrintService.cs:35-44`
```csharp
var printerId = await epsonPrinter.GetPrinterIdAsync();
var printerAdapter = _wrapperPrinterFactory.CreatePrinterFromId(printerId.Value, ...);
```
- `GetPrinterIdAsync()` can return null but `.Value` is called without check
- **Fix:** Add null check before accessing `.Value`
### 5. Thread Safety: Race Condition in PrintServer
**File:** `Inspectron.Epson/Queue/PrintServer.cs:19-23`
```csharp
if (_printerQueues.ContainsKey(printerIp))
return;
var queue = new PrinterQueue(...);
_printerQueues[printerIp] = queue;
```
- TOCTOU bug: two threads could create duplicate queues
- **Fix:** Use `ConcurrentDictionary.GetOrAdd()` or `TryAdd()`
---
## High Priority Issues
### 6. Missing CancellationToken Support
**File:** `Inspectron.Epson/EpsonPrinter.cs` - Lines 52, 96, 129, 205, 335, 408, etc.
- Core async methods don't accept CancellationToken
- Callers cannot cancel long-running operations
- **Fix:** Add optional `CancellationToken cancellationToken = default` to all async methods
### 7. Fire-and-Forget Tasks Without Exception Handling
**File:** `Inspectron.Epson/PrintServer/PrintLoop.cs:29`
```csharp
_= Task.Run(Loop); // No exception handling
```
**File:** `EpsonPrintService/Program.cs:71-72`
```csharp
_ = discoveryTask.StartAsync(cts.Token);
_ = heartbeatTask.StartAsync(cts.Token);
```
- Unobserved exceptions will terminate the process
- **Fix:** Use exception handler or track tasks for proper completion
### 8. Resource Leaks (IDisposable Not Implemented)
**File:** `EpsonPrintService/HeartbeatBackgroundTask.cs` - No IDisposable
- HttpClient never disposed
**File:** `Inspectron.Epson/Queue/PrinterQueue.cs:13-14`
- SemaphoreSlim and CancellationTokenSource never disposed
**File:** `Inspectron.Epson/EpsonPrinter.cs:757-761`
```csharp
public async ValueTask DisposeAsync()
{
Disconnect();
await Task.CompletedTask; // Missing GC.SuppressFinalize
}
```
- **Fix:** Implement proper disposal pattern with try/finally
### 9. Synchronous Wait in Async Context
**File:** `Inspectron.Epson/Queue/PrintServer.cs:32`
```csharp
queue.StopAsync().Wait(); // Deadlock risk
```
- **Fix:** Use `await` or proper async pattern
### 10. Configuration Not Validated at Startup
**File:** `EpsonPrintService/Program.cs:20-21`
```csharp
var config = JsonSerializer.Deserialize<EpsonPrintServiceConfiguration>(...);
// No null check or validation
```
- **Fix:** Validate all required fields immediately after deserialization
---
## Medium Priority Issues
### 11. Console.WriteLine in Library Code
**File:** `Inspectron.Epson/Queue/PrintServer.cs:25, 34, 64`
```csharp
Console.WriteLine($"Printer {printerIp} registered");
```
- Library code should use ILogger only
- **Fix:** Replace with `_logger.LogInformation()`
### 12. Logger Factory Created Repeatedly
**File:** `EpsonPrintService/Program.cs:33-40`
- Creates new LoggerFactory for every ILogger injection
- **Fix:** Create factory once and bind as singleton
### 13. Magic Numbers Throughout
**File:** `Inspectron.Epson/EpsonPrinter.cs:263, 273, 282, 327, 463`
- Hard-coded ESC/POS bytes scattered in code
- **Fix:** Move to `EpsonCommands` static class
**File:** `Inspectron.Epson/Queue/PrinterQueue.cs:87-88`
```csharp
if (job.RetryCount >= 3) // Magic number
```
- **Fix:** Extract to configurable constant
### 14. SOLID Violations
**SRP Violation - EpsonPrinter.cs (763 lines)**
- Handles: connection, status, image processing, text printing, cutting
- **Fix:** Split into smaller classes (ConnectionManager, StatusProvider, ImagePrinter)
**SRP Violation - EpsonPrintServiceConfiguration**
- Implements both `IPrinterConfigurationSource` AND `IAssignedPrinterRepository`
- **Fix:** Separate into two classes
### 15. Code Duplication
**Image Conversion:**
- `ConvertToRasterData()` and `ConvertToColumnFormat()` share 50%+ logic
- **Fix:** Extract common image processing pipeline
**Image Loading:**
- Same file loading pattern repeated 4 times in EpsonPrinter.cs
- **Fix:** Create private helper method
### 16. Performance Issue in Image Conversion
**File:** `Inspectron.Epson/EpsonImageConverter.cs:243-247`
```csharp
image.ProcessPixelRows(accessor => { ... }); // Called per pixel!
```
- O(n²) performance instead of O(n)
- **Fix:** Call ProcessPixelRows once and cache accessor
### 17. Missing Input Validation
- `ConnectAsync()` doesn't validate IP format
- `PrintTextAsync()` doesn't validate text encoding
- `FeedLinesAsync()` doesn't validate line count > 0
- **Fix:** Add guard clauses with ArgumentException
### 18. Static State Thread Safety
**File:** `Inspectron.Epson/EpsonPrinter.cs:38-43`
```csharp
private static bool _initialized;
private static void RegisterCodepages()
{
if (_initialized) return; // No lock!
Encoding.RegisterProvider(...);
_initialized = true;
}
```
- **Fix:** Use lock or `Interlocked.CompareExchange()`
---
## Low Priority Issues
### 19. Commented-Out Code
- `EpsonPrintService/Program.cs:27-28` - Old factory bindings
- `Inspectron.Epson/PrintServer/Printers/TM-T30III.cs:25` - Dead code
- **Fix:** Remove or track in issue tracker
### 20. TODO Comments Left in Code
**File:** `Inspectron.Epson/Queue/PrintJob.cs:5`
```csharp
// todo: change to ip address
public string IP { get; set; }
```
- **Fix:** Resolve or create proper issue
### 21. Unused Serilog Dependencies
**File:** `EpsonPrintService/EpsonPrintService.csproj:14-16`
- Serilog packages referenced but not used
- **Fix:** Remove unused packages
### 22. Inconsistent Logging
- String interpolation used instead of structured logging placeholders
- Log levels inconsistent (LogWarning for config errors that should fail startup)
- **Fix:** Use `LogWarning("Printer {PrinterId} not found", printerIp)` pattern
### 23. API Design Inconsistencies
- `PrintTextAsync()` vs `PrintTextAndCutAsync()` naming
- 3 overloads of `PrintImageBitModeAsync()` and `LoadImageAsync()`
- Interface doesn't match all implementation overloads
- **Fix:** Rationalize method naming and signatures
---
## Suggested Refactoring Projects
### Project 1: Resource Management Overhaul
- Implement IAsyncDisposable properly across all classes
- Fix all HttpClient instantiation
- Add proper cleanup on shutdown
- **Scope:** ~10 files, ~200 lines changed
### Project 2: Async Pattern Modernization
- Add CancellationToken to all async methods
- Fix fire-and-forget patterns
- Replace synchronous waits
- **Scope:** ~8 files, ~150 lines changed
### Project 3: Thread Safety Fixes
- Fix race conditions in PrintServer and PrinterQueue
- Add proper locking for static initialization
- **Scope:** ~4 files, ~50 lines changed
### Project 4: Split EpsonPrinter Class
- Extract ConnectionManager
- Extract StatusProvider
- Extract ImagePrinter
- Keep EpsonPrinter as facade
- **Scope:** ~5 new files, ~800 lines reorganized
### Project 5: Configuration & DI Cleanup
- Add configuration validation
- Fix Logger factory singleton
- Separate configuration concerns
- Move secrets to environment variables
- **Scope:** ~5 files, ~100 lines changed
---
## Verification
After implementing any changes:
1. Build the solution: `dotnet build Inspectron.Epson.slnx`
2. Run existing tests: `dotnet test EpsonTest/EpsonTest.csproj`
3. Test printer connectivity manually with EpsonTest project
4. Verify systemd service starts/stops cleanly