Go Down

Topic: Serial port "rx_buffer" gobbles up unnessary RAM (Read 3796 times) previous topic - next topic

fat16lib

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

fungus

#16
Sep 11, 2011, 01:32 am Last Edit: Sep 11, 2011, 01:35 am by fungus Reason: 1

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?

Code: [Select]

#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, I don't answer questions sent in private messages (but I do accept thank-you notes...)

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.

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.

robtillaart

Quote
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




Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

fungus


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.


No, I don't answer questions sent in private messages (but I do accept thank-you notes...)

fat16lib

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

mellis

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: http://code.google.com/p/arduino/issues/detail?id=27.  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.

fat16lib

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.

sixeyes

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.

fat16lib

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. 

robtillaart

For those interested, I updated the issue on github - https://github.com/arduino/Arduino/issues/637

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.


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


Code: [Select]
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
}


Code: [Select]
// 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
Code: [Select]
void resetOverflow(void)
{
  _rx_buffer->lost = 0;
}
Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Go Up