store different values into two variables from rotary encoder

Thread Starter

anishkgt

Joined Mar 21, 2017
350
Hi All,
I am trying to store two variables within a while loop after reading a rotary encoder but it stores the same value of w1 when switching over to w2
C:
#include<PinChangeInterrupt.h>
#include<Wire.h>
#include<hd44780.h>
#include<hd44780ioClass/hd44780_I2Cexp.h>
hd44780_I2Cexp lcd;
// Const Variables and pin assinged
constbyte ENC_SW =3; // Encoder Switch
constbyte ENC_PinA =6; // PIN A of Encoder
constbyte ENC_PinB =8; // PIN B of Encoder
constbyte buttonPin =9; // Weld button
// Variables
byte prevENC_SWstate;
byte ENC_SWstate;
byte Inconfig; // Display config
int ENC_PinAState;
int ENC_PinALastState;
int Counter1;
int select;
int w1Value;
int w2Value;
unsignedlong TimeStamp;
unsignedlong ENC_SWpressDuration;
unsignedlong delayTime =50;
void setup()
{
lcd.begin(16, 2);
lcd.clear();
pinMode(ENC_PinA, INPUT_PULLUP);
pinMode(ENC_PinB, INPUT_PULLUP);
pinMode(ENC_SW, INPUT_PULLUP);
attachPCINT(digitalPinToPCINT(ENC_PinA), rotaryEncoder, CHANGE);
lcd.setCursor(5, 0);
lcd.print("Start");
delay(1000);
lcd.clear();
Serial.begin(115200);
}

void loop()
{
lcd.setCursor(1, 0);
lcd.print("W1:");
lcd.setCursor(1, 1);
lcd.print("W2:");
lcd.setCursor(9, 0);
lcd.print("T:29");
lcd.setCursor(9, 1);
lcd.print("P:20");
lcd.print("%");

ENC_SWstate =digitalRead(ENC_SW);
if (ENC_SWstate != prevENC_SWstate)
{
if (ENC_SWstate ==LOW)
{
Inconfig =1;
}
}
prevENC_SWstate = ENC_SWstate;

while (Inconfig ==1)
{
ENC_SWstate =digitalRead(ENC_SW);
if (ENC_SWstate != prevENC_SWstate)
{
if (ENC_SWstate ==LOW)
{
select +=1;
select = select %4;
}
}
prevENC_SWstate = ENC_SWstate;

switch (select)
{

case0: //W1
clearSelectPointer(1);
lcd.setCursor(0, 0);
lcd.print(">");
w1Value =readEncoder() %6*10;
lcd.setCursor(1, 0);
lcd.print("W1:");
lcd.print(w1Value);
lcd.print(" ");
Serial.print("Case:");
Serial.println(select);
Serial.print("W1:");
Serial.println(w1Value);
break;
case1: //W2
clearSelectPointer(2);
lcd.setCursor(0, 1);
lcd.print(">");
w2Value =readEncoder() %10*10;
lcd.setCursor(1, 1);
lcd.print("W2:");
lcd.print(w2Value);
lcd.print(" ");
Serial.print("Case:");
Serial.println(select);
Serial.print("W2:");
Serial.println(w2Value);
break;
case2: //P
clearSelectPointer(3);
lcd.setCursor(8, 1);
lcd.print(">");
break;
case3:
lcd.setCursor(0, 0); // 1
lcd.print(" ");
lcd.setCursor(0, 1); // 2
lcd.print(" ");
lcd.setCursor(8, 1);
lcd.print(" ");
break;
}
staticint w2Value = w2Value;
}
}

voidrotaryEncoder()
{
ENC_PinAState =digitalRead(ENC_PinA);
if (digitalRead(ENC_PinB) != ENC_PinAState)
{
Counter1--;
}
else
{
Counter1++;
}
}

intreadEncoder()
{
noInterrupts();
int copyCounter = Counter1;
interrupts();
return (copyCounter) >>1;
}

intclearSelectPointer(int checkPointer)
{
switch (checkPointer)
{
case1: // for select 1
lcd.setCursor(0, 1); // 2
lcd.print(" ");
lcd.setCursor(8, 1); // 3
lcd.print(" ");
break;
case2: // for select 2
lcd.setCursor(0, 0); // 1
lcd.print(" ");
lcd.setCursor(8, 1); // 3
lcd.print(" ");
break;
case3: // for select 3
lcd.setCursor(0, 0); // 1
lcd.print(" ");
lcd.setCursor(0, 1); // 2
lcd.print(" ");
break;
}
return (checkPointer);
}
Mod edit: added code tags
 

