Handle errors from the read/write method

[DaveDavenport]

It looks like currently the return value from the read/write method (in scpi_interface_t) is not taken into account.

My use case:

I made an interface to a TCP socket, this all works. But when f.e. the connection gets disconnected while sending (and I return -1 in the write method), the scpi-parser keeps trying to send data like nothing happened.

I think it would be nice if it would handle this more gracefully, so I can recover from it more gracefully at a higher level.

I am on commit: 4e879901b51cbb43dab36dd83f95a23f1dbaa4c0

[jancumps]

Dave, what would the higher level (the SCPI engine) have to do in that case? Because we're implementing all inputs and outputs in our own code (in your case: integration with TCP sockets), would it work to develop the communication error handling in that layer too?

[DaveDavenport]

I need to look into it in a bit more details.

At the moment I saw a lot of calls to the write method even after it errored out and I returned -1 (it got aborted in the middle of returning a large array of sample points).

I have not looked at how buffering is done in the library. I am also not sure what to do with the return value in the write method?

If I write 10 out of 20 bytes do I need to block until all 20 are written? or do I return 10 and then I get called again with the next 10 bytes?

In general (from orther libs that have a custom read/write method) I would expect the following:

• I push in data as I receive it. the library either takes the whole buffer or part of it and tells me how much it consumed.
• I write data and return how much data I've written. If I written less then the buffer, I get called again with the remainder or I always send the whole buffer (blocking) and return success/failure.
• If I fail to write data I return an error (-1?) and the library goes into an error mode. How the error is handled (f.e. flush outstanding buffers and return to 'idle' operation) is up to the user (within the capabilities of the library).

I probably take a better look at this in the next few weeks.

[jancumps]

The tcp example writes 0 (not -1) if no values could be written.

[DaveDavenport]

Is that taken into account?
If I look here: https://github.com/j123b567/scpi-parser/blob/master/libscpi/src/parser.c#L134 it does not seems to do anything with the result. Also in other methods, it sums it up, but does not handle the result. (e.g. try resend/error out, etc.).

[jancumps]

that line above is to take care to send a separator back before the reply, except if it's the first reply. There's no use for the return value in that situation - no upstream call to give it to.

The SCPI lib handles SCPI errors - and allows you push your your own to the scpi error stack.

It doesn't assume or do anything in the communication layer. I think it should also not assume a right way to handle errors in that layer.

[DaveDavenport]

I am confused.

If I fail to write data I want a way to stop the library from trying to push out data.

At the moment it seems to keep going as nothing is wrong. (and if in the middle or a large array this can be a lot of calls).

I could (in my interface function) ignore all followup calls and just discard the data? is this the way to 'handle errors'?

I would expect that if I return an error, the loop (at a higher level) will break out, and this will propagate back up.

[jancumps]

I think that if you could define

• what the scpi lib should do, in case the communication layer flags it is in an error state and not able to write data,
• what it should do with the (maybe lots of?) data while the error status is active, and
• what it should do after that error state is resolved,

it could be turned into code.

[jancumps]

thought: an other option would be to flag the comms error the part of your design that generates that data. It can then stop pushing data to the SCPI lib.

[DaveDavenport]

I mostly want loops to break out, this is what I currently run in my copy.

(return value is unsigned, so check against 0 for now).

--- a/libscpi/src/parser.c
+++ b/libscpi/src/parser.c
@@ -1613,7 +1613,9 @@ static size_t produceResultArrayBinary(scpi_t * context, const void * array, siz
     if (format == SCPI_FORMAT_ASCII) {\
         size_t i;\
         for (i = 0; i < count; i++) {\
-            result += func(context, array[i]);\
+           size_t res = func(context, array[i]);\
+           if ( res == 0 ) { break; }\
+            result += res;\
         }\
     } else {\
         result = produceResultArrayBinary(context, array, count, sizeof(*array), format);\

This for me improves things a lot already, as it just aborts the write , makes it that I quicker return from my higher level call and I can start handling the disconnect correctly.

(hope I did not make typo, I just quickly tried to redo the patch quickly in one point, as I cannot test it right now and not at pc with the source)

[jancumps]

I see what you're doing there. That will work to make the functions that return arrays stop faster. It has to be validated that none of the possible (func) can return 0 in a valid case ...

[DaveDavenport]

Yes, I want 'errors' to propagate as quickly as possible to the top level method, so I can start handling the disconnect/error/etc (the interface callback functions should be as simple/short as possible imho.).

I can try to make a PR later (next weeks), if this is acceptable.