ICP Frequency capture

Go To Last Post
40 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi im trying to get the frequency or PulseWidth using ICP. Im just using a standard square wave of 1ms PulseWidth.
Is there anything wrong with this code as i don't think im getting the right values.
Am i right in assuming that (in my case).

Time = (7.3728M / 8) * PulseWidth

#include 
#define F_CPU 7372800
#include 
#include 
#define BAUDRATE 115200
//calculate UBRR value
#define UBRRVAL 3//((F_CPU/(BAUDRATE*16))-1) (9600bps 47ubbr) (115200 3ubbr)

void USART_Init()
{
    //Set baud rate
	UBRR0L=(uint8_t)UBRRVAL; //low byte register
	UBRR0H=(UBRRVAL>>8);	 //high byte register
	//Set data frame format:
	UCSR0C &=~(1<<UMSEL01)|(1<<UMSEL00); //asynchronous mode
	UCSR0C &=~(1<<UPM01)|(1<<UPM00);     //No parity
	UCSR0C &=~(1<<USBS0);                //1 stop bit
	UCSR0C  |=(1<<UCSZ01)|(1<<UCSZ00);   //8 bit size
    UCSR0B &=~(0<<UCSZ02);               //8 bit size
	UCSR0B |=(1<<RXEN0)|(1<<TXEN0);      //Enable Receiver & Transmitter
//	UCSR0B |=(1<<RXCIEO)|(1<<TXCIEO);	
}

void USART_SendByte(uint8_t u8Data)
{
    while((UCSR0A&(1<<UDRE0)) == 0); // Wait if a byte is being transmitted   
    UDR0 = u8Data; // Transmit data
//	while((UCSR0A & (1<<TXC0)) == 0); //Must be done. Wait until byte has been transmitted
//	UCSR0A |= (1<<TXC0); 
}

void ICP_Init(void)
{
	//ICP1 enable
	TIMSK1 |=(1<<ICIE1); //enable input capture interrupt
	TCCR1B |=(1<<ICNC1)|(1<<ICES1)|(1<<CS11); //Noise canceller, rising edge, with 8 prescaler
	TIFR1 |= (1<<ICF1); //clear interrupt-flag
}

volatile uint16_t PulseWidth; //Kept in memory, not forgotten until re-written

ISR(TIMER1_CAPT_vect)
{
	PulseWidth = ICR1L;
	PulseWidth = (ICR1H<<8);
	//TCNT1 = 0;
}

int main(void)
{
	USART_Init();
	ICP_Init();
	sei();
	for(;;)
	{
		if((TIFR1 & (1<<ICF1)) == 0)
		{
		USART_SendByte(PulseWidth);
		USART_SendByte(PulseWidth>>8);
		}
	}
}
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You have an atomicity problem first up with PulseWidth. what's atomicity? Read my tutorial on the 'traps when using interupts' in the tutorial section.

Time = (7.3728M / 8) * PulseWidth
Note quite - let's not confuse period with frequency. One is the inverse of the other.

capture count = (FCPU/8) / 500 so in your case of 1mS pulse width which is a frequency of 500Hz (1ms high, 1ms low) should give a result of 1843 +/- a couple of counts.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

There is another major mistake in the code:

   PulseWidth = ICR1L;
   PulseWidth = (ICR1H<<8);

Warning: Grumpy Old Chuff. Reading this post may severely damage your mental health.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

In addition to Mr Kartman's comments, several things to note:

There's a difference between measuring *pulse width* and measuring *period between rising edges*. Your code seems to be attempting the latter. To measure pulse width, you need to capture both the rising and falling edges. It's a bit tricky to do this on the atmega32u4 - there's only one capture unit on the timers, so you need to flip it between capturing a rising edge and the subsequent falling edge.

Secondly, the "if" condition in the main loop is an unreliable way of testing to see if the capture has happened - the condition is true whenever there isn't a pending input capture ISR. Instead, you should have a variable that flags that a new value has been captured. Set the flag in the ISR, and clear it when you are about to print the result.

You should print the result in ASCII, at least while you're debugging. Also, you should test your UART code by printing out a short string at the beginning of main (I usually print a version number here).

When clearing TIFR1, you should just assign TIFR1 = (1 << ICF1);

Finally, the best way of using the input capture unit is to capture two separate events (either rising edge and falling edge, or rising edge and subsequent rising edge), and calculating the difference between the two captured values.

- S

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ok i have changed the code to the below after reading your replies, thank you.

