2 Interrupts, 2 Pins, Simultaneous usage

no1010:
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:

        A_flag, B_flag = false;

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.

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

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;

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:

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:

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);
}

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.

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.

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.. :wink:

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.

:slight_smile:

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!

"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 :wink:

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.

You were thinking well :slight_smile:

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.

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?

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);
}

mromani: I got an email with a post by you asking if I took the delay away, but now that post is gone...
Answer is below anyway:
Yes, it increases the frequency of the square wave, but nothing else.
(Motor was running at different speed)
As seen below(Orange):

That delay could be make you loose some interrupts (I don't think two interrupts on the same pin can be queued), depending on the actual timing of the signals that come from the motor.
I would try to comment out the delay() statement, and perhaps insert a counter to stop the sketch after, say, 100 iterations. Then you can save the serial monitor buffer to a file, IIRC.

As stated above, I took out the delay. It only increases the frequency of the square wave.
The orange graph is the sketch running with no delay.
I just unchecked the autoscrolling box and copied it directly to Numbers.

I wrote "have you tried without delay" but then removed it.

You are still disabling interrupts, copying data, and enabling interrupts, even if no interrupts have occurred. I'm curious why.

What are you plotting? The vertical axis seems to be time between interrupts. The horizontal axis is? Time?

PaulS:
You are still disabling interrupts, copying data, and enabling interrupts, even if no interrupts have occurred. I'm curious why.

The interrupts are triggered continuously (I just tried it without disabling the interrupts and it makes no noticable difference (Have not compared it properly yet though))- I'm following mromani's advice:

mromani:
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):

PaulS:
What are you plotting? The vertical axis seems to be time between interrupts. The horizontal axis is? Time?

Remember in the beginning of the post I printed 3 values, The time when A and B is triggered and B-A. The values you see now are B-A in microseconds. Thus time yes.
[EDIT] - Sorry, misread that. Horizontal is time. Vertical is the time calculated by subtracting A from B. The highs for the blue line are times roughly at 10 000 microseconds and the lows are times at roughly 7000 microseconds.

My big question is why a couple of interrupts occur at say 10000 micro seconds and then they drop to 6000 microseconds before jumping back up to 10000 again. The values on the left should give you an idea of what is going on. Lefthand column is the blue graph.

I just checked - there is 4microseconds difference between the noInterrupts() version and the one without it.

My last suggestion (I'm running out of ideas, sorry) is to drop Serial.println altogether, and use pin toggling as a debugging aid (plus, of course, an oscilloscope :wink:

By the way, I forgot what you're trying to achieve :slight_smile:

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;
unsigned long error_count = 0UL ;

void loop()
{
  // ONLY care about a change in values if BOTH
  // interrupts have fired.
  if( A_flag && B_flag )
    {
      // Interupts must be disabled because of volatile variable state changes
      noInterrupts() ;
      A_time_set = A_time;
      B_time_set = B_time;

      // In order to avoid race condition, this MUST ONLY be reset with
      // interrupts disabled. Its volatile and the ISR is asynchronous, remember?
      A_flag = B_flag = false ;
      interrupts();

      // There is no need to do this with interrupts disabled
      if( A_time_set < B_time_set )
	{
	  // You have a good sample - now what?
	}
      else
	{
	  error_count += 1UL ;
	}

    }

  // Do debugging output here
  // Don't forget to report our error count
  // Delay for visual debugging
  // though you'd would probably be better served by not using delay as
  // the Serial.xxxx is already going to impose a high delay.
  delay(100) ;
}

ISRs were left out. You need to put them back in. By not protecting two, non-atomic state changes, you created the potential for a race condition. I can't say for sure if that's the nature of the issue, but the code above is what you want. Notice checking A and B can be safely done because the state will not toggle out from under us. We only ever care when they are both true and that's what this does.

no1010:
I have to measure the time between two interrupts, one on the falling edge, and the other on the rising edge.

Now that we have reached page 3 of this thread, I have to ask why do you need to do this? To get the speed of the turbine?

Can't you do that with one sensor?

If you must use two, can't you just get the difference in the ISR? If A always happens before B, something like:

volatile unsigned long fireTime;
volatile long difference;
volatile boolean flag;

void A_ISR()
  {
   fireTime = micros();
  }

void B_ISR()
  {
  difference = micros () - fireTime;
  flag = true;
  }

Now all you have to do is detect the flag set, grab the difference, clear the flag, and repeat. If you want to make sure you grab the difference before it changes, do this:

volatile unsigned long fireTime;
volatile long difference;
volatile boolean flag;

void A_ISR()
  {
  if (!flag)
     fireTime = micros();
  }

void B_ISR()
  {
  if (!flag)
    {
    difference = micros () - fireTime;
    flag = true;
    }
  }

You still might have to discard any negative differences, in case interrupts were enabled after A but before B passed the sensor.