Problems implementing digital pot control via button presses

Hi everyone,

I'm currently trying to control a digital pot, a MCP4261's resistance values via two tactile button switches. I got a crude implementation to work, except in incrementing the values, I can't get them to come back down properly. What I'm trying to do is limit the number of button presses to 20, so I'm using a constrain function, but I can't figure out how to lower the button presses for each button with the press of the other. Here's my code. I commented out the MCP part, that's not what's giving me problems. It's figuring out the code to go up the scale, and down the scale for the button presses, like a volume controller on a tiny mp3 player. I could sure use some help on this one.

/* This employs two buttons to turn on and off the LEDS, and to count the number of times
between each push. This is to later be incorporated with the MCP4261 Digital Pot to
control up and down movement of the resistance wiper. 
*MCP4251 Digital Potentiometer
  *CS to pin 10
  *SCK to pin 13
  *SDI to pin 11
  *LED attached to MCP4251's wiper 0
Good exercise
*/
#include <DigiPot.h>

const int pin0 = 0;        // setup the analog pin 0 as an input read out pin
int val = 0;               // set up the value for transmission to the MCP4251

// this constant won't change:
const int buttonPin1 = 2;  // Pin 2(nonPWM) is attached to the first button
const int buttonPin2 = 4;  // Pin 4 (nonPWM) is attached to the second button
const int ledPin1 = 5;     // the pin that the LED is attached to
const int ledPin2 = 6;    // the pin that LED 2 is attached to


// Variables will change:
int buttonPushCounter1 = 0;   // counter for the number of button1 presses
int buttonPushCounter2 = 0;  // counter for the number of button2 presses
int potlevel = 0;            // This is the counter number that's added to or subtracted from for the digital pot
int steplevel = 0;          // The pot's step value from buttonPushCounter1

int button1 = 0;
int button2 = 0;

int buttonState1 = 0;         // current state of button1
int buttonState2 = 0;          // current state of button 2

int lastButtonState1 = 0;     // previous state of button 1
int lastButtonState2 = 0;     // previous state of button 2

int potval = 0;               // the value initial of the digital pot

DigiPot mcp4251(10, 13, 11); //Start a new instance called "mcp4251"
//format: DigiPot <instance>(CS, SCK, SDI), and connect it to the digital outputs of
// 10, 13, and 11 from the Arduino board.

void setup() {
  // initialize the button pins as a input:
  pinMode(buttonPin1, INPUT);
  pinMode(buttonPin2, INPUT);
  
  // initialize the LED as an output:
  pinMode(ledPin1, OUTPUT);
  pinMode(ledPin2, OUTPUT);
  
  // initialize the digital pot
  mcp4251.clear(0); //clear both pots; they default to 127
  mcp4251.clear(1); //format: <instance>.clear(pot)....pot is either 0 or 1
  
  // initialize serial communication:
  Serial.begin(9600);
}


void loop() {
  
  val = analogRead(pin0);  // read the input from digital pot wiper 0
  
  // read the pushbutton input pin:
  buttonState1 = digitalRead(buttonPin1);
  buttonState2 = digitalRead(buttonPin2);

  // compare the buttonState to its previous state
  if (buttonState1 != lastButtonState1) {
    // if the state has changed, increment the counter
    if (buttonState1 == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      digitalWrite(ledPin1, HIGH);    // And let's indicate the button's being pushed on the led
      buttonPushCounter1 = constrain(buttonPushCounter1++, 1, 20);  // And increment the counter, but constrain it
    } 
    else {
      digitalWrite(ledPin1, LOW);    // Turn it off when released
    }
  }
  // save the current state as the last state, 
  //for next time through the loop
  lastButtonState1 = buttonState1;

  // compare the buttonState2 to its previous state
  if (buttonState2 != lastButtonState2) {
    // if the state has changed, increment the counter
    if (buttonState2 == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      digitalWrite(ledPin2, HIGH);    // Let's indicate the LED on when button 2 is pressed
      buttonPushCounter2 = constrain(buttonPushCounter2++, 1, 20);
    } 
    else {
      digitalWrite(ledPin2, LOW);    // And shut it off when it's off.
    }
  }
  // save the current state as the last state, 
  //for next time through the loop
  lastButtonState2 = buttonState2;

  if (buttonPushCounter1 > buttonPushCounter2) {
    buttonPushCounter1 = buttonPushCounter1 - buttonPushCounter2;
  } else {
    buttonPushCounter2 = buttonPushCounter2 - buttonPushCounter1;
  }
  Serial.println(buttonPushCounter1);  
 
// Serial.println(potlevel);
 
//  if (potlevel < 1) {
//    steplevel1 = 1;
//  } else {
//    steplevel1 = potlevel;
//  }
//  Serial.println(steplevel1);
//  int potval = map(steplevel1, 3, 1023, 1, 255);  // map the values from 
  // the steplevel1 which is 0-1023
  // re spread them across 0-255, which is the amount the 
  // mcp4251 can process
  
//  mcp4251.write(0, potval);  // send the value to the digital pot
// Serial.println(val);
 
  
}

