Pages: 1 [2]   Go Down
Author Topic: Serial port "rx_buffer" gobbles up unnessary RAM  (Read 2929 times)
0 Members and 1 Guest are viewing this topic.
0
Offline Offline
Edison Member
*
Karma: 44
Posts: 1482
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Valencia, Spain
Offline Offline
Faraday Member
**
Karma: 119
Posts: 4591
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:
#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.

« Last Edit: September 10, 2011, 06:35:56 pm by fungus » Logged

No, I don't answer questions sent in private messages...

0
Offline Offline
Edison Member
*
Karma: 44
Posts: 1482
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged

Global Moderator
Netherlands
Offline Offline
Shannon Member
*****
Karma: 170
Posts: 12483
In theory there is no difference between theory and practice, however in practice there are many...
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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




Logged

Rob Tillaart

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

Valencia, Spain
Offline Offline
Faraday Member
**
Karma: 119
Posts: 4591
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.


Logged

No, I don't answer questions sent in private messages...

0
Offline Offline
Edison Member
*
Karma: 44
Posts: 1482
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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
Logged

Forum Administrator
Cambridge, MA
Offline Offline
Faraday Member
*****
Karma: 9
Posts: 3538
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged

0
Offline Offline
Edison Member
*
Karma: 44
Posts: 1482
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged

Guildford, UK
Offline Offline
Full Member
***
Karma: 0
Posts: 217
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged

0
Offline Offline
Edison Member
*
Karma: 44
Posts: 1482
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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. 
Logged

Global Moderator
Netherlands
Offline Offline
Shannon Member
*****
Karma: 170
Posts: 12483
In theory there is no difference between theory and practice, however in practice there are many...
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:
struct ring_buffer
{
  unsigned char buffer[RX_BUFFER_SIZE];
  int head;
  int tail;
  long lost;  // <<<< ADDED
};

Code:
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:
// 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:
void resetOverflow(void)
{
  _rx_buffer->lost = 0;
}
Logged

Rob Tillaart

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

Pages: 1 [2]   Go Up
Jump to: