The actual interrupt vectors are of course defined as before, but they
let new methods in the HardwareSerial class do the actual work. This
greatly reduces code duplication and prepares for one of my next commits
which requires the tx interrupt handler to be called from another
context as well.
The actual content of the interrupts handlers was pretty much identical,
so that remains unchanged (except that store_char was now only needed
once, so it was inlined).
Now all access to the buffers are inside the HardwareSerial class, the
buffer variables can be made private.
One would expect a program size reduction from this change (at least
with multiple UARTs), but due to the fact that the interrupt handlers
now only have indirect access to a few registers (which previously were
just hardcoded in the handlers) and because there is some extra function
call overhead, the code size on the uno actually increases by around
70 bytes. On the mega, which has four UARTs, the code size decreases by
around 70 bytes.
Previously, the constants to use for the bit positions of the various
UARTs were passed to the HardwareSerial constructor. However, this
meant that whenever these values were used, the had to be indirectly
loaded, resulting in extra code overhead. Additionally, since there is
no instruction to shift a value by a variable amount, the 1 << x
expressions (inside _BV and sbi() / cbi()) would be compiled as a loop
instead of being evaluated at compiletime.
Now, the HardwareSerial class always uses the constants for the bit
positions of UART 0 (and some code is present to make sure these
constants exist, even for targets that only have a single unnumbered
UART or start at UART1).
This was already done for the TXC0 constant, for some reason. For the
actual register addresses, this approach does not work, since these are
of course different between the different UARTs on a single chip.
Of course, always using the UART 0 constants is only correct when the
constants are actually identical for the different UARTs. It has been
verified that this is currently the case for all targets supported by
avr-gcc 4.7.2, and the code contains compile-time checks to verify this
for the current target, in case a new target is added for which this
does not hold. This verification was done using:
for i in TXC RXEN TXEN RXCIE UDRIE U2X UPE; do echo $i; grep --no-filename -r "#define $i[0-9]\? " /usr/lib/avr/include/avr/io* | sed "s/#define $i[0-9]\?\s*\(\S\)\+\s*\(\/\*.*\*\/\)\?$/\1/" | sort | uniq ; done
This command shows that the above constants are identical for all uarts
on all platforms, except for TXC, which is sometimes 6 and sometimes 0.
Further investigation shows that it is always 6, except in io90scr100.h,
but that file defines TXC0 with value 6 for the UART and uses TXC with
value 0 for some USB-related register.
This commit reduces program size on the uno by around 120 bytes.
This simplifies the baud rate calculation, removing the need for a goto
and shortening the code a bit. Other than that, this code should not use
any different settings than before.
Code was suggested by Rob Tillaart on github.
Closes: #1262
There are two begin methods, one which accepts just a baud rate and
uses the default bit settings and one which accepts both a baudrate and
a bit config. Previously, both of these contained a complete
implementation, but now the former just calls the latter, explicitely
passing the default 8N1 configuration.
Technically, this causes a small change: Before the UCSRC register was
untouched when calling begin(baud), now it is explicitely initialized
with 8N1. However, since this is the default configuration for at least
the Uno and the Mega (didn't check any others), probably for all avrs,
this shouldn't effectively change anything. Given that the Arduino
documentation also documents this as the default when none is passed,
explicitly setting it is probably a good idea in any case.
These functions do not modify the IPAddress object, but were not marked
as const. This meant that you could not do:
void set_ip(const IPAddress& ip) {
uint32_t copy = ip;
}
Since calling operator uint32_t() on ip would discard the constness of
the reference.
On later versions of avr-libc, prog_char is deprecated. In 0acebeeff48
the one occurence of prog_char was replaced by "char PROGMEM", which is
not entirely correct (PROGMEM is supposed to be an attribute on a
variable, not on a type, even though this is how things work in older
libc versions). However, in 1130fede3a2 a few new occurences of
prog_char are introduced, which break compilation on newer libc versions
again.
This commit changes all these pointer types to use the PGM_P macro from
<avr/pgmspace.h>. This macro is just "const char *" in newer libc
versions and "const prog_char *" in older versions, so it should always
work.
References #795
The new function just calls Print::write(const uint8_t *, size_t), but
this allows writing out a buffer of chars (without having to learn about
casts).
This removes the need for doing an extra pointer dereference on every
access to the buffers, shrinking the code by around 100 bytes.
The members for these buffers must be public for now, since the
interrupt handlers also need to access them. These can later be made
private again.
Furthermore, the struct ring_buffer was removed. This allows the all
head and tail pointers to be put into the HardwareSerial struct before
the actual buffers, so the pointers all end up in the first 32 bytes of
the struct that can be accessed using a single instruction (ldd).
References: #947
Since the buffers aren't bigger than 64 bytes, these values can be
smaller. This saves a few bytes of ram, but also saves around 50 bytes
of program space, since the values can now be loaded using a single
instruction.
To prevent problems when people manually increase the buffer size, a
compile-time check is added.
Closes: #1078
Matthijs Kooijman
Cristian Maglie
Federico Fissore
Kristian Lauszus
Paul Brook
Angus Gratton