In commit 0e97bcb (Put each HardwareSerial instance in its own .cpp
file), the serial event handling was changed. This was probably a
copy-paste typo.
The effect of this bug was that SerialEvent3 would not run, unless
SerialEvent2 was defined, but also that if SerialEvent2 is defined but
SerialEvent3 is not, this could cause a reset (call to NULL pointer).
This closes#1967, thanks to Peter Olson for finding the bug and fix.
Added support for buffer sizes bigger than 256 bytes.
Added possibility to overrule the default size.
Added support for different size of TX and RX buffer sizes.
The default values remain the same. You can however specify a different
value for TX and RX buffer
Added possibility to overrule the default size.
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.
Because of the possibility to change the size of the buffer sizes longer
than 256 must be supported.
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
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
Members of this array are later passed to functions that accept
non-const pointers. These functions probably don't modify their
arguments, so a better solution would be to update those functions to
accept const pointers. However, they look like third-party code, so that
would require changing the code again on every update. Removing const
here fixes at least the compiler warning for now.
This helps towards #1792.
This makes the declaration of sprintf available, so the function is not
implicitely declared, which triggers two compiler warnings.
This helps towards #1792
A bunch of functions have parameters they do not use, but which cannot
be removed for API compatibility.
In syscalls_sam3.c, there are a lot of these, so this adds an "UNUSED"
macro which adds the "unused" variable attribute if supported (GCC
specific), or is just a noop on other compilers.
In CDC.cpp, there's only three of these variables, so this commit just
forces a dummy evaluation of them to suppress the warnings.
This helps towards #1792.
peekNextDigit() returns an int, so it can return -1 in addition to all
256 possible bytes. By putting the result in a signe char, all bytes
over 128 will be interpreted as "no bytes available". Furthermore, it
seems that on SAM "char" is unsigned by default, causing the
"if (c < 0)" line a bit further down to always be false.
Using an int is more appropriate.
A different fix for this issue was suggested in #1399. This fix helps
towards #1728.
In C++, true and false are language keywords, so there is no need to
define them as macros. Including stdbool.h in C++ effectively changes
nothing. In C, true, false and also the bool type are not available, but
including stdbool.h will make them available.
Using stdbool.h means that we get true, false and the bool type in
whatever way the compiler thinks is best, which seems like a good idea
to me.
This also fixes the following compiler warnings if a .c file includes
both stdbool.h and Arduino.h:
warning: "true" redefined [enabled by default]
#define true 0x1
warning: "false" redefined [enabled by default]
#define false 0x0
This fixes#1570 and helps toward fixing #1728.
This only changed the AVR core, the SAM core already doesn't define true
and false (but doesn't include stdbool.h either).
Previously, pointer casting was used, but this resulted in strict-aliasing warnings:
IPAddress.h: In member function ‘IPAddress::operator uint32_t() const’:
IPAddress.h:46:61: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
operator uint32_t() const { return *((uint32_t*)_address); };
^
IPAddress.h: In member function ‘bool IPAddress::operator==(const IPAddress&) const’:
IPAddress.h:47:81: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
bool operator==(const IPAddress& addr) const { return (*((uint32_t*)_address)) == (*((uint32_t*)addr._address)); };
^
IPAddress.h:47:114: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
bool operator==(const IPAddress& addr) const { return (*((uint32_t*)_address)) == (*((uint32_t*)addr._address)); };
Converting between unrelated types like this is commonly done using a union,
which do not break the strict-aliasing rules. Using that union, inside
IPAddress there is now an attribute _address.bytes for the raw byte
arra, or _address.dword for the uint32_t version.
Since we now have easy access to the uint32_t version, this also removes
two memcpy invocations that can just become assignments.
This patch does not change the generated code in any way, the compiler
already optimized away the memcpy calls and the previous casts mean
exactly the same.
This is a different implementation of a part of #1399 and it helps
toward fixing #1728.
The code used to say:
while (EFC0->EEFC_FSR & EEFC_FSR_FRDY == 0);
This triggered a compiler warning, which is why I looked at this line
more closely:
warning: suggest parentheses around comparison in operand of '&'
As the warning indicates, because the == operator has higher precedence
than the & operator, the compiler is interpreting this line as:
while (EFC0->EEFC_FSR & (EEFC_FSR_FRDY == 0));
Since EEFC_FSR_FRDY is defined as 1, (EEFC_FSR_FRDY == 0) is always
false (== 0) and this reduces to:
while (EFC0->EEFC_FSR & 0);
Which reduces to:
while (0);
So effectively this line is a no-op.
This commit adds parenthesis to restore the intended behaviour.
This was already fixed for HardwareSerial.cpp in #1863, but there was
one more case hidden in HardwareSerial_private.h.
The index attributes have been uint8_t for a while, so there is no point
in using int for local variables. This should allow the compiler to
generate slightly more efficient code, but (at least on gcc 4.8.2) it
also confuses the register allocator, causing this change to increase
code size by 2 bytes instead due to extra push/pop instructions (but
this will probably change in the future if the compiler improves).
"readed" is no longer incremented in requestFrom() if
TWI_WaitByteReceived() gets a NACK or times out. This corrects the
behavior (return values) of requestFrom() and available() to match the
Arduino reference. Fixesarduino/Arduino#1311
The index attributes have been uint8_t for a while, so there is no point
in using int for local variables. This should allow the compiler to
generate slightly more efficient code, but (at least on gcc 4.8.2) it
also confuses the register allocator, causing this change to increase
code size by 2 bytes instead due to extra push/pop instructions (but
this will probably change in the future if the compiler improves).
Switch the tx and rx buffer head/tail entries in the HardwareSerial
initialisation list so that they match the order the fields are defined
in. This fixes a compiler warning (repeated for each of the
HardwareSerial source files the header is used in).
This helps improve the effective datarate on high (>500kbit/s) bitrates,
by skipping the interrupt and associated overhead. At 1 Mbit/s the
implementation previously got up to about 600-700 kbit/s, but now it
actually gets up to the 1Mbit/s (values are rough estimates, though).
Moreover, declaring pointers-to-registers as const and using initializer
list in class constructor allows the compiler to further improve inlining
performance.
This change recovers about 50 bytes of program space on single-UART devices.
See #1711
By putting the ISRs and HardwareSerial instance for each instance in a
separate compilation unit, the compile will only consider them for
linking when the instance is actually used. The ISR is always referenced
by the compiler runtime and the Serialx_available() function is always
referenced by SerialEventRun(), but both references are weak and thus do
not cause the compilation to be included in the link by themselves.
The effect of this is that when multiple HardwareSerial ports are
available, but not all are used, buffers are only allocated and ISRs are
only included for the serial ports that are used. On the mega, this
lowers memory usage from 653 bytes to just 182 when only using the first
serial port.
On boards with just a single port, there is no change, since the code
and memory was already left out when no serial port was used at all.
This fixes#1425 and fixes#1259.
Before, this decision was made in few different places, based on
sometimes different register defines.
Now, HardwareSerial.h decides wich UARTS are available, defines
USE_HWSERIALn macros and HardwareSerial.cpp simply checks these macros
(together with some #ifs to decide which registers to use for UART 0).
For consistency, USBAPI.h also defines a HAVE_CDCSERIAL macro when
applicable.
For supported targets, this should change any behaviour. For unsupported
targets, the error messages might subtly change because some checks are
moved or changed.
Additionally, this moves the USBAPI.h include form HardareSerial.h into
Arduino.h and raises an error when both CDC serial and UART0 are
available (previously this would silently use UART0 instead of CDC, but
there is not currently any Atmel chip available for which this would
occur).
Before, the interrupt was disabled when it was triggered and it turned
out there was no data to send. However, the interrupt can be disabled
already when the last byte is written to the UART, since write() will
always re-enable the interrupt when it adds new data to the buffer.
Closes: #1008
When interrupts are disabled, writing to HardwareSerial could cause a
lockup. When the tx buffer is full, a busy-wait loop is used to wait for
the interrupt handler to free up a byte in the buffer. However, when
interrupts are disabled, this will of course never happen and the
Arduino will lock up. This often caused lockups when doing (big) debug
printing from an interrupt handler.
Additionally, calling flush() with interrupts disabled while
transmission was in progress would also cause a lockup.
When interrupts are disabled, the code now actively checks the UDRE
(UART Data Register Empty) and calls the interrupt handler to free up
room if the bit is set.
This can lead to delays in interrupt handlers when the serial buffer is
full, but a delay is of course always preferred to a lockup.
Closes: #672
References: #1147
It turns out there is an additional corner case. The analysis in the
previous commit wrt to flush() assumes that the data register is always
kept filled by the interrupt handler, so the TXC bit won't get set until
all the queued bytes have been transmitted. But, when interrupts are
disabled for a longer period (for example when an interrupt handler for
another device is running for longer than 1-2 byte times), it could
happen that the UART stops transmitting while there are still more bytes
queued (but these are in the buffer, not in the UDR register, so the
UART can't know about them).
In this case, the TXC bit would get set, but the transmission is not
complete yet. We can easily detect this case by looking at the head and
tail pointers, but it seems easier to instead look at the UDRIE bit
(the TX interrupt is enabled if and only if there are bytes in the
queue). To fix this corner case, this commit:
- Checks the UDRIE bit and only if it is unset, looks at the TXC bit.
- Moves the clearing of TXC from write() to the tx interrupt handler.
This (still) causes the TXC bit to be cleared whenever a byte is
queued when the buffer is empty (in this case the tx interrupt will
trigger directly after write() is called). It also causes the TXC bit
to be cleared whenever transmission is resumed after it halted
because interrupts have been disabled for too long.
As a side effect, another race condition is prevented. This could occur
at very high bitrates, where the transmission would be completed before
the code got time to clear the TXC0 register, making the clear happen
_after_ the transmission was already complete. With the new code, the
clearing of TXC happens directly after writing to the UDR register,
while interrupts are disabled, and we can be certain the data
transmission needs more time than one instruction to complete. This
fixes#1463 and replaces #1456.
The flush() method blocks until all characters in the serial buffer have
been written to the uart _and_ transmitted. This is checked by waiting
until the "TXC" (TX Complete) bit is set by the UART, signalling
completion. This bit is cleared by write() when adding a new byte to the
buffer and set by the hardware after tranmission ends, so it is always
guaranteed to be zero from the moment the first byte in a sequence is
queued until the moment the last byte is transmitted, and it is one from
the moment the last byte in the buffer is transmitted until the first
byte in the next sequence is queued.
However, the TXC bit is also zero from initialization to the moment the
first byte ever is queued (and then continues to be zero until the first
sequence of bytes completes transmission). Unfortunately we cannot
manually set the TXC bit during initialization, we can only clear it. To
make sure that flush() would not (indefinitely) block when it is called
_before_ anything was written to the serial device, the "transmitting"
variable was introduced.
This variable suggests that it is only true when something is
transmitting, which isn't currently the case (it remains true after
transmission is complete until flush() is called, for example).
Furthermore, there is no need to keep the status of transmission, the
only thing needed is to remember if anything has ever been written, so
the corner case described above can be detected.
This commit improves the code by:
- Renaming the "transmitting" variable to _written (making it more
clear and following the leading underscore naming convention).
- Not resetting the value of _written at the end of flush(), there is
no point to this.
- Only checking the "_written" value once in flush(), since it can
never be toggled off anyway.
- Initializing the value of _written in both versions of _begin (though
it probably gets initialized to 0 by default anyway, better to be
explicit).
