PinChangeInt library- To attach interrupts to multiple Arduino (Uno/Mega) pins

Jante,
No idea what was causing the peaks, why don't you try the code here including my new servo library -

I could use a friendly tester.

I would suggest testing outputs instead of the measured signal - thats the hard part getting highly consistent output pulses - the new servo library helps with this.

As this is now off topic I will start a new topic on the library in a day or two when I have a chance to document it a little more, but Jante, you should be able to find your way around it and have a go at testing in the meantime - if you can find the time.

Duane B

@GreyGnome
did you have time to look at my remark in #28 regarding your optimization proposal of the boolean expression.

there is a diff in the formulas when rising = 0, falling = 1 and curr = 0

robtillaart:
@GreyGnome
did you have time to look at my remark in #28 regarding your optimization proposal of the boolean expression.

there is a diff in the formulas when rising = 0, falling = 1 and curr = 0

Yes- sorry. I have been a busy busy busy little beaver! Geez, what a week!

You're right- my optimization was messed up because I made a mistake in my truth table. I think your expression is about the best that we can do. Second- there are more dragons lying about than I had bargained for! I spent the better part of a week's free time wrestling with a big bug in my posted code; I said: PCintPort::curr ^ lastPinView & (portRisingPins... and I should have said: (PCintPort::curr ^ lastPinView) & (portRisingPins...

...Operator precedence. It's not just a good idea, it's the law! OMG what a debugging session I had.

The extra nice thing is, because of my debugging, I uncovered a significant flaw in my code! This is a big deal if you have fast interrupts, and it is real nasty when you have, for example, switches that bounce (don't they all?). The end of the PCint() function should look like this:

   #ifndef DISABLE_PCINT_MULTI_SERVICE
        pcifr = PCIFR & PCICRbit;
        PCIFR = pcifr;  // clear the interrupt if we will process it (no effect if bit is zero)
        PCintPort::curr=portInputReg; // BUG! ...Fixed in 2.11beta. Without this, we get really weird interrupt behavior. Sheesh.
                                      // This fix slows down the code, though. Even with only 1 pin interrupting.
    } while(pcifr);
    #endif

...don't worry about the speed slowdown mentioned in the comment. With your fix, robtillaart, my tests speed up by a couple of micros. Sadly, that PCintPort::curr=portInputReg will be run even if pcifr is 0. But it's either that or an if statement, in any event. If you don't need it, best to define DISABLE_PCINT_MULTI_SERVICE.

I hope to post version 2.11beta soon. Right now I think I've uncovered/fixed/optimised/edited/compiled everything necessary, but the code is in a bit of a shambles right now so give me a day or two to clean up. In the meantime, there is that damn bug that you can fix in v. 2.01beta by copying from my snippet above.

Hi,
There are two current forum topics where pinchangeint is being used to read incoming rc signals. In both topics, the ops would like to detach pin change interrupts and later reattach them to enter and exit autonomous and RC Modes. I have suggested that this is not the best approach due to the memory leak in detach. As an alternative, would you consider adding an enable function that will set or clear the relevant bit for given pin in the pin change port masks allowing the user to disable pin changes for one more pins and reenable them later without memory leaks.

Duane B

DuaneB:
Hi,
There are two current forum topics where pinchangeint is being used to read incoming rc signals. In both topics, the ops would like to detach pin change interrupts and later reattach them to enter and exit autonomous and RC Modes. I have suggested that this is not the best approach due to the memory leak in detach. As an alternative, would you consider adding an enable function that will set or clear the relevant bit for given pin in the pin change port masks allowing the user to disable pin changes for one more pins and reenable them later without memory leaks.

Duane B

Memory leaks! Yikes! Could you tell me more about that issue? On the face of it I don't mind such a "pause" function, but moreso I am not comfortable with such a bug. It should be fixed.

Hi,

I assume its a memory leak as my understanding is that delete does not free memory in Arduino and the current implementation of detach seems to assume that delete works.

Duane B

DuaneB:
I assume its a memory leak as my understanding is that delete does not free memory in Arduino and the current implementation of detach seems to assume that delete works.

Yes, I ran a test and noticed that my memory got gobbled up pretty quickly. Drat!!! ...The problem is not merely in delete(), because delete just calls free(). The problem is that free() doesn't seem to work.

Anyway, regardless of that I like your idea of having detachInterrupt() simply disable the pin from interrupting, rather than using the delete() operator. I am working on it doing just that.

attachInterrupt() will return -1 on error, 0 if the pin has already been added to the list (the pin will be enabled if it is called and had previously been created already), and 1 if a new pin is created.

Perfect, I will post a link to this topic from the two related threads so that the ops are aware of this and can follow the progress.

Thanks

Duane B

DuaneB:
In both topics, the ops would like to detach pin change interrupts and later reattach them to enter and exit autonomous and RC Modes.

Is that done in the context of the interrupt handler?


PinChangeInt
---- RELEASE NOTES ---


Version 2.13 (beta) Mon Nov 12 09:33:06 CST 2012
SIGNIFICANT BUGFIX release! Significant changes:

  1. PCintPort::curr bug. Interrupts that occur rapidly will likely not get serviced properly by PCint().
  2. PCint() interrupt handler optimization.
  3. PCIFR port bit set bug fix.
  4. Many static variables added for debugging; used only when #define PINMODE is on.
  5. detachInterrupt() no longer does a delete(), since that wasn't working anyway. When you detachInterrupt(), the PORT just disables interrupts for that pin; the PCintPin object remains in memory and in the linked list of pins (possibly slowing down your interrupts a couple of micros). You can reenable a detached interrupt- but you must do it within the PinChangeInt library (would anyone ever enable an interrupt on a pin, then disable it, then have need to reenable it but not using the library?).
  6. attachInterrupt() now returns a uint8_t value: 1 on successful attach, 0 on successful attach but using an already-enabled pin, and -1 if the new() operator failed to create a PCintPin object.
    Also, modified these release notes.

Details:

Uncovered a nasty bug, thanks to robtillaart on the Arduino Forums and Andre' Franken who posted to the PinChangeInt groups. This bug was introduced by me when I assigned PCintPort::curr early in the interrupt handler:

ISR(PCINT0_vect) {
    PCintPort::curr = portB.portInputReg;
    portB.PCint();
}

Later, in the interrupt handler PCint(), we loop as long as PCIFR indicates a new interrupt wants to be triggered, provided DISABLE_PCINT_MULTI_SERVICE is not defined (it is not by default):

#ifndef DISABLE_PCINT_MULTI_SERVICE
        pcifr = PCIFR & PCICRbit;
        PCIFR = pcifr;  // clear the interrupt if we will process it (no effect if bit is zero)
} while(pcifr);
#endif

...Well. Problem is, if a pin pops up and causes the PCIFR to change, we have to reread the port and look at how it is now! I wasn't doing that before, so if a new interrupt appeared while I was still servicing the old one, odd behavior would take place. For example, an interrupt would register but then the userFunc would not be called upon to service it. The code needs to be:

        pcifr = PCIFR & PCICRbit;
        PCIFR = pcifr;  // clear the interrupt if we will process it (no effect if bit is zero)
        PCintPort::curr=portInputReg; // ...Fixed in 2.11beta.
} while(pcifr);

Also, made the interrupt handler even faster with an optimization from robtillaart to take out the checks for changed pins from the while() loop that steps through the pins:

uint8_t changedPins = (PCintPort::curr ^ lastPinView) &
                      ((portRisingPins & PCintPort::curr ) | ( portFallingPins & ~PCintPort::curr ));

...This speedup is offset by more changes in the PCint() handler, which now looks like the following; there are two bug fixes:

FIX 1: sbi(PCIFR, PCICRbit);
FIX 2: ...the aforementioned PCintPort::curr=portInputReg;
Here's the new code:

    #ifndef DISABLE_PCINT_MULTI_SERVICE
        pcifr = PCIFR & PCICRbit;
        if (pcifr) {
        //if (PCIFR & PCICRbit) { // believe it or not, this adds .6 micros
            sbi(PCIFR, PCICRbit); // This was a BUG: PCIFR = pcifr  ...And here is the fix.
            #ifdef PINMODE
            PCintPort::pcint_multi++;
            if (PCIFR & PCICRbit) PCintPort::PCIFRbug=1; // PCIFR & PCICRbit should ALWAYS be 0 here!
            #endif
            PCintPort::curr=portInputReg; // ...Fixed in 2.11beta.
            goto loop;  // A goto!!! Don't want to look at the portInputReg gratuitously, so the while() will not do.
        }
    #endif

Also I added a lot of variables for debugging when PINMODE is defined, for routing out nasty bugs. I may need them in the future... :frowning:

Finally, I am not putting newlines in this commentary so I can make it easier to paste online.

BTW, I am still testing this release but I wanted it to get out there because I uncovered some nasty bugs. They made my interrupts unreliable! So it's important if you use this code, to realize that the new version should work better.

I realized that about 5-10% of my interrupts were missed due to my switch bounce; some of the bounces happen only 50 microseconds apart or so. And my code was not processing them properly. With this code, I notice that my switches are more reliable.

I also notice that I am not getting the speed bump that I hoped to get, and some of my bugfixes actually introduce a few extra microseconds of processing. This is disappointing and I hope to find them if I can.

I already thought of a bug in v. 2.13beta. If you detach an interrupt and attach it again, and give it a new user function, it will not attach the new user function- it will use the first user function that you attached.

However you are able to change the mode at this point (RISING, FALLING, or CHANGE).

Version 2.15beta is out now, and I allow for attaching a new user function. I don't know if anyone would really do that, but there should be no nasty surprises in the code, and attachInterrupt() will thus do exactly what you tell it to do in the argument list.

URL: Google Code Archive - Long-term storage for Google Code Project Hosting.

  1. goto loop: ? why not use break?

addpcint

	if (firstPin != NULL) {
		tmp=firstPin;
		if (tmp->arduinoPin == arduinoPin) { enable(tmp, userFunc, mode); return(0); }
		while (tmp->next != NULL) {
			tmp=tmp->next;
			if (tmp->arduinoPin == arduinoPin)  { enable(tmp, userFunc, mode); return(0); }
		};
	}

shorter == smaller footprint? faster?

	if (firstPin != NULL) {
		tmp=firstPin;
                do {
        		if (tmp->arduinoPin == arduinoPin) { enable(tmp, userFunc, mode); return(0);
                        tmp=tmp->next;
                } while (tmp != NULL);
	}

Thanks for the input. This saved 12 bytes; I haven't measured the speed:

    if (firstPin != NULL) {
        tmp=firstPin;
        do {
            if (tmp->arduinoPin == arduinoPin) { enable(tmp, userFunc, mode); return(0); }
            if (tmp->next == NULL) break;
            tmp=tmp->next;
        } while (true);
    }

I have also converted the goto to a do/while/break situation. No change to interrupt speed, but I guess it looks better :-).

                } while (tmp != NULL);

This won't work because tmp will become null, which will exit us from the loop.

I need tmp->next later in the code, therefore I need the last non-null value of tmp.

oops,
I figured that out too and you replied while I removed my previous post,

Version 2.19beta has been released. The library now includes Sanguino support. There have been a number of recent bugfixes, so be sure to look at the Release Notes. If you have any library prior to version 2.17beta, it is highly recommended that you upgrade to 2.17beta or later.

hey,

I like to use the PINCHANGE Library with the Arduino DUE.
But I still get this failure:

In file included from FirstTask.ino:24:
.....\Arduino\libraries\PinChangeInt/PinChangeInt.h:103: fatal error: new.h: No such file or directory
compilation terminated.

Maybe some know this failure?

I need to use the pinchange lib because I need to know of many pins, which pin has changed without using for every pin a own function like this:

     attachInterrupt(1, action1, HIGH);
attachInterrupt(2, action2, HIGH);

Maybe someone knows also an alternative solution :slight_smile:

(very little Due experience)
you could try to uncomment it , and see if it is needed for the DUE
The DUe is an ARM processor while the lib was originally written for the AVR.

just a thought!