From 0a0cda4f04844cc209c0523ba59e8a2f47fc502e Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 16 Jan 2014 13:50:59 +0100 Subject: Slightly reduce code utilization by inlining HardwareSerail begin(baud) and operator bool() --- cores/arduino/HardwareSerial.h | 56 +++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 28 deletions(-) (limited to 'cores/arduino/HardwareSerial.h') diff --git a/cores/arduino/HardwareSerial.h b/cores/arduino/HardwareSerial.h index 0f62262..6dba734 100644 --- a/cores/arduino/HardwareSerial.h +++ b/cores/arduino/HardwareSerial.h @@ -37,6 +37,32 @@ #define SERIAL_BUFFER_SIZE 64 #endif +// Define config for Serial.begin(baud, config); +#define SERIAL_5N1 0x00 +#define SERIAL_6N1 0x02 +#define SERIAL_7N1 0x04 +#define SERIAL_8N1 0x06 +#define SERIAL_5N2 0x08 +#define SERIAL_6N2 0x0A +#define SERIAL_7N2 0x0C +#define SERIAL_8N2 0x0E +#define SERIAL_5E1 0x20 +#define SERIAL_6E1 0x22 +#define SERIAL_7E1 0x24 +#define SERIAL_8E1 0x26 +#define SERIAL_5E2 0x28 +#define SERIAL_6E2 0x2A +#define SERIAL_7E2 0x2C +#define SERIAL_8E2 0x2E +#define SERIAL_5O1 0x30 +#define SERIAL_6O1 0x32 +#define SERIAL_7O1 0x34 +#define SERIAL_8O1 0x36 +#define SERIAL_5O2 0x38 +#define SERIAL_6O2 0x3A +#define SERIAL_7O2 0x3C +#define SERIAL_8O2 0x3E + class HardwareSerial : public Stream { protected: @@ -70,7 +96,7 @@ class HardwareSerial : public Stream volatile uint8_t *ucsra, volatile uint8_t *ucsrb, volatile uint8_t *ucsrc, volatile uint8_t *udr, uint8_t rxen, uint8_t txen, uint8_t rxcie, uint8_t udrie, uint8_t u2x); - void begin(unsigned long); + void begin(unsigned long baud) { begin(baud, SERIAL_8N1); } void begin(unsigned long, uint8_t); void end(); virtual int available(void); @@ -83,35 +109,9 @@ class HardwareSerial : public Stream inline size_t write(unsigned int n) { return write((uint8_t)n); } inline size_t write(int n) { return write((uint8_t)n); } using Print::write; // pull in write(str) and write(buf, size) from Print - operator bool(); + operator bool() { return true; } }; -// Define config for Serial.begin(baud, config); -#define SERIAL_5N1 0x00 -#define SERIAL_6N1 0x02 -#define SERIAL_7N1 0x04 -#define SERIAL_8N1 0x06 -#define SERIAL_5N2 0x08 -#define SERIAL_6N2 0x0A -#define SERIAL_7N2 0x0C -#define SERIAL_8N2 0x0E -#define SERIAL_5E1 0x20 -#define SERIAL_6E1 0x22 -#define SERIAL_7E1 0x24 -#define SERIAL_8E1 0x26 -#define SERIAL_5E2 0x28 -#define SERIAL_6E2 0x2A -#define SERIAL_7E2 0x2C -#define SERIAL_8E2 0x2E -#define SERIAL_5O1 0x30 -#define SERIAL_6O1 0x32 -#define SERIAL_7O1 0x34 -#define SERIAL_8O1 0x36 -#define SERIAL_5O2 0x38 -#define SERIAL_6O2 0x3A -#define SERIAL_7O2 0x3C -#define SERIAL_8O2 0x3E - #if defined(UBRRH) || defined(UBRR0H) extern HardwareSerial Serial; #elif defined(USBCON) -- cgit v1.2.3-18-g5258 From 3babfc2a855e49c5781f433fb5bd1227c8161dd8 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 18 Apr 2013 14:17:47 +0200 Subject: Use constants for register bit positions in HardwareSerial 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. --- cores/arduino/HardwareSerial.h | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'cores/arduino/HardwareSerial.h') diff --git a/cores/arduino/HardwareSerial.h b/cores/arduino/HardwareSerial.h index 6dba734..5513226 100644 --- a/cores/arduino/HardwareSerial.h +++ b/cores/arduino/HardwareSerial.h @@ -72,11 +72,6 @@ class HardwareSerial : public Stream volatile uint8_t *_ucsrb; volatile uint8_t *_ucsrc; volatile uint8_t *_udr; - uint8_t _rxen; - uint8_t _txen; - uint8_t _rxcie; - uint8_t _udrie; - uint8_t _u2x; bool transmitting; public: @@ -94,8 +89,7 @@ class HardwareSerial : public Stream HardwareSerial( volatile uint8_t *ubrrh, volatile uint8_t *ubrrl, volatile uint8_t *ucsra, volatile uint8_t *ucsrb, - volatile uint8_t *ucsrc, volatile uint8_t *udr, - uint8_t rxen, uint8_t txen, uint8_t rxcie, uint8_t udrie, uint8_t u2x); + volatile uint8_t *ucsrc, volatile uint8_t *udr); void begin(unsigned long baud) { begin(baud, SERIAL_8N1); } void begin(unsigned long, uint8_t); void end(); -- cgit v1.2.3-18-g5258 From 80d6af62730f11f9f23ec7a14d69bc6cfa01dd03 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 18 Apr 2013 11:38:13 +0200 Subject: Move interrupt handlers into HardwareSerial class 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. --- cores/arduino/HardwareSerial.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'cores/arduino/HardwareSerial.h') diff --git a/cores/arduino/HardwareSerial.h b/cores/arduino/HardwareSerial.h index 5513226..cf3f6bd 100644 --- a/cores/arduino/HardwareSerial.h +++ b/cores/arduino/HardwareSerial.h @@ -74,7 +74,6 @@ class HardwareSerial : public Stream volatile uint8_t *_udr; bool transmitting; - public: volatile uint8_t _rx_buffer_head; volatile uint8_t _rx_buffer_tail; volatile uint8_t _tx_buffer_head; @@ -86,6 +85,7 @@ class HardwareSerial : public Stream unsigned char _rx_buffer[SERIAL_BUFFER_SIZE]; unsigned char _tx_buffer[SERIAL_BUFFER_SIZE]; + public: HardwareSerial( volatile uint8_t *ubrrh, volatile uint8_t *ubrrl, volatile uint8_t *ucsra, volatile uint8_t *ucsrb, @@ -104,6 +104,10 @@ class HardwareSerial : public Stream inline size_t write(int n) { return write((uint8_t)n); } using Print::write; // pull in write(str) and write(buf, size) from Print operator bool() { return true; } + + // Interrupt handlers - Not intended to be called externally + void _rx_complete_irq(void); + void _tx_udr_empty_irq(void); }; #if defined(UBRRH) || defined(UBRR0H) -- cgit v1.2.3-18-g5258 From 560295c983d6def24021ac94c40d7bbc8912c1ea Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 18 Apr 2013 21:12:01 +0200 Subject: Improve HardwareSerial::flush() 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). --- cores/arduino/HardwareSerial.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'cores/arduino/HardwareSerial.h') diff --git a/cores/arduino/HardwareSerial.h b/cores/arduino/HardwareSerial.h index cf3f6bd..0ae51f4 100644 --- a/cores/arduino/HardwareSerial.h +++ b/cores/arduino/HardwareSerial.h @@ -72,7 +72,8 @@ class HardwareSerial : public Stream volatile uint8_t *_ucsrb; volatile uint8_t *_ucsrc; volatile uint8_t *_udr; - bool transmitting; + // Has any byte been written to the UART since begin() + bool _written; volatile uint8_t _rx_buffer_head; volatile uint8_t _rx_buffer_tail; -- cgit v1.2.3-18-g5258 From 99f7ef7c673e02d559eb55d064f38a105393c8f6 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Sun, 1 Dec 2013 17:21:54 +0100 Subject: Centrally decide which hardware UARTS are available 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). --- cores/arduino/HardwareSerial.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'cores/arduino/HardwareSerial.h') diff --git a/cores/arduino/HardwareSerial.h b/cores/arduino/HardwareSerial.h index 0ae51f4..da9efbc 100644 --- a/cores/arduino/HardwareSerial.h +++ b/cores/arduino/HardwareSerial.h @@ -113,18 +113,19 @@ class HardwareSerial : public Stream #if defined(UBRRH) || defined(UBRR0H) extern HardwareSerial Serial; -#elif defined(USBCON) - #include "USBAPI.h" -// extern HardwareSerial Serial_; + #define HAVE_HWSERIAL0 #endif #if defined(UBRR1H) extern HardwareSerial Serial1; + #define HAVE_HWSERIAL1 #endif #if defined(UBRR2H) extern HardwareSerial Serial2; + #define HAVE_HWSERIAL2 #endif #if defined(UBRR3H) extern HardwareSerial Serial3; + #define HAVE_HWSERIAL3 #endif extern void serialEventRun(void) __attribute__((weak)); -- cgit v1.2.3-18-g5258 From 49fc2ab8ad43b86f7668da8d0186abe0d48cd6d9 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 22 Jan 2014 10:12:56 +0100 Subject: Inlined HardwareSerial calls to RX ISR. 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 --- cores/arduino/HardwareSerial.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'cores/arduino/HardwareSerial.h') diff --git a/cores/arduino/HardwareSerial.h b/cores/arduino/HardwareSerial.h index da9efbc..bd3c4c4 100644 --- a/cores/arduino/HardwareSerial.h +++ b/cores/arduino/HardwareSerial.h @@ -66,12 +66,12 @@ class HardwareSerial : public Stream { protected: - volatile uint8_t *_ubrrh; - volatile uint8_t *_ubrrl; - volatile uint8_t *_ucsra; - volatile uint8_t *_ucsrb; - volatile uint8_t *_ucsrc; - volatile uint8_t *_udr; + volatile uint8_t * const _ubrrh; + volatile uint8_t * const _ubrrl; + volatile uint8_t * const _ucsra; + volatile uint8_t * const _ucsrb; + volatile uint8_t * const _ucsrc; + volatile uint8_t * const _udr; // Has any byte been written to the UART since begin() bool _written; @@ -87,7 +87,7 @@ class HardwareSerial : public Stream unsigned char _tx_buffer[SERIAL_BUFFER_SIZE]; public: - HardwareSerial( + inline HardwareSerial( volatile uint8_t *ubrrh, volatile uint8_t *ubrrl, volatile uint8_t *ucsra, volatile uint8_t *ucsrb, volatile uint8_t *ucsrc, volatile uint8_t *udr); @@ -107,7 +107,7 @@ class HardwareSerial : public Stream operator bool() { return true; } // Interrupt handlers - Not intended to be called externally - void _rx_complete_irq(void); + inline void _rx_complete_irq(void); void _tx_udr_empty_irq(void); }; -- cgit v1.2.3-18-g5258