used this

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 ?

Unless I'm misreading this, when you capture data from the encoder value and save it as w1 or w2, you're starting with the raw encoder value, then limiting its max value with a modulo function, then multiplying it by 10:

w1Value =readEncoder() %20*10;
w2Value =readEncoder() %50*10;

So, if your Counter1 encoder variable was 3, what you would store as w1Value is 30. Therefore, if you're later copying w1Value back into Counter1, you need to divide by 10 to end up with 3 again. If you don't divide by 10, and you copy 30 back in as the Counter1 value, then the next attempt to calculate w1Value from Counter1 will be 30 modulo 20 times 10. That works out to 100, which will bear no obvious relationship to the 30 you're expecting to see.

What makes you think everything will reset to zero? Did you run it and get zero every time, or is that just what you'd expect from the code? If it's actually giving you zero each time with my recommended code, then I've obviously gotten myself turned around somewhere!

 

That doesn't matter if it is not using interrupts.

It is using interrupts for the rotary part. The while loop is just handling the logic of what to do with values set by the rotary encoder, but it's not polling the encoder or slowing things down. Look at line 36, and then at the function starting on line 177 (line numbers from the last full code copy in post 15.

It's not using interrupts for the pushbutton part, but I don't think that will matter.

 

Unless I'm misreading this, when you capture data from the encoder value and save it as w1 or w2, you're starting with the raw encoder value, then limiting its max value with a modulo function, then multiplying it by 10:

w1Value =readEncoder() %20*10;
w2Value =readEncoder() %50*10;

So, if your Counter1 encoder variable was 3, what you would store as w1Value is 30. Therefore, if you're later copying w1Value back into Counter1, you need to divide by 10 to end up with 3 again. If you don't divide by 10, and you copy 30 back in as the Counter1 value, then the next attempt to calculate w1Value from Counter1 will be 30 modulo 20 times 10. That works out to 100, which will bear no obvious relationship to the 30 you're expecting to see.

What makes you think everything will reset to zero? Did you run it and get zero every time, or is that just what you'd expect from the code? If it's actually giving you zero each time with my recommended code, then I've obviously gotten myself turned around somewhere!

Ah yes, Thanks that makes it clearer.
So when i divide 'em by 10 i get a different value, w1 = 40 and Counter1 = w1/10, the value of w1 becomes 4. Or i've got it wrong.

 

Ah yes, Thanks that makes it clearer.
So when i divide 'em by 10 i get a different value, w1 = 40 and Counter1 = w1/10, the value of w1 becomes 4. Or i've got it wrong.

Close, but not quite - Counter1 becomes 4, but w1 should still end up at 40, since w1 gets defined as Counter1*10.

 

Can't seem to get it through. I understand the Counter1 value has to be w1Value / 10 before it reaches the while loop but where do i set it.

 

What is the maximum pulse frequency you expect from the encoder?

RPM * 60 * Encoder Pulses per Revolution = Freq. Then consider you have 2 channels, both with 2 edges each.

Doing encoders in code is a very bandwidth intensive proposition, make sure you have done the math before pulling hair out.

 

Can't seem to get it through. I understand the Counter1 value has to be w1Value / 10 before it reaches the while loop but where do i set it.

I thought your code from post 15 looked right, except it was missing the "/10" part. I think the lines setting Counter1 from w1 and w2 were in the right spots there, and you just needed to change them from

Counter1=w1Value;

to:

Counter1=w1Value/10;

Etc.

Did you try running that code, or had you also made other changes before adding the "/10" parts? If you did run it just like that, what were the results? If not, try going back to your post 15 code and just adding the "/10" parts.

If it's still not working for you after that, please post the current state of your code and also give me an example of complete results you're getting, with the value of each setting on each pass, something like:

First pass:
Set w1=20
Set w2=30
Set p=40

Second pass:
w1 showed 20 (ok)
w2 showed 89 (error)
p showed 40 (ok)

Third pass:
w1 showed 20 (ok)
w2 showed 121 (error)
p showed 40 (ok)

The numbers above are deliberately random and meaningless - i was just providing an example of the kind of detail I'm looking for. It will be easier to recognize a pattern in the failures if I can see what inputs yield what outputs.

 

#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;
int Counter;
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:");
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;
Counter++;
Serial.print("Pass:");
Serial.println(Counter);
Serial.print("Counter:");
Serial.println(Counter1);
Serial.print("W1:");
Serial.println(w1Value);
Serial.print("W2:");
Serial.println(w2Value);
Serial.print("pValue:");
Serial.println(pValue);
Serial.println(" ");
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 /10;
interrupts();
}
if (select ==1)
{
noInterrupts();
Counter1 = w2Value /10;
interrupts();
}
if (select ==2)
{
noInterrupts();
Counter1 = pValue /10;
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);
}

Pass:1
Counter:0
W1:0
W2:0
pValue:0

Pass:2
Counter:30
W1:30
W2:50
pValue:40

Pass:3
Counter:150
W1:150
W2:20
pValue:20

Pass 2 shows the value after setting them. Pass 3 shows the value by just pressing the encoder and without turning, these were the values shown in the display.

 

#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;
int Counter;
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:");
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;
Counter++;
Serial.print("Pass:");
Serial.println(Counter);
Serial.print("Counter:");
Serial.println(Counter1);
Serial.print("W1:");
Serial.println(w1Value);
Serial.print("W2:");
Serial.println(w2Value);
Serial.print("pValue:");
Serial.println(pValue);
Serial.println(" ");
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 /10;
interrupts();
}
if (select ==1)
{
noInterrupts();
Counter1 = w2Value /10;
interrupts();
}
if (select ==2)
{
noInterrupts();
Counter1 = pValue /10;
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);
}

Pass 2 shows the value after setting them. Pass 3 shows the value by just pressing the encoder and without turning, these were the values shown in the display.

Bizarre! Thanks for the detailed report. That's perfect. At first glance, I don't have any answers. Sorry. I'll dig deeper later this afternoon and see if I can help. Cheers!

 

#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;
int Counter;
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:");
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;
Counter++;
Serial.print("Pass:");
Serial.println(Counter);
Serial.print("Counter:");
Serial.println(Counter1);
Serial.print("W1:");
Serial.println(w1Value);
Serial.print("W2:");
Serial.println(w2Value);
Serial.print("pValue:");
Serial.println(pValue);
Serial.println(" ");
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 /10;
interrupts();
}
if (select ==1)
{
noInterrupts();
Counter1 = w2Value /10;
interrupts();
}
if (select ==2)
{
noInterrupts();
Counter1 = pValue /10;
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);
}

Pass 2 shows the value after setting them. Pass 3 shows the value by just pressing the encoder and without turning, these were the values shown in the display.

Found the problems!

There are two issues. I've tested the math and confirmed that the problems I found explain the weird numbers you're getting, so I'm pretty confident that I know what's going on:

On line 61, you need to divide w1Value by ten, just like the other places where we set Counter1 based on existing values. Just add "/10" to the end of that formula.

On line 191, there's a bit-shift operation that I completely overlooked. It's the ">>1" at the end of the line. Bit shifting one digit to the right divides by two and rounds down. There's no use for that in your function, so just delete ">>1" from the end of that line.

I'm pretty sure that's it. Let me know if it works or if I missed something else.

Cheers!

 

Thank you very much ebeowulf17. That did the trick.

The bit shift was earlier set because a value from the encoder would shift one place forward. Now i left it added and added '<< 1' after dividing by 10.

All looking good now.

Output:

Pass:1
Counter:0
W1:0
W2:0
pValue:0

Pass:2
Counter:4
W1:20
W2:30
pValue:60

Pass:3
Counter:4
W1:20
W2:30
pValue:60

Pass:4
Counter:2
W1:10
W2:20
pValue:100

 

Thank you very much ebeowulf17. That did the trick.

The bit shift was earlier set because a value from the encoder would shift one place forward. Now i left it added and added '<< 1' after dividing by 10.

All looking good now.

Output:

Sweet, glad to hear it's working!

I must confess, the logic of bit shifting twice escapes me, and sounds dangerous. Maybe I'll understand it better when I have time to make notes and play with code. My gut feeling is that you're safer without the bit shifts in place because of rounding errors, but maybe I've misunderstood how it will work.

Here's my fear. It seems the shifting works for even numbers, but not for odd ones:
4 >> 1 = 2
2 << 1 = 4

5 >> 1 = 2
2 << 1 = 4

So 4 in ultimately yields 4 out, as it should, but 5 in also yields 4 out, which it shouldn't.

 

Me too. I really don’t get the idea behind but shifting. Maybe if I dig in a bit I would.

It’s shifting a bit in the binary line of an integer is what i understand at a glance. So I just took the advice to bit shift in my code.

 

Me too. I really don’t get the idea behind but shifting. Maybe if I dig in a bit I would.

It’s shifting a bit in the binary line of an integer is what i understand at a glance. So I just took the advice to bit shift in my code.

