Go Down

Topic: 2 Interrupts, 2 Pins, Simultaneous usage (Read 3928 times) previous topic - next topic

PaulS

If you want to measure the time between interrupts, you need to record when they occur. If you don't, then there is no reason to disable/enable interrupts.

Quote
Could I then use millis() to calculate the time by noting when A is triggered and the again with B?

Try reading mromani's code again.

no1010

First:
Thanks for all the help so far.
I had a really weird problem, I was using an Arduino Nano, plugged into a solderless breadboard.
I was getting these results:
A: 640647 | B: 640656 | A-B: 9
A: 640666 | B: 640676 | A-B: 10
A: 640687 | B: 640676 | A-B: 4294967285
A: 640706 | B: 640696 | A-B: 4294967286
A: 640727 | B: 640716 | A-B: 4294967285
...
A and B just kept on being triggered.
I tried literally everything I could think of.
I then plugged everything into my UNO to see if I could replicate the result, and it dissapeared.
I now get reliable values everytime!
Thank you very much for all the help that you guys offered!
Sorry for my lame coding. I'm very rusted and hardly ever used interrupts before.

A: 135624 B: 136090 A-B: 466
A: 136478 B: 136940 A-B: 462
A: 137410 B: 138824 A-B: 1414
A: 139336 B: 140298 A-B: 962
A: 140694 B: 141208 A-B: 514
A: 141524 B: 141940 A-B: 416
And this is the code:
Code: [Select]

volatile unsigned long fall_time;
volatile unsigned long rise_time;
volatile boolean fall_flag = false;
volatile boolean rise_flag = false;

void fall_isr()
  {
    fall_flag = true;
    fall_time = millis();
  }

void rise_isr()
  {
    rise_flag = true;
    rise_time = millis();
  }

void setup()
  {
    Serial.begin(115200);
    attachInterrupt(0, fall_isr, FALLING);       //A getting pulled low on pin 2
    attachInterrupt(1, rise_isr, RISING);      //B rising on pin 3
  }

void loop()
{
    if (fall_flag && rise_flag)
    {
        fall_flag, rise_flag = false;
        Serial.print(" A: ");
        Serial.print(fall_time);
        Serial.print(" B: ");
        Serial.print(rise_time);
        Serial.print(" A-B: ");
        Serial.println(rise_time - fall_time);
    }
}

Any further suggestions?

mromani

#17
Jan 09, 2012, 05:23 pm Last Edit: Jan 09, 2012, 05:40 pm by mromani Reason: 1
As PaulS wrote, that code might behave weridly if an interrupt occurs during the execution of the code block inside the IF statement... For example if the signal falls after print(fall_time) but before println(rise-time-fall_time), then you'd have a fall time which is greater than rise-time! Worst, the interrupt could happen in the middle of the access to the rise_time or fall_time variable by the print function, which would result in print/println reading e.g. one byte as it was before the interrupt and another one belonging to the _new_ time value. A disaster.
All of this won't happen if the signal timing guarantees nothing would happen during the execution of the if block. A better solution would be this (I'm writing in pseudocode what PaulS said before):

unsigned long aux_fall_time;
unsigned long aux_rise_time;

- disable_interrupts()
- aux_fall_time = fall_time
- aux_rise_time = rise_time
- enable interrupts()
- print/ln as before, using aux_ variables instead of the other ones.

Also, I'd put a delay() as the last statement of the loop() function, to bring the ratio between int_disabled_time and int_enabled_time near zero.

HTH

