PaulStoffregen / AltSoftSerial

Software emulated serial using hardware timers for improved compatibility
http://www.pjrc.com/teensy/td_libs_AltSoftSerial.html
328 stars 131 forks source link

Fix race condition cutting stop bit short #15

Closed matthijskooijman closed 8 years ago

matthijskooijman commented 8 years ago

I encountered a race condition that occurs because the TX code does not wait for the stop bit of the last byte in the buffer to complete. If a new byte is written during that stop bit, its start bit gets sent right away, halfway through the previous stop bit, breaking the transmission.

This can be reproduced using this bit of code:

  altSerial.begin(9600);
  altSerial.write('X');
  delayMicroseconds(925);
  altSerial.write('X');

Which looks like this:

selection_010

(Note that there is only a very short high time before the initial start bit, because the first byte is sent directly after reset) For more details, see the first commit with a fix.

The second commit fixes a minor nitpick with the interrupt handler I noticed, which causes one less interrupt to be needed for bytes whose MSB is 1. It's nothing important, just a small improvement. If you don't like the way it is coded, an alternative could be to make the return in the if for the stop bit optional, e.g.:

   if (state == 9) {
           tx_state = 10;
           if (!tx_bit) {
                   CONFIG_MATCH_SET();
                   SET_COMPARE_A(target + ticks_per_bit);
                   return;
           }
   }

This is essentially the same as the commit I proposed, but with the last loop iteration "unrolled" (but it also increases code by 4 bytes instead of decreasing by 6).

If you think that the current code is more clear and don't mind the extra interrupt, that's also fine with me :-)

matthijskooijman commented 8 years ago

@PaulStoffregen, any chance you could have a look at this PR and #16? I am finalizing a book that makes use of AltSoftSerial, but one of the examples triggers this race condition exactly. Ideally, this would be fixed, but otherwise I'll have to add a workaround.

PaulStoffregen commented 8 years ago

Yes, I'll get this merged soon.

At this moment I'm dealing with supporting Arduino 1.6.5-r4. The github logs show only changes to the bootloads and text files, but the java bytecode is indeed different from prior releases. :(

matthijskooijman commented 8 years ago

Thanks! Good luck with the 1.6.5 stuff :-)

matthijskooijman commented 8 years ago

Thanks!