nc_program_manager/.paul/ISSUES.md
dtrentin 19da0e598e feat(mazak): add Mazak CNC support via FTP (EIA/ISO)
New Cnc/Mazak/ machine integration mirroring the Fagor/Mitsubishi FTP
pattern: MazakConnectionConfig, IMazakFtpClient, MazakFtpClient,
MazakMachine (ICncMachine). Transfers EIA/ISO G-code text over FTP to
Smooth-family controllers. Mazatrol CMT (proprietary binary) out of scope.

Design (verified against Mazak Matrix EIA manual + field reports):
- Filenames preserved verbatim; no O#### rename, no extension append.
- Symmetric ResolvePath for Read/Write/Delete (avoids ISS-020 class;
  Delete resolved too, unlike copied Mitsubishi source).
- Validator = full passthrough for Mazak (EIA permits ';' EOB, '[ ]',
  '#'; restrictive Fanuc checks would false-reject valid programs).
- Port 21 default, -porta=23 for Smooth Ai IIS variant.
- ProgramDirectory site-specific, no default.

Wiring: CncManufacturer.Mazak, ArgParser 'mazak' token, CncMachineFactory.
Tests: MazakMachineTests (unit), MazakMachineIntegrationTests (FTP-stub
roundtrip + symmetry guard), Mazak cases in validator/factory tests.
Docs: README Controller Mazak section + table row; ISS-021 (constraints,
EIA-option requirement), ISS-022 (Mitsubishi delete asymmetry logged).

NOT build-verified (no .NET toolchain in build env) — verify on Windows.

Projects: NcProgramManager, NcProgramManager.Tests

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-16 19:12:07 +02:00

22 KiB
Raw Permalink 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

ISS-020: Write/Read FTP path asymmetry — ResolvePath (Mitsubishi fixed; Siemens/Fagor open)

  • Discovered: Field test docker FTP, post ISS-018/019 fix (2026-06-02)
  • Mitsubishi fix applied: 2026-06-02 — WriteProgramAsync now resolves the path identically to ReadProgramAsync; ResolvePath simplified to single-arg (path = identifier or full path, combined with ProgramDirectory). Verified against Mitsubishi NC Explorer manual IB-1500904 (PRG/USER/<Program name>, no extension, addressed by program name/number). Unit tests updated + roundtrip regression test added. program.Name now used for validation only.
  • Still open (Siemens/Fagor): same asymmetric pattern, but their ResolvePath ties extension handling (.MPF / .nc) to name — needs separate fix + entangled with ISS-013 (Fanuc-parsing coupling in Program.Invia). Untested (no field rig yet).
  • Default ProgramDirectory note: real Mitsubishi path is /PRG/USER/; current config default is /PRG/. Adjust per controller (config-level, not code).
  • Type: Bug (design) — criticità C2/C3
  • Description: -path integer is resolved differently for read vs write.
    • Read ResolvePath("1", null)"1" has no /ProgramDirectory + "1" = /PRG/1 (correct, file exists).
    • Write ResolvePath("1", program.Name="O0001")folder="1" treated as directory → "1/" + "O0001" = 1/O0001 (relative dir 1 does not exist → STOR fails with WebException surfaced as "Impossibile effettuare la connessione al server remoto"). Confirmed against docker FTP: STOR 1/O0001 → curl exit 9; STOR /PRG/1 → exit 0; no 1/ dir created (app does not create dirs).
  • Effect: INVIO of any valid program fails (-204); roundtrip impossible — write target ≠ read target. Was previously masked by ISS-018 (validator blocked all uploads) and ISS-019 (truncated path).
  • Files: Cnc/Mitsubishi/MitsubishiMachine.cs ResolvePath, same pattern in Cnc/Siemens/SiemensMachine.cs, Cnc/Fagor/FagorMachine.cs
  • Open design question: canonical Mitsubishi FTP filename — bare integer (/PRG/1, symmetric with read) vs ISO title (/PRG/O0001)? Needs real-CNC convention. ResolvePath write branch was designed assuming path is a directory; CLI passes it as a program identifier.
  • Impact: High — CLI Invia broken for all FTP makers
  • Effort: Medium — fix ResolvePath + decide naming convention; verify all 3 makers
  • Related: ISS-013 (Fanuc parsing coupling)

