Twilight-Logic
commented
3 years ago 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
Thanks for creating this and releasing with a GPL license.
I found changes that improve the ability to send data without needing arbitrary delays.
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.
if not defined (USE_SERIALEVENT) && not defined (SERIALEVENT1) && not defined (SERIALEVENT2) && not defined (SERIALEVENT3)
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.
I had started doing some work on a more complete fix before I found the issues above. I think to allow buffering of multiple commands up to buffer size the parsing needs to be separate from the buffering so we don't loose where the command boundaries are. I was looking at changing the serial interrupt/polling to copy to a buffer with a new routine. Those polls could also be put in the instrument transfer loops to allow the non interrupt version to not loose serial data while sending/receiving data. The main loop would then use a modified parseInput which removes the next command from the buffer for processing.
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 ++?
Was looking at using circular buffer to make it easy to remove the command processed without clearing the entire buffer and not have race conditions with interrupt.
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.
Changes in pull request.