I2C LCD causes problems

Hello all, this is my first post in a good while. I do hope I've put this in the correct spot - if not let me know with info on how to move topic to correct place.

I've got a powerfeed for a milling machine (Grizzly G-619) project using a Nano, NEMA 24 stepper motor, and DM556S driver. The code has worked good for a couple years. I've decided it would be good to put an LCD display for motor RPM and Inches Per Minute (IPM). For testing I used serial monitor to display RPM and IPM. That works good.

BUT, when I use a LCD with I2C the motor then the info is displayed on the LCD, but the update is slow. Also the motor only turns while the LCD update is taking place. Motor turns left in 1/8 turn steps, pauses until next update (couple of seconds or so), then another 1/8th turn. Pressing STOP button stops.

Even with all the lcd portion and from "lcd.backlight(); commented out, and ONLY the "lcd.begin();" line active it doesn't work. That single line "lcd.begin();" causes the problems as described above. As if there were something in the "<LiquidCrystal_I2C.h>" that's causing the problem. Is that when the library actually becomes active when the "lcd.begin();" line is invoked?

I test each section by commenting out (/....../) each unused section.

I edited post to put code in proper window

#include <AccelStepper.h>
#include <LiquidCrystal_I2C.h>

LiquidCrystal_I2C lcd(0x27, 16, 2);

// Define the stepper and the pins it will use
AccelStepper stepper1(AccelStepper::DRIVER, 8, 9); //8=PUL, 9=DIR "AccelStepper::DRIVER" cam be changed to "1" ok

// Define our three input button pins
#define  LEFT_PIN  5
#define  STOP_PIN  4
#define  RIGHT_PIN 3
#define ENA 7  //So the stepper will turn freely when STOP is pressed.

// Define our analog pot input pin
#define  SPEED_PIN A0

// Define our maximum and minimum speed in steps per second (scale pot to these)
#define  MAX_SPEED 5000 //this was set to 500, but I changed to higher number, might set higher later
#define  MIN_SPEED 0.0  //This was set to 0.1, but I changed to 0 so rotation would stop at min setting

float current_speed = 0.0;         // Holds current motor speed in steps/second
int analog_read_counter = 1000;    // Counts down to 0 to fire analog read
char sign = 0;                     // Holds -1, 1 or 0 to turn the motor on/off and control direction
int analog_value = 0;  // Holds raw analog value.
int rpm = 0;
float ips = 0.0;

void setup() {
  // The only AccelStepper value we have to set here is the max speeed, which is higher than we'll ever go
  stepper1.setMaxSpeed(10000.0);

  // initialize serial communications at 9600 bps:
  Serial.begin(9600);

  // Set up the three button inputs, with pullups
  pinMode(LEFT_PIN, INPUT_PULLUP);
  pinMode(STOP_PIN, INPUT_PULLUP);
  pinMode(RIGHT_PIN, INPUT_PULLUP);
  pinMode (ENA, OUTPUT);
}

void loop() {

  // If a switch is pushed down (low), set the sign value appropriately
  if (digitalRead(LEFT_PIN) == 0) {
    sign = 1; digitalWrite(ENA, HIGH);
  }
  else if (digitalRead(RIGHT_PIN) == 0) {
    sign = -1; digitalWrite(ENA, HIGH);
  }
  else if (digitalRead(STOP_PIN) == 0) {
    sign = 0; digitalWrite(ENA, LOW);
  }

  // We only want to read the pot every so often (because it takes a long time we don't
  // want to do it every time through the main loop).
  if (analog_read_counter > 0) {
    analog_read_counter--;
  }

  else {
    analog_read_counter = 3000;  //originally at 3000

    // Now read the pot (from 0 to 1023)
    analog_value = analogRead(SPEED_PIN);

    // Give the stepper a chance to step if it needs to
    stepper1.runSpeed();

    //  And scale the pot's value from min to max speeds
    current_speed = sign * (((analog_value / 1023.0) * (MAX_SPEED - MIN_SPEED)) + MIN_SPEED);

    // Update the stepper to run at this new speed
    stepper1.setSpeed(current_speed);

  rpm = (analog_value / 5.5297);
  ips = (analog_value / 56.8333);
    
    
         Serial.print("RPM  = ");
          Serial.println(rpm);
          Serial.println();


          //rpm = (analog_value / 5.297);  //These numbers will need fiddling with to provide around IPM speed
          Serial.print("IPM = ");  //the (F) reduces the amount of dynamic memory used
          Serial.print(ips);  //serial print works good, prints everything as it should and runs motor smooth
          Serial.println();
          Serial.println();
  }

  /* 
    lcd.begin();

    lcd.backlight();
    lcd.clear();

    lcd.setCursor(2, 0); // set the LCD cursor position
    lcd.print("RPM = ");
    lcd.print(rpm);

    lcd.setCursor(2, 1); // set the LCD cursor position
    lcd.print("IPM = ");
    lcd.print(ips, 1);
  }
*/
  // This will run the stepper at a constant speed
  stepper1.runSpeed();
}

belongs in the void setup (),
NEVER in void loop () this brakes the loop and thus the entire engine run completely.

Your post was MOVED to its current location as it is more suitable.

Please follow the advice given in the link below when posting code, in particular the section entitled 'Posting code and common code problems'

Use code tags (the </> icon above the compose window) to make it easier to read and copy for examination

Dummy me - of course lcd.begin should be up in the void setup {}, right where I've got the serial.begin(9600); line. The code now works good.

Thank you for moving the topic. I looked over several of the forums and just couldn't make up my mind where it should go.

I've still got some fiddling to do with something - the motor has a "jerky" movement in time with the flickering in the LCD, from updating I would think? Comments please.

There is no need to update the LCD every time through loop. 3 or 4 times a second is more than a human can really keep up with. Use the Blink without delay method to update the display less often. Plus writing to an LCD is kind of slow so don't do it unless it is appropriate.

Or better yet, only update the display when the data changes.

Do not use the clear() function in loop(). The clear() function is slooow so is a major cause of flickering. Use the setCursor function to move the cursor and write spaces to the display to overwrite old data.

here is code to illustrate regular but less often updates and using spaces to overwrite old data:

void loop()
{
   // the other part of your code
   updateDisplay();   
}

void updateDisplay()
{
   static unsigned long timer = 0;
   unsigned long interval = 500;  // update display 2 times / second
   if (millis() - timer >= interval)
   {
      timer = millis();
      lcd.setCursor(2, 0); // set the LCD cursor position
      lcd.print("RPM = ");
      lcd.setCursor(8,0);
      lcd.print("      "); // overwrite old data
      lcd.setCursor(8,0);  // reset cursor
      lcd.print(rpm);

      lcd.setCursor(2, 1); // set the LCD cursor position
      lcd.print("IPM = ");
      lcd.setCursor(8,1);
      lcd.print("      "); // overwrite old data
      lcd.setCursor(8,1);  // reset cursor
      lcd.print(ips, 1);
   }
}

Read the forum guidelines to see how to properly post code and some hints on how to get the most from this forum.
Use the IDE autoformat tool (ctrl-t or Tools, Auto format) before posting code in code tags.

OK, I hope I posted the code correctly this time.

Thank you groundFungus for the info. I've modified the code as below. I think I followed your instructions. The code uploads and has improved operation. The LCD updates nicely with no flicker at all. The motor runs, but there is a "jerk" about 1 per second during the rotation. With the LCD portion active, only printing to serial monitor the motor runs very smooth. Only with the LCD is the motor no smooth.

Thank all of ya'll (UKHeliBob also) for the help. The program is a LOT closer to working with LCD than before.


#include <AccelStepper.h>
#include <LiquidCrystal_I2C.h>

LiquidCrystal_I2C lcd(0x27, 16, 2);

// Define the stepper and the pins it will use
AccelStepper stepper1(AccelStepper::DRIVER, 8, 9); //8=PUL, 9=DIR "AccelStepper::DRIVER" cam be changed to "1" ok

// Define our three input button pins
#define  LEFT_PIN  5
#define  STOP_PIN  4
#define  RIGHT_PIN 3
#define ENA 7  //So the stepper will turn freely when STOP is pressed.

