HI

For the last few days we had a lot of activity with serial comm problems. While (mostly Kevin) was able to improve the situation by allowing more than 1 byte per main loop to be read, reduce the number of sensor cycles or adding delays in MRL send commands, unfortunately I have still been able to raise the errors for first byte in new message is not the magic number.

Taking a step back and swallowing "rock solid communication" I am tempted to ask for a change the protocoll and add a message sequence into it. Messages from MRL would get a sequence number and the Arduino has to commit the "last good message received". On this message MRL can remove the top message in the send queue and send the next one.

This would make sure the Arduino gets all the message in normal mode.

The sequence number could be an additional byte (which will increase all message length's with 1 byte) or it could use the high 3 bits of the message length byte.

I see 2 additional things that have to go with it

1) send messages need a creation time stamp and outdated messages (like older than 2 seconds?) need to get removed.

2) send buffer could get filled up and I suggest a flag to 
a) block the program flow until we get room for new messages
b) throw away messages the do not fit into the send queue

Any other ideas welcome

Juerg

GroG

7 years 11 months ago

Hello Juerg,

Not sure I understand what you are saying, typically, a sequence number guarantees sequence, which is not an issue on single line serial connection. But this would be more apropos to a packet network. For example, TCP has a sequence number used in re-assembly.

MAGIC_NUMBER is currently used as a message boundary, and to a certain degree a valid message check. Messages which do not begin with a MAGIC_NUMBER are thrown away, and the system tries to recover - (or in the past it did)

The timestamp is an interesting idea, but if we are considering adding a new byte to the message, I think I would prefer finally going with a standardized CRC.

Perhaps you should post your test which still generates errors.  Since the comunity has a variety of systems, we should aim for a 'sweet spot' of performance and robustness.

calamity

7 years 11 months ago

Hi juerg

I agree with you that would be a more robust way to communicate, but I think it will also add to the bottleneck we have

I did more test this morning. MRLComm 31 is working well... as long as the arduino does not have to send data. 

Serial.write takes about 1000us to send data, slowing down the main loop and let the arduino unable to read the input buffer quick enough, resulting in a buffer overflow.

So in the current version, sending 10k getVersion still trigger errors, but sending 10k writeDigital (who is not sending back messages to MRL) do not.

If I set the delay in sendMsg to 1-2ms/output I expect, everything work fine

I have try reading 16 analog pins, setting the arduino.delay to 32ms and i did not receive any error when I spam 10K getVersion

 So having the arduino to acknoledge messages as you propose will slow down things even more (adding almost 1ms to the time to process each messages).

So I think the latest version offer everything we need to keep a stable communication. But it would be great if we can increase it's speed even more

Christian

PS: Here the script I use for my test

from datetime import datetime
from time import sleep
arduino = Runtime.createAndStart("arduino","Arduino")
arduino.connect("COM15")
arduino.setLoadTimingEnabled(True)
arduino.delay=0
servo=[]
 
def pinData(pin):
print str(pin.value)+"-"+str(pin.pin)
 
arduino.addListener("publishPin","python","pinData")
for x in range(0,16):
   arduino.analogReadPollingStart(x)
   arduino.delay=arduino.delay+2
#for x in range(0,5):
# servo.append(Runtime.createAndStart("servo"+str(x),"Servo"));
# servo[-1].attach("arduino",2+x)
# servo[-1].setSpeedControlOnUC(true);
# servo[-1].sweep()
# arduino.servoEventEnabled(servo[-1])
# sleep(1)
 
arduino.pinMode(22,Arduino.OUTPUT)
arduino.publishState()
print(datetime.now().time())
for x in range (0,10000):
   arduino.digitalWrite(6,0)
#print str(x)
#sleep(0.001)
print(datetime.now().time())
 
#for ser in servo:
# ser.stop()
 
#arduino.setLoadTimingEnabled(False)

@GroG, I was referring to the phrase "rock solid communication" and throwing away messages is not rock solid in my opinion.

