rotaryEncoder increment problemW/full code

HI! I am using rotary encoder with LCD and serial monitor. To get an increment of 1, I need to turn click the rotary button twice as if I wanted to increase or decrease of 2 increments...
Any Idea of why and how to fix it .
Does it have anything to do with de attach interrupt code pinA and B are clk and dt encoder pins

//********************WORKS ON SERIAL MONITOR****************
/*******Interrupt-based Rotary Encoder Menu Sketch*******
 * by Simon Merrett, based on insight from Oleg Mazurov, Nick Gammon, rt and Steve Spence, and code from Nick Gammon
 * 3,638 bytes with debugging on UNO, 1,604 bytes without debugging
 */

 //Ajout mon lcd
 #include <LiquidCrystal_I2C.h> 
LiquidCrystal_I2C lcd(0x27, 20, 4);

// Rotary encoder declarations
static int pinA = 2; // Our first hardware interrupt pin is digital pin 2
static int pinB = 3; // Our second hardware interrupt pin is digital pin 3
volatile byte aFlag = 0; // let's us know when we're expecting a rising edge on pinA to signal that the encoder has arrived at a detent
volatile byte bFlag = 0; // let's us know when we're expecting a rising edge on pinB to signal that the encoder has arrived at a detent (opposite direction to when aFlag is set)
volatile byte encoderPos = 0; //this variable stores our current value of encoder position. Change to int or uin16_t instead of byte if you want to record a larger range than 0-255
volatile byte oldEncPos = 0; //stores the last encoder position value so we can compare to the current reading and see if it has changed (so we know when to print to the serial monitor)
volatile byte reading = 0; //somewhere to store the direct values we read from our interrupt pins before checking to see if we have moved a whole detent
// Button reading, including debounce without delay function declarations
const byte buttonPin = 4; // this is the Arduino pin we are connecting the push button to
byte oldButtonState = HIGH;  // assume switch open because of pull-up resistor
const unsigned long debounceTime = 10;  // milliseconds
unsigned long buttonPressTime;  // when the switch last changed state
boolean buttonPressed = 0; // a flag variable
// Menu and submenu/setting declarations
byte Mode = 0;   // This is which menu mode we are in at any given time (top level or one of the submenus)
const byte modeMax = 4; // This is the number of submenus/settings you want
const byte tempMax = 20; //Temperature max dans setting1
const byte tempMin = -5; //Temperature min dans setting1
const byte humiMax = 99; //Humidite max dans setting2
const byte humiMin = 35; //Humidite min dans setting2
byte setting1 = 0;  // a variable which holds the value we set 
byte setting2 = 0;  // a variable which holds the value we set 
byte setting3 = false;  // a variable which holds the value we set 
byte setting4 = false;  // a variable which holds the value we set 
/* Note: you may wish to change settingN etc to int, float or boolean to suit your application. 
 Remember to change "void setAdmin(byte name,*BYTE* setting)" to match and probably add some 
 "modeMax"-type overflow code in the "if(Mode == N && buttonPressed)" section*/

void setup() {
  //Rotary encoder section of setup
  pinMode(pinA, INPUT_PULLUP); // set pinA as an input, pulled HIGH to the logic voltage (5V or 3.3V for most cases)
  pinMode(pinB, INPUT_PULLUP); // set pinB as an input, pulled HIGH to the logic voltage (5V or 3.3V for most cases)
  attachInterrupt(0,PinA,RISING); // set an interrupt on PinA, looking for a rising edge signal and executing the "PinA" Interrupt Service Routine (below)
  attachInterrupt(1,PinB,RISING); // set an interrupt on PinB, looking for a rising edge signal and executing the "PinB" Interrupt Service Routine (below)
  // button section of setup
  pinMode (buttonPin, INPUT_PULLUP); // setup the button pin
  // DEBUGGING section of setup
  Serial.begin(9600);     // DEBUGGING: opens serial port, sets data rate to 9600 bps

  //démarre LCD
  lcd.init();                      // initialize the lcd   
  lcd.backlight();
  //message power ''on'' sur LCD
  lcd.setCursor(5,0); //Defining positon to write from first row, first column .
  lcd.print("CHARCUPRO");
  lcd.setCursor(3,2);
  lcd.print("Version 1.1.1");

  lcd.setCursor(0,2); //Second row, first column
  lcd.print(""); 
  delay(3000); //wait 8 sec
  lcd.clear(); //clear the whole LCD
   lcd.setCursor(1,1); 
  lcd.print("TEMP"); //text
  //----------------------
  lcd.setCursor(1,3); 
  lcd.print("HUMI"); //text
  //----------------------
  lcd.setCursor(13,1); 
  lcd.print("FAN"); //text
  //----------------------
  lcd.setCursor(13,3); 
  lcd.print("UV"); //text
  //----------------------
    lcd.setCursor(9,1); 
  lcd.print("C"); //text
  //----------------------
  lcd.setCursor(9,3); 
  lcd.print("%"); //text
  //----------------------
  lcd.setCursor(17,1); 
  lcd.print("0/1"); //text
  //----------------------
  lcd.setCursor(17,3); 
  lcd.print("0/1"); //text
  //----------------------
  
}

//void printLCD(){
  //These are the values which are not changing the operation
  
//}
void loop() {
  rotaryMenu();
  // carry out other loop code here 
 
}

void rotaryMenu() {  //This handles the bulk of the menu functions without needing to install/include/compile a menu library

     
  //DEBUGGING: Rotary encoder update display if turned
   if(oldEncPos != encoderPos) { // DEBUGGING
    Serial.println(encoderPos);// DEBUGGING. Sometimes the serial monitor may show a value just outside modeMax due to this function. The menu shouldn't be affected.
    oldEncPos = encoderPos;// DEBUGGING

   }
  // DEBUGGING
  // Button reading with non-delay() debounce - thank you Nick Gammon!
  byte buttonState = digitalRead (buttonPin); //toujours lire si bouton pesé ou pas
  if (buttonState != oldButtonState){
    if (millis () - buttonPressTime >= debounceTime){ // debounce
      buttonPressTime = millis ();  // when we closed the switch 
      oldButtonState =  buttonState;  // remember for next time 

      if (buttonState == LOW){
        Serial.println ("Button was pressed"); // DEBUGGING: print that button has been closed
        buttonPressed = 1;
      }
      else {
        Serial.println ("Button opened"); // DEBUGGING: print that button has been opened
        buttonPressed = 0;  
      }  
    }  // end if debounce time up
  } // end of  button state change

  //Main menu section
  if (Mode == 0) {
    if (encoderPos > (modeMax+10)) encoderPos = modeMax; // check we haven't gone out of bounds below 0 and correct if we have
    else if (encoderPos > modeMax) encoderPos = 0; // check we haven't gone out of bounds above modeMax and correct if we have

    //FLECHE DE CURSEUR
     switch(encoderPos){
     case 1:
     lcd.setCursor(0,3);
     lcd.print(" ");
     lcd.setCursor(11,3);
     lcd.print(" ");
     lcd.setCursor(11,1);
     lcd.print(" ");
     lcd.setCursor(0,1);
     lcd.print(">");
     break;

     case 2:
     lcd.setCursor(0,1);
     lcd.print(" ");
     lcd.setCursor(11,1);
     lcd.print(" ");
     lcd.setCursor(11,3);
     lcd.print(" ");
     lcd.setCursor(0,3);
     lcd.print(">");
     break;

     case 3:
     lcd.setCursor(0,1);
     lcd.print(" ");
     lcd.setCursor(0,3);
     lcd.print(" ");
     lcd.setCursor(11,3);
     lcd.print(" ");
     lcd.setCursor(11,1);
     lcd.print(">");
     break;

     case 4:
     lcd.setCursor(0,1);
     lcd.print(" ");
     lcd.setCursor(0,3);
     lcd.print(" ");
     lcd.setCursor(11,1);
     lcd.print(" ");
     lcd.setCursor(11,3);
     lcd.print(">");
     break;
     }

    if (buttonPressed){ 
      Mode = encoderPos; // set the Mode to the current value of input if button has been pressed
      Serial.print("Mode selected: "); //DEBUGGING: print which mode has been selected
      Serial.println(Mode); //DEBUGGING: print which mode has been selected
      buttonPressed = 0; // reset the button status so one press results in one action
      while (buttonPressed = 0){
        lcd.setCursor(7,1);
        lcd.print(setting1);
        lcd.setCursor(7,1);
        lcd.print("  ");

      }
    
      if (Mode == 1){

        Serial.println("TEMP_Mode 1"); //DEBUGGING: print which mode has been selected
        encoderPos = setting1; // start adjusting TEMP from last set point
        lcd.setCursor(7,1);
        lcd.print(encoderPos);
        lcd.setCursor(7,1);
        lcd.print("  ");

        
        
      }
      if (Mode == 2) {
        Serial.println("HUMID_Mode 2"); //DEBUGGING: print which mode has been selected
        encoderPos = setting2; // start adjusting HUMIDITY from last set point
        
      }
      if (Mode == 3) {
        Serial.println("FAN_Mode 3"); //DEBUGGING: print which mode has been selected
        encoderPos = setting3; // start turning FAN on or off
       
      }
      if (Mode == 4) {
        Serial.println("UV_Mode 4"); //DEBUGGING: print which mode has been selected
        encoderPos = setting4; // start turning UV on or off
       
      }
    }   
  }
   
  if (Mode == 1 && buttonPressed) {
    setting1 = encoderPos; // record whatever value your encoder has been turned to, to setting 1
    setAdmin(1,setting1);
    //code to do other things with setting1 here, perhaps update display 
    lcd.setCursor(7,1); 
    lcd.print(setting1);
    
  }
   
  if (Mode == 2 && buttonPressed) {
    setting2 = encoderPos; // record whatever value your encoder has been turned to, to setting 2
    setAdmin(2,setting2);
    //code to do other things with setting1 here, perhaps update display
    lcd.setCursor(7,3); 
    lcd.print(setting2);
   
  }
  if (Mode == 3 && buttonPressed){
    setting3 = encoderPos; // record whatever value your encoder has been turned to, to setting 3
    setAdmin(3,setting3);
    //code to do other things with setting3 here, perhaps update display 
  }
  if (Mode == 4 && buttonPressed){
    setting3 = encoderPos; // record whatever value your encoder has been turned to, to setting 4
    setAdmin(4,setting4);
    
    //code to do other things with setting4 here, perhaps update display 
  }
} 


// Carry out common activities each time a setting is changed
void setAdmin(byte name, byte setting){
  Serial.print("Setting "); //DEBUGGING
  Serial.print(name); //DEBUGGING
  Serial.print(" = "); //DEBUGGING
  Serial.println(setting);//DEBUGGING
  encoderPos = 0; // reorientate the menu index - optional as we have overflow check code elsewhere
  buttonPressed = 0; // reset the button status so one press results in one action
  Mode = 0; // go back to top level of menu, now that we've set values
  Serial.println("Main Menu"); //DEBUGGING
}

//Rotary encoder interrupt service routine for one encoder pin
void PinA(){
  cli(); //stop interrupts happening before we read pin values
  reading = PIND & 0xC; // read all eight pin values then strip away all but pinA and pinB's values
  if(reading == B00001100 && aFlag) { //check that we have both pins at detent (HIGH) and that we are expecting detent on this pin's rising edge
    encoderPos --; //decrement the encoder's position count
    bFlag = 0; //reset flags for the next turn
    aFlag = 0; //reset flags for the next turn
  }
  else if (reading == B00000100) bFlag = 1; //signal that we're expecting pinB to signal the transition to detent from free rotation
  sei(); //restart interrupts
}

//Rotary encoder interrupt service routine for the other encoder pin
void PinB(){
  cli(); //stop interrupts happening before we read pin values
  reading = PIND & 0xC; //read all eight pin values then strip away all but pinA and pinB's values
  if (reading == B00001100 && bFlag) { //check that we have both pins at detent (HIGH) and that we are expecting detent on this pin's rising edge
    encoderPos ++; //increment the encoder's position count
    bFlag = 0; //reset flags for the next turn
    aFlag = 0; //reset flags for the next turn
  }
  else if (reading == B00001000) aFlag = 1; //signal that we're expecting pinA to signal the transition to detent from free rotation
  sei(); //restart interrupts
}
// end of sketch!

The problem likely is somewhere in the code you did not include.

Okay would you clearly see it if you saw the code ?

Perhaps we could see why you are mixing 8 bit and 16 bit data and compares.

Here is a novel idea: post all the code, along with a wiring diagram with pins and connections clearly labeled, then check back later to see if anyone has spotted the errors.

amazing Idea I will start by updating the full code post. and I will check the wiring of the encoder. I wonder if it is not a pullup or down resistor needed on the rotary pins ...

Perhaps you can tell from the code identifying those particular pins.

I am on 2 and 3 the right pins for encoder interrupt

Your question was relating to pullup and pulldown resistors. If your code sets the pins as inpullup, then the internal resistor is used. If then pins are identified as just input, then external resistors are required and then the decision depends on how you have the interrupt reason defined. See why the full code is important?

Which Arduino are you using?

The code you posted will work only on ATmega328-based Arduinos, like the Uno R3.

I am using original Uno with atmega 328p