I wouldn't add bit shifts unless you need the math they provide. Why divide a number by two and then multiply it by two for no reason?

If you need to multiply or divide by two for some particular operation, that's great. But don't add unnecessary math that could corrupt your outputs!

 

For what it's worth, here's a response from another forum that I think does a good job of describing some bit shifting scenarios:

https://stackoverflow.com/questions/11072202/what-is-the-purpose-of-bit-shifting

The primary use is when you have some part of a larger item defined in terms of specific bits.

For an obvious example, consider a 32-bit number holding a color -- 8-bits each for red, green, and blue, and (possibly) the other 8 bits for alpha (signifying how transparent this color/pixel should be). In hexadecimal, the digits would look like:

(i.e., two digits, or 8 bits) for each component).

We can take such a thing and break it into components something like:

Conversely, we can put components together:

You also typically end up doing bit-twiddling like this when dealing with hardware. For example, you might have a 16-bit register that dedicates 5 bits to one thing, 2 more bits to something else, 6 bits to a third, and so on. When/if you want to change one of those, you do about like the color example, above: isolate the bits that represent one field, modify as needed, then put them back together with the other bits.

Another (quite unrelated) application is in things like hashing. Here we don't typically have fields as such, but we want some bytes of input to produce a single output, with all the bits of the output affected to at least some degree by the bytes of the input. To accomplish that, most end up shifting bits so each byte of input has at least some chance of affecting different parts of the result.

I'd add that although quite a bit of older code uses bit shifts to optimize multiplication or division by powers of 2, this is usually a waste of time with modern hardware and compilers. You will see it in existing code, and should understand what it's trying to accomplish -- but don't try to emulate its example.

 

Hello,

While your suggestion here in this code works. I don't seem to get the expected results in my actual code. here is the output

Pass:1
Counter:0
W1:0
W2:0
pValue:0

Pass:2
Counter:4
W1:20
W2:40
pValue:30

Pass:3
Counter:4
W1:20
W2:40
pValue:0

Pass:4
Counter:4
W1:20
W2:0
pValue:0

Pass:5
Counter:0
W1:0
W2:0
pValue:0

One every pass one or the other value resets to zero and sometimes it does not.

 

i suspect it is the

Counter1 = w1 / 10 << 1;

that is executed in each loop that is failing.


here is the partial code where it is used.

int w1Value;
int pValue;
int select;
int Counter1;
int Counter;
void setup ()
{
............
}
voidloop()
{
lcd.setCursor(1, 1);
lcd.print("W2:");
lcd.print(WeldTime);
lcd.setCursor(9, 0);
lcd.print("T:");
lcd.print(String(thermistor->readCelsius()));
lcd.setCursor(9, 1);
lcd.print("P:");
digitalWrite(rdy, HIGH);

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
{
prevENC_SWstate = ENC_SWstate;
Counter1 = w1Value /10<<1;
confMenu =1;
Counter++;
Serial.print("Pass:");
Serial.println(Counter);
Serial.print("Counter:");
Serial.println(Counter1);
Serial.print("W1:");
Serial.println(w1Value);
Serial.print("W2:");
Serial.println(WeldTime);
Serial.print("pValue:");
Serial.println(pValue);
Serial.println(" ");
}
}

while (confMenu ==1)
{
ENC_SWstate =digitalRead(ENC_SW);
if (ENC_SWstate != prevENC_SWstate)
{
if (ENC_SWstate == LOW)
{
delay(10);
ENC_SWstate =digitalRead(ENC_SW);
if (ENC_SWstate == LOW)
{
select +=1;
select = select %4;
}
if (select ==0)
{
noInterrupts();
Counter1 = w1Value /10<<1;
interrupts();
}

if (select ==1)
{
noInterrupts();
Counter1 = WeldTime /10<<1;
interrupts();
}

if (select ==2)
{
noInterrupts();
Counter1 = pValue /10<<1;
interrupts();
}
}
prevENC_SWstate = ENC_SWstate;
}

switch (select)
{
case0: //W1
clearSelectPointer(1);
lcd.setCursor(0, 0);
lcd.print(">");
w1Value =readEncoder() %6*10;
if (w1Value <=0)
{
w1Value =0;
}
lcd.setCursor(1, 0);
lcd.print("W1:");
lcd.print(w1Value);
lcd.print(" ");
break;

case1: //W2
clearSelectPointer(2);
lcd.setCursor(0, 1);
lcd.print(">");
WeldTime =readEncoder() %51*10;
if (WeldTime <=0)
{
WeldTime =0;
}
lcd.setCursor(1, 1);
lcd.print("W2:");
lcd.print(WeldTime); //lcd.setCursor(5, 1);
lcd.print(" ");
break;

case2: //P
clearSelectPointer(3);
lcd.setCursor(8, 1);
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);
confMenu =0;
select =0;
break;
}
}

 

