[BUG]coCognexVision.TcoDataman_v_5_x_x Read should not prodduce error when data were not read

[github-actions]

closes #46

[Copilot]

Pull request overview

Adjusts the Dataman v_5_x_x read task so that a “no-read” does not automatically transition the task into an error state (Issue #46), while still reacting to genuine acquisition/device errors.

Changes:

• Gates result copying on ResultCode success and avoids throwing on ResultCode=0 / ResultLength=0.
• Adds additional ThrowWhen checks for MissedAcquisition and ErrorDetected.
• Clears IO data on “read not successful” (current implementation).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

MEMCPY uses n := inoResults.Length without any bounds check against the source buffer (inoIoData.ResultData.ResultData, 246 bytes) and destination buffer (inoResults.Data, 256 bytes). If ResultLength is larger than either buffer, this will read/write out of bounds and the subsequent SIZEOF(...) - Length can underflow. Clamp inoResults.Length to MIN(ResultLength, SIZEOF(src), SIZEOF(dst)) (and handle truncation explicitly).

[Copilot]

In the “read not successful” path, inoResults is not cleared. Because the task now completes successfully even when no data was read, callers can observe stale inoResults.Data from a previous successful read. Consider explicitly setting inoResults.Length := 0 and clearing inoResults.Data (and/or setting a status flag) when ResultCode indicates no-read.

Suggested change

	ELSIF THIS^.inoIoData.ResultData.ResultCode.0=0 THEN // read not successfull
	ELSIF THIS^.inoIoData.ResultData.ResultCode.0=0 THEN // read not successfull
	// Clear results to avoid exposing stale data when read was not successful
	THIS^.inoResults.Length := 0;
	Tc2_System.MEMSET(destAddr:= ADR(THIS^.inoResults.Data), fillByte:= 0, n:= SIZEOF(THIS^.inoResults.Data));

[Copilot]

Tc2_System.MEMSET(ADR(inoIoData), SIZEOF(inoIoData)) clears the entire IO struct, including mapped %Q outputs (PnIoBoxCtrl, AcquisitionControl, ResultsControl, etc.) and %I inputs. This can unintentionally reset device control signals (e.g., TriggerEnable) and also masks real input status values in the process image. Prefer clearing only the result-related fields (e.g., inoIoData.ResultData / inoResults) rather than zeroing the whole IO structure.

Suggested change

	Tc2_System.MEMSET(destAddr:= ADR(THIS^.inoIoData) , fillByte:= 0 , n:= SIZEOF(THIS^.inoIoData));
	Tc2_System.MEMSET(destAddr:= ADR(THIS^.inoIoData.ResultData), fillByte:= 0, n:= SIZEOF(THIS^.inoIoData.ResultData));
	Tc2_System.MEMSET(destAddr:= ADR(THIS^.inoResults), fillByte:= 0, n:= SIZEOF(THIS^.inoResults));

[Copilot]

The success criteria here (ResultCode.0=1) is inconsistent with the rest of the Dataman implementation, e.g. TcoDataman_v_5_x_x.ContinuousReading uses ResultCode > 0 AND ResultLength > 0 (TcoDataman_v_5_x_x.TcPOU:160). If ResultCode can be non-zero but not have bit0 set, this task will treat it as “not successful” and skip copying. Consider aligning the condition (or document why bit0 specifically is required).

Suggested change

	IF THIS^.inoIoData.ResultData.ResultCode.0=1 THEN // read successfull
	IF Tc2_System.MEMCPY(srcAddr := ADR(THIS^.inoIoData.ResultData.ResultData) , destAddr:= ADR(THIS^.inoResults.Data) , n:= THIS^.inoResults.Length) > 0 AND
	Tc2_System.MEMSET(destAddr:= ADR(THIS^.inoResults.Data) + THIS^.inoResults.Length, fillByte:= 0 , n:= SIZEOF(THIS^.inoResults.Data) - THIS^.inoResults.Length)>0 THEN
	CallTimers(FALSE);
	_progress := _progress + 1;
	END_IF;
	ELSIF THIS^.inoIoData.ResultData.ResultCode.0=0 THEN // read not successfull
	IF (THIS^.inoIoData.ResultData.ResultCode > 0) AND (THIS^.inoIoData.ResultData.ResultLength > 0) THEN // read successful
	IF Tc2_System.MEMCPY(srcAddr := ADR(THIS^.inoIoData.ResultData.ResultData) , destAddr:= ADR(THIS^.inoResults.Data) , n:= THIS^.inoResults.Length) > 0 AND
	Tc2_System.MEMSET(destAddr:= ADR(THIS^.inoResults.Data) + THIS^.inoResults.Length, fillByte:= 0 , n:= SIZEOF(THIS^.inoResults.Data) - THIS^.inoResults.Length)>0 THEN
	CallTimers(FALSE);
	_progress := _progress + 1;
	END_IF;
	ELSIF (THIS^.inoIoData.ResultData.ResultCode <= 0) OR (THIS^.inoIoData.ResultData.ResultLength = 0) THEN // read not successful

[Copilot]

Spelling in the inline comments: “successfull” should be “successful” (also in “not successfull”).

Suggested change

	IF THIS^.inoIoData.ResultData.ResultCode.0=1 THEN // read successfull
	IF Tc2_System.MEMCPY(srcAddr := ADR(THIS^.inoIoData.ResultData.ResultData) , destAddr:= ADR(THIS^.inoResults.Data) , n:= THIS^.inoResults.Length) > 0 AND
	Tc2_System.MEMSET(destAddr:= ADR(THIS^.inoResults.Data) + THIS^.inoResults.Length, fillByte:= 0 , n:= SIZEOF(THIS^.inoResults.Data) - THIS^.inoResults.Length)>0 THEN
	CallTimers(FALSE);
	_progress := _progress + 1;
	END_IF;
	ELSIF THIS^.inoIoData.ResultData.ResultCode.0=0 THEN // read not successfull
	IF THIS^.inoIoData.ResultData.ResultCode.0=1 THEN // read successful
	IF Tc2_System.MEMCPY(srcAddr := ADR(THIS^.inoIoData.ResultData.ResultData) , destAddr:= ADR(THIS^.inoResults.Data) , n:= THIS^.inoResults.Length) > 0 AND
	Tc2_System.MEMSET(destAddr:= ADR(THIS^.inoResults.Data) + THIS^.inoResults.Length, fillByte:= 0 , n:= SIZEOF(THIS^.inoResults.Data) - THIS^.inoResults.Length)>0 THEN
	CallTimers(FALSE);
	_progress := _progress + 1;
	END_IF;
	ELSIF THIS^.inoIoData.ResultData.ResultCode.0=0 THEN // read not successful

[Copilot]

The new unconditional ThrowWhen(...) checks dereference inoIoData even when _progress is 0 and __ISVALIDREF(inoIoData) is false (the task does not early-return after pinning the invalid-reference message). This can cause a runtime fault when the FB is invoked with an invalid VAR_IN_OUT reference. Guard these ThrowWhen calls with __ISVALIDREF(inoIoData) (or execute them only after the reference-validation block has passed).

Suggested change

	THIS^.ThrowWhen(THIS^.inoIoData.AcquisitionStatus.MissedAcquisition );
	THIS^.ThrowWhen(THIS^.inoIoData.ResultsStatus.ErrorDetected);
	IF __ISVALIDREF(inoIoData) THEN
	THIS^.ThrowWhen(THIS^.inoIoData.AcquisitionStatus.MissedAcquisition );
	THIS^.ThrowWhen(THIS^.inoIoData.ResultsStatus.ErrorDetected);
	END_IF