Serial & Timer

Could someone please explain why would Timer1 affect Serial communication? (using Arduino Mega 2560)

There are many lines in this project so I'll give only relevant codes.

void setup() {
  ...
  Timer1.initialize(100);
  Timer1.attachInterrupt(update,100);
  ...
}

void loop() {
  checkTouch();
  ...
}

Update function calls functions to read current state of 2 quadrature encoders. It usually takes 30 microseconds to run. So an interval of 100 microseconds usually do not lock the processor.

checkTouch calls following functions. Puts a circle on the touchscreen if there is a touch. Resets the screen if the home icon of SMARTGPU is pressed.

uint8_t SMARTGPU::touchScreen(int buffer[]){          //Ask for a touch on the screen, if return=1, touch coordinates are stored on the buffer[]
  Serial2.write('G');
  while(Serial2.available() < 5);
  buffer[0]=Serial2.read();
  buffer[0]=buffer[0]<<8;
  buffer[0]|=Serial2.read();
  buffer[1]=Serial2.read();
  buffer[1]=buffer[1]<<8;
  buffer[1]|=Serial2.read();
  Serial2.read();  
  if(buffer[0]<0x0200){
	return 1;
  }else{
    return 0;
  }
}

uint8_t SMARTGPU::touchIcon(char buffer[]){          //Ask for a touch on the icons of the screen, if return=1, icon name is stored on the buffer[]
  Serial2.write('G');
  while(Serial2.available() < 5);
  buffer[0]=Serial2.read();
  buffer[1]=Serial2.read();
  buffer[2]=Serial2.read();
  buffer[3]=Serial2.read();
  Serial2.read();  
  if(!(buffer[0]<0x02) & (buffer[0]!=0x4E)){
	return 1;
  }else{
    return 0;
  }
}

This works fine if I comment out the Timer1 lines in setup().

Thanks in advance.

I would restructure all this:

  while(Serial2.available() < 5);

If for some reason you don't get 5 bytes your program will hang.

Update function calls functions to read current state of 2 quadrature encoders. It usually takes 30 microseconds to run. So an interval of 100 microseconds usually do not lock the processor.

Why not use interrupts to read the encoders? Or just do it in loop? Doing it the way you have makes it all extremely time-sensitive.

In setup(), I attach the update function to timer1 interrupt. Encoder data must be read even if they don't change. I mean they are already time-sensitive.

As I said, these functions work perfectly if I don't use timer1. Would the serial data be lost if it is received while the update() function is running??

Encoder data must be read even if they don't change.

Care to explain that further?

acarb:
Encoder data must be read even if they don't change.

Why? If you use interrupts then you know when they change. Hence you also know when they don't change.

Let's say you get a change at x seconds and x+10 seconds. Clearly you had no change at x+5 seconds. You don't need a timer to find that out.

acarb:
As I said, these functions work perfectly if I don't use timer1. Would the serial data be lost if it is received while the update() function is running??

Possibly. How long does update run? What does it do? While an ISR is running interrupts are disabled. Thus your loop here:

  while(Serial2.available() < 5);

... may not catch all 5 bytes.

We don't know what baud rate you are using, but let's guess 115200. That is a character arriving every 87 uS. Now you are interrupting every 100 uS for Timer1. That's a lot of interrupts. Merely servicing an ISR takes around 7 uS, plus whatever it actually does. You could easily use up most of your 100 uS in the interrupt routine, thus leaving practically no time over for any other processing.

(Edit) You said earlier the ISR takes 30 uS. I don't know how long the serial interrupt handler takes to service an interrupt (which it would be getting every 87 uS) but let's guess 50 uS. You are really running out of processing time. Your interrupt takes 30 uS, the serial one 50 uS (guess) and you need to service the serial port every 87 uS.

Just how fast is this encoder running? I suspect your design needs rework.

Here is the update fuction.

void encoder::update(){
  currentPinA = digitalRead(Ap);
  currentPinB = digitalRead(Bp);
  if ((prevPinA == HIGH) && (prevPinB == LOW)) {
    if ( currentPinA == LOW)
      currentPos--;
    else if(currentPinB == HIGH)
      currentPos++;
  }
  else if ((prevPinA == HIGH) && (prevPinB == HIGH)) {
    if ( currentPinA == LOW)
      currentPos++;
    else if(currentPinB == LOW)
      currentPos--;
  }
  else if ((prevPinA == LOW) && (prevPinB == HIGH)) {
    if ( currentPinA == HIGH)
      currentPos--;
    else if(currentPinB == LOW)
      currentPos++;
  }
  else{
    if ( currentPinA == HIGH)
      currentPos++;
    else if(currentPinB == HIGH)
      currentPos--;
  }
    //Serial.println(currentPos);
  prevPinA = currentPinA;
  prevPinB = currentPinB;
}

It simply implements the following FSM to calculate position from quadrature signals.

I can read 10,000 positions per round from the encoder that I am currently using. I didn't want to limit the max. rpm so I used the timer as rapidly as possible. After some discussion, we decided to put a speed limit for Arduino's sake. The current period to call update() is 3 milliseconds which will allow a turn of 12 degrees per second. But it doesn't help much. The serial communication is interrupted either immediately or after a while.

I am using 2,000,000 as it is the highest supported by the LCD. I think it is much faster than your guess. But I think it won't help to read encoder on interrupt for the current problem. I need to guarantee that Serial2.available() is not interrupted.

Is it OK to detach the ISR function before "while(Serial2.available() < 5);" and attach it back again after that?

acarb:
The current period to call update() is 3 milliseconds which will allow a turn of 12 degrees per second.

Can you run your calculations past us? 12 degrees per second is 30 times (360/12). That is, once every 33.33 mS.

But earlier in this thread you had:

Timer1.attachInterrupt(update,100);

That's once every 100 uS (ie. every 0.1 mS).

So where does the figure of 3 mS come from? And why are you interrupting every 0.1 mS?

I am using 2,000,000 as it is the highest supported by the LCD.

What does that mean? 2 million what?

Is it OK to detach the ISR function before "while(Serial2.available() < 5);" and attach it back again after that?

If you don't mind the timer being wrong.

If you could respond to why you don't use interrupts to notice when the encoder changes state, that might help.

To be able to get position from quadrature signals, I have to detect every change in 2 digital signals (A and B that goes high low, high high, low high, low low...). I have also shown actions to be taken between transition of those states. The encoder prodives me with 10,000 positions per round, which is 10000/360 positions for a degree. So if the shaft turns 6 degrees per second, it means position will increase
6x10000/360=1000/6=166,666.... every second.

If shaft turn 12 degrees per second, position will increase
2x166,666....=333,333.... every second. To be able to detect a position change, I need to read the encoder state at least 333,333... times a second, which is every 3 milliseconds.

We assumed the speed limit as 6 degrees per second, did those calculations and decided to change timer period to 3 milliseconds which is above our limitations.

baud. Highest rate supported by LCD.

How do you make Arduino Mega detect changes in 2 digital signals using interrupts? Using a timer is the only way I know. And can you guarantee that an interrupt won't affect "while(Serial2.available() < 5);" ?

I'm pretty sure the "while()..." is already interrupted. Timer0 (used for millis(), delay()...) ticks away constantly and interrupts any piece of ordinary code. The question is: "does it matter?".

You need to look into external interrupts. Polling a high resolution quadrature encoder is quite insane. You spend all of your time to see if things have changed, so you don't miss a tick.

Now I removed the timer and just wrote attachInterrupts for encoder outputs. But http://arduino.cc/en/Reference/AttachInterrupt also mentions that serial data may be lost during an interrupt.

I think I will change LCD codes. For example I can change

  Serial2.write('U');
  while(Serial2.available() == 0);

into

  Serial2.write('U');
  while(Serial2.available() == 0){
    a++;
    if(a==500){
      Serial2.write('U');
      a=0;
    }
  }

so that it will resend the query and wait for another answer. Hoping that it won't get interrupted this time.

  while(Serial2.available() == 0){
    a++;
    if(a==500){
      Serial2.write('U');
      a=0;
    }
  }

How long do you think this while loop will take to send another 'U'?

PaulS:

  while(Serial2.available() == 0){

a++;
    if(a==500){
      Serial2.write('U');
      a=0;
    }
  }



How long do you think this while loop will take to send another 'U'?

The problem here is not being able to send 'U' but receiving something properly. If LCD sends a reply while an interrupt is running, the reply is lost, leading to an infinite loop. I wrote that in order to send query again and wait for another reply.

I think the hardware UART will receive a single character no matter what else is going on at the moment. Only the associated interrupts may be delayed and fire a bit later. The data will show up a bit later in the input buffer. So if you don't flood the receiver, you shouldn't loose data. If your quadrature encoder interrupt code is lean an mean (highest priority) and you give the LCD code the time it deserves (almost 0) it will work.

You shouldn't start with making the LCD stuff work first and then adding the encoder code, but the other way around. What good is the LCD display if you can't reliably read the encoder? And if you can't get the serial LCD working, use one with SPI. You can clock the data out as slow as you want and it doesn't matter a thing if it gets interrupted as it is a synchronous transfer.

madworm:
I think the hardware UART will receive a single character no matter what else is going on at the moment. Only the associated interrupts may be delayed and fire a bit later. The data will show up a bit later in the input buffer. So if you don't flood the receiver, you shouldn't loose data. If your quadrature encoder interrupt code is lean an mean (highest priority) and you give the LCD code the time it deserves (almost 0) it will work.

That's what I thought. But there is a note saying "Inside the attached function, delay() won't work and the value returned by millis() will not increment. Serial data received while in the function may be lost. You should declare as volatile any variables that you modify within the attached function." on attachInterrupt() - Arduino Reference. I haven't actually tested this yet. But if it is on a reference page then it must be true. I tried the method I have posted here, but that doesn't work even if I don't interrupt LCD code :frowning: Something is wrong around here. I just can't put my finger on it yet.

