davide.trentin/nc_program_manager
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions
nc_program_manager/.paul/ISSUES.md
Trentin Davide 85d0b579aa fix(validator,field-test): unblock Fanuc/Mitsubishi upload (ISS-018, ISS-019) 
ISS-018: FanucProgram.Standardize() prepends a leading '\n' to the
program body, so the O#### header is not on lines[0]. The validator
first-line check read lines[0] and always failed "First line must be
O####", rejecting every valid Fanuc/Mitsubishi upload. Scan the first
non-blank line instead. Upload (FOCAS tape) format unchanged.

ISS-019: field-test .bat files passed -pathprogramma unquoted; on
install paths with spaces ("visual studio prove") cmd split the arg and
ArgParser captured only the first token -> truncated path. Quote the
arg in all 7 scripts.

Add regression test Validate_LeadingBlankLineBeforeONumber for both
Fanuc and Mitsubishi.

Surfaced by field testing against a local docker FTP server.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-02 09:42:46 +02:00

227 lines
13 KiB
Markdown
Raw Blame History

# Project Issues Log
Enhancements and known issues discovered during execution and codebase analysis.
## Open Issues
### ~~ISS-001: MachineFingerprint null dereference on unusual hardware~~ ✅ CLOSED
- **Closed:** 2026-05-18 (Phase 13, commit `4cd7715`)
- **Fix applied:** `?.Value?.ToString() ?? string.Empty` on both WMI reads in `Licensing/MachineFingerprint.cs`
---
### ISS-002: Blocking async calls at CLI boundary
- **Discovered:** Codebase analysis (2026-05-18)
- **Type:** Refactoring / Reliability
- **Description:** 56+ `.GetAwaiter().GetResult()` calls in `Program.cs` and machine classes block the thread pool. .NET 4.7.2 supports `async Task<int> Main`. Risk of deadlock in certain SynchronizationContext scenarios.
- **Files:** `Program.cs` (lines 90, 101, 122, 142, 166, 200, 221, 240, 262)
- **Fix:** Refactor `Main` to `static async Task<int> Main(string[] args)`, propagate `await` throughout
- **Impact:** Medium — currently works, but fragile; blocks async benefits
- **Effort:** Medium (1-2 hours)
- **Suggested phase:** Future refactor phase
---
### ~~ISS-003: Silent exception handlers swallow failures~~ ✅ CLOSED
- **Closed:** 2026-05-18 (Phase 15)
- **Fix applied:** Event handler catches (`SetState`) → `_log.Warn(ex, "StateChanged handler threw")`; cleanup/Dispose catches annotated with `/* cleanup — intentionally swallowed */`
---
### ~~ISS-004: No structured logging framework~~ ✅ CLOSED
- **Closed:** 2026-05-18 (Phase 15)
- **Fix applied:** NLog 5.2.8 wired into all 5 production classes; rolling file target (daily, 7-day archive) + console target (Warn+); `_log.Error` in Program.cs error catch blocks
---
### ~~ISS-005: SHA256 crypto provider not disposed~~ ✅ CLOSED
- **Closed:** 2026-05-18 (Phase 13, commit `4cd7715`)
- **Fix applied:** `SHA256CryptoServiceProvider` wrapped in `using` block in `Licensing/LicenseValidator.cs`
---
### ~~ISS-006: LSV2 connect timeout doesn't abort hung task~~ ✅ CLOSED
- **Closed:** 2026-05-18 (Phase 13, commit `4cd7715`)
- **Fix applied:** `_tcp.Close()` called in timeout branch of `Connect()` in `Cnc/Heidenhain/Lsv2Client.cs` — aborts pending socket task
---
### ~~ISS-007: Integration tests use hardcoded IPs~~ ✅ CLOSED
- **Closed:** 2026-05-18 (Phase 14)
- **Fix applied:** `HardwareTestHelper.GetIpOrSkip(envVarName)` reads `HEIDENHAIN_IP` / `SIEMENS_IP` / `MITSUBISHI_IP`; `Assert.Ignore` fires when env var not set. Hardware tests `[Category("Hardware")]` skip in standard CI; stub-based tests unaffected.
---
### ISS-008: MachineFingerprint — full license flow unverified on real hardware
- **Discovered:** STATE.md deferred issues (ongoing)
- **Partial fix:** 2026-05-18 (Phase 14) — `LicenseFile` edge-case tests added; `Validate_EmptyFingerprint_ReturnsFalse` confirms validator is safe against empty input
- **Remaining:** Full end-to-end (WMI fingerprint → sign → deploy → validate) requires real Windows machine run with `HEIDENHAIN_IP` etc. set
- **Type:** Testing
- **Files:** `Licensing/MachineFingerprint.cs`, `Licensing/LicenseFile.cs`
- **Impact:** High — license flow unverified on hardware before customer delivery
- **Effort:** Quick — scaffold is in place; just needs real hardware
---
### ISS-009: Heidenhain ReadActiveProgramAsync / SelectMainProgram not implemented
- **Discovered:** STATE.md deferred issues (2026-05-13)
- **Type:** Feature / Incomplete Implementation
- **Description:** `HeidenhainMachine.ReadActiveProgramAsync()` and `SelectMainProgramAsync()` return hard-coded `CncResult.Fail("Not supported via basic LSV2")`. LSV2 may support these via additional commands not yet implemented.
- **Files:** `Cnc/Heidenhain/HeidenhainMachine.cs`
- **Impact:** Low — current use case requires explicit program path; feature unused
- **Effort:** Substantial (requires LSV2 spec research + hardware testing)
- **Suggested phase:** Heidenhain advanced features (future)
---
### ISS-010: Siemens ReadActiveProgramAsync / SelectMainProgram not implemented
- **Discovered:** STATE.md deferred issues (2026-05-13)
- **Type:** Feature / Incomplete Implementation
- **Description:** Same as ISS-009 for Siemens. FTP protocol may not expose active program info natively.
- **Files:** `Cnc/Siemens/SiemensMachine.cs`
- **Impact:** Low — current use case requires explicit program path
- **Effort:** Substantial (requires Sinumerik FTP extension research)
- **Suggested phase:** Siemens advanced features (future)
---
### ISS-011: Obsolete crypto APIs (RSACryptoServiceProvider, SHA256CryptoServiceProvider)
- **Discovered:** Codebase analysis (2026-05-18)
- **Type:** Technical Debt
- **Description:** Both classes marked obsolete in .NET 6+. Currently safe on .NET 4.7.2 (only warnings), but blocks future framework upgrade path.
- **Files:** `Licensing/LicenseValidator.cs`
- **Fix (when upgrading):** `RSA.Create()` + `rsa.VerifyData(data, sig, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1)` — eliminates both obsolete types
- **Impact:** Low — no runtime impact on .NET 4.7.2
- **Effort:** Quick (when .NET version is upgraded)
- **Suggested phase:** Future .NET upgrade phase
---
### ISS-012: Fagor `ReadActiveProgramAsync` / `SelectMainProgramAsync` not implemented
- **Discovered:** Code review v0.6 (2026-05-25)
- **Type:** Feature / Incomplete Implementation
- **Description:** `FagorMachine.ReadActiveProgramAsync()` e `SelectMainProgramAsync()` ritornano `CncResult.Fail("Not supported via FTP")`. Analogo a ISS-009 (Heidenhain) e ISS-010 (Siemens). Fagor espone stato runtime via DNC/OPC-UA non FTP.
- **Files:** `Cnc/Fagor/FagorMachine.cs`
- **Impact:** Low — caso d'uso corrente richiede path esplicito
- **Effort:** Substantial (richiede protocollo DNC/OPC-UA Fagor)
- **Suggested phase:** Future Fagor advanced features
---
### ISS-013: `Program.Invia` accoppia `FanucProgram` parsing a tutti i produttori
- **Discovered:** Code review v0.6 (2026-05-25)
- **Type:** Bug / Refactoring
- **Description:** `Program.Invia` crea `new FanucProgram(File.ReadAllText(...))` e chiama `GetProgramTitle()` con regex `O####` o `\d{4}` per tutti i produttori. Per Siemens/Mitsubishi/Fagor con programmi non in formato Fanuc, lancia `InvalidOperationException`. Fagor incrementa esposizione del bug.
- **Files:** `Program.cs` (linee 188-202), `FanucProgram.cs`
- **Fix:** Per non-Fanuc usare `Path.GetFileNameWithoutExtension(inputArgs.pathLocaleProgramma)` come `cncProg.Name`. Bypassare parsing Fanuc.
- **Impact:** Medium — CLI `Invia` rotta per tutti i non-Fanuc (Siemens/Mitsubishi/Fagor)
- **Effort:** Medium (1-2h) — refactor Program.Invia con switch su manufacturer
- **Suggested phase:** Future CLI cleanup phase
---
### ISS-014: `ResolvePath` null-folder NRE shared pattern (Siemens/Mitsubishi/Fagor)
- **Discovered:** Code review v0.6 (2026-05-25)
- **Type:** Bug / Defensive coding
- **Description:** `ResolvePath(folder, name)` in tutti e 3 le macchine FTP dereferenzia `folder` senza null-guard nel ramo `IsNullOrEmpty(name)`. Caller che passa `path = null` (es. `ReadProgramAsync(null)`) provoca NullReferenceException.
- **Files:** `Cnc/Siemens/SiemensMachine.cs`, `Cnc/Mitsubishi/MitsubishiMachine.cs`, `Cnc/Fagor/FagorMachine.cs`
- **Fix:** `if (folder == null) folder = _config.ProgramDirectory;` all'inizio del metodo. Pattern condiviso → considerare helper unitario.
- **Impact:** Low — null path raro in pratica, ma viola contratto API
- **Effort:** Quick (10 min) — refactor uniforme
---
### ISS-015: Encoding source-file rischio `ñ` su build Windows non-UTF8
- **Discovered:** Code review v0.6 (2026-05-25)
- **Type:** Build / Encoding
- **Description:** File sorgenti UTF-8 senza BOM. `csc.exe` su Windows con codepage 1252 (Italian) può decodificare letterali non-ASCII come Mojibake. Risolto in v0.6 (Phase 16-02) per `ñ` in `NcProgramValidator.cs` usando `ñ`. Altri file (`Program.cs` con `è`, `à` in stringhe italiane) ancora potenzialmente a rischio. Funziona se build box codepage compatibile, ma fragile.
- **Files:** `Program.cs`, `Cnc/NcProgramValidator.cs`, file `.cs` con letterali italiani
- **Fix:** (a) Aggiungere UTF-8 BOM a tutti i sorgenti, OR (b) sostituire tutti letterali non-ASCII con escape `\uXXXX`, OR (c) aggiungere `/codepage:65001` ai compile args MSBuild.
- **Impact:** Medium — silent corruption se build box ha codepage diversa
- **Effort:** Medium (audit completo file + correzione)
- **Suggested phase:** Future build hardening phase
---
### ISS-016: `WriteProgramAsync` validator riceve `path` se `program.Name` null (Siemens/Mitsubishi/Fagor)
- **Discovered:** Code review v0.6 (2026-05-25)
- **Type:** API contract / Validation
- **Description:** `_validator.Validate(program.Name ?? path, program.Content)` — se `program.Name` null, validator valida l'intero path FTP come fosse il nome programma. Errori validatore confusi.
- **Files:** `Cnc/Siemens/SiemensMachine.cs`, `Cnc/Mitsubishi/MitsubishiMachine.cs`, `Cnc/Fagor/FagorMachine.cs`
- **Fix:** Fail-fast con `CncError("WriteProgram", "Program.Name is required")` quando `program.Name == null`. Documentare in `CncProgram.cs`.
- **Impact:** Low — caller di solito imposta `Name`
- **Effort:** Quick
---
### ISS-017: `FagorFtpClient.Ping` swallow exception senza log
- **Discovered:** Code review v0.6 (2026-05-25)
- **Type:** Diagnostics
- **Description:** `Ping()` catch generico ritorna `false` senza log. Stesso pattern in Siemens/Mitsubishi. Diagnostica ridotta su problemi di connessione (timeout vs credenziali vs path mancante).
- **Files:** `Cnc/Siemens/SiemensFtpClient.cs`, `Cnc/Mitsubishi/MitsubishiFtpClient.cs`, `Cnc/Fagor/FagorFtpClient.cs`
- **Fix:** Aggiungere campo `_log` + `_log.Debug(ex, "FTP ping failed")` nel catch. Mantenere return false. Pattern condiviso.
- **Impact:** Low — diagnostica
- **Effort:** Quick
---
### ~~ISS-018: `Standardize()` `\n`-prefix breaks `NcProgramValidator` first-line check~~ ✅ CLOSED
- **Discovered:** Field test docker FTP (2026-06-02)
- **Closed:** 2026-06-02
- **Type:** Bug (CRITICAL)
- **Description:** `FanucProgram.Standardize()` prepends `"\n"` to `Program` (`FanucProgram.cs:147-153`). `NcProgramValidator` first-line check reads `lines[0]` and expects `^O\d{4}` (`NcProgramValidator.cs:40-46`). After standardize, `lines[0] == ""` → check always fails `"First line must be O#### program number"`. **Every Fanuc/Mitsubishi upload rejected**, including valid `O0001(TEST HENESIS)`. Confirmed via field test: all 5 T03 samples + T01 + T07 INVIO returned `-204`.
- **Files:** `Cnc/NcProgramValidator.cs`, `FanucProgram.cs`
- **Fix applied:** validator first-line check now scans first **non-blank** line instead of `lines[0]`, tolerating the leading `\n` that `Standardize()` injects. Upload format unchanged (FOCAS tape format preserved).
- **Impact:** Critical — no valid program uploadable for Fanuc/Mitsubishi
- **Effort:** Quick
---
### ~~ISS-019: `-pathprogramma` truncated at first space (unquoted `.bat` args)~~ ✅ CLOSED
- **Discovered:** Field test docker FTP (2026-06-02)
- **Closed:** 2026-06-02
- **Type:** Bug (CRITICAL)
- **Description:** Field-test `.bat` files pass `-pathprogramma=%~dp0samples\...` and `%RESULTS%\...` **unquoted**. When the install path contains spaces (`...\visual studio prove\...`), cmd splits the argument into multiple argv tokens. `ArgParser.cs:73` `Find` matches only the first token → `Substring(15)` yields a truncated path (e.g. `C:\Users\trent\Desktop\davide\visual`). Effect: SCARICA writes the download to the wrong file (still exit 0, masking failure); INVIA later reads that stray file → garbage. Explains the observed T02→T01 interaction.
- **Files:** `field-test/t01_invia_valido.bat`, `t02_scarica.bat`, `t03_validatore.bat`, `t04_errori_connessione.bat`, `t05_offset_non_supportati.bat`, `t07_roundtrip.bat`
- **Fix applied:** all `-pathprogramma=...` args quoted (`-pathprogramma="..."`) across the 6 `.bat` files. `ArgParser` itself is correct when argv is passed properly (Windows delivers a quoted spaced path as a single token).
- **Impact:** Critical — all file I/O hit wrong path on space-containing install dirs
- **Effort:** Quick
---
## Closed Issues
| ID | Summary | Phase | Commit |
|----|---------|-------|--------|
| ISS-001 | MachineFingerprint null dereference on unusual hardware | 13 | `4cd7715` |
| ISS-005 | SHA256 crypto provider not disposed | 13 | `4cd7715` |
| ISS-006 | LSV2 connect timeout doesn't abort hung task | 13 | `4cd7715` |
| ISS-007 | Integration tests use hardcoded IPs | 14 | `32f68c5` |
| ISS-003 | Silent exception handlers swallow failures | 15 | (Phase 15 commit) |
| ISS-004 | No structured logging framework | 15 | (Phase 15 commit) |
| ISS-018 | Standardize() \n-prefix breaks validator first-line check | field-test | (pending) |
| ISS-019 | -pathprogramma truncated at first space (unquoted .bat) | field-test | (pending) |
---
*Last updated: 2026-06-02*
*Sources: STATE.md deferred issues + CONCERNS.md codebase analysis + field-test docker FTP*
Reference in a new issue View git blame Copy permalink