// Define our analog pot input pin
#define  SPEED_PIN A0

// Define our maximum and minimum speed in steps per second (scale pot to these)
#define  MAX_SPEED 5000 //this was set to 500, but I changed to higher number, might set higher later
#define  MIN_SPEED 0.0  //This was set to 0.1, but I changed to 0 so rotation would stop at min setting

float current_speed = 0.0;         // Holds current motor speed in steps/second
int analog_read_counter = 1000;    // Counts down to 0 to fire analog read
char sign = 0;                     // Holds -1, 1 or 0 to turn the motor on/off and control direction
int analog_value = 0;  // Holds raw analog value.
int rpm = 0;
float ips = 0.0;
int timer = 0;

void setup() {
  // The only AccelStepper value we have to set here is the max speeed, which is higher than we'll ever go
  stepper1.setMaxSpeed(10000.0);

  // initialize serial communications at 9600 bps:
  //Serial.begin(9600);
  lcd.begin();

  // Set up the three button inputs, with pullups
  pinMode(LEFT_PIN, INPUT_PULLUP);
  pinMode(STOP_PIN, INPUT_PULLUP);
  pinMode(RIGHT_PIN, INPUT_PULLUP);
  pinMode (ENA, OUTPUT);
}

void loop() {

  // If a switch is pushed down (low), set the sign value appropriately
  if (digitalRead(LEFT_PIN) == 0) {
    sign = 1; digitalWrite(ENA, HIGH);
  }
  else if (digitalRead(RIGHT_PIN) == 0) {
    sign = -1; digitalWrite(ENA, HIGH);
  }
  else if (digitalRead(STOP_PIN) == 0) {
    sign = 0; digitalWrite(ENA, LOW);
  }

  // We only want to read the pot every so often (because it takes a long time we don't
  // want to do it every time through the main loop).
  if (analog_read_counter > 0) {
    analog_read_counter--;
  }

  else {
    analog_read_counter = 3000;  //originally at 3000

    // Now read the pot (from 0 to 1023)
    analog_value = analogRead(SPEED_PIN);

    // Give the stepper a chance to step if it needs to
    stepper1.runSpeed();

    //  And scale the pot's value from min to max speeds
    current_speed = sign * (((analog_value / 1023.0) * (MAX_SPEED - MIN_SPEED)) + MIN_SPEED);

    // Update the stepper to run at this new speed
    stepper1.setSpeed(current_speed);

  rpm = (analog_value / 5.5297);
  ips = (analog_value / 56.8333);

//    updateDisplay();
    /*
         Serial.print("RPM  = ");
          Serial.println(rpm);
          Serial.println();


          //rpm = (analog_value / 5.297);  //These numbers will need fiddling with to provide around IPM speed
          Serial.print("IPM = ");  //the (F) reduces the amount of dynamic memory used
          Serial.print(ips);  //serial print works good, prints everything as it should and runs motor smooth
          Serial.println();
          Serial.println();
  }*/
  }
  {
void updateDisplay();

  static unsigned long timer = 0;
   unsigned long interval = 500;  // update display 2 times / second
   if (millis() - timer >= interval)
   {
      timer = millis();
      lcd.setCursor(2, 0); // set the LCD cursor position
      lcd.print("RPM = ");
      lcd.setCursor(8,0);
      lcd.print("      "); // overwrite old data
      lcd.setCursor(8,0);  // reset cursor
      lcd.print(rpm);

      lcd.setCursor(2, 1); // set the LCD cursor position
      lcd.print("IPM = ");
      lcd.setCursor(8,1);
      lcd.print("      "); // overwrite old data
      lcd.setCursor(8,1);  // reset cursor
      lcd.print(ips, 1);
   }
/*
      lcd.backlight();
    //lcd.clear();

    lcd.setCursor(2, 0); // set the LCD cursor position
    lcd.print("RPM = ");
    lcd.print(rpm);

    lcd.setCursor(2, 1); // set the LCD cursor position
    lcd.print("IPM = ");
    lcd.print(ips, 1);
    */
  }

  // This will run the stepper at a constant speed
  stepper1.runSpeed();

}

The stepper1.runSpeed(); should be in loop() not the updateDisplay function.

Untested:

void loop()
{
   // If a switch is pushed down (low), set the sign value appropriately
   if (digitalRead(LEFT_PIN) == 0)
   {
      sign = 1; digitalWrite(ENA, HIGH);
   }
   else if (digitalRead(RIGHT_PIN) == 0)
   {
      sign = -1; digitalWrite(ENA, HIGH);
   }
   else if (digitalRead(STOP_PIN) == 0)
   {
      sign = 0; digitalWrite(ENA, LOW);
   }

   // We only want to read the pot every so often (because it takes a long time we don't
   // want to do it every time through the main loop).
   if (analog_read_counter > 0)
   {
      analog_read_counter--;
   }

   else
   {
      analog_read_counter = 3000;  //originally at 3000

      // Now read the pot (from 0 to 1023)
      analog_value = analogRead(SPEED_PIN);

      // Give the stepper a chance to step if it needs to
      stepper1.runSpeed();

      //  And scale the pot's value from min to max speeds
      current_speed = sign * (((analog_value / 1023.0) * (MAX_SPEED - MIN_SPEED)) + MIN_SPEED);

      // Update the stepper to run at this new speed
      stepper1.setSpeed(current_speed);

      rpm = (analog_value / 5.5297);
      ips = (analog_value / 56.8333);
   }
   // This will run the stepper at a constant speed
   stepper1.runSpeed();
   updateDisplay();
}

void updateDisplay()
{
   static unsigned long timer = 0;
   unsigned long interval = 500;  // update display 2 times / second
   if (millis() - timer >= interval)
   {
      timer = millis();
      lcd.setCursor(2, 0); // set the LCD cursor position
      lcd.print("RPM = ");
      lcd.setCursor(8, 0);
      lcd.print("      "); // overwrite old data
      lcd.setCursor(8, 0); // reset cursor
      lcd.print(rpm);

      lcd.setCursor(2, 1); // set the LCD cursor position
      lcd.print("IPM = ");
      lcd.setCursor(8, 1);
      lcd.print("      "); // overwrite old data
      lcd.setCursor(8, 1); // reset cursor
      lcd.print(ips, 1);
   }
}

The hd44780 library for I2C backpack 1602 and 2004 LCDs (among others) is faster than the old LiquidCrystal_I2C libraries. Plus it is actively maintained. The library is available through the library manager.

The stepper library you are using, AccelStepper, is, IMO, not a very robust design, in terms of ensuring reliable/stable operation.
This is because it has some critical realtime requirements, but does do all it can to ensure that those realtime requirements are always met.
i.e. it requires constant and rapid calling of functions and if those are not called fast enough it can cause issues.
The best way to handle stuff like this in a simple/non-scheduling environment like Arduino is using interrupts, but this code doesn't work that way.
What that means is that if you do something in your code that takes longer than that code can tolerate, there can be issues.

Writing to an I2C lcd using a PCF8574 backpack can take quite a bit of time.
When using the LiquidCrystal_I2C library on a 16mHz avr, it takes just under 1487us (1.5ms) per byte write to the display.
You could reduce that to 554us by switching to the hd44780 library.
You could further reduce that to 200us if you use the hd44780 library and bump the i2c clock up to 400kHz.

So for example, here are the times for what you are doing when using the LiquidCrystal_I2C library:

1.5ms      lcd.setCursor(2, 0); // set the LCD cursor position
  9ms      lcd.print("RPM = ");
1.5ms      lcd.setCursor(8,0);
  9ms      lcd.print("      "); // overwrite old data
1.5ms      lcd.setCursor(8,0);  // reset cursor
  6ms      lcd.print(rpm); // assuming 4 digits

1.5ms      lcd.setCursor(2, 1); // set the LCD cursor position
  9ms      lcd.print("IPM = ");
1.5ms      lcd.setCursor(8,1);
  9ms      lcd.print("      "); // overwrite old data
1.5ms      lcd.setCursor(8,1);  // reset cursor
  6ms      lcd.print(ips, 1); // assuming 4 digits

