fix(mitsubishi): symmetric read/write FTP path (ISS-020)
WriteProgramAsync resolved -path as a directory and combined it with
program.Name ("1" + "O0001" -> "1/O0001"), while ReadProgramAsync
resolved it as a program identifier under ProgramDirectory ("1" ->
"/PRG/1"). The write target therefore never matched the read target:
STOR went to a nonexistent relative dir and failed with "unable to
connect to remote server" (-204). Surfaced once ISS-018/019 stopped
masking it.
Per Mitsubishi NC Explorer manual IB-1500904, programs live under
PRG/USER and are addressed by their program name/number with no
extension. Resolve write the same way as read: ResolvePath is now
single-arg (path = identifier or full path, joined with
ProgramDirectory). program.Name is used for validation only.
Update write unit test to the symmetric contract and add a roundtrip
regression test (-path "1" -> /PRG/1).
Siemens/Fagor share the pattern but tie extension handling to name
(entangled with ISS-013); left open in ISS-020.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
85d0b579aa
commit
4067366d06
3 changed files with 56 additions and 12 deletions
|
|
@ -208,6 +208,26 @@ Enhancements and known issues discovered during execution and codebase analysis.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
### 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)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
## Closed Issues
|
## Closed Issues
|
||||||
|
|
||||||
| ID | Summary | Phase | Commit |
|
| ID | Summary | Phase | Commit |
|
||||||
|
|
|
||||||
|
|
@ -70,7 +70,7 @@ namespace NcProgramManager.Cnc.Mitsubishi
|
||||||
return RunGuardedAsync(() =>
|
return RunGuardedAsync(() =>
|
||||||
{
|
{
|
||||||
EnsureConnected();
|
EnsureConnected();
|
||||||
string ncPath = ResolvePath(path, null);
|
string ncPath = ResolvePath(path);
|
||||||
try { return _ftp.DownloadFile(ncPath); }
|
try { return _ftp.DownloadFile(ncPath); }
|
||||||
catch (Exception ex)
|
catch (Exception ex)
|
||||||
{
|
{
|
||||||
|
|
@ -99,7 +99,12 @@ namespace NcProgramManager.Cnc.Mitsubishi
|
||||||
_errors.Add(new CncError(0, "Validator", msg));
|
_errors.Add(new CncError(0, "Validator", msg));
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
string ncPath = ResolvePath(path, program.Name);
|
// ISS-020: resolve the write path the same way ReadProgramAsync does.
|
||||||
|
// `path` is the program identifier (e.g. "1" -> /PRG/1) or a full FTP
|
||||||
|
// path, NOT a directory to combine with program.Name. On Mitsubishi the
|
||||||
|
// file in PRG/USER is addressed by its program name/number; program.Name
|
||||||
|
// (the ISO O-number) is used for validation only, not the filename.
|
||||||
|
string ncPath = ResolvePath(path);
|
||||||
try { _ftp.UploadFile(ncPath, program.Content); return true; }
|
try { _ftp.UploadFile(ncPath, program.Content); return true; }
|
||||||
catch (Exception ex)
|
catch (Exception ex)
|
||||||
{
|
{
|
||||||
|
|
@ -199,15 +204,16 @@ namespace NcProgramManager.Cnc.Mitsubishi
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>Build full FTP path. Mitsubishi programs have no extension.</summary>
|
/// <summary>
|
||||||
private string ResolvePath(string folder, string name)
|
/// Build full FTP path. Mitsubishi programs have no extension and are addressed
|
||||||
|
/// by their program name/number under <see cref="MitsubishiConnectionConfig.ProgramDirectory"/>.
|
||||||
|
/// A bare identifier ("1") is combined with the program directory ("/PRG/1");
|
||||||
|
/// an explicit path containing '/' is used as-is. Read and write resolve identically.
|
||||||
|
/// </summary>
|
||||||
|
private string ResolvePath(string path)
|
||||||
{
|
{
|
||||||
if (string.IsNullOrEmpty(name))
|
if (string.IsNullOrEmpty(path)) return _config.ProgramDirectory;
|
||||||
return folder.Contains("/") ? folder : _config.ProgramDirectory + folder;
|
return path.Contains("/") ? path : _config.ProgramDirectory + path;
|
||||||
|
|
||||||
string dir = folder ?? _config.ProgramDirectory;
|
|
||||||
if (!dir.EndsWith("/")) dir += "/";
|
|
||||||
return dir + name;
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -96,20 +96,38 @@ namespace NcProgramManager.Tests.Unit
|
||||||
// WriteProgramAsync
|
// WriteProgramAsync
|
||||||
|
|
||||||
[Test]
|
[Test]
|
||||||
public void WriteProgramAsync_ValidContent_CallsUploadFileNoExtension()
|
public void WriteProgramAsync_FullPath_CallsUploadFileNoExtension()
|
||||||
{
|
{
|
||||||
|
// ISS-020: write resolves the path identically to read (path = program
|
||||||
|
// identifier / full path, not a directory combined with program.Name).
|
||||||
_ftpMock.Setup(f => f.Ping()).Returns(true);
|
_ftpMock.Setup(f => f.Ping()).Returns(true);
|
||||||
_ftpMock.Setup(f => f.UploadFile("/PRG/O1234", "CONTENT"));
|
_ftpMock.Setup(f => f.UploadFile("/PRG/O1234", "CONTENT"));
|
||||||
_machine.ConnectAsync().GetAwaiter().GetResult();
|
_machine.ConnectAsync().GetAwaiter().GetResult();
|
||||||
|
|
||||||
var prog = new CncProgram { Name = "O1234", Content = "CONTENT" };
|
var prog = new CncProgram { Name = "O1234", Content = "CONTENT" };
|
||||||
var result = _machine.WriteProgramAsync("/PRG/", prog).GetAwaiter().GetResult();
|
var result = _machine.WriteProgramAsync("/PRG/O1234", prog).GetAwaiter().GetResult();
|
||||||
|
|
||||||
Assert.That(result.Success, Is.True);
|
Assert.That(result.Success, Is.True);
|
||||||
_ftpMock.Verify(f => f.UploadFile("/PRG/O1234", "CONTENT"), Times.Once);
|
_ftpMock.Verify(f => f.UploadFile("/PRG/O1234", "CONTENT"), Times.Once);
|
||||||
_ftpMock.Verify(f => f.UploadFile(It.Is<string>(s => s.EndsWith(".MPF")), It.IsAny<string>()), Times.Never);
|
_ftpMock.Verify(f => f.UploadFile(It.Is<string>(s => s.EndsWith(".MPF")), It.IsAny<string>()), Times.Never);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Test]
|
||||||
|
public void WriteProgramAsync_ShortName_ResolvesWithDirectory_SymmetricWithRead()
|
||||||
|
{
|
||||||
|
// ISS-020 regression: -path "1" must write to ProgramDirectory + "1"
|
||||||
|
// (the same location ReadProgramAsync("1") reads), so roundtrip works.
|
||||||
|
_ftpMock.Setup(f => f.Ping()).Returns(true);
|
||||||
|
_ftpMock.Setup(f => f.UploadFile("/PRG/1", "CONTENT"));
|
||||||
|
_machine.ConnectAsync().GetAwaiter().GetResult();
|
||||||
|
|
||||||
|
var prog = new CncProgram { Name = "O0001", Content = "CONTENT" };
|
||||||
|
var result = _machine.WriteProgramAsync("1", prog).GetAwaiter().GetResult();
|
||||||
|
|
||||||
|
Assert.That(result.Success, Is.True);
|
||||||
|
_ftpMock.Verify(f => f.UploadFile("/PRG/1", "CONTENT"), Times.Once);
|
||||||
|
}
|
||||||
|
|
||||||
[Test]
|
[Test]
|
||||||
public void WriteProgramAsync_EmptyContent_FailsWithoutCallingUploadFile()
|
public void WriteProgramAsync_EmptyContent_FailsWithoutCallingUploadFile()
|
||||||
{
|
{
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue