Hardware Serial Buffer Size

[Victor Aprea]

Hey all,

I really want to increase the Hardware Serial receive buffer from the default 64 bytes to more, say 256 bytes. My board uses the standard Arduino core (i.e. build.core=arduino:arduino). 

So, is this kind of thing (tl;dr copying the standard core, making a tiny tweak to a #define in HardwareSerial.cpp, and re-packaging it as a differently-named core) still the state-of-the-art method to get that done? 

Kind Regards,

Vic

Victor Aprea // Wicked Device

victor...@wickeddevice.com

T: @vicatcu

[Andrew Kroll]

A project settings file is something many have asked for in the past, but has been denied so to keep things simple for new users. Personally, I don't agree with this particular reasoning, because there could always be defaults, if there is no such file present, you just use the defaults. This could even extend to settings files in libraries, where most new users aren't looking anyway, to provide library-specific tweaks, and that only get used for the compile of the library as defaults for the library. Anything else would be left alone, and collisions would be avoided. Sometimes you need a different flag to avoid compiler bugs. This is also very difficult to do without affecting every sketch.

Overall I don't use the IDE myself, I use GNU make + the arduino builder, so I end up with better control of such things on a case-by-case and sketch-by-sketch basis. I can insert -D for defines and compiler/linker flags at-will. It actually becomes useful to me personally at that point.

Also, what I do for my libraries that need to work from the restrictive IDE, is load the code in-line as headers that contain the code and become part of the user's sketch. This gives defaults as well as tweaks within the sketch. Yes it may look funny to some, but it does work, and provides the flexibility for libraries that can benefit from it. Unfortunately the Serial class is a built-in, not a library, which is kind of a shame. Alternate libraries could have been provided and would not have affected anything. Perhaps Serial (and some other stuff, where it makes sense) should be moved to libraries instead, allowing different, and possibly fuller featured Serial classes for things such as hardware flow control.
A Serial library would be very welcomed, especially once people want to do things that are a bit more demanding, without leaving the environment, tools and other great offerings that Arduino brings to the table as a whole, such as the initial simplicity.

HOWEVER.... that said.... even though we are currently stuck with this problem...
There are possibly other ways than what you mention, I think... Not sure but...

Idea 1:
If you add the -D flag, in preferences.txt, it will override what is in the headers...
...but for EVERYTHING until you change it back!

What would be nice is perhaps an option specifically to each serial instance in the settings, so you could at least tweak there. Last I checked one define affects all serial buffer sizes. Not exactly flexible if you have multiple serial ports... this would end up bloating all the buffers. Correct me if I am wrong here, but IIRC buffers get allocated even if you don't use all the ports, or any of them. I can not recall if this has been fixed or not. If it has been fixed, then it would be OK to do, providing you realize the effects on RAM for all the serial instances used.

Idea 2:
Sub-class Serial and make the buffers any size you desire. Of course with this approach, the class name 'Serial' can't be used anymore. So anything using Serial, Serial1, etc would need modification. This could balloon if libraries need to use Serial and lack a way to be told where the output goes, and then you have to modify all those libraries too. Not very handy, and why nobody does it. You also lose all the Serial macro hints too.

--
You received this message because you are subscribed to the Google Groups "Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to developers+unsubscribe@arduino.cc.

--

Visit my github for awesome Arduino code @ https://github.com/xxxajk

[Dieter.Burandt]

Hi Andrew,

I am one of those how asked again and again to enhance some buffer sizes and speed settings:

If I understand the compilation process of an ino-file correctly, all include files are "concatinated" and then compiled.

Then it shoud be possible to do the following:

Idea 3 (or is it your idea 1?):

For the serial buffer enclose the "SERIAL_BUFFER_SIZE" in RingBuffer.h in:

#ifndef   SERIAL_BUFFER_SIZE
    #define SERIAL_BUFFER_SIZE 64
#endif

For I2C-devices enclose the "BUFFER_LENGTH" in Wire.h in:

#ifndef BUFFER_LENGTH
    #define BUFFER_LENGTH 32
#endif

Give the user the option to specify a special h-file in the ino-file of the project (or global variables), that defines the buffer sizes.
That's all, except to communicate this to the potential users that needs bigger buffers.

(I would be fully satisfied, if the SPI-speed settings could also be set from within the project for the SD-card and the Ethernet interface. Here a small extension of the begin-functions are required that do not restrict user settings.)

Clearly it would be a great to define different buffer sizes for each of the serial devices. But the above simple solution fixes most of the problems as long as you take a board with anough RAM. Using newer libraries only buffers are allocated  that are really used.)

Best Regards

Dieter Burandt

-- You received this message because you are subscribed to the Google Groups "Developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@arduino.cc.

[Thomas Roell]

Would it perhaps make sense to move those buffer sizes into defines into the respective variant.h files ?

I mean there is a consistent set of UART/Wire/I2S devices per core.

For the STM32L4 core, I did move all the device-allocation type items into variant.cpp and some configuration options into variant.h. Right now I am hitting a similar issue with the buffer sizes, where I want to have "Serial1" (D0/D1) as high speed with a larger buffer, while all the other UARTs should use smaller ones. So something like this is rather variant specific, and in my case to a large degree depends on the MCU (some have less SRAM than others, some can use FIFO/DMA, some cannot, and of course there are per UART instance limitations). Hence the solution to move that into variant.h / variant.cpp.

The I2C BUFFER_LENGTH is an awful thing. For "avr" it's there explicitly, for "samd" the rx/tx buffer have been replaced by a "RingBuffer" object (shared potentially with CDC/UART), where the buffer size is 64 (defined via SERIAL_BUFFER_SIZE). So at the end of the day the end-user with an Arduino Zero has no way of finding out how much data can be buffered for the Wire object.

- Thomas

-- You received this message because you are subscribed to the Google Groups "Developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to developers+unsubscribe@arduino.cc.

[Dieter.Burandt]

I am using an Arduino Due board.

The size of my serial buffer is defined by the amount of meteorological data I receive from up to 8 sensors within 3 Minute intervals. The data may come nearly at the same time. The interface does not support any flow control. So I must be in receive mode all over the time. Using a 64 byte buffer I have to empty the buffer each 100ms. This is not possible when I do SD-card-i/o, Ethernet-i/o, or floating point data processing. Using a 256 byte buffer I must do it within 3 Minute: No problem.

I think that is in effect the same problem as Victor has.

The size of the I2C buffer is defined by the EEPROM 24LC1025 I use.  I need here 128 byte or I must use another, smaller and slower EEPROM with smaller page size.

Both buffer sizes are defined by the attached devices, not by the CPU. So Variant.h is not a good idea.

My idea is to keep all as simple as possible, to run properly on all plattforms and not to change anything for the normal user who is struggling with the technique.
Also, testing should be as simple as possible or mostly not required. It should be a methode, that can easily used for other purpose too.

 

Best Regards

Dieter Burandt

-- You received this message because you are subscribed to the Google Groups "Developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@arduino.cc.

[Mike]

With more modern processors like the SAMD etc. overtaking the ATmega, and parallels with other modern processors, this may be the time to consider an ability to selectively allocate resources beyond those fixed by previous Arduino constants.

Several very thoughtful methods have been discussed in the last several months, this appears to be a good time to make the leap for modern processor support.

Mike

[Victor Aprea]

Interesting side note, I just scoured the github issues and turned up this (closed) one: https://github.com/arduino/Arduino/issues/1929

So AFAICT there's not a current open issue associated with this (particular question about HardwareSerial buffers), unless there's a more broadly scoped one that covers it. I'm sure someone (e.g. @matthijskooijman) might identify one that I've missed.

Regards,

Vic

Victor Aprea // Wicked Device

victor...@wickeddevice.com

