Hi and hello to everyone!

I've read the threads regarding tachs and got a pretty good idea about their operating principles and have developed my own code for such a task, but I have encountered a problem (code will be at the end of the post). The software for writing the code is Codevision AVR and for the simulation I used Proteus.

I'm using an Atmega16's ICP functionality to capture the pulses from the engine's coil (well not yet anyway) using timer1. It's divided by a factor of 1024 so my timer's period is 128us. In the ICP's interrupt routine I pretty much save the value in ICR1L (disregarding the Higher 8bits and assuming the counter started from 0), calculate the time between the last pulse by multiplying ICR1L with T1_Period and then reversing it to obtain the frequency. I am also using a 4 digit 7 segment display to show my results, which seems to be working fine with other numbers, but not with the frequency. After a lot of tests and retries, I somehow noticed that my frequency computation result is always somewhat close to 1, so the best result should be 1, but it isn't always. I tried with several different frequencies, but ended up with no other solutions. I think the problem is the float variable used for holding the division answer, but I am not sure. It also won't let me use doubles.

Also, for a frequency of 5 hz, ICR's value was 195 in decimal, so , when computing the frequency by hand I get about 40 Hz. Am i thinking something wrong?

Thanks!

#include <mega16.h>

#define MAX_UL 213
#define INTI 10

#define DIG_1   PORTB.0 //OUTPUT    H1
#define DIG_2   PORTB.1 //OUTPUT    H2
#define DIG_3   PORTB.2 //OUTPUT    M1
#define DIG_4   PORTB.3 //OUTPUT    M2

#define SEG_A   PORTA.0 //OUTPUT    L1
#define SEG_B   PORTA.1 //OUTPUT    L2
#define SEG_C   PORTA.2  //OUTPUT    L3
#define SEG_D   PORTA.3 //OUTPUT
#define SEG_E   PORTA.4 //OUTPUT
#define SEG_F   PORTA.5 //OUTPUT
#define SEG_G   PORTA.6 //OUTPUT
#define SEG_DP  PORTA.7 //OUTPUT

#define pulse   PORTC.0 //OUTPUT

#define var1   PORTB.6 //OUTPUT
#define var2   PORTB.7 //OUTPUT

#define F_OSC   8000000
#define F_T1    7813
//#define T_T1    1/F_T1
//#define T_T1    0.000128
#define T_T1    128

int  counter=0;
unsigned char new_val = 0;
unsigned char old_val = 0;
unsigned char dif = 0;
unsigned char dif1 = 0;
float time = 0;
float freq = 0;
float tix = T_T1;

int x = 0;
static unsigned char dig = 0;
static unsigned char seg_1 = 0;
static unsigned char seg_2 = 0;
static unsigned char seg_3 = 0;
static unsigned char seg_4 = 0;

void selectDigit(unsigned char digit);
void displayDigit(unsigned char number);
void display7SEG_ch(unsigned char h1, unsigned char h2, unsigned char m1, unsigned char m2);
void my_delay(int rep);
void pulse_f();

// Timer1 input capture interrupt service routine
interrupt [TIM1_CAPT] void timer1_capt_isr(void)
{
    counter++;
    pulse_f();
    
    //dif = TCNT1L;
    dif1 = ICR1L;
    dif = TCNT1L;
    //dif = new_val - old_val;
    time = dif * T_T1;
    freq = 1 / time;
    TCNT1L = 0x00;
    TCNT1H = 0x00;
    ICR1L = 0x00;
    ICR1H = 0x00;        
}

// Timer 0 output compare interrupt service routine
interrupt [TIM0_COMP] void timer0_comp_isr(void)
{
    x++;
    if (x == INTI)    // == 10 => 100ms
    {
        dig++;   
        if (dig == 5)
        {
            dig = 1;
        }
        selectDigit(dig);
        
        switch (dig) 
        {
        case 1:
        displayDigit(seg_1);        
        break; 
        
        case 2:
        displayDigit(seg_2);
        break; 
        
        case 3:
        displayDigit(seg_3);
        break; 
        
        case 4:
        displayDigit(seg_4);
        break;                
        };        
         x=0;         
    }
}

// Declare your global variables here

