Reading input through an attached handle is not equivalent to read()/available()

[woj76]

The board I use is Leonardo CAN-BUS (see http://www.hobbytronics.co.uk/leonardo-canbus). I have written the small sketch below. The device I read is a Zeitronix Zt-2 Wideband data logger, it essentially constantly spits 17 bytes frames at 9600 baud. When I read the input through the regular available() / read() scheme, everything works fine. When I do the same (in essence) through the attached interrupt, I periodically get garbage data read out. To fix this I need to call available() periodically nevertheless. (For me it defeats the whole purpose of having the interrupt handler where I could collect the 17 bytes frames in one cyclic array to always have fresh frame readout without directly controlling the serial input). A fix to this might be obvious (I can see that calling available() triggers character collection in some cases), but I do not see it. (I also checked, I do not miss characters in my sketch).

The sketch:

#include <NeoSWSerial.h>

NeoSWSerial mySerial(8, 9);
int i = 0;

volatile uint8_t r=0;
volatile uint8_t cr;

static void handleRxChar( uint8_t c ) {
  cr = c;
  r = 1;
}

void setup() {
  Serial.begin(115200);
  mySerial.attachInterrupt(handleRxChar);
  mySerial.begin(9600);
}

void loop() {
  if(r) { // normally mySerial.available()
    // cr = mySerial.read();
    r = 0;
    Serial.print(cr & 0xFF, HEX); Serial.print(" ");
    if(i++ % 17 ==0) Serial.println();
  }else{
    mySerial.available(); // without this spurious / wrong bytes are read
  }  
}

[woj76]

Forgot to say, I had to comment out this in the library, it would not compile otherwise, but this seems to be a nop for Leonardo anyhow:

/*
  if (F_CPU == 8000000L) {
    // Have to use timer 2 for an 8 MHz system.
    #if defined(__AVR_ATtiny25__) | \
        defined(__AVR_ATtiny45__) | \
        defined(__AVR_ATtiny85__).
      TCCR1  = 0x06;  // divide by 32
    #else
      TCCR2A = 0x00;
      TCCR2B = 0x03;  // divide by 32
    #endif
  }
*/

[woj76]

Also, this is the readout that I get without available():

0
1 2 93 C8 0 0 0 5B 80 0 10 3 1 3 7 5 9 
9B C8 0 0 0 5B 80 0 10 FF 1 3 1 0 1 2 93 
C8 0 0 0 5B 80 0 10 FF 1 1 3 7 5 13 4D C8 
0 0 0 5B 80 0 10 FF 1 3 1 0 1 2 93 C8 0 
0 0 5B 80 0 10 FF 0 0 80 0 1 2 93 C8 0 0 
0 5B 80 0 10 FF 1 3 1 0 1 2 93 C8 0 0 0 
5B 80 0 11 3 1 3 7 5 9 4D C8 0 0 0 5B 80 
0 11 FF 0 0 80 0 1 2 93 C8 0 0 0 5B 80 0 
11 FF 1 3 1 0 1 2 93 C8 0 0 0 5B 80 0 11 
FF 1 1 3 7 5 9 4D C8 0 0 0 5B 80 0 11 FF 
1 1 3 7 5 9 4D C8 0 0 0 5B 80 0 11 3 1 
3 7 5 9 4D C8 0 0 0 5B 80 0 12 FF 1 3 1 
0 1 2 93 C8 0 0 0 5B 80 0 12 3 1 3 3 B 
9 4D C8 0 0 0 5B 80 0 12 3 1 3 7 5 9 4D 
C8 0 0 0 5B 80 0 12 FF 1 3 1 0 1 2 93 C8 

This is the readout I should get (with available()):

0 
1 2 93 C8 0 0 0 5B 80 0 F FF FF 0 0 80 0 
1 2 93 C8 0 0 0 5B 80 0 F FF FF 0 0 80 0 
1 2 93 C8 0 0 0 5B 80 0 F FF FF 0 0 80 0 
1 2 93 C8 0 0 0 5B 80 0 F FF FF 0 0 80 0 
1 2 93 C8 0 0 0 5B 80 0 F FF FF 0 0 80 0 
1 2 93 C8 0 0 0 5B 80 0 F FF FF 0 0 80 0

And actually, it is almost clear to me that it is the FF data that throws the reading off.

[SlashDevin]

This is bug #2.      :-(

As you noted, calling available is sufficient to complete the 0xFF character. I have toyed with several different techniques to detect the condition where the 0xC0-0xFF characters do not have a final bit transition.

The problem is that there is no actual interrupt at the end of these characters. The next interrupt occurs on the next character's start bit. It seems like the next start bit in rxISR should be able to complete the previous character. IIRC, I was running into the problem that 9600 baud characters take 1.04ms to complete. That is longer than the 1ms overflow of TIMER0 (8-bit TCNT), so it requires calling the expensive micros().

A subtle aspect to this is that it could be a long time before another character starts. If a 0xC0-0xFF character is the last character of a packet, it won't be completed until the next packet starts... however long that is.

I considered using another timer resource, but that really requires some restructuring to maintain portability. AltSoftSerial has a nice structure for that, and maybe NeoSWSerial could do something similar (see #17). I would want it to be a configuration option, because the majority of users do not need attachInterrupt. Not requiring additional resources is one of NeoSWSerial's advantages over other libraries. OTOH, that could allow other baud rates (TIMER0 is too slow for faster baud rates and too fast for slower baud rates).

The work-around is just what you did: call available() from anywhere, periodically, as a way to invoke the private checkRxTime(). Other suggestions welcome!

[woj76]

Sorry for duplicating bugs, I simply assumed that this must have been unnoticed so far, because if it had been noticed it would be fixed by now. But I see the issue is not trivial (I actually looked a bit at the bit calculation routines, but they seem spotless). In my target project I cannot assume I will be able to call available() often enough / at the right time (it has to be called at the end of 0b11xxxxxx character, right?), and the ability to use attachInterrupt() is essential. So what I look at is to have a timer capture interrupt triggered at an assumed end of character to call checkRxTime() attached to a coarse / fine grained enough timer to do the right thing. Or trigger the ISR of the library (inject in essence). Perhaps that's a solution you are looking for? It would only trigger one more interrupt per character. Otherwise, I think I may dump attachInterrupt() altogether, set up a periodic interrupt (with half a character time or so) and do available() / read() in there. Just have to refresh my knowledge about how this is done on the Arduino (pretty sure I did something like this before).

[woj76]

OK, I played a bit with Timer1 library, I can essentially toss a coin instead to produce characters :/. (There is probably a solution to this.) But for a simpler fix, for my case, I am certain that the characters will keep coming, so my issue is completing characters on start bit reception. The issue seems to be 0xFF duration that you mentioned and overflown T0 counter, so my next attempt will be to also record millis() in the library code along with t0 and do something clever (detect the overflow and do the equivalent of checkRxTime). Too tired and too late here, but I will come back to this.

[iglushko]

I would suggest that you should update the documentation describing this problem. Or probably just disable the feature of attaching an interrupt altogether.

Even if you would use micros() to avoid multiple counter overflows that will not solve a problem of delay between the moment the source sent something and the moment your code receives the last byte (if it has ones in the two most significant bits). And that might be important in some cases.