Go Down

Topic: Variable Initialization/Scope Issue *SOLVED* (Read 195 times) previous topic - next topic

loberd

Sep 19, 2019, 01:42 am Last Edit: Oct 18, 2019, 03:21 pm by loberd
So I am having an intermittent issue with a tachometer on INT0 (D2) on the Nano.  I beleive the issue is either with the test signal i'm generating on the same Nano, or a variable initialization/scope issue.  The full code is below.  It is more complex than it needs to be as i'm including the code for the test/debug signal I'm using.  I don't have access to any components or instruments as I'm traveling for the next two weeks and I'm working on the code aspect of it.

It' a typical tachometer circuit, in that an external interrupt triggers an ISR (hotPlateTach) that changes a flag variable and captures the current microseconds, subtracts the previous microsecond (signal period), saves the microsecond as the old, and then it GTFO the ISR.  The signal would be generated by a phototransistor which gets passed through a Schmittt triggered.  Typical frequency is 6 Hz to ~ 120 Hz (which is 60 RMP to about 1200 Hz, about 50% duty cycle. 

In the getHotPlateFreq function, the period is converted to the frequency and then multiplied by 10 (which has to do with number of pulses per revolution and seconds to minutes).  It's a float, yes I know inefficent, and I will optamize later but right now i'm trying to figure it out.

As mentioned above I have code in there to generate a test signal created by bit banging a pin without using delay().  I've got the signal on two pins (D3 which is connected dirrectly to D2) and Pin 13 just for diagnostic purpses.  Also i'm printing out the period and frequency. 

The issue is the first reading has an incorrect period (and as such the frequency is off).  Most of the time the first period works out to be 16 microseconds.  Which is suspiciously close to either the clock jitter and/or the time to enter and exit ISR.  I have tried changing wher the ariables are initialized (e.g. set or reset).  Right now it is acting a little diferent in that it is giving me an initial period of 168 micro seconds. 

Any obvious errors.

Code: [Select]
//Actual code
const byte hotPlate = 2;                  //pin the sensor is on
volatile uint32_t freqMicrosHP;           //Current time
volatile uint32_t previousFreqMicrosHP;   //old time
volatile uint32_t periodHP;               //period=1/fq
volatile bool haveFreqHP;                 //has ISR been triggered
float hotPlateRPM = 0.0;                  //Result

//ISR for INT0 when D2 falls
void hotPlateTach() {                             //ISR for INT0
  freqMicrosHP = micros();                        //Current time
  periodHP = freqMicrosHP - previousFreqMicrosHP; //period
  previousFreqMicrosHP = freqMicrosHP;            //new time become old
  haveFreqHP = true;                              //you've got new data
}

//Processes data from ISR
void getHotPlateFreq() {                        //when there is new data, calculate RPM
  if (haveFreqHP) {                             //when ISR fires
    hotPlateRPM = 10 * (1000000.0 / periodHP);  //10 is a sensor related constant 1000000.0 keeps it a float and is us to S
    Serial.print(hotPlateRPM);
    Serial.print("\t");
    Serial.println(periodHP);
    haveFreqHP = false; //
  }
}

//Code for generating a test signal
const byte testSignalPin = 3;           //pin for signal to connect too ISR
const byte testSignalPin2 = 13;         //led pin for diagnostic
unsigned long previousSigMicros = 0;    //time of last toggle
unsigned long interval = 166666;        //test signal frequency
bool pinState = false;                  //State variable


//Generates a test signal
void testSignal() { //Toggle testSignalPin/2 on/off w/o delay()
  unsigned long currentSigMicros = micros();
  if ((unsigned long)(currentSigMicros - previousSigMicros) >= interval / 2) {
    pinState = !pinState;
    digitalWrite(testSignalPin, pinState);
    digitalWrite(testSignalPin2, pinState);
    previousSigMicros = micros();
  }
}


void setup() {
  //Signal generator code
  pinMode(testSignalPin, OUTPUT); //fq out
  pinMode(testSignalPin2, OUTPUT); //letd
  Serial.begin(9600);
  Serial.print("Frequency");
  Serial.print("\t");
  Serial.print("Period");
  Serial.println();
  //Actual Code
  pinMode(hotPlate, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(hotPlate), hotPlateTach, FALLING);

}