T: @vicatcu

[Victor Aprea]

I tend to agree with Dieter that the place for defining a buffer size (statically) is most naturally in the application scope, not the board scope. That being said, a way to have a variant-defined default buffer size, which would override the default specified by a referenced core (e.g. arduino:arduino), would not be a terrible idea either. The two concepts are not mutually exclusive in my mind.

Kind Regards,

Vic

Victor Aprea // Wicked Device

victor...@wickeddevice.com

T: @vicatcu

[Massimo Banzi]

So, to summarise, the two use cases are :

1: Developer of an arduino compatible core wants to change the buffer size for a specific board

I think we agree this would go into a variant. 

2: Allowing users to set the buffer size while developing code for an application.

What do you guys recommend to allow users to do this? this would allow to come up with a rule on how to solve this issues and document it for the benefit of core/library developers .

A number of parametres like this are buried in the code in different places, not sure ho many can be safely brought out.

m

--

Massimo Banzi <m.b...@arduino.cc>

Arduino LLC

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@arduino.cc.

[Thomas Roell]

Dieter,

looks like we all have the same problem. I think there are 2 long term answers for that;

(1) Add a Serial.onReceive() callback mechanism. This way you can drain the small buffer from the UART into your larger application buffer. The I2S.h class uses this scheme already, and there is no good reason not to add that to the HardwareSerial class.

(2) I2C buffering is not the best idea for larger transfer sizes. The right thing to do is to allow a unbuffered transfer ... "Wire::transfer(uint8_t address, const uint8_t *txBuffer, size_t txSize, uint8_t *rxBuffer, size_t rxSize, bool stopBit = true);".

- Thomas 

[Victor Aprea]

Thomas,

It may be a minor point ultimately, but one argument for keeping the buffer in HardwareSerial itself is that the ring buffer implementation is there. It would be kind of unfortunate to add the burden to every application that wants non-default buffer sizes to implement its own buffer management algorithm when all they really want to do is change the amount of buffering (not the buffering algorithm). Again, though, I would emphasize that adding onReceive() handler support to allow optional delegation of the ISR is not a mutually exclusive approach.

Kind Regards,

Vic

Victor Aprea // Wicked Device

victor...@wickeddevice.com

T: @vicatcu

[Paul Carpenter]

The problems for this come from many parts of the software

1. There is no COMMON byte (uint8_t) buffer handling class partly due to many times the classes have been defined differently but most commonly with the actual buffer instantiated inside the class with a common define macro (all RX and TX buffers must be same size even if one not used). This means the array CANNOT be resized at run time.
   
2. Serial class was originaly written on Uno and assumed there would ONLY ever be one serial port all the rest for multiple ports on Mega, Due and others have been at best a kludge
3. No simple dependency in build on project/sketch including a file like custom.h and create define CUSTOM_H, to rebuild core for that project and preferably keep the compiled core in a build directory of the project. So differing projects have their own project core built for faster compiles when swapping projects and no issues of older custom.h in this project building with the wrong core.

Personally there should be custom.h style defines for things like

SERIAL[1 | 2 | 3 | 4]_RX_DISABLE

SERIAL[1 | 2 | 3 | 4]_TX_DISABLE

SERIAL[1 | 2 | 3 | 4]_RX_BUFF_SIZE

SERIAL[1 | 2 | 3 | 4]_TX_BUFF_SIZE

I2Cn_DISABLE

I2Cn_BUFF_SIZE

SPIn_DISABLE

SPIn_BUFF_SIZE

SDCARD_BUFF_SIZE

ETHER_BUFF_SIZE

Then core part has

#if not defined SERIAL2_RX_DISABLE

#if not defined SERIAL2_RX_BUFF_SIZE

#define SERIAL2_RX_BUFF_SIZE  <defualt>

#endif

uint8_t serial2_rx_buff[ SERIAL2_RX_BUFF_SIZE ];

Buffer Serial2_RX( serial2_rx_buff, SERIAL2_RX_BUFF_SIZE );

#endif

Repeat for TX buffer then add serial class instantiation

Buffer class to have read and write handling with volatiles for ptrs or index in buffer for Read and Write but consist of

uint8_t   *buff;

int         size;

int         RD_idx;

int         WR_idx

uint8_t  HW_over;

uint8_t  SW_over;

HW_over being count of discarded bytes for Pairty, or Overrun errors

SW_over being discards becasue before was full

Reading of either status resets them to zero.

Once you have that THEN you can consider having variable buffer sizing.

I looked at it for Mega and Due and gave up as would take to long to restructure code properly as I would have preferred buffers for a Due first of

serial n        RX         TX

1               16 or 64  256

2                512        512

3                0           512

4               disabled

Persoanlly building a common buffer class would help more libraries for other modules, and reduce code size and associated handling issues, and would be a good first step

[Thomas Roell]

Vic,

it's not so much as to re-implement the buffer. The key problem is more, as you outlined correctly, that there might be other code in "loop()" that makes draining the buffer a problem due to the long latency from data arriving to data being processed. By having a callback that is closer the when the data receive happened you can either process data directly or buffer it (perhaps in preprocessed form). Think about a GPS parser that takes data directly, assemblers proper sentences, checksum them, and only when a complete GPS sentence is buffered it interprets them. This sentence packaging could be done in the callback. 

A generic scheme will always run into problems. What is the upper limit (code sometime assumes a ringbuffer is only 256 bytes). Some implementations might use DMA for RX/TX out/into this ringbuffer, and that might require the ringbuffer to have special alignment or needs to reside in a special memory region. For some MCUs you have to have a minimum ringbuffer size, so that DMA works. In fact the HardwareSerial API does not require a ringbuffer to begin with. On some MCUs (like TM4C123GH / Energia) I could see an implementation using the hardware FIFO directly.

So by exposing something like a customizable buffer size you create a lot of compatibility problems that the API itself does not really have.

- Thomas 

[Dieter.Burandt]

Hi Thomas,

thanks for your answer.

(1) The UART buffer size in the SAM3X8E is only one byte. The Serial Buffer is managed by the library code not only on reading the data from the chip as an ISR but also manages the data transfer to the user buffer partially in an interrupt disabled state properly. When I would use Serial.onReceive() I must duplicate most of the code of the library but with a changed buffer size. That is possible but not really a good.

(2) In my Arduino Due library code, I cannot find a call of Wire::transfer(...). I use "WireDev.write(Data, DataSize);" and "WireDev.requestFrom(..)". It may be that your call is available for the 8 -bit CPUs. I read or write only one page at a time. After this time, the CPU is clearly in interrupt enable state and can handle other interrupts e.g. from the serial device or a power fail interrupt. A power fail immediatly aborts my normal execution flow handling the power fail. I big data transfer is not appropriate, but the buffer must have a size of at least the page size of the EEPROM.

Best Regards,

Dieter

-- You received this message because you are subscribed to the Google Groups "Developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@arduino.cc.

[Victor Aprea]

Thomas, 

Yes I understand what you mean, but I would still contend that the configuration exposure which is most sensible is simply "whether or not the interface provides buffering, and if so how much." The important detail here, I think, is that the configuration exposure allow for compile time declaration and not insist on dynamic memory allocation. 

The only ways I know to deal with that generally are to figure out a way in the build support overriding #defines, or to support an (possibly optional) BYOB (bring your own buffer) mechanism in the runtime API of a library. The prior allows the library to encapsulate and manage its buffering however it likes so long as it is compliant with a #define override-able naming convention, whereas the latter delegates the buffering responsibility to application. Again these strategies are not mutually exclusive (you could do both). I certainly don't know everything, so perhaps there are other intelligent ways to go about it. But my preference, I think, would be pursuit of a "universal" #define override capability supported by the standard Arduino cores and the Arduino builder / build system.

Kind Regards,

Vic

Victor Aprea // Wicked Device

victor...@wickeddevice.com

T: @vicatcu

[Dieter.Burandt]

Hi,

I asked the team to allow, that a user can change the default sizes from within the project.

But of cause, you are not and cannot be responsible for the bugs a user implements. Don't try it.

Best Regards,

Dieter

-- You received this message because you are subscribed to the Google Groups "Developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@arduino.cc.

[Thomas Roell]

Dieter,

I was not commenting on what was currently implemented, but what should be done to avoid those issues going forward. So none of the 2 APIs is implemented currently in the official Arduino source base (yes, they are implemented in the STM32L4 core, but that is not really relevant here).

The whole issue is a tad of a mess. A callback from the ISR level may have an impact on the responsiveness of the overall system. So a callback is best handled via the ARM Cortex PendSV mechanism. On the other hand it seems bad to expose the serial buffer size as a configurable option to the end user. Just because it would work for a simple ISR driver UART driver does not mean it would work for a FIFO/DMA based UART. So why drive towards a predictable incompatibility ?

N.b. that in POSIX you have setvbuf(). Perhaps something like that where the user can specify an additional RX/TX buffer over a builtin unspecified buffering scheme might be a better option.

- Thomas

[Martino Facchin]

About using the capabilities of new processors, I once proposed to dynamically add storage to a RingBuffer instance to overcome define-based solutions.

The code is attached, and the overhead is minimal. The core could keep safe low values for buffer sizes and the application can dynamically require a bigger one.

Of course, a common RingBuffer implementation for 32bit platforms is a must in this case, or we are going to introduce a massive API breakage

[Paul Carpenter]

If you are doing DMA then a standard ringbuffer is not your chosen
implementation, you are more likely to be using double buffering
(in software alone or hardware and software).

Considering how I have dealt with SMA, DUAL channel DMA, multiple DMA
buffers and other techniques since about 1979, on many systems and never was
a DMA to Ring Buffer ever considered.

[Thomas Roell]

Paul,

that is correct. That's part of why I am not a big fan is the idea of exposing the compile time sizing of a ringbuffer that may or may not be present to begin with.

- Thomas

[Andrew Kroll]

... and all of this is exactly why it should be a library. :-)

It solves all the major issues:

1: allows you to #define before reading in the header if you inline the code in the header

2: full replacement with something else

On Fri, Jan 13, 2017 at 3:04 PM, Thomas Roell <grumpyo...@gmail.com> wrote:

Paul,

that is correct. That's part of why I am not a big fan is the idea of exposing the compile time sizing of a ringbuffer that may or may not be present to begin with.

- Thomas

On Fri, Jan 13, 2017 at 1:00 PM, Paul Carpenter <paul@pcserviceselectronics.co.uk> wrote:

If you are doing DMA then a standard ringbuffer is not your chosen
implementation, you are more likely to be using double buffering
(in software alone or hardware and software).

Considering how I have dealt with SMA, DUAL channel DMA, multiple DMA
buffers and other techniques since about 1979, on many systems and never was
 a DMA to Ring Buffer ever considered.

Thomas Roell wrote:

Dieter,

I was not commenting on what was currently implemented, but what should be done to avoid those issues going forward. So none of the 2 APIs is implemented currently in the official Arduino source base (yes, they are implemented in the STM32L4 core, but that is not really relevant here).

The whole issue is a tad of a mess. A callback from the ISR level may have an impact on the responsiveness of the overall system. So a callback is best handled via the ARM Cortex PendSV mechanism. On the other hand it seems bad to expose the serial buffer size as a configurable option to the end user. Just because it would work for a simple ISR driver UART driver does not mean it would work for a FIFO/DMA based UART. So why drive towards a predictable incompatibility ?

N.b. that in POSIX you have setvbuf(). Perhaps something like that where the user can specify an additional RX/TX buffer over a builtin unspecified buffering scheme might be a better option.

- Thomas

--
You received this message because you are subscribed to the Google Groups "Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to developers+unsubscribe@arduino.cc.

--
You received this message because you are subscribed to the Google Groups "Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to developers+unsubscribe@arduino.cc.

[Paul Carpenter]

It would make no difference to special DMA cases that need other
buffer handling class, and then may need a dual/multiple buffer handling
which even then may be a whole set of different sized buffers. May even
support ring buffer and DMA buffers to switch between commands and block
transfer modes.

A standard ring buffer class would support many cases that currently support
ring buffers, then they can be changed by whatever means with peripheral
control to stem flow and stop race or other conditions. The best way to be
able to change a buffer size is when the buffer is EXTERNAL to any form of
a class where a fixed type buffer can only be instantiated at one fixed
size. Otherwise we start getting into vectors and the like.

Thomas Roell wrote:
> Paul,
>
> that is correct. That's part of why I am not a big fan is the idea of
> exposing the compile time sizing of a ringbuffer that may or may not be
> present to begin with.
>
> - Thomas
>
>
> On Fri, Jan 13, 2017 at 1:00 PM, Paul Carpenter
> <pa...@pcserviceselectronics.co.uk

[Andrew Kroll]

Ring buffer libraries exist... perhaps they should be leveraged.

--
You received this message because you are subscribed to the Google Groups "Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to developers+unsubscribe@arduino.cc.

[Paul Carpenter]

One already exists as yet another ring buffer implementation in Due Core
set, probably in other ARM implementations maybe in x86 and other
architectures.

None have been used in serial as far as I can see. I looked at sorting it
briefly, but saw the core build issues amongst other things like NIH

Andrew Kroll wrote:
> Ring buffer libraries exist... perhaps they should be leveraged.
>
> On Fri, Jan 13, 2017 at 3:13 PM, Paul Carpenter
> <pa...@pcserviceselectronics.co.uk

> <mailto:pa...@pcserviceselectronics.co.uk>> wrote:
>
> It would make no difference to special DMA cases that need other
> buffer handling class, and then may need a dual/multiple buffer handling
> which even then may be a whole set of different sized buffers. May even
> support ring buffer and DMA buffers to switch between commands and block
> transfer modes.
>
> A standard ring buffer class would support many cases that currently
> support
> ring buffers, then they can be changed by whatever means with peripheral
> control to stem flow and stop race or other conditions. The best way
> to be
> able to change a buffer size is when the buffer is EXTERNAL to any
> form of
> a class where a fixed type buffer can only be instantiated at one fixed
> size. Otherwise we start getting into vectors and the like.
>
> Thomas Roell wrote:
>
> Paul,
>
> that is correct. That's part of why I am not a big fan is the
> idea of exposing the compile time sizing of a ringbuffer that
> may or may not be present to begin with.
>
> - Thomas
>
>
> On Fri, Jan 13, 2017 at 1:00 PM, Paul Carpenter
> <pa...@pcserviceselectronics.co .uk

> <mailto:pa...@pcserviceselectronics.co.uk>
> <mailto:paul@pcserviceselectro nics.co.uk

> <mailto:pa...@pcserviceselectronics.co.uk>>> wrote:
>
> If you are doing DMA then a standard ringbuffer is not your
> chosen
> implementation, you are more likely to be using double buffering
> (in software alone or hardware and software).
>
> Considering how I have dealt with SMA, DUAL channel DMA,
> multiple DMA
> buffers and other techniques since about 1979, on many
> systems and
> never was
> a DMA to Ring Buffer ever considered.
>

--
Paul Carpenter | pa...@pcserviceselectronics.co.uk
<http://www.pcserviceselectronics.co.uk/> PC Services
<http://www.pcserviceselectronics.co.uk/LogicCell/> Logic Gates Education
<http://www.pcserviceselectronics.co.uk/pi/> Raspberry Pi Add-ons
<http://www.pcserviceselectronics.co.uk/fonts/> Timing Diagram Font
<http://www.badweb.org.uk/> For those web sites you hate

