Joseph Doherty
7ead3581ab
Closes the Wave 3 (cleanup) tier of codereviews/2026-05-14/RemediationPlan.md.
Tests: 378 pass / 0 fail (baseline 370 + 8 new W3 regression tests).
Code cleanups:
* PlcMultiplexer: removed dead `elapsedMs` calculation (the actual EWMA
conversion uses Stopwatch ticks two lines below).
* UpstreamPipe.FillAsync: dropped the meaningless `firstRead && remaining
== count ? false : false` ternary; both branches were `false`.
* InFlightByKeyMap.TryAttachOrCreate (always returned `true`) renamed to
`AttachOrCreate` and made `void`. Test sites updated to drop the dead
`bool ok = ...; ok.ShouldBeTrue();` assertions.
* BcdCodec.HasBadNibble promoted from private to internal; the duplicate
copy in BcdPduPipeline removed and the call sites updated to
`BcdCodec.HasBadNibble`.
* PlcMultiplexer watchdog comment fixed: said "1-second floor", code uses
100 ms. Now both agree.
* StatusSnapshotBuilder: simplified the unreachable
`RemoteEp?.ToString() ?? RemoteEp?.Address.ToString() ?? "?"` to
`RemoteEp?.ToString() ?? "?"`.
* Mbproxy.csproj: stale "deferred" Polly comment replaced with a real
description of where Polly is used (BackendConnect + ListenerRecovery).
Doc updates:
* README: added a callout about the unconventional 32-bit BCD wire format
("two base-10000 digits in CDAB", not standard binary CDAB Int32) so
integrators using off-the-shelf clients learn about the silent-corruption
hazard before configuring writes.
* docs/design.md: clarified `cacheMissCount` and `coalescedMissCount`
semantics — "miss" means "did not find a fresh entry / did not coalesce",
NOT "produced a backend round-trip". Operators wanting actual backend
traffic should compute `miss − coalescedHit − exception04`.
* docs/Architecture/ResponseCache.md: documented the structural
"skip invalidation while recovering" gating (no backend reader during
recovery → no FC06/FC16 response → no invalidation).
* docs/Operations/Configuration.md: noted that the Event Log sink is the
custom EventLogBridge, not Serilog.Sinks.EventLog (W2.23 cached check).
* docs/plan/README.md: added a Phase 12 row pointing at the remediation
plan and linking out to codereviews/2026-05-14/.
Test additions (W3 high-value gaps):
* BcdPduPipelineTests:
- FC16_WriteStartsOnHighWord_Of32BitPair_PassesThroughRaw_WithPartialWarning
(symmetric inverse of the existing low-side partial-overlap test).
- FC03_Mixed_16Bit_32Bit_AndNonBcd_InOneRead_OnlyConfiguredSlotsRewritten
(mixed-slot routing in a single FC03 read).
- FC16_Response_PassesThroughUnchanged_RegardlessOfTagMap (FC16 response
carries no register data; rewriter must pass through).
* AdminEndpointTests:
- NonGetMethod_AgainstAdminRoutes_Returns405 (Theory: POST/PUT/DELETE/
PATCH against `/` and `/status.json` must return 405; guards against
an accidental MapPost being added later).
* HotReloadE2ETests:
- E2E_TagListReload_OnCacheablePlc_EmitsCacheFlushedEvent (validates the
W2.8 cache.flushed wiring end-to-end via the real FileSystemWatcher
reload path).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
124 lines
6.8 KiB
Markdown
124 lines
6.8 KiB
Markdown
# mbproxy
|
||
|
||
A .NET 10 Windows Service that sits inline as a Modbus TCP proxy in front of a fleet of AutomationDirect DirectLOGIC DL205/DL260 controllers, rewriting BCD-encoded registers bidirectionally so upstream clients can read and write them as plain integers. The proxy also offers an opt-in per-tag response cache (default OFF) for FC03/FC04 reads with bounded operator-configured staleness — see [`docs/Architecture/ResponseCache.md`](docs/Architecture/ResponseCache.md) before enabling it.
|
||
|
||
> ⚠ **32-bit BCD wire format is "two base-10000 digits in CDAB", not standard CDAB binary Int32.** A 32-bit BCD tag at address `A` decodes as `decimal = high * 10_000 + low` where `low` is the register at `A` and `high` is the register at `A+1`. Each word independently must be 0–9999. Standard Modbus clients (NModbus, FluentModbus, Wonderware DAServer) that interpret CDAB as straight binary Int32 will silently corrupt any value > 9999 on writes and read garbage on reads. Configure your client to send/receive each register as a separate base-10000 BCD digit pair, not as a single binary Int32. Full details in [`docs/Features/BcdRewriting.md`](docs/Features/BcdRewriting.md).
|
||
|
||
## Hard constraints / prerequisites
|
||
|
||
- **Windows 10 / Server 2019 or later, 64-bit.** No Linux or Docker support — the service uses `Microsoft.Extensions.Hosting.WindowsServices` and the Windows Event Log.
|
||
- **Modbus TCP backends reachable** from the proxy host on port 502 (or the port configured per PLC). The H2-ECOM100 module caps simultaneous connections at **4 per PLC** — a fifth upstream client will fail to connect.
|
||
- **Admin rights** to install the service (`install.ps1` requires elevation).
|
||
- **No COM dependency** — this is a pure .NET 10 socket-level proxy (unlike the `.NET Framework 4.8 / x86` siblings in this repo).
|
||
- **Python 3.10+** on the test machine to run the pymodbus-backed E2E simulator (not needed to run the service in production).
|
||
|
||
## Layout
|
||
|
||
```
|
||
src/Mbproxy/ Main C# project (net10.0, Microsoft.NET.Sdk.Worker)
|
||
tests/Mbproxy.Tests/ xUnit v3 test project (314 unit + 48 E2E tests)
|
||
install/ PowerShell install/uninstall scripts and config template
|
||
docs/ Architecture, features, operations, reference, and testing docs
|
||
DL260/ DL205/DL260 reference material and pymodbus simulator profile
|
||
```
|
||
|
||
## Resource index
|
||
|
||
| Task | Go to |
|
||
|---|---|
|
||
| End-to-end architectural design (entry point — routes into focused docs below) | [`docs/design.md`](docs/design.md) |
|
||
| Phase-by-phase implementation plan and history | [`docs/plan/README.md`](docs/plan/README.md) |
|
||
| Install, upgrade, uninstall, log file locations, first-install smoke checklist | [`docs/operations.md`](docs/operations.md) |
|
||
| Dashboard KPI catalog | [`docs/kpi.md`](docs/kpi.md) |
|
||
| DL205/DL260 Modbus quirks (BCD, CDAB, octal V-memory, FC limits) | [`DL260/dl205.md`](DL260/dl205.md) |
|
||
| pymodbus simulator profile (register seeds for E2E tests) | [`DL260/dl205.json`](DL260/dl205.json) |
|
||
| Agent-oriented coding guide (architecture bullets, device quirks, phase context) | [`CLAUDE.md`](CLAUDE.md) |
|
||
|
||
## Detailed documentation
|
||
|
||
The `docs/` tree is organized by topic. Start with [`docs/design.md`](docs/design.md) for the canonical end-to-end design; jump to the focused pages below when you need depth on one area.
|
||
|
||
### Architecture
|
||
|
||
- [`Architecture/Overview.md`](docs/Architecture/Overview.md) — Listener topology, request flow, per-PLC isolation.
|
||
- [`Architecture/ConnectionModel.md`](docs/Architecture/ConnectionModel.md) — Single backend connection per PLC, TxId multiplexing, request-timeout watchdog, disconnect cascade.
|
||
- [`Architecture/ReadCoalescing.md`](docs/Architecture/ReadCoalescing.md) — In-flight FC03/FC04 deduplication via `InFlightByKeyMap`.
|
||
- [`Architecture/ResponseCache.md`](docs/Architecture/ResponseCache.md) — Opt-in per-tag response cache with bounded operator-configured staleness.
|
||
|
||
### Features
|
||
|
||
- [`Features/BcdRewriting.md`](docs/Features/BcdRewriting.md) — BCD codec, CDAB word order, FC03/04/06/16 scope, partial-overlap policy.
|
||
- [`Features/HotReload.md`](docs/Features/HotReload.md) — `IOptionsMonitor`-driven config reload with per-change-kind reconcile rules.
|
||
|
||
### Operations
|
||
|
||
- [`Operations/Configuration.md`](docs/Operations/Configuration.md) — Full `appsettings.json` reference: every `Mbproxy:*` key, default, and validation rule.
|
||
- [`Operations/StatusPage.md`](docs/Operations/StatusPage.md) — Admin endpoint surface (`/`, `/status.json`) with every JSON field documented.
|
||
- [`Operations/Troubleshooting.md`](docs/Operations/Troubleshooting.md) — Diagnosis playbook keyed to log events and status counters.
|
||
|
||
### Reference
|
||
|
||
- [`Reference/LogEvents.md`](docs/Reference/LogEvents.md) — Stable `mbproxy.*` event catalog (28 events across 7 categories).
|
||
|
||
### Testing
|
||
|
||
- [`Testing/Simulator.md`](docs/Testing/Simulator.md) — pymodbus DL205 fixture, skip policy, and the load-bearing pymodbus 3.13 framer quirk.
|
||
|
||
## Build and run
|
||
|
||
**Build (Debug, multi-file — fast for iteration):**
|
||
|
||
```powershell
|
||
dotnet build Mbproxy.slnx -c Debug
|
||
```
|
||
|
||
**Publish (Release, single-file self-contained, win-x64):**
|
||
|
||
```powershell
|
||
dotnet publish src/Mbproxy/Mbproxy.csproj -c Release -r win-x64 --self-contained true -o C:\build\mbproxy-publish
|
||
```
|
||
|
||
The published output is a single `Mbproxy.exe` (~100 MB). The self-contained publish bundles the full .NET 10 + ASP.NET Core runtime. No .NET installation is required on the target machine.
|
||
|
||
**Run tests:**
|
||
|
||
```powershell
|
||
dotnet test Mbproxy.slnx -c Debug # all tests
|
||
dotnet test Mbproxy.slnx -c Debug --filter Category=Unit # unit tests only (no Python required)
|
||
dotnet test Mbproxy.slnx -c Debug --filter Category=E2E # E2E tests (require Python + pymodbus)
|
||
```
|
||
|
||
**Run interactively (without installing as a service):**
|
||
|
||
```powershell
|
||
cd src/Mbproxy
|
||
dotnet run --configuration Debug
|
||
```
|
||
|
||
Edit `src/Mbproxy/appsettings.json` to configure PLCs before running. The admin status page will be at `http://localhost:8080/` by default.
|
||
|
||
## Install
|
||
|
||
Full detail is in [`docs/operations.md`](docs/operations.md). Quick path:
|
||
|
||
```powershell
|
||
# 1. Publish
|
||
dotnet publish src/Mbproxy/Mbproxy.csproj -c Release -r win-x64 --self-contained true -o C:\build\mbproxy-publish
|
||
|
||
# 2. Install (elevated PowerShell)
|
||
.\install\install.ps1 -PublishOutput C:\build\mbproxy-publish -Start
|
||
|
||
# 3. Edit the config that was placed at %ProgramData%\mbproxy\appsettings.json
|
||
|
||
# 4. Verify
|
||
Invoke-WebRequest http://localhost:8080/ -UseBasicParsing
|
||
```
|
||
|
||
## Maintenance
|
||
|
||
Documentation doctrine for this repo: [`../DOCS-GUIDE.md`](../DOCS-GUIDE.md).
|
||
|
||
- This README routes to deep docs — it does not duplicate them.
|
||
- Design decisions: [`docs/design.md`](docs/design.md) is the source of truth.
|
||
- When the service's public surface or task→tool mapping changes, update this README and the root [`../CLAUDE.md`](../CLAUDE.md) index row.
|