I am trying to create a program with a pic to:

output a PWM signal that increments by 25% to 75% if right button is pressed.

output a PWM signal that decrements by 25% to 0% if left button is pressed.

I have done the inc/decrement part but a static variable that holds the present value isn't working so after a short delay the PWM% goes back to 0%.

I don't fully understand the static class specifier and I'm not sure if the flow of the program is correct. Can someone tell me what I'm doing wrong?

The code is below:

#include <stdio.h>
#include <stdlib.h>
#include <pic16f1939.h>
#include <xc.h>
#define _XTAL_FREQ 1000000

typedef unsigned char uint_8;
typedef unsigned int uint_16;
typedef unsigned short long int uint_24;
typedef unsigned long int uint_32;

#define left PORTAbits.RA0
#define right PORTAbits.RA1
#define dl __delay_ms(100)

//dcyc_0() to set initial PWM to 0%
void dcyc_0();

//dcyc_left() to decrease PWM by 25% if PWM is not 0%
uint_8 dcyc_left(uint_8 dc_rtn_value);

//dcyc_right() to increase PWM by 25% if PWM is not 75%
uint_8 dcyc_right(uint_8 dc_rtn_value);

//A static value is set to zero


void main()
{
    //PWM (CPP1) pin is set
    PORTA = 0x00;
    LATA = 0x00;
    ANSELA = 0x00;
    TRISA = 0x03;

    while(1)
    {      
        while(left == 0 && right == 0)
        {
        /*static dc_rtn_value is zero
          until right is called and dc_rtn_value
          is incremented by 25 and the PWM value
          for CCPR1L is 25
        */
        static uint_8 dc_rtn_value = 0;
        TRISC = 0xFB;
        CCPTMRS0bits.C1TSEL = 0b00;
        PR2 = 100;
        CCP1CON = 0x3C;
        CCPR1L = dc_rtn_value;
        PIR1bits.TMR2IF = 0;
        T1CONbits.T1CKPS = 0b10;
        T2CONbits.TMR2ON = 1;
        while(PIR1bits.TMR2IF == 0);
       
        /*if left is pressed call dcyc_left()
          send dc_rtn_value to it
          if dc_rtn_value is zero add zero and return zero
          if not decrement dc_rtn_value by 25             
        */
        if(left == 1)
        {
            dc_rtn_value = dcyc_left(dc_rtn_value);
            dl;
        }
       
        /*if right is pressed call dcyc_right()
          send dc_rtn_value to it
          if dc_rtn_value is 75 add zero and return 75
          if not increment dc_rtn_value by 25             
        */
        else if(right == 1)
        {
            dc_rtn_value = dcyc_right(dc_rtn_value);
            dl;
        }
        }
    }

}

uint_8 dcyc_left(uint_8 dc_rtn_value)
{
    if(dc_rtn_value == 0)
    {
        dc_rtn_value = dc_rtn_value + 0;
    }
    else
    {
        dc_rtn_value = dc_rtn_value - 25;
    }
   
    return dc_rtn_value;
}

uint_8 dcyc_right(uint_8 dc_rtn_value)
{
    if(dc_rtn_value == 75)
    {
        dc_rtn_value = dc_rtn_value + 0;
    }
    else
    {
        dc_rtn_value = dc_rtn_value + 25;
    }
   
    return dc_rtn_value;
}

 

You need to go back and study a bit more on the C programming language. Look for global / local memory use.

 

I've looked at some examples using static and can't figure it out. I tried something simple like:

static int counter = 1;

void main()
{

while(1)
    {
       
        PORTD = counter;
        if(button == 1)
        {
            ++counter;
            if(counter==4)
            {
                counter = 1;
            }
           
        }
    }
}

but it still hasn't worked.

 

When you define a variable as static you are telling the compiler you want it to maintain the value between times you run some routine, so the variable is stored in some permanent place.

Since you never leave the main() function defining a main() variable as static has no effect.

 

A static variable defined outside of any function has it's scope limited to the file in which it is defined. It is completely unknown to any functions defined in other files or in libraries.
A static variable defined inside a function keeps it's value between invocations of the function, and is local to that function.

https://en.wikipedia.org/wiki/Static_variable

 

I did try and put it just before PORTD = counter; but it didn't work.

 

In your simple example, it is not clear to me what doesn't work. Could you elaborate?

 

I thought the program would:

set a static variable initially to 1
set PORTD = counter as 1 (binary 00000001) so an led on PORTD bit 0 is on
if button is pressed counter is incremented to 2 (binary 00000010)
when button is released PORTD = counter is 2 so an led on PORTD bit 1 is on
if button is pressed again counter is incremented to 3 (binary 00000011)
when button is released PORTD = counter is 3 so an led on PORTD bit 0 and bit 1 is on

but once button is released counter goes back to 1.

 

Are you de-bouncing the button? Each time it is depressed or released a fast running program can detect multiple close release cycles.

http://web.engr.oregonstate.edu/~traylor/ece473/lectures/debounce.pdf

 

Even if debounced you wrote code to continuously increment the variable as long as the button was pressed.

You might want to think of alternatives such as "stop here and wait until the button is released."

 

I have debounce code in now but the counter returns to 1.

 while(1)
    {
        
        PORTD = counter;
        if(button == 1)
        {
          
            ++counter;
            if(counter==4)
            {
                counter = 1;
            }
            for(int i=0; i<=10; i++)
            {
                dl_1; //delay is 100ms
                if(button==0)
                {
                    int i = 0;
                }
            }
        }
    }
}

 

I do not see anything in that code that will debounce either the button press or the button release, and you need both.

