EnviroDIY / Arduino-SDI-12

An Arduino library for SDI-12 communication with a wide variety of environmental sensors. This library provides a general software solution, without requiring any additional hardware.
https://github.com/EnviroDIY/Arduino-SDI-12/wiki
BSD 3-Clause "New" or "Revised" License
159 stars 100 forks source link

SDI-12 and SoftwareSerial Compatibility Issues #8

Open Kevin-M-Smith opened 10 years ago

Kevin-M-Smith commented 10 years ago

Problem Overview

Refresher on Interrupts

If you look at the SoftwareSerial library: https://github.com/arduino/Arduino/blob/master/libraries/SoftwareSerial/SoftwareSerial.cpp

In SoftwareSerial.cpp:

#if defined(PCINT0_vect)
ISR(PCINT0_vect)
{
  SoftwareSerial::handle_interrupt();
}
#endif

#if defined(PCINT1_vect)
ISR(PCINT1_vect)
{
  SoftwareSerial::handle_interrupt();
}
#endif

#if defined(PCINT2_vect)
ISR(PCINT2_vect)
{
  SoftwareSerial::handle_interrupt();
}
#endif

#if defined(PCINT3_vect)
ISR(PCINT3_vect)
{
  SoftwareSerial::handle_interrupt();
}
#endif

Code Interpretation

As an example:

Pins 2 and 3 map to the PCINT Request Vector PCINT2. As a simple fix we can comment out the sections of code we are not using. (You may want to include a note as to why you changed it.)

In SoftwareSerial.cpp:

// Last edit: June 30th, 2014 by KMS
// Lines in this block are commented out
// because PCINT0, PCINT1, and PCINT3 
// are not used by SoftwareSerial for this
// project. 
//#if defined(PCINT0_vect)
//ISR(PCINT0_vect)
//{
//  SoftwareSerial::handle_interrupt();
//}
//#endif

//#if defined(PCINT1_vect)
//ISR(PCINT1_vect)
//{
//  SoftwareSerial::handle_interrupt();
//}
//#endif

#if defined(PCINT2_vect)
ISR(PCINT2_vect)
{
  SoftwareSerial::handle_interrupt();
}
#endif

//#if defined(PCINT3_vect)
//ISR(PCINT3_vect)
//{
//  SoftwareSerial::handle_interrupt();
//}
//#endif

Next, we are going to do the complementary action in the SDI-12 Library. Since we are using pin 9 for SDI-12, we only need to assign our ISR to PCINT Request Vector PCINT0.

In SDI12.cpp:

// Last edit: June 30th, 2014 by KMS
// Lines in this block are commented out
// because PCINT1, PCINT2, and PCINT3 
// are not used by SDI-12 for this
// project. 
//6.3
#if defined(PCINT0_vect)
ISR(PCINT0_vect){ SDI12::handleInterrupt(); }
#endif

//#if defined(PCINT1_vect)
//ISR(PCINT1_vect){ SDI12::handleInterrupt(); }
//#endif

//#if defined(PCINT2_vect)
//ISR(PCINT2_vect){ SDI12::handleInterrupt(); }
//#endif

//#if defined(PCINT3_vect)
//ISR(PCINT3_vect){ SDI12::handleInterrupt(); }
//#endif

Restart the Arduino IDE, and it should recognize the changes to the libraries and your code should compile without issue.

Towards A Better Fix?

Limitations

Looking forward to your thoughts.

SRGDamia1 commented 7 years ago

Some thoughts:

Could the communication be done with a timer, like AltSoftSerial? Or, perhaps, given the low baud rate, with delays like OneWire?

Kevin-M-Smith commented 7 years ago

I quick look at the AltSoftSerial library looks promising. I hope to get a chance to take a more in-depth look in the next few days.

SRGDamia1 commented 7 years ago

I'm playing with an Adafruit Feather 32u4 (same processor as the Leonardo) and the shortcut of using specific interrupt vectors doesn't seem to be working for me.