That seems overly complex to my mind.

Why not just increment or decrement a value and then constrain that value?

Something along the lines of, for example (untested, just written in here):

void loop()
{
    static int myValue;
    static unsigned char button1State=LOW;
    static unsigned char button2State=LOW;
    bool changed = false;

    if(digitalRead(buttonPin1)!=buttonState1)
    {
        buttonState1 = digitalRead(buttonPin1);
        if(buttonState1 == HIGH)
        {
            myValue++;
            changed = true;
        }
    }

    if(digitalRead(buttonPin2)!=buttonState2)
    {
        buttonState2 = digitalRead(buttonPin2);
        if(buttonState2 == HIGH)
        {
            myValue--;
            changed=true;
        }
    }

    if(changed==true)
    {
        myValue = constrain(myValue,0,20);
        sendTheValueToThePot(myValue);
    }
}
    if(digitalRead(buttonPin1)!=buttonState1)
    {
        buttonState1 = digitalRead(buttonPin1);

Wrong. The two calls to digitalRead do not necessarily return the same value.

        if(buttonState1 == HIGH)

Whether HIGH represents pressed or released depends on how the switches are wired. OP, how are your switches wired? You are not using the internal pullup resistors, so you must have external pullup or pulldown resistors. Do you? What kind?

Wrong. The two calls to digitalRead do not necessarily return the same value.

As I said - untested - just something I threw together on here.

Whether HIGH represents pressed or released depends on how the switches are wired.

This is just for illustration purposes - it's to give a general idea of the method - it's not meant to be drop-in code.

As I said - untested - just something I threw together on here.

Understood. But, in the future, please take a little care to no introduce subtle errors like this. Newbies have enough trouble with tried and tested code. The concept is fine. Just needs a little work, that you or I could do, but I'm not sure about OP.

This is just for illustration purposes - it's to give a general idea of the method

Sure. My comment was really directed at OP. Often posters come in assuming that all their problems with reading from switches is software related, when the real problem is that the switch is not wired correctly. I'm trying to make sure that we are addressing a software issue, rather than a hardware issue.

PaulS:

    if(digitalRead(buttonPin1)!=buttonState1)
{
    buttonState1 = digitalRead(buttonPin1);


Wrong. The two calls to digitalRead do not necessarily return the same value.



    if(buttonState1 == HIGH)


Whether HIGH represents pressed or released depends on how the switches are wired. OP, how are your switches wired? You are not using the internal pullup resistors, so you must have external pullup or pulldown resistors. Do you? What kind?

The resistors and the switch are wired like so..


I'm using 10kohm resistors.

Uh, what's a good resource that explains pullup and pulldown resistors?

What I did was I took the button state change detection example, modified it, and merged it with some of my own code with the MCP. Yes, I'm still learning.

Essentially, I want to set up a range of button presses, 20 maximum. So you have two buttons to press. As you go up in range, the MCP increases it's resistance load. As you go down in range, it decreases its resistance load. If you try to go above 20, the max limit is 20. If you go below 1, the min limit is 1. No matter how many times the buttons are pressed, when at either 1 or 20, the moment the opposing button is pressed, the main value that gets sent to the digital pot is either 1 + 1, or 20 - 1. And then of course that gets mapped to the MCP's 1 to 255 range.

You can tell that what's giving me problems is
A.) Establishing the max and min limit
B.) Somehow getting the button press counter to either reset or respond so that in the scale of 1-20, it can simply lower the scale. I did a serial print of the buttonpress counter, and it goes all the way up past 20, so I initially tried subtracting the values between the two counters, but got negative values (obvious), and an abs function gives you reverse values.

Very interesting problem. I'm not an experienced programmer by any means.

Hi there.

Try to read Lady Ada's tutorial on switches, it explains a couple of important issues very well (pull up / down and switch debouncing)

http://www.ladyada.net/learn/arduino/lesson5.html

Then try to do as suggested above use only one counter and have one button increase it and the other decrease it, that will be much simpler to deal with.

Shouldn't be too complicated, but you need to understand pull up / down resitors and switch debouncing first, that is very important.