buf zero fix for batches

[divi255]

When calling GET_SYMINFO_BYNAME_EX for non-existing variables, TwinCAT servers sometimes respond with no error but with invalid data block instead. This makes fixup_write_read_return_buffers crash.

The suggested quick-fix solves the problem.

[birkenfeld]

Hm, I agree with the continue but zeroing the buffer shouldn't be necessary?

[divi255]

Yep, this looks like a weird construction. But for some internal logic, either yours or Beckhoff's (I didn't dig yet), a failed response buf always contains data from the next one, unless it's the last one.

The same behavior is for any failed AdsSum responses, but in case of others it's possible to check the returned payload for errors with data() method, while GET_SYMINFO_BYNAME_EX sometimes reports no problems.

You might check internal logic of buffer processing, but for the moment I found only the pulled workaround.

[birkenfeld]

Well it is kind of by design that the buffers can contain data from other sub requests (since the buffers have the maximum read size while Beckhoff returns the actual read size). This is what this function is supposed to fix up by moving the contents around. Unfortunately the sumup requests are not documented particularly well by BH.

To diagnose this, it would be very helpful to have a dump of the complete response you get.

[birkenfeld]

In any case, you should get the buffer contents using data() which will truncate the returned slice to the length indicated by Beckhoff. I can make this more apparent in the docs. An example would also be good to have I think.

[divi255]

you mean both fixing the buffer processing with if size == 0 { continue } and using later data() to get a zero-length buf? Sounds reasonable, I will try later today

[birkenfeld]

Actually there should be no change required here. size==0 should be handled correctly by the existing logic, it is also covered in the unit test.

If this is not the case (as you indicated saying the function crashes) I would be interested in the data you got, and the traceback (run in debug mode with RUST_BACKTRACE=full).

[divi255]

I can confirm that the strategy of checking data().len() works perfectly. So I've cut the pull request and kept only continue statement

[birkenfeld]

Merged manually (I removed an unneded semicolon). Thanks!