Total of around 57ms.
That may be a bit too much time delay for the AccelStepper library and motor to to handle for maintaining smooth operation.

While you can reduce this amount of time significantly by doing various things, you can't eliminate it.
I'm not sure how much jitter in the calls the AccelStepper library can tolerate.

The easiest thing would be convert to the hd44780 library using the hd44780_I2Cexp i/o class.
That will cut the display update overhead by 2/3 without changing any code.
Then, for further reduction, you can be smarter about what you write to the display.
While updating less often can help, it only reduces how often the overhead occurs but does not reduce the actual overhead - so it may not solve the issues.
What really helps is to be smarter about what is sent to the display by only writing the areas that have changed to reduce the number of total writes to the display which will reduce the total overhead of an update.
For example, if you have "RPM = " always on the display, only write it once.
Then always write the value as a fixed field, say 4 digits.
That will reduce your display writes to only 10 data bytes. for the two fields.

And I would advise against doing things like writing spaces to clear a field before writing the new value.
The way that code does it, not only will it cause some flicker during the update, add extra time overhead but the value displayed will dance around since it is left adjusted.
The best way to do this is to NOT use the number formatting capability built into the Arduino Print class since it is too wimpy.
But rather write a fixed width number field to the display, say 4 or 6 digits so that the number is left adjusted.
You can either do the formatting yourself, or be lazy (like I am) and use sprintf() to format the number into a buffer - you can choose to fill the leading digit positions with either spaces or zeros - and print the buffer using a single print() call.
This reduces the display update to being just two setCursor() and two print() calls (one for rpm and one for ips)
For a total of 10 total byte writes to the display. 1 for each setCursor() and 1 for each character.

So if you switched libraries and only wrote two 4 digit fixed width fields, the overhead would reduce from
57ms per update to 5.5ms (10 total display byte writes) and if you bumped the clock to 400kHz it reduces to around 2ms.

--- bill

1 Like

Bill, Thank you for input. You've written some stuff that's really over my head. So, it's time for me to do some digging and reading. I did do some looking for that hd44780 library, and added it to the IDE. Does using that library do away with I2C and wire the LCD with the 4 data lines?

Anyway, let me to some "absorbing" on your post.

groundFungus: Allow me a bit of time to look over your response. I just installed the hd44780 library by Bill Perry and need a few minutes to see the different commands to use to implement that library. It doesn't seem to be as simple as removing the "include <LiquidCrystal_I2C.h>" line and adding the hd44780 line. Wife is hollering for me now - Thank you all for the help and guidance.

It is that simple.
The hd44780 API functions are compatible with API functions in LiquidCrystal and LCD API 1.0
The only changes you should need to do are to change the include and lcd object declaration/constructor.
But as recommended, you should first run the included I2CexpDiag sketch to verify that everything is working.
After that reports that everything is working, you can look at the other hd44780_I2Cexp i/o class examples and your own code.

You bring up the documentation using the Documentation sketch.

--- bill

Thank ya'll for all the help - I've changed to the hd44780 library and motor is running smooth, but the LCD isn't updating.

It was mentioned "The stepper1.runSpeed(); should be in loop() not the updateDisplay function."

   if (digitalRead(LEFT_PIN) == 0)
  {
    sign = 1; digitalWrite(ENA, HIGH);
  }
  else if (digitalRead(RIGHT_PIN) == 0)
  {
    sign = -1; digitalWrite(ENA, HIGH);
  }
  else if (digitalRead(STOP_PIN) == 0)
  {
    sign = 0; digitalWrite(ENA, LOW);
  }
stepper1.runSpeed();

when I moved it up stepper motor runs smoothly, only thing is the LCD doesn't update with speed change. With the serial code in for the serial monitor motor runs smoothly and serial monitor shows speed changes smoothly. Here is the code as described above.

#include <Wire.h>

#include <AccelStepper.h>


#include <hd44780.h>  // main hd44780 header
#include <hd44780ioClass/hd44780_I2Cexp.h>  // i2c expander i/o class header
hd44780_I2Cexp lcd; // declare lcd object: auto locate & auto config expander chip

const int LCD_COLS = 16;
const int LCD_ROWS = 2;

