Comparing two float values to a particular decimal place.

Using Arduino Programming Questions
1

**Found the problem. Thanks for the input.

I have a problem with trying to compare 2 float values with a if (value >=value) statement.

My goal is to compare 2 voltages(one that is from an analog pin, the other is a "set" voltage by 2 buttons) within a hundredth of a volt.

I cannot wrap my head around on how to complete this task and the result of the values not being accurately compared is a nasty loop that slows down my program to 1 complete cycle of void loop() every 5 seconds.

Heres my code:

/* WIGGLES' Universal VMod tool V0.2
  This program is used to control up to 6 different voltage rails up to 5V 
  utilizing a MCP42XXX or MCP41XXX series digital potentiometer. Currently 
  uses USB Console to view set and read voltages. -Wiggles5289, Overclock.net
  
    ISSUES:
  1: NEED TO CONVERT CHxVRead and CHxVtgt from 100ths to  without interference
    from logic in voltageAdjust
  2: Result of Issue 1 is that the program goes at an arduous rate of 1 complete cycle of void loop()
    every 5 seconds. adjustVoltage() is the cause of this, it should stop if only voltage values are
    compared in 100ths instead of near infinite.

  CHANGE LOG:
  Disabled printOut() as wrong or no values were being printed on console
  Added print function to readVoltage()
  
  This code used to control the digital potentiometer
  MCP4XXXX connected to  arduino Board
  CS >>> D10
  SCLK >> D13
  DI  >>> D11
  PA0 TO VCC
  PBO TO GND
  SHDN >> 9
  PW0 TO led with resistor 100ohm. JUST TO TEST.

  Thanks to all who helped, Ill be listing people here.
*/
const int buttonUpCH0Pin = 2;
const int buttonDownCH0Pin = 3;
const int buttonUpCH1Pin = 4;
const int buttonDownCH1Pin = 5;
const int sensor0Val = A0;
const int sensor1Val = A1;

int buttonUpCH0State = 0;
int buttonDownCH0State = 0;
int buttonUpCH1State = 0;
int buttonDownCH1State = 0;
int lastButtonUpCH0State = 0;
int lastButtonDownCH0State = 0;
int lastButtonUpCH1State = 0;
int lastButtonDownCH1State = 0;
int CH0VRead = 0;
int CH1VRead = 0;
float CH0Vtgt = 1; 
float CH1Vtgt = 1; 

#include <SPI.h>
byte addressPot0 = 0b00010001;      //To define potentiometer use last two BITS 01= POT 0
byte addressPot1 = 0b00010010;      //To define potentiometer use last two BITS 10= POT 1
byte addressPot0and1 = 0b00010011;  //To define potentiometer use last two BITS 10= POT 0 and 1
byte addressPotNA = 0b00000000; //To define no write to potentiometer use BITS 5 and 6 or last two BITS= 00
byte CS= 10;    //Chip control goes to pin 10
byte SHDN = 9;  //Chip SHUTDOWN - PIN 9
byte RS = 8;    //Chip RESET - PIN 8

void setup()
{
  pinMode (CS, OUTPUT); //CS - When High, sets chip to read the data.
  pinMode (SHDN, OUTPUT); //CS - When High, sets chip to read the data.
  pinMode (RS, OUTPUT); //CS - When High, sets chip to read the data.
  pinMode(buttonUpCH0Pin, INPUT);
  pinMode(buttonDownCH0Pin, INPUT);
  pinMode(buttonUpCH1Pin, INPUT);
  pinMode(buttonDownCH1Pin, INPUT);
  
  Serial.begin(9600); 
  digitalWrite(SHDN, HIGH); //Power ON (HIGH)
  digitalWrite(RS, HIGH); //Power NO RESET (LOW)
  SPI.begin();

  delay(50); // Increase to 3000 to allow for breakout boards to initialize or external device boot
}

void loop(){
  setVoltageCH0(); // Sets voltage via 2 buttons
  setVoltageCH1(); // Sest voltage via 2 buttons but will be changed to 1 set and a single "Channel Select" button
  readVoltage(); // Reads voltage from Analog pins
  voltageAdjust(); // Determines if the voltage needs to be changed based upon read voltage
 
  delay(10); //Delay to allow Pots to adjust voltage feedback.
}


void setVoltageCH0() {
  // Sets resistance up or down from 100 to 210 with 2 buttons:
  buttonUpCH0State = digitalRead(buttonUpCH0Pin);
  
  if (buttonUpCH0State != lastButtonUpCH0State) {
    if (buttonUpCH0State == HIGH) {
      Serial.println("UP CH0"); // REMOVE POST DEBUG
        if (CH0Vtgt >= 1.00 && CH0Vtgt < 2.10){
          CH0Vtgt += 0.01;
          }
    } else {
      Serial.println("UP CH0 Released"); // REMOVE POST DEBUG
    }
    delay(5);
  }
  
  buttonDownCH0State = digitalRead(buttonDownCH0Pin);

  if (buttonDownCH0State != lastButtonDownCH0State) {
    if (buttonDownCH0State == HIGH) {
      Serial.println("DOWN CH0"); // REMOVE POST DEBUG
        if (CH0Vtgt > 1.00 && CH0Vtgt <= 2.10){
          CH0Vtgt -= 0.01;
          }
    } else {
      Serial.println("Down CH0 Released"); // REMOVE POST DEBUG
    }
    delay(5);
  }
      Serial.print("Channel 1 Target: ");
      Serial.print(CH0Vtgt);
      Serial.print("\t");
  
  lastButtonDownCH0State = buttonDownCH0State;  
  lastButtonUpCH0State = buttonUpCH0State;
}

void setVoltageCH1() {
  // Sets resistance up or down from 100 to 210 with 2 buttons:
  buttonUpCH1State = digitalRead(buttonUpCH1Pin);

  if (buttonUpCH1State != lastButtonUpCH1State) {
    if (buttonUpCH1State == HIGH) {
      Serial.println("UP CH1"); // REMOVE POST DEBUG
        if (CH1Vtgt >= 1.00 && CH1Vtgt < 2.10){
          CH1Vtgt += 0.01;
          }
    } else {
      Serial.println("UP CH1 Released"); // REMOVE POST DEBUG
    }
    delay(5);
  }
  
  buttonDownCH1State = digitalRead(buttonDownCH1Pin);

  if (buttonDownCH1State != lastButtonDownCH1State) {
    if (buttonDownCH1State == HIGH) {
      Serial.println("DOWN CH1"); // REMOVE POST DEBUG
        if (CH1Vtgt > 1.00 && CH1Vtgt <= 2.10){
          CH1Vtgt -= 0.01;
          }
    } else {
      Serial.println("Down CH1 Released"); // REMOVE POST DEBUG
    }
    delay(5);
  }
      Serial.print("Channel 2 Target: ");
      Serial.print(CH1Vtgt);
      Serial.print("\t");
  
  lastButtonDownCH1State = buttonDownCH1State;  
  lastButtonUpCH1State = buttonUpCH1State;
}

void readVoltage() {
  // Reads voltage from Analog Pins
  {
    int sensor0Val = analogRead(A0);
    float CH0VRead = sensor0Val * (5.0 / 1023.0); 
    Serial.print("Channel 1 Voltage:    ");
    Serial.print(CH0VRead); 
    Serial.print("\t");
  }
  {
    int sensor1Val = analogRead(A1);
    float CH1VRead = sensor1Val * (5.0 / 1023.0);
    Serial.print("Channel 2 Voltage:    ");
    Serial.println(CH1VRead); 
  }
  //Duplicate or simplify above code for up to 6 channels
  delay(10); //delay between readys for Pot adj CAN BE REMOVED
}

void voltageAdjust(){
  // Adjusts Voltage up or down
if (CH0VRead != CH0Vtgt || CH1VRead != CH1Vtgt) { 
  if (CH0VRead > CH0Vtgt && CH0VRead != CH0Vtgt) { 
    voltageDecrease(addressPot0);
  } else {
    voltageIncrease(addressPot0);
  }
  
  if (CH1VRead > CH1Vtgt && CH1VRead != CH1Vtgt) {
    voltageDecrease(addressPot1);
  } else {
    voltageIncrease(addressPot1);
  }
  
  delay(10); //Delay to allow for voltage feedback
 }
}