void loop() {
  testSignal(); //create test signal on D3 (fed to D2) and D13 a
  getHotPlateFreq(); //get fq
}



DrAzzy

#1
Sep 19, 2019, 02:04 am Last Edit: Sep 19, 2019, 02:07 am by DrAzzy
You can get a fair amount of jitter from the millis timer interrupt, but probably not quite that much - 16us @16mhz is 256 clock cycles. Iirc micros() has resolution of 4us (timer0 clocked off system clock prescaled by 64), meaning 192 clocks, but still, I didnt think the millis timer isr was that slow.

You may just be getting nickle and dimed to death.

I'd also, for starters, replace the digitalWrite calls that generated the signal with direct port writes (writing 1 to corresponding bit in the PINx register will toggle the pin state in a single clock without having to briefly disable interrupts ti make the write atomic like digitalWrite has to). DigitalWrite is slow because of the pin number lookup

If you have to, you can take over timer1 and use input capture on pin.... 9 iirc?  Takes some mental effort to grok how input capture works at first, but its not that bad. Theres probably a library to do it for you, but using peripherals manually is good for the soul
ATTinyCore for x4/x5/x61/x7/x8/x41/1634/828/x313 megaTinyCore for the megaavr ATtinies - Board Manager:
http://drazzy.com/package_drazzy.com_index.json
ATtiny breakouts, mosfets, awesome prototyping board in my store http://tindie.com/stores/DrAzzy

david_2018

The 168 microSeconds is the number of micro seconds from when the arduino is powered on (or reset) until the first interrupt occurs for your ISR (at least as far as the micros() function is concerned).  When you are actually using the real tach signal for an input, you will get the same type of error, because you have no way of starting the sketch so that it is synchronized with the tach input.  If the first reading being off is a problem, put a flag in the program to indicate that the first reading has not been processed, then the first time haveFreqHP is true, reset that flag and skip the calculation and printout (basically throw away the first result from the ISR).

cattledog

#3
Sep 19, 2019, 05:16 am Last Edit: Sep 19, 2019, 05:17 am by cattledog
Quote
Right now it is acting a little diferent in that it is giving me an initial period of 168 micro seconds.
I think the first reading anomaly can be avoided by disabling interrupts before the attachInterrupt(), clearing any flags set, and re-enabling interrupts.

Code: [Select]

  cli();
  attachInterrupt(digitalPinToInterrupt(hotPlate), hotPlateTach, FALLING);
  EIFR = 0x01;//clear INT0 flag
  sei();


The measurement jitter does not seem particularly important at 60Hz.

Whandall

I think the first reading anomaly can be avoided by disabling interrupts before the attachInterrupt(),
clearing any flags set, and re-enabling interrupts.
I don't think so.

How would that give previousFreqMicrosHP a correct value?
Ah, this is obviously some strange usage of the word 'safe' that I wasn't previously aware of. (D.Adams)

cattledog

#5
Sep 19, 2019, 05:15 pm Last Edit: Sep 19, 2019, 05:17 pm by cattledog
Quote
I don't think so.
How would that give previousFreqMicrosHP a correct value?
I think that in this case, with the internal test signal synched to the start of the sketch, the first test pulse starting with a low to high transition, and the interrupt on the next FALLING edge the first pulse timing works out correctly, especially since the frequency is only 60Hz and small errors are lost in noise.

With an external signal, picked up at a random point in its cycle, the initial pulse can never be measured. You have to synchronize to a specific transition. Clearing stray interrupt flags before doing that helps.

Whandall

I would not care if the in-sketch simulation works,
I would be interested in the measurement working on any source, YMMV.
Ah, this is obviously some strange usage of the word 'safe' that I wasn't previously aware of. (D.Adams)

loberd

I haven't gotten to spend much time on his with actual hardware (and won't until week after next).  I ran the sketch on a different computer and on a different arduino as well as the actual sensor and IIRC  the problem became more intermittent and it had a different initial value.  Now I haven't experimented enough to "confirm" this is true.  I'll have to also look at the code I ran and verify that its the same/document any changes.  Also it looked like the delay between when the arduino turned on (RESET going high) and when the test signal started outputing wasn't a multiple of any of the odd readings.

