PWM on PIC18f6720

[PacSea]

Hi there,
we are trying to run two motors using the PIC18f6720 prototyping board from CCS. We are using the PWM at around 50% duty cycle but somehow one of the motors speeds up for no reason.
Here's the code
Can somebidy help us out...

[/code]
#include <18F6720.h>
#use delay(clock=20000000)
#fuses HS,NoLVP,NoWDT,PUT
#use rs232 (baud=9600, parity=N,xmit=PIN_c6,rcv=PIN_c7,stream=port1)

unsigned long value1,value2;

#define PUSH_BUTTON PIN_A4

long int optodetect1=0,optodetect2=0;
int obit6, obit7;

int i=0;

float duty1,duty2;

void l_bck();
void l_fwd();
void r_bck();
void r_fwd();
//////////////////////////////////////////////////////

#int_timer1
void clock_isr()
{

//if(duty2==duty1)
//{
//printf("\n\r duty2 is duty1");

if(optodetect2<optodetect1)
{
duty2=duty2+0.01;
printf("\n\rduty2=%f",duty2);
optodetect1=0;
optodetect2=0;
}

if(optodetect2>optodetect1)
{
duty2=duty2-0.01;
printf("\n\rduty2=%f",duty2);
optodetect1=0;
optodetect2=0;
}
//}
optodetect1=0;
optodetect2=0;
//set_timer1(56160);
set_timer1(49911);
}

/////////////////////////////////////////////////////
#INT_RB

void OptoDetect_isr() {
int value,ovalue,nvalue;
int nbit7,nbit6;

obit7=nbit7;
obit6=nbit6;
value = input_b(); //read optodetect inputs

nbit7= value & 0x80; // B8 mask unwanted bits
nbit6= value & 0x40; //B5 mask unwanted bits

if (nbit7 != obit7)
{
optodetect1=optodetect1+1;
printf("\n\roptodetect1 is %ld", optodetect1);
}

if (nbit6 != obit6)
{
optodetect2=optodetect2+1;
printf("\n\rOPTODETECT2 is %ld", optodetect2);
}

//delay_ms(5);//++++++++++++++++DELAY

/*
//if(duty2==duty1)
//{
//printf("\n\r duty2 is duty1");

if(optodetect1<optodetect1)
{
duty1=duty1+0.1;
printf("\n\rduty1=%f",duty1);
optodetect1=0;
optodetect2=0;
}

if(optodetect1>optodetect2)
{
duty1=duty1-0.1;
printf("\n\rduty1=%f",duty2);
optodetect1=0;
optodetect2=0;
}
if(optodetect2==optodetect1)
{
duty1=duty2;
}
*/
delay_ms(5);//++++++++++++++++DELAY
//}

}
///////////////////////////////////////////////////////
void l_bck()
{
//unsigned long value1,value2;

setup_ccp1(CCP_PWM);//wire5
setup_ccp2(CCP_OFF);//wire7
setup_timer_2(T2_DIV_BY_16,207,1);
value1=(float)(duty1/1500)*20000000/16;//duty value

set_pwm1_duty(value1);//wire5
set_pwm2_duty(0);//wire7

output_low(pin_c1);//wire7
output_low(pin_d1);//wire6
output_high(pin_d2);//wire8
output_low(pin_d3);//wire4
}
////////////////////////////////////////////////////////
void l_fwd()
{
//unsigned long value1,value2;
printf("\n\n\n\nLEFT forward\r");
setup_ccp1(CCP_OFF);//wire5
setup_ccp2(CCP_PWM);//wire7
setup_timer_2(T2_DIV_BY_16,207,1);
value1=(float)(duty1/1500)*20000000/16;//duty value

set_pwm1_duty(0);//wire5
set_pwm2_duty(value1);//wire7

output_low(pin_c2);//wire5
output_high(pin_d1);//wire6
output_low(pin_d2);//wire8
output_low(pin_d3);//wire4
}
//////////////////////////////////////////////////////////
void r_bck()
{
//unsigned long value1,value2;
setup_ccp3(CCP_PWM);//wire5
setup_ccp4(CCP_OFF);//wire7
setup_timer_2(T2_DIV_BY_16,207,1);
value2=(float)(duty2/1500)*20000000/16;//duty value

set_pwm3_duty(value2);//wire5
set_pwm4_duty(0);//wire7

output_low(pin_g0);//wire7
output_low(pin_d4);//wire6
output_high(pin_d5);//wire8
output_low(pin_d6);//wire4
}
////////////////////////////////////////////////////////
void r_fwd()
{
//unsigned long value1,value2;
printf("\n\n\n\nrightFORWARD\r");
setup_ccp3(CCP_OFF);//wire5
setup_ccp4(CCP_PWM);//wire7
setup_timer_2(T2_DIV_BY_16,207,1);
value2=(float)(duty2/1500)*20000000/16;//duty value

set_pwm3_duty(0);//wire5
set_pwm4_duty(value2);//wire7

output_low(pin_g3);//wire5
output_high(pin_d4);//wire6
output_low(pin_d5);//wire8
output_low(pin_d6);//wire4
}
//////////////////////////////////////////////////////////
void main()
{
//unsigned long value1,value2;

set_tris_a(0xff);
set_tris_c(0);
set_tris_d(0);
set_tris_g(0);

enable_interrupts(INT_TIMER1);
enable_interrupts(INT_RB);
enable_interrupts(GLOBAL);

while (input(PUSH_BUTTON));
while (!input(PUSH_BUTTON));

setup_timer_1 ( T1_INTERNAL | T1_DIV_BY_8 );
//set_timer1(56160);
set_timer1(49911);
duty1=0.5;
duty2=0.5;
l_fwd();
r_fwd();

while (true)
{
/*
set_pwm1_duty(0);//wire5
set_pwm2_duty(value1);//wire7
set_pwm3_duty(0);//wire5
set_pwm4_duty(value2);//wire7
*/
l_fwd();
r_fwd();
}

disable_interrupts(global);

//delay_ms(2000);
output_high(pin_d3);//wire4
output_high(pin_d6);//wire4

}//end of main

[kender]

Is the speed of that motor up by much?

[PacSea]

yes the wheel speeds up to the max after a few normal runs. the left wheel is kept as a reference and the right wheel follows to to correct its speed.

[Ttelmah]

I really would consider making your 'duty' values integers, rather than floats. Use a 'scaled' value, that is a binary integer multiple of the working value required, then you can just use a shift, or even better a union to extract the bits required. The amount of time involved in the interrupt routine, adding 0.01 to a float, is significant, and then in the servo control loop, the arithmetic will be drinking time. You also take a float, perform floating point arithmetic on it, then cast the result to a float (unecessary), before transferring it into an integer.
Working with (say) a 32bit integer, then extracting just the high bytes for the timing value, will typically be perhaps 50* faster than using a float, if you get the scaling right. Also if divisions are kept to always be binary values, and all the rest of the arithmetic is additions, or multiplications, this will be a lot faster.
There will also be problems with testing values for equality using floats, if rounding errors start to appear, two sums that sould have the same result, may well only be 'close' to the same answer...

That having been said, I suspect your actual problem, is with rounding errors, and the use of 'unsigned'. If the value ends up for a 'stop', actually being -0.000001 (for example), and then this is converted to an unsigned, the result will be a 'full on' condition for the drive.

Best Wishes