Skip to content

Commit e0c50a9

Browse files
KurtEKurtE
committed
Serial: Tx use FIFO plus fixes
This is a reasonably major update to the Serial(UART) code base, which is mostly concentrated on the Output(TX) side of it. Simple part of the description: The currently released code, did not use the FIFO object that was built into the Serial object. Instead it would output one character at a time and then wait for a callback from the lower level code base, which could be TDRE or TEND (mostly likely TDRE) before it would return from the call. There were several other methods that did not work or were not implemented including: flush(), availableForWrite(). Also the write with buffer and count did not work as it did not have the same parameters as the one in the Print class, so the print class simply enumerated each byte. **More Complex stuff:** **Write Call** I added a 16 byte buffer to the UART class that I use to pass down to the R_SCI_UART_Write calls, with as much data as I have up to 16 from the write method if a write is not currently in process. If not active and the FIFO is empty. I detect that and bypass using FIFO up to the first 16 characters. If write is active, the new data is added to the FIFO. **UART_EVENT_TX_DATA_EMPTY** Note: I mention TDRE here, but also cases for FIFO which are different. When lower level completes the write, it will do a callback with the event: UART_EVENT_TX_DATA_EMPTY. First try was to simply grab what is available up to 16 bytes and recall R_SCI_UART_Write with that. But that can lose data, as the callback is called when the ISR code has put the last byte into the TDR register, and the Write call always tries to put the first character out, which if the TDRE state is not set, will lose the last byte. Found I needed to handle two cases: 1) If TDRE I call the write function again. 2) if not TDRE, I update the Buffer/count member variables and reset which interrupt events which will happen. **TIMING** I was still running into conditions where either data was lost or the transfers did not work correctly. RingBuffer: Found in several cases we were spending too much time in the ISR and we could lose interrupts on either UARTS or the same UARTS but different Interrupt. Found a lot of the time was spent in the SafeRingBufferN code, where it creates a class object which disables all interrupts and maybe reenables them at the end. This is needed as the API/ RingBufferN code is inherently unsafe to use here. I created my own versions of these classes that make the RingBuffer class safer such that the Safe version could be a minimal subset. That decreased the overhead a lot. FIFO Size: When using one of the UARTS with FIFO, we filled the RX before any was returned. So the ISR might need to process 16 characters, which took awhile. I have it currently set to 8. I updated all of the Variants to allow each to set their preference. **RX** As mentioned above, when we receive bytes and the ISR is triggered, it calls our callback with the UART_EVENT_RX_CHAR event. For simple double buffer UARTS, we simply store one character away. However for FIFO UARTS, it will callback to us one time for each character in the FIFO. Which before took a lot of time. As for character it would setup the callback, call it, where we disabled the interrupts... The new cod instead extracts the remaining characters from the FIFO and stuffs them directly back into our fifo queue. To keep from having to look each time at, am I running on a FIFO queue or not, I have two different callbacks, so they don't have to keep asking. **DEBUG CODE** Currently I have a fair amount of debug code built into the Serial class, to help debug, including fast functions to set/clear/toggle digital pins as well as to maybe save some state information. I have hopefully all of this under #ifdef
1 parent 05feca5 commit e0c50a9

File tree

9 files changed

+567
-42
lines changed

9 files changed

+567
-42
lines changed

Diff for: ‎cores/arduino/FasterSafeRingBuffer.h