I forgot to say it was a for a Mega168.

I want to read from high to high point again. So it will be high for 500us & low for 500us. Which is 1ms total that i want to read.

The code below is working now but im not 100% sure if im doing it the correct way.

Also my scope measured 1.022ms & the AVR measured 1.011ms. Very close but i guess there is a resolution issue? Would this resolution be FCPU/8 = 1.08us?

Also are the upper & lower resolutions:

upper = FCPU/8 = 1.08us?
lower = 65535/(FCPU/8) = 71.1ms?

I'm not 100% on how they are calculated but i guess the AVR wouldn't achieve those extremes.

#include 
#define F_CPU 7372800
#include 
#include 
#define BAUDRATE 115200
//calculate UBRR value
#define UBRRVAL 3//((F_CPU/(BAUDRATE*16))-1) (9600bps 47ubbr) (115200 3ubbr)

void USART_Init()
{
    //Set baud rate
	UBRR0L=(uint8_t)UBRRVAL; //low byte register
	UBRR0H=(UBRRVAL>>8);	 //high byte register
	//Set data frame format:
	UCSR0C &=~(1<<UMSEL01)|(1<<UMSEL00); //asynchronous mode
	UCSR0C &=~(1<<UPM01)|(1<<UPM00);     //No parity
	UCSR0C &=~(1<<USBS0);                //1 stop bit
	UCSR0C  |=(1<<UCSZ01)|(1<<UCSZ00);   //8 bit size
    UCSR0B &=~(0<<UCSZ02);               //8 bit size
	UCSR0B |=(1<<RXEN0)|(1<<TXEN0);      //Enable Receiver & Transmitter
//	UCSR0B |=(1<<RXCIEO)|(1<<TXCIEO);	
}

void USART_SendByte(uint8_t u8Data)
{
    while((UCSR0A&(1<<UDRE0)) == 0); // Wait if a byte is being transmitted   
    UDR0 = u8Data; // Transmit data
}

void ICP_Init(void)
{
	//ICP1 enable
	TIMSK1 |=(1<<ICIE1); //enable input capture interrupt
	TCCR1B |=(1<<ICNC1)|(1<<ICES1)|(1<<CS11); //Noise canceller, rising edge, with 8 prescaler
	TIFR1 |= (1<<ICF1); //clear interrupt-flag
}

volatile uint16_t PulseWidth; //Kept in memory, not forgotten until re-written

void Capture(void)
{
	cli();
	PulseWidth = ICR1;
	USART_SendByte(PulseWidth);
	USART_SendByte(PulseWidth>>8);
	TCNT1 = 0;
	sei();
}

ISR(TIMER1_CAPT_vect)
{
	Capture();
}

int main(void)
{
	USART_Init();
	ICP_Init();
	sei();
	for(;;)
	{
	}
}
Last Edited: Thu. May 17, 2012 - 03:23 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I don't think you've grasped the concept of atomicity or interrupts. You've fixed one problem and created a few more. Calling a function from the isr that disables interrupts that are already disabled then reading the ICR1 register correctly, then outputting the result via the uart ( which is a very bad idea in the isr). Your original code had the basics there. Reading icr1 incorrectly was the root of the problem as pointed out by stephen and then you need to add the cli/sei around the main line code that read tne value and outputted it.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I thought i had fixed the atomicity problem. The sound of that word drives me nuts. I read the atomicity thing but like all ways i get it wrong.

I read this in the datasheet but didn't understand it at first but i think i do now.

Quote:

The Input Capture Flag (ICF1) is set at
the same system clock as the TCNT1 value is copied into ICR1 Register. If enabled (ICIE1 = 1),
the Input Capture Flag generates an Input Capture interrupt. The ICF1 Flag is automatically
cleared when the interrupt is executed.

So if the ICF1 flag is set there is data to be read.
If i clear it even though the ISR will anyway; it will stop the same data going out the uart until the next new capture.

volatile uint16_t PulseWidth;

ISR(TIMER1_CAPT_vect)
{
   //Do i even need to declear this ISR?
}

