Advise on how to optimize my LCD serial communication

Using Arduino Programming Questions
#1

Hi,

I’m new to Arduino and trying to control a 16x2 LCD through serial from a C# application.
I have the following requirements in terms of functionality:

  • Reset the display
  • Turn the display on
  • Turn the display off
  • Send text to a specific line and align it

After some tinkering I got the following code. While everything seems to work, I’m sure this is far from a good or “best practice” solution. Especially the part handling the text feels horrible dirty. Any help on optimizing this would be highly appreciated! :slight_smile:

/*
  LiquidCrystal Library - RetroBox

  Useful links
  https://omerk.github.io/lcdchargen/
*/

// include the library code:
#include <LiquidCrystal.h>

// initialize the library with the numbers of the interface pins
LiquidCrystal lcd(7, 8, 9, 10, 11, 12);

// custom lcd characters used by the logo
byte logoChar0100[8] = {0b00000, 0b00000, 0b00000, 0b00000, 0b00000, 0b00000, 0b00000, 0b00001};
byte logoChar0101[8] = {0b00011, 0b00011, 0b00011, 0b00001, 0b00001, 0b01111, 0b11001, 0b00011};
byte logoChar0102[8] = {0b11000, 0b11000, 0b11000, 0b10000, 0b10000, 0b11110, 0b10011, 0b11000};
byte logoChar0103[8] = {0b00000, 0b00000, 0b00000, 0b00000, 0b00000, 0b00000, 0b00000, 0b10000};

byte logoChar0200[8] = {0b00001, 0b00001, 0b00001, 0b00001, 0b00001, 0b00000, 0b00000, 0b00000};
byte logoChar0201[8] = {0b00000, 0b01000, 0b11100, 0b01000, 0b00000, 0b11000, 0b01110, 0b00001};
byte logoChar0202[8] = {0b00000, 0b00010, 0b00110, 0b00100, 0b00000, 0b00011, 0b01110, 0b10000};
byte logoChar0203[8] = {0b10000, 0b10000, 0b10000, 0b10000, 0b10000, 0b00000, 0b00000, 0b00000};

// define the backlight pin
int backLight = 13;

#define SOP '<'
#define EOP '>'

bool started = false;
bool ended = false;
char inData[80];
byte index;
int i;

void setup() {
  // set the size of the LCD
  lcd.begin(16, 2);

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

  // enable backlight
  pinMode(backLight, OUTPUT);
  digitalWrite(backLight, HIGH);

  // write the logo
  writeLogo();
}

void loop() {
  // Read all serial data
  while(Serial.available() > 0)
  {
    char inChar = Serial.read();
    if(inChar == SOP)
    {
       index = 0;
       inData[index] = '\0';
       started = true;
       ended = false;
    }
    else if(inChar == EOP)
    {
       ended = true;
       break;
    }
    else
    {
      if(index < 21)
      {
        inData[index] = inChar;
        index++;
        inData[index] = '\0';
      }
    }
  }

  // Process the result
  if(started && ended)
  {
    // Do the actual processing
    processData();

    // Reset for the next packet
    started = false;
    ended = false;
    index = 0;
    inData[index] = '\0';
  }
}

void processData() {
    // Text command
    // Arguments: line number, alignment, text
    // Example: <text:1:r:foobar>
    if (memcmp(inData, "text", 4) == 0)
    {     
      // Parse the line number
      // Valid arguments: 1 or 2
      int row = 0;
      if (inData[5] == '2') {
          row = 1;
      }
            
      // Parse the text
      // Valid arguments: String (12 characters)
      int textIndex = 0;
      char text[12];
      for(int i = 9; inData[i] != '\0'; ++i) {
         text[textIndex] = inData[i];
         textIndex++;
      }

      // Parse the direction
      // Valid arguments: (l)eft, (c)enter, (r)ight
      int offset = 4;
      if (inData[7] == 'r') {
          offset = 16 - strlen(text);
      }
      else if (inData[7] == 'c') {
        offset = ((16 - strlen(text)) / 2);
      }

      // Clear the current text
      lcd.setCursor(4, row);
      lcd.write("            ");

      // Write the new text
      lcd.setCursor(offset, row);
      lcd.write(text);
    }
    
    // Reset command
    // Command: <reset>
    if (strcmp(inData, "reset") == 0)
    { 
      lcd.clear();
      writeLogo();
    }

    // LCD off command
    // Command: <off>
    if (strcmp(inData, "off") == 0)
    { 
      digitalWrite(backLight, LOW);
      lcd.noDisplay();
    }

    // LCD on command
    // Command: <on>
    if (strcmp(inData, "on") == 0)
    { 
      digitalWrite(backLight, HIGH);
      lcd.display();
    }
}

void writeLogo() {
  // Clear the lcd
  lcd.clear();
  
  // Define custom characters
  lcd.createChar(0, logoChar0100);
  lcd.createChar(1, logoChar0101);
  lcd.createChar(2, logoChar0102);
  lcd.createChar(3, logoChar0103);
  lcd.createChar(4, logoChar0200);
  lcd.createChar(5, logoChar0201);
  lcd.createChar(6, logoChar0202);
  lcd.createChar(7, logoChar0203);

  // Write logo
  lcd.setCursor(0, 0);
  lcd.write(byte(0));
  lcd.write(byte(1));
  lcd.write(byte(2));
  lcd.write(byte(3));
  lcd.setCursor(0, 1);
  lcd.write(byte(4));
  lcd.write(byte(5));
  lcd.write(byte(6));
  lcd.write(byte(7));

  // Write text
  lcd.setCursor(4, 0);
  lcd.write("RetroBox");
  lcd.setCursor(4, 1);
  lcd.write("idmedia.no");
}
#2

That looks particularly clean and simple. What is it doing wrong? I wouldn't change it.

#3

Below the two that immediately stood out.

This could be a little more flexible

    int row = 0;
    if (inData[5] == '2')
    {
      row = 1;
    }

What if it ever needs to support a four-line LCD?

    int row = inData[5] - '1';

You might want to add some checking against boundaries which makes it longer again (extra lines of code).

This

    int textIndex = 0;
    char text[12];
    for (int i = 9; inData[i] != '\0'; ++i)
    {
      text[textIndex] = inData[i];
      textIndex++;
    }

could be changed to use a strcpy().

    strcpy(text, &inData[9]);

Changes not tested :wink:

#4