// Define the stepper and the pins it will use
AccelStepper stepper1(AccelStepper::DRIVER, 8, 9); //8=PUL, 9=DIR "AccelStepper::DRIVER" cam be changed to "1" ok

// Define our three input button pins
#define  LEFT_PIN  5
#define  STOP_PIN  4
#define  RIGHT_PIN 3
#define ENA 7  //So the stepper will turn freely when STOP is pressed.

// Define our analog pot input pin
#define  SPEED_PIN A0

// Define our maximum and minimum speed in steps per second (scale pot to these)
#define  MAX_SPEED 5000 //this was set to 500, but I changed to higher number, might set higher later
#define  MIN_SPEED 0.0  //This was set to 0.1, but I changed to 0 so rotation would stop at min setting

float current_speed = 0.0;         // Holds current motor speed in steps/second
int analog_read_counter = 1000;    // Counts down to 0 to fire analog read
char sign = 0;                     // Holds -1, 1 or 0 to turn the motor on/off and control direction
int analog_value = 0;  // Holds raw analog value.
int rpm = 0;
float ips = 0.0;
int timer = 0;

void setup() {
  // The only AccelStepper value we have to set here is the max speeed, which is higher than we'll ever go
  stepper1.setMaxSpeed(10000.0);

  // initialize serial communications at 9600 bps:
  //Serial.begin(9600);

  //int status;

  // Set up the three button inputs, with pullups
  pinMode(LEFT_PIN, INPUT_PULLUP);
  pinMode(STOP_PIN, INPUT_PULLUP);
  pinMode(RIGHT_PIN, INPUT_PULLUP);
  pinMode (ENA, OUTPUT);
}

void loop() {

  // If a switch is pushed down (low), set the sign value appropriately
  if (digitalRead(LEFT_PIN) == 0)
  {
    sign = 1; digitalWrite(ENA, HIGH);
  }
  else if (digitalRead(RIGHT_PIN) == 0)
  {
    sign = -1; digitalWrite(ENA, HIGH);
  }
  else if (digitalRead(STOP_PIN) == 0)
  {
    sign = 0; digitalWrite(ENA, LOW);
  }
  stepper1.runSpeed();  // This will run the stepper at a constant speed

  // We only want to read the pot every so often (because it takes a long time we don't
  // want to do it every time through the main loop).
  if (analog_read_counter > 0) {
    analog_read_counter--;
  }

  else {
    analog_read_counter = 3000;  //originally at 3000

    // Now read the pot (from 0 to 1023)
    analog_value = analogRead(SPEED_PIN);

    // Give the stepper a chance to step if it needs to
    stepper1.runSpeed();

    //  And scale the pot's value from min to max speeds
    current_speed = sign * (((analog_value / 1023.0) * (MAX_SPEED - MIN_SPEED)) + MIN_SPEED);

    // Update the stepper to run at this new speed
    stepper1.setSpeed(current_speed);


    rpm = (analog_value / 5.5297);
    ips = (analog_value / 56.8333);

    {
      void updateDisplay();

      static unsigned long timer = 0;
      unsigned long interval = 500;  // update display 2 times / second
      if (millis() - timer >= interval)

        timer = millis();

      lcd.setCursor(2, 0); // set the LCD cursor position
      lcd.print("RPM = ");
      lcd.setCursor(8, 0);
      lcd.print("      "); // overwrite old data
      lcd.setCursor(8, 0); // reset cursor
      lcd.print(rpm);

      lcd.setCursor(2, 1); // set the LCD cursor position
      lcd.print("IPM = ");
      lcd.setCursor(8, 1);
      lcd.print("      "); // overwrite old data
      lcd.setCursor(8, 1); // reset cursor
      lcd.print(ips, 1);
    }
  }
}

You can't write to the LCD display until you initialize it.
You must first call begin(cols, rows) to initialize the lcd just like with any other "LiquidCrystal" API type library.

Also you have a dummy function declaration for a function that you never define or call.

this line should be removed:

       void updateDisplay();

--- bill

Also noticed the way you refactored the code, the code to update the display will be called all the time.
You have broken the code by removing some necessary curly braces on the if() that checks for elapsed time.