int main(void)
{
   USART_Init();
   ICP_Init();
   sei();
   for(;;)
   {
      if((TIFR1 & (1<<ICF1)) != 0)
      {
      cli();
      PulseWidth = ICR1;
      USART_SendByte(PulseWidth);
      USART_SendByte(PulseWidth>>8);
      TIFR1 = (1 << ICF1); //Clears the flag.
      sei();
      }
   }
}
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Since you've eliminated the shared variable, then the issue of atomicity disappears. The downside of your current code is that you need to send the two bytes out the uart in less time than the shortest pulse you want to measure.if that doesnt happen, then you'll get measurement errors. Thats why you have the isr to grab the value and the mainline code outputs it at its leisure. This is called decoupling.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I tested the code & it was crap as you suggested.
I think this is what mnehpets was getting at.

volatile uint16_t PulseWidth; //Kept in memory, not forgotten until re-written
volatile int NewCapture = 0;

ISR(TIMER1_CAPT_vect)
{
   NewCapture = 1;
}

int main(void)
{
   USART_Init();
   ICP_Init();
   sei();
   for(;;)
   {
      if(NewCapture != 0)
      {
      cli();
      PulseWidth = ICR1;
      USART_SendByte(PulseWidth);
      USART_SendByte(PulseWidth>>8);
      NewCapture = 0; //Clears the flag.
      TCNT1 = 0;
      sei();
      }
   }
}
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
volatile int NewCapture = 0; 

Why waste two bytes on a flag when one would do?

Also be cautious about passing anything but 8bits between ISR and main(). While a 0/1 flag doesn't matter if you start passing numeric values and the variable is larger than 8bits you will need atomicity protection. This is why it's always a good idea to stick to 8bits when you can.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

Why waste two bytes on a flag when one would do?

What do you mean how would you do it?

Quote:

Also be cautious about passing anything but 8bits between ISR and main(). While a 0/1 flag doesn't matter if you start passing numeric values and the variable is larger than 8bits you will need atomicity protection. This is why it's always a good idea to stick to 8bits when you can.

I was thinking that so thanks for confirming it. I think i will stay with atomicity protection even with 8bit. I will have to keep in mind this problem for the future.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
What do you mean how would you do it? 

unsigned char

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

So do you mean skip the volatile & just

int NewCapture = 0;

I have also added to the code to stop incorrect values from being sent. It seems to work well. Is it ok to do it this way? It appears TOV1 flag doesn't work with ICP.

volatile uint16_t PulseWidth; //Kept in memory, not forgotten until re-written
volatile int NewCapture = 0;

ISR(TIMER1_CAPT_vect)
{
   NewCapture = 1;
}

int main(void)
{
	USART_Init();
	ICP_Init();
	sei();
	for(;;)
	{
		if (TCNT1 != 65535) //Dont send value or it will be corrupt.
		{
			if(NewCapture != 0)
     		{
			cli();
			PulseWidth = ICR1;
			USART_SendByte(PulseWidth);
			USART_SendByte(PulseWidth>>8);
			NewCapture = 0; //Clears the flag.
			TCNT1 = 0;
			sei();
			}
		}
	}
}
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hey Rabbit,

volatile uint16_t PulseWidth; //Kept in memory, not forgotten until re-written 
volatile uint8_t Capture;


ISR(TIMER1_CAPT_vect) 
{ 
static uint16_t prev,curr;

   curr = ICR1;
   PulseWidth =  curr - prev;
   prev = curr;
   Capture = 1;
} 

int main(void) 
{ 
uint16_t pulse;

   USART_Init(); 
   ICP_Init(); 
   sei(); 
   for(;;) 
   { 
   if (Capture)
      {
      Capture = 0;
      cli(); 
      pulse = PulseWidth; 
      sei();
      USART_SendByte(pulse); 
      USART_SendByte(pulse>>8); //how you determine which byte is high and low, I have no idea!
      delay_us(10000);
      }
   } 
} 

The above code should be closer to what you require. It would make more sense to print out the value in a human readable form. You might want to investigate printf() and itoa(). You can see we've decoupled the pulse measurement from the outputting of the value by using a flag and having the real-timecalculation done in the isr itself. You will also see that the atomic access only stops interrrupts for the copying of the variable (as detailed in my tutorial) whereas you disabled interrupts whilst the data was being sent out the uart which is slow. See how you fly with this.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

What do you mean how would you do it?

I meant replace:

volatile int NewCapture = 0; 

with

volatile unsigned char NewCapture = 0; 

Personally I prefer to use the stdint.h type names and if these were used instead what is changing here is even more obvious:

volatile int16_t NewCapture = 0; 

is being replaced by:

volatile uint8_t NewCapture = 0; 