+196
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
/*
2+
Copyright (c) 2020 Arduino. All right reserved.
3+
4+
This library is free software; you can redistribute it and/or
5+
modify it under the terms of the GNU Lesser General Public
6+
License as published by the Free Software Foundation; either
7+
version 2.1 of the License, or (at your option) any later version.
8+
9+
This library is distributed in the hope that it will be useful,
10+
but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
See the GNU Lesser General Public License for more details.
13+
14+
You should have received a copy of the GNU Lesser General Public
15+
License along with this library; if not, write to the Free Software
16+
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
17+
*/
18+
19+
#ifdef __cplusplus
20+
21+
#ifndef _FASTER_SAFE_RING_BUFFER_
22+
#define _FASTER_SAFE_RING_BUFFER_
23+
#include "sync.h"
24+
25+
26+
// Note: Below is a modified version of the RingBuffer.h, that removes the
27+
// count member variable, such that the producer and consumer don't need to
28+
// modify the same members. Allows one or both of them to run without needing
29+
// to disable interrupts for reading and writing members.
30+
31+
// Define constants and variables for buffering incoming serial data. We're
32+
// using a ring buffer (I think), in which head is the index of the location
33+
// to which to write the next incoming character and tail is the index of the
34+
// location from which to read.
35+
namespace arduino {
36+
37+
#define SERIAL_BUFFER_SIZE 64
38+
39+
template <int N>
40+
class SaferRingBufferN
41+
{
42+
public:
43+
uint8_t _aucBuffer[N] ;
44+
volatile int _iHead ;
45+
volatile int _iTail ;
46+
47+
public:
48+
SaferRingBufferN( void ) ;
49+
void store_char( uint8_t c ) ;
50+
void clear();
51+
int read_char();
52+
int available();
53+
int availableForStore();
54+
int peek();
55+
bool isFull();
56+
57+
private:
58+
int nextIndex(int index);
59+
inline bool isEmpty() const { return (_iHead == _iTail); }
60+
};
61+
62+
typedef SaferRingBufferN<SERIAL_BUFFER_SIZE> SaferRingBuffer;
63+
64+
65+
template <int N>
66+
SaferRingBufferN<N>::SaferRingBufferN( void )
67+
{
68+
memset( _aucBuffer, 0, N ) ;
69+
clear();
70+
}
71+
72+
template <int N>
73+
void SaferRingBufferN<N>::store_char( uint8_t c )
74+
{
75+
// if we should be storing the received character into the location
76+
// just before the tail (meaning that the head would advance to the
77+
// current location of the tail), we're about to overflow the buffer
78+
// and so we don't write the character or advance the head.
79+
int newhead = nextIndex(_iHead);
80+
if (newhead != _iTail)
81+
{
82+
_aucBuffer[_iHead] = c ;
83+
_iHead = newhead;
84+
}
85+
}
86+
87+
template <int N>
88+
void SaferRingBufferN<N>::clear()
89+
{
90+
_iHead = 0;
91+
_iTail = 0;
92+
}
93+
94+
template <int N>
95+
int SaferRingBufferN<N>::read_char()
96+
{
97+
if (_iHead == _iTail)
98+
return -1;
99+
100+
uint8_t value = _aucBuffer[_iTail];
101+
_iTail = nextIndex(_iTail);
102+
103+
return value;
104+
}
105+
106+
template <int N>
107+
int SaferRingBufferN<N>::available()
108+
{
109+
// grab state of head and tail, to keep result consistent
110+
int head = _iHead;
111+
int tail = _iTail;
112+
113+
if (head >= tail)
114+
return head - tail;
115+
return N + head - tail;
116+
}
117+
118+
template <int N>
119+
int SaferRingBufferN<N>::availableForStore()
120+
{
121+
// grab state of head and tail, to keep result consistent
122+
int head = _iHead;
123+
int tail = _iTail;
124+
if (head >= tail) return N - 1 - head + tail;
125+
return tail - head - 1;
126+
}
127+
128+
template <int N>
129+
int SaferRingBufferN<N>::peek()
130+
{
131+
if (isEmpty())
132+
return -1;
133+
134+
return _aucBuffer[_iTail];
135+
}
136+
137+
template <int N>
138+
int SaferRingBufferN<N>::nextIndex(int index)
139+
{
140+
return (uint32_t)(index + 1) % N;
141+
}
142+
143+
template <int N>
144+
bool SaferRingBufferN<N>::isFull()
145+
{
146+
int newhead = nextIndex(_iHead);
147+
return (newhead == _iTail);
148+
}
149+
150+
///////////////////////////////////
151+
152+
// Protect writes for potential multiple writers
153+
template <int N>
154+
class FasterSafeWriteRingBufferN : public SaferRingBufferN<N>
155+
{
156+
public:
157+
void store_char( uint8_t c ) ;
158+
};
159+
160+
typedef FasterSafeWriteRingBufferN<SERIAL_BUFFER_SIZE> FasterSafeWriteRingBuffer;
161+
162+
163+
template <int N>
164+
void FasterSafeWriteRingBufferN<N>::store_char(uint8_t c) {
165+
synchronized {
166+
SaferRingBufferN<N>::store_char(c);
167+
}
168+
}
169+
170+
template <int N>
171+
class FasterSafeReadRingBufferN : public SaferRingBufferN<N>
172+
{
173+
public:
174+
int read_char();
175+
};
176+
177+
typedef FasterSafeReadRingBufferN<SERIAL_BUFFER_SIZE> FasterSafeReadRingBuffer;
178+
179+
template <int N>
180+
int FasterSafeReadRingBufferN<N>::read_char() {
181+
synchronized {
182+
return SaferRingBufferN<N>::read_char();
183+
}
184+
185+
// We should never reached this line because the synchronized {} block gets
186+
// executed at least once. However the compiler gets confused and prints a
187+
// warning about control reaching the end of a non-void function. This
188+
// silences that warning.
189+
return -1;
190+
}
191+
192+
193+
}
194+
195+
#endif /* _SAFE_RING_BUFFER_ */
196+
#endif /* __cplusplus */

Diff for: ‎cores/arduino/IRQManager.cpp

+6-5
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#define SDCARD_CARD_PRIORITY 12
1313
#define EXTERNAL_PIN_PRIORITY 12
1414
#define UART_SCI_PRIORITY 12
15+
#define UART_SCI_RX_PRIORITY 10
1516
#define USB_PRIORITY 12
1617
#define AGT_PRIORITY 14
1718
#define RTC_PRIORITY 12
@@ -487,14 +488,14 @@ bool IRQManager::addPeripheral(Peripheral_t p, void *cfg) {
487488
last_interrupt_index++;
488489

489490
/* RX interrupt */
490-
p_cfg->rxi_ipl = UART_SCI_PRIORITY;
491+
p_cfg->rxi_ipl = UART_SCI_RX_PRIORITY;
491492
p_cfg->rxi_irq = (IRQn_Type)last_interrupt_index;
492493
*(irq_ptr + last_interrupt_index) = (uint32_t)sci_uart_rxi_isr;
493494
set_sci_rx_link_event(last_interrupt_index, p_cfg->channel);
494495
last_interrupt_index++;
495496

496497
/* RX-ERROR interrupt */
497-
p_cfg->eri_ipl = UART_SCI_PRIORITY;
498+
p_cfg->eri_ipl = UART_SCI_RX_PRIORITY;
498499
p_cfg->eri_irq = (IRQn_Type)last_interrupt_index;
499500
*(irq_ptr + last_interrupt_index) = (uint32_t)sci_uart_eri_isr;
500501
set_sci_eri_link_event(last_interrupt_index, p_cfg->channel);
@@ -1672,7 +1673,7 @@ void IRQManager::set_can_tx_link_event(int li, int ch)
16721673
#endif
16731674
}
16741675

1675-
void IRQManager::set_canfd_error_link_event(int li, int ch)
1676+
void IRQManager::set_canfd_error_link_event(__attribute__((unused)) int li, __attribute__((unused)) int ch)
16761677
{
16771678
if (0) {}
16781679
#ifdef ELC_EVENT_CAN0_CHERR
@@ -1687,7 +1688,7 @@ void IRQManager::set_canfd_error_link_event(int li, int ch)
16871688
#endif
16881689
}
16891690

1690-
void IRQManager::set_canfd_rx_link_event(int li, int ch)
1691+
void IRQManager::set_canfd_rx_link_event(__attribute__((unused)) int li, __attribute__((unused)) int ch)
16911692
{
16921693
if (0) {}
16931694
#ifdef ELC_EVENT_CAN0_COMFRX
@@ -1702,7 +1703,7 @@ void IRQManager::set_canfd_rx_link_event(int li, int ch)
17021703
#endif
17031704
}
17041705

1705-
void IRQManager::set_canfd_tx_link_event(int li, int ch)
1706+
void IRQManager::set_canfd_tx_link_event(__attribute__((unused)) int li, __attribute__((unused)) int ch)
17061707
{
17071708
if (0) {}
17081709
#ifdef ELC_EVENT_CAN0_TX

0 commit comments

Comments
 (0)