[PIC] led dimmer with PWM giving wrong math answer

[Kfir Maymon]

i`m building led dimmer with PWM for some reason i get wrong math answer,

unsigned short  duty1=127;    // pwm duty cycle
unsigned long temp=0;
unsigned int  adcval1=0;      //ADC 10 bit value
char ADCTXT[7],TEMPTXT[11];   //debug string
char DUTYTXT[5];              //debug string

void init(){
CM1CON0=0x07;
ANSEL=0x0f;
TRISC=0x00;
TRISB=0xf0;
}


void main() {
init();
UART1_Init(19200);
ADC_Init();
PWM1_Init(20000);
PWM1_Start();
PWM1_Set_Duty(duty1);

while(1){
adcval1 = ADC_Read(0);   // read ADC value
delay_ms(100);
   IntToStrWithZeros(adcval1, ADCTXT);
   UART1_Write_Text("ADC: ");
   UART1_Write_Text(ADCTXT);
   UART1_Write_Text("\r\n");
   temp = (adcval1 * 254)/1024;   // make adc value fit short to duty cycle
duty1= temp;
   LongWordToStrWithZeros(temp,TEMPTXT);
   UART1_Write_Text("TEMP ");
   UART1_Write_Text(TEMPTXT);
   UART1_Write_Text("\r\n");
   ShortToStr(duty1,DUTYTXT);
   UART1_Write_Text("duty:");
   UART1_Write_Text(DUTYTXT);
   UART1_Write_Text("\r\n");
PWM1_Set_Duty(duty1);
delay_ms(1000);
}
}

when ADC is max (1023)
i get this, temp = (adcval1 * 254)/1024 = 61 it cant be.

 

[KlausST]

Hi,

1022 x 254 = 259588

but thats too big for a 16 bit value.

overflow happens and 62980 remains in the 16 bit value.

this is divided by 1024.

the reuslt is 61.5039

The integer value is 61.

**********
i don´t know why you multiply with 254.

254/1024 is about 1/4. thus dividing by 4 gives almost the same result as * 254/1024, but without overflow.
or two bits shift right.


Klaus

 

[Kfir Maymon]

i need number between 0-255 for the duty cycle 0% to 100%,
i am use "unsigned long temp=0;"
i am trying to debug it and doing just this code:

   temp = (1020 * 254);   // make adc value fit short to duty cycle
   temp= temp/1024;
   duty1 = temp;

now i getting duty1= -2

for some reason he don't do above 127 Although the i set: "unsigned short duty1=127;"

 

[KlausST]

Hi

temp = (1020 * 254)

here again the result is 259080 and does not fit into a 16 bit word.

as a result you will see 62472

then you divide it by 1024. the result is 61.

***********

try: duty = temp1 >> 2 ; just shift it two bits right.

Klaus

BTW: valid range for uint16 = 0...65535

 

[Kfir Maymon]

temp is a "unsigned long" its a 32 bit (0 to 2147483647)
i think my problem is with the "unsigned short"
if i write the code:
duty1= 255;
i get output
"duty1 = -3"

 

[FvM]

You should learn quite a bit about implicite and explicite type conversions in C and how to take advantage from it.
To get a correct result, you have to add a simple type cast. I'm assuming that long has a size of 32 bit, which may be not the case with some embedded compilers.

unsigned long temp;
unsigned adcval1;
temp = (adcval1 * 254)/1024;

changes to

 temp = ((unsigned long)adcval1 * 254)/1024;

The problem is that the type of an arithmetic expression is determined by the right hand side only, so to change the result of a 16bit * 16 bit multiply to 32 bit, you must convert one of both multiplicands to 32 bit.