Curious why you decided to try to pull the updateDisplay() code into loop() rather than just leave it as its own function.

--- bill

Thanks for the fast response Bill - ALL the responses have been very fast on this forum. Thank ya'll again.

int status;
status = lcd.begin(LCD_COLS, LCD_ROWS);

I sure thought I had that line included - now it prints to LCD nicely.
When compiling why does it give me the error msg of status set but not used?: "/tmp/arduino_modified_sketch_226747/StepperDriver_Ken_I2C-44780.ino: In function 'void setup()':
/tmp/arduino_modified_sketch_226747/StepperDriver_Ken_I2C-44780.ino:41:5: warning: variable 'status' set but not used [-Wunused-but-set-variable]
int status;"

Bill, I have no real idea why I'm doing anything with code - well, some idea but not all that much. You mentioned I "refactored the code" causing the display to be called all the time - what do you mean by that? Is that the necessary curly braces I removed on the if()? Could you perhaps provide a tad more guidance for this neophyte?

Didn't I remove the "updateDisplay()" code?

I moved the updateDisplay function back to being a function outside of loop(). It is a good idea to write code using functions instead of one long in line program. See the tutorial that I linked.

You see that the function is defined outside of loop(), but is called from within loop(). This code will compile but is not tested. The updateDisplay and stepper1.setSpeed functions should be called every time through loop(). Those functions should not be in an else if or if. Those functions have timers inside so that they only do things when it is time. The display will only update 2 times a second and the stepper will only step when a step is due even though the function is called hundreds or thousand of times a second. See the tutorial that I previously posted and
Beginner's guide to millis().
Several things at a time.

#include <Wire.h>

#include <AccelStepper.h>

#include <hd44780.h>  // main hd44780 header
#include <hd44780ioClass/hd44780_I2Cexp.h>  // i2c expander i/o class header
hd44780_I2Cexp lcd; // declare lcd object: auto locate & auto config expander chip

const int LCD_COLS = 16;
const int LCD_ROWS = 2;

// Define the stepper and the pins it will use
AccelStepper stepper1(AccelStepper::DRIVER, 8, 9); //8=PUL, 9=DIR "AccelStepper::DRIVER" cam be changed to "1" ok

// Define our three input button pins
#define  LEFT_PIN  5
#define  STOP_PIN  4
#define  RIGHT_PIN 3
#define ENA 7  //So the stepper will turn freely when STOP is pressed.

// Define our analog pot input pin
#define  SPEED_PIN A0

// Define our maximum and minimum speed in steps per second (scale pot to these)
#define  MAX_SPEED 5000 //this was set to 500, but I changed to higher number, might set higher later
#define  MIN_SPEED 0.0  //This was set to 0.1, but I changed to 0 so rotation would stop at min setting

float current_speed = 0.0;         // Holds current motor speed in steps/second
int analog_read_counter = 1000;    // Counts down to 0 to fire analog read
char sign = 0;                     // Holds -1, 1 or 0 to turn the motor on/off and control direction
int analog_value = 0;  // Holds raw analog value.
int rpm = 0;
float ips = 0.0;
int timer = 0;

void setup()
{
   // The only AccelStepper value we have to set here is the max speeed, which is higher than we'll ever go
   stepper1.setMaxSpeed(10000.0);

   // initialize serial communications at 9600 bps:
   //Serial.begin(9600);

   //int status;

   // Set up the three button inputs, with pullups
   pinMode(LEFT_PIN, INPUT_PULLUP);
   pinMode(STOP_PIN, INPUT_PULLUP);
   pinMode(RIGHT_PIN, INPUT_PULLUP);
   pinMode (ENA, OUTPUT);
}

