Issues with serial data buffering

[twilight-logic]

Issues with serial data buffering

[Twilight-Logic]

Thank you for taking the time to submit this detailed list of proposed modifications.

#if used or of not defines so if any of the USE_SERIAL weren't defined it activated the polling code. Since normally only one is defined the polling was always active.

I take your point here and this will be changed in the next release. While reviewing this point I also noticed that the code does not take into account the instance of the Mega where USE_SERIALEVENT1, USE_SERIALEVENT2 or USE_SERIALEVENT3 might be used. The update will take care of these possibilities.

And this didn't terminate the loop when it found a command so if two commands were sent close enough together they would be concatenated and it would try to parse them as one command. I made it exit and process the command when found.

while (arSerial->available() && lnRdy == 0) {

This gets it to the state for me on an Uno board that I can send ++ commands without delays and they are correctly processed for the commands I tried.

With these changes I can send another instrument command while the current is processing without delay but can't send more than that without it getting corrupted.

This is an interesting one and I understand what your modification is doing. The parser is one of the most difficult parts to process. It was written so that it wouldn't matter whether there was one command or multiple in the buffer - it should still split them up, but I have occasionally observed the problem you mention. On the other hand, if you add && lnRdy = 0, then there is a problem that while lnRdy > 1 (i.e. the previous command is still being processed) then the serial buffer is then blocking which leads to characters being lost and the corruption problems that you observed. The buffer is designed to read up to a CR and pass those characters to the command processor. It then reads up to the next CR and passes that command to the command processor and so on until all characters have been processed, then the buffer is cleared and we start over. I have spent hourse on this problem alone, however bearing in mind your suggestion, will spend some more time on it. I welcome any ideas that might improve this situation.

This does complicate readBreak. I wasn't sure how that was supposed to work. Should read exit if anything is received, only if ++ is received, what about if a non ++ command is received before a ++?

Initially there were maybe 4 conditions where read was supposed to break. At present is should break when a '++' has been received indicating a controller command. The other condtion is in device mode where an ATN is received. At present the gpibReceiveData() routine handles both controller and device mode, but I am considering restructuring this routine as it has become rather messy.

I also removed this line since it was unlikely to be doing anything useful. If it needs to check those three characters it needs to be three separate comparisons.
Currently its checking for *buffr == 1 since (0 || 10 || 13) == 1
//if (*buffr == (NULL || CR || LF) ) return; // empty line: nothing to parse.

The purpose of that if the buffer was empty, contained a CR or LF as the first character, then since a blank line was submitted the routine should immediately exit without further processing. As it is, the code is erroneous, so thank you for pointing it out. That will be amended in the next release to have 3 comparisons as you suggest:

if (buffr[0] == NULL) return;
if (buffr[0] == CR) return;
if (buffr[0] == LF) return;

Thank you for pointing that out.

I need a little time to work on the gpibReceiveData() routine and once I have that done to my satisfaction I will release an update.

[dgesswein]

Thank you for taking the time to submit this detailed list of proposed modifications.

I have an open source project so submitted what I would like to get. I have AR488 to a point where it seems pretty stable to me and I don't need to sprinkle the code with sleeps to make it work so more improvements are nice but not critical for me. I can look at implementing some of what I discussed if its a direction you wish to go.

This is an interesting one and I understand what your modification is doing. The parser is one of the most difficult parts to process. It was written so that it wouldn't matter whether there was one command or multiple in the buffer - it should still split them up, but I have occasionally observed the problem you mention. On the other hand, if you add && lnRdy = 0, then there is a problem that while lnRdy > 1 (i.e. the previous command is still being processed) then the serial buffer is then blocking which leads to characters being lost and the corruption problems that you observed. The buffer is designed to read up to a CR and pass those characters to the command processor. It then reads up to the next CR and passes that command to the command processor and so on until all characters have been processed, then the buffer is cleared and we start over. I have spent hourse on this problem alone, however bearing in mind your suggestion, will spend some more time on it. I welcome any ideas that might improve this situation.

I was assuming ++ commands execute quick enough that you can process one before the serial starts loosing characters. With interrupt serial I think you are ok since it should still read new characters. Will polled serial I don't know. Most ++ commands should be processed fast enough but I think you hade some that write to the non volatile memory that may take a while. To make polled serial not loose character would need to add the polling to any location that takes a long time. Having the character receive routine do the parsing into commands makes handling multiple commands more difficult. Have it just buffer the data and do the parsing when pulling the data from the buffer may simplify the parsing but readBreak would need to do more work to find ++ commands in the buffer.

…

I need a little time to work on the gpibReceiveData() routine and once I have that done to my satisfaction I will release an update.

[Twilight-Logic]

I have been aware for some time that serialEvent() is not attached to an actual hardware interrupt although I have researched this in detail only recently. The event is executed at the end of each cycle of void loop() and an examination of the Arduino code shows that it is equivalent to placing Serial.available() at the end of the loop. Given that some boards do not support serialEvent, the coding was a bit of a cludge and I have been wanting to remove any reference to it for some time and unify the code across all boards. I have been reliably informed that doing so would have no impact on performance as it is doing exactly the same thing.

I have placed an updated AR488.ino in the GitHub repository under /experimental for you to try. In this update, serialEvent has been removed altogether and replaced it with a serial handler that is placed at the end of void loop(). This makes the coding that was previously associated with USE_SERIALEVENT completely redundant.

I have also removed the USE_PINHOOKS define, as the presence of abscence of the USE_INTERRUPTS define is sufficient to determine whether the state of the ATN and SRQ pins needs to be polled within void loop() or handled with interrupts.

Finally, I have removed the readBreak() function and the tranBrk variable completely. This had gotten a bit messy over time and it getting difficult to follow what was going on. You will see that this is being handled differently in the updated version.

For now, I have not made any changes to the parser buffer.

If you have time to try it, I would be interested to know how this update it performs. If it works OK then I will issue it as a formal update. If not, then hopefully the experiment will yield some useful information!

Theoretically there should be no performance difference. I have also included some of your modifications including the erroneous line in the parser. Thank you your contribution. This update worked fine in my test environment although I have not yet tested everything. Please let me know how you get on with it.

Regards.

[dgesswein]

You have this warning. /home/djg/gpib/AR488/src/AR488/AR488.ino:1076:19: warning: NULL used in arithmetic [-Wpointer-arith] if (buffr[0] == NULL) return; ^ Should replace NULL with 0 since its not comparing to a pointer. I did some testing with my DVM. It seems like back to back ++ commands work fine. Back to back GPIB commands don't. My code ran fine as long as I sent one command at a time. Let me know if you and any specific tests I should do.

…

On Tue, Dec 22, 2020 at 06:27:47AM -0800, Twilight-Logic wrote: I have been aware for some time that serialEvent() is not attached to an actual hardware interrupt although I have researched this in detail only recently. The event is executed at the end of each cycle of void loop() and an examination of the Arduino code shows that it is equivalent to placing Serial.available() at the end of the loop. Given that some boards do not support serialEvent, the coding was a bit of a cludge and I have been wanting to remove any reference to it for some time and unify the code across all boards. I have been reliably informed that doing so would have no impact on performance as it is doing exactly the same thing. I have placed an updated AR488.ino in the GitHub repository under /experimental for you to try. In this update, serialEvent has been removed altogether and replaced it with a serial handler that is placed at the end of void loop(). This makes the coding that was previously associated with USE_SERIALEVENT completely redundant. I have also removed the USE_PINHOOKS define, as the presence of abscence of the USE_INTERRUPTS define is sufficient to determine whether the state of the ATN and SRQ pins needs to be polled within void loop() or handled with interrupts. Finally, I have removed the readBreak() function and the tranBrk variable completely. This had gotten a bit messy over time and it getting difficult to follow what was going on. You will see that this is being handled differently in the updated version. For now, I have not made any changes to the parser buffer. If you have time to try it, I would be interested to know how this update it performs. If it works OK then I will issue it as a formal update. If not, then hopefully the experiment will yield some useful information! Theoretically there should be no performance difference. I have also included some of your modifications including the erroneous line in the parser. Thank you your contribution. This update worked fine in my test environment although I have not yet tested everything. Please let me know how you get on with it. Regards. -- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: #12 (comment)

[Twilight-Logic]

Thank for pointing out the warning. For some reason the 'Compiler Warnings' setting in my Arduino client preferences were set to 'None' so I didn't pick this up. I usually like to have it set to 'All' to make sure I pick up all the warnings. Not sure how/why this changed (update perhaps?) but I can confirm now that I got the same warning and appreciate your reporting this.

Could you elaborate a bit more on what happens when you send multiple GPIB commands? What is the sequence you are sending? What happens? Any errors or timeouts? Except for looking for a terminator, the interface makes no attempt to process non ++ commands and just sends the characters already in the buffer directly to the instrument whenever a terminator is detected. For some direct instrument commands it may also take some time for the instrument to respond. That may or may not be the issue here, so any further information you can give may be helpful.

In the meantime I have uploaded an update that deals with the error you mentioned.

[dgesswein]

The issue was the test equipment turned on the error indicator saying it got a bad command. I should be able to look into it further to determine what is really going wrong in the next couple days.

…

On Wed, Dec 23, 2020 at 05:15:56AM -0800, Twilight-Logic wrote: Thank for pointing out the warning. For some reason the 'Compiler Warnings' setting in my Arduino client preferences were set to 'None' so I didn't pick this up. I usually like to have it set to 'All' to make sure I pick up all the warnings. Not sure how/why this changed (update perhaps?) but I can confirm now that I got the same warning and appreciate your reporting this. Could you elaborate a bit more on what happens when you send multiple GPIB commands? What is the sequence you are sending? What happens? Any errors or timeouts? Except for looking for a terminator, the interface makes no attempt to process non ++ commands and just sends the characters already in the buffer directly to the instrument whenever a terminator is detected. For some direct instrument commands it may also take some time for the instrument to respond. That may or may not be the issue here, so any further information you can give may be helpful. In the meantime I have uploaded an update that deals with the error you mentioned. -- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: #12 (comment)

[dgesswein]

If I send each of these strings as separate back to back writes "++addr 22\n"; "++verbose\n"; "INBUF ON\n"; "END ALWAYS\n"; "TERM 2\n"; the output is Verbose: ON ^

INBUF ON END ALWAYSTERM 2

If I send "++addr 22\n"; "++verbose\n"; "INBUF ON\n"; "END ALWAYS\n"; "++eoi\n"; I get Verbose: ON

…

INBUF ON END ALWAYS++eoi

[Twilight-Logic]

Thank you for the update. I presume you are sending these using your own script?
I tested with a simple copy and paste (after removing superfluous quotes and \n characters) from a text editor and also got lines running into each other as illustrated with your examples, so the problem is confirmed. I will investigate.

[dgesswein]

Yes, I'm using C code to talk to the serial port. Thats why the " and \n.

…

On Fri, Jan 08, 2021 at 06:51:09AM -0800, Twilight-Logic wrote: Thank you for the update. I presume you are sending these using your own script? I tested this with a simple copy and paste (after removing superfluous quotes and \n characters) and got a similar result with lines running into each other so the problem is confirmed. I will investigate.

[Twilight-Logic]

I think I have solved that problem with the commands running into each other. There was a bug with the way that the new serial handling routine was implemented. I have uploaded an updated AR488.ino into the experimental directory for you to try.

[dgesswein]

Looks good to me. I was able to send 5 commands back to back without problem. I assume the USB serial doesn't have flow control so sending more than that I am just overrunning the buffer. I have deleted my fork since your code is now working better for me. Thanks

…

On Sun, Jan 10, 2021 at 05:57:39AM -0800, Twilight-Logic wrote: I think I have solved that problem with the commands running into each other. There was a bug with the way that the new serial handling routine was implemented. I have uploaded an updated AR488.ino into the experimental directory for you to try. -- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: #12 (comment)

[Twilight-Logic]

Thank you for confirming the update works. You are correct that on Arduinos there is no flow control on the serial port. I believe thst some aspects of hardware flow control may be supported on genuine Arduinos but definitely not on the cheap clones with CH340 chips. The built-in Arduino serial buffer is 64 bytes in size and as you will already be aware, due in large part to memory limitations, the parsing buffer in the AR488 code is 128 bytes. MCU processing is significantly faster than serial transfer speeds so the Arduino can keep up quite well, but it should be bourne in mind that if data comes in faster than the Arduino can handle, then the buffer may overflow which will result in the loss of at least some data. It might be possible at some point to implement a software flow control mechanism e.g. xon/xoff, but i haven't experimented with that yet.

[Twilight-Logic]

Fixed in version 0.49.12.
Implementing a cyclical buffer is being considered for a future release.