Skip to content

Commit 275c0a0

Browse files
cmagliecmaglie
committed
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
1 parent 0e97bcb commit 275c0a0

File tree

7 files changed

+108
-98
lines changed

7 files changed

+108
-98
lines changed

Diff for: ‎hardware/arduino/avr/cores/arduino/HardwareSerial.cpp

+1-90
Original file line numberDiff line numberDiff line change
@@ -26,64 +26,14 @@
2626
#include <string.h>
2727
#include <inttypes.h>
2828
#include "Arduino.h"
29-
#include "wiring_private.h"
3029

3130
#include "HardwareSerial.h"
31+
#include "HardwareSerial_private.h"
3232

3333
// this next line disables the entire HardwareSerial.cpp,
3434
// this is so I can support Attiny series and any other chip without a uart
3535
#if defined(HAVE_HWSERIAL0) || defined(HAVE_HWSERIAL1) || defined(HAVE_HWSERIAL2) || defined(HAVE_HWSERIAL3)
3636

37-
// Ensure that the various bit positions we use are available with a 0
38-
// postfix, so we can always use the values for UART0 for all UARTs. The
39-
// alternative, passing the various values for each UART to the
40-
// HardwareSerial constructor also works, but makes the code bigger and
41-
// slower.
42-
#if !defined(TXC0)
43-
#if defined(TXC)
44-
// On ATmega8, the uart and its bits are not numbered, so there is no TXC0 etc.
45-
#define TXC0 TXC
46-
#define RXEN0 RXEN
47-
#define TXEN0 TXEN
48-
#define RXCIE0 RXCIE
49-
#define UDRIE0 UDRIE
50-
#define U2X0 U2X
51-
#define UPE0 UPE
52-
#define UDRE0 UDRE
53-
#elif defined(TXC1)
54-
// Some devices have uart1 but no uart0
55-
#define TXC0 TXC1
56-
#define RXEN0 RXEN1
57-
#define TXEN0 TXEN1
58-
#define RXCIE0 RXCIE1
59-
#define UDRIE0 UDRIE1
60-
#define U2X0 U2X1
61-
#define UPE0 UPE1
62-
#define UDRE0 UDRE1
63-
#else
64-
#error No UART found in HardwareSerial.cpp
65-
#endif
66-
#endif // !defined TXC0
67-
68-
// Check at compiletime that it is really ok to use the bit positions of
69-
// UART0 for the other UARTs as well, in case these values ever get
70-
// changed for future hardware.
71-
#if defined(TXC1) && (TXC1 != TXC0 || RXEN1 != RXEN0 || RXCIE1 != RXCIE0 || \
72-
UDRIE1 != UDRIE0 || U2X1 != U2X0 || UPE1 != UPE0 || \
73-
UDRE1 != UDRE0)
74-
#error "Not all bit positions for UART1 are the same as for UART0"
75-
#endif
76-
#if defined(TXC2) && (TXC2 != TXC0 || RXEN2 != RXEN0 || RXCIE2 != RXCIE0 || \
77-
UDRIE2 != UDRIE0 || U2X2 != U2X0 || UPE2 != UPE0 || \
78-
UDRE2 != UDRE0)
79-
#error "Not all bit positions for UART2 are the same as for UART0"
80-
#endif
81-
#if defined(TXC3) && (TXC3 != TXC0 || RXEN3 != RXEN0 || RXCIE3 != RXCIE0 || \
82-
UDRIE3 != UDRIE0 || U3X3 != U3X0 || UPE3 != UPE0 || \
83-
UDRE3 != UDRE0)
84-
#error "Not all bit positions for UART3 are the same as for UART0"
85-
#endif
86-
8737
// SerialEvent functions are weak, so when the user doesn't define them,
8838
// the linker just sets their address to 0 (which is checked below).
8939
// The Serialx_available is just a wrapper around Serialx.available(),
@@ -127,28 +77,6 @@ void serialEventRun(void)
12777

12878
// Actual interrupt handlers //////////////////////////////////////////////////////////////
12979

130-
void HardwareSerial::_rx_complete_irq(void)
131-
{
132-
if (bit_is_clear(*_ucsra, UPE0)) {
133-
// No Parity error, read byte and store it in the buffer if there is
134-
// room
135-
unsigned char c = *_udr;
136-
int i = (unsigned int)(_rx_buffer_head + 1) % SERIAL_BUFFER_SIZE;
137-
138-
// if we should be storing the received character into the location
139-
// just before the tail (meaning that the head would advance to the
140-
// current location of the tail), we're about to overflow the buffer
141-
// and so we don't write the character or advance the head.
142-
if (i != _rx_buffer_tail) {
143-
_rx_buffer[_rx_buffer_head] = c;
144-
_rx_buffer_head = i;
145-
}
146-
} else {
147-
// Parity error, read byte but discard it
148-
unsigned char c = *_udr;
149-
};
150-
}
151-
15280
void HardwareSerial::_tx_udr_empty_irq(void)
15381
{
15482
// If interrupts are enabled, there must be more data in the output
@@ -169,23 +97,6 @@ void HardwareSerial::_tx_udr_empty_irq(void)
16997
}
17098
}
17199