void voltageDecrease(byte address){
  // Decreases Voltage increasing resistance 1/255 away from Terminal A(Pin PAx)
   for (int i = 0; i <= 255; i++){
    digitalPotWrite(i, address);
    delay(10);
   }
}

void voltageIncrease(byte address){
   // Increases Voltage by decreasing resistance 1/255 towards Terminal A(Pin PAx)
  for (int i = 255; i >= 0; i--){
    digitalPotWrite(i, address);
    delay(10);
  }
}

int digitalPotWrite(byte value, byte address)
{ //SPI Write program. Single Chip, 2 Channels.
  digitalWrite(CS, LOW); //Set Chip Active
  SPI.transfer(address);
  SPI.transfer(value);
  digitalWrite(CS, HIGH); //Set Chip Inactive
}
2

If 'CH0VRead' is larger than 'CH0Vtgt', then it's very unlikely to be equal to it, isn't it? :-

if (CH0VRead > CH0Vtgt && CH0VRead != CH0Vtgt)

So you only need:-

if (CH0VRead > CH0Vtgt)

The whole function would look like:-

void voltageAdjust()
{
    // Adjusts Voltage up or down
    if (CH0VRead != CH0Vtgt || CH1VRead != CH1Vtgt)
    {
        if (CH0VRead > CH0Vtgt)
            voltageDecrease(addressPot0);
        else
            voltageIncrease(addressPot0);

        if (CH1VRead > CH1Vtgt)
            voltageDecrease(addressPot1);
        else
            voltageIncrease(addressPot1);
        delay(10); //Delay to allow for voltage feedback
    }
}

I've never used a digital pot, but looking at your 'voltageIncrease()' and 'voltageDecrease()' functions, do you really mean to write all values between 0 and 255 or 255 and 0 each time you try to increase/decrease the voltage?
Won't that just run through all values each time, leaving the pot value at each respective end limit? It won't do what your comment says - increase/decrease the value by 1/255.

And it's actually those functions taking up much of the time, since you do 256 writes each time, with a 10ms delay -> 2560ms

While it doesn't affect the actual operation of your program, there are two extra sets of unnecessary braces in 'readVoltage()', too.

3

I've never used a digital pot, but looking at your 'voltageIncrease()' and 'voltageDecrease()' functions, do you really mean to write all values between 0 and 255 or 255 and 0 each time you try to increase/decrease the voltage?
Won't that just run through all values each time, leaving the pot value at each respective end limit? It won't do what your comment says - increase/decrease the value by 1/255.

While it doesn't affect the actual operation of your program, there are two extra sets of unnecessary braces in 'readVoltage()', too.
[/quote]

I literally just figured this out before seeing you post this. The pot was going through the entire 0 to 255 stepping. I used the code from a fade using the digital pot and it didn't occur to me to change the code to a single increment per loop.

As for the extra brackets, the code is written to be easily duplicated if more voltage read channels are added. My target audience are hardware savvy individuals but unfamiliar to coding.

4

Don't forget to make the other change, too.
The second half of this is totally useless:-
(No point in having the micro do more work than it needs to. :slight_smile: )

if (CH0VRead > CH0Vtgt && CH0VRead != CH0Vtgt)

Oh, I almost forgot to mention, you can make all of these 'const' too, to save on SRAM space:-

byte addressPot0 = 0b00010001;      //To define potentiometer use last two BITS 01= POT 0
byte addressPot1 = 0b00010010;      //To define potentiometer use last two BITS 10= POT 1
byte addressPot0and1 = 0b00010011;  //To define potentiometer use last two BITS 10= POT 0 and 1
byte addressPotNA = 0b00000000; //To define no write to potentiometer use BITS 5 and 6 or last two BITS= 00
byte CS = 10;   //Chip control goes to pin 10
byte SHDN = 9;  //Chip SHUTDOWN - PIN 9
byte RS = 8;    //Chip RESET - PIN 8

Could be:-

const byte addressPot0 = 0b00010001;      //To define potentiometer use last two BITS 01= POT 0
const byte addressPot1 = 0b00010010;      //To define potentiometer use last two BITS 10= POT 1
const byte addressPot0and1 = 0b00010011;  //To define potentiometer use last two BITS 10= POT 0 and 1
const byte addressPotNA = 0b00000000; //To define no write to potentiometer use BITS 5 and 6 or last two BITS= 00
const byte CS = 10;   //Chip control goes to pin 10
const byte SHDN = 9;  //Chip SHUTDOWN - PIN 9
const byte RS = 8;    //Chip RESET - PIN 8

It also prevents you from accidentally changing them in your code - the compiler will throw a warning.

5

In your readVoltage function, your declaration of CH0VRead and CH1VRead hides the declarations outside the function. Furthermore, your main declaration is an int rather than a float, which is a bit odd.

6

I have refactored your code by creating a 'channel' object with constants for the pins it works with. This will permit you to create any number of channels. The the definition of the 'channel' array down the bottom to see how this is done.

The debugging code uses the sensor pin number to indicate where the message is coming from, as the chgannels dont 'know' which channel they are.

Obvious bugs fixed - variable hiding, inappropriate types.

10ms delay moved into digitalPotWrite()

I left digitalpotwrite out of the Channel class on a hunch - you have this addressPot0and1 thing which seems to indicate that you can talk to all the post you have simultaneously. A function that can do that doesn't belong in the class.

I did not touch the extremely odd voltageIncrease/Decrease methods. I think some comments above adress them.

To refactor this further, you could pull 'button' out into a class and get rid of duplicate code in setVoltage().

You set the voltage by adding/subtracting 0.01. This is a little unsafe, as floating point values are not accurate. I'd suggest using an integer calibrated in millivolts - mVtgt. (Always include the unit in the variable name, btw :slight_smile: ).

#include <SPI.h>
const byte CS = 10;   //Chip control goes to pin 10
const byte SHDN = 9;  //Chip SHUTDOWN - PIN 9
const byte RS = 8;    //Chip RESET - PIN 8

void digitalPotWrite(byte value, byte address); // forward declaration

struct Channel {
  const byte buttonUpPin;
  const byte buttonDownPin;
  const byte sensorValPinA; // analog input
  const byte addressPot;

  byte buttonUpState = 0;
  byte buttonDownState = 0;
  byte lastButtonUpState = 0;
  byte lastButtonDownState = 0;
  float Vread = 0; // changing this to a float, because obvious bug is obvious
  float Vtgt = 1;

  Channel(byte buttonUpPin, byte buttonDownPin, byte sensorValPinA, byte addressPot) :
    buttonUpPin(buttonUpPin),
    buttonDownPin(buttonDownPin),
    sensorValPinA(sensorValPinA),
    addressPot(addressPot)
  {
    // we don't set pinmode here in the constructor: that's what setup is for
  }

  void setup() {
    pinMode(buttonUpPin, INPUT);
    pinMode(buttonDownPin, INPUT);
  }

  void loop() {
    setVoltage();
    readVoltage();
    voltageAdjust();
  }

  void setVoltage() {
    // Sets resistance up or down from 100 to 210 with 2 buttons:
    buttonUpState = digitalRead(buttonUpPin);

    if (buttonUpState != lastButtonUpState) {
      if (buttonUpState == HIGH) {
        Serial.print("UP CH"); // REMOVE POST DEBUG
        Serial.print(sensorValPinA); // REMOVE POST DEBUG
        Serial.println(); // REMOVE POST DEBUG
        if (Vtgt >= 1.00 && Vtgt < 2.10) {
          Vtgt += 0.01;
        }
      } else {
        Serial.print("UP CH"); // REMOVE POST DEBUG
        Serial.print(sensorValPinA); // REMOVE POST DEBUG
        Serial.println(" Released"); // REMOVE POST DEBUG
      }
      delay(5);
    }

    buttonDownState = digitalRead(buttonDownPin);

    if (buttonDownState != lastButtonDownState) {
      if (buttonDownState == HIGH) {
        Serial.println("DOWN CH"); // REMOVE POST DEBUG
        Serial.print(sensorValPinA); // REMOVE POST DEBUG
        Serial.println(); // REMOVE POST DEBUG
        if (Vtgt > 1.00 && Vtgt <= 2.10) {
          Vtgt -= 0.01;
        }
      } else {
        Serial.print("Down CH"); // REMOVE POST DEBUG
        Serial.print(sensorValPinA); // REMOVE POST DEBUG
        Serial.println(" Released"); // REMOVE POST DEBUG
      }
      delay(5);
    }
    Serial.print("Channel ");
    Serial.print(sensorValPinA);
    Serial.print(" Target: ");
    Serial.print(Vtgt);
    Serial.print("\t");

    lastButtonDownState = buttonDownState;
    lastButtonUpState = buttonUpState;
  }

