Jomelo / LCDMenuLib2

Create a tree menu. Use it with different lcd types / console output / ssh console.
MIT License
251 stars 48 forks source link

Overflow problem in LCDML_control #48

Closed paytongop closed 4 years ago

paytongop commented 5 years ago

Dear Jomelo, Thank you very much for the contribution. I found a bug in encoder menu control of LCDML_control. Current version can produce overflow problem after 30 days operation, and the press of encoder can't work any more. I fixed the problem with the following code. Thank you.

`

define encoder_A_pin 20 // physical pin has to be 2 or 3 to use interrupts (on mega e.g. 20 or 21), use internal pullups

define encoder_B_pin 21 // physical pin has to be 2 or 3 to use interrupts (on mega e.g. 20 or 21), use internal pullups

define encoder_button_pin 49 // physical pin , use internal pullup*/

define g_LCDML_CONTROL_button_long_press 800 // ms

define g_LCDML_CONTROL_button_short_press 120 // ms

define ENCODER_OPTIMIZE_INTERRUPTS //Only when using pin2/3 (or 20/21 on mega)

include //for Encoder Download: https://github.com/PaulStoffregen/Encoder

Encoder ENCODER(encoder_A_pin, encoder_B_pin);

unsigned long g_LCDML_CONTROL_button_press_time = 0; bool g_LCDML_CONTROL_button_press_time_positive = true; bool g_LCDML_CONTROL_button_prev = HIGH;

// void lcdml_menu_control(void) // { // If something must init, put in in the setup condition if(LCDML.BT_setup()) { // runs only once

// init pins  
pinMode(encoder_A_pin      , INPUT_PULLUP);
pinMode(encoder_B_pin      , INPUT_PULLUP);
pinMode(encoder_button_pin  , INPUT_PULLUP); 

}

//volitail Variabl long g_LCDML_CONTROL_Encoder_position = ENCODER.read(); bool button = digitalRead(encoder_button_pin);

if(g_LCDML_CONTROL_Encoder_position <= -3) {

if(!button)
{
  LCDML.BT_left();
  g_LCDML_CONTROL_button_prev = LOW;
  g_LCDML_CONTROL_button_press_time = 0;//g_LCDML_CONTROL_button_press_time = -1;
  g_LCDML_CONTROL_button_press_time_positive = false;
}
else
{
  LCDML.BT_down();
}    
ENCODER.write(g_LCDML_CONTROL_Encoder_position+4);

}
else if(g_LCDML_CONTROL_Encoder_position >= 3) {

if(!button)
{
  LCDML.BT_right();
  g_LCDML_CONTROL_button_prev = LOW;
  g_LCDML_CONTROL_button_press_time = 0;//g_LCDML_CONTROL_button_press_time = -1;
  g_LCDML_CONTROL_button_press_time_positive = false;
}
else
{
  LCDML.BT_up();
}
ENCODER.write(g_LCDML_CONTROL_Encoder_position-4);

}
else { if(!button && g_LCDML_CONTROL_button_prev) //falling edge, button pressed { g_LCDML_CONTROL_button_prev = LOW; g_LCDML_CONTROL_button_press_time = millis(); g_LCDML_CONTROL_button_press_time_positive = true; } else if(button && !g_LCDML_CONTROL_button_prev) //rising edge, button not active { g_LCDML_CONTROL_button_prev = HIGH;

   if(!g_LCDML_CONTROL_button_press_time_positive)//g_LCDML_CONTROL_button_press_time < 0)
   {
     g_LCDML_CONTROL_button_press_time = millis();
     g_LCDML_CONTROL_button_press_time_positive = true;
     //Reset for left right action
   }
   else if((millis() - g_LCDML_CONTROL_button_press_time) >= g_LCDML_CONTROL_button_long_press) 
   {
    if(LCDML.MENU_getScrollDisableStatus() != 0)
    {
      LCDML.MENU_enScroll(); 
    }
    else
    { 
      LCDML.BT_quit();
    }
   }
   else if((millis() - g_LCDML_CONTROL_button_press_time) >= g_LCDML_CONTROL_button_short_press)
   {
     LCDML.BT_enter();
   }
}

} } `

Jomelo commented 5 years ago

Hello, can you expain me where the overflow bug is ?

I use this code:

https://playground.arduino.cc/Code/TimingRollover/

...
if((millis() - g_LCDML_DISP_press_time) >= 200) {
      g_LCDML_DISP_press_time = millis(); // reset press time
   ...

and this code

...
else if((millis() - g_LCDML_CONTROL_button_press_time) >= g_LCDML_CONTROL_button_long_press)
       {
         LCDML.BT_quit();
       }
       else if((millis() - g_LCDML_CONTROL_button_press_time) >= g_LCDML_CONTROL_button_short_press)
       {
         LCDML.BT_enter();
       }
...

The calculation seems to be okay.

Thanks

paytongop commented 5 years ago

Hi, Majorly, the problem is come from "long g_LCDML_CONTROL_button_press_time" This variable use 'long' type. However, millis() return 'unsigned long' values. Hence, after 30 days, the return values make g_LCDML_CONTROL_button_press_time overflow.

Jomelo commented 5 years ago

Hey, thanks for reporting i will fix this in the next version.