(disclaimer: I'm a bit in a hurry, so errors might have slipped through the keyboard :-)

no1010

Thanks for all the help so far. I am getting readings that are usable, but I still have a few problems.
Note: mromani - I think this is what you predicted would happen. I tried some rudimentry solutions, but none seem to work.

So this is the output I am getting: (formatted so I can directly copy to CSV for graphing) First value is time A is triggered and second is B. Third is B-A.
Code: [Select]

43332872;43341556;8684
43491812;43500484;8672
43571028;43578624;7596
43650740;43659416;8676
43809688;43737548;4294895156
43888876;43896480;7604
43968632;43977320;8688
44047912;44055512;7600
44206928;44214520;7592
44286664;44295340;8676
44365924;44373524;7600
44524984;44454412;4294896724
44604732;44613420;8688
44683952;44691552;7600
44763720;44772404;8684
44922684;44931360;8676
45001916;45009508;7592
45081660;45090344;8684
45240748;45168552;4294895100
45320012;45327604;7592
45399828;45408520;8692
45479184;45486792;7608
45638404;45646008;7604
45718264;45726964;8700
45797584;45805188;7604
45956676;45886056;4294896676
46036456;46045132;8676
46115724;46123320;7596
46195496;46204180;8684
46354552;46363236;8684
46433792;46441388;7596
46513532;46522204;8672
46592788;46600384;7596
46751820;46759416;7596


Notice the "4294896724" values... they happen when B is triggered before A... which whould be impossible since the turbine is rotating in a circle and A will always be triggered first, no matter what direction it is rotating in.
Thus I can only conclude that A is skipped some how, maybe because B interrupts it... or and the more likely reason (I think), B's value is saved first and then A.
So this has to be a coding problem, but I'm not sure where to look.
The weird thing is the pattern that is created by the readout.

Here is my code, directly from Arduino 1.0
I took al the above posts into consideration and tried to create a "safe" time zone during which the values can be stored.
Quote

volatile unsigned long A_time;
volatile unsigned long A_time_set;
volatile unsigned long B_time;
volatile unsigned long B_time_set;
volatile boolean A_flag = false;
volatile boolean B_flag = false;

void A_ISR()
  {
    A_flag = true;
    A_time = micros();
  }

void B_ISR()
  {
          B_flag = true;
          B_time = micros();
  }

void setup()
  {
    Serial.begin(115200);
    Serial.println("Testing:");
    attachInterrupt(0, A_ISR, FALLING);       //A getting pulled low on pin 2
    attachInterrupt(1, B_ISR, RISING);      //B rising on pin 3
  }

void loop()
{
    noInterrupts();
      A_time_set = A_time;
      B_time_set = B_time;
    interrupts();
    
    if (A_flag && B_flag)
    {
        A_flag, B_flag = false;
        Serial.print(A_time_set);
        Serial.print(";");
        Serial.print(B_time_set);
        Serial.print(";");
        Serial.println(B_time_set - A_time_set
    }
    delay(100);
}


Nick Gammon

Quote
Code: [Select]
void loop()
{
    noInterrupts();
      A_time_set = A_time;
      B_time_set = B_time;
    interrupts();
   
    if (A_flag && B_flag)
    {
        A_flag, B_flag = false;
        Serial.print(A_time_set);
        Serial.print(";");
        Serial.print(B_time_set);
        Serial.print(";");
        Serial.println(B_time_set - A_time_set
    }
    delay(100);
}


You only really need to "carefully" get the time if you are planning to use it, so how about:

Code: [Select]
void loop()
{
   
    if (A_flag && B_flag)
    {
        noInterrupts();
        A_time_set = A_time;
        B_time_set = B_time;
        interrupts();

        A_flag, B_flag = false;
        Serial.print(A_time_set);
        Serial.print(";");
        Serial.print(B_time_set);
        Serial.print(";");
        Serial.println(B_time_set - A_time_set
    }
    delay(100);
}


This doesn't do what you think:

Code: [Select]
        A_flag, B_flag = false;

Try:

Code: [Select]
        A_flag = B_flag = false;

I would also put that at the end of that block.

A_time_set  and B_time_set don't need to be volatile, the ISR isn't changing them.

I'm not sure what the delay achieves, maybe you want time to read the serial output.
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

Nick Gammon


they happen when B is triggered before A... which whould be impossible since...


I'm not sure about that. Interrupts are remembered (queued) so it is possible that you would process an interrupt for A but not B yet. Then you get an interrupt for B and subsequently A. So it looks like B came before A.

Anyway, this probably didn't help:

Code: [Select]
        A_flag, B_flag = false;
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

kf2qd

You are wanting to see if the interrupts were triggered in the routine inside loop(), so don't worry about starting and stopping interrupts there. The interrupts will "interrupt" the main loop because of the way you have configures the pins and you don't need to do much else. loop() will continue to do its thing and you won't have to do anything other than read the values and print them. Realize that the 2 interrupts will be doing their thing may times a second so that number will be updating very often. As you don't know where you are in the update process, stopping interupts could happen between A & B which could mess up the info, before A/after B which would work out OK.

Interrupts are not part of your main program sequence and almost seem invisible to it.

no1010

Once again, thank you for all the help, especially mromani and paulS.


I'm not sure what the delay achieves, maybe you want time to read the serial output.

Thats exactly why it's there, I'm just testing for the moment. If I take it away, there is no way I could read that fast.

I have moved and chaged this:
A_flag = B_flag = false;

A_time_set  and B_time_set don't need to be volatile, the ISR isn't changing them.

To be honest, I not exactly sure where to use volatile, unsigned etc... just going with the flow. I have not yet found a cheat sheet I like that I can refer to.
Removed it.

To deal with A being bigger than B, I put in some (a bit clumsy, alternatives are welcome) code that checks If A is smaller than B and if not, waits a bit before checking again.

These are the results:
Code: [Select]

1199228;216032;16804
273232;295968;22736
356684;373508;16824
514172;530980;16808
588208;610932;22724
671596;688392;16796
829040;845852;16812
903092;925812;22720
986480;1003276;16796
1143852;1160636;16784
1217820;1240544;22724
1301236;1318040;16804
1458620;1475408;16788
1532576;1555280;22704
1615904;1632696;16792
1773228;1790012;16784
1847200;1869908;22708
1930532;1947328;16796
2087892;2104680;16788
2161848;2184552;22704

Nice smooth output.
In graph form:

The weird thing I'm noticing is that it makes these lovely jumps, always to similar values. [note]I am using a spinning motor, instead of blowing, at a set speed to trigger the sensors to keep it constant.
Blue is clockwise rotation and green is counterclockwise.
Another weird observation - the motor is powered from a bench PSU, set at .4V and rotating at a slowish speed. The clockwise rotation inverts the graph a couple of times, while the counter clockwise rotation is constant. Could this be the motor that has more friction in one direction - or the location of the sensors? I actually thought clockwise would be the most accurate direction.
What can cause the consistant double low reading in both directions - why not a continuos seesaw?
Is the gap (1000-10000 microseconds) big enough to accurately determine direction? Why such a big, consistant difference? Arduino's clockspeed?

Oh and my code:
Code: [Select]

volatile unsigned long A_time;
unsigned long A_time_set;
volatile unsigned long B_time;
unsigned long B_time_set;
volatile boolean A_flag = false;
volatile boolean B_flag = false;

void A_ISR()
  {
    A_flag = true;
    A_time = micros();
  }

void B_ISR()
  {
          B_flag = true;
          B_time = micros();
  }

void setup()
  {
    Serial.begin(115200);
    Serial.println("Testing:");
    attachInterrupt(0, A_ISR, FALLING);    //A getting pulled low on pin 2
    attachInterrupt(1, B_ISR, RISING);     //B rising on pin 3
  }

void loop()
{
    noInterrupts();
     
      start:                //Test if A is smaller than B, otherwise, wait a bit.
      if (A_time > B_time)
        {
          goto wait;        //wait if true
        }
      else
{
          A_time_set = A_time;
          B_time_set = B_time;
        }
    interrupts();
      goto cont;           //skips the wait routine
   
      wait:
      interrupts();        //Start interrupts again, otherwise the Arduino crashes since it keeps on looping
        delay(10);
       goto start;
      cont:         
   
    if (A_flag && B_flag)
    {
        Serial.print(A_time_set);
        Serial.print(";");
        Serial.print(B_time_set);
        Serial.print(";");
        Serial.println(B_time_set - A_time_set);
        A_flag = B_flag = false;
    }
    delay(100);
}

PaulS

#23
Jan 10, 2012, 12:31 pm Last Edit: Jan 10, 2012, 12:33 pm by PaulS Reason: 1
You took some relatively straight-forward code, and made a spaghetti pile out of it. Get rid of the gotos. All of them. Every single one of them. None are needed.

If you get the interrupts in the wrong order, just ignore that pair. The value of A_time and of B_time is of no interest until both interrupts have occurred. So, you should only be doing something if they have.
Code: [Select]
void loop()
{
   if (A_flag && B_flag)
   {
      noInterrupts();
      A_time_set = A_time;
      B_time_set = B_time;
      interrupts();

      if(A_time < B_time)
      {
         // Use this pair
      }

      Serial.print(A_time_set);
      Serial.print(";");
      Serial.print(B_time_set);
      Serial.print(";");
      Serial.println(B_time_set - A_time_set);
      A_flag = B_flag = false;
  }
}

No delay, either. You can't expect any sort of reasonable output if you ignore what is going on 99% of the time.

mromani

IIRC I was the one to suggest inserting a delay(). At that point in the discussion, it seemed to me that the code for the loop() funcion was long (it had print statements) and almost 100% inside the no_int zone, so I thought that if it would run continuously, then the time when the interrupts were enabled would have been so short that it would have missed many interrupts.
With the current version of the sketch, it's probably useless... worse, it's probably harmful. So I suggest you take PaulS word and drop it. Or at least try the sketch without the delay() and see how it goes.
(sorry for my english.. ;-)

About the "volatile" keyword: please slow down and take some time to do you homework. Here on the arduino website there is a very clear explanation of its meaning and of when to use it. Interrupt-based programs are the trickiest to write, and trial-and-error is only going to bring you heisenbugs, unless you fully understand what's going on.

:)

PaulS

Quote
and trial-and-error is only going to bring you heisenbugs, unless you fully understand what's going on.

Heisebbugs? There is nothing uncertain about them!

mromani

http://en.wikipedia.org/wiki/Heisenbug

"Heisenbug is a whimsical computer programming jargon term for a software bug that seems to disappear when one attempts to study it.[1] The term is a pun on the name of Werner Heisenberg, the physicist who is commonly associated to the observer effect of quantum mechanics; which states that the act of observing a system inevitably alters its state."

For example, you think you found the cause of an interrupt-related bug (or you think a badly written isr is the cause of the bug): you modify the code and it seems to work. But then your input signal timings change a little and another bug shows up. And so on.

You seem to have a very good understanding of al the bits and pieces that make up an avr micro (certainly better than mine), so for you it's probably all quite clear. Anyway, that was just a ;-)

PaulS

I was thinking in terms of Heisenburg's Uncertainty Principle, that says that you can never really know the state of anything. Well, one thing you can know with the code in this thread is that there are bugs.

mromani

You were thinking well :)

The term heisenbugs is IMHO more "suited" to normal (i.e. desktop software development) debugging procedures: you observe the bug when you run the program normally, but when you run it in debug mode, and either step through or use breakpoints, you cannot reproduce it, because it depends on things happening with certain timings.

no1010

Listen, I'm sorry if I do not know everything there is to know about programming in C. I acknowledged from the beginning that I am apt to make mistakes and probably will make some of you laugh, but I'm here to learn and appreciate the help. I'm on a bit of a tight schedule, that is why I'm skipping ahead and just jumping in with the interrupts, it's the best way for me to learn and has always worked, that way stuff sticks and I will actually remember it.

Thanks for all your patience so far. I have made great progress (on my scale) thanks to the guidance from everyone chipping in.

Lastly, this is test code. the goto function is my quick way to see what works and what does not.
It did the trick, but paulS's code completely did it - now I have nice square waves, although a bit out of phase.
The delay is there so I can read it. I'm not a cyborg, and thus I need a few milliseconds to actually see and confirm the values.
When I take away the delay it does indeed make a difference, it increases the frequency of the graph.
I will definitely take it and all the Serial.prints out in the final code - this is my way of debugging, since I'm still learning.

Below is a graph drawn with the values: Can someone please explain why I am seeing a square wave? I suspect this is due to the micro timer being multiples of four, or the 16Mhz clock of the Atmega.
Green is clockwise, and blue counterclockwise.


And the code - Is it minimalist enought yet?
Code: [Select]

volatile unsigned long A_time;
unsigned long A_time_set;
volatile unsigned long B_time;
unsigned long B_time_set;
volatile boolean A_flag = false;
volatile boolean B_flag = false;

void A_ISR()
  {
    A_flag = true;
    A_time = micros();
  }

void B_ISR()
  {
          B_flag = true;
          B_time = micros();
  }

void setup()
  {
    Serial.begin(115200);
    Serial.println("Testing:");
    attachInterrupt(0, A_ISR, FALLING);    //A getting pulled low on pin 2
    attachInterrupt(1, B_ISR, RISING);     //B rising on pin 3
  }

void loop()
{
    noInterrupts();
      if (A_time < B_time)
        {
          A_time_set = A_time;
          B_time_set = B_time;
        }
    interrupts();       
   
    if (A_flag && B_flag)
    {
        Serial.println(B_time_set - A_time_set);
        A_flag = B_flag = false;
    }
    delay(100);
}

Go Up