Thread Starter

anishkgt

Joined Mar 21, 2017
350
i am using a rotary encoder. My problem is that i want the values to remain after the first change and when returning to the loop it should remain and increment from where it was.
 

MrSoftware

Joined Oct 29, 2013
1,564
First thing is make any variables that are set in an interrupt routine volatile. i.e. "volatile int iMyVar". I only skimmed it quickly, but give that a shot.
 

Thread Starter

anishkgt

Joined Mar 21, 2017
350
I am using an arduino,i doubt Arduinos would have any debuggers or a possibility to set break points.
 

MrSoftware

Joined Oct 29, 2013
1,564
It's possible to debug on the atmega chips, this might help you:

https://arduino.stackexchange.com/questions/482/how-do-i-debug-on-chip-with-arduino

Being able to debug on the chip can be a really really really big help. Option 2 is to debug on an arduino emulator. You probably won't be able to read from your encoder, but maybe you can emulate your encoder by using a timer to call your interrupt routine, thereby allowing you to at least check your logic:

https://arduino.stackexchange.com/questions/61/can-i-program-for-arduino-without-having-a-real-board

Or you could also run the code on Windows, but slip in a timer thread to emulate your encoder, again allowing you to at least check your logic. I hope this helps. :)
 

MrSoftware

Joined Oct 29, 2013
1,564
@kubeek it looks like when he copy/paste, a lot of spaces were lost. It should read "case 0", "case 1", ... instead of "case0", "case1". You are correct though, big difference.

@anishkgt You have defined w2Value twice, are you getting any compiler warnings? Also do your w1 and w2 always match, or only sometimes?
 

ebeowulf17

Joined Aug 12, 2014
2,942
i am using a rotary encoder. My problem is that i want the values to remain after the first change and when returning to the loop it should remain and increment from where it was.
Which values should remain the same as what? Kind of lost on what you're currently getting vs what you want.

It looks like you've got a configuration loop in which you cycle between 4 states with each button press:
  • first set W1
  • then set W2,
  • then move the cursor to a third position with no apparent function
  • then clear all pointers
As I read it, it looks like the number defined by the rotary encoder is persistent, whereas W1 and W2 are lost easily. In other words, if you set W1=2 and W2=4 on your first pass through the loop, you might expect them to show those values when you get back to them. Instead, when you loop back to W1 for a second visit, it immediately jumps to 4, because that's the current value of the rotary encoder (from you spinning to select the number 4 while setting W2.) Similarly, if you then set W1 back to 2 and advance to the W2 position, W2 immediately jumps to 2 instead of its previous value of 4.

Does this sound like the behavior you're experiencing? Am I on the right track? If not, please explain in more detail.
 

Thread Starter

anishkgt

Joined Mar 21, 2017
350
Which values should remain the same as what? Kind of lost on what you're currently getting vs what you want.

It looks like you've got a configuration loop in which you cycle between 4 states with each button press:
  • first set W1
  • then set W2,
  • then move the cursor to a third position with no apparent function
  • then clear all pointers
As I read it, it looks like the number defined by the rotary encoder is persistent, whereas W1 and W2 are lost easily. In other words, if you set W1=2 and W2=4 on your first pass through the loop, you might expect them to show those values when you get back to them. Instead, when you loop back to W1 for a second visit, it immediately jumps to 4, because that's the current value of the rotary encoder (from you spinning to select the number 4 while setting W2.) Similarly, if you then set W1 back to 2 and advance to the W2 position, W2 immediately jumps to 2 instead of its previous value of 4.

Does this sound like the behavior you're experiencing? Am I on the right track? If not, please explain in more detail.
Yes that is how it is.
 

ebeowulf17

Joined Aug 12, 2014
2,942
Yes that is how it is.
Ok, I think what you need to do is redefine your rotary encoder number value immediately after button presses that put you in W1 set mode or W2 set mode.

What follows will be essentially psuedo-code. I make no promises on spelling or syntax. You'll need to get the details right. It should go inside the button press "if" conditional, probably between lines 69-70 (line numbers from your first post.)
Code:
If (select==0){
     noInterrupts();
     counter1=w1Value;
     interrupts();
}
You'll need a second snippet just like it for select==1 leads to counter1=w2Value. Or you could use a switch case statement if you prefer.