Right now I have a version of the SDI-12 library, a version of Software Serial, and a version of a PCInt library all integrated into the ModularSensors library. It works fine on the Mayfly (AtMega 1284P with TQFN) and would probably be ok on an Uno, but I'm trying to see what it would take to make it work with other boards. (32u4, SAMD21, ESP8266, maybe Teensy or STM32... ) The SoftwareSerial/SDI12 conflict is an ugly hold-up and I'm just not sure how to get around it.

I've thought about forcing users to create sensor objects with the SS or SDI12 instance as part of the constructor, but that just doesn't seem as clean as creating it within the sensor. And it doesn't really solve anything because the user would be stuck in the same position I'm in, digging around for a way of making the libraries compatible when they want to combine SDI12 and serial sensors.

SRGDamia1 commented 7 years ago

In combining them, I think I would prefer to have a version of SS that allows as many pins as possible, but don't see the need the SDI12 to support more than one pin. With SDI12, 62 can be added to the same Arduino pin, but each serial sensor needs one two completely independent pins. So, in that way, I guess making SDI12 more like AltSS and monopolizing the single input capture pin would be better, although that still knocks out the one or two output compare pins.

I'm also integrating the OneWire library for the Dallas temperature probes, which is why I've looked at its method of using delays for the timing and was wondering if it could work at the 1200 baud rate. I know using delays isn't great either because it's wasting time doing nothing and could miss other interrupts, but at least it would solve the compatibility problem.

SRGDamia1 commented 7 years ago

Another SS library I came across: https://github.com/SlashDevin/NeoSWSerial. This one still uses interrupts, but does have the "clever set of #define statements" to allow it to play more nicely with other libraries. It also allows you to use it to define other pin change interrupts, so although it clashes with them, it might not require the use of a separate library for other interrupts.

SRGDamia1 commented 7 years ago

Actually.. thinking about it even more.. using only delays clearly won't work because, while we can always depend on a device responding within 15ms of a request and shutting up within 7.5ms of finishing, devices can also send out service requests if they finish up early. We'd never be able to catch the service request if we're depending on the 15ms/7.5ms timing.

SRGDamia1 commented 7 years ago

I've added the "clever defines" so an pin change interrupt can set up the interrupts for this library and tested it with Grey Gnomes EnableInterrupt on the Mayfly, 32u4, and M0. This requires the user to uncomment the line #define SDI12_EXTERNAL_PCINT and recompile the library and then call SDI12::handleInterrupt() on the receive pin from their other pin change library.

I'm debating about merging the cpp and h files into a single header file so that the library is not pre-compiled the statement #define SDI12_EXTERNAL_PCINT can just be added to any other sketch or library to stop SDI12 from hogging the interrupts. @Kevin-M-Smith , what do you think?

nimasaj commented 5 years ago

I have this issue with Arduino pro mini 3.3V and proposed solution in post#1 regarding commenting PCINT Request Vector lines is not working. Has anyone solved this issue or found a solution for this? I would be thankful if you share your solutions with me.

Sample code ``` #include #include // the issue exists even with NeoSWserial #define DATA_PIN 7 #define rxPin 5 #define txPin 6 SoftwareSerial Sigfox = SoftwareSerial(rxPin, txPin); //((Module.Tx>>>MCU.Rx), (Module.Rx>>>MCU.Tx)) boolean debug = true; SDI12 mySDI12(DATA_PIN); void setup() { if (debug){ Serial.begin(9600); Serial.println("Hello there!"); } mySDI12.begin(); delay(500); } void loop() { String cmd5="0M!"; delay(2500); SDICommand(cmd5); delay(2500); Serial.println(""); delay(10*1000); } String SDIgetData(){ String data = ""; char output; while (!mySDI12.available()){ //blink(); } while(mySDI12.available()){ output = mySDI12.read(); if (output!=0x0A){ ///// Ignore new line feed in responses data += output; } delay(10); } return data; } String SDICommand(String cmd){ mySDI12.sendCommand(cmd); String ret=SDIgetData(); if (debug){ Serial.print("Response of "+cmd+" : "); Serial.println(ret); } //delay(2*500); mySDI12.flush(); return ret; } ```

This is the error I get after compiling samples code on Arduino IDE 1.8.9:

Compilation error ``` In file included from C:\Users\XXX\AppData\Local\Arduino15\packages\arduino\hardware\avr\1.6.23\libraries\SoftwareSerial\src\SoftwareSerial.cpp:41:0: C:\Users\XXX\AppData\Local\Arduino15\packages\arduino\hardware\avr\1.6.23\libraries\SoftwareSerial\src\SoftwareSerial.cpp:239:5: error: 'void __vector_5()' aliased to undefined symbol '__vector_3' ISR(PCINT2_vect, ISR_ALIASOF(PCINT0_vect)); ^ C:\Users\XXX\AppData\Local\Arduino15\packages\arduino\hardware\avr\1.6.23\libraries\SoftwareSerial\src\SoftwareSerial.cpp:239:5: error: 'void __vector_5()' aliased to undefined symbol '__vector_3' Using library Arduino-SDI-12 at version 1.3.4 in folder: C:\Users\XXX\Documents\Arduino\libraries\Arduino-SDI-12 Using library SoftwareSerial at version 1.0 in folder: C:\Users\XXX\AppData\Local\Arduino15\packages\arduino\hardware\avr\1.6.23\libraries\SoftwareSerial exit status 1 Error compiling for board Arduino Pro or Pro Mini. ```
SoftwareSerial.cpp file ``` /* SoftwareSerial.cpp (formerly NewSoftSerial.cpp) - Multi-instance software serial library for Arduino/Wiring -- Interrupt-driven receive and other improvements by ladyada (http://ladyada.net) -- Tuning, circular buffer, derivation from class Print/Stream, multi-instance support, porting to 8MHz processors, various optimizations, PROGMEM delay tables, inverse logic and direct port writing by Mikal Hart (http://www.arduiniana.org) -- Pin change interrupt macros by Paul Stoffregen (http://www.pjrc.com) -- 20MHz processor support by Garrett Mace (http://www.macetech.com) -- ATmega1280/2560 support by Brett Hagman (http://www.roguerobotics.com/) This library is free software; you can redistribute it and/or modify it under the terms of the GNU Lesser General Public License as published by the Free Software Foundation; either version 2.1 of the License, or (at your option) any later version. This library is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more details. You should have received a copy of the GNU Lesser General Public License along with this library; if not, write to the Free Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA The latest version of this library can always be found at http://arduiniana.org. */ // When set, _DEBUG co-opts pins 11 and 13 for debugging with an // oscilloscope or logic analyzer. Beware: it also slightly modifies // the bit times, so don't rely on it too much at high baud rates #define _DEBUG 0 #define _DEBUG_PIN1 11 #define _DEBUG_PIN2 13 // // Includes // #include #include #include #include #include // // Statics // SoftwareSerial *SoftwareSerial::active_object = 0; uint8_t SoftwareSerial::_receive_buffer[_SS_MAX_RX_BUFF]; volatile uint8_t SoftwareSerial::_receive_buffer_tail = 0; volatile uint8_t SoftwareSerial::_receive_buffer_head = 0; // // Debugging // // This function generates a brief pulse // for debugging or measuring on an oscilloscope. #if _DEBUG inline void DebugPulse(uint8_t pin, uint8_t count) { volatile uint8_t *pport = portOutputRegister(digitalPinToPort(pin)); uint8_t val = *pport; while (count--) { *pport = val | digitalPinToBitMask(pin); *pport = val; } } #else inline void DebugPulse(uint8_t, uint8_t) {} #endif // // Private methods // /* static */ inline void SoftwareSerial::tunedDelay(uint16_t delay) { _delay_loop_2(delay); } // This function sets the current object as the "listening" // one and returns true if it replaces another bool SoftwareSerial::listen() { if (!_rx_delay_stopbit) return false; if (active_object != this) { if (active_object) active_object->stopListening(); _buffer_overflow = false; _receive_buffer_head = _receive_buffer_tail = 0; active_object = this; setRxIntMsk(true); return true; } return false; } // Stop listening. Returns true if we were actually listening. bool SoftwareSerial::stopListening() { if (active_object == this) { setRxIntMsk(false); active_object = NULL; return true; } return false; } // // The receive routine called by the interrupt handler // void SoftwareSerial::recv() { #if GCC_VERSION < 40302 // Work-around for avr-gcc 4.3.0 OSX version bug // Preserve the registers that the compiler misses // (courtesy of Arduino forum user *etracer*) asm volatile( "push r18 \n\t" "push r19 \n\t" "push r20 \n\t" "push r21 \n\t" "push r22 \n\t" "push r23 \n\t" "push r26 \n\t" "push r27 \n\t" ::); #endif uint8_t d = 0; // If RX line is high, then we don't see any start bit // 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); // Read each of the 8 bits for (uint8_t i=8; i > 0; --i) { tunedDelay(_rx_delay_intrabit); d >>= 1; DebugPulse(_DEBUG_PIN2, 1); if (rx_pin_read()) d |= 0x80; } if (_inverse_logic) d = ~d; // if buffer full, set the overflow flag and return 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 = next; } else { DebugPulse(_DEBUG_PIN1, 1); _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 // Work-around for avr-gcc 4.3.0 OSX version bug // Restore the registers that the compiler misses asm volatile( "pop r27 \n\t" "pop r26 \n\t" "pop r23 \n\t" "pop r22 \n\t" "pop r21 \n\t" "pop r20 \n\t" "pop r19 \n\t" "pop r18 \n\t" ::); #endif } uint8_t SoftwareSerial::rx_pin_read() { return *_receivePortRegister & _receiveBitMask; } // // Interrupt handling // /* static */ inline void SoftwareSerial::handle_interrupt() { if (active_object) { active_object->recv(); } } /*#if defined(PCINT0_vect) ISR(PCINT0_vect) { SoftwareSerial::handle_interrupt(); } #endif #if defined(PCINT1_vect) ISR(PCINT1_vect, ISR_ALIASOF(PCINT0_vect)); #endif*/ #if defined(PCINT2_vect) ISR(PCINT2_vect, ISR_ALIASOF(PCINT0_vect)); #endif /*#if defined(PCINT3_vect) ISR(PCINT3_vect, ISR_ALIASOF(PCINT0_vect)); #endif*/ // // Constructor // SoftwareSerial::SoftwareSerial(uint8_t receivePin, uint8_t transmitPin, bool inverse_logic /* = false */) : _rx_delay_centering(0), _rx_delay_intrabit(0), _rx_delay_stopbit(0), _tx_delay(0), _buffer_overflow(false), _inverse_logic(inverse_logic) { setTX(transmitPin); setRX(receivePin); } // // Destructor // SoftwareSerial::~SoftwareSerial() { end(); } void SoftwareSerial::setTX(uint8_t tx) { // 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 high. 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); } void SoftwareSerial::setRX(uint8_t rx) { pinMode(rx, INPUT); if (!_inverse_logic) digitalWrite(rx, HIGH); // pullup for normal logic! _receivePin = rx; _receiveBitMask = digitalPinToBitMask(rx); uint8_t port = digitalPinToPort(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 // void SoftwareSerial::begin(long speed) { _rx_delay_centering = _rx_delay_intrabit = _rx_delay_stopbit = _tx_delay = 0; // Precalculate the various delays, in number of 4-cycle delays uint16_t bit_delay = (F_CPU / speed) / 4; // 12 (gcc 4.8.2) or 13 (gcc 4.3.2) cycles from start bit to first bit, // 15 (gcc 4.8.2) or 16 (gcc 4.3.2) cycles between bits, // 12 (gcc 4.8.2) or 14 (gcc 4.3.2) cycles from last bit to stop bit // These are all close enough to just use 15 cycles, since the inter-bit // timings are the most critical (deviations stack 8 times) _tx_delay = subtract_cap(bit_delay, 15 / 4); // Only setup rx when we have a valid PCINT for this pin if (digitalPinToPCICR(_receivePin)) { #if GCC_VERSION > 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 // 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)); // 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 } #if _DEBUG pinMode(_DEBUG_PIN1, OUTPUT); pinMode(_DEBUG_PIN2, OUTPUT); #endif listen(); } void SoftwareSerial::setRxIntMsk(bool enable) { if (enable) *_pcint_maskreg |= _pcint_maskvalue; else *_pcint_maskreg &= ~_pcint_maskvalue; } void SoftwareSerial::end() { stopListening(); } // Read data from buffer int SoftwareSerial::read() { if (!isListening()) return -1; // Empty buffer? if (_receive_buffer_head == _receive_buffer_tail) return -1; // Read from "head" uint8_t d = _receive_buffer[_receive_buffer_head]; // grab next byte _receive_buffer_head = (_receive_buffer_head + 1) % _SS_MAX_RX_BUFF; return d; } int SoftwareSerial::available() { if (!isListening()) return 0; return (_receive_buffer_tail + _SS_MAX_RX_BUFF - _receive_buffer_head) % _SS_MAX_RX_BUFF; } size_t SoftwareSerial::write(uint8_t b) { if (_tx_delay == 0) { setWriteError(); 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 if (inv) *reg |= reg_mask; else *reg &= inv_mask; tunedDelay(delay); // Write each of the 8 bits for (uint8_t i = 8; i > 0; --i) { if (b & 1) // choose bit *reg |= reg_mask; // send 1 else *reg &= inv_mask; // send 0 tunedDelay(delay); b >>= 1; } // restore pin to natural state if (inv) *reg &= inv_mask; else *reg |= reg_mask; SREG = oldSREG; // turn interrupts back on tunedDelay(_tx_delay); return 1; } void SoftwareSerial::flush() { // There is no tx buffering, simply return } int SoftwareSerial::peek() { if (!isListening()) return -1; // Empty buffer? if (_receive_buffer_head == _receive_buffer_tail) return -1; // Read from "head" return _receive_buffer[_receive_buffer_head]; } ```