that is this is using now using 8 not 16 bits of storage. (in case you don't know int=int16_t and unsigned char=uint8_t). I also switched from signed to unsigned as a 0/1 flag is never going to need to be interpreted as if it held a negative value.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The below is re-joined at the PC end so its readable.

USART_SendByte(pulse);
USART_SendByte(pulse>>8);

I have found that the first capture is incorrect but it is fine after that. I have been thinking about it & tried some code but i don't see a way around it. Is it not possible to throw away the first incorrect capture?

Kartman.
What was the delay_us(10000); for?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

But how do you guarantee synchronisation? The delay was to give some time so you can resync based on the time gap between transmissions.

Of course, the first capture is incorrect - the prev value will be 0 when first initialised. How to avoid this? I'll leave this as an exercise for the reader. The hint is that you need at least two capture events to get a valid reading.

No one picked up on my rabbit reference. get it? rabbit - warren? Tough crowd.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I think i see what you mean. Im not worried if i skip a value. After all it will be one every now & then. So i think the result will be the same in the space of a second. I don't think there is a way to get out of this at all but it should be less when logging them with SPI. The project will eventually be Logging results to a flash device with two ADC results at 10mS intervals. At that interval rate some results will be the same anyway. Logging at 10mS intervals is my next big issue.

Give me a week & i might come up with a solution, it wont be pretty though lol.
I know you need to get two results Bartman. :)
Damn it being dumb, it takes me ages to get anywhere, its been months & months now.

lol yeah i did. I didn't get teased much except for two occasions. One was a teacher who said does anyone know what a rabbit hole is called to the class. No one knew except me; then i said yeah its a Warren. Everyone looked confused then the whole class cracked up.
Second was a guy who sometimes called me Rabbit borrow Bennett. haha; cunt!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
Second was a guy who sometimes called me Rabbit borrow Bennett. haha; cunt!

Hey! That was me!

In the ISr, have something like this:

ISR(TIMER1_CAPT_vect) 
{ 
static uint16_t prev,curr; 
static uint8_t startup_count = 2;

   curr = ICR1; 
   PulseWidth =  curr - prev; 
   prev = curr; 
   if (startup_count)
     {
     startup_count--;
     }
   else
     {
     Capture = 1; 
     }
} 

As for sending out every 10ms, have a read of my other tutorial, "multitasking part 1"

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

Hey! That was me!

Quote: I don't think so Tim lol. I think i know the guys name.

Well, you legend. You just removed the two counts like that, man i wish i could have thought of that. I tried all sorts of crap. :)
I had to add to it as well i found that if i stopped it & started it again it would be back to its usual antics.

volatile uint16_t PulseWidth;
volatile uint8_t Capture;
volatile uint8_t startup_count = 2;

ISR(TIMER1_CAPT_vect)
{
	static uint16_t prev,curr;

	curr = ICR1;
	PulseWidth =  curr - prev;
	prev = curr;
	if (startup_count)
	{
		startup_count--;
	}
	else
	{
		Capture = 1;
	}
} 

int main(void)
{
	uint16_t pulse;

	USART_Init();
	ICP_Init();
	sei();
	for(;;)
	{
		if (TCNT1 == 65535) //If signal is lost during capture you have to restart.
		{
			startup_count = 2;
		}
		if (Capture)
		{
			Capture = 0;
			cli();
			pulse = PulseWidth;
			sei();
			USART_SendByte(pulse);
			USART_SendByte(pulse>>8);
		}
	}
}

I will have a read through your tutorial & see if i can work it out.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Why do you test TCNT1? I don't think it will have the effect you want. You need to add a timeout methinks. Read up on the timer compare feature. Enable a compare channel . In the compare isr, reload startup_count and disable the compare interrupt. In the capture isr, have OCR1x= ICR1+ timeout in ticks and enable the compare interrupt.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

By looking at TCNT1 if it reaches 65535 it is at the end so do a restart. It was what i came up with, that i knew how to do & it worked. I cant work out your theory on what you are trying to do.
This would consume less processing power i think

//Add this
TIMSK1 |=(1<<TOIE1); //Enable Timer/Counter1 Overflow Interrupt

ISR(TIMER1_OVF_vect)
{
	startup_count = 2;
}

//Remove
if (TCNT1 == 65535)
{
	startup_count = 2;
}
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

Why do you test TCNT1? I don't think it will have the effect you want. You need to add a timeout methinks. Read up on the timer compare feature. Enable a compare channel . In the compare isr, reload startup_count and disable the compare interrupt. In the capture isr, have OCR1x= ICR1+ timeout in ticks and enable the compare interrupt.

