From 937cc51c693241cf6b7450ef5c61287bf73ee30e Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 4 Jun 2013 11:04:30 +0200 Subject: [PATCH 01/20] Clear SoftwareSerial rx delay if no interrupt register is found Before enabling interupts, begin would see if the given receive pin actually has an associated PCINT register. If not, the interrupts would not be enabled. Now, the same check is done, but when no register is available, the rx parameters are not loaded at all (which in turn prevents the interrupt from being enabled). This allows all code to use the same "is rx enabled" (which will be added next). --- .../libraries/SoftwareSerial/SoftwareSerial.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp index d1f6c9256a6..8476deb8a40 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp @@ -385,9 +385,13 @@ void SoftwareSerial::begin(long speed) long baud = pgm_read_dword(&table[i].baud); if (baud == speed) { - _rx_delay_centering = pgm_read_word(&table[i].rx_delay_centering); - _rx_delay_intrabit = pgm_read_word(&table[i].rx_delay_intrabit); - _rx_delay_stopbit = pgm_read_word(&table[i].rx_delay_stopbit); + if (digitalPinToPCICR(_receivePin)) + { + // Only setup rx when we have a valid PCINT for this pin + _rx_delay_centering = pgm_read_word(&table[i].rx_delay_centering); + _rx_delay_intrabit = pgm_read_word(&table[i].rx_delay_intrabit); + _rx_delay_stopbit = pgm_read_word(&table[i].rx_delay_stopbit); + } _tx_delay = pgm_read_word(&table[i].tx_delay); break; } @@ -396,11 +400,8 @@ void SoftwareSerial::begin(long speed) // Set up RX interrupts, but only if we have a valid RX baud rate if (_rx_delay_stopbit) { - if (digitalPinToPCICR(_receivePin)) - { - *digitalPinToPCICR(_receivePin) |= _BV(digitalPinToPCICRbit(_receivePin)); - *digitalPinToPCMSK(_receivePin) |= _BV(digitalPinToPCMSKbit(_receivePin)); - } + *digitalPinToPCICR(_receivePin) |= _BV(digitalPinToPCICRbit(_receivePin)); + *digitalPinToPCMSK(_receivePin) |= _BV(digitalPinToPCMSKbit(_receivePin)); tunedDelay(_tx_delay); // if we were low this establishes the end } From b1c7a3d05f7b7c69fe60279ea9037e130d32e8c3 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 4 Jun 2013 10:58:56 +0200 Subject: [PATCH 02/20] Let SoftwareSerial::listen() fail on invalid rx baud rates In this case, SoftwareSerial::begin will not have enabled the interrupts, so better not allow the SoftwareSerial instance to enter the listening state either. --- .../arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp index 8476deb8a40..605c78840ee 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp @@ -178,6 +178,9 @@ inline void SoftwareSerial::tunedDelay(uint16_t delay) { // one and returns true if it replaces another bool SoftwareSerial::listen() { + if (!_rx_delay_stopbit) + return false; + if (active_object != this) { _buffer_overflow = false; From 1704e7e820c78f38153e55b9ea3cc991095e9a57 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 4 Jun 2013 11:19:58 +0200 Subject: [PATCH 03/20] Let SoftwareSerial::end also check against _rx_delay_stopbit The current check is still always false when the old check was, but additionally it will not disable the interrupts when they were never enabled (which shouldn't matter much, but this is more consistent). --- .../arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp index 605c78840ee..ac9e30b8fac 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp @@ -418,7 +418,7 @@ void SoftwareSerial::begin(long speed) void SoftwareSerial::end() { - if (digitalPinToPCMSK(_receivePin)) + if (_rx_delay_stopbit) *digitalPinToPCMSK(_receivePin) &= ~_BV(digitalPinToPCMSKbit(_receivePin)); } From db1a4ad139b7efc52b3be4560fb5dddc3992f12a Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 4 Jun 2013 11:22:19 +0200 Subject: [PATCH 04/20] Add SoftwareSerial::setRxIntMsk() This moves the interrupt mask enabling / disabling code into a separate method, so we can call it from multiple spots next. --- .../libraries/SoftwareSerial/SoftwareSerial.cpp | 15 +++++++++++++-- .../avr/libraries/SoftwareSerial/SoftwareSerial.h | 1 + 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp index ac9e30b8fac..8bf56e5d41a 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp @@ -403,8 +403,11 @@ void SoftwareSerial::begin(long speed) // Set up RX interrupts, but only if we have a valid RX baud rate if (_rx_delay_stopbit) { + // Enable the PCINT for the entire port here, but never disable it + // (others might also need it, so we disable the interrupt by using + // the per-pin PCMSK register). *digitalPinToPCICR(_receivePin) |= _BV(digitalPinToPCICRbit(_receivePin)); - *digitalPinToPCMSK(_receivePin) |= _BV(digitalPinToPCMSKbit(_receivePin)); + setRxIntMsk(true); tunedDelay(_tx_delay); // if we were low this establishes the end } @@ -416,10 +419,18 @@ void SoftwareSerial::begin(long speed) listen(); } +void SoftwareSerial::setRxIntMsk(bool enable) +{ + if (enable) + *digitalPinToPCMSK(_receivePin) |= _BV(digitalPinToPCMSKbit(_receivePin)); + else + *digitalPinToPCMSK(_receivePin) &= ~_BV(digitalPinToPCMSKbit(_receivePin)); +} + void SoftwareSerial::end() { if (_rx_delay_stopbit) - *digitalPinToPCMSK(_receivePin) &= ~_BV(digitalPinToPCMSKbit(_receivePin)); + setRxIntMsk(false); } diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h index a6a60b5560f..31bd229efb4 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h @@ -74,6 +74,7 @@ class SoftwareSerial : public Stream void tx_pin_write(uint8_t pin_state); void setTX(uint8_t transmitPin); void setRX(uint8_t receivePin); + void setRxIntMsk(bool enable); // private static method for timing static inline void tunedDelay(uint16_t delay); From 934393398b5a5db0e822622ce7392f5d28143e9d Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 4 Jun 2013 11:28:30 +0200 Subject: [PATCH 05/20] Add SoftwareSerial::stopListening() This allows one to explicitly stop a SoftwareSerial instance from listening, without having to make another one listening. --- .../avr/libraries/SoftwareSerial/SoftwareSerial.cpp | 11 +++++++++++ .../avr/libraries/SoftwareSerial/SoftwareSerial.h | 1 + 2 files changed, 12 insertions(+) diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp index 8bf56e5d41a..a429f7d2d88 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp @@ -195,6 +195,17 @@ bool SoftwareSerial::listen() return false; } +// Stop listening. Returns true if we were actually listening. +bool SoftwareSerial::stopListening() +{ + if (active_object == this) + { + active_object = NULL; + return true; + } + return false; +} + // // The receive routine called by the interrupt handler // diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h index 31bd229efb4..e0e4746cd4c 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h @@ -87,6 +87,7 @@ class SoftwareSerial : public Stream bool listen(); void end(); bool isListening() { return this == active_object; } + bool stopListening(); bool overflow() { bool ret = _buffer_overflow; _buffer_overflow = false; return ret; } int peek(); From b1de3e66219bd220743333404b464bc1e16c774a Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 4 Jun 2013 11:30:26 +0200 Subject: [PATCH 06/20] Toggle SoftwareSerial interrupts when starting / stopping to listen This prevents interrupts from triggering when the SoftwareSerial instance is not even listening. Additionally, this removes the need to disable interrupts in SoftwareSerial::listen, since no interrupts are active while it touches the variables. --- .../avr/libraries/SoftwareSerial/SoftwareSerial.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp index a429f7d2d88..b98451f9ee0 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp @@ -183,12 +183,14 @@ bool SoftwareSerial::listen() if (active_object != this) { + if (active_object) + active_object->stopListening(); + _buffer_overflow = false; - uint8_t oldSREG = SREG; - cli(); _receive_buffer_head = _receive_buffer_tail = 0; active_object = this; - SREG = oldSREG; + + setRxIntMsk(true); return true; } @@ -200,6 +202,7 @@ bool SoftwareSerial::stopListening() { if (active_object == this) { + setRxIntMsk(false); active_object = NULL; return true; } @@ -418,7 +421,6 @@ void SoftwareSerial::begin(long speed) // (others might also need it, so we disable the interrupt by using // the per-pin PCMSK register). *digitalPinToPCICR(_receivePin) |= _BV(digitalPinToPCICRbit(_receivePin)); - setRxIntMsk(true); tunedDelay(_tx_delay); // if we were low this establishes the end } From f3aa5f23c4b73028bb7403dc90469571eef5cc59 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 13 Jun 2013 09:56:46 +0200 Subject: [PATCH 07/20] Fix race condition in SoftwareSerial::overflow() If an interrupt causing overflow would occur between reading _buffer_overflow and clearing it, this overflow condition would be immediately cleared and never be returned by overflow(). By only clearing the overflow flag if an overflow actually occurred, this problem goes away (worst case overflow() returns false even though an overflow _just_ occurred, but then the next call to overflow() will return true). --- hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h index e0e4746cd4c..27c0bfca9cf 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h @@ -88,7 +88,7 @@ class SoftwareSerial : public Stream void end(); bool isListening() { return this == active_object; } bool stopListening(); - bool overflow() { bool ret = _buffer_overflow; _buffer_overflow = false; return ret; } + bool overflow() { bool ret = _buffer_overflow; if (ret) _buffer_overflow = false; return ret; } int peek(); virtual size_t write(uint8_t byte); From 6685aa999cf828a994cd81fac78b077a9692d654 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 13 Jun 2013 10:26:15 +0200 Subject: [PATCH 08/20] Use stopListening() in SoftwareSerial::end() stopListening also disabled the interrupt, if needed, so calling that function makes more sense. Since stopListening only disables the interrupt when the current SoftwareSerial is the active object, and that can only be the case when _rx_delay_stopbit is non-zero, there is no need to separately check _rx_delay_stopbit anymore. --- .../arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp index b98451f9ee0..0e70cb7fd96 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp @@ -442,8 +442,7 @@ void SoftwareSerial::setRxIntMsk(bool enable) void SoftwareSerial::end() { - if (_rx_delay_stopbit) - setRxIntMsk(false); + stopListening(); } From 416198a03b3517a76a6267680b42f8cf12b5c551 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 22 Apr 2014 16:30:13 +0200 Subject: [PATCH 09/20] Simplify SoftwareSerial::write Before, there was nearly identical code for the inverted and regular cases. However, simply inverting the byte in the inverted case allows using the regular code twice, reducing the generated code size by 100 bytes (on an Arduino Uno and gcc 4.3, on gcc 4.8 the reduction is 50 bytes). --- .../SoftwareSerial/SoftwareSerial.cpp | 33 ++++++------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp index 0e70cb7fd96..da9af306350 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp @@ -486,34 +486,21 @@ size_t SoftwareSerial::write(uint8_t b) // Write each of the 8 bits if (_inverse_logic) - { - for (byte mask = 0x01; mask; mask <<= 1) - { - if (b & mask) // choose bit - tx_pin_write(LOW); // send 1 - else - tx_pin_write(HIGH); // send 0 - - tunedDelay(_tx_delay); - } + b = ~b; - tx_pin_write(LOW); // restore pin to natural state - } - else + for (byte mask = 0x01; mask; mask <<= 1) { - for (byte mask = 0x01; mask; mask <<= 1) - { - if (b & mask) // choose bit - tx_pin_write(HIGH); // send 1 - else - tx_pin_write(LOW); // send 0 - - tunedDelay(_tx_delay); - } + if (b & mask) // choose bit + tx_pin_write(HIGH); // send 1 + else + tx_pin_write(LOW); // send 0 - tx_pin_write(HIGH); // restore pin to natural state + tunedDelay(_tx_delay); } + // restore pin to natural state + tx_pin_write(_inverse_logic ? LOW : HIGH); + SREG = oldSREG; // turn interrupts back on tunedDelay(_tx_delay); From 80ea38b1dc55ffa43521c7e3ad55b79728fdf3cf Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 22 Apr 2014 17:45:21 +0200 Subject: [PATCH 10/20] Mark SoftwareSerial::tx_pin_write as "always_inline" Somehow gcc 4.8 doesn't inline this function, even though it is always called with constant arguments and can be reduced to just a few instructions when inlined. Adding the always_inline attribute makes gcc inline it, saving 46 bytes on the Arduino uno. gcc 4.3 already inlined this function, so there are no space savings there. --- hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h index 27c0bfca9cf..778fffbf56a 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h @@ -71,7 +71,7 @@ class SoftwareSerial : public Stream // private methods void recv(); uint8_t rx_pin_read(); - void tx_pin_write(uint8_t pin_state); + void tx_pin_write(uint8_t pin_state) __attribute__((__always_inline__)); void setTX(uint8_t transmitPin); void setRX(uint8_t receivePin); void setRxIntMsk(bool enable); From 764f24266ec769af6f4d9b7bd1ebb18c7bbf0c3c Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 22 Apr 2014 20:52:09 +0200 Subject: [PATCH 11/20] Further optimize SoftwareSerial::write This change restructures the loop, to help the compiler generate shorter code (because now only the LSB of the data byte is checked and subsequent bytes are shifted down one by one, it can use th "skip if bit set" instruction). Furthermore, it puts most attributes in local variables, which causes the compiler to put them into registers. This makes the timing-critical part of the code smaller, making it easier to provide accurate timings. On an Arduino uno using gcc 4.3, this saves 58 bytes. On gcc 4.8, this saves 14 bytes. --- .../SoftwareSerial/SoftwareSerial.cpp | 48 +++++++++++-------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp index da9af306350..621a2af89e5 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp @@ -292,14 +292,6 @@ void SoftwareSerial::recv() #endif } -void SoftwareSerial::tx_pin_write(uint8_t pin_state) -{ - if (pin_state == LOW) - *_transmitPortRegister &= ~_transmitBitMask; - else - *_transmitPortRegister |= _transmitBitMask; -} - uint8_t SoftwareSerial::rx_pin_read() { return *_receivePortRegister & _receiveBitMask; @@ -477,29 +469,47 @@ size_t SoftwareSerial::write(uint8_t b) return 0; } + // By declaring these as local variables, the compiler will put them + // in registers _before_ disabling interrupts and entering the + // critical timing sections below, which makes it a lot easier to + // verify the cycle timings + volatile uint8_t *reg = _transmitPortRegister; + uint8_t reg_mask = _transmitBitMask; + uint8_t inv_mask = ~_transmitBitMask; uint8_t oldSREG = SREG; + bool inv = _inverse_logic; + uint16_t delay = _tx_delay; + + if (inv) + b = ~b; + cli(); // turn off interrupts for a clean txmit // Write the start bit - tx_pin_write(_inverse_logic ? HIGH : LOW); - tunedDelay(_tx_delay + XMIT_START_ADJUSTMENT); + if (inv) + *reg |= reg_mask; + else + *reg &= inv_mask; - // Write each of the 8 bits - if (_inverse_logic) - b = ~b; + tunedDelay(delay); - for (byte mask = 0x01; mask; mask <<= 1) + // Write each of the 8 bits + for (uint8_t i = 8; i > 0; --i) { - if (b & mask) // choose bit - tx_pin_write(HIGH); // send 1 + if (b & 1) // choose bit + *reg |= reg_mask; // send 1 else - tx_pin_write(LOW); // send 0 + *reg &= inv_mask; // send 0 - tunedDelay(_tx_delay); + tunedDelay(delay); + b >>= 1; } // restore pin to natural state - tx_pin_write(_inverse_logic ? LOW : HIGH); + if (inv) + *reg &= inv_mask; + else + *reg |= reg_mask; SREG = oldSREG; // turn interrupts back on tunedDelay(_tx_delay); From 87f89f610072c3f4add1370b876b95d6793a8af8 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 22 Apr 2014 22:24:43 +0200 Subject: [PATCH 12/20] Optimize SoftwareSerial::recv Similar to SoftwareSerial::write, this rewrites the loop to only touch the MSB and then shift those bits up, allowing the compiler to generate more efficient code. Unlike the write function however, it is not needed to put all instance variables used into local variables, for some reason the compiler already does this (and doing it manually even makes the code bigger). On the Arduino Uno using gcc 4.3 this saves 26 bytes. Using gcc 4.8 this saves 30 bytes. Note that this removes the else clause in the code, making the C code unbalanced, which looks like it breaks timing balance. However, looking at the code generated by the compiler, it turns out that the old code was actually unbalanced, while the new code is properly balanced. --- .../avr/libraries/SoftwareSerial/SoftwareSerial.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp index 621a2af89e5..974aa1e310c 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp @@ -242,15 +242,13 @@ void SoftwareSerial::recv() DebugPulse(_DEBUG_PIN2, 1); // Read each of the 8 bits - for (uint8_t i=0x1; i; i <<= 1) + for (uint8_t i=8; i > 0; --i) { tunedDelay(_rx_delay_intrabit); + d >>= 1; DebugPulse(_DEBUG_PIN2, 1); - uint8_t noti = ~i; if (rx_pin_read()) - d |= i; - else // else clause added to ensure function timing is ~balanced - d &= noti; + d |= 0x80; } // skip the stop bit From 5af847d3a4fd7e055b591e3ba38a19da6d251926 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Wed, 23 Apr 2014 14:41:12 +0200 Subject: [PATCH 13/20] In SoftwareSerial, use ISR_ALIASOF to prevent duplication Previously, up to four separate but identical ISR routines were defined, for PCINT0, PCINT1, PCINT2 and PCINT3. Each of these would generate their own function, with a lot of push-popping because another function was called. Now, the ISR_ALIASOF macro from avr-libc is used to declare just the PCINT0 version and make all other ISRs point to that one, saving a lot of program space, as well as some speed because of improved inlining. On an Arduino Uno with gcc 4.3, this saves 168 bytes. With gcc 4.8, this saves 150 bytes. --- .../libraries/SoftwareSerial/SoftwareSerial.cpp | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp index 974aa1e310c..cafd0c4980e 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp @@ -316,24 +316,15 @@ ISR(PCINT0_vect) #endif #if defined(PCINT1_vect) -ISR(PCINT1_vect) -{ - SoftwareSerial::handle_interrupt(); -} +ISR(PCINT1_vect, ISR_ALIASOF(PCINT0_vect)); #endif #if defined(PCINT2_vect) -ISR(PCINT2_vect) -{ - SoftwareSerial::handle_interrupt(); -} +ISR(PCINT2_vect, ISR_ALIASOF(PCINT0_vect)); #endif #if defined(PCINT3_vect) -ISR(PCINT3_vect) -{ - SoftwareSerial::handle_interrupt(); -} +ISR(PCINT3_vect, ISR_ALIASOF(PCINT0_vect)); #endif // From 9d8f350ffe6892b619bc7cd6dd40033cab9d4621 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Wed, 23 Apr 2014 15:52:57 +0200 Subject: [PATCH 14/20] Mark SoftwareSerial::recv and handle_interrupt as always_inline Since those functions are only called once now, it makes sense to inline them. This saves a few bytes of program space, but also saves a few cycles in the critical RX path. --- .../arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h index 778fffbf56a..aa9bbf38d0b 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h @@ -69,7 +69,7 @@ class SoftwareSerial : public Stream static SoftwareSerial *active_object; // private methods - void recv(); + void recv() __attribute__((__always_inline__)); uint8_t rx_pin_read(); void tx_pin_write(uint8_t pin_state) __attribute__((__always_inline__)); void setTX(uint8_t transmitPin); @@ -99,7 +99,7 @@ class SoftwareSerial : public Stream using Print::write; // public only for easy access by interrupt handlers - static inline void handle_interrupt(); + static inline void handle_interrupt() __attribute__((__always_inline__)); }; // Arduino 0012 workaround From ddbe3174f03c3ef9a3862bea038673fa253b8821 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Wed, 23 Apr 2014 17:45:10 +0200 Subject: [PATCH 15/20] In SoftwareSerial::recv, only calculate the new tail once This shortens the generated code a bit more. --- .../arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp index cafd0c4980e..42f796b5aac 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp @@ -259,11 +259,12 @@ void SoftwareSerial::recv() d = ~d; // if buffer full, set the overflow flag and return - if ((_receive_buffer_tail + 1) % _SS_MAX_RX_BUFF != _receive_buffer_head) + uint8_t next = (_receive_buffer_tail + 1) % _SS_MAX_RX_BUFF; + if (next != _receive_buffer_head) { // save new data in buffer: tail points to where byte goes _receive_buffer[_receive_buffer_tail] = d; // save new byte - _receive_buffer_tail = (_receive_buffer_tail + 1) % _SS_MAX_RX_BUFF; + _receive_buffer_tail = next; } else { From ce6b0f89e3622124745a29b60e02955c7e7f38e8 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Wed, 23 Apr 2014 19:13:58 +0200 Subject: [PATCH 16/20] Optimize SoftwareSerial::setRxIntMsk() This precalculates the mask register and value, making setRxIntMask considerably less complicated. Right now, this is not a big deal, but simplifying it allows using it inside the ISR next. --- .../avr/libraries/SoftwareSerial/SoftwareSerial.cpp | 9 +++++++-- .../avr/libraries/SoftwareSerial/SoftwareSerial.h | 2 ++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp index 42f796b5aac..bee1107b7cf 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp @@ -403,6 +403,11 @@ void SoftwareSerial::begin(long speed) // (others might also need it, so we disable the interrupt by using // the per-pin PCMSK register). *digitalPinToPCICR(_receivePin) |= _BV(digitalPinToPCICRbit(_receivePin)); + // Precalculate the pcint mask register and value, so setRxIntMask + // can be used inside the ISR without costing too much time. + _pcint_maskreg = digitalPinToPCMSK(_receivePin); + _pcint_maskvalue = _BV(digitalPinToPCMSKbit(_receivePin)); + tunedDelay(_tx_delay); // if we were low this establishes the end } @@ -417,9 +422,9 @@ void SoftwareSerial::begin(long speed) void SoftwareSerial::setRxIntMsk(bool enable) { if (enable) - *digitalPinToPCMSK(_receivePin) |= _BV(digitalPinToPCMSKbit(_receivePin)); + *_pcint_maskreg |= _pcint_maskvalue; else - *digitalPinToPCMSK(_receivePin) &= ~_BV(digitalPinToPCMSKbit(_receivePin)); + *_pcint_maskreg &= ~_pcint_maskvalue; } void SoftwareSerial::end() diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h index aa9bbf38d0b..302a23bf729 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h @@ -53,6 +53,8 @@ class SoftwareSerial : public Stream volatile uint8_t *_receivePortRegister; uint8_t _transmitBitMask; volatile uint8_t *_transmitPortRegister; + volatile uint8_t *_pcint_maskreg; + uint8_t _pcint_maskvalue; uint16_t _rx_delay_centering; uint16_t _rx_delay_intrabit; From 08fa593916cf5b0a1ee93245e2f3363966083c98 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Wed, 23 Apr 2014 19:16:21 +0200 Subject: [PATCH 17/20] Disable the RX PCINT inside SoftwareSerial::recv Before, the interrupt would remain enabled during reception, which would re-set the PCINT flag because of the level changes inside the received byte. Because interrupts are globally disabled, this would not immediately trigger an interrupt, but the flag would be remembered to trigger another PCINT interrupt immediately after the first one is processed. Typically this was not a problem, because the second interrupt would see the stop bit, or an idle line, and decide that the interrupt triggered for someone else. However, at high baud rates, this could cause the next interrupt for the real start bit to be delayed so much that the byte got corrupted. By clearing the interrupt mask bit for just the RX pin (as opposed to the PCINT mask bit for the entire port), any PCINT events on other bits can still set the PCINT flag and be processed as normal. In this case, it's likely that there will be corruption, but that's inevitable when (other) interrupts happen during SoftwareSerial reception. --- .../avr/libraries/SoftwareSerial/SoftwareSerial.cpp | 7 +++++++ .../arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp index bee1107b7cf..d1d2008fef5 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp @@ -237,6 +237,11 @@ void SoftwareSerial::recv() // so interrupt is probably not for us if (_inverse_logic ? rx_pin_read() : !rx_pin_read()) { + // Disable further interrupts during reception, this prevents + // triggering another interrupt directly after we return, which can + // cause problems at higher baudrates. + setRxIntMsk(false); + // Wait approximately 1/2 of a bit width to "center" the sample tunedDelay(_rx_delay_centering); DebugPulse(_DEBUG_PIN2, 1); @@ -255,6 +260,8 @@ void SoftwareSerial::recv() tunedDelay(_rx_delay_stopbit); DebugPulse(_DEBUG_PIN2, 1); + // Re-enable interrupts when we're sure to be inside the stop bit + setRxIntMsk(true); if (_inverse_logic) d = ~d; diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h index 302a23bf729..2b125985e19 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h @@ -76,7 +76,7 @@ class SoftwareSerial : public Stream void tx_pin_write(uint8_t pin_state) __attribute__((__always_inline__)); void setTX(uint8_t transmitPin); void setRX(uint8_t receivePin); - void setRxIntMsk(bool enable); + void setRxIntMsk(bool enable) __attribute__((__always_inline__)); // private static method for timing static inline void tunedDelay(uint16_t delay); From ead2881b1c0e2b650d8970185717b3f5ac6e517e Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Wed, 23 Apr 2014 19:23:58 +0200 Subject: [PATCH 18/20] Fix SoftwareSerial timings Instead of using a lookup table with (wrong) timings, this calculates the timings in SoftwareSerial::begin. This is probably a bit slower, but since it typically happens once, this shouldn't be a problem. Additionally, since the lookup tables can be removed, this is also a lot smaller, as well as supporting arbitrary CPU speeds and baudrates, instead of the limited set that was defined before. Furthermore, this switches to use the _delay_loop_2 function from avr-libc instead of a handcoded delay function. The avr-libc function only takes two instructions, as opposed to four instructions for the old one. The compiler also inlines the avr-libc function, which makes the timings more reliable. The calculated timings directly rely on the instructions generated by the compiler, since a significant amount of time is spent processing (compared to the delays, especially at higher speeds). This means that if the code is changed, or a different compiler is used, the calculations might need changing (though a few cycles more or less shouldn't cause immediate breakage). The timings in the code have been calculated from the assembly generated by gcc 4.8.2 and gcc 4.3.2. The RX baudrates supported by SoftwareSerial are still not unlimited. At 16Mhz, using gcc 4.8.2, everything up to 115200 works. At 8Mhz, it works up to 57600. Using gcc 4.3.2, it also works up to 57600 at 16Mhz and up to 38400 at 8Mhz. Note that at these highest speeds, communication works, but is still quite sensitive to other interrupts (like the millis() interrupts) when bytes are sent back-to-back, so there still are corrupted bytes in RX. TX works up to 115200 for all combinations of compiler and clock rates. This fixes #2019 --- .../SoftwareSerial/SoftwareSerial.cpp | 186 ++++++------------ .../libraries/SoftwareSerial/SoftwareSerial.h | 4 + 2 files changed, 69 insertions(+), 121 deletions(-) diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp index d1d2008fef5..2977b1cba0f 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp @@ -42,92 +42,7 @@ The latest version of this library can always be found at #include #include #include -// -// Lookup table -// -typedef struct _DELAY_TABLE -{ - long baud; - unsigned short rx_delay_centering; - unsigned short rx_delay_intrabit; - unsigned short rx_delay_stopbit; - unsigned short tx_delay; -} DELAY_TABLE; - -#if F_CPU == 16000000 - -static const DELAY_TABLE PROGMEM table[] = -{ - // baud rxcenter rxintra rxstop tx - { 115200, 1, 17, 17, 12, }, - { 57600, 10, 37, 37, 33, }, - { 38400, 25, 57, 57, 54, }, - { 31250, 31, 70, 70, 68, }, - { 28800, 34, 77, 77, 74, }, - { 19200, 54, 117, 117, 114, }, - { 14400, 74, 156, 156, 153, }, - { 9600, 114, 236, 236, 233, }, - { 4800, 233, 474, 474, 471, }, - { 2400, 471, 950, 950, 947, }, - { 1200, 947, 1902, 1902, 1899, }, - { 600, 1902, 3804, 3804, 3800, }, - { 300, 3804, 7617, 7617, 7614, }, -}; - -const int XMIT_START_ADJUSTMENT = 5; - -#elif F_CPU == 8000000 - -static const DELAY_TABLE table[] PROGMEM = -{ - // baud rxcenter rxintra rxstop tx - { 115200, 1, 5, 5, 3, }, - { 57600, 1, 15, 15, 13, }, - { 38400, 2, 25, 26, 23, }, - { 31250, 7, 32, 33, 29, }, - { 28800, 11, 35, 35, 32, }, - { 19200, 20, 55, 55, 52, }, - { 14400, 30, 75, 75, 72, }, - { 9600, 50, 114, 114, 112, }, - { 4800, 110, 233, 233, 230, }, - { 2400, 229, 472, 472, 469, }, - { 1200, 467, 948, 948, 945, }, - { 600, 948, 1895, 1895, 1890, }, - { 300, 1895, 3805, 3805, 3802, }, -}; - -const int XMIT_START_ADJUSTMENT = 4; - -#elif F_CPU == 20000000 - -// 20MHz support courtesy of the good people at macegr.com. -// Thanks, Garrett! - -static const DELAY_TABLE PROGMEM table[] = -{ - // baud rxcenter rxintra rxstop tx - { 115200, 3, 21, 21, 18, }, - { 57600, 20, 43, 43, 41, }, - { 38400, 37, 73, 73, 70, }, - { 31250, 45, 89, 89, 88, }, - { 28800, 46, 98, 98, 95, }, - { 19200, 71, 148, 148, 145, }, - { 14400, 96, 197, 197, 194, }, - { 9600, 146, 297, 297, 294, }, - { 4800, 296, 595, 595, 592, }, - { 2400, 592, 1189, 1189, 1186, }, - { 1200, 1187, 2379, 2379, 2376, }, - { 600, 2379, 4759, 4759, 4755, }, - { 300, 4759, 9523, 9523, 9520, }, -}; - -const int XMIT_START_ADJUSTMENT = 6; - -#else - -#error This version of SoftwareSerial supports only 20, 16 and 8MHz processors - -#endif +#include // // Statics @@ -162,16 +77,7 @@ inline void DebugPulse(uint8_t pin, uint8_t count) /* static */ inline void SoftwareSerial::tunedDelay(uint16_t delay) { - uint8_t tmp=0; - - asm volatile("sbiw %0, 0x01 \n\t" - "ldi %1, 0xFF \n\t" - "cpi %A0, 0xFF \n\t" - "cpc %B0, %1 \n\t" - "brne .-10 \n\t" - : "+r" (delay), "+a" (tmp) - : "0" (delay) - ); + _delay_loop_2(delay); } // This function sets the current object as the "listening" @@ -256,12 +162,6 @@ void SoftwareSerial::recv() d |= 0x80; } - // skip the stop bit - tunedDelay(_rx_delay_stopbit); - DebugPulse(_DEBUG_PIN2, 1); - - // Re-enable interrupts when we're sure to be inside the stop bit - setRxIntMsk(true); if (_inverse_logic) d = ~d; @@ -280,6 +180,14 @@ void SoftwareSerial::recv() #endif _buffer_overflow = true; } + + // skip the stop bit + tunedDelay(_rx_delay_stopbit); + DebugPulse(_DEBUG_PIN1, 1); + + // Re-enable interrupts when we're sure to be inside the stop bit + setRxIntMsk(true); + } #if GCC_VERSION < 40302 @@ -378,6 +286,13 @@ void SoftwareSerial::setRX(uint8_t rx) _receivePortRegister = portInputRegister(port); } +uint16_t SoftwareSerial::subtract_cap(uint16_t num, uint16_t sub) { + if (num > sub) + return num - sub; + else + return 1; +} + // // Public methods // @@ -386,26 +301,55 @@ void SoftwareSerial::begin(long speed) { _rx_delay_centering = _rx_delay_intrabit = _rx_delay_stopbit = _tx_delay = 0; - for (unsigned i=0; i 40800 + // Timings counted from gcc 4.8.2 output. This works up to 115200 on + // 16Mhz and 57600 on 8Mhz. + // + // When the start bit occurs, there are 3 or 4 cycles before the + // interrupt flag is set, 4 cycles before the PC is set to the right + // interrupt vector address and the old PC is pushed on the stack, + // and then 75 cycles of instructions (including the RJMP in the + // ISR vector table) until the first delay. After the delay, there + // are 17 more cycles until the pin value is read (excluding the + // delay in the loop). + // We want to have a total delay of 1.5 bit time. Inside the loop, + // we already wait for 1 bit time - 23 cycles, so here we wait for + // 0.5 bit time - (71 + 18 - 22) cycles. + _rx_delay_centering = subtract_cap(bit_delay / 2, (4 + 4 + 75 + 17 - 23) / 4); + + // There are 23 cycles in each loop iteration (excluding the delay) + _rx_delay_intrabit = subtract_cap(bit_delay, 23 / 4); + + // There are 37 cycles from the last bit read to the start of + // stopbit delay and 11 cycles from the delay until the interrupt + // mask is enabled again (which _must_ happen during the stopbit). + // This delay aims at 3/4 of a bit time, meaning the end of the + // delay will be at 1/4th of the stopbit. This allows some extra + // time for ISR cleanup, which makes 115200 baud at 16Mhz work more + // reliably + _rx_delay_stopbit = subtract_cap(bit_delay * 3 / 4, (37 + 11) / 4); + #else // Timings counted from gcc 4.3.2 output + // Note that this code is a _lot_ slower, mostly due to bad register + // allocation choices of gcc. This works up to 57600 on 16Mhz and + // 38400 on 8Mhz. + _rx_delay_centering = subtract_cap(bit_delay / 2, (4 + 4 + 97 + 29 - 11) / 4); + _rx_delay_intrabit = subtract_cap(bit_delay, 11 / 4); + _rx_delay_stopbit = subtract_cap(bit_delay * 3 / 4, (44 + 17) / 4); + #endif + - // Set up RX interrupts, but only if we have a valid RX baud rate - if (_rx_delay_stopbit) - { // Enable the PCINT for the entire port here, but never disable it // (others might also need it, so we disable the interrupt by using // the per-pin PCMSK register). diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h index 2b125985e19..e28da98e517 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.h @@ -56,6 +56,7 @@ class SoftwareSerial : public Stream volatile uint8_t *_pcint_maskreg; uint8_t _pcint_maskvalue; + // Expressed as 4-cycle delays (must never be 0!) uint16_t _rx_delay_centering; uint16_t _rx_delay_intrabit; uint16_t _rx_delay_stopbit; @@ -78,6 +79,9 @@ class SoftwareSerial : public Stream void setRX(uint8_t receivePin); void setRxIntMsk(bool enable) __attribute__((__always_inline__)); + // Return num - sub, or 1 if the result would be < 1 + static uint16_t subtract_cap(uint16_t num, uint16_t sub); + // private static method for timing static inline void tunedDelay(uint16_t delay); From 9cf3740a03da197e49101c0fc07d732735d76bb8 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Mon, 8 Dec 2014 09:27:41 +0100 Subject: [PATCH 19/20] Remove unneeded #ifdef in SoftwareSerial The debugPulse function definition already checks for _DEBUG, resulting in an empty function definiton and the function call being optimized away. --- .../arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp index 2977b1cba0f..b7dc5c2de52 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp @@ -175,9 +175,7 @@ void SoftwareSerial::recv() } else { -#if _DEBUG // for scope: pulse pin as overflow indictator DebugPulse(_DEBUG_PIN1, 1); -#endif _buffer_overflow = true; } From 90ca3934f25c992ebab0dfffc5715e949ee4081e Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Mon, 26 Jan 2015 17:04:26 +0100 Subject: [PATCH 20/20] Prevent low pulse on TX initialization in SoftwareSerial Previously, the TX pin would be set to output first and then written high (assuming non-inverted logic). When the pin was previously configured for input without pullup (which is normal reset state), this results in driving the pin low for a short when initializing. This could accidenttally be seen as a stop bit by the receiving side. By first writing HIGH and then setting the mode to OUTPUT, the pin will have its pullup enabled for a short while, which is harmless. --- .../arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp index b7dc5c2de52..527f3f9f03f 100644 --- a/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp +++ b/hardware/arduino/avr/libraries/SoftwareSerial/SoftwareSerial.cpp @@ -266,8 +266,12 @@ SoftwareSerial::~SoftwareSerial() void SoftwareSerial::setTX(uint8_t tx) { - pinMode(tx, OUTPUT); + // First write, then set output. If we do this the other way around, + // the pin would be output low for a short while before switching to + // output hihg. Now, it is input with pullup for a short while, which + // is fine. With inverse logic, either order is fine. digitalWrite(tx, _inverse_logic ? LOW : HIGH); + pinMode(tx, OUTPUT); _transmitBitMask = digitalPinToBitMask(tx); uint8_t port = digitalPinToPort(tx); _transmitPortRegister = portOutputRegister(port);