void main(void)
{
// Declare your local variables here

// Input/Output Ports initialization
// Port A initialization
// Func7=Out Func6=Out Func5=Out Func4=Out Func3=Out Func2=Out Func1=Out Func0=Out 
// State7=0 State6=0 State5=0 State4=0 State3=0 State2=0 State1=0 State0=0 
PORTA=0x00;
DDRA=0xFF;

// Port B initialization
// Func7=Out Func6=Out Func5=Out Func4=Out Func3=Out Func2=Out Func1=Out Func0=Out 
// State7=0 State6=0 State5=0 State4=0 State3=0 State2=0 State1=0 State0=0 
PORTB=0x00;
DDRB=0xFF;

// Port C initialization
// Func7=In Func6=In Func5=In Func4=In Func3=In Func2=In Func1=In Func0=In 
// State7=T State6=T State5=T State4=T State3=T State2=T State1=T State0=T 
PORTC=0x00;
DDRC=0x01;

// Port D initialization
// Func7=In Func6=In Func5=In Func4=In Func3=In Func2=In  Func1=In Func0=In 
// State7=T State6=T State5=T State4=T State3=T State2=T State1=T State0=T 
PORTD=0x00;
DDRD=0x00;

// Timer/Counter 0 initialization
// Clock source: System Clock
// Clock value: 125,000 kHz
// Mode: CTC top=OCR0
// OC0 output: Disconnected
TCCR0=0x0B;
TCNT0=0x00;
OCR0=0x26;

// Timer/Counter 1 initialization
// Clock source: System Clock
// Clock value: 7,813 kHz
//  Mode: Normal top=0xFFFF
// OC1A output: Discon.
// OC1B output: Discon.
// Noise Canceler: Off
// Input Capture on Rising Edge
// Timer1 Overflow Interrupt: Off
// Input Capture Interrupt: On
// Compare A Match Interrupt: Off
// Compare B Match Interrupt: Off
TCCR1A=0x00;
//TCCR1B=0xC5;
TCCR1B=0x45;
TCNT1H=0x00;
TCNT1L=0x00;
ICR1H=0x00;
ICR1L=0x00;
OCR1AH=0x00;
OCR1AL=0x00;
OCR1BH=0x00;
OCR1BL=0x00;

// Timer/Counter 2 initialization
// Clock source: System Clock
// Clock value: Timer2 Stopped
// Mode: Normal top=0xFF
// OC2 output: Disconnected
ASSR=0x00;
TCCR2=0x00;
TCNT2=0x00;
OCR2=0x00;

// External Interrupt(s) initialization
// INT0: Off
// INT1: Off
// INT2: Off
MCUCR=0x00;
MCUCSR=0x00;

// Timer(s)/Counter(s) Interrupt(s) initialization
TIMSK=0x22;

// USART initialization
// USART disabled
UCSRB=0x00;

// Analog Comparator initialization
// Analog Comparator: Off
// Analog Comparator Input Capture by Timer/Counter 1: Off
ACSR=0x80;
SFIOR=0x00;

// ADC initialization
// ADC disabled
ADCSRA=0x00;

// SPI  initialization
// SPI disabled
SPCR=0x00;

// TWI initialization
// TWI disabled
TWCR=0x00;

// Global enable interrupts
#asm("sei")

while (1)
      {
      // Place  your code here
      float y = 0;
      int i=0,m,s,z,d;
        
        for (i=0;i<10000;i++)
        {   
            //i = counter;
            //i = TCNT1L //-> works
            //i = freq;
            m = i / 1000;
            s = (i / 100) % 10;
            z = (i / 10) % 10;
            d = i % 10;
            display7SEG_ch(m,s,z,d);
            my_delay(100);
            if(freq < -1 && freq > -2)
                {var1 = 1;
                 var2 = 0;}                
            else
                {var1 = 0;
                 var2 = 1;} 
        }
        //display7SEG_ch(1,2,3,4);
      }
}

void my_delay(int rep)
{
    int a;
    unsigned long i;
        
    for (a=0;a<rep;a++)
    {    for  (i=0;i<MAX_UL;i++) 
        {
            
        }//3
    }//rep    
}//my_delay

void pulse_f() // debug purposes
{
    int i=1;

    pulse = 1;
    while(i--){};
    i=1;
    pulse = 0;
    while(i--){};
}

 

I removed the functions regarding the 7seg display in order to fit the text in my previous post, I doubt they are important, but can post them at request

 

I think you need to post the specs before we can really nut-out timer period calcs.

What is the minimum and maximum RPM value (or Hz) that you expect from the ignition coil?

I don't work with Atmega16 but with a PIC I would use the capture/compare hardware module to automatically grab the period between two tach pulses.

Then the next step would be to display that capture value, to confirm the capture and timers are all working ok.

Once you have confirmed the period capture is working and giving the correct period, the math in C code to convert that period to RPS or RPM is quite straightforward.

 