I'd also, for starters, replace the digitalWrite calls that generated the signal with direct port writes (writing 1 to corresponding bit in the PINx register will toggle the pin state in a single clock without having to briefly disable interrupts ti make the write atomic like digitalWrite has to). DigitalWrite is slow because of the pin number lookup

If you have to, you can take over timer1 and use input capture on pin.... 9 iirc?  Takes some mental effort to grok how input capture works at first, but its not that bad. Theres probably a library to do it for you, but using peripherals manually is good for the soul
For the quick testing noted above I did you one beter and hooked the nano to the actual sensor, and it seemed much better.  I had debated about doing dirrect port manipulation but was holding off on that.  I'm new to Arduino and C/C++ (I had some Java experinence ~15 years ago) and have really only done some coding in R (stats scripting language based on C/C++ IIRC).  So right now i'm on a steep learning curve and in a time crunch.  I am spoiled with a good IDE for R, R Studio (I'ts what I taught my self on).  I do have an Atmel ICE and want to work on weening myself off of Arduino IDE to Atmel Studio 7.

I had considered trying the input capture but the issue is I have 2 sensors that I need to measure the frequency on.  Both are square waves and the frequencies are in the range between 6 Hz and 6 kHz.  

loberd

The 168 microSeconds is the number of micro seconds from when the arduino is powered on (or reset) until the first interrupt occurs for your ISR (at least as far as the micros() function is concerned).
Agreed, I mentioned in a previous reply I'm not able to do a lot of debugging/diagnostics with hardware ATM.  But some quick testing on a different computer (probably a red herring) and arduino (still a nano) yeiled different "bad" values.  Also it seemed like the time between the reset and the signal outputting wasn't a multiple of the first value. 


The 168 microSeconds is the number of micro seconds from when the arduino is powered on (or reset) until the first interrupt occurs for your ISR (at least as far as the micros() function is concerned).  When you are actually using the real tach signal for an input, you will get the same type of error, because you have no way of starting the sketch so that it is synchronized with the tach input.  If the first reading being off is a problem, put a flag in the program to indicate that the first reading has not been processed, then the first time haveFreqHP is true, reset that flag and skip the calculation and printout (basically throw away the first result from the ISR).
That is a good thought.  Normally I wouldn't care about a reading being off but it's so far off that it effects the average too much.  I like the elegance of the flag option and throwing out the first data.  What I may wind up doing is putting in a lower limit for the period of the signal, maybe around 100 us (10 kHz), since I know that my signal shouldn't ever be over 6 kHz (600 us period).  If it's smaller than 100 us then it's "noise" and should be discarded.  I will play around with both as soon as I can.

david_2018

Something else you could do is to wait at the end of setup until the first interrupt has been received, discard the results from the interrupt, then continue on to loop.  That way you don't have to check a flag in the main code, but risk the program hanging in setup if the tach input is not pulsing.

loberd

I would not care if the in-sketch simulation works,
I would be interested in the measurement working on any source, YMMV.
I agree with getting to the real signal first.  Unfortunately I'm flying on Sunday and if I have a chance I'm going to toss a breadboard with an astable 555 timer as a poor man's signal generator so that I can do more debugging in hardware.  Bringing the sensor wouldn't be an option as it's some chemistry equipment, and i think a breadboard and some wires might be pushing it with airport security.

I think what cattledog was referencing was something Nick Gammon mentions on his Interrupt tutorial:

Quote
Something to be aware of is that these flags can be set before you attach the interrupt handler. For example, it is possible for a rising or falling level interrupt on pin D2 to be "flagged", and then as soon as you do an attachInterrupt the interrupt immediately fires, even if the event occurred an hour ago. To avoid this you can manually clear the flag.
and
Quote
However the "low level" interrupts are continuously checked, so if you are not careful they will keep firing, even after the interrupt has been called. That is, the ISR will exit, and then the interrupt will immediately fire again. To avoid this, you should do a detachInterrupt immediately after you know that the interrupt fired.
He then goes on with some different ways to attach/detach interrupts to avoid this.

loberd

All:
The solution was that the signal generated by the arduino was taking too long.  When i used the real sensor the code worked.

Go Up