madworm:
You shouldn't start with making the LCD stuff work first and then adding the encoder code, but the other way around. What good is the LCD display if you can't reliably read the encoder? And if you can't get the serial LCD working, use one with SPI. You can clock the data out as slow as you want and it doesn't matter a thing if it gets interrupted as it is a synchronous transfer.

That's what I have done. I've finished my work on encoder first. I used to poll but now I am using external interrupts. I haven't even plugged my encoder yet. Just working on LCD code and trying to make it interruptable.

acarb:

[quote author=Nick Gammon link=topic=78661.msg596254#msg596254 date=1321265354]
What does that mean? 2 million what?

baud. Highest rate supported by LCD.
[/quote]

You are talking to the LCD at 2 million baud? Wow. That is sending one character every 5 uS. Besides, updating it that fast is, ah, not necessary. You can't read that fast.

acarb:
Just working on LCD code and trying to make it interruptable.

You shouldn't need to "make it interruptable". All this sort of stuff is just not recommended:

  Serial2.write('U');
  while(Serial2.available() == 0);

What happens if there is no reply? The code hangs indefinitely.

In fact I did a post about that recently:

Your main loop should just be checking for available characters, and then buffering them up. When you get a useful number (eg. 5 in your case it seems) you process them.

Hmm.

To implement LCD library the way you suggested, I will need a queue for any operation such as drawing line, erasing screen etc. To be able to send 1 byte and check for a reply byte in loop(), I will use 2 separate queues (number of bytes to send and number of bytes to receive) since they differ from operation to operation. I will also use a send buffer and a receive buffer.

For example, when I want to put a string on the screen this was what I normally used:

uint8_t SMARTGPU::string(int x1, int y1, int x2, int y2, int colour, uint8_t font, uint8_t fill, char text[]){       //Draw a string on the screen
  int counter=0;
  
  Serial.write('S'); 
  Serial.write('N'); //not SD
  Serial.write(x1>>8); 
  Serial.write(x1);
  Serial.write(y1>>8);
  Serial.write(y1);
  Serial.write(x2>>8); 
  Serial.write(x2);
  Serial.write(y2>>8);
  Serial.write(y2);  
  Serial.write(colour>>8);
  Serial.write(colour);
  Serial.write(font); 
  Serial.write(fill); 
  while(1){
	Serial.write(text[counter]);
    if(text[counter]==0x00){
      break;
	}	
	counter++;
  }
  while(Serial.available() == 0);  
  return Serial.read();
}

where the GPU puts the text[] into an imaginary text box between points (x1, y1) and (x2, y2) using given font and colour.

But now I will change it to this:

uint8_t SMARTGPU::string(int x1, int y1, int x2, int y2, int colour, uint8_t font, uint8_t fill, char text[]){       //Draw a string on the screen
  /*calculate all characters to be send here*/
  enqueue( charArray , numberOfCharsToSend, numberOfCharsToReceive, typeOfOperation);
}

Thus, I will be able to enqueue different operations without waiting for them to actually finish. All I need to do is to check how many bytes do I need to read and how many did I receive so far, send 1 or 2 bytes if there are any waiting to be sent, in every iteration of loop. If received enough, dequeue that operation, use the received output and start with the next one.

This is the only way I could come up with. Do you think this will work? If there is a problem, I don't want to get into coding a huge change in my program before being sure about it.

I am sure it will help Arduino by dividing serial communication into smaller sub-problems and run other programs in the meanwhile. But I am still not sure what would happen when an interrupt is received.

acarb:
To implement LCD library the way you suggested, I will need a queue for any operation such as drawing line, erasing screen etc. To be able to send 1 byte and check for a reply byte in loop(), I will use 2 separate queues (number of bytes to send and number of bytes to receive) since they differ from operation to operation. I will also use a send buffer and a receive buffer.

Well now you've lost me. It just seems far to complicated for what you are appearing to try to do. Have you heard of sprintf? That huge function could be simplified considerably.

You don't have to "send 1 byte and check for a reply byte" surely? Why? Incoming serial data is buffered. Just send them all, and then grab any incoming bytes (in the way I illustrated). When the correct number have arrived process them.

Yes, I know. That was the way it was implemented in the original SMARTGPU library.

If so, I don't need a queue for bytes to send, they are already buffered. The only thing I have added to your example was a queue. I need to know the meaning of incoming bytes. For example, SMARTGPU sends 4 bytes when I ask if there was a touch on the screen. (e.g. 'H' 'O' 'M' 'E' means there is a touch on HOME icon, 0x00100100 means there is a touch on (X,Y)=(0x0010,0x0100) , 'N' 'O' 'N' 'E' means there is no touch detected). On the other hand, I can ask for a memory read between (X1,Y1) and (x2,Y2) (which indicate a rectangular area) and SMARTGPU would send 2 bytes for each pixel that I asked starting from (X1,Y1) to (X2,Y2). So I need to keep track of type of incoming data.

Well, It turns out that I really needed a queue for both incoming and outgoing bytes. SMART GPU may get stuck, or may get confused and display unintended frames when a second command is sent before a reply is received for the first command.

I have completed this SMART GPU library. It works fine so far thanks to your help. I can share it on Playground after doing some finishing touches.