172-
// Constructors ////////////////////////////////////////////////////////////////
173-
174-
HardwareSerial::HardwareSerial(
175-
volatile uint8_t *ubrrh, volatile uint8_t *ubrrl,
176-
volatile uint8_t *ucsra, volatile uint8_t *ucsrb,
177-
volatile uint8_t *ucsrc, volatile uint8_t *udr)
178-
{
179-
_tx_buffer_head = _tx_buffer_tail = 0;
180-
_rx_buffer_head = _rx_buffer_tail = 0;
181-
_ubrrh = ubrrh;
182-
_ubrrl = ubrrl;
183-
_ucsra = ucsra;
184-
_ucsrb = ucsrb;
185-
_ucsrc = ucsrc;
186-
_udr = udr;
187-
}
188-
189100
// Public Methods //////////////////////////////////////////////////////////////
190101

191102
void HardwareSerial::begin(unsigned long baud, byte config)

Diff for: ‎hardware/arduino/avr/cores/arduino/HardwareSerial.h

+8-8
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,12 @@
6666
class HardwareSerial : public Stream
6767
{
6868
protected:
69-
volatile uint8_t *_ubrrh;
70-
volatile uint8_t *_ubrrl;
71-
volatile uint8_t *_ucsra;
72-
volatile uint8_t *_ucsrb;
73-
volatile uint8_t *_ucsrc;
74-
volatile uint8_t *_udr;
69+
volatile uint8_t * const _ubrrh;
70+
volatile uint8_t * const _ubrrl;
71+
volatile uint8_t * const _ucsra;
72+
volatile uint8_t * const _ucsrb;
73+
volatile uint8_t * const _ucsrc;
74+
volatile uint8_t * const _udr;
7575
// Has any byte been written to the UART since begin()
7676
bool _written;
7777

@@ -87,7 +87,7 @@ class HardwareSerial : public Stream
8787
unsigned char _tx_buffer[SERIAL_BUFFER_SIZE];
8888

8989
public:
90-
HardwareSerial(
90+
inline HardwareSerial(
9191
volatile uint8_t *ubrrh, volatile uint8_t *ubrrl,
9292
volatile uint8_t *ucsra, volatile uint8_t *ucsrb,
9393
volatile uint8_t *ucsrc, volatile uint8_t *udr);
@@ -107,7 +107,7 @@ class HardwareSerial : public Stream
107107
operator bool() { return true; }
108108

109109
// Interrupt handlers - Not intended to be called externally
110-
void _rx_complete_irq(void);
110+
inline void _rx_complete_irq(void);
111111
void _tx_udr_empty_irq(void);
112112
};
113113

Diff for: ‎hardware/arduino/avr/cores/arduino/HardwareSerial0.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include "Arduino.h"
22
#include "HardwareSerial.h"
3+
#include "HardwareSerial_private.h"
34

45
// Each HardwareSerial is defined in its own file, sine the linker pulls
56
// in the entire file when any element inside is used. --gc-sections can

Diff for: ‎hardware/arduino/avr/cores/arduino/HardwareSerial1.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include "Arduino.h"
22
#include "HardwareSerial.h"
3+
#include "HardwareSerial_private.h"
34

45
// Each HardwareSerial is defined in its own file, sine the linker pulls
56
// in the entire file when any element inside is used. --gc-sections can

Diff for: ‎hardware/arduino/avr/cores/arduino/HardwareSerial2.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include "Arduino.h"
22
#include "HardwareSerial.h"
3+
#include "HardwareSerial_private.h"
34

45
// Each HardwareSerial is defined in its own file, sine the linker pulls
56
// in the entire file when any element inside is used. --gc-sections can

Diff for: ‎hardware/arduino/avr/cores/arduino/HardwareSerial3.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include "Arduino.h"
22
#include "HardwareSerial.h"
3+
#include "HardwareSerial_private.h"
34

45
// Each HardwareSerial is defined in its own file, sine the linker pulls
56
// in the entire file when any element inside is used. --gc-sections can

Diff for: ‎hardware/arduino/avr/cores/arduino/HardwareSerial_private.h

+95
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
#include "wiring_private.h"
2+
3+
// this next line disables the entire HardwareSerial.cpp,
4+
// this is so I can support Attiny series and any other chip without a uart
5+
#if defined(HAVE_HWSERIAL0) || defined(HAVE_HWSERIAL1) || defined(HAVE_HWSERIAL2) || defined(HAVE_HWSERIAL3)
6+
7+
// Ensure that the various bit positions we use are available with a 0
8+
// postfix, so we can always use the values for UART0 for all UARTs. The
9+
// alternative, passing the various values for each UART to the
10+
// HardwareSerial constructor also works, but makes the code bigger and
11+
// slower.
12+
#if !defined(TXC0)
13+
#if defined(TXC)
14+
// On ATmega8, the uart and its bits are not numbered, so there is no TXC0 etc.
15+
#define TXC0 TXC
16+
#define RXEN0 RXEN
17+
#define TXEN0 TXEN
18+
#define RXCIE0 RXCIE
19+
#define UDRIE0 UDRIE
20+
#define U2X0 U2X
21+
#define UPE0 UPE
22+
#define UDRE0 UDRE
23+
#elif defined(TXC1)
24+
// Some devices have uart1 but no uart0
25+
#define TXC0 TXC1
26+
#define RXEN0 RXEN1
27+
#define TXEN0 TXEN1
28+
#define RXCIE0 RXCIE1
29+
#define UDRIE0 UDRIE1
30+
#define U2X0 U2X1
31+
#define UPE0 UPE1
32+
#define UDRE0 UDRE1
33+
#else
34+
#error No UART found in HardwareSerial.cpp
35+
#endif
36+
#endif // !defined TXC0
37+
38+
// Check at compiletime that it is really ok to use the bit positions of
39+
// UART0 for the other UARTs as well, in case these values ever get
40+
// changed for future hardware.
41+
#if defined(TXC1) && (TXC1 != TXC0 || RXEN1 != RXEN0 || RXCIE1 != RXCIE0 || \
42+
UDRIE1 != UDRIE0 || U2X1 != U2X0 || UPE1 != UPE0 || \
43+
UDRE1 != UDRE0)
44+
#error "Not all bit positions for UART1 are the same as for UART0"
45+
#endif
46+
#if defined(TXC2) && (TXC2 != TXC0 || RXEN2 != RXEN0 || RXCIE2 != RXCIE0 || \
47+
UDRIE2 != UDRIE0 || U2X2 != U2X0 || UPE2 != UPE0 || \
48+
UDRE2 != UDRE0)
49+
#error "Not all bit positions for UART2 are the same as for UART0"
50+
#endif
51+
#if defined(TXC3) && (TXC3 != TXC0 || RXEN3 != RXEN0 || RXCIE3 != RXCIE0 || \
52+
UDRIE3 != UDRIE0 || U3X3 != U3X0 || UPE3 != UPE0 || \
53+
UDRE3 != UDRE0)
54+
#error "Not all bit positions for UART3 are the same as for UART0"
55+
#endif
56+
57+
// Constructors ////////////////////////////////////////////////////////////////
58+
59+
HardwareSerial::HardwareSerial(
60+
volatile uint8_t *ubrrh, volatile uint8_t *ubrrl,
61+
volatile uint8_t *ucsra, volatile uint8_t *ucsrb,
62+
volatile uint8_t *ucsrc, volatile uint8_t *udr) :
63+
_ubrrh(ubrrh), _ubrrl(ubrrl),
64+
_ucsra(ucsra), _ucsrb(ucsrb), _ucsrc(ucsrc),
65+
_udr(udr),
66+
_tx_buffer_head(0), _tx_buffer_tail(0),
67+
_rx_buffer_head(0), _rx_buffer_tail(0)
68+
{
69+
}
70+
71+
// Actual interrupt handlers //////////////////////////////////////////////////////////////
72+
73+
void HardwareSerial::_rx_complete_irq(void)
74+
{
75+
if (bit_is_clear(*_ucsra, UPE0)) {
76+
// No Parity error, read byte and store it in the buffer if there is
77+
// room
78+
unsigned char c = *_udr;
79+
int i = (unsigned int)(_rx_buffer_head + 1) % SERIAL_BUFFER_SIZE;
80+
81+
// if we should be storing the received character into the location
82+
// just before the tail (meaning that the head would advance to the
83+
// current location of the tail), we're about to overflow the buffer
84+
// and so we don't write the character or advance the head.
85+
if (i != _rx_buffer_tail) {
86+
_rx_buffer[_rx_buffer_head] = c;
87+
_rx_buffer_head = i;
88+
}
89+
} else {
90+
// Parity error, read byte but discard it
91+
unsigned char c = *_udr;
92+
};
93+
}
94+
95+
#endif // whole file

0 commit comments

Comments
 (0)