I hope I've understood the desired behavior correctly. If so, I think the additions above should accomplish it. Cheers!
 

Thread Starter

anishkgt

Joined Mar 21, 2017
350
Thanks that did job for the first cycle only.

When returning w1Value takes a random value which is not equal to the previously set.

Code:
#include<Arduino.h>
#include<PinChangeInterrupt.h>
#include<Wire.h>
#include<hd44780.h>
#include<hd44780ioClass/hd44780_I2Cexp.h>
hd44780_I2Cexp lcd;
// Const Variables and pin assinged
constbyte ENC_SW =3; // Encoder Switch
constbyte ENC_PinA =6; // PIN A of Encoder
constbyte ENC_PinB =8; // PIN B of Encoder
constbyte buttonPin =9; // Weld button
// Variables
byte prevENC_SWstate;
byte prevENC_SWstate1;
byte ENC_SWstate;
byte Inconfig; // Display config
int ENC_PinAState;
int ENC_PinALastState;
int Counter1;
int select;
int w1Value;
int w2Value;
int pValue;
int prevW1Value;
unsignedlong TimeStamp;
unsignedlong ENC_SWpressDuration;
unsignedlong delayTime =50;
void setup()
{
lcd.begin(16, 2);
lcd.clear();
pinMode(ENC_PinA, INPUT_PULLUP);
pinMode(ENC_PinB, INPUT_PULLUP);
pinMode(ENC_SW, INPUT_PULLUP);
attachPCINT(digitalPinToPCINT(ENC_PinA), rotaryEncoder, CHANGE);
lcd.setCursor(5, 0);
lcd.print("Start");
delay(1000);
lcd.clear();
Serial.begin(115200);
}

void loop()
{
lcd.setCursor(1, 0);
lcd.print("W1:");
lcd.setCursor(1, 1);
lcd.print("W2:");
lcd.setCursor(9, 0);
lcd.print("T:29");
lcd.setCursor(9, 1);
lcd.print("P:");
Serial.println("In Main Loop");

ENC_SWstate =digitalRead(ENC_SW);
if (ENC_SWstate ==LOW&& prevENC_SWstate ==HIGH)
{
delay(50); //debounce period
ENC_SWstate =digitalRead(ENC_SW);
if (ENC_SWstate ==LOW) //stable LOW
{
Counter1 = w1Value;
Serial.print("Counter:");
Serial.println(Counter1);
Serial.print("W1:");
Serial.println(w1Value);
Serial.print("W2:");
Serial.println(w2Value);
Serial.print("pValue:");
Serial.println(pValue);
Inconfig =1;
}
}
prevENC_SWstate = ENC_SWstate;

while (Inconfig ==1)
{
//wait for keypress to move selector
ENC_SWstate =digitalRead(ENC_SW);
if (ENC_SWstate != prevENC_SWstate)
{
if (ENC_SWstate ==LOW)
{
select +=1;
select = select %4;

if (select ==0)
{
noInterrupts();
Counter1 = w1Value;
interrupts();
}
if (select ==1)
{
noInterrupts();
Counter1 = w2Value;
interrupts();
}
if (select ==2)
{
noInterrupts();
Counter1 = pValue;
interrupts();
}
}
prevENC_SWstate = ENC_SWstate;
}

switch (select)
{

case0: //W1
clearSelectPointer(1);
lcd.setCursor(0, 0);
lcd.print(">");
lcd.setCursor(1, 0);
lcd.print("W1:");
w1Value =readEncoder() %20*10;
if (w1Value <=0)
{
w1Value =0;
}
else
{
w1Value = w1Value;
}
lcd.print(w1Value);
lcd.print(" ");
break;
case1: //W2
clearSelectPointer(2);
lcd.setCursor(0, 1);
lcd.print(">");
w2Value =readEncoder() %50*10;
if (w2Value <=0)
{
w2Value =0;
}
else
{
w2Value = w2Value;
}
lcd.setCursor(1, 1);
lcd.print("W2:");
lcd.print(w2Value);
lcd.print(" ");
break;
case2: //P
clearSelectPointer(3);
lcd.setCursor(8, 1); //3
lcd.print(">");
pValue =readEncoder() %11*10;
if (pValue <=0)
{
pValue =0;
}
else
{
pValue = pValue;
}
lcd.setCursor(9, 1);
lcd.print("P:");
lcd.print(pValue);
lcd.print("%");
lcd.print(" ");
break;
default:
clearSelectPointer(4);
Inconfig =0;
select =0;
break;
}
}
}