We could lose all kinds of commands if we do not repeat messages that do not arrive in the proper message format.

The serial communication is duplex, so sending and receiving are not depending on each other (at least in theory)

I get incomplete messages when using Kevin's original "getVersion" loop, 10'000 times arduino.sendMsg(2), not many but they show up. And rerunning the script does not work, it's not responding to getVersion messages anymore. Only the inital version sent by the MRLConn.ino itself is shown.

Let's wait for Markus's test scenario, he looks to be the one mostly suffering with his script.

Juerg

 

Mats

7 years 11 months ago

We need to handle at least two types of problems.

1. Data loss.

This should be handled by the protocol. And a protocol is not only a datastructure, but the data structre and the logic, like what to do with a corrput package, and how to get back in sync again. HDLC is an example of such a protocol. http://stackoverflow.com/questions/815758/simple-serial-point-to-point-…

2. Data overflow / flow control.

Flow control is the mechanism that prevents buffer overflow by the receiver having a way to tell the sender that it can't receive more data, or has lost a package. https://en.wikipedia.org/wiki/Flow_control_(data)

I'm not sure if some type of flow control already exists on the hardware side. Otherwise it needs o be implemented in the serial communication to prevent buffer overflow. 

GroG

7 years 11 months ago

In reply to by Mats

Interesting links Mats  !  The HDLC is fairly similar to MRLComm's current implementation.

I have not read it all, but is the framing it does mean 8-N-1 is not required ?  And if so, would the Arduino serial library be compatible or would there be problems ?   8-N-1 is about the most ubiquitous format around, and although its not a protocol - if HDLC expects different, there may be problems.  

The only other mentionable difference I can see is of course the crc.

Thanks for the input Mats.

calamity

7 years 11 months ago

In reply to by Mats

Having a good protocol to transfer the data is alway a good thing, But I think the one we had can do the job. We only need things to stay in control so the buffer won't overrun.

So I think our solution reside in the flow control of the data. And I think it should not be hard to control it.

Over the past few days I was trying to mesure how long each function of the arduino takes to run to try to improve it's total run time of the main loop.

In MRLComm 31, it takes about 160us to loop when there is no message to process and no servo/pins defined, it was more 250us before kevin cut on the SENSORS_MAX and I have a version that could go as fast as 50us.

I have calculate that analog read takes about 150us to read, then about 1000us to do the serialWrite.

getVersion takes about 1000us to do (because it have a serialWrite in it)

digitalWrite takes only a few us.

And it's relatively easy to evaluate the time for each message type processing and each kind of pin polling

The messages MRL send to the arduino have all between 5 and 12 bytes long. let use 5 bytes to simplify this.

If my maths are right, 5 bytes over a 57600 baudrate takes a bit less than 1ms to send. So as long as arduino takes less than 1ms to do the task, we are good, but if it takes longer, message will accumulate in the serial buffer and eventually overflow.

Let me do some example:

on a digitalWrite, it takes 1000us to get the message in, and about 200us to do the task. Processing is faster than message reading so almost nothing could accumulate in the buffer.

on a getVersion, it takes 1000us to get the message in, then about 1150us to do the task. Processing is slower than the message reading and data accumulte in the buffer.

Knowing that it take 150us more to process getVersion than sending the message, if MRL wait 150us (or round up to 1ms) before sending another command, the arduino will have enough time to process the function and won't get in the buffer overflow problem.

If I ask for an analog polling on a pin, each loop to process message now takes 1200us more to do, so adding a delay of 1200us between each messages would prevent the arduino to have loss data to buffer overflow.

I think that way, we can control the flow of data going to the arduino to give it the time to do it's stuff

Christian

I see the problem with getVersion as we receive a short command (3 bytes) and have to respond with a longer answer (4 bytes).

If we do that in a tight loop we will start to fill our send buffer. But that should not impact our read buffer? Instead of sending a single byte we should use Serial.write(buf, len) and not wait for each byte to be transfered? I will give that a try later in the day and let you know.

Thanks Chrisition for your input