Warning, this ones a doozy.
This function does do a lot but I've got it this way because the only time I want to sample the rpm is when I'm going to change the servo position. The servo position is what I've prioritized. I've got two ISRs that handle the servo position. A timer1 overflow and compare match. The only thing that interferes with these ISRs is the ISR that timestamps the tach signal. The tach detecting ISR is only enabled for brief periods to prevent skewing the servo position.These are the timer1 ISRs.
/*
This ISR Ends the pulse that controls the servo
and sets the new pulse duration
*/
ISR(TIMER1_COMPA_vect)
{
// interrupt on Timer1 compare match
PORTB &= 0xFD; // Clears pin 9 (PORTB[1])
OCR1A = ServoPos; // Sets duration of pulse to control servo
return;
}
/*
This ISR Starts the pulse that controls the servo
and waits between pulses
*/
ISR(TIMER1_OVF_vect)
{
// interrupt on Timer1 overflow
if (i > 2)
{
PORTB |= 0x02; // Sets pin 9 (PORTB[1])
i=0;
}
else
{
i++; // Waits long enough before another pulse
}
return;
}
The way the tach signal is time stamped uses interupt0. Heres that ISR.
/*
This ISR measures the tach signal
*/
ISR(INT0_vect)
{
Time = micros(); //acquire current time
if((Time - TimeOld) < 1000) //this is a noise filter, ignores any two events that occur within 0.001 secs of each other
{ //at 2 events (rising and falling edge) per revolution this sets the maximum measurable
return; //engine speed at 500 Hz or 30,000 RPM (I believe this is a non-issue :D )
}
TimeOld = Time; //if the engine is still running update "TimeOld" for comparison next time
if(j == 0)
{
StartTime = Time; //sets starttime time stamp if its the first event of this sample set
}
else
{
EndTime = Time; //updates endtime time stamp for all subsequent events
}
j++; // j is controlled from the UpdateServo function
return;
}
That ISR is called in my UpdateServo function I posted.
So that's how I'm controlling the servo and measuring my generator's engine speed.
The way I'm correcting for speed error like you said is just using a proportional controller. This is because the servo takes time to acquire its new position. I then have to wait (a very small amount of time) for the engine to respond to the new throttle position. I use the delays after my UpdateServo function finishes to account for this. I've got it waiting half a second after each correction at the moment.
Anyway my original question had more to do with these calculations in the UpdateServo function:
/* How everything is being declared
unsigned int NewServoPos = ServoPos; //ServoPos is the current servo position being used by the servo controlling ISRs
long intermidiate = 0;
unsigned long Period;
long Error;
unsigned long Freq;
unsigned long StartTime = 0;
unsigned long EndTime = 0;
*/
//The calculations...
Period = EndTime - StartTime;
Freq = 16000000UL/Period;
Error = (240L - Freq)/8;
intermidiate = NewServoPos;
intermidiate += Error;
NewServoPos = intermidiate;
I'm concerned with what I get when " 240L - Freq " results in a negative number and what dividing that by 8 will yield. Does this look like it would come out right?
Also, full code attached. Commenting may vary, working on it.
Yeah_Finished_1.0.0 (6.52 KB)