  void readVoltage() {
    // Reads voltage from Analog Pins
    int sensorVal = analogRead(sensorValPinA);
    Vread = sensorVal * (5.0 / 1023.0);
    Serial.print("Channel ");
    Serial.print(sensorValPinA);
    Serial.print(" Voltage:    ");
    Serial.print(Vread);
    Serial.print("\t");
    delay(10); //delay between readys for Pot adj CAN BE REMOVED
  }

  void voltageAdjust() {
    // TODO: don't you want a little hysteresis here?

    if (Vread > Vtgt) {
      voltageDecrease();
    } else if (Vread < Vtgt) {
      voltageIncrease();
    }
  }

  void voltageDecrease() {
    // I have left this code as-is, but it seems pretty wrong to me.

    // Decreases Voltage increasing resistance 1/255 away from Terminal A(Pin PAx)
    for (int i = 0; i <= 255; i++) {
      digitalPotWrite(i, addressPot);
    }
  }

  void voltageIncrease() {
    // I have left this code as-is, but it seems pretty wrong to me.

    // Increases Voltage by decreasing resistance 1/255 towards Terminal A(Pin PAx)
    for (int i = 255; i >= 0; i--) {
      digitalPotWrite(i, addressPot);
    }
  }

};

// this macro works out how many channels we have from the size of the array
// the compiler will work it out in-line, and it will reduce down to a constant
// at run-time

#define NCHANNELS (sizeof(channel)/sizeof(*channel))

/*
byte addressPot0 = 0b00010001;      //To define potentiometer use last two BITS 01= POT 0
byte addressPot1 = 0b00010010;      //To define potentiometer use last two BITS 10= POT 1
byte addressPot0and1 = 0b00010011;  //To define potentiometer use last two BITS 10= POT 0 and 1
byte addressPotNA = 0b00000000; //To define no write to potentiometer use BITS 5 and 6 or last two BITS= 00
*/

struct Channel channel[] = {
  Channel(2, 3, A0, 0b00010001),
  Channel(4, 5, A1, 0b00010010)
};

void digitalPotWrite(byte value, byte address)
{ //SPI Write program. Single Chip, 2 Channels.
  digitalWrite(CS, LOW); //Set Chip Active
  SPI.transfer(address);
  SPI.transfer(value);
  digitalWrite(CS, HIGH); //Set Chip Inactive
  delay(10);
}

void setup() {
  pinMode (CS, OUTPUT); //CS - When High, sets chip to read the data.
  pinMode (SHDN, OUTPUT); //CS - When High, sets chip to read the data.
  pinMode (RS, OUTPUT); //CS - When High, sets chip to read the data.

  for (int i = 0; i < NCHANNELS; i++) {
    channel[i].setup();
  }

  Serial.begin(9600);
  digitalWrite(SHDN, HIGH); //Power ON (HIGH)
  digitalWrite(RS, HIGH); //Power NO RESET (LOW)
  SPI.begin();

  delay(50); // Increase to 3000 to allow for breakout boards to initialize or external device boot
}

void loop() {
  // do stuff that applies across all channels
  // (if anything)

  for (int i = 0; i < NCHANNELS; i++) {
    channel[i].loop();
  }

}
7

PaulMurrayCbr:
In your readVoltage function, your declaration of CH0VRead and CH1VRead hides the declarations outside the function. Furthermore, your main declaration is an int rather than a float, which is a bit odd.

I missed the re-declaration locally of those vars. That can't have been helping either.