SlashDevin / NeoSWSerial

Efficient alternative to SoftwareSerial with attachInterrupt for RX chars, simultaneous RX & TX
169 stars 42 forks source link

Copying your improvements to SS to an SDI-12 library #23

Closed SRGDamia1 closed 6 years ago

SRGDamia1 commented 6 years ago

Would you mind if I use some of the improvements you've made to the timing and interrupts from SoftwareSerial to improve an SDI-12 library?

SDI-12 is a single-wire protocol that's defined to be at only 1200 baud. Because it's such a slow baud rate, the current implementation (based on SoftwareSerial) keeps all interrupts disabled for a long time while characters are sent and received. I think your method of using the timers to reduce the amount of time interrupts are off would be a nice improvement. I have no idea when (if?) I'll ever have time to make the changes to the SDI-12 library, and I'll definitely give you credit for the ideas, but I wanted to check that it's ok with you first.

SlashDevin commented 6 years ago

Lower baud rates are not supported by NeoSWSerial, because it uses the 8-bit timer that the Arduino core is using for the micros() and millis() built-in functions. This timer (TIMER0) rolls over from 256 to 0 once every 1.024ms.

If you look at the code, it assumes that subtracting the previous timer value from the current timer value is a valid measure of the number of elapsed bit times. At 9600, it can just barely measure all 10 bit times of a 0xFF character (all ones), because that takes about 1ms.

At slower baud rates, the calculation of bit times can be incorrect when the transition from 0 to 1 (or 1 to 0) is more than 1ms away from the previous transition. You would have to keep track of the overflows, or call the expensive micros() to get a 32-bit timestamp for the current-previous calculation.

This might work for 1200 baud, because there is lots of time between bits. It does require changing many of the 8-bit variable declarations in NeoSWSerial to 16- or even 32-bit variables. This would make the calculations too slow for the faster baud rates (9600 to 38400).

There are some other techniques I have considered for these slow baud rates, but it requires using another ISR vector (overflow is used by the core, so maybe the compare vector could be used). Essentially, extra interrupts could provide extra bits to the 8-bit timer variables. Unfortunately, it's not on my list of things to do right now...

BTW, NeoSWSerial is not a modified version of SoftwareSerial. It is a new class that was originally written by an Arduino forum user. He let me polish it up into a class and distributable Arduino Library.

SRGDamia1 commented 6 years ago

Thank you so much for the thoughts. I didn't look that closely into the timing and roll-overs. My understanding of the guts of the timers and the "expense" of a lot of the built-in functions is weak, at best. I'll have to give it a lot more thought.

SRGDamia1 commented 6 years ago

I just wanted to let you know that I did was able to use your timing examples to re-write the interrupts on the SDI-12 library. I did have to use a new timer/prescaler to get it working. It gave me a whole new appreciation for the amount of thought that went into this library in the first place.

Because 1200 baud is slow, I added this chunk to actually calculate the position of the last out bit in writing characters:

  // Calculate the position of the last bit that is a 0/HIGH (ie, HIGH, not marking)
  // That bit will be the last time-critical bit.  All bits after that can be
  // sent with interrupts enabled.

  uint8_t lastHighBit = 9;  // The position of the last bit that is a 0 (ie, HIGH, not marking)
  uint8_t msbMask = 0x80;  // A mask a 1 at the msb
  while (msbMask & outChar) {
    lastHighBit--;
    msbMask >>= 1;
}

I wrote it to begin sending the start bit, and then to calculate parity and the last 0 bit while the start bit was going out. I'm not sure if that would work at any faster baud rates.

SlashDevin commented 6 years ago

Very nice! That was a TO DO item for me here.

At 1200, you might be able to use micros(). With 833us per bit, there's lots of time to do things. You may not need a timer for that, and you might be able to leave interrupts enabled. Since the write is blocking, the sketch won't be trying to do something else. Only ISRs could perturb the timing, but they should be quick enough. There is some slop allowed in the bit edges, too.

I've been thinking about seeing if NeoSWSerial could work at slower baud rates by switching to using micros() instead of TIMER0 TCNTX. Idle thoughts.

Have you posted this new library somewhere?

SRGDamia1 commented 6 years ago

I only got the idea from your TODO. I would never have thought of that on my own.

The library is here: https://github.com/EnviroDIY/Arduino-SDI-12

I actually tried using micros(), but it didn't work for me. I found some other problems in my implementation after switching from micros() to a timer, but I didn't go back and re-try micros afterwards. I should double check my commit history.

I thought about allowing interrupts during the whole write process, but I'm wary about what others might put into their ISR's. SDI-12 is usually used for environmental sensors, so I'm worried that the same sort of people who would be using it might also have an ISR set up to do something like write a line to an SD card.

SlashDevin commented 6 years ago

an ISR set up to do something like write a line to an SD card.

Well, that would never work, because SPI reads/writes use interrupts.

That is a valid concern, though. Moreso at 2400 and 4800.

SRGDamia1 commented 6 years ago

I double checked my history, it was definitely the switch from using micros to using a timer that got things working, so even at the amazingly slow 1200 baud, the micros() function isn't fast enough.