I am needing some advice/guidance on triggering state changes with a pin change interrupt on an arduino nano. I have an instrument that outputs a TTL pulse (~10 ms wide) that I need to use a pin change interrupt to trigger data collection. I have the input pulse going through a Schmitt inverter to help clean up the signal (74HC14 and i had some left over inverters so why not). The external interrupts (INT0, INT1) are used for some more critical timing input applications, otherwise this would be easy as I could trigger off of RISING or FALLING as needed. But I have to use a PCINT which is just CHANGE.
What I'd like to happen is when the instrument (pmc in the code) sends the first trigger signal (pulse 1) to D5 (PD5) it sets a flag variable (expFlag) to indicate that the pulse was recived and that data needs to be collected, store the current millis() (pmcTriggerTime), and increment a counter (expCounter). When the next pulse comes in (pulse 2) the flag gets cleared, the counter doesn't increment, and the time doesn't need to be stored. The next pulse (pulse 3) will be same as pulse 1 (set a flag variable, log millis, inc counter), etc. This should be easy enough but i've started over thinking the problem, which is for each pulse you get 2 interrupts (as set up now you get a falling edge, 10 ms low, then rising edge.
I've reduced the code down to a smaller example. As I don't have the instrument handy I'm posting the code as is which does contain variables that aren't needed as well as the output that I recieved. In this example the instrument sent out 1 pulse every 5 seconds for a total of 10 pulses(so the result at the end is that 5 experiments run). Some of the experiments only logged 4 seconds, but I'm assuming that is because the arduino might have been tied up with serial (NOT in the ISR ). The issue I'm over thinking ishow do i set a flag varible every 2 triggers and increment every 2 triggers.
In this example my PCINT plan was:
Increment a counter for each change. This would increase by 2 every pulse as it's triggered on both rising and falling
Log the time
Set a flag (pmcFlag) which is probably not needed
Do a dirrect port read of PD5 to tell if it is low (which happens 1x per pulse)
If it is toggle the expFlag to indicate that this pulse was recieved and increment the counter.
The rest of the code is to print out the variables every second.
It would seem that I just need a modulo somewhere but I can't figure out where. Or is there a better approach?
const byte pmcTriggerOutPin = 5; //Pin for PCM to trigger
unsigned long currentMillis = 0; // stores the value of millis() in each iteration of loop()
const unsigned int printInterval_ms = 1000; //how often to print
unsigned long previousPrintInterval = 0; //when was last print
// PCINT21 Variables
boolean expFlag = false; //should be a volatile but this is what I ran. Flips 1x per pulse
volatile boolean pmcFlag = false; //probably unnecissary
volatile unsigned long pmcTriggerTime; //time that pulse was recieved
volatile unsigned int pmcChangeCount = 0; //how many changes recieved.
unsigned int expCounter = 0; //should be a volatile but this is what I ran. Should become 1 after 1st pulse, 2 after 3rd pulse
void setup() {
pinMode(pmcTriggerOutPin, INPUT);
PCMSK2 |= 0b00100000; //PCINT21 PD5 D5
PCIFR = 0b00000000; //Clear all flags
PCICR |= 0b00000100; //Turn on port D PCIE2
Serial.begin(115200);
}
void loop() {
//pmcTrig();
if (millis() - previousPrintInterval >= printInterval_ms) {
if (expFlag) {
Serial.print("true");
Serial.print("\t");
Serial.print(pmcTriggerTime);
Serial.print("\t");
Serial.print(expCounter);
Serial.println();
} else {
Serial.print("false");
Serial.print("\t");
Serial.print("\t");
Serial.print("\t");
Serial.print(expCounter);
Serial.println();
}
previousPrintInterval += printInterval_ms;
}
}
// PCINT21
ISR(PCINT2_vect) {
pmcChangeCount++; //Will increase by 2 every pulse (1 falling 1 rising)
pmcTriggerTime = millis(); //Current time
pmcFlag = true; //probably not needed (from earlier testing)
if (!(PIND & (1 << PD5))) { //if PD5 is low do this as it only occurs 1x per pulse
expFlag = !expFlag; //toggle state
expCounter++; // should only increment 1x per pulse (when low). So starts 0, 1st pulse recieved becomes 1, 2nd 2, etc.
}
}
If I'm understandng you correctly the ISR would be something like:
volatile boolean state;
volatile unsigned long pmcTriggerTime=0;
ISR(PCINT2_vect) {
state= digitalRead(pmcTriggerOutPin);
pmcTriggerTime = millis(); //Current time
}
This just toggles state 1 time for each pulse. I thought i had been doing that with if (!(PIND & (1 << PD5))) but I added that at the end of the day.
So then in the main code (or another function) something like
unsigned int expCounter=0; //this increases once for every 2 pulses (every odd pulse)
unsigned int tempCounter=0; //this is just counting pulses
boolean isExperimentRunning=false //initially no experiment running.
and then
//in loop
if(state==LOW){
isExperimentRunning = !isExperimentRunning; //1st pulse recieved experiment is running
tempCounter++;
}
if(tempCounter%2==1){ // this is here because tempCounter increments every pulse we want to increment on only odd ones
expCounter++;
}
The second if statement is where I think I'm overthinking it. Would it be better to just do the old old_var routine? Same ISR as above but:
boolean isExperimentRunning=false //initially no experiment running.
boolean old_isExperimentRunning=false;
unsigned int expCounter=0;
if(state==LOW){
old_isExperimentRunning=isExperimentRunning;
isExperimentRunning = !isExperimentRunning; //1st pulse recieved experiment is running
}
if(old_isExperimentRunning!=isExperimentRunning){
old_isExperimentRunning=isExperimentRunning
expCounter++;
}
So initially both isExperimentRunning (new) and old_isExperimentRunning (old) are false.
ISR triggers and we read the state of the pin. If it's low that means we are mid pulse. N
Then we check if the pin is low. If it is then we have a unique pulse
We then store the current value of isExperimentRunning as old_isExperimentRunning
We toggle isExperimentRunning
The second if statement checks to see if we have changed states by comparing old_isExperimentRunning to isExperimentRunning
If they aren't equal we know we have completed a cycle so we increment and then set to equal
Would this be a viable solution in that it is relatively fast and small? It just feels like I'm overcomplicating it.
Sorry for not being clear. Essentially it boils down to a state machine problem. I want to have some variable that changes once per pulse (equivelant to 2 pin changes), and it's an important signal so it needs to take priority over other tasks. I don't care if it's a falling edge, low, or rising edge that causes the variable to change.
So I looked at how the stateChange example is run. In that example the state is continuously polled and when there is a change it increments. In my case the polling is handled by the ISR in the background so to speak.
The ISR checks to see if it is low, and if it is we toggle isExperimentRunning and note the time. We now get back into loop and if the current value of isExperimentRunning is different than old_isExperimentRunning, we set them to equal and then increment expCounter.
Delta_G:
Put an if statement. If the pin is high then it was a rising transition. So your rising stuff. If the pin is low then it’s a falling transition. Do your falling stuff.
ISR(PCINT2_vect) {
if (the pin is high) then it was a rising transition. So your rising stuff.
else (the pin is low) then it’s a falling transition. Do your falling stuff.
}
Absolutely I'm overthinking it (and running my head into a wall). So I seem to be confusing everyon. Here is a rough sketch of what is going on. There is an input signal that is normally high. From the arduino start up there is no experiment running. At some point there will be a 10 ms pulse, which signifies that an experiment is starting. I need to set a flag for other logical conditions that the experiment is running, and count what experiment number it is. Once that is done the next pulse that comes in signifies that the experiment is over. The flag variable needs to be reset, the counter shouldn't increment. At some time later another pulse comes in, flag variable indicates the experiment is running and the experminet counter increments. I've included an even rougher sketch
OK. so I think between everyone's help and being away from the "problem" for a few days I think i have clairty on what to do.
So in the ISR I check to see if the pin is High/Low. In my case low is more logical as it's a pulse low. That gets me if the signal has been recieved. So then in the ISR I can increment a trigger counter and/or set a flag. In the main loop (or a function) I would either increment the counter (if not done in ISR) and reset the flag. With the counter I could do a modulo division by 2 (counter%2). If it is TRUE than the experiment is running (since 1%2=1). And to get the experiment number i'd just divide the trigger counter by 2 (I'm OK with starting with 0).
So in psudeo code it would be something like this:
//ISR
volatile boolean newPulse = false;
volatile unsigned long pmcTriggerTime = 0;
ISR(PCINT2_vect) {
if (digitalRead(pmcTriggerOutPin) == LOW) {
newPulse = true;
pmcTriggerTime = millis();
}
else {
//do nothing
}
//main code
unsigned int pulseCounter;
if (newPulse == true) {
pulseCounter++;
newPulse = false;
}
//to get if an experiment is running
boolean expRunning;
expRunning = pulseCounter % 2;
//to get experiment number
unsigned int experimentNumber = 0;
experimentNumber = pulseCounter / 2;
Fair enough point. This is not my wheelhouse (I'm a chemist), so my reasoning may be flawed, which is why I'm glad youre asking. I actually tell my interns that I don't care at how you get to an answer, just as long as it is correct and you can justify why you did it that way.
Going down the interrupt route as this trigger signal is pretty important to the project as it signifies the start/stop of data collection. I would normally go with just digitalRead but I'm concerned about loop bloat. In addition to this input, I'm using INT0, INT1 for two different tachometers. I have temperature coming in via SPI, timing information comming in via I2C, a PWM out, and a couple of ancillary I/Os that are low priority. Oh yeah and USART out. In addition there is some time averaging going on with the signals but I'm using fixed point. Down the road I may consider going to a Mega as there are additional inputs I'd like to capture/act on but for now
Right now I beleive that with everything said and done the loop time is on the order of 78 ms, as measured by toggling a pin at the start and end of loop and using oscilloscope to monitor. Input pulse is 10 ms, so it would seem to me that even if I could reduce a lot of the overhead (which i'm still planning to do via dirrect register i.e. avoid digitalWrite/Read), there is a high probability of missing the input.
Sorry for the delay. I wrote to here and wanted to verify timings of everything. I'm using larryd's method of PORTX=0x???????? (ad done by #define). I wound up using A3 for it so i used:
#define PulseA3 PINC=0x08;
//in setup
pinMode(A3,OUTPUT);
//start of loop
PulseA3;
//end of loop
PulseA3;
.
I've attached the results on a couple of time scales. I've got a 2 seconds/division (because there is a serial print every 1 second), 500 ms/division, and a measured 100 ms/division (measured the longest gap between
loberd:
Going down the interrupt route as this trigger signal is pretty important to the project as it signifies the start/stop of data collection.
...
Right now I believe that with everything said and done the loop time is on the order of 78 ms, as measured by toggling a pin at the start and end of loop and using oscilloscope to monitor. Input pulse is 10 ms, so it would seem to me that even if I could reduce a lot of the overhead (which I'm still planning to do via direct register i.e. avoid digitalWrite/Read), there is a high probability of missing the input.
I perceive an inconsistency there.
Given this trigger signal defines the start and stop of data collection, I have to ask (since I did not follow all of the foregoing discussion) - what is actually performing the data collection?
If it is code in the loop that is collecting the data and the loop is taking 78 ms for a round trip, it sounds as if the loop may also have problems reliably collecting the data. Is your "loop bloat" already compromising this? (Or is the data collection inefficient and causing loop bloat?)
Will do, probably tonight/tomorrow. I want to clean it up and make sure things make sense. It's in multiple files, but the two external interrupt routines are almost identical to eachother (just different variables), same with the PCINT routines. I'll see about attaching ALL in one post/attachment and then a "reduced instruction set" where it's just one of each kind of interrupt (with the thought that it would make less work for you).
Sorry for the delay in response but I belive that I see the solution now. In the ISR read the pin and if it is low (in my case), that's when you get the time, set the flag. Then in the main loop you can increment the counter/do whatever and clear the flag.
Thank you all for getting me unstuck brain was just completly not working right.
One point that was brought up that I will
Paul__B:
If it is code in the loop that is collecting the data and the loop is taking 78 ms for a round trip, it sounds as if the loop may also have problems reliably collecting the data. Is your "loop bloat" already compromising this? (Or is the data collection inefficient and causing loop bloat?)
I was troubled by how long the loop was taking. So I went digging with some good old pin toggling. Exhaustive search later I believe that I have the answer. One of the sensors I'm using is a PT100 RTD and the AdaFruit library for it. In the library I found this:
Now Im not good at programing but I do know two things:
delay() delays code execution
10+65=75 ms
So unless I'm offbase as written the RTD can not be measured faster than 75 ms, but realistically it's going to be longer (overhead). Also it doesn't make sense to measure temperature that fast with an RTD
No, the problem is that library code is for simple beginners who do not want to do more than one thing at a time.
You need to "unroll" it into your main code loop so that (using millis()), each action between the delay()s is a separate step and is not performed until on a later cycle through the loop(), millis() is found to have advanced by the necessary amount.
Properly written, the loop() should be executing many times per millisecond, and interrupts should be unnecessary unless they must be serviced in under a millisecond.
No, the problem is that library code is for simple beginners who do not want to do more than one thing at a time.
Where as I am a simple beginner who wants to do all the things. BUt I completely get what you are saying.
You need to "unroll" it into your main code loop so that (using millis()), each action between the delay()s is a separate step and is not performed until on a later cycle through the loop(), millis() is found to have advanced by the necessary amount.
At this point I'm probably NOT going to modify my "requirements" for this, as the amount of work to do this (as a layman) outweighs the benifits of minimizing the loop. Although down the road I may revisit it. If I were to do that would it be just nesting a series of:
if(millis()-previousMillis>=interval){
//do the stuff
previousMillis+=interval
or would it be more effienct to do it some other way?
Properly written, the loop() should be executing many times per millisecond, and interrupts should be unnecessary unless they must be serviced in under a millisecond.
As it stands now I think most functions run in under about 110 us with the exception of my printing to serial which is clocking in at around 4.5 ms. I think there is one or two other pieces that may be longer than that, but I don't have my thumb drive right now.
I'm on the fence about closing out this thread and starting a new one as I think I understand conceptually how to do the original question but it seems that I have either a wiring issue or code issue in the implementation. I check the pin state in the PCINT routine and if it's low note the time, set the flag and increment the counter. Then in a function in the loop I clear the flag. In theory, with a clean input, I should increment once per pulse. However it has erratic behavior, randomly incrementing w/o input. I believe that I found the problem but haven't tested yet. In writting this post I noticed that after I set a pin to be an input, set the PCMSK, clear PCIFR, and turn on the PCICR bits i've accidently then included pinMode(PCINT_PIN,OUTPUT); which probably explains the behaviour.
loberd:
I'm on the fence about closing out this thread and starting a new one
Always an exceptionally bad idea!
Possibly equivalent to "cross posting", means that having finally provided a fairly good description of your setup and the problems you have with it and received relevant assistance, you invite people to go through the entire process of tedious discovery again unless the new post is rapidly detected as such and merged with the old.
So I have gotten something that works (below). The wiring issue was in my brain. What I have now works but I feel like it isn't optimal. What happens is when I enter the ISR I check to see if the pin is low. If it is I set a flag, note the time, and increment a counter. Then in the loop I check to see if the flag is set. If it is I toggle a variable (for later logic), clear the flag and determine if the number of times the ISR has fired is even or odd. If it is odd I then increment the experiment counter.
I suspect that I could eliminate the second counter if I have a varible that is only toggled in the ISR which I can then just check in the main loop and increment accordingly. That might help eliminate another variable or two as well. I know you want your code to be fast/efficient in general but more so in ISRs. This method keeps the ISR small and presumably fast. I'm basing that off of the assumption that it is quicker to increment a counter than check a variable and then conditionally increment.
The code below is extracted and fixed from my main code. That's why there is a few extra bits set in the PCINT setup.
const byte pmcTriggerOutPin = 5; //Pin for PCM to trigger
const byte buttonStartPin = 8; //Pin for button for arming system
unsigned long printInterval = 500;
unsigned long previousPrint = 0;
int expCount = 0;
boolean pmcFlag = false;
void setPCINT() {
pinMode(pmcTriggerOutPin, INPUT); //does this need to be input pull up
pinMode(buttonStartPin, INPUT);
PCMSK2 |= 0b00100000; //PCINT21 PD5 D5
PCMSK0 |= 0b00000001; //PCINT0 PB0 D8
PCIFR = 0b00000000; //Clear all flags
PCICR |= 0b00000101; //Turn on port D PCIE2 and port B PCIE0
}
volatile unsigned int pmcChangeCount;
volatile boolean pmcTriggerRcvd;
volatile unsigned long pmcTriggerTime;
ISR(PCINT2_vect) { //doing the bare minimum
if (digitalRead(pmcTriggerOutPin) == LOW) { //only if this is a falling edge
pmcTriggerRcvd = true; //set flag so that any logic that doesn't need to be in ISR isnt
pmcTriggerTime = millis(); //capture time
pmcChangeCount++; //increment 1x per pulse. odd pulse signals exp start, even end (on/off behaviuour)
}
}
// Sees if PCINT2_vect has fired
void pmcTrigTest() { //this has the logic for what needs to happen when pulse is recieved
if (pmcTriggerRcvd == true) { // if PCINT2 has fired
pmcFlag = !pmcFlag; //toggle state of pmcFlag for later logic
pmcTriggerRcvd = false; //clear the pmcTriggerRcvd flag
if (pmcChangeCount % 2 == 1) expCount++;// if this is an odd numbered pulse increment the experiment counter
}
}
void setup() {
Serial.begin(115200);
setPCINT();
//just debugging
Serial.print("pmcFlag");
Serial.print("\t");
Serial.print("pmcTriggerTime");
Serial.print("\t");
Serial.print("pmcChangeCount");
Serial.print("\t");
Serial.print("expCount");
Serial.println();
}
void loop () {
pmcTrigTest();
//just debugging below
if (millis() - previousPrint >= printInterval) {
if (pmcFlag) Serial.print("true");
else Serial.print("false");
Serial.print("\t");
Serial.print(pmcTriggerTime);
Serial.print("\t");
Serial.print(pmcChangeCount);
Serial.print("\t");
Serial.print(expCount);
Serial.println();
previousPrint += printInterval;
}
//go trick or treating
}
). The issue I'm over thinking ishow do i set a flag varible every 2 triggers and increment every 2 triggers.