voidrotaryEncoder()
{
ENC_PinAState =digitalRead(ENC_PinA);
if (digitalRead(ENC_PinB) != ENC_PinAState)
{
Counter1--;
}
else
{
Counter1++;
}
}

intreadEncoder()
{
noInterrupts();
int copyCounter = Counter1;
interrupts();
return (copyCounter) >>1;
}

intclearSelectPointer(int checkPointer)
{
switch (checkPointer)
{
case1: // for select 1
lcd.setCursor(0, 1); // 2
lcd.print(" ");
lcd.setCursor(8, 1); // 3
lcd.print(" ");
break;
case2: // for select 2
lcd.setCursor(0, 0); // 1
lcd.print(" ");
lcd.setCursor(8, 1); // 3
lcd.print(" ");
break;
case3: // for select 3
lcd.setCursor(0, 0); // 1
lcd.print(" ");
lcd.setCursor(0, 1); // 2
lcd.print(" ");
break;
case4:
lcd.setCursor(0, 0); // 1
lcd.print(" ");
lcd.setCursor(0, 1); // 2
lcd.print(" ");
lcd.setCursor(8, 1); //3
lcd.print(" ");
break;
}
return (checkPointer);
}
 

ebeowulf17

Joined Aug 12, 2014
2,942
Thanks that did job for the first cycle only.

When returning w1Value takes a random value which is not equal to the previously set.

Code:
#include<Arduino.h>
#include<PinChangeInterrupt.h>
#include<Wire.h>
#include<hd44780.h>
#include<hd44780ioClass/hd44780_I2Cexp.h>
hd44780_I2Cexp lcd;
// Const Variables and pin assinged
constbyte ENC_SW =3; // Encoder Switch
constbyte ENC_PinA =6; // PIN A of Encoder
constbyte ENC_PinB =8; // PIN B of Encoder
constbyte buttonPin =9; // Weld button
// Variables
byte prevENC_SWstate;
byte prevENC_SWstate1;
byte ENC_SWstate;
byte Inconfig; // Display config
int ENC_PinAState;
int ENC_PinALastState;
int Counter1;
int select;
int w1Value;
int w2Value;
int pValue;
int prevW1Value;
unsignedlong TimeStamp;
unsignedlong ENC_SWpressDuration;
unsignedlong delayTime =50;
void setup()
{
lcd.begin(16, 2);
lcd.clear();
pinMode(ENC_PinA, INPUT_PULLUP);
pinMode(ENC_PinB, INPUT_PULLUP);
pinMode(ENC_SW, INPUT_PULLUP);
attachPCINT(digitalPinToPCINT(ENC_PinA), rotaryEncoder, CHANGE);
lcd.setCursor(5, 0);
lcd.print("Start");
delay(1000);
lcd.clear();
Serial.begin(115200);
}

void loop()
{
lcd.setCursor(1, 0);
lcd.print("W1:");
lcd.setCursor(1, 1);
lcd.print("W2:");
lcd.setCursor(9, 0);
lcd.print("T:29");
lcd.setCursor(9, 1);
lcd.print("P:");
Serial.println("In Main Loop");

ENC_SWstate =digitalRead(ENC_SW);
if (ENC_SWstate ==LOW&& prevENC_SWstate ==HIGH)
{
delay(50); //debounce period
ENC_SWstate =digitalRead(ENC_SW);
if (ENC_SWstate ==LOW) //stable LOW
{
Counter1 = w1Value;
Serial.print("Counter:");
Serial.println(Counter1);
Serial.print("W1:");
Serial.println(w1Value);
Serial.print("W2:");
Serial.println(w2Value);
Serial.print("pValue:");
Serial.println(pValue);
Inconfig =1;
}
}
prevENC_SWstate = ENC_SWstate;

while (Inconfig ==1)
{
//wait for keypress to move selector
ENC_SWstate =digitalRead(ENC_SW);
if (ENC_SWstate != prevENC_SWstate)
{
if (ENC_SWstate ==LOW)
{
select +=1;
select = select %4;

if (select ==0)
{
noInterrupts();
Counter1 = w1Value;
interrupts();
}
if (select ==1)
{
noInterrupts();
Counter1 = w2Value;
interrupts();
}
if (select ==2)
{
noInterrupts();
Counter1 = pValue;
interrupts();
}
}
prevENC_SWstate = ENC_SWstate;
}

switch (select)
{

case0: //W1
clearSelectPointer(1);
lcd.setCursor(0, 0);
lcd.print(">");
lcd.setCursor(1, 0);
lcd.print("W1:");
w1Value =readEncoder() %20*10;
if (w1Value <=0)
{
w1Value =0;
}
else
{
w1Value = w1Value;
}
lcd.print(w1Value);
lcd.print(" ");
break;
case1: //W2
clearSelectPointer(2);
lcd.setCursor(0, 1);
lcd.print(">");
w2Value =readEncoder() %50*10;
if (w2Value <=0)
{
w2Value =0;
}
else
{
w2Value = w2Value;
}
lcd.setCursor(1, 1);
lcd.print("W2:");
lcd.print(w2Value);
lcd.print(" ");
break;
case2: //P
clearSelectPointer(3);
lcd.setCursor(8, 1); //3
lcd.print(">");
pValue =readEncoder() %11*10;
if (pValue <=0)
{
pValue =0;
}
else
{
pValue = pValue;
}
lcd.setCursor(9, 1);
lcd.print("P:");
lcd.print(pValue);
lcd.print("%");
lcd.print(" ");
break;
default:
clearSelectPointer(4);
Inconfig =0;
select =0;
break;
}
}
}

voidrotaryEncoder()
{
ENC_PinAState =digitalRead(ENC_PinA);
if (digitalRead(ENC_PinB) != ENC_PinAState)
{
Counter1--;
}
else
{
Counter1++;
}
}

intreadEncoder()
{
noInterrupts();
int copyCounter = Counter1;
interrupts();
return (copyCounter) >>1;
}

intclearSelectPointer(int checkPointer)
{
switch (checkPointer)
{
case1: // for select 1
lcd.setCursor(0, 1); // 2
lcd.print(" ");
lcd.setCursor(8, 1); // 3
lcd.print(" ");
break;
case2: // for select 2
lcd.setCursor(0, 0); // 1
lcd.print(" ");
lcd.setCursor(8, 1); // 3
lcd.print(" ");
break;
case3: // for select 3
lcd.setCursor(0, 0); // 1
lcd.print(" ");
lcd.setCursor(0, 1); // 2
lcd.print(" ");
break;
case4:
lcd.setCursor(0, 0); // 1
lcd.print(" ");
lcd.setCursor(0, 1); // 2
lcd.print(" ");
lcd.setCursor(8, 1); //3
lcd.print(" ");
break;
}
return (checkPointer);
}
I previously missed the fact that W1 and W2 get set to 10x the encoder value, not just the encoder value. That means when forcing Counter1 to a value it would need to be W1/10 or W2/10... I think. Maybe try that and see if it helps. I was skimming quickly just now and may have overlooked something.
 

Thread Starter

anishkgt

Joined Mar 21, 2017
350
I previously missed the fact that W1 and W2 get set to 10x the encoder value, not just the encoder value. That means when forcing Counter1 to a value it would need to be W1/10 or W2/10... I think. Maybe try that and see if it helps. I was skimming quickly just now and may have overlooked something.
used this
C:
switch (select)
{
case0:
noInterrupts();
Counter1 = w1Value/10;
interrupts();
break;
case1:
noInterrupts();
Counter1 = w2Value/10;
interrupts();
break;
This would just reset all to zero.

But why divide them 10 ? We are forcing it take the value of each variable, right ?
 

spinnaker

Joined Oct 29, 2009
7,815
A while loop in the main loop is not the best way to read a rotary encoder. You really should be using interrupts or you could be missing data. I don't use the Arduino but I would be shocked if they don't have a rotary encoder library. If so, why are you trying to reinvent the wheel?
 

Thread Starter

anishkgt

Joined Mar 21, 2017
350
A while loop in the main loop is not the best way to read a rotary encoder. You really should be using interrupts or you could be missing data. I don't use the Arduino but I would be shocked if they don't have a rotary encoder library. If so, why are you trying to reinvent the wheel?
The encoder is read from a function outside the main loop.
 
Top