Input Capture - pulse width problem

Hi guys,
I'm pretty new to Arduino, I've been programming another AVR boards for some time.
I have a problem with Input Capture, there's probably some mistake in the code but it's already taken 12h of my life...
I need to decode encoder 10bit PWM output signal. It's about 1kHz with pulses beginning from 1us. I work with Arduino MEGA2560.

Because pulseIn() can measure pulses from 10us, I have to use Input Capture. I've written some test code just to check if it works before plugging in the encoder. I try to measure both t_on and t_off as in the link above and what I get is proper value of t_off and completely random value of t_on. I have no idea what I'm doing wrong.
I use PWM signal generated with TIMER3 - 1kHz wave with 50% duty cycle. With TIMER4 setting as in the code I should get t_on=t_off=1000.

Moreover, I've read much about pulse width measuring. In general people say that it would be hard to measure 1us width pulse. I'm aware of that but I'd like this code to work properly anyway - for this example t_on=t_off=500us. Do you have any advice for me? How should I decode signal from the encoder? Is Input Capture good idea or should I try something else? I need position update of about 200Hz.

Thanks in advance for your help!

#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/delay.h>

#define MEASUREMENT_BEGIN 0
#define AFTER_1_INTERRUPT 1
#define T_ON_AVAILABLE 2
#define MEASUREMENT_AVAILABLE 3

volatile uint8_t Control_var;
volatile uint16_t LastCapture;
volatile uint16_t t_on;
volatile uint16_t t_off;
volatile float position;

void encoder_measure_init()
{
	DDRL &= ~(1<<PL0);	//encoder input
	PORTL |= (1<<PL0);	//pin pull-up
	
        TCCR4A = 0;             //TIMER4 normal mode
        TCCR4B = 0;
        TCCR4C = 0;             //TIMER4 normal mode
	TCCR4B |= (1<<CS41);	//prescaler -> Timer4 / 8
	TCCR4B |= (1<<ICES4);	//rising edge
}

void PWM_init()
{	
	DDRE |= (1<<PE3);	//set as an output

        TCCR3A = 0;        
        TCCR3B = 0;
        TCCR3C = 0;
        
	TCCR3A |= (1<<WGM31);		//setting Fast PWM mode, count up to ICR3
	TCCR3B |= (1<<WGM32) | (1<<WGM33);	//setting Fast PWM mode, count up to ICR3
	TCCR3A |= (1<<COM3A1);	//set OC3A at BOTTOM, clear on OCR3A
	TCCR3B |= (1<<CS30);	//prescaler -> Timer3 / 1
	
	ICR3 = 15999;      //I want to get 1000Hz frequency. F_CPU= 16MHz
	OCR3A = 7999;      //I should get 50% duty cycle
}


//interrupt procedure
ISR(TIMER4_CAPT_vect)
{	
        if( Control_var == AFTER_1_INTERRUPT )
		{
                        t_on = ICR4 - LastCapture;
		}

        if( Control_var == T_ON_AVAILABLE )
		{
			TIMSK4 &= ~(1<<ICIE4);  //turn off capture interrupt
			t_off = ICR4 - LastCapture;
		}
		
	LastCapture = ICR4;
        Control_var++;
        TCCR4B ^= (1<<ICES4);	//toggle the edge
}

float get_position()
{  
	uint16_t pos;
	
	Control_var = MEASUREMENT_BEGIN;
	TCNT4 = 0;    //not to let the register overflow
        TCCR4B |= (1<<ICES4);    //set rising edge
	TIMSK4 |= (1<<ICIE4);	//turn on capture interrupt
	
	while(Control_var != MEASUREMENT_AVAILABLE)	{}
	
	position = ( ((float)t_on * 1026.) / ((float)t_on + (float)t_off) ) - 1.;
	
	if (pos == 1024) pos = 1023;
	
	return pos;
}

void setup() {
  // put your setup code here, to run once:
        encoder_measure_init();
        PWM_init();
        sei();        
        Serial.begin(9600);
}

void loop() {
  // put your main code here, to run repeatedly:
        position = get_position();
        //Serial.write((int)position&0xff);
        //Serial.write((int)position>>8);
        Serial.write(t_on&0xff);
        Serial.write(t_on>>8);
        Serial.write(t_off&0xff);
        Serial.write(t_off>>8);
        Serial.write(0);
       
       _delay_ms(1000);
      }

Moreover, I've read much about pulse width measuring. In general people say that it would be hard to measure 1us width pulse.

With a 16 Mhz Arduino I don't believe you can get below 5us pulse width measurement. Nick Gammon has provided timer based pulse width measuring code in this example.

Okay, I've solved my problem. As always there was little expression missing that changes everything.
The get_position() function should look like this:

float get_position()
{  
 float pos;
 
        begin:
 Control_var = MEASUREMENT_BEGIN;
 TCNT4 = 0;    //not to let the register overflow
        TCCR4B |= (1<<ICES4);    //set rising edge
        TIFR4 |= (1<<ICF4);      //this should be added!!!
 TIMSK4 |= (1<<ICIE4); //turn on capture interrupt 

 while(Control_var != MEASUREMENT_AVAILABLE) {}
 
        if( (t_on + t_off) > 16*1076 ) goto begin;
        //mozna zrobic diode, ktora bedzie informowala o niepozadanym zakresie
        
 pos = ( ((float)t_on * 1026.) / ((float)t_on + (float)t_off) ) - 1.;
 
 if (pos > 1023) pos = 1023;
 
 return pos;
}

In general there is only one change - I added TIFR4 |= (1<<ICF4); to clear ICF4. It turns out that the flag in TIFRn register is set always when change on a pin occures (it's set on rising or falling edges accordingly to value of ICESn bit in TCCRnB register).
It doesn't matter if you clear corresponding bit in TIMSKn register (TIMSK4 &= ~(1<<ICIE4);), the flag is still being set and when only you write ICIEn bit to one to enable Input Capture interrupt the program goes straight to interrupt procedure without waiting for another interrupt after enabling it.
It's a problem if you don't want to get the interrupt all the time when the pin status changes and you want to measure eg. the position every 1 second like me in the code above. During that one second there are always changes on the pin with both falling and rising edges because the encoder is still working and that results in setting the flag in TIFRn register. It causes incorrect measurements because whole procedure starts in the wrong moment.

As I supposed I'm unable to measure 1us pulses but it's still a success - I got down to 3.3us with changing the prescaler to 1. The interrupt procedure is as slim as it was possible to write. I've got now +/-1.05 degree dead zone in encoder readings but for me it's fine. Maybe someone will find it useful :slight_smile: