LCD not displaying correctly

I'm trying to build a system to display on multiple screens, using a manager class, but I've run into trouble with a basic 16x2 Text LCD. I have the LCD attached correctly, as it displays correctly without the manager, but not with. The print command sometimes displays nonsense, but at other times, the command will seem to restart the sketch, rerunning code up until the problem command.

I realize the DisplayText class is unnecessary in this code - there are other functions I added for the LCD not relevant to the issue.

I'm running this on an a Mega 2560, with Windows 10.

#include <LiquidCrystal.h>

class DisplayText { // Object for displaying on a lcd screen

  private:

  public:

    LiquidCrystal lcd;

    DisplayText(int pinRS, int pinE, int pin0, int pin1, int pin2, int pin3)
      : lcd(pinRS, pinE, pin0, pin1, pin2, pin3)
    {

      lcd.begin(16, 2);
      lcd.clear();
      lcd.setCursor(0, 0);

    }

};


class DisplayManager { // manages various display types (currently only text lcds)

  private:

  public:

    DisplayText *textLCDs;

    DisplayManager() {
    }

    DisplayManager(byte _textLCDs[][6]) {

      textLCDs = (DisplayText *)malloc(sizeof(DisplayText) * sizeof(_textLCDs)); // allocates memory for number of lcds based on array of pins

      for (int i = 0; i < sizeof(textLCDs); i++) {

        textLCDs[i] = DisplayText(_textLCDs[i][0], _textLCDs[i][1], _textLCDs[i][2], _textLCDs[i][3], _textLCDs[i][4], _textLCDs[i][5]); // initializes all lcds

      }

    }

};


byte textLCDsSetup[][6] = { { 52, 48, 36, 34, 32, 30 } };

DisplayManager display(textLCDsSetup); // initialize manager

//DisplayText displayDirect(52, 48, 36, 34, 32, 30); // initialize directly to lcd


void setup() {
  //Displays correctly:
  //displayDirect.lcd.print("test1");
  
  //does not display correctly:
  display.textLCDs[0].lcd.print("test2");
}


void loop() {

}

Edit:

(sizeof(_textLCDs)/sizeof(_textLCDs[0])
changed to
sizeof(_textLCDs)
as the purpose was to get number of items in _textLCDS

That is a problem because _textLCDs is a pointer with a length of 2. The thing it points to has a length of 6. The result of 2 / 6 is 0 so no memory is allocated.

WHEN YOU PASS AN ARRAY TO A FUNCTION, THE SIZE INFORMATION IS LOST. You must pass the size in a separate argument.

Thanks, I changed it, but this doesn't seem to solve the overall issue. The Arduino is still restarting on the print line

When you change your code, post the latest version so that we can keep up.

Updated Manager class:

class DisplayManager { // manages various display types (currently only text lcds

  private:

  public:

    DisplayText *textLCDs;

    DisplayManager() {
    }

    DisplayManager(byte _textLCDs[][6]) {

      textLCDs = (DisplayText *)malloc(sizeof(DisplayText) * sizeof(_textLCDs)); // allocates memory for number of lcds based on array of pins

      for (int i = 0; i < sizeof(textLCDs); i++) {

        textLCDs[i] = DisplayText(_textLCDs[i][0], _textLCDs[i][1], _textLCDs[i][2], _textLCDs[i][3], _textLCDs[i][4], _textLCDs[i][5]); // initializes all lcds

      }

    }

};

Try this version just to see if it works:

#include <LiquidCrystal.h>

class DisplayText   // Object for displaying on a lcd screen
{
  public:
    LiquidCrystal lcd;

    DisplayText(int pinRS, int pinE, int pin0, int pin1, int pin2, int pin3)
      : lcd(pinRS, pinE, pin0, pin1, pin2, pin3)
    {
      lcd.begin(16, 2);
      lcd.clear();
      lcd.setCursor(0, 0);
    }
};


class DisplayManager   // manages various display types (currently only text lcds
{
  public:

    DisplayText *Displays;

    DisplayManager(byte _textLCDs[][6], const unsigned count)
    {
      // allocates memory for number of lcds based on array of pins
      Displays = (DisplayText *) malloc(sizeof (DisplayText) * count);

      for (unsigned i = 0; i < count; i++)
      {
        Displays[i] = DisplayText(
                        _textLCDs[i][0],
                        _textLCDs[i][1],
                        _textLCDs[i][2],
                        _textLCDs[i][3],
                        _textLCDs[i][4],
                        _textLCDs[i][5]); // initialize each lcd
      }
    }
};

byte textLCDsSetup[][6] =
{
  { 52, 48, 36, 34, 32, 30 }
};
const unsigned DisplayCount = sizeof textLCDsSetup / sizeof textLCDsSetup[0];

DisplayManager manager(textLCDsSetup, DisplayCount); // initialize manager

void setup()
{
  //does not display correctly:
  manager.Displays[0].lcd.print("test2");
}

void loop() {}

Thanks, this looks better, but the arduino still has the same issue. It restarts the program when it runs into the print line.

Sketch from reply #6 in Wokwi simulation:

I have tested the simulation with normal code and the display works.

@mathcat06 you have to start very simple and try to show text after the lcd.begin(). However the 'lcd' object is not initialized, you have to fix that first.

If you want to connect the other displays in Wokwi, then you can log in to store your project.

This is left-field though, sorry, but maybe are you running out of memory somewhere.
It seems that when you introduce a new function, the arduino restarts. Could it be a memory issue?

Also, how are you powering the displays. Are you using the VCC power supplied by the arduino? Maybe more than one display is causing the onboard regulator to drop-out?

I hope this might help.

There are a number of Arduino library calls that can't safely be executed before the Arduino runtime is initialized, just before setup() is called. That is why so many library objects have a 'begin()' function as well as a constructor. The 'begin()' will do the setup that can't be done in a constructor because global constructors execute before the Arduino library is initialized.

Here is a version that has the lcd.begin() in a DisplayText.begin() which is called from DisplayManager.begin() which is called in setup().

#include <LiquidCrystal.h>

class DisplayText   // Object for displaying on a lcd screen
{
  public:
    LiquidCrystal lcd;

    DisplayText(int pinRS, int pinE, int pin0, int pin1, int pin2, int pin3)
      : lcd(pinRS, pinE, pin0, pin1, pin2, pin3)
    {}

    void begin()
    {
      lcd.begin(16, 2);
      lcd.clear();
      lcd.setCursor(0, 0);
    }
};


class DisplayManager   // manages various display types (currently only text lcds
{
  public:

    DisplayText *Displays;
    unsigned displayCount;

    void begin()
    {
      for (unsigned i = 0; i < displayCount; i++)
        Displays[i].begin();
    }

    DisplayManager(byte _textLCDs[][6], const unsigned count) : displayCount(count)
    {
      // allocates memory for number of lcds based on array of pins
      Displays = (DisplayText *) malloc(sizeof (DisplayText) * displayCount);

      for (unsigned i = 0; i < displayCount; i++)
      {
        Displays[i] = DisplayText(
          _textLCDs[i][0],
          _textLCDs[i][1],
          _textLCDs[i][2],
          _textLCDs[i][3],
          _textLCDs[i][4],
          _textLCDs[i][5]); // initialize each lcd
      }
    }
};

byte textLCDsSetup[][6] =
{
  { 52, 48, 36, 34, 32, 30 }
};

const unsigned DisplayCount = sizeof textLCDsSetup / sizeof textLCDsSetup[0];

DisplayManager manager(textLCDsSetup, DisplayCount); // initialize manager

void setup()
{
  manager.begin(); // Initialize displays

  // Put text on the first display
  manager.Displays[0].lcd.print("test2");
}

void loop() {}

@johnwasser, it does not work in Wokwi: https://wokwi.com/arduino/projects/303378991727772224.
How does the begin-begin-begin solve the problem that the LiquidCrystal lcd object is not created with the pin numbers ?

But it is:

class DisplayText
{
  public:
    LiquidCrystal lcd;

    DisplayText(int pinRS, int pinE, int pin0, int pin1, int pin2, int pin3)
      : lcd(pinRS, pinE, pin0, pin1, pin2, pin3)
    {}

The arguments to the 'lcd' constructor are provided in the DisplayText constructor.

This sketch works on WOKWI. That seems to indicate that the problem is not in DisplayText but in DisplayManager.

I suspect the problem is in using malloc() rather than 'new' to allocate space for the objects. With malloc() the objects are not initialized. When the objects are created later and copied over the existing uninitialized objects that may be a problem.

Using 'new' requires a default constructor and that constructor must provide arguments to the 'lcd' constructor. Passing all 0's seems to work.

#include <LiquidCrystal.h>

class DisplayText   // Object for displaying on a lcd screen
{
  public:
    LiquidCrystal lcd;

    DisplayText(int pinRS, int pinE, int pin0, int pin1, int pin2, int pin3)
      : lcd(pinRS, pinE, pin0, pin1, pin2, pin3)
    {}

    void begin()
    {
      lcd.begin(16, 2);
      lcd.clear();
      lcd.setCursor(0, 0);
    }
};

DisplayText display(52, 48, 36, 34, 32, 30);

void setup()
{
  display.begin();

  // Put text on the first display
  display.lcd.print("test2");
}

void loop() {}
1 Like

Using 'new' requires a default constructor and that constructor must provide arguments to the 'lcd' constructor. Passing all 0's seems to work.

Could you explain what you mean by this? from my understanding of how "new" works, it isn't what I need.
I agree that the malloc is probably the problem, and I would be happy for any suggestions on how to replace it.

Displays = new DisplayText[displayCount];
It works for me. Why is doing it the right way not what you need?

Thanks, I finally got it. I didn't realize you could use new on arrays like this.

Final code:

#include <LiquidCrystal.h>

class DisplayText   // Object for displaying on a lcd screen
{
  public:
    LiquidCrystal lcd;

    DisplayText(int pinRS, int pinE, int pin0, int pin1, int pin2, int pin3)
      : lcd(pinRS, pinE, pin0, pin1, pin2, pin3)
    {}

    DisplayText()
      : lcd(0,0,0,0,0,0)
    {}

    void begin()
    {
      lcd.begin(16, 2);
      lcd.clear();
      lcd.setCursor(0, 0);
    }
};


class DisplayManager   // manages various display types (currently only text lcds
{
  public:

    DisplayText *Displays;
    unsigned displayCount;

    void begin()
    {
      for (unsigned i = 0; i < displayCount; i++)
        Displays[i].begin();
    }

    DisplayManager(byte _textLCDs[][6], const unsigned count) : displayCount(count)
    {
      // allocates memory for number of lcds based on array of pins
      Displays = new DisplayText[displayCount];

      for (unsigned i = 0; i < displayCount; i++)
      {
        Displays[i] = DisplayText(
          _textLCDs[i][0],
          _textLCDs[i][1],
          _textLCDs[i][2],
          _textLCDs[i][3],
          _textLCDs[i][4],
          _textLCDs[i][5]); // initialize each lcd
      }
    }
};

byte textLCDsSetup[][6] =
{
  { 52, 48, 36, 34, 32, 30 }
};

const unsigned DisplayCount = sizeof textLCDsSetup / sizeof textLCDsSetup[0];

DisplayManager manager(textLCDsSetup, DisplayCount); // initialize manager

void setup()
{
  manager.begin(); // Initialize displays

  // Put text on the first display
  manager.Displays[0].lcd.print("test");
}

void loop() {}

Thanks everyone for your help!

1 Like

@johnwasser, thanks for explaining.
@mathcat06, it is working very nice. I have connected the other displays as well in Wokwi. Sign up at Wokwi to be able to save a project.

When I write to a display, I do this:

manager.Displays[0].lcd.print("number one");

Would it be better to use "Display[0]" instead of "Displays[0]", since I'm selecting a single display from an array. The longer version would be "DisplayList[0]" (also without 's').

Simulation with four displays:

Wokwi is still in development. The backlight does not seem to work at the moment.

This topic was automatically closed 120 days after the last reply. New replies are no longer allowed.