void loop()
{

   // If a switch is pushed down (low), set the sign value appropriately
   if (digitalRead(LEFT_PIN) == 0)
   {
      sign = 1; digitalWrite(ENA, HIGH);
   }
   else if (digitalRead(RIGHT_PIN) == 0)
   {
      sign = -1; digitalWrite(ENA, HIGH);
   }
   else if (digitalRead(STOP_PIN) == 0)
   {
      sign = 0; digitalWrite(ENA, LOW);
   }
   stepper1.runSpeed();  // This will run the stepper at a constant speed

   // We only want to read the pot every so often (because it takes a long time we don't
   // want to do it every time through the main loop).
   if (analog_read_counter > 0)
   {
      analog_read_counter--;
   }

   else
   {
      analog_read_counter = 3000;  //originally at 3000

      // Now read the pot (from 0 to 1023)
      analog_value = analogRead(SPEED_PIN);

      // Give the stepper a chance to step if it needs to
      stepper1.runSpeed();  //  ********  should not need this.  

      //  And scale the pot's value from min to max speeds
      current_speed = sign * (((analog_value / 1023.0) * (MAX_SPEED - MIN_SPEED)) + MIN_SPEED);


   }

   // Update the stepper to run at this new speed
   stepper1.setSpeed(current_speed);

   rpm = (analog_value / 5.5297);
   ips = (analog_value / 56.8333);

   updateDisplay();
}

void updateDisplay()
{
   static unsigned long timer = 0;
   unsigned long interval = 500;  // update display 2 times / second
   if (millis() - timer >= interval)
   {
      timer = millis();

       lcd.setCursor(2, 0); // set the LCD cursor position
       lcd.print("RPM = ");
       lcd.setCursor(8, 0);
       lcd.print("      "); // overwrite old data
       lcd.setCursor(8, 0); // reset cursor
       lcd.print(rpm);

       lcd.setCursor(2, 1); // set the LCD cursor position
       lcd.print("IPM = ");
       lcd.setCursor(8, 1);
       lcd.print("      "); // overwrite old data
       lcd.setCursor(8, 1); // reset cursor
       lcd.print(ips, 1);
   }
}

Edit: fixed the { and } in the updateDisplay if statement code.

Thank you so much for taking the time to help. Allow me a bit of time to work thru those two tutorials you linked to. They look good.

I've got it working "ok" now with only a small "bump" in the motor rotation at high speeds.

Again, thank ALL of ya'll very much.

@KenH , the code that @groundFungus provided still has the bug that I mentioned that you introduced when you refactored the code to place/move all the code from updateDisplay() into loop().

You didn't just move the code into loop(), you changed it and while it is a very small change, the way you changed it, changes what it does.
As it is, the LCD will be updated every time updateDisplay() is called which is not the intent of the code.

Look at original updateDisplay() and then look at your modified code which is the same as the new version of udpateDispla()

You removed a pair of curly braces after the if() which totally changes what the code does.
C/C++ is not sensitive to whitespace / spacing / tabs / indentation.
Curly braces are used to group blocks of code.

The original code had a curly brace after the if()
which meant everything until the matching closing brace was only executed when the if() statement was true. The curly braces indicated what is done when the if() is true.
The code no longer has these braces:

      if (millis() - timer >= interval)

        timer = millis();

In C/C++ curly braces are optional for if() statements.
If you do not use them, then only the next expression (typically the next line) is executed.

So what happens is that when the if() is true, the variable timer is set to return value of millis()
BUT..... everything else is ALWAYS executed.
You need to put the curly brace pair back in to do what you are trying to do which is only update the LCD every 500ms.

As far as why the compiler is complaining about the variable status
the warning is exactly what it says.

The hd44780 begin() function is slightly different / better than all the other libraries. It returns a value to indicate if the initialization worked. If the return value is non zero, then the initialization did not work and attempting to write to the display will not work.
Other libraries do not return anything from begin()

you called begin() and assigned the return value from begin() to a variable called status, but then never did anything with variable.
If you are not going to check the return value from begin() then don't bother to save the return value into a variable and don't even declare the variable.

--- bill

Bill, thank you for the detailed post. I'll have to work on that tomorrow, now I do better understand. The wife has "pressured" me to stay in house watching the boob tube with her at night. When I first retired 10 yrs ago I'd stay out in workshop most all day, come in for lunch 'n supper then back to workshop until bedtime. She got tired of that and told me I "had to" spend some time with her. So, now about 5:30pm I try to come in for rest of night and only check computer by my recliner.

I'll be back out tomorrow :slight_smile: