Serial port "rx_buffer" gobbles up unnessary RAM

fat16lib:
I like buffer size in begin(). In 1.0 begin() needs to specify RX and TX buffer sizes.

That would need dynamic memory allocation. It seems much more efficient to use #define/#ifdef for this.

WRT overflow() I added the lost char counter into the ringbuffer

HardwareSerial.cpp

struct ring_buffer
{
  unsigned char buffer[RX_BUFFER_SIZE];
  int head;
  int tail;
  long lost;  // <<<< ADDED
};

and

inline void store_char(unsigned char c, ring_buffer *rx_buffer)
{
  int i = (unsigned int)(rx_buffer->head + 1) % RX_BUFFER_SIZE;

  // if we should be storing the received character into the location
  // just before the tail (meaning that the head would advance to the
  // current location of the tail), we're about to overflow the buffer
  // and so we don't write the character or advance the head.
  if (i != rx_buffer->tail) {
    rx_buffer->buffer[rx_buffer->head] = c;
    rx_buffer->head = i;
  } else rx_buffer->lost++;  // <<<<<<<<<<< ADDED
}

Added this function

// note: lost counter is not reset ...
uint32_t HardwareSerial::overflow(void)
{
 return  _rx_buffer->lost;
}

Added in hardwareSerial.h the prototype of course.

virtual uint32_t overflow(void);

Works like a charm!

test sketch, just keep sending chars :slight_smile:

void setup()
{
  Serial.begin(115200);
}

void loop()
{
  Serial.println(Serial.overflow());
  Serial.println(Serial.available());
  Serial.println("----------");
  delay(1000);
}

reported as issue 637, - http://code.google.com/p/arduino/issues/detail?id=637 -

It's dead simple to modify HardwareSerial, that's why I have rewritten it the way I like for my own use.

The problem is you can't ask users to replace HardwareSerial in order to use your software. HardwareSerial is in the Arduino core in a fundamental way since it uses the serial ISRs.

I don't see a clean way to use a #define in a sketch to override the size of the buffers in the HardwareSerial library.

Show me a working example of how to change the size of the ISR receive buffer in the library by using a #define in the user's sketch.

I would like to avoid dynamic memory but simple ideas using a #define don't work.

I don't see a clean way to use a #define in a sketch to override the size of the buffers in the HardwareSerial library.

I don't even see an ugly way :wink:
pity ...

I thought more and your right, my ugly way won't work.

The fact that HardwareSerial.h is included in almost every library via WProgram.h or Arduino.h in 1.0 killed my ugly idea.

fat16lib:
I don't see a clean way to use a #define in a sketch to override the size of the buffers in the HardwareSerial library.

Show me a working example of how to change the size of the ISR receive buffer in the library by using a #define in the user's sketch.

There isn't one, that's why this is in the "suggestions" area...!

Why suggest it if it can't be done with a #define.

How would you modify the library to make a #define work?

I have written dozens of Arduino libraries and don't know I would use a #define in a user sketch to set the size of an ISR receive buffer in a library.

fat16lib:
How would you modify the library to make a #define work?

I have written dozens of Arduino libraries and don't know I would use a #define in a user sketch to set the size of an ISR receive buffer in a library.

Like this:

#ifndef RX_BUFFER_SIZE
#define RX_BUFFER_SIZE 128
#endif

Then the user can do:

#define RX_BUFFER_SIZE 16
#include "serial.h"
...

The user #define will not be used when the library is compiled so this will not work.

fat16lib:
The user #define will not be used when the library is compiled so this will not work.

Ok, how about something like this in the header file?

#ifndef USER_RX_BUFFER
static byte rx_buffer[128];
#endif


class Serial {

  // User provides the rx buffer
  void begin(int baud, byte *rxBuffer, int rxBufferSize);

#ifndef USER_RX_BUFFER
  // Default - use the buffer defined in this header
  void begin(int baud) {
    begin(baud, rx_buffer, sizeof(rx_buffer));
  }
#endif

};

Then the user can define a macro "USER_RX_BUFFER" to exclude the built-in buffer and provide his/her own.

No there are libraries that call begin() to change baud rate for auto baud detection. These libraries also include the header so multiple buffers will be created and used.

My idea of dynamic RAM will not even work with existing code.

Serial is designed so you can call begin() and end() as often as you please in any place.

I give up on this issue.

Serial is designed so you can call begin() and end() as often as you please in any place.

So begin should

  • test if a buffer exists,
  • free it and
  • malloc a new one.

end should

  • test if a buffer exists,
  • free it

fat16lib:
No there are libraries that call begin() to change baud rate for auto baud detection. These libraries also include the header so multiple buffers will be created and used.

Ok...maybe making it completely transparent to people in all situations isn't going to easy.

But ... at least you make the buffer in the library tiny (eight bytes?) and give power users the option to expand it via a function call. Space is very tight on Arduino, there's no point in everybody paying "power user" prices for things they don't even use.

fungus,

I am sold that HardwareSerial need to be fixed. The problem is to sell this to the Arduino developers.

Arduino 1.0 beta 3 is really bad. Serial gets loaded even if you don't use it. You can't easily control buffer sizes even by editing HardwareSerial.h/HardwareSerial.cpp. There are both input and output buffers and they must be the same size.

One value controls the size of all eight buffers on a Mega and they are all allocated, even if you don't use any Serial ports.

Here are links to what I found:

http://arduino.cc/forum/index.php/topic,72106.0.html

http://arduino.cc/forum/index.php/topic,72087.0.html

There have been suggestions that if you add a specially named header file to your sketch (e.g. options.h) it should be automatically included by libraries and the core during compilation: Google Code Archive - Long-term storage for Google Code Project Hosting.. This is mostly motivated by precisely this issue (RAM usage for serial buffers). So far I've been reluctant to add it because there are lots of other possible complications (and, so far, I've seen few other use cases). But I'm not necessarily opposed to ever including it, if there are good reasons.

If there's a reasonable way to have the compiler only include the buffers for the specific serial ports that are actually used by the sketch, that would be great. Any thoughts on how that might happen?

Adding parameters to Serial.begin() to specify the buffer size (and dynamically allocating the buffers) seems like overkill.

Beta 4 fixes the problem that Serial is loaded even if it is not used.

I would like to see a way to independently specify buffer sizes for the eight buffers on the Mega if all eight must be loaded when one port is used.

There are apps that require one large receive buffer on a Mega and this is not currently possible. For example, to log data from a 115200 baud Serial port to an SD card reliably requires about a 2500 byte receive buffer to allow for the maximum possible write latency of an SD card.

I would accept any improvement, even if it required edits of defines or const parameters in HardwareSerial.

I am writing a replacement for HardwareSerial for my own use. I am using an idea from several commercial embedded systems.

All Serial ports start in an unbuffered mode and do not use interrupts. This seems to work fine for most sketches since few sketched benefit from output buffering and most sketches do little input.

Even sketches that do multiple character input work o.k. since most do input in a wait for character loop.

I added an attachInterrupts() call to allocate buffers and switch to interrupt driven mode. This allows run time control of serial buffering. You can just attach a buffer to the input side of a single port when required.

Are you planning to share this library when it's finished? I'd be interested. I've already modified my own version of HardwareSerial to support buffered output but different buffer sizes for each port sounds really useful.

I am not planning on sharing this library. It is intended as a true replacement for HardwareSerial, not just an additional serial library.

I tried the idea as an additional serial library and found that other libraries use Serial. There can only be one ISR for a serial interrupt vector so replacement of HardwareSerial is the only practical solution.

The policy of the Arduino developers is that they control features of the core for comparability reasons. Therefore I have not released core components that I have redesigned for my use.

For those interested, I updated the issue on github - HardwareSerial overflow of input detection [imported] · Issue #223 · arduino/ArduinoCore-avr · GitHub

Missing in the discussion in the thread is a reset function for the lost counter,
This makes the proposal functional complete while staying backwards compatible.

struct ring_buffer
{
  unsigned char buffer[RX_BUFFER_SIZE];
  int head;
  int tail;
  long lost;  // <<<< ADDED
};
inline void store_char(unsigned char c, ring_buffer *rx_buffer)
{
  int i = (unsigned int)(rx_buffer->head + 1) % RX_BUFFER_SIZE;

  // if we should be storing the received character into the location
  // just before the tail (meaning that the head would advance to the
  // current location of the tail), we're about to overflow the buffer
  // and so we don't write the character or advance the head.
  if (i != rx_buffer->tail) {
    rx_buffer->buffer[rx_buffer->head] = c;
    rx_buffer->head = i;
  } else rx_buffer->lost++;  // <<<<<<<<<<< ADDED
}
// note: lost counter is not reset ...
uint32_t HardwareSerial::overflow(void)
{
 return  _rx_buffer->lost;
}

New addition as it makes sense to be able to reset the overflow/ lost counter

void resetOverflow(void)
{
  _rx_buffer->lost = 0;
}