ISS-021: Mazak support — requires EIA/ISO option active on control (Mazatrol out of scope)

  • Discovered: Intake/online research for Mazak integration (2026-06-16)
  • Type: Enhancement constraint / known limitation — pre-implementation
  • Decision: Mazak support will transfer EIA/ISO G-code (plain text) over FTP only, reusing the Mitsubishi/Siemens/Fagor *FtpClient pattern. Target controllers: Smooth family (SmoothG/X/Ai/EZ).
  • Hard requirement — EIA/ISO option: modern Mazatrol controls run both Mazatrol conversational and EIA/ISO G-code, but EIA/ISO is frequently a license-gated option (paid activation) and is NOT guaranteed present on a given machine. If the EIA option is NOT active on the target control, this feature cannot send programs to that machine — only Mazatrol (proprietary binary) would work there, which is out of scope. Operator must verify the EIA/ISO option is enabled on the control before use.
  • Mazatrol CMT out of scope: Mazatrol programs are a proprietary binary format (CMT) with no public PC-side authoring/transfer API. Supporting them would require a licensed vendor library (CIMCO/MazaCAM) or a DNC box (e.g. Mazak LAN Connect, Calmotion LANUSB-MAZ). Explicitly excluded.
  • No FOCAS equivalent: unlike Fanuc (FOCAS fwlib32.dll) and Mitsubishi, Mazak exposes no native DLL/SDK for program up/download. MTConnect / SmartBox / SmartLink are monitoring-only (read), not an upload channel. Transfer is file-drop (FTP/SMB/serial) only.
  • Config variance to expose (not hardcode):
    • FTP port: 21 vs 23 — some Smooth Ai units run FTP via IIS on port 23 because port 21 conflicts with Mazak's own boot/shutdown data-backup. Default 21, configurable.
    • Credentials + NC program directory: undocumented, site-specific (NC dir often surfaced via MC_DIRECT MODE PROGRAMS share or a user-created exchange folder). Configurable, no safe public default.
    • File extension: .eia default (configurable, like Fagor .nc).
  • README note pending: add Mazak row to "Controller supportati" table + a ### Controller Mazak section documenting the EIA-option requirement when the feature is implemented (not before — avoid claiming support that doesn't exist yet).
  • Validator ruleset (VERIFIED online 2026-06-16 vs Mazak Matrix EIA manual H740PB0030E + field reports): Mazak EIA is NOT Fanuc-restrictive. Do NOT reuse the Fanuc/Mitsubishi restrictive gate. Verified facts:
    • Program number = O + up to 8 digits (O[0-9]{1,8}), and is optional — programs may lead with % (EOR) or have no O-line. A 4-digit-required O#### check would reject valid programs.
    • ; is the literal EOB (End-Of-Block) code in Mazak EIA file content — MUST be allowed, not forbidden.
    • [ ] # * = : are valid ISO codes (TAP9TAP14) — allow.
    • ( ) = comment Control-Out/In — allowed; only nested parens are risky.
    • No 80-char block cap (input buffer 248×5). Drop the cap.
    • Lowercase: not in EIA punch-code table → likely insignificant; UNCERTAIN for FTP text upload — verify on real control before hard-reject, do not assume.
    • → Mazak validator must be lenient (optional 18 digit O-header, permissive char set), NOT added to IsRestrictiveDialect().
  • Addressing (verified UNCERTAIN): over FTP a program is a file on a Windows filesystem → addressed by full filename incl. .eia, not bare O-number (control maps file↔work-number internally). Verified: each program/subprogram = separate file named by number sans O (e.g. O1234 content → file 1234.eia); main calls subprograms at runtime via M98 P1234; control links files by name/number at execution, NOT bundled.
  • DECISION (user, 2026-06-16) — preserve filenames verbatim: O#### naming is a Fanuc-only constraint (old-NC name reuse across same family). NOT relevant for Mazak. Therefore:
    • MazakMachine.ResolvePath uses the operator-supplied filename verbatim — no O-number derivation, no rename, no auto-.eia append. Symmetric read/write on the same literal name. MazakConnectionConfig has NO ProgramExtension field.
    • Validator for Mazak = FULL PASSTHROUGHValidate(CncManufacturer.Mazak, …) returns ValidationResult.Success unconditionally. No name check, no content check (FTP sends file as-is). Mazak NOT in IsRestrictiveDialect(), no forbidden-char/header/length rules. Verified Mazak EIA permissiveness (; EOB, [ ], # valid) makes restrictive checks wrong anyway; passthrough is the deliberate choice.
    • Multi-file jobs (main + subprograms) sent as separate transfers, each file keeping its own name.
  • .eia extension + port 21/23 + license-gating: CONFIRMED online (Versicor Smooth Ai FTP article; Practical Machinist EIA-option threads). .eia vs .EIA case + exact NC dir path per Smooth firmware = still site-specific, settle on real machine.
  • Impact: Medium — feature usable only where EIA option licensed; documented limitation, not fixable in code.
  • Effort: Medium — mirrors Fagor phase (machine + FtpClient + config + lenient validator ruleset + factory + tests).
  • Suggested phase: Future milestone v0.8 — Mazak Support.

ISS-022: DeleteProgramAsync raw-path asymmetry (Mitsubishi — same class as ISS-020)

  • Discovered: Mazak implementation review (2026-06-16) — Mazak copied the pattern, then fixed its own copy.
  • Type: Bug (design) — path asymmetry.
  • Description: MitsubishiMachine.DeleteProgramAsync calls _ftp.DeleteFile(path) with the RAW path, bypassing ResolvePath, while ReadProgramAsync/WriteProgramAsync resolve via ResolvePath. So Delete("1") targets 1 (relative) instead of /PRG/1 — asymmetric with read/write, same defect family as ISS-020 (which fixed read/write but left Delete).
  • Files: Cnc/Mitsubishi/MitsubishiMachine.cs DeleteProgramAsync (~line 122). Likely also Siemens/Fagor — verify.
  • Mazak status: FIXED in new code (MazakMachine.DeleteProgramAsync resolves via ResolvePath). Mitsubishi/Siemens/Fagor still open.
  • Impact: Medium — delete-by-identifier hits wrong FTP path; delete fails or no-ops silently. Masked until someone deletes by bare number.
  • Effort: Low — one-line per maker + regression test.
  • Related: ISS-020.

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