By the way, where/how can I find PCINT structure (pin to port mapping) for Arduino pro mini to know which pin in associated with which PCINT Request Vector (PCINT0, PCINT1, PCINT2)?

SRGDamia1 commented 5 years ago

Look at example J. It's working code of calling the SDI-12 interrupt from another interrupt library. The "bestiary" of the EnableInterrupt library is helpful for interrupt mapping.

Kalirren commented 2 years ago

Sometime ago, this issue seems to have been addressed through the release of the SDI12_PCINT3 branch. now renamed mayfly branch. Currently, SDI12_PCINT3.cpp in that branch seems NOT to have the other interrupts cropped, as advertised. I had to comment them out myself to make code compile correctly. If I can figure out how to submit a pull request in the next few hours, I will do so.

hexad commented 1 year ago

@Kalirren I am trying to replicate what you did. On that branch, the only function that gives trouble is the receiveISR(). What exactly did you comment out to make that work?

Kalirren commented 1 year ago

Hi @hexad, It has been several years, but thankfully I left a trail for myself to follow. I opened a pull request to fix this bug, which I consider a regression, almost 1.5 years ago. No one has merged it?

https://github.com/EnviroDIY/Arduino-SDI-12/pull/84

You can see the code I commented out in that section. Note that that code is actually the code that concerns the interrupts that the mayfly branch documentation CLAIMS are disabled, but apparently aren't actually disabled...?

hexad commented 1 year ago

@Kalirren Thank you very much for your quick reply! I tried to comment out your code but it didn't work... I have this to note:

  1. With your modification the code compiled fine (even with SoftwareSerial). However, I kept receiving bad values of data (The Volumetric Water Content for Mineral Soils is: -0.70 m^3/m^3).
  2. When I removed the SoftwareSerial library, then I received junk data. (Opening SDI-12 bus... ⸮\⸮y==⸮⸮⸮⸮⸮⸮⸮⸮%⸮⸮⸮⸮͹rrj⸮⸮)

Please note that I know my code works fine. Because when I don't include the SoftwareSerial library and keep the code without your modifications, the values I get are correct...

hexad commented 1 year ago

In other words, while the library may compile, it does not work without those timer interrupts that you removed.

Kalirren commented 1 year ago

Ahh, this is enlightening. I recall that I had written a different section of code explicitly enabling some interrupts on registers 0 and 2 on assumption that the Mayfly branch documentation was correct. Because the interrupts were being enabled twice, once in my code and once in the library, there was a compile error, which I fixed by commenting out the version in the library to make it consistent with branch documentation.

I don't believe I ever used the receiveISR() function. If I recall correctly, I simply couldn't get it to work...