Oh, i forgot about that The engine is a four-stroke four-cylinder gas engine, and after some digging I found that for every 30 pulses I get an RPM. That, or another option was that the sparks come at half the RPM Don't know yet, I need to experiment with that. The maximum would be 5500RPM. Thus, my freq range should be between 0 and 200 Hz max (25-175 actually).

Sorry, my knowledge in PIC uCs is pretty useless, but I didn't figure in the Atmega an option to automatically give the difference between pulses, so I compute it by resetting the timer at every RETI.

I still think that the issue is somewhere concerning variables and copmutation, but any idea is an idea

 

Oh, i forgot about that The engine is a four-stroke four-cylinder gas engine, and after some digging I found that for every 30 pulses I get an RPM. That, or another option was that the sparks come at half the RPM Don't know yet, I need to experiment with that. The maximum would be 5500RPM. Thus, my freq range should be between 0 and 200 Hz max (25-175 actually).

That would be unusual... Normally a 4 cylinder engine with one coil and a distributor makes two sparks per crankshaft rev.

Maybe you meant 1Hz = 30 RPM? That sounds more like it, or 2Hz = 60 RPM, because there are two pulses per rev.

...
Sorry, my knowledge in PIC uCs is pretty useless, but I didn't figure in the Atmega an option to automatically give the difference between pulses, so I compute it by resetting the timer at every RETI.

I still think that the issue is somewhere concerning variables and copmutation, but any idea is an idea

Resetting the timer introduces errors. It is better to let the timer free-run, and just grab its value, then compute the timer period between two grabbed values. That way there is no accumulated error from delays reading or resetting the timer.

 

That would be unusual... Normally a 4 cylinder engine with one coil and a distributor makes two sparks per crankshaft rev.

Maybe you meant 1Hz = 30 RPM? That sounds more like it, or 2Hz = 60 RPM, because there are two pulses per rev.

This makes sense to me, but I haven't taken this into consideration due to the previous problem Let me take this again, 2 sparks/rev makes my "input frequency" from 0->5500 to 0->5500/30(least) and by multiplying the result by 2 I can find the number of rotations /minute? I dunno why this is giving me such a headache

This is how I thought of it:
From the coil, using your suggestion, there are X pulses. From that, the number of revolutions per second is X/2, therefore by transforming this to RPM we multiply it by 60 and the result is 30*X. So, by employing this algorithm, I need to sample the number of pulses from the coil during a second, then multiply them by 30 and get the RPM, right? And if the sampling period is 1/10th of a second, this also needs to multiply my result by 10, right? Sorry for the idiocracy of my comment

Resetting the timer introduces errors. It is better to let the timer free-run, and just grab its value, then compute the timer period between two grabbed values. That way there is no accumulated error from delays reading or resetting the timer.

That was the first idea, but I could only get the values from the input capture register in Hi & Lo 8 bit variables, so concatenating them seemed to be a more cumbersome operation for me and the uC in order for proper results.

Thank you for your responses !

 

This makes sense to me, but I haven't taken this into consideration due to the previous problem Let me take this again, 2 sparks/rev makes my "input frequency" from 0->5500 to 0->5500/30(least) and by multiplying the result by 2 I can find the number of rotations /minute? I dunno why this is giving me such a headache

OK, so it's a standard 4 cyl 4 stroke engine with distributor, giving 2 pulses per rev. We're slowly getting there.

Revs are not 0-5500, the engine idle speed will be maybe 500 RPM at the lowest, so the RPM is 500-5500. That might sound like nitpicking but it is important in choosing the software method used to measure the period because now we have a defined minimum and maximum period.

This is how I thought of it:
From the coil, using your suggestion, there are X pulses. From that, the number of revolutions per second is X/2, therefore by transforming this to RPM we multiply it by 60 and the result is 30*X. So, by employing this algorithm, I need to sample the number of pulses from the coil during a second, then multiply them by 30 and get the RPM, right? And if the sampling period is 1/10th of a second, this also needs to multiply my result by 10, right? Sorry for the idiocracy of my comment
...

I get 600 RPM = 10 RPS = 20Hz. (ratio RPM:Hz = 600:20 = 30:1)

So you did well, if you multiply Hz *30 you get the figure in RPM.

Of course, you are not measuring Hz (freq) at all, you're measuring period. And your real calc should convert period to RPM;
X / period = RPM
(where you need to decide the value of constant "X")

So at 600 RPM, ie 20Hz pulses, if your period is in uS the period = 50000uS
so constant X = 30mil;
30000000 / 50000 = 600 RPM

Please be aware that the forum does not allow discussion of automotive modifications on road vehicles for safety reasons. If the moderators think this is for a vehicle they will shut down the thread. Up to this point I have assumed this might be for a generator or something, but you should clarify that.