I have a PPS 5v coming from a GPS receiver that connects to Interrupt 0 on my Arduino Atmega368. Seems as the interrupt routine might be running twice, which is triggered on the rising edge of the PPS.
int pinout = 13;
unsigned long time = 0;
int pulseCount = 0;
boolean ppsStart = false;
void setup()
{
pinMode(pinout, OUTPUT);
attachInterrupt(0, pulsein, RISING);
}
void loop()
{
if(micros() - time > 2000 && ppsStart == true) //this gives me a 50us pulese every 2ms
{
time = micros();
digitalWrite(pinout, HIGH);
delayMicroseconds(50);
digitalWrite(pinout, LOW);
pulseCount++;
if(pulseCount > 400) //runs 400 times so to notoverlap the next second
{
ppsStart = false;
}
}
}
void pulsein() //interrupt routine and a single 50us pulse
{
time = micros();
digitalWrite(pinout, HIGH);
delayMicroseconds(50);
digitalWrite(pinout, LOW);
ppsStart = true
pulseCount = 1;
}
The idea of my project is to make my chip generate 50us pulses every 2ms on the trigger of a PPS on Interrupt 0. Looks like it works ok other than the first pulse happens twice. Any suggestions?
This might help visualize my problem...
I commented all inside loop as to leave only the interrupt's function, now it works as it should making only one initial pulse. That leaves me with the idea that time = micros() is not working correctly inside of my interrupt function. Any idea?
I haven't fully analysed the code but
void loop()
{
if(micros() - time > 2000 && ppsStart == true) //this gives me a 50us pulese every 2ms
{
time = micros();
[glow]digitalWrite(pinout, HIGH);
delayMicroseconds(50);
digitalWrite(pinout, LOW);[/glow]
pulseCount++;
if(pulseCount > 400) //runs 400 times so to notoverlap the next second
{
ppsStart = false;
}
}
}
void pulsein() //interrupt routine and a single 50us pulse
{
time = micros();
[glow]digitalWrite(pinout, HIGH);
delayMicroseconds(50);
digitalWrite(pinout, LOW);[/glow]
ppsStart = true
pulseCount = 1;
}
You have a pulse in the ISR and also in the main loop. That's the two you are seeing.
Having read further, your problem is that you only get 1 pulse not 400. Is that the case?
I'd ditch all the ISR code for starters
void pulsein()
{
ppsStart = true
}
then look at why the loop isn't working right.
gardner says:
"time" is handled in both the main loop and ISR. It must be "volatile".
DOH! Can't believe I didn't see that 
Rob
unsigned long time = 0;
"time" is handled in both the main loop and ISR. It must be "volatile".
Thank yo so much for going over my code
Graynomad: During troubleshooting I came to the same conclusion to take out the first pulse line as you suggested.
garder: I'm actually having problems dealing with time, how can I fix this, do I have to declare it on the loop only?
Another thing I wonder if someone else can help me with is that I see that the accuracy of my pulses diminishes as i goes on. I decided to use a 10 PPS for my project now, but even then on my 49th pulse I see is off by as much as 300 micro seconds which leads me to believe that either my system (code) to get this done might not be the best idea.
Any suggestions as to how is the best way to make a pulse every 2 ms that is accurate?
My current code...
int pinout = 13;
unsigned long time = 0;
int pulseCount = 0;
boolean ppsStart = false;
boolean timing = false;
void setup()
{
pinMode(pinout, OUTPUT);
pinMode(2, INPUT);
attachInterrupt(0, pulsePPS, RISING);
}
void loop()
{
if(micros() - time > 1996 & ppsStart == true) //this gives me a 50us pulese every 2ms
{
time = micros();
delayMicroseconds(35); //A necesary ofset I need to be able to adjust
digitalWrite(pinout, HIGH);
delayMicroseconds(50); //50us wave
digitalWrite(pinout, LOW);
pulseCount++;
if(pulseCount > 48) //runs 49 times so to notoverlap the next second
{
ppsStart = false;
pulseCount = 0;
}
}
}
void pulsePPS() //interrupt routine and a single 50us pulse
{
ppsStart = true;
}
The volatile is no longer necessary because you aren't accessing "time" in the ISR, but for future reference
volatile unsigned long time = 0;
would do and leave it as a global where it is.
Here's a simplified version of the main loop
if(ppsStart == true) {
ppstart = false;
delayMicroseconds(35); //A necesary ofset I need to be able to adjust
for (int i = 0; i < 50; i++) {
digitalWrite(pinout, HIGH);
delayMicroseconds(50); //50us wave
digitalWrite(pinout, LOW);
delayMicroseconds(1940); // tweak for correct pulse spacing
}
}
Which I think is about right although it's late here and I'm falling asleep at the keyboard
This version however uses delays and so does not allow for the loop to do anything else while the pulses are being generated.
Rob
boolean ppsStart = false;
ppsStart should be volatile since it is set in the ISR and tested in the main loop.
if(micros() - time > 1996 [glow]&[/glow] ppsStart == true) //this gives me a 50us pulese every 2ms
You probably mean && here, not &.
Graynomad
Your code works better than what I had, I tweaked the delay to 1931 and it is right on where I need it to be!
The only thing now is that I can only get it to work with 48 pulses, when I use the 49 pulses, since the loop still delays it at the end it gets interrupted by my PPS interrupt (0) and it goes bananas! So I'm trying to find a way to add that last pulse without a delay on it so that it won't go bananas.
My current code
int pinout = 13;
unsigned long time = 0;
int pulseCount = 0;
boolean ppsStart = false;
boolean timing = false;
void setup()
{
pinMode(pinout, OUTPUT);
pinMode(2, INPUT);
attachInterrupt(0, pulsePPS, RISING);
}
void loop()
{
if(ppsStart == true) {
ppsStart = false;
delayMicroseconds(35); //A necesary ofset I need to be able to adjust
for (int i = 0; i < ; i++) {
digitalWrite(pinout, HIGH);
delayMicroseconds(50); //50us wave
digitalWrite(pinout, LOW);
delayMicroseconds(1931); // tweak for correct pulse spacing
}
}
}
void pulsePPS() //interrupt routine and a single 50us pulse
{
ppsStart = true;
}
for (int i = 0; i < ; i++) {
i less than what? I'm surprised this builds.
I apologize, I made a typo on my copy/paste
int pinout = 13;
unsigned long time = 0;
int pulseCount = 0;
boolean ppsStart = false;
boolean timing = false;
void setup()
{
pinMode(pinout, OUTPUT);
pinMode(2, INPUT);
attachInterrupt(0, pulsePPS, RISING);
}
void loop()
{
if(ppsStart == true) {
ppsStart = false;
delayMicroseconds(35); //A necesary ofset I need to be able to adjust
for (int i = 0; i < 49 ; i++) {
digitalWrite(pinout, HIGH);
delayMicroseconds(50); //50us wave
digitalWrite(pinout, LOW);
delayMicroseconds(1931); // tweak for correct pulse spacing
}
}
}
void pulsePPS() //interrupt routine and a single 50us pulse
{
ppsStart = true;
}
Thank you guys for your help, I finally have a working code that does what I need.
int pinout = 13;
unsigned long time = 0;
int pulseCount = 0;
boolean ppsStart = false;
boolean timing = false;
void setup()
{
pinMode(pinout, OUTPUT);
pinMode(2, INPUT);
attachInterrupt(0, pulsePPS, RISING);
}
void loop()
{
if(ppsStart == true) {
ppsStart = false;
delayMicroseconds(35); //A necesary ofset I need to be able to adjust
for (int i = 0; i < 49 ; i++) {
digitalWrite(pinout, HIGH);
delayMicroseconds(50); //50us wave
digitalWrite(pinout, LOW);
delayMicroseconds(1931); // tweak for correct pulse spacing
}
digitalWrite(pinout, HIGH);
delayMicroseconds(50); //50us wave
digitalWrite(pinout, LOW);
}
}
void pulsePPS() //interrupt routine and a single 50us pulse
{
ppsStart = true;
}
You could do it a little bit simpler
for (int i = 0; i < 49 ; i++) {
digitalWrite(pinout, HIGH);
delayMicroseconds(50); //50us wave
digitalWrite(pinout, LOW);
if (i < 48) delayMicroseconds(1931); // tweak for correct pulse spacing
}
Also you still don't have the volatile on ppsStart.
Rob
Pardon my question as I don't understand that, what is it supposed to be? My understanding about volatile is that is only inside a loop? I'm not sure...
Also, correct me if I'm mistaken, wouldn't another if statement inside of the for loop add greater overhead?
Volatile basically means that "things" may change the variable in a manner that's not obvious when the compiler looks at the code. Such as say a input register that changes state when inputs change. To quote from the Netrino site
C's volatile keyword is a qualifier that is applied to a variable when it is declared. It tells the compiler that the value of the variable may change at any time--without any action being taken by the code the compiler finds nearby.
Frankly I don't know why it should be required in your exact code because the compiler should know what's going on, and as it's working maybe that is in fact the case. Still, it's good practice to use the qualifier for vars shared with ISRs.
One place this will bite you is with optimisation
loop () {
long i;
for (i = 0; i < 100; i++ ) {
i = i;
}
pulse_pin ();
}
In this simple loop I am just trying to output some pulses to see some timing for whatever reason and I want to vary the loop time by changing the 100. But no matter what I change it to, even 1,000,000 the frequency of the pulse stays the same.
The compiler has decided that the loop actually doesn't do anything and has removed it from the run time code. Changing it to
volatile long i;
fixes the problem by causing the compiler to butt out.
wouldn't another if statement inside of the for loop add greater overhead?
Yes but there's plenty of fat because of the delay, so adjust the
delayMicroseconds(1931);
to accomodate the extra code in the loop.
Rob
Thanks for the tips, with your code and eliminating some statements I found how to run my code even smaller and smoother than before.
int pinout = 12;
void setup()
{
pinMode(pinout, OUTPUT);
pinMode(2, INPUT);
attachInterrupt(0, pulsePPS, RISING);
}
void loop(){
}
void pulsePPS() //interrupt routine
{
delayMicroseconds(35);
for (int i = 0; i < 50 ; i++) { //create 49 pulses
digitalWrite(pinout, HIGH);
delayMicroseconds(50); //50us pulse
digitalWrite(pinout, LOW);
if (i < 49) { //If statement removes delay on last pulse
delayMicroseconds(1943); // tweak this for correct pulse spacing
}
}
}
