Volume Control controlled by incremental encoder, need some help

Hi folks,

Newbie here needing a little help.

I'm using a rotary encoder, sensed through the Uno interrupts.
The volatile variable used to control the effective volume is encoderPos,
this value is supposed to be among 0 and 255. I've added two lines of code to avoid that
the count continues after reaching the Max Val of 255, same for Min Val, i.e. 0.

This value is then converted in binary form to be used by the volume controlling external circuitry.

I'm also using an IR remote function, however, in the listed code below, I skipped it, as it works just fine.

The problem is just on the interrupt/encoder section.

The problem is:
when the encoderPos variable value reaches '0', which is supposed to be the minimum
allowed, if I continue turning the rotary encoder down, I still get pulses on the binary decoded output,
when reaching maximum (255), turning the encoder up, gives neg. pulses for a split second to the decoded bit port.

that is an 8-bit parallel port, named ledPin.

In words, when at min. (0), the decoded bit port is '00000000', and it is supposed to remain
in this state even if I continue to decrease the rotary encoder.

By contrast, if I continue to decrease, for a fraction of a second I get '11111111' decoded.

The same specular things happens when at maximum value.

The decoded binary value is '11111111', and even if I encrease the encoder, it is supposed to remain there,
on the contrary, I get pulses of '00000000' whenever I continue to increase the encoder.

The strange thing is that I have this behaviour only using the rotary encoder, not if using the remote.

Something to do with the interrupts?

I came to a complete stop, can you help?

The code I'm using:

#include <boarddefs.h>
#include <IRremote.h>
#include <EEPROM.h>


int receiver = 13;            // TSOP pin 1 to Arduino pin13
const int screenWidth = 128;  // set Width for Right justification
const int dly1 = 90;          // set delay for IR receiver antibounce
#define encDelay        120   // set delay for encoder anti-bounce

IRrecv irrecv(receiver);      // create instance of 'irrecv'
decode_results results;

/* -------------------------
    -- Pin Assignments -----
    ------------------------
*/
enum PinAssignments {
  encoderPinA = 2,  //interrupt 2
  encoderPinB = 3,  //interrupt 3
  clearButton = 4   //pushButton
};

/*
   Variable declarations
*/
volatile int encoderPos = 0;
int lastReportedPos = 1;
int z = 0;
int volAddr1 = 0;        //Address 1 for EEPROM Volume store
int volAddr2 = 1;        //Address 2 for EEPROM Volume store
int volAddr3 = 2;        //Address 3 for EEPROM Volume store
int volAddr4 = 3;        //Address 4 for EEPROM Volume store

// Define variable to store last code received
unsigned long lastCode;

//8-bit parallel port
int ledPin[] = {5, 6, 7, 8, 9, 10, 11, 12};       //LSB to MSB
//             D0 D1 D2 D3 D4  D5  D6  D7

boolean A_set = false;
boolean B_set = false;


//--------------------
//----- SET-UP -------
//--------------------
void setup() {
  pinMode(encoderPinA, INPUT);
  pinMode(encoderPinB, INPUT);
  pinMode(clearButton, INPUT);
  digitalWrite(clearButton, HIGH);

  for (int i = 0; i < 8; i++)
  {
    pinMode(ledPin[i], OUTPUT);
  }

  // encoder pin A on interrupt 0 (pin 2)
  attachInterrupt(0, doEncoderA, CHANGE);
  // encoder pin B on interrupt 1 (pin 3)
  attachInterrupt(1, doEncoderB, CHANGE);

  
  irrecv.enableIRIn(); // Start the receiver
}


/*  -------------------------
    ----- App start ---------
    -------------------------
*/
void loop(void) {
  /*
     Rotary encoder position limited between 0 and 255
  */
  if (encoderPos > 255) {
    encoderPos = 255;
  }
  if (encoderPos < 0) {
    encoderPos = 0;
  }

  //calculate 'z' for dB print
  z = (128 - ((encoderPos + 1) / 2.0)) * 10;

  if (lastReportedPos != encoderPos) {
   
    lastReportedPos = encoderPos;

    displayBinary(encoderPos);  // 8-bit parallel port decoding

    
    delay(10);

  }

  /* --------
     --MUTE--
     --------
  */
  if (digitalRead(clearButton) == LOW)  {
    encoderPos = 0;
  }

  /* -----------------
     ---IR Receiver---
     -----------------
  */
  //skipped

}   //end loop

/* -----------
   Subroutines
   -----------
*/

// Interrupt on A changing state
void doEncoderA() {
  // Test transition
  A_set = digitalRead(encoderPinA) == HIGH;
  // and adjust counter + if A leads B
  encoderPos += (A_set != B_set) ? +1 : -1;
  delay(encDelay);
}

// Interrupt on B changing state
void doEncoderB() {
  // Test transition
  B_set = digitalRead(encoderPinB) == HIGH;
  // and adjust counter + if B follows A
  encoderPos += (A_set == B_set) ? +1 : -1;
  delay(encDelay);
}


// decimal to 8-bit binary converter
void displayBinary(byte numToShow)
{
  for (int i = 0; i < 8; i++)
  {
    if (bitRead(numToShow, i) == 1)
    {
      digitalWrite(ledPin[i], HIGH);
    }
    else
    {
      digitalWrite(ledPin[i], LOW);
    }
  }
}