Hello,

While your suggestion here in this code works. I don't seem to get the expected results in my actual code. here is the output


One every pass one or the other value resets to zero and sometimes it does not.

You didn't follow my advice. That was not my suggestion. I suggested you get rid of the bit shift entirely, not add a whole lot more of them!

I suggest you get rid of all them. It might fix all your problems. Even if it doesn't, it'll make the logic much easier to follow so we can debug any remaining problems.

I'm not sure yet why your current code makes things zero sometimes. If I have time later, I'll take a closer look. Either way, the bit shifts are not helping you at all. Why do insist on using them where they serve no purpose?!

 

You moved a block of conditionals to the wrong spot. Why did you move them? These lines:

if (select ==0)
{
noInterrupts();
Counter1 = w1Value /10<<1;
interrupts();
}
if (select ==1)
{
noInterrupts();
Counter1 = WeldTime /10<<1;
interrupts();
}
if (select ==2)
{
noInterrupts();
Counter1 = pValue /10<<1;
interrupts();
}

Should all be inside the switch read conditional below, not after it where you have it now:

if (ENC_SWstate == LOW)
{
select +=1;
select = select %4;
}

I suspect that is the cause of your cascading zeros. Move the conditionals which redefine Counter1 back into the brackets for the button press and you'll probably be ok.

Also, I double checked your bit-shift operations and I don't think they'll corrupt anything... although I think they will mean it takes two rotary encoder steps to make any change where it used to take only 1 (not totally sure on that, but I think I got that right.) I still think they're unnecessary and potentially confusing, but I don't think they're the cause of your current problems.

 

Ummm i've put them back into the place they were and now all resets zero on the second pass

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 /10<<1;
Serial.print("Pass:");
Serial.println(Counter);
Serial.print("Counter:");
Serial.println(Counter1);
Serial.print("W1:");
Serial.println(w1Value);
Serial.print("W2:");
Serial.println(WeldTime);
Serial.print("pValue:");
Serial.println(pValue);
Serial.println(" ");
confMenu =1;
Counter++;
}
}
prevENC_SWstate = ENC_SWstate;
while (confMenu ==1)
{
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 /10<<1;
interrupts();
}
if (select ==1)
{
noInterrupts();
Counter1 = WeldTime /10<<1;
interrupts();
}
if (select ==2)
{
noInterrupts();
Counter1 = pValue /10<<1;
interrupts();
}
}
prevENC_SWstate = ENC_SWstate;
}

switch (select)
{
case0: //W1
clearSelectPointer(1);
lcd.setCursor(0, 0);
lcd.print(">");
w1Value =readEncoder() %6*10;
if (w1Value <=0)
{
w1Value =0;
}
lcd.setCursor(1, 0);
lcd.print("W1:");
lcd.print(w1Value);
lcd.print(" ");
break;

case1: //W2
clearSelectPointer(2);
lcd.setCursor(0, 1);
lcd.print(">");
WeldTime =readEncoder() %51*10;
if (WeldTime <=0)
{
WeldTime =0;
}
lcd.setCursor(1, 1);
lcd.print("W2:");
lcd.print(WeldTime); //lcd.setCursor(5, 1);
lcd.print(" ");
break;

case2: //P
clearSelectPointer(3);
lcd.setCursor(8, 1);
lcd.print(">");
pValue =readEncoder() %11*10;
if (pValue <=0)
{
pValue =0;
}
lcd.setCursor(9, 1);
lcd.print("P:");
lcd.print(pValue);
lcd.print("%");
lcd.print(" ");
break;
default:
clearSelectPointer(4);
confMenu =0;
select =0;
break;
}
}

here is the output

Pass:0
Counter:0
W1:0
W2:0
pValue:0

Pass:1
Counter:8
W1:40
W2:30
pValue:20

Pass:2
Counter:0
W1:0
W2:0
pValue:0

Pass:3
Counter:0
W1:0
W2:0
pValue:0

Pass:4
Counter:0
W1:0
W2:0
pValue:0

Pass:5
Counter:4
W1:20
W2:40
pValue:30

Pass:6
Counter:0
W1:0
W2:0
pValue:0