problem in sending and receiving values serially

hi everyone
I am realy not gud programmer.I am trying to make dc postion control system.i am using vee pro software inwhich i hav implemented PID algorithm so now I want to use arduino as data acquisition device which sends and receives data.i have mixed up two individual sketches which sends and receives data.but it wont work
wht should be the reason?
the code is as follows

#include <avr/io.h>
#include <avr/interrupt.h>

#define BAUD_RATE 115200
#define INPUT_PIN 3
#define LED_PIN 13

//#define TEST_MODE // comment out to read analog pin, uncomment for test ramp wave

volatile int j;
int ledPin=9;
int val;

void setup()
{
pinMode(ledPin,OUTPUT);
Serial.begin(BAUD_RATE);
pinMode(LED_PIN, OUTPUT);
cli(); // disable interrupts while messing with their settings
TCCR1A = 0x00; // clear default timer settings, this kills the millis() funciton
TCCR1B = 0x00;
TCCR1B |= (1 << WGM12); // Configure timer 1 for CTC mode
TCCR1B |= (0 << CS12); // Set timer prescaling by setting 3 bits
TCCR1B |= (1 << CS11); // 001=1, 010=8, 011=64, 101=1024
TCCR1B |= (1 << CS10);
TIMSK1 |= (1 << OCIE1A); // Enable CTC interrupt
OCR1A = 50; // Set CTC compare value
sei(); // turn interrupts back on
}

void loop() {if (Serial.available()) {
// read the most recent byte (which will be from 0 to 255)
val = Serial.read();
// set the brightness of the LED
analogWrite(ledPin, val);
}
// nothing to do, its all in the interrupt handler!
}

ISR(TIMER1_COMPA_vect) // when timer counts down it fires this interrupt
{
#ifdef TEST_MODE
//Serial.print((j%64)*4 , DEC); // test mode, generate a ramp wave
j++;
#else
Serial.println( analogRead(INPUT_PIN), DEC); // real mode, sample analog pin
#endif
}

can anyone having any other idea?
thank you all in advance

but it wont work

How I hate that phrase.

It's not normally considered a good idea to put serial prints inside an interrupt routine.
Best way to debug the interrupt routine is simply, toggling something like a LED or output pin.

I am realy not gud programmer.

Spelling is not so hot either. I am dyslexic myself and find the spell checker invaluable.

Are you sure you are sending byte data and not ASCII data, as you haven't posted that code it is hard to say.
Also to prevent annoying people on this site post any code using the # icon in the reply box.

It also helps if you describe what the nothing is that works, there are two aspects to your sketch, one is receiving and setting the LED the other is the periodic invocation of an interrupt service routine. What have you tested?

hey mike thanx for your precious feedback
As i have mentioned earlier that i am using vee pro software for sending the values to arduino.yes i am sending those values in byte format only
.
the strange thing happened here is if i am not sending value to arduino then this program works well and displays the values read by analog pin3
but whenever i tried to write any byte value execution stops.And no more values been fetched.
i have tried simple program without interrupt but it does not shows the accurate reading as the interrupt one

This might be a long-shot, but you could try placing a delayMicroseconds(150); directly after the analogRead();
As it says in the reference, it can take upto ~100 microseconds for it to read an analog value.

Also, a delayMicroseconds(1); after the Serial.read() might solve your issue, I can't read valid values from the serial line without such a delay in anycase.

Try to avoid, at all costs, using delay() or delayMicroseconds() in order to get Serial.read to work.

The root of the problem is that the Arduino is much faster than a character can be sent from a host computer to the Arduino.
It takes about 1 millisecond for a character to be read by the Arduino (at 9600 baud).
It can be worse, because the 'clever' USB hardware might hold a few characters up, and send them in a group. Worse, the host computer might have got distracted for a few milliseconds.
So, when the Arduino is trying to read a character, one might not have arrived yet.

Using a delay can be very unreliable, because the character still might not have arrived. So, a delay may sometimes fail to fix the problem.

This is a horrible problem to debug, made worse because when you put in Serial.prints to help you see what is happening, it can change the way the program works. So one program may work perfectly, a slightly different one fails, but when you try to find out why, it seems to work or break in a different way, because you have caused something to change by trying to debug it (some programers call this the "Heisenberg effect" because of Heisenberg's uncertainty principle).

The Arduino environment has some example programs in
File->Examples->Communication
for example SerialCallResponse or SerialCallResponseASCII

Arduino examples are well thought out, to make sure everything works well.

When you look at them, you'll see

if (Serial.available() > 0) {
// get incoming byte:
inByte = Serial.read();
...
}

The Serial.available() returns the number of characters that have arrived, and are waiting to be read.

So when Serial.available() returns a number bigger than zero, there is stuff waiting to be Serial.read().

If the program doesn't do a Serial.available() first, and only does a Serial.read(), but there are no characters ready and waiting, Serial.read() will return -1, to say that it doesn't have anything, as explained here Serial.read() - Arduino Reference

If you use the second approach, checking for -1, then notice that all the possible values for any character and the -1 failure value can't fit into a char (otherwise one of the values of a char couldn't be sent, which would be silly and annoying). So the value from Serial.read() must be put into something bigger than a char, and that is an int.

So don't do delays in the hope that a character will arrive. It is usually unreliable, horrible to debug, wastes time unecessarily (and has other problems, but that's enough fud for today :))

Always do a Serial.available() before a Serial.read() for a character (like the Arduino examples), or check the value returned by Serial.read() to make sure it read a character, and it isn't a -1. Hold the value from Serial.read in an int variable, or you'll not be able to tell the difference between the character 0xff and the -1 "nothing to read" value.

I apologise for all of the detail, but, talking between computers is a bit harder to arrange than talking to dumb electronics like an LED.

HTH

@gbulmer

So, when the Arduino is trying to read a character, one might not have arrived yet.

If you look at his code it is only reading one byte when it is available.

@vismay

the strange thing happened here is if i am not sending value to arduino then this program works well

That is exactly the sort of think I mean about describing what doesn't work. This is an anomaly we can get our teeth into and worry. Not that there is an answer but if you review what it is telling us:-

  1. Interrupts are being generated and serviced in a regular fashion.
  2. A serial input disturbs that.
    So we have to tease out why.
    Suggestion, just have a blank loop function but send some bytes does that stop things? That will tell us if the act or receiving a byte is the problem or if it is the act of reading it out of the buffer.
    I can't see an "attach interrupts" statement, could it be that you are hijacking the vector that the interrupt from the serial input uses?

If you look at his code it is only reading one byte when it is available.

:-[

Sorry, I over reacted to the suggestion of using delay or delayMicrosecond to synch communications, and missed the Serial.available() in the otherwise nicely layed out program source.

Apologies

I think I spotted something important.

Timer1 pre-scaler is set to 64, so it counts every 4 uSec
OCR1A is set to 50, and so will compare match every 50*4 = 200 uSec

an analogRead in

ISR(TIMER1_COMPA_vect)                            // when timer counts down it fires this interrupt
{  
 #ifdef TEST_MODE
   //Serial.print((j%64)*4 , DEC);                // test mode, generate a ramp wave
   j++;
 #else
   Serial.println( analogRead(INPUT_PIN), DEC);   // real mode, sample analog pin
 #endif  
}

Takes 100 uSec

The baud rate is 115200, so about 115200/9 bytes/second = 12,800 bytes/second

So, assuming the value being Serial.println's is 3 digits, + 1 for the '\n' that will take 313 uSec

So the ISR takes 413 uSec, but is triggered every 200 uSec.

TIMER1_COMPA_vect interrupt is higher priority than USART, so many (all?) USART interrupts will be lost.

Move the Serial.println( analogRead(INPUT_PIN), DEC); out of the ISR, and just print something in the main loop.

I don't think it is feasible to read an analogue value at full speed, and send over the serial port.

It would be as fast to just Serial.println( analogRead(INPUT_PIN), DEC); in the main loop() without the ISR.

The baud rate is 115200, so about 115200/9 bytes/second = 12,800 bytes/second

sp."The baud rate is 115200, so about 115200/10 bytes/second = 11520 bytes/second"

sp."The baud rate is 115200, so about 115200/10 bytes/second = 11520 bytes/second"

Yes. I was looking for the initialisation code, so I was being very optimistic (I can't find UPM0 USBS0 data frame initialisation in wiring files, so I assume it's in the bootloader).

I felt safe, as that makes it even less likely to get a USART interrupt (assuming the theory is correct).

[edit]Yup, it's in the bootloader; 1 start, 1 stop, no parity, 8-data bits = 10 bits,

For the geeks, "19.10.4 UCSRnC – USART Control and Status Register n C" page 119 of doc8271.pdf[/edit]

I can't find UPM0 USBS0 data frame initialisation in wiring files

Either way, it's difficult to imagine an eight bit data frame with fewer than ten bits. :wink:

As Groove pointed out earlier, just about any kind of serial activity in an interrupt is going to be suspect using the "Serial" object, certainly for more than one character.

Agree and understood.

[edit]I was assuming we'd agreed that, but reading back, I was very unclear. I was looking at the next step, which is trying to make it work[/edit]

I wasn't sure if it was 1 or 2 stops, and 0 or 1 parity (I thought no parity), but you're right, I didn't think the extra few seconds, and say 10 bits/char; at least that has a statistically significant chance of being correct :slight_smile:

One point is, in this case, output rate is slower than 1/2 the speed of the interrupts.

So even using pure binary, with two bytes/ADC value, and no '\n', it is still tight.

I assume the second byte of a two byte transfer will block until the first byte has been sent. So writing two bytes using Serial.print(val) will take 87uSec, with a byte from the following analogue reading able to go at, a theoretical, 174 uSec. So their is 26 uSec to play with using this, and this seems tight to me.

With 'analogueRead', at 13 ADC clocks of 125KHz, that's 104 uSec, leaving 9uSec per cycle for everything else. Very tight (I feel under 5% slack is very hard). Unless the ADC is 'pipelined with sending the last byte of the two byte ADC.

I think I could do it, but it is quite careful, intricate code, and it would lose much of the Arduino library support (I'd try to stick with the core settings of init()) (not a complaint, the framework is beautifully designed for safe, easy and productive use).

Okay?

hi everyone
I have tried debug this sketch in this manner

#include <avr/io.h>
#include <avr/interrupt.h>

#define BAUD_RATE 115200
#define INPUT_PIN 3
#define LED_PIN 13

//#define TEST_MODE // comment out to read analog pin, uncomment for test ramp wave

volatile int j;
int ledPin=9;
int val=255;

void setup()
{
pinMode(ledPin,OUTPUT);
Serial.begin(BAUD_RATE);
pinMode(LED_PIN, OUTPUT);
cli(); // disable interrupts while messing with their settings
TCCR1A = 0x00; // clear default timer settings, this kills the millis() funciton
TCCR1B = 0x00;
TCCR1B |= (1 << WGM12); // Configure timer 1 for CTC mode
TCCR1B |= (0 << CS12); // Set timer prescaling by setting 3 bits
TCCR1B |= (1 << CS11); // 001=1, 010=8, 011=64, 101=1024
TCCR1B |= (1 << CS10);
TIMSK1 |= (1 << OCIE1A); // Enable CTC interrupt
OCR1A = 50; // Set CTC compare value
sei(); // turn interrupts back on
}

void loop() {//if (Serial.available()) {
// read the most recent byte (which will be from 0 to 255)
//val = Serial.read();
// set the brightness of the LED
analogWrite(ledPin, val);
//}
// nothing to do, its all in the interrupt handler!
}

ISR(TIMER1_COMPA_vect) // when timer counts down it fires this interrupt
{
#ifdef TEST_MODE
//Serial.print((j%64)*4 , DEC); // test mode, generate a ramp wave
j++;
#else
Serial.println( analogRead(INPUT_PIN), DEC); // real mode, sample analog pin
#endif
}

according to my knowledge this sketch should give me 5v at pin no 9
right?
but it does not happen.it seems like whatever is inside the loop statement its not been executed.
what should i do?

I am sorry if I dont understand everything, but I think everyone says this does not work

ISR(TIMER1_COMPA_vect)                            // when timer counts down it fires this interrupt
{  
 #ifdef TEST_MODE
   //Serial.print((j%64)*4 , DEC);                // test mode, generate a ramp wave
   j++;
 #else
   [glow]Serial.println( analogRead(INPUT_PIN), DEC);[/glow]   // real mode, sample analog pin
 #endif  
}

The Serial.println( analogRead(INPUT_PIN), DEC);
got to be taken out.

They say it takes too long so it will not work.

Maybe take out ISR, and put that Serial.println( analogRead(INPUT_PIN), DEC);
in loop()?

Maybe not everything you want, but maybe it is something?

I think if you take out the ISR stuff
You can take out
cli(); // disable interrupts while messing with their settings
TCCR1A = 0x00; // clear default timer settings, this kills the millis() funciton
TCCR1B = 0x00;
TCCR1B |= (1 << WGM12); // Configure timer 1 for CTC mode
TCCR1B |= (0 << CS12); // Set timer prescaling by setting 3 bits
TCCR1B |= (1 << CS11); // 001=1, 010=8, 011=64, 101=1024
TCCR1B |= (1 << CS10);
TIMSK1 |= (1 << OCIE1A); // Enable CTC interrupt
OCR1A = 50; // Set CTC compare value
sei(); // turn interrupts back on

Then maybe loop() is just
void loop() {if (Serial.available()) {
// read the most recent byte (which will be from 0 to 255)
val = Serial.read();
// set the brightness of the LED
analogWrite(ledPin, val);
}
Serial.println( analogRead(INPUT_PIN), DEC);
}

hey glerk
sorry for the late reply
yes you are right.i put serial.print in the loop.
program runs now perfectly,but now what will be the requirement of ISR?????????
thanx