This is the best of what i could make out.

ISR (TIMER1_COMPA_vect)
{
	startup_count = 2;
	TIMSK1 &=~ (1<<OCIE1A); //Disable Interrupt
}

ISR(TIMER1_CAPT_vect)
{
	static uint16_t prev,curr;

	curr = ICR1;
	PulseWidth =  curr - prev;
	prev = curr;
	if (startup_count)
	{
		startup_count--;
	}
	else
	{
		Capture = 1;
	}
	OCR1A = ICR1 + timeout in ticks??
	TIMSK1 |= (1<<OCIE1A); //Enable Interrupt
}
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Testing tcnt1 for overflow wont work as timer1 will overflow and wrap back around to 0 and keep going.in the couese of doung your meaduremrnt, timer1 will most likely overflow. This is not a problem for the period measurement as the unsigned math makes it all work out correctly.

You latest bit of code is exactly what I'm suggesting. What timeout do you want? Ie the amount of time to elapse before you declare the input invalid? Once you've decided what timeout you want, represent that in terms of timer counts. The code loads a compare to happen x ticks after the last capture. If you keep getting a capture before that time, the compare will never happen as it keeps on getting reloaded.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

What timeout do you want?

The maximum possible & that's why i kept specifying 65535 or overflow events. If i use a 7.3728MHz / 64prescaller * 65535 i will get 568.88ms or 105.47RPM.
105.47RPM is acceptable & ill never push it past 3000RPM either.
I cant imagine ICR1 + timeout.
65535 + 65535 =??

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

set the timeout to -1 which is 65535 as an unsigned value. You can count overflows if you want a greater timeout value but you want to count more than one overflow - reason being if your last capture is close to 65535, then you'll timeout almost immediately. I counted a number of overflows in my tacho with the assumption that the human user would not be able to perceive a imprecise timeout. Basically if the engine stopped and the tacho drops to 0 soonafter, then all is well. Another technique is to create a system tick of say 10ms using another timer or one of the capture channels and count down a variable, when it gets to 0, then load the startup_count value. On each capture, reloadvthis variable with the timeout in 10ms ticks. This will give you a timeout accurate to +/- 10ms.

In future, tell us what you want to achieve - only now have you told us you want to measure rpm. If you're using a capacitive pickup from a coil or points, we can also tell you how to ignore the coil ringing.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The signal is already coming from a micro & it its just a square wave. I thought it wasn't going to be to complicated but its certainly there. I haven't got the code to work properly yet but im not 100% certain on how it works all though i do understand how its supposed to work. I have the code setup as in my last code post but it produces nothing on screen. I will have a go at working it out more.

Quote:

You can count overflows if you want a greater timeout value but you want to count more than one overflow - reason being if your last capture is close to 65535, then you'll timeout almost immediately.

That is exactly what i was going for so yes you have a lower limit. Two events must happen within 65535. Unlike your example which is able to go over two 65535 counts.

I am sending the captured values to a flash device then graphing them on the computer. I don't want a glitch or it will make a mess of the line graph.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

My code allows the timer to rollover. The timer is never reset, so one capture can happen at count 2000 and the next at 1999 which means the timer rolled over and the period was 65535 counts. Unsigned binary math makes it work.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I tired to get your code working but it just didn't. I understand what you are doing but i don't have enough ram in my head to make the maths/code work :)

I came up with this as i can only visualize one wave length in my head & it works. Its probably really bad though lol.

volatile uint16_t PulseWidth;
volatile uint8_t Count;
volatile uint8_t Capture;

ISR(TIMER1_OVF_vect)
{
	Count = 0;
}

ISR(TIMER1_CAPT_vect)
{
	++Count;

	if (Count == 2)
	{
		PulseWidth = ICR1;
		Count = 0;
		Capture = 1;
	}

	TCNT1 = 0; //Reset Counter
}

Your Awesome Kartman! Sooo much patience!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

By touching tcnt1, you're losing the precision that using the input capture gives you. Since you're running a /64 prescaler, this cuts you a little slack. If the time between the input capture, getting into the isr, executing the code then clearing TCNT1 is less than 64 clocks, then you're home free. Have a look at your assembler listing and count the cycles to see if you scrape in. Anyway, the technique I used avoids this problem as long as the period between captures is longer than it takes the isr to execute. Let's try the 10ms tick method. Set up a compare channel to interrupt on compare. In the compare isr have some code like this:

OCR1A += 10ms worth of timer ticks;
If (timeout)
  {
  timeout--;
  If ( timeout==0)
     {
     Startup_count =2;
      }
   }

You can also use this isr for a timer tick to schedule tasks. Refer to my tut on multitasking.

Last Edited: Mon. May 21, 2012 - 03:24 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It is a pain in the bum using an iPad to write code in a browser so please excuse my spellink and lazy use of punctuation.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Here's my version for freq and duty that doesnt use any interrupts or input capture, just for comparison.

Attachment(s): 

Imagecraft compiler user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

volatile uint8_t timeout;

   TIMSK1 |= (1<<OCIE1A); //Enable timer tick Interrupt 


ISR (TIMER1_COMPA_vect) 
{ 
   OCR1A += 10ms in ticks; //next compare in 10ms time
   if (timeout)
      {
      timeout--;
      if (timeout == 0)
         {
         startup_count = 2; //reset the capture machine
         }
      }
} 

ISR(TIMER1_CAPT_vect) 
{ 
   static uint16_t prev,curr; 

   curr = ICR1; 
   PulseWidth =  curr - prev; 
   prev = curr; 
   if (startup_count) 
   { 
      startup_count--; 
   } 
   else 
   { 
      Capture = 1; 
      timeout = number of 10ms ticks for timeout;
   } 
} 

Note that I use downcounters - this is a throwback to my assembler days as most processors have a conditional jump for zero/non-zero, whereas to test for a specific value, you need to do a compare then a conditional jump. Whether it makes a difference when using C, I have no idea, but in some cases using downcounters seem more logical to me - you load them up and they count down to 0.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

It is a pain in the bum using an iPad to write code in a browser

+1

(I've been using an iPad recently when reading Freaks some times and it is just plain irritating in many senses - give me a real computer any day).

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

By touching tcnt1, you're losing the precision that using the input capture gives you. Since you're running a /64 prescaler, this cuts you a little slack. If the time between the input capture, getting into the isr, executing the code then clearing TCNT1 is less than 64 clocks, then you're home free.

I know, i was being sneaky & trying to get away with it; that's why i reset the count as the last thing i did. :)

I worked out why the code wasn't working.
Somehow; by mistake; I didn't put OCR1A into CTC mode.
Bastard. This is now working good.

void ICP_Init(void)
{
	//ICP1 enable & CTC
	TIMSK1 |=(1<<ICIE1);	//Enable input capture interrupt
	TCCR1B |=(1<<WGM12);	//CTC Mode. This is used as an overflow.
	TCCR1B |=(1<<ICNC1)|(1<<ICES1)|(1<<CS11)|(1<<CS10); //Noise canceller, rising edge, with 64 prescaler
	TIFR1 |= (1<<ICF1);		//clear interrupt-flag
}


volatile uint16_t PulseWidth;
volatile uint8_t startup_count;
volatile uint8_t Capture;

ISR (TIMER1_COMPA_vect)
{
   startup_count = 2;
   TIMSK1 &=~ (1<<OCIE1A);
   PORTD ^= (1<<PD3);
} 

ISR(TIMER1_CAPT_vect)
{
   static uint16_t prev,curr;

   curr = ICR1;
   PulseWidth =  curr - prev;
   prev = curr;
   if (startup_count)
   {
      startup_count--;
   }
   else
   {
      Capture = 1;
   }
   
   OCR1A = ICR1 + 65535;
   TIMSK1 |= (1<<OCIE1A);
}

int main(void)
{
	DDRD |= (1<<PD3);
	uint16_t pulse;

	USART_Init();
	ICP_Init();
	sei();
	for(;;)
	{
		if (Capture)
		{
			Capture = 0;
			cli();
			pulse = PulseWidth;
			sei();
			USART_SendByte(pulse);
			USART_SendByte(pulse>>8);
		}
	}	
}

Quote:

It is a pain in the bum using an iPad to write code in a browser

+1

Texting is a pain in the ass even if you have a swipe feature.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You don't want CTC mode.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Oh no. I think that's it for me. I don't have any more time left for this project.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Well, did you learn anything in the process?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Something way over the top for me.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Did you look at the Complete One File Compilable example that prints out freq and duty cycle? It compiles right up on the iccv8avr compiler. Might just light the lightbulb. Worth a try.

Imagecraft compiler user