Help with Simple Code Please!

Discussion in 'Embedded Systems and Microcontrollers' started by GammaRay86, Feb 18, 2010.

  1. GammaRay86

    Thread Starter Member

    Dec 9, 2007
    Hey guys,

    I'm having some trouble with some math code used for my PIC16F684.

    Code ( (Unknown Language)):
    1. int16 PV, SP;
    2. int16 E0;
    3. uns8 gain;
    4. int16 output;                          
    5. uns16 temp;
    7. SP = 128;
    9. PV = ADRESH;    // ADC value = ADRESH (8 MSbs)
    11. E0 = PV - SP;
    13. output = gain*E0;
    15. if(output > 0){
    16.     P1M1 = 0;
    17.     temp = output;         
    18.     temp /= 4;      // Shift bits 3 to 10 of temp twice to the right
    19.     CCPR1L = temp;
    20.     }
    22. if(output < 0){
    23.     P1M1 = 1;          
    24.     temp = -output;
    25.     temp /= 4;      // Shift bits 3 to 10 of temp twice to the right
    26.     CCPR1L = temp; 
    27.     }
    Don't worry about the A/D conversion, I just deleted the other code. The problem I'm having is that I don't believe the second if statement is working properly. I want to take the absolute value of "gain" but my compiler doesn't have the absolute function. Instead I just put "-gain" which compiles fine and I think works.

    When output > 0, the PIC outputs a PWM square wave which shows up fine on the oscilloscope. It's when output should be < 0, the output on the oscilloscope is a very strange square wave, that looks noisy and has weird superimposing square waves. I think the problem is with the math though.

    Can someone verify if the way I initialized the variables will properly store data? I get confused when I'm doing math with mixes of signed and unsigned numbers.

    I'd appreciate the help a lot. Thanks!
  2. russ_hensel

    Distinguished Member

    Jan 11, 2009
    Is there a loop somewhere?
    I always like to explicitly declare variables as signed or unsigned.
    I do not see gain ever asigned to anything.
  3. GammaRay86

    Thread Starter Member

    Dec 9, 2007
    Gain is just given an integer value at the start. I did not put the whole code up as it would just make it harder to see and is irrelevant to the problem.

    For example: gain = 2;

    My concern is that because i'm using a mix of signed and unsigned integers, the sign bit might be getting lost somewhere, or even worse used as a bit that is placed inside a register such as CCPR1L.

    Code ( (Unknown Language)):
    2. int16 temp;
    4. if(output < 0){
    5.     P1M1 = 1;          
    6.     temp = -output;
    7.     CCPR1L = (unsigned char)temp/4;
    8.     }
    Would the code above guarantee that CCPR1L only gets the MSBs without the sign bit? For example, if temp is 10 bits of data + 1 sign bit (11 bits), would the CCPR1L register be given bits 2-9 (disregarding the 10th bit)? Hope this ain't too confusing lol
  4. russ_hensel

    Distinguished Member

    Jan 11, 2009
    temp is a 16 bit value, so the cast could fail if the value is too big. I am not certain what happens if it it negative. Some of this depends upon how temp is assigned.
    If you are not sure how big it is why not test it in code with at least some sort of assert?
    By fail, i mean do something you do not expect, it will not directly produce a run time error.
  5. GammaRay86

    Thread Starter Member

    Dec 9, 2007
    I simulated the code and the CCPR1L values are not correct in the (output < 0) part of the code. There is definitely something wrong with them but I can't figure it out. The maximum value for temp is about + or - 256, so no more than 10 bits can be placed inside temp. CCPR1L is only an 8 bit register so that is why I only want the 8 MSBs of temp.