[Thomas Roell]

Paul,

let's go back to the original problem. Somebody needed larger buffering capability to be able to absorbe longer latency between when UART data arrived and when it was processed by the code.

As is right now, the public API of the HardwareSerial class does neither document nor require the use of a ringbuffer. There are enough examples where you simply will not have a resizeable ringbuffer (double-buffered DMA, or hardware FIFO). 

So that really leaves only 2 options:

(1) Add a callback mechanism so that the existing API can be used to read data from whatever mechanism is used internally (that means the application needs to do the buffering)

(2) Add a secondary mechanism to plug in a secondary ringbuffer (that may or may not replace the primary mechanism).

However (2) is more or less the same as (1), but this time the code to deal with the additional buffer is in the HardwareSerial class and carried around for all applications, whether they use this mechanism or not.

IMHO (1) is doable with a standard RingBuffer class and a simple callback. It would be odd to force the overhead of (2) for everybody else.

- Thomas

[Andrew Kroll]

From my bag of tricks that I have used in the past:

Serial can be intense IRQ-wise, so you do not want to do a callback every character.

What would solve the situation, but cause change to the end user code as far as rx data, is to have a timer call a routine every few ms and copy the data to a regular ring buffer. Then the new class just slurps from the doubled larger buffer. The same could also be done for TX if wanted, of course.

On Fri, Jan 13, 2017 at 3:28 PM, Thomas Roell <grumpyo...@gmail.com> wrote:

Paul,

let's go back to the original problem. Somebody needed larger buffering capability to be able to absorbe longer latency between when UART data arrived and when it was processed by the code.

As is right now, the public API of the HardwareSerial class does neither document nor require the use of a ringbuffer. There are enough examples where you simply will not have a resizeable ringbuffer (double-buffered DMA, or hardware FIFO). 

So that really leaves only 2 options:

(1) Add a callback mechanism so that the existing API can be used to read data from whatever mechanism is used internally (that means the application needs to do the buffering)

(2) Add a secondary mechanism to plug in a secondary ringbuffer (that may or may not replace the primary mechanism).

However (2) is more or less the same as (1), but this time the code to deal with the additional buffer is in the HardwareSerial class and carried around for all applications, whether they use this mechanism or not.

IMHO (1) is doable with a standard RingBuffer class and a simple callback. It would be odd to force the overhead of (2) for everybody else.

- Thomas

[Victor Aprea]

All,

BYOB with support for a RingBuffer classes would be all well and good, but an important facet of the problem I outlined is static allocation of the memory used by a class (with the intent of using it for buffering in this case). So I don't think we need to be talking about RingBuffer class, as it just pushes that problem out of HardwareSerial and into some new class. BYOB as I see it looks like this. Add the following two methods to the HardwareSerial class with this signature:

void HardwareSerial::setRxBuffer(uint8_t * buf, uint16_t buf_size);

void HardwareSerial::setTxBuffer(uint8_t * buf, uint16_t buf_size);

... and add two internal pointers to the HardwareSerial class like so:

uint8_t * _active_rx_buffer; // default to point at this->_rx_buffer[0]

uint8_t * _active_tx_buffer; // default to point at this->_tx_buffer[0] 

... if you call setRxBuffer it re-wires the class to point active_rx_buffer at buf, and likewise with setTxBuffer and active_tx_buffer. Then just replace all the uses of _rx_buffer and _tx_buffer with their _active counterparts. It would need some testing to be sure, but it would be a fairly low impact to the existing code base I think, and merely extends existing functionality without requirements placed on the build system. It just means application code has to explicitly declare the memory it's going to use, which imho isn't a bad thing, and let the library know about it.

Kind Regards,

Vic

Victor Aprea // Wicked Device

victor...@wickeddevice.com

T: @vicatcu

On Fri, Jan 13, 2017 at 3:28 PM, Paul Carpenter <pa...@pcserviceselectronics.co.uk> wrote:

One already exists as yet another ring buffer implementation in Due Core
set, probably in other ARM implementations maybe in x86 and other
architectures.

None have been used in serial as far as I can see. I looked at sorting it
briefly, but saw the core build issues amongst other things like NIH

Andrew Kroll wrote:

Ring buffer libraries exist... perhaps they should be leveraged.

On Fri, Jan 13, 2017 at 3:13 PM, Paul Carpenter <pa...@pcserviceselectronics.co.uk <mailto:paul@pcserviceselectronics.co.uk>> wrote:

    It would make no difference to special DMA cases that need other
    buffer handling class, and then may need a dual/multiple buffer handling
    which even then may be a whole set of different sized buffers. May even
    support ring buffer and DMA buffers to switch between commands and block
    transfer modes.

    A standard ring buffer class would support many cases that currently
    support
    ring buffers, then they can be changed by whatever means with peripheral
    control to stem flow and stop race or other conditions. The best way
    to be
    able to change a buffer size is when the buffer is EXTERNAL to any
    form of
    a class where a fixed type buffer can only be instantiated at one fixed
    size. Otherwise we start getting into vectors and the like.

    Thomas Roell wrote:

        Paul,

        that is correct. That's part of why I am not a big fan is the
        idea of exposing the compile time sizing of a ringbuffer that
        may or may not be present to begin with.

        - Thomas


        On Fri, Jan 13, 2017 at 1:00 PM, Paul Carpenter
        <pa...@pcserviceselectronics.co .uk

        <mailto:paul@pcserviceselectronics.co.uk>
        <mailto:paul@pcserviceselectro nics.co.uk

        <mailto:paul@pcserviceselectronics.co.uk>>> wrote:

            If you are doing DMA then a standard ringbuffer is not your
        chosen
            implementation, you are more likely to be using double buffering
            (in software alone or hardware and software).

            Considering how I have dealt with SMA, DUAL channel DMA,
        multiple DMA
            buffers and other techniques since about 1979, on many
        systems and
            never was
             a DMA to Ring Buffer ever considered.

--
Paul Carpenter          | pa...@pcserviceselectronics.co.uk
<http://www.pcserviceselectronics.co.uk/>    PC Services
<http://www.pcserviceselectronics.co.uk/LogicCell/>   Logic Gates Education
<http://www.pcserviceselectronics.co.uk/pi/>  Raspberry Pi Add-ons
<http://www.pcserviceselectronics.co.uk/fonts/> Timing Diagram Font
<http://www.badweb.org.uk/> For those web sites you hate

[Victor Aprea]

the downside to this approach, of course, is the memory associated with the internally declared buffers goes to waste. But it's better than the status quo, I think.

Vic

Victor Aprea // Wicked Device

victor...@wickeddevice.com

T: @vicatcu

On Fri, Jan 13, 2017 at 4:12 PM, Victor Aprea <victor...@wickeddevice.com> wrote:

All,

BYOB with support for a RingBuffer classes would be all well and good, but an important facet of the problem I outlined is static allocation of the memory used by a class (with the intent of using it for buffering in this case). So I don't think we need to be talking about RingBuffer class, as it just pushes that problem out of HardwareSerial and into some new class. BYOB as I see it looks like this. Add the following two methods to the HardwareSerial class with this signature:

void HardwareSerial::setRxBuffer(uint8_t * buf, uint16_t buf_size);

void HardwareSerial::setTxBuffer(uint8_t * buf, uint16_t buf_size);

... and add two internal pointers to the HardwareSerial class like so:

uint8_t * _active_rx_buffer; // default to point at this->_rx_buffer[0]

uint8_t * _active_tx_buffer; // default to point at this->_tx_buffer[0] 

... if you call setRxBuffer it re-wires the class to point active_rx_buffer at buf, and likewise with setTxBuffer and active_tx_buffer. Then just replace all the uses of _rx_buffer and _tx_buffer with their _active counterparts. It would need some testing to be sure, but it would be a fairly low impact to the existing code base I think, and merely extends existing functionality without requirements placed on the build system. It just means application code has to explicitly declare the memory it's going to use, which imho isn't a bad thing, and let the library know about it.

Kind Regards,

Vic

Victor Aprea // Wicked Device

victor...@wickeddevice.com

T: @vicatcu

[Thomas Roell]

Victor,

Arduino Zero does not implement a tx buffer for example. 

- Thomas

On Fri, Jan 13, 2017 at 2:12 PM, Victor Aprea <victor...@wickeddevice.com> wrote:

All,

BYOB with support for a RingBuffer classes would be all well and good, but an important facet of the problem I outlined is static allocation of the memory used by a class (with the intent of using it for buffering in this case). So I don't think we need to be talking about RingBuffer class, as it just pushes that problem out of HardwareSerial and into some new class. BYOB as I see it looks like this. Add the following two methods to the HardwareSerial class with this signature:

void HardwareSerial::setRxBuffer(uint8_t * buf, uint16_t buf_size);

void HardwareSerial::setTxBuffer(uint8_t * buf, uint16_t buf_size);

... and add two internal pointers to the HardwareSerial class like so:

uint8_t * _active_rx_buffer; // default to point at this->_rx_buffer[0]

uint8_t * _active_tx_buffer; // default to point at this->_tx_buffer[0] 

... if you call setRxBuffer it re-wires the class to point active_rx_buffer at buf, and likewise with setTxBuffer and active_tx_buffer. Then just replace all the uses of _rx_buffer and _tx_buffer with their _active counterparts. It would need some testing to be sure, but it would be a fairly low impact to the existing code base I think, and merely extends existing functionality without requirements placed on the build system. It just means application code has to explicitly declare the memory it's going to use, which imho isn't a bad thing, and let the library know about it.

Kind Regards,

Vic

Victor Aprea // Wicked Device

victor...@wickeddevice.com

T: @vicatcu

[Thomas Roell]

Victor,

Arduino Zero and UNO do not make use of any TX buffering on CDC.

So do you suggest that all those needed to be modified to make use of that ?

- Thomas

On Fri, Jan 13, 2017 at 2:17 PM, Thomas Roell <grumpyo...@gmail.com> wrote:

Victor,

Arduino Zero does not implement a tx buffer for example. 

- Thomas

On Fri, Jan 13, 2017 at 2:12 PM, Victor Aprea <victor...@wickeddevice.com> wrote:

All,

BYOB with support for a RingBuffer classes would be all well and good, but an important facet of the problem I outlined is static allocation of the memory used by a class (with the intent of using it for buffering in this case). So I don't think we need to be talking about RingBuffer class, as it just pushes that problem out of HardwareSerial and into some new class. BYOB as I see it looks like this. Add the following two methods to the HardwareSerial class with this signature:

void HardwareSerial::setRxBuffer(uint8_t * buf, uint16_t buf_size);

void HardwareSerial::setTxBuffer(uint8_t * buf, uint16_t buf_size);

... and add two internal pointers to the HardwareSerial class like so:

uint8_t * _active_rx_buffer; // default to point at this->_rx_buffer[0]

uint8_t * _active_tx_buffer; // default to point at this->_tx_buffer[0] 

... if you call setRxBuffer it re-wires the class to point active_rx_buffer at buf, and likewise with setTxBuffer and active_tx_buffer. Then just replace all the uses of _rx_buffer and _tx_buffer with their _active counterparts. It would need some testing to be sure, but it would be a fairly low impact to the existing code base I think, and merely extends existing functionality without requirements placed on the build system. It just means application code has to explicitly declare the memory it's going to use, which imho isn't a bad thing, and let the library know about it.

Kind Regards,

Vic

Victor Aprea // Wicked Device

victor...@wickeddevice.com

T: @vicatcu

[Victor Aprea]

Then a user calling setTxBuffer would have no effect on the Arduino Zero, and it's implementation would otherwise be un-perturbed in that case at the expense of a few extra bytes of program memory... it would have a very low probability oft breaking anything. Maybe the methods could even be virtual and optionally implemented in derived classes, or a degenerate implementation could exist in the base class, or something like that (my C++ foo is a bit rusty here)?

Vic

Victor Aprea // Wicked Device

victor...@wickeddevice.com

T: @vicatcu

On Fri, Jan 13, 2017 at 4:17 PM, Thomas Roell <grumpyo...@gmail.com> wrote:

Victor,

Arduino Zero does not implement a tx buffer for example. 

- Thomas

On Fri, Jan 13, 2017 at 2:12 PM, Victor Aprea <victor...@wickeddevice.com> wrote:

All,

BYOB with support for a RingBuffer classes would be all well and good, but an important facet of the problem I outlined is static allocation of the memory used by a class (with the intent of using it for buffering in this case). So I don't think we need to be talking about RingBuffer class, as it just pushes that problem out of HardwareSerial and into some new class. BYOB as I see it looks like this. Add the following two methods to the HardwareSerial class with this signature:

void HardwareSerial::setRxBuffer(uint8_t * buf, uint16_t buf_size);

void HardwareSerial::setTxBuffer(uint8_t * buf, uint16_t buf_size);

... and add two internal pointers to the HardwareSerial class like so:

uint8_t * _active_rx_buffer; // default to point at this->_rx_buffer[0]

uint8_t * _active_tx_buffer; // default to point at this->_tx_buffer[0] 

... if you call setRxBuffer it re-wires the class to point active_rx_buffer at buf, and likewise with setTxBuffer and active_tx_buffer. Then just replace all the uses of _rx_buffer and _tx_buffer with their _active counterparts. It would need some testing to be sure, but it would be a fairly low impact to the existing code base I think, and merely extends existing functionality without requirements placed on the build system. It just means application code has to explicitly declare the memory it's going to use, which imho isn't a bad thing, and let the library know about it.

Kind Regards,

Vic

Victor Aprea // Wicked Device

victor...@wickeddevice.com

T: @vicatcu

[Paul Carpenter]

Commonly known as putting some form of scheduling higher up, which is
often timer based indirectly, putting another interrupt is not wise.

The common FIFO method is use of threshold interrupts as well, then when in
interrupt routine read as many characters as possible from FIFO. Thus reduce
interrupt overhead.

Often TX on Arduino platforms is a major problem unlike RX as small buffers
and the BLOCKING nature of Arduino core and libraries mean the system
spends time spin lock checking buffer empty.

To effectively change buffer size by just changing one define, bearing in
mind that CURRENT HardwareSerial implementation is ALL buffers MUST be the
same size due to implementation (poorly done). Which means that core has
larger buffer size it is same for all other projects.

If you want to change buffer sizes you need to bear in mind ALL
architectures and number of serial ports, means that even on Mega running
DMX type applications with 513 byte packets, all FOUR ports would have to
have 2 x 513 (or 512 byte) buffers, so 4kB of RAM gets swallowed up because
one port is using a large buffer and other serial ports may be in use.

How often you may need large TX buffers to spit data to other system but
rarely receive any data the other way.

Personally callback is a bad method as this just extends interrupt routines
BLOCKING everything else.

If you are relying on a ringbuffer for DMA modes then you are most likely
doing things the wrong way round.

The method of ringbuffer class does NOT change existing HardwareSerial API
it is a sub class, in some cases alias to similar functions (write),
sometimes wrapper function (print). RingBuffer deals with the interrupt
handling of read or write.

None of which is any use without ability to change buffer sizes on
project by project basis NOT on platform basis. Without by project build
you can extend and cause other problems to other projects that don't need
other functionality. In some cases with increasing buffer sizes will mean
different compile sizes (or run out of RAM) when same core build used on
different project. Buffer size is CURRENTLY determined in core files define
for most architectures.

> <mailto:pa...@pcserviceselectronics.co.uk>
> <mailto:paul@pcserviceselectro nics.co.uk

[Thomas Roell]

Victor,

I did some experiments with the STM32L4 core and ran into quite a few issues. Above a certain baudrate, a standard 64 byte buffer is not sufficient anymore, even if all the code in "loop()" does is to read data ... (I suspect some USB protocol handling throws a wrench into the "loop()" code). Increasing the size to 128 bytes provided enough buffering for the tight loop, but that is in no way a guarantee that it would be sufficient for a more sophisticated application ...

Conclusion is that the user needs to be able to specify a 2nd level buffer where RX data can go into automatically. TX is not really affected, as there are no overruns possible there.

The API that will be added now looks like this:

   HardwareSerial::begin(unsigned long baudrate, uint16_t config, void *buffer = NULL, unsigned int size = 0)

The idea to put that at "begin()" time is that putting in a new buffer is a tricky control flow that is best handled with the understanding that data WILL get lost. So why not put that there atomically before the UART starts receiving.

My earlier suggestions with a onReceive() type callback are somewhat orthogonal to that. Turns out that you really want to use the ARMV7M PENDSV mechanism to queue up a callback handler rather than to call it from the ISR level (because then the callback could conceivably block the whole I/O system). But given that there might be some delay between the queueing of the callback, and the execution it seems reasonable to still have a buffering mechanism.

- Thomas

[Paul Stoffregen]

On 01/13/2017 11:01 AM, Martino Facchin wrote:
> About using the capabilities of new processors, I once proposed to
> dynamically add storage to a RingBuffer instance to overcome
> define-based solutions.
> The code is attached, and the overhead is minimal. The core could keep
> safe low values for buffer sizes and the application can dynamically
> require a bigger one.

I like this approach. While a little extra overhead is added, it seems
the simplest, most Arduino-like way. In the absence of any official API
from Arduino and continued requests from users, I'm planning to
implement this on Teensy sometime "soon".

Any thoughts on what the public API ought to be? I see you used
"addStorage" in this patch, at least for the internal ring buffer. I
believe another time you mentioned "addMemory". I've been thinking of
patterning in similar to availableForWrite, perhaps:

Serial.addStorageForRead(void *buffer, size_t length);
Serial.addStorageForWrite(void *buffer, size_t length);

Or maybe:

Serial.addMemoryForRead(void *buffer, size_t length);
Serial.addMemoryForWrite(void *buffer, size_t length);

Ideally my hope is Arduino can define an official API, and *really*
ideal would be quickly adding do-nothing placeholders in 1.8.3 for
forward compatibility. I understand this is probably very
controversial, perhaps to the point of merely mentioning it again will
touch off another lengthy and unproductive thread encompassing nearly
every way everyone is dissatisfied with any aspect of Arduino. But
maybe, just maybe, consensus can build around an API and we can all work
together towards compatibility for Arduino users?

[Adrian Godwin]

Is it necessary to have multiple chained buffers with their related inefficiencies? 

I'd envisage just an optional argument in serial.begin to attach a buffer of a convenient size.

Sorry if this is rehashing an old argument.

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+unsubscribe@bcmi-labs.cc.

[Thomas Roell]

Ardian,

like this here:

    void begin(unsigned long baudRate);

    void begin(unsigned long baudrate, uint16_t config);

    void begin(unsigned long baudRate, uint8_t *buffer, size_t size);

    void begin(unsigned long baudrate, uint16_t config, uint8_t *buffer, size_t size);

Did not go via optional argument. Instead that begin() without the buffer calls the begin() with the buffer arguments, passing the default buffer, which is Uart instance dependent. This way you can hide the default buffers behind the API (perhaps only allocate them if needed ?)

- Thomas

[Tom Igoe]

Better to hide the buffers if possible. We’ve avoided pointers in the API calls where possible, and I think it’s good to continue that.  I like the simplicity of Adrian’s approach. There are many users that may know they need a bigger buffer without knowing the details of how to set up memory buffers, so doing that work under the hood for them and just asking how big they want is a good approach. Overloading for the advanced user seems OK, as long as we provide good examples. I’m still not clear on why the chaining is needed, though. 

t.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

[Thomas Roell]

Tom,

the problem with hiding buffer allocation is that you need to essentially go throu dynamic memory allocation (new() -> malloc() -> sbrk(), heap allocation). On ARM the heap grows towards the stack. It's hence easy for the user to oversubscribe the available memory.

Passing in pointers to the API (which is solving a rather special advanced problem) allows static allocation. Hence if you oversubscribe memory you'll get a link failure.

- Thomas

[Tom Igoe]

All the more reason I wouldn’t want to put the memory management in the hands of beginners, and makes me lean towards Adrian’s approach.

t.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

[J.C. Wren]

So why not make it flexible? If you pass in buffer as NULL, it will allocate a buffer of length bytes (returning an error if it fails, of course). If it's a user's static buffer, they pass in a non-void pointer.

--jc

Save a life; adopt a shelter animal.

[Todd Krein]

+1

Please, no pointers. 

Sent from my iPhone

[Thomas Roell]

void begin(unsigned long baudrate, uint16_t config, size_t size, uint8_t *buffer = NULL);

Surely doable. But that gives the user again too much rope to hang themselfs. And embedded system like Arduino should as much as possible avoid dynamic memory allocation at runtime. 

You also suck in the whole malloc() subsystem as a side-effect as soon as you use any "Serial*" then ...

- Thomas

[J.C. Wren]

"And embedded system like Arduino should as much as possible avoid dynamic memory allocation at runtime."

It's generally considered acceptable to use dynamic memory at startup, basically as a form of static allocation. It is indeed generally considered to be very bad form to malloc()/free() once the system is up.

Almost all Arduino programs allocate Serial objects either at instantiation or in setup(). I agree that if someone is destroying and re-creating Serial() it could create a problem. And, of course, you're right about it pulling in malloc() and friends. Serious question: how often does that occur anyway? I generally don't worry too much about that sort of thing, mostly because on a 256K+ ARM-based processor, it's so much more difficult to hit the wall than on a Atmel 3U24.

--jc

Save a life; adopt a shelter animal.

[Thomas Roell]

J.C., 

your comment about "setup()" is well taken. Some RTOSes that are certified for some safety critical standards have a switch after which they always fail dynamic memory allocation ...

Calling "Serial.begin()" is quite frequently done to change the baud-rate. Suppose you have a GPS that starts up in 9600, and then switches to 115200. 

If the "Serial.begin()" allocates, then you are in trouble if you call the interface too often.

Attaching a buffer via separate API is undesirable, as then you have no way to omit the allocation of the default rx/tx buffer. You can avoid the static allocation of the default buffers if you never call an API that reference them (like always a Serial.begin() with buffers pointed to).

- Thomas

[Rob Tillaart]

>> Calling "Serial.begin()" is quite frequently done to change the baud-rate. Suppose you have a GPS that starts up in 9600, and then switches to 115200. 

I read between the lines a equest for a call like  "Serial.setBaudrate(uint32_t baud); "

[Kurt Eckhardt]

For what it is worth:

 

Personally I for the most part don’t care what the API is, however I do think it is worthwhile to have some simple way for a user to specify per program a way to specify how much memory to use for different resources like devices.  And I personally believe in many cases it would be great, if no buffers were allocated for devices I don’t use.

 

That is suppose on the Teensy world, I develop a simple application for the Teensy LC, which I may use a reasonable amount of memory and Suppose this program does not use Serial2 or Serial3, how do I reclaim the memory that were allocated for this?  Likewise for all the Wire objects.    

 

If it were me, I would probably add a couple of methods like: setTXBufferSize() and setRXBufferSize(), which values when you first call begin… This would call so function like malloc.  I know  many don’t like malloc, I am not one of them, I don’t like free… That is I like the J.C. Wren I think it is fine to do allocations at startup.   If there are issues with malloc running into stack, maybe we should update the malloc function to know this and/or have some way to set the max size of the heap.

 

I believe this aversion to memory allocations hits in many places.  Example I am playing around with some Sparkfun Teensyview  display(SSD1306) 128x32, and I have another SSD1306 display which is 128x64.  If I use the Adafruit library, I have to change their header file to say that I am either using the 32 line one or 64 line one.   Why? Mainly to allocate a display buffer for it (plus a few minor init settings) and …

 

So suppose I wish to handle both?  Without using memory allocations, the two main ways of doing this, is:

1. Make two versions of the library one for 32 and other for 64… - Takes work to make different class names, hard to keep in sync…
2. Make one version that handles both and allocate the buffer for the largest one… Which wastes half the memory for the 32 line one.

So in my hacked up version of the Teensyview library, I use malloc to only allocate the size I need.

 

So again I think it would be great if a simple solution could be found.

 

Kurt

[Paul Stoffregen]

Several replies, one message...

On 05/03/2017 07:46 AM, Adrian Godwin wrote:

Is it necessary to have multiple chained buffers with their related inefficiencies? 

I'd envisage just an optional argument in serial.begin to attach a buffer of a convenient size.

I had imagined a sketch would only call these once, and the implementation would handle multiple calls by simply using the larger or last buffer.  But yeah, I can see how the name "addMemory" does imply chaining multiple buffers.

If there's a consensus for overloading Serial.begin(), that's perfectly fine with me.  I just want to *finally* provide this long requested feature to users.


On 05/03/2017 08:56 AM, Todd Krein wrote:

+1

Please, no pointers. 

To the Arduino user, this looks like ordinary arrays, not pointers.  It's the same as Serial.print(buffer, length) we have now.  Perhaps the use of pointer syntax wasn't the clearest way to state this proposal.  But please don't worry, to make use of this the user simply creates an array and gives it to this function.  No pointer syntax required.

On 05/03/2017 08:42 AM, Thomas Roell wrote:

the problem with hiding buffer allocation is that you need to essentially go throu dynamic memory allocation

No, you don't.  There's no need to use malloc or new.  If begin() is overloaded, you can simply use a static buffer in the case where no buffer is given.

Calling "Serial.begin()" is quite frequently done to change the baud-rate .... If the "Serial.begin()" allocates, then you are in trouble if you call the interface too often.

Then check if memory was already allocated to avoid redundant allocation.  Really, how hard is that?  Or init to use of a static buffer if the user hasn't provided their own buffer.

Thomas, you've implemented so many features in your custom core library, many of them using quite cunning techniques.  Surely you can imagine better implementation!

[Todd Krein]

Paul,

                Perfect. I’m happy…

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

[Thomas Roell]

Comments embedded.

- Thomas

On Wed, May 3, 2017 at 3:19 PM, Paul Stoffregen <pa...@pjrc.com> wrote:

On 05/03/2017 08:42 AM, Thomas Roell wrote:

the problem with hiding buffer allocation is that you need to essentially go throu dynamic memory allocation

No, you don't.  There's no need to use malloc or new.  If begin() is overloaded, you can simply use a static buffer in the case where no buffer is given.

Paul, I did suggest actually:

void begin(unsigned long baudrate, uint16_t config, uint8_t *buffer, size_t size);

In that case, no malloc is required. Somebody else commented that they'd rather not pass pointers in, so what about a "NULL" pointer for buffer means that "size" should be allocated. The latter one is the problem where you'd need malloc. I do absolutely dislike the idea of forcing dynamic allocation if one can handle it with static allocation as easily.

Calling "Serial.begin()" is quite frequently done to change the baud-rate .... If the "Serial.begin()" allocates, then you are in trouble if you call the interface too often.

Then check if memory was already allocated to avoid redundant allocation.  Really, how hard is that?  Or init to use of a static buffer if the user hasn't provided their own buffer.

That again is a comment towards "let-begin-allocate-the-buffer". The problem is that if somebody passes in a pointer to a chunk that does not come from malloc, and then the next time around a NULL pointer is passed in. How do you know you can free ... Do you free, or reallocate ? It just seems awfully complicated for a simple problem.

 

Thomas, you've implemented so many features in your custom core library, many of them using quite cunning techniques.  Surely you can imagine better implementation!

Actually, I do prefer simply passing in a pointer and be done. Surely one could come up with all complicated schemes that require a lot of even more complicated code. It just seems the problem has a simple and adequate solution, and that is passing in pointers.

[Andrew Kroll]

Easier yet, apply a -DSERBUFSIZE to EXTRA_FLAGS, no API changes
.

[Thomas Roell]

Andrew,

this is exactly what you don't want. Suppose you are using 3 Serial ports, and only one needs a bigger buffer ...

- Thomas

[Andrew Kroll]

use 3 flags

[Adrian Godwin]

On Wed, May 3, 2017 at 10:19 PM, Paul Stoffregen <pa...@pjrc.com> wrote:

Several replies, one message...


On 05/03/2017 07:46 AM, Adrian Godwin wrote:

Is it necessary to have multiple chained buffers with their related inefficiencies? 

I'd envisage just an optional argument in serial.begin to attach a buffer of a convenient size.

I had imagined a sketch would only call these once, and the implementation would handle multiple calls by simply using the larger or last buffer.  But yeah, I can see how the name "addMemory" does imply chaining multiple buffers.

I assumed they'd be multiple because there's be some default allocation and then another added block. If the original buffer were reclaimed that would be fine, but then it would have to use malloc rather than a static buffer.

[Thomas Roell]

STM32L4 has 6 serial ports ... That gets out of hand real soon.

- Thomas

[Andrew Kroll]

Not really. You have a default, and anything not defined gets the new value.

[Tom Igoe]

That's not a bad idea, Rob.

[Todd Krein]

+1

 

I can’t recall ever calling .begin() a second time for anything but a baudrate change.

 

From: Tom Igoe [mailto:t.i...@bcmi-labs.cc]
Sent: Wednesday, May 3, 2017 6:28 PM
To: devel...@bcmi-labs.cc
Subject: Re: [Developers] Hardware Serial Buffer Size

 

That's not a bad idea, Rob.

 

 

t.

 

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--

You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--

You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

[J.C. Wren]

I honestly never understood why there wasn't a call to change the baud rate. Closing and re-opening a port simply to change the baud rate is kinda of insane.

--jc

Save a life; adopt a shelter animal.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+unsubscribe@bcmi-labs.cc.

[Martino Facchin]

Overloading begin() is ok for me too; if we only "allow" static allocation we can also avoid asking the user to pass the size explicitly.

Since the underlying implementation of "modern" {Serial,Wire,SPI} buffers is a common RingBuffer, I'm pushing a PR on samd repo to implement this behaviour and continue the discussion there :)

[Rob Tillaart]

Looking through this discussion I see only small differences in implementation but definitely not in the goals.

Goal 1: keep is simple for the beginner

Goal 2: empower experienced users

To meet a request from Paul Stoffregen, find below the API I would like to propose for the (HW)Serial Class.

class HardwareSerial : public Stream

{

  public:

    // relevant part of existing API 

    // - stays same for backwards compatibility

    // - stays simple for the beginner

    void begin(unsigned long baud);

    void begin(unsigned long, uint8_t);

    // proposed API extensions

    // extreme simple for the beginners

    void begin();     // 9600, SERIAL_8N1  uses a default buffer size

    

    // get and set baudrate; getBaudRate() can possibly be an inliner

    // programmaticaly getting and setting the baudrate makes it possible

    // - to scan a device for its baudrate

    // - to "encrypt" communication by jumping in speed

    // - to find optimal speed for a given communication line runtime

    // - your favo serial hack :)

    void      setBaudRate(unsigned long);

    uint32_t  getBaudRate();

    // set and get the config

    // returns success or fail; params trivial - no magic #define SERIAL8N1

    bool      setConfig(uint8_t databits, char parity, uint8_t stopbits);

    // read them back

    uint8_t   getDataBits();

    char      getParity();

    uint8_t   getStopBits();

    // changes the buffers for Serial Class

    void      setTXBufferSize(uint32_t);  // allow buffers > 65K   (someone will find a purpose) BTW I can live with uint16_t 

    uint32_t  getTXBufferSize();

    void      setRXBufferSize(uint32_t);

    uint32_t  getRXBufferSize();

};

setting the buffersize in the begin() call is not good imho as one might need different sizes for send and receive.

putting both buffersizes in one begin() call gives the risk of swapping them, so better set them separately.

In fact I have done some projects where I only needed TX or RX but not both. So no need to allocate 

two buffers.

So the above API proposal leaves the exisiting intact and empowers power users. 

Opinions, remarks?

Regards,

Rob

[Tom Igoe]

Looks pretty good, except I’d make sure each of the data types are compatible with an Arduino type (int, long, etc). I didn’t see any that weren’t, at first blush.

t.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

[Thomas Roell]

Martino,

passing in a ringbuffer object is a really bad idea. There are many implementations not using this class. In our implementation the RX part of the UART is in a lower driver layer (ISR / PendSV / loop() concurrency issues).

Also by passing in a user accessible object you are opening up a can of worms, because now users could access the ringbuffer directly and bypass the Uart API. That would really mess up more advanced implementations.

In reality it should be either "pass down the requested size" (and deal with reallocation and dynamic allocation), or pass down a pointer/size pair.

- Thomas

[Thomas Roell]

Rob,

nice API, with a few exceptions.begin() already is defined with a "baudrate" and a "config". I kind of do like the idea of the config mask, as one can add additional features without having to introduce a million new APIs (like CTS/RTS). 

The point about separate rx/tx buffers is well taken. The major concern is simply, what do you do with the default buffers. The nice part about overloading "begin()" is that you can avoid references to the default buffer and hence avoid the allocation of those. 

There is a need for having different RX buffers (latency between when the data arrives and when you have time to process it). But the TX does not seem to be a good idea. If the problem that it solves is high-speed DMA transfers to Uart, why not adding async writes:

    bool write(const uint8_t *buffer, size_t size, void(*callback)(void));

    bool done(void);

If you are doing larger chunks, the extra copy is wasted time anyway. Putting data into a ringbuffer is inefficient to pull out via DMA to begin with. So why not leave the TX buffer at a smaller size and allow the power users to send large chunks directly ?

- Thomas

[Thomas Roell]

Martino,

thinking about this some more.

Perhaps the right idea is to have a "UartBufferClass" that can only be created with a "size" and destroyed, and has not other user accessible methods. This would allow an implementation to simply use it as a wrapper for a "RingBuffer" class if that's what's convenient. It would allow static and dynamic allocation, but under application control, rather than implicitly in "begin()".

The real interesting part is that this class could internally use a different private allocator. For MCUs where you have 2 sets of disjoint SRAM areas this private allocator could select the proper SRAM area that would have the required DMA properties, and deal with the proper alignment as well.

IMHO this approach would satisfy all requirements that various contributors brought up. No pointers, dynamic and static allocation, repeated calls to "begin()", default "begin()" uses the default allocation, no other interfaces and dependencies other than a raw buffer object.

- Thomas

[Tom Igoe]

As long as we're discussing an abstract class...

Given that serial gets wrapped in Stream, is there any sense to building the functionality were discussing into the Stream class? I find it a really useful class because it makes operations on serial and nework mostly interchangeable. I could also see similar arguments for changing the size of network buffers in some cases

[Kurt Eckhardt]

I personally like this approach.   I think you could easily like with 65K limit.   

 

I assume this requires some form of dynamic memory allocation, which I know that some dislike as it can at times make it difficult to locate issues like, the heap running into the stack.

 

But I can also see some other equally bad problems with passing in an object or pointer to buffer.

Example suppose you can pass in the buffer like in method, like setStorageForWrite()….

 

And the user does something like:

    Void setup() {

        Uint8_t My_Buffer[4000];

        Serial.setStorageForWrite(My_Buffer, sizeof(My_Buffer);

        …

    }

 

Needless to say having the buffer on the stack first off compiler most likely won’t catch if the value is too large, and 2^nd will most likely be clobbered, when the code exits setup.

 

Or suppose the user creates the buffer one size and passes in the size value larger than the buffer? 

Or suppose the user passes the same buffer to setStorageForWrite and setStorageFor Read?  Or likewise to multiple objects…

 

 

 

 

 

 

From: Rob Tillaart [mailto:rob.ti...@gmail.com]
Sent: Thursday, May 04, 2017 2:33 AM
To: devel...@bcmi-labs.cc
Subject: Re: [Developers] Hardware Serial Buffer Size

 

Looking through this discussion I see only small differences in implementation but definitely not in the goals.

[Thomas Roell]

Tom,

that is an interesting question. First off you suggest a "StreamBufferClass" which is usable directly for all derived objects.

The thing that I was wondering is whether different use cases have different constraints. Suppose your network code requires a 64 bit alignment and the size being multiples of 64 bytes, while the Uart use case has a 32 bit alignment and the size being multiples of 16 bytes. EABI on ARM requires almost a 64 bit alignement and the size being a multiple of 8 bytes. Is that good enough ?

- Thomas

[Todd Krein]

Interesting idea…

 

Can we have a default behavior that “does the right thing” for people without any special buffer requirements, or existing sketches?

 

t.

 

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--

You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--

You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

[Thomas Roell]

Todd,

the idea would be that we had a few apis:

    void begin(unsigned long baudRate);

    void begin(unsigned long baudrate, uint16_t config);

    void begin(unsigned long baudRate, const StreamBufferClass &buffer);

    void begin(unsigned long baudrate, uint16_t config, const StreamBufferClass &buffer);

If you call an API without explicit buffer passed in you'd use the system supplied one. That means existing code works as is. If you pass in a StreamBufferClass, then your RX buffer storage would be replaced by the new storage.

It undefined whether the begin() just takes the pointer and size, or keeps a reference to the StreamBufferClass object 

N.b. there are a lot of Arduino cores out there that have a RX ringbuffer, but no TX buffer (like SAMD). So the API extension shoud perhaps focus on that use case only.

- Thomas

To unsubscribe from this group and stop receiving emails from it, send an email to developers+unsubscribe@bcmi-labs.cc.

[Todd Krein]

Got it, thanks.

 

And after I hit return I saw the sample API that had been posted. I suppose my remaining question would be, for what cases does this *not* work? As someone else posted, we seem to be converging. Perhaps we’ve converged.

 

FWIW, I think this is a good approach. It will let me build my manufacturing test jigs a lot more robustly, and my 12-year-old can still make stuff work for school projects…

 

From: Thomas Roell [mailto:grumpyo...@gmail.com]
Sent: Thursday, May 4, 2017 11:28 AM
To: devel...@bcmi-labs.cc
Subject: Re: [Developers] Hardware Serial Buffer Size

 

Todd,

 

t.

 

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--

You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--

You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.

 

--
You received this message because you are subscribed to the Google Groups "Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@bcmi-labs.cc.