Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Interrupt safe serial --- Guard against non-atomic variable changes from interrupt routines #2994

Merged
merged 2 commits into from
Mar 6, 2016
Merged

Interrupt safe serial --- Guard against non-atomic variable changes from interrupt routines #2994

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dc0f418
Make rx tail/head atomic
AnHardt Feb 3, 2016
cb88fdd
Protect MarlinSerial against interrupts
AnHardt Feb 24, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 33 additions & 20 deletions Marlin/MarlinSerial.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,19 @@
#endif

FORCE_INLINE void store_char(unsigned char c) {
int i = (unsigned int)(rx_buffer.head + 1) % RX_BUFFER_SIZE;

// if we should be storing the received character into the location
// just before the tail (meaning that the head would advance to the
// current location of the tail), we're about to overflow the buffer
// and so we don't write the character or advance the head.
if (i != rx_buffer.tail) {
rx_buffer.buffer[rx_buffer.head] = c;
rx_buffer.head = i;
}
CRITICAL_SECTION_START;
uint8_t h = rx_buffer.head;
uint8_t i = (uint8_t)(h + 1) & (RX_BUFFER_SIZE - 1);

// if we should be storing the received character into the location
// just before the tail (meaning that the head would advance to the
// current location of the tail), we're about to overflow the buffer
// and so we don't write the character or advance the head.
if (i != rx_buffer.tail) {
rx_buffer.buffer[h] = c;
rx_buffer.head = i;
}
CRITICAL_SECTION_END;
}


Expand Down Expand Up @@ -101,24 +104,32 @@ void MarlinSerial::end() {


int MarlinSerial::peek(void) {
if (rx_buffer.head == rx_buffer.tail) {
return -1;
int v;
CRITICAL_SECTION_START;
uint8_t t = rx_buffer.tail;
if (rx_buffer.head == t) {
v = -1;
}
else {
return rx_buffer.buffer[rx_buffer.tail];
v = rx_buffer.buffer[t];
}
CRITICAL_SECTION_END;
return v;
}

int MarlinSerial::read(void) {
// if the head isn't ahead of the tail, we don't have any characters
if (rx_buffer.head == rx_buffer.tail) {
return -1;
int v;
CRITICAL_SECTION_START;
uint8_t t = rx_buffer.tail;
if (rx_buffer.head == t) {
v = -1;
}
else {
unsigned char c = rx_buffer.buffer[rx_buffer.tail];
rx_buffer.tail = (unsigned int)(rx_buffer.tail + 1) % RX_BUFFER_SIZE;
return c;
v = rx_buffer.buffer[t];
rx_buffer.tail = (uint8_t)(t + 1) & (RX_BUFFER_SIZE - 1);
}
CRITICAL_SECTION_END;
return v;
}

void MarlinSerial::flush() {
Expand All @@ -131,7 +142,9 @@ void MarlinSerial::flush() {
// the value to rx_buffer_tail; the previous value of rx_buffer_head
// may be written to rx_buffer_tail, making it appear as if the buffer
// were full, not empty.
rx_buffer.head = rx_buffer.tail;
CRITICAL_SECTION_START;
rx_buffer.head = rx_buffer.tail;
CRITICAL_SECTION_END;
}


Expand Down
50 changes: 34 additions & 16 deletions Marlin/MarlinSerial.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@
#define MarlinSerial_h
#include "Marlin.h"

#ifndef CRITICAL_SECTION_START
#define CRITICAL_SECTION_START unsigned char _sreg = SREG; cli();
#define CRITICAL_SECTION_END SREG = _sreg;
#endif


#ifndef SERIAL_PORT
#define SERIAL_PORT 0
#endif
Expand Down Expand Up @@ -69,13 +75,18 @@
// using a ring buffer (I think), in which rx_buffer_head is the index of the
// location to which to write the next incoming character and rx_buffer_tail
// is the index of the location from which to read.
#define RX_BUFFER_SIZE 128

// 256 is the max limit due to uint8_t head and tail. Use only powers of 2. (...,16,32,64,128,256)
#ifndef RX_BUFFER_SIZE
#define RX_BUFFER_SIZE 128
#endif
#if !((RX_BUFFER_SIZE == 256) ||(RX_BUFFER_SIZE == 128) ||(RX_BUFFER_SIZE == 64) ||(RX_BUFFER_SIZE == 32) ||(RX_BUFFER_SIZE == 16) ||(RX_BUFFER_SIZE == 8) ||(RX_BUFFER_SIZE == 4) ||(RX_BUFFER_SIZE == 2))
#error RX_BUFFER_SIZE has to be a power of 2 and >= 2
#endif

struct ring_buffer {
unsigned char buffer[RX_BUFFER_SIZE];
int head;
int tail;
volatile uint8_t head;
volatile uint8_t tail;
};

#if UART_PRESENT(SERIAL_PORT)
Expand All @@ -92,8 +103,12 @@ class MarlinSerial { //: public Stream
int read(void);
void flush(void);

FORCE_INLINE int available(void) {
return (unsigned int)(RX_BUFFER_SIZE + rx_buffer.head - rx_buffer.tail) % RX_BUFFER_SIZE;
FORCE_INLINE uint8_t available(void) {
CRITICAL_SECTION_START;
uint8_t h = rx_buffer.head;
uint8_t t = rx_buffer.tail;
CRITICAL_SECTION_END;
return (uint8_t)(RX_BUFFER_SIZE + h - t) & (RX_BUFFER_SIZE - 1);
}

FORCE_INLINE void write(uint8_t c) {
Expand All @@ -105,16 +120,19 @@ class MarlinSerial { //: public Stream
FORCE_INLINE void checkRx(void) {
if (TEST(M_UCSRxA, M_RXCx)) {
unsigned char c = M_UDRx;
int i = (unsigned int)(rx_buffer.head + 1) % RX_BUFFER_SIZE;

// if we should be storing the received character into the location
// just before the tail (meaning that the head would advance to the
// current location of the tail), we're about to overflow the buffer
// and so we don't write the character or advance the head.
if (i != rx_buffer.tail) {
rx_buffer.buffer[rx_buffer.head] = c;
rx_buffer.head = i;
}
CRITICAL_SECTION_START;
uint8_t h = rx_buffer.head;
uint8_t i = (uint8_t)(h + 1) & (RX_BUFFER_SIZE - 1);

// if we should be storing the received character into the location
// just before the tail (meaning that the head would advance to the
// current location of the tail), we're about to overflow the buffer
// and so we don't write the character or advance the head.
if (i != rx_buffer.tail) {
rx_buffer.buffer[h] = c;
rx_buffer.head = i;
}
CRITICAL_SECTION_END;
}
}

Expand Down