Your button press (pin 4) is reading PRESS and RELEASE every button press and your rotary switch is doing two things: moving the cursor AND increasing the number of button presses AND if you rotate CCW when count is ZERO, you get 255 button presses.

Draw a flow chart to show what happens when the knob is rotated and when the switch/button is pressed.

There are basically two types of rotary encoders. One has the same number of detents as pulses per revolution. A pulse is a full cycle of both switches, with both switches being open at every detent. It looks like your code is written for that type of encoder.

The other type has half as many pulses as detents per revolution. This means detents alternate between both swithes open and both switches closed. So essentially a detent is inserted in the middle of a full pulse.

I'd suggest you test your encoder to see which type it is. If your software is written for the first type, but you have the second type, then you will need to turn it through two detents to get one tick. If that's the problem, the solution would be to get the right kind of encoder, or change your code to something involving a pin change instead of just the rising edge.

I think that would make sense, the encoder type. I will take the time to check that. Thanks for the suggestion.

If you show us what encoder you have (Picture + link ) and how you have connected it, that would help in solving your issue.

It is a HW-040. Some litterature sais 20 clicks per revolution and other sais 30 click per revolution. Mine is 30 clicks per revolution, I get 15 HIGH per revolution. I plugged pinA and pinB each on a different led and I get this:
1st click = two leds HIGH. Second click = two leds LOW. Of course I get one of the two leds HiGH or LOW before the other one in between the clicks to figure CW and CCW

It is a HW-040. Some litterature sais 20 clicks per revolution and other sais 30 click per revolution. Mine is 30 clicks per revolution, I get 15 HIGH per revolution. I plugged pinA and pinB each on a different led and I get this:
1st click = two leds HIGH. Second click = two leds LOW. Of course I get one of the two leds HiGH or LOW before the other one in between the clicks to figure CW and CCW

HW-40 is the name of the module. Does the encoder itself have a number on the side?

Anyway, if you get both lines low at a detent, alternating with both high at the next detent, then you have the second type of encoder, which doesn't match your software. What would match would always give you both high at every detent. I don't know if the HW-40 is supposed to be this way or not. I have a couple KY-040 modules, and they have the first type of encoder.

I think you can change your software and make it work. I think you're doing everything with a rising edge when both lines return to high. Maybe you could then switch it over to falling edge and do an additional tick when both lines are low. You can change the EICRA register to switch the edge. Or maybe you could use pin change interrupts.

Do you do any debouncing?

Here's my idea of code changes that might work with that encoder. I haven't run it, or even compiled it, but it might work. Basically, you just switch back and forth between rising and falling edge, looking for either both high or both low, instead of only both high.

bool rising = true;   //global variable

void setup() {
  pinMode(pinA, INPUT_PULLUP);
  pinMode(pinB, INPUT_PULLUP);
  if (digitalRead(pinA)) {
    attachInterrupt(0,PinA,FALLING);
    attachInterrupt(1,PinB,FALLING);
    rising = false;
  }
  else {
    attachInterrupt(0,PinA,RISING);
    attachInterrupt(1,PinB,RISING);
  }
.
.
.


void PinA(){
  cli();
  reading = PIND & 0xC;
  if (rising) {
    if(reading == B00001100 && aFlag) {
      encoderPos --;
      bFlag = 0;
      aFlag = 0;
      EICRA &= B11111010;
      rising = false;
    }
    else if (reading == B00000100) bFlag = 1;
  }
  else {
    if(reading == 0 && aFlag) {
      encoderPos --;
      bFlag = 0;
      aFlag = 0;
      EICRA |= B00000101;
      rising = true;
    }
    else if (reading == B00001000) bFlag = 1;
  }
  sei();
}

void PinB(){
  cli();
  reading = PIND & 0xC;
  if (rising) {
    if (reading == B00001100 && bFlag) {
      encoderPos ++;
      bFlag = 0;
      aFlag = 0;
      EICRA &= B11111010;
      rising = false;
    }
    else if (reading == B00001000) aFlag = 1;
  }
  else {
    if (reading == 0 && bFlag) {
      encoderPos ++;
      bFlag = 0;
      aFlag = 0;
      EICRA |= B00000101;
      rising = true;
    }
    else if (reading == B00000100) aFlag = 1;
  }
    sei();
}

My code does not seem to be a debounce on the A and B pins, only on the press button. I am not advanced in coding ( this one is based on a borrowed code).

I was going to chage the encoder. But seeing your code, I will read, understand and try it. Thanks for this one. I will get back to you rather if it works or not.