Support for different size of HardwareSerial TX and RX buffers.

[jantje]

This relates to issue #1929
Added support for different size of TX and RX buffer sizes.
Added support for buffer sizes bigger than 256 bytes.

Added support for different size of TX and RX buffer sizes.
The default values remain the same. If you want to have different values
define SERIAL_TX_BUFFER_SIZE and SERIAL_RX_BUFFER_SIZE on the command
line

Added support for buffer sizes bigger than 256 bytes.
The type of the indexes is decided upon the size of the buffers. So
there is no increase in program/data size when the buffers are smaller
than 257

[matthijskooijman]

The gist of the commit looks good to me. I've left some comments inline in the commit.

As you are saying, your commit contains two changes. I'd rather see them as two separate commits, since these changes are really different and unrelated. Could you separate them?

[jantje]

Actually there are 3 changes.
I foregot to mention the possibility to overrule the default size.
It is due to this that you need the different types for the index.
So to me they are kind of one "possibility to dynamically size the hardware serial buffers".
I could split them but I'd prefer them as one go. The main reason is that if I split them I need to test more.

I think there should be some whitespace around && here.

The white space around && is not needed (code runs like that on my system) but it makes the code more readable.

Shouldn't this be > 256 ?
yes

[matthijskooijman]

I foregot to mention the possibility to overrule the default size.

Oh, indeed.

I could split them but I'd prefer them as one go. The main reason is that if I split them I need to test more.

True. Also, in the end it's the Arduino team that gets to decide, not me :-)

If you keep it as one commit, perhaps you could change the first line (which should be a useful summary by itself) to something like "Improve HardwareSerial buffer size handling"?

The white space around && is not needed (code runs like that on my system) but it makes the code more readable.

Yeah, that's what I meant, sorry if I was unclear.

[jantje]

I did the changes as you proposed. Lets hope the arduino core team takes this one aboard.

[matthijskooijman]

Changes look good to me, I have no further remarks on the code. Regarding the commits, you should probably squash the two commits together, using the commit message of the second one, or perhaps the core team can just do that when merging this request.

Thanks!

[jantje]

can the core team please comment on whether this could be accepted or not?

[cmaglie]

The patch looks good, but it gives me a compile error if I compile the AsciiTableExample for the Uno Board:

In file included from /home/megabug/git/arduino/build/linux/work/hardware/arduino/avr/cores/arduino/HardwareSerial2.cpp:27:
/home/megabug/git/arduino/build/linux/work/hardware/arduino/avr/cores/arduino/HardwareSerial_private.h: In member function ‘void HardwareSerial::_rx_complete_irq()’:
/home/megabug/git/arduino/build/linux/work/hardware/arduino/avr/cores/arduino/HardwareSerial_private.h:102: error: ‘SERIAL_BUFFER_SIZE’ was not declared in this scope

probably you forget to update HardwareSerial_private.h with the new constants name.
If you fix that I'll merge your pull request straight away.

C

[jantje]

I forgot to upload the HardwareSerial_private.h file.
I added it now.
Thanks for processing the request.
Jantje

[cmaglie]

Thanks, I've added just a little change to the variable type for index calculation.

1ad74ce

[jantje]

thanks indeed I overlooked that one.