I also do not see anything to stop counting over and over for a single press.

Why do you define a variable "i" twice? Does this code actually compile?

I strongly suggest you stop writing C and write the steps in words, in detail, then translate the words into C.

 

I don't see anything in the simple code that defines what button is or that it is even testing for a button press anywhere? There must be additional code missing... Same for the debounce which you claim you are doing. Without something changing "button", your "if" statement will not get executed and counter will not get changed.

As far as the dual definition of "i" is concerned, IMHO each declaration refers to a different object.
"Each declaration of an identifier with no linkage denotes a unique entity."
...and hence changing "i" in the "if" block, does not affect the loop index. The following sample (Arduino) code illustrates this.

void setup() {
  // put your setup code here, to run once:
  Serial.begin(9600);
  int button =0;
  int j =-1;
              for(int i=0; i<=10; i++)
            {
                 j++;
                button=1-int(i/4);

                if(button==0)
                {
                    int i = 0;
                    Serial.print("i changed to ");
                    Serial.println(i);
                }
                Serial.print("i,j ");
                Serial.print(i);
                Serial.print(",");
                Serial.println(j);
            }
}

void loop() {
}

This program produces the following output:

i,j 0,0
i,j 1,1
i,j 2,2
i,j 3,3
i changed to 0
i,j 4,4
i changed to 0
i,j 5,5
i changed to 0
i,j 6,6
i changed to 0
i,j 7,7
i,j 8,8
i,j 9,9
i,j 10,10

 

There is code missing to make the counter hold after each press but I'm not sure what.

button is a pull down tactile switch on PORTA0 set as a digital input so when pressed button == 1.

My thinking was:

after static int counter was set to 1
enter the while(1) loop
as counter was set to 1, PORTD = counter = 1(00000001) so PORTD0 = 1 and remain at 1 until button is 1
when button is pressed counter is incremented so now 2(00000010) so PORTD1 = 1
a small delay occurs and button = 0 then leaves the if block so PORTD = counter = 2(00000010) so PORTD1 = 1 and remain at 1 until button is 1 again.

In my mind I know what I want to do and think the steps are right but obviously not. I need help with thinking in steps in C and then actually programming in C.

 

Your thinking is a curious mix of English and C. Do try to think in one or the other. Also don't mix "button pressed" with "button is one." Just use"pressed" and "unpressed" so you can use equate statements to make the thinking and logic clear and bullet proof.

While I prefer to read and debounce my buttons in a background interrupt routine it is simpler to do it inline with direct code(or function), say something like this:

unsigned char GetButton(void)
{
  unsigned char last = ! button;
   while ( last != button)
  {
    last = button;
    // delay here for about 25 to 50 mS
  }
  return last;
}

This keeps you stuck in this loop until the button is stable.

 

The code I've used is doing what thought it would do without holding the incremented value. So I can keep pressing it and it will continuously change and light up led1(PORTD1) then both(PORTD0,D1) then back to led2(PORTD0). But when both are turned on, after the delay the counter goes back to 1 and led2(PORTD0) is on.

I was thinking that if the static value has been incremented from 1 to 2 in the if block statement then it is exited when button == 0 then PORTD = counter would be PORTD = 2 and remain as 2 until button == 1.

 

Your code is pretty confusing.
Looks like you don't understand how and when to declare variables.
You don't need a static variable to use.
I have made my text a different font.

while(1)
{

PORTD = counter;
if(button == 1) // will only go inside when 1​

{

++counter;
if(counter==4)
{

counter = 1;​

}
for(int i=0; i<=10; i++) // delay 1 second
{

dl_1; //delay is 100ms​

if(button==0) // button released during this second
{

int i = 0; // Huh?, whats this do again? Since you have a new i I think nothing.​

}​

}​

}
} // end of while(1)

 

OK, the system here changed the font so my comments are after the //.

 

Here is how I do button logic. Probably many other ways.

In the Header file I define the bit positions for buttons:
// MCP23S08 bits---------------------------
// Inputs
typedef union
{
UINT16 summary;
struct __PACKED
{
UINT8 LB; // Addr 00
UINT8 HB; // Addr 01
} byte;

struct __PACKED
{
unsigned :2; // b0,1
unsigned Time :1; // b2 S5
unsigned Set :1; // b3 S2
unsigned Pgm :1; // b4 S13
unsigned Fan :1; // b5 S9
unsigned Cool :1; // b6 S6
unsigned Heat :1; // b7 S3

unsigned Up :1; // b8 S8
unsigned Escape :1; // b9 S12
unsigned Left :1; // b10 S1
unsigned Right :1; // b11 S4
unsigned Down :1; // b12 S7
unsigned Enter :1; // b13 S11
unsigned :2; // b14,15
} bits;
} unionDigIn;
extern unionDigIn Button, ButtonDown;

Back to my comments:
Inside a function which gets called from my main loop
I use this code to detect a button press. The actual read of the button
bit is done in another function which reads the MCP23S08 device on SPI bus &
this can happen fairly often.
The buttons are de-bounced in hardware.
This is only one button
if( Button.bits.Fan ) // Fan button pressed - can come here lots while looping
{
if( ButtonDown.bits.Fan == 0 ) // This flag will prevent multiple access during one key press.
{
ButtonDown.bits.Fan = 1; // Set the flag preventing a second access.
return FanKey; // FanKey will only get returned once during a single key press.
}
} else { ButtonDown.bits.Fan = 0; } // Now clear the flag as user
// has released the button.

 

so what is FanKey that is returned and where would counter be incremented?