next topic »

[Reply #15 on:]

There could very well be bugs so let me know if you find a slowdown problem.  I only put two days into this so far.  I haven't done much testing.

I put this code out so I could get comments while I am early in development.  I really appreciate suggestions like robtillaart's suggestion to add parity, character size, and stop-bit options.  Also to return error information for parity, framing, receiver overruns, and buffer overruns.

OK. It's taken a while to isolate the problem. It seems that creating the second serial port is causing all the problems. This simple program demonstrates the problem. I've no idea what's causing it but if you don't add the SerialPort<1, 16, 16> but it's fine. No idea if the parameters are important. Each loop should report either 0us or 4us. However after the first loop, it reports 156us each time. I'm guessing something in an interrupt is eating all my cycles.

Code:

#include <SerialPort.h>

static SerialPort<0, 0, 256> serialPort;
static SerialPort<1, 16, 16> serialPort1;

void setup(void)
{
SerialPort<0, 0, 256> serialPort;

serialPort.begin(115200);
serialPort1.begin(115200);
for (int route = 0; route < 11; route++)
{
uint32_t start, stop;

start = micros();
stop = micros();
serialPort.print(stop - start, DEC);
serialPort.println("us");
delay(400);
}
}

void loop(void)
{
}

Hope that helps.

Iain

[Reply #16 on:]

Looks like I didn't clean up the code correctly. Remove the SerialPort<> declaration inside setup().

Here's the cleaner version:

Code:

#include <SerialPort.h>

static SerialPort<0, 0, 256> serialPort;
static SerialPort<1, 16, 16> serialPort1;

void setup(void)
{
serialPort.begin(115200);
serialPort1.begin(115200);
for (int route = 0; route < 11; route++)
{
uint32_t start, stop;

start = micros();
stop = micros();
serialPort.print(stop - start, DEC);
serialPort.println("us");
delay(400);
}
}

void loop(void)
{
}

Iain

[Reply #17 on:]

I run your example with the version SerialPortBeta20111230.zip on a Mega and get this??

4us
0us
0us
4us
4us
4us
4us
0us
4us
0us
4us

[Reply #18 on:]

I've run it on two different Mega's and that's not what I get. :-((

[Reply #19 on:]

Just run it again and it's not going wrong here either. I'll have another look in the morning.

Iain

[Reply #20 on:]

Do you have anything connected to port1?  If nothing is connected, you could get really high speed framing errors on port1 from noise.

I put a wire in RX1 and get your result and the RX1 buffer is full of junk.

You could try connecting RX1 to 5V so it appears to be connected to an idle port.  Serial ports idle HIGH.

Edit: There is a bug when the RX1 buffer is full.  I don't clear the interrupt when I can't buffer the character.

Here is a fix for the receive ISR:

Code:

//------------------------------------------------------------------------------
static  void rx_isr(volatile uint8_t* udr, SerialRingBuffer* pb) {
  uint8_t b = *udr;
  uint16_t tmp = pb->head + 1;
  if (tmp >= pb->size) tmp = 0;
  if (tmp != pb->tail) {
    pb->buf[pb->head] = b;
    pb->head = tmp;
  }
}

I posted the fixed version as SerialPortBeta20111231.zip here http://code.google.com/p/beta-lib/downloads/list.

[Reply #21 on:]

Do you have anything connected to port1?  If nothing is connected, you could get really high speed framing errors on port1 from noise.

I put a wire in RX1 and get your result and the RX1 buffer is full of junk.

You could try connecting RX1 to 5V so it appears to be connected to an idle port.  Serial ports idle HIGH.

Edit: There is a bug when the RX1 buffer is full.  I don't clear the interrupt when I can't buffer the character.

That would explain my issue. Another Mega is connected to serial port 1 and it streams data continuously. So when this Mega starts up, before I can do anything the port will be full. I'll check out the fix when I power up the complete system and report back.

Thanks for your efforts. Glad you could figure it out from my incomplete ramblings. I hadn't realised the importance of my system since I was seeing timing issues on Serial0, and only discovered that Serial1 was required well after midnight.

Iain

[Reply #22 on:]

Thanks for the fix. It's all working correctly now.

Iain

[Reply #23 on:]

I'd like to know if it's possible to have a faster method to upload a string to the the transmit buffer.

In my system I have a maximum processing time of 390us. This all works well but when debugging, the amount of text output causes the timing to be exceeded. I was hoping that a larger output buffer would help. It's definitely an improvement over HardwareSerial.

I've been testing with a 29 character message and HardwareSerial took 892us to send the message. Using SerialPort with a transmit buffer size of 32 bytes it's down to 168us. However just doing a memory copy takes 16us.

So I've been wondering if there's a way to allow the SerialPort class to have a fast copy into the transmit buffer?

Code:

#include <SerialPort.h>

SerialPort<0, 0, 32> port;

void setup(void)
{
port.begin(115200);
for (int route = 0; route < 11; route++)
{
uint32_t start;

start = micros();
DebugPrintLine("Selecting passenger route x");
ShowMessageTime(start, micros());
start = micros();
DebugPrintLine2("Selecting passenger route x");
ShowMessageTime(start, micros());
}
}

void loop(void)
{
}

void DebugPrintLine(const char* message)
{
while (*message != '\0')
{
port.write(*message++);
}

port.write('\r');
port.write('\n');
}

void DebugPrintLine2(const char* message)
{
char buffer[50];
char* dest = buffer;

while (*dest++ = *message++)
;

*dest++ = '\r';
*dest++ = '\n';
}

void ShowMessageTime(uint32_t start, uint32_t stop)
{
port.print("Message time:");
port.print(stop - start, DEC);
port.println("us");
delay(400);
}

Iain

[Reply #24 on:]

It should be possible to speed up write(const char* str). 

I now use the default implementation in Print which is not very efficient.  Here is the default version:

Code:

    size_t write(const char *str) { return write((const uint8_t *)str, strlen(str)); }

I also use the default version of write(const uint8_t*, size_t)

Code:

size_t Print::write(const uint8_t *buffer, size_t size)
{
  size_t n = 0;
  while (size--) {
    n += write(*buffer++);
  }
  return n;
}

If I override write(const uint8_t*, size_t) with an efficient version that just copies the bytes into the ring buffer it should speed up.  I will try this but it may be a few days.

Thanks for suggesting this.  I now must redesign/rewrite the internals to implement all the good suggestions people have made.

[Reply #25 on:]

An efficient copy of the write methods would be good. I've also discovered that accessing the SerialPort class via Stream* is bad for performance. The message which took 168us before, takes 244us via Stream*. I'm guessing that's due to the extra indirection caused by virtual functions. So having efficient write methods in SerialPort would be excellent.

Iain

[Reply #26 on:]

After a few seconds thought, I realized that an efficient read would also be good.

Code:

int read(uint8_t* buf, size_t size);

would return as much input data as possible.  I use int for the return type in case I need -1 for an error return.  I guess input buffer overrun might be such a condition.

My to-do list is growing really fast.

[Reply #27 on:]

Lets hope you've got enough free time to get it all done

Iain

[Reply #28 on:]

Iain,

Here is output after optimizing write(const char*).

Selecting passenger route x
Message time:64us
Message time:16us
Selecting passenger route x
Message time:68us
Message time:16us
Selecting passenger route x
Message time:68us
Message time:16us
Selecting passenger route x
Message time:68us
Message time:16us
Selecting passenger route x
Message time:64us
Message time:16us
Selecting passenger route x
Message time:64us
Message time:16us
Selecting passenger route x
Message time:64us
Message time:16us
Selecting passenger route x
Message time:76us
Message time:16us
Selecting passenger route x
Message time:76us
Message time:16us
Selecting passenger route x
Message time:72us
Message time:16us
Selecting passenger route x
Message time:76us
Message time:16us

The time includes the interrupt to send the first character.  I enable the TX interrupt in write() so the interrupt happens immediately.  All the setup plus the interrupt means that write() of a one character string takes a bit over 20 us. 

Copying the string into the ring buffer requires two memcpy() calls when the string wraps in the ring buffer.

Here is the sketch.  I replaced your while loop with port.write(message).

Code:

#include <SerialPort.h>

SerialPort<0, 0, 32> port;

void setup(void)
{
  port.begin(115200);
  for (int route = 0; route < 11; route++)
  {
    uint32_t start;

    start = micros();
    DebugPrintLine("Selecting passenger route x");
    ShowMessageTime(start, micros());
    start = micros();
    DebugPrintLine2("Selecting passenger route x");
    ShowMessageTime(start, micros());
  }
}

void loop(void)
{
}

void DebugPrintLine(const char* message)
{
  /*
  while (*message != '\0')
  {
    port.write(*message++);
  }
  */
  port.write(message);
  port.write('\r');
  port.write('\n');
}

void DebugPrintLine2(const char* message)
{
  char buffer[50];
  char* dest = buffer;

  while (*dest++ = *message++)
    ;

  *dest++ = '\r';
  *dest++ = '\n';
}

void ShowMessageTime(uint32_t start, uint32_t stop)
{
  port.print("Message time:");
  port.print(stop - start, DEC);
  port.println("us");
  delay(400);
}

I remembered the easy way to prevent in-lining of functions defined in a template.  You just use attribute like this.

Code:

__attribute__((noinline))
int read() {
  if (!RxBufSize) {
    return usart_->read();
  }else {
    uint8_t b;
    return rxbuf_->get(&b) ? b : -1;
  }
}

[Reply #29 on:]

That's awesome. Your work has already helped me out. The debug messages I was using in my actual program were taking 1500us to produce. With your SerialPort and some serious tuning I was able to get this down to 320us which is inside my 390us limit.

Thanks for your efforts.

Iain