Anyone?

Commenting yourself makes many people think that you already got an answer, certainly not what you want.

I'd check the range and skip increment/decrement already in the ISR.

Which rotary encoder are you using and how is it wired to the Arduino? Is it one like this ?

DrDiettrich:
I'd check the range and skip increment/decrement already in the ISR.

Thank you for the reply.
Could you please clarify how to do this? Some example?
I already have added some lines of code to restrict the value from 0 to 255, the first lines in the void loop section.
Are you referring to additional lines to limit the range in the void doEncoder section?

stowite:
Which rotary encoder are you using and how is it wired to the Arduino? Is it one like this ?

Yes, the exact part no. is PEC11R-4230F-S0024 from Bourns, and I'm using the exact suggested filter circuit shown on page 2 in the datasheet

Its resolution is 24 pulses per rotation, 24 detents.

I already have added some lines of code to restrict the value from 0 to 255, the first lines in the void loop section.
Are you referring to additional lines to limit the range in the void doEncoder section?

If you limit the range inside the ISR, you can drop the range management in loop().

 delay(encDelay);

delay() does not work within an ISR.

If contact bounce from the encoder is indeed a problem, you will need to use hardware debounce or else don't use interrupts and move to a debounced polling routine. You could also try to add a lockout period to the interrupt so you don't increment the value if the interrupt is triggered within a certain time from the last interrupt.

cattledog:
If you limit the range inside the ISR, you can drop the range management in loop().

Hey thanks!

I've added a constrain function in the ISR routine, it seems to be working.
I hope it's correct, and that's what you were suggesting.

Code below.

// Interrupt on A changing state
void doEncoderA() {
  // Test transition
  A_set = digitalRead(encoderPinA) == HIGH;
  // and adjust counter + if A leads B
  encoderPos += (A_set != B_set) ? +1 : -1;
  encoderPos = constrain(encoderPos, 0, 255);
}

// Interrupt on B changing state
void doEncoderB() {
  // Test transition
  B_set = digitalRead(encoderPinB) == HIGH;
  // and adjust counter + if B follows A
  encoderPos += (A_set == B_set) ? +1 : -1;
  encoderPos = constrain(encoderPos, 0, 255);
}

One last thing:

The encoder I'm using, features 24PPR resolution and 24 detents.
Each detent advances the encoderPos count by 2.
I'd like this to increase/decrease by 1.

Is it possible to implement this by software, or should I change the encoder altogether?

Any suggestion welcome.

Thanks.

If you remove the ISR for channel A or B, you get only half the counts.

To align the counts with the detents, you can divide the counts by 2.

DrDiettrich:
If you remove the ISR for channel A or B, you get only half the counts.

I deleted the Ch B code:

 // encoder pin B on interrupt 1 (pin 3)
  attachInterrupt(1, doEncoderB, CHANGE);

but it doesn't work.
I'm not sure how to implement this, do you have some code example?

cattledog:
To align the counts with the detents, you can divide the counts by 2.

I've added this

encoderPos = (encoderPos / 2);

to both ISR sections, but it didn't work.
What would be the correct implementation?

Thanks.

Divide the encoder position only when reading it in loop().

You might want to take a look at https://www.circuitsathome.com/mcu/reading-rotary-encoder-on-arduino/ .

DrDiettrich:
Divide the encoder position only when reading it in loop().

Thanks! What worked for me was adding a new variable, ranging from 0 to 511, as encoder counter,
then ecoderPos is dividing this by 2 in the loop.

Still it counts 1 at mid click.
Would it be better to change the encoder to a 24 detents but 12PPR resolution, like this one PEC11R-4220F-S0012 ?

stowite:
You might want to take a look at https://www.circuitsathome.com/mcu/reading-rotary-encoder-on-arduino/ .

Did not work smoothly, however interesting reading.

Thank you!

Would it be better to change the encoder to a 24 detents but 12PPR resolution, like this one PEC11R-4220F-S0012 ?

You do not need a new encoder.

I deleted the Ch B code but it doesn't work.

I think this is because your reading algorithm depends on both ISR's active. A_set and B_set are only determined by digitalRead() in the ISR for the A or B pin.

If dividing by two doesn't make you comfortable, then you need to use a one pin interrupt A CHANGE or B CHANGE algorithm which reads both pins in the ISR.

Try this algorithm. It may be the A or B pin interrupt which aligns best with the detents. You may have to adjust the logic to get the clockwise and counter clockwise correct.

void readEncoder()
{
  aState = digitalRead(outputA);
  bState = digitalRead(outputB);

  // If the outputB state is different to the outputA state, that means the encoder is rotating clockwise

  if (bState != aState)

  {
    counter ++;
  }
  else
  {
    counter --;
  }
}

cattledog:
Try this algorithm. It may be the A or B pin interrupt which aligns best with the detents. You may have to adjust the logic to get the clockwise and counter clockwise correct.

void readEncoder()

{
 aState = digitalRead(outputA);
 bState = digitalRead(outputB);

// If the outputB state is different to the outputA state, that means the encoder is rotating clockwise

if (bState != aState)

{
   counter ++;
 }
 else
 {
   counter --;
 }
}

Thanks!
Unfortunately it still counts 2 each detent.

The solution for me was:
changing the counter variable to constrain from 0 to 1023,
then have encoderPos to divide the result by 4.

Thank you all.