Using LCD Class Object as static member

Hi there,
Can someone explain how to use the LiquidChrystal lcd object as a static member of singleton a class with only static members?
I just copied and pasted code from here to create a singleton class and i want to make the lcd libraries functions to be accessible through a static member of it.

PCI.h

#ifndef PCI_CLASS
#define PCI_CLASS

#include "LCD.h"

// Singleton Class
// Got it from here : https://forum.arduino.cc/t/getting-a-singleton-to-work-on-arduino/580159/4
class PCI {
  public:
    static PCI& Instance(); // Class method as the only access to the Singleton Object

    static LCD lcd(OLED_V2, A13, A12, A14, A8, A9, A10, A11);
    static void test();
  
  private:
    static PCI* pInstance; // Static variable holding the pointer to the only instance of this 
    PCI();
};
#endif

PCI.cpp

#include <Arduino.h>
#include "PCI.h"

PCI& PCI::Instance(){
  if (PCI::pInstance == nullptr)
    PCI::pInstance = new PCI();
  return *PCI::pInstance;
}

// There is only this private constructor, no public one!
PCI::PCI() {
  Serial.begin(115200);
}
void PCI::test() {
  Serial.println("test");
}
PCI* PCI::pInstance;

This does not work, i get alot of random errors. For example:

/Users/florian/Documents/Arduino/libraries/LCD/LCD.h:9:17: error: expected identifier before numeric constant
 #define OLED_V2 0x02
                 ^

The goal is to access the lcd functions through the singleton in different other classes by refering to the singleton there.

...
PCI* pci;
pci = &tpci->Instance();
pci->lcd.print("hallo")

best,
Flub

I don't see the benefit to tie together one special LCD and a fixed 115200 baud rate,
maybe you could enlighten me.

A static function that returns a LCD with syntax errors in the parameter definition. Strange.

A pointer to what? There is no dynamic part that could have a different address.
What makes that different from this (in class context)?

That is strange for the same reasons.
Using a hidden constructor and pointer access to a static only object is senseless,
because any member can always be accessed via the <class>::<attribute> syntax.

So, this whole project seems to be rather strange and superfluous, besides syntactically wrong.
You have no embedded LCD object.

I see static only objects as a kind of automatic singleton, all users use the same object.

Not 100% what you're trying to do with it, but this works:

I think I'm using a different LCD library to you so yours might need to be thumbed into this.

#include <LiquidCrystal_I2C.h>

class PCI {

  public:
  
    static PCI& Instance() {
      if (PCI::pInstance == nullptr) PCI::pInstance = new PCI();
    return *PCI::pInstance;
    }
    
    static LiquidCrystal_I2C lcd;
    
    static void test() { Serial.println("Serial Test"); }
  
  private:
    
    static PCI* pInstance; // Static variable holding the pointer to the only instance of this 
    
    PCI() {
      lcd.init();
      lcd.backlight();
    }
};
 
PCI* PCI::pInstance = nullptr;
LiquidCrystal_I2C PCI::lcd(0x27,16,2);

void setup() {
  Serial.begin(115200);
  PCI::test();
  PCI::Instance().lcd.print("LCD Test");
}

void loop() { }

Why would you type that extra Instance(). on each access,
just to smuggle in the lcd initialization?

PCI::lcd.print("LCD Test"); could be used freely IMHO,
if the initialization had not been hidden in the first access.

That's on the OP to justify :smiley: It's how he wanted to access it.

Sure, I was just asking.

I have not seen a definition of tpci yet, and would use pci = &PCI::Instance();.

I do happen to agree though... I don't know what a PCI is in this regards or why you need only one instance of it but if the LCD is static to PCI like this then access via PCI::lcd.print() is simpler for sure. You'd probably need to add something like PCI::init() and use that to hit lcd.begin() etc or whatever the LCD needs.

But even...does the LCD need to be static at all? If you only ever allow one instance of a PCI then isn't that no different from just having it as a PCI member variable? Only one LCD will get instantiated too.

It's odd all around. But, if you want a SINGLETON:
PCI.h:

#ifndef PCI_CLASS
#define PCI_CLASS

#include <LiquidCrystal.h>

class PCI {

  public:
    PCI(const PCI &) = delete;
    PCI &operator=(const PCI &) = delete;
    static PCI &getInstance();
    LiquidCrystal lcd;

  private:
    PCI() : lcd(12, 11, 5, 5, 3, 2) {}

};

extern PCI &singleton;

#endif

PCI.cpp:

#include "PCI.h"

PCI & PCI::getInstance() {
  static PCI instance;
  return instance;
}

PCI &singleton {PCI::getInstance()};

Main .ino:

#include "PCI.h"

void setup() {
  singleton.lcd.begin(16, 2);
  singleton.lcd.print("Hello World");
}

void loop() {
}

:point_up: Yeah that's even better

Thank you so much! I think that works. I can try to explain why I asked this.
I am reworking old code for a music drum sequencer device with alot of features, an oled display, 25 buttons, a lot of leds, an encoder and a switch, 20 Trigger outputs, 2 analog outputs, 2 analog inputs, midi output and midi input. I am not a pro programmer, but I try my best to do it better than my previous procedural version of the code which consisted of about 15 ino tabs in the arduino IDE in which there were many globally accessible variables defined.

Now I develop a Class Object for every different Feature. For example a User input Interface class which contains all code needed for this and of course the "DisplayInterfaceClass --> DCI.

When combining all these classes in the main deviceClass I had the problem that I needed to put all following code which needed access the these inside the deviceClass which would end up as a huge amount of code. So I thought how can I keep the structure but get access to the lcd, the leds etc from anywhere. First I tried to just make every member of the class Object I wanted to use everywhere as static, but that did not work with the LCD Object. Then while researching I stumbled upon the singleton class concept and I thought that might be the right thing for me.

For testing I just created a Class called PCI instead of DCI (Display Control Interface) because i had that one already.

Now it seems to work, I will have to change alot of code now, but I think now I can use the dci object anyhwere. I think I will do the same with the main Device Class.

I am actually not really sure of what the best concept is to bring all those elements together, I am just trying it out forever.

This is the code of you with my Inclusions.
DCI.h

#ifndef DCI_CLASS
#define DCI_CLASS
#include "DCI_constants.h"
/*
 * DISPLAY CONTROL INTERFACE
 * ------------------------------------------------------------------------------------------------------
 * The DCI Class contains all function to controll the LCD Display and the LEDs of the SEQ-V100 Sequencer.
 */
 
#include "LCD.h"
/*
    LCD.h by David Mellis < Elco Jacobs < W.Earl from Adafruit -> you need to copy the Library from this repos (Firmware/Bibilotheken/LCD) to your Arduino IDEs libraries folder.
    This is a custom modified (by me) Library to use the 16x2 Raystar OLED Display of the Sequencer
    https://github.com/technobly/Adafruit_CharacterOLED/blob/master/Adafruit_CharacterOLED.h
    It itself again was based on the LiquidCrystal Library
    I followed this displayde to hook the display up.
    https://learn.sparkfun.com/tutorials/oled-display-hookup-displayde/all
    More Info about it in this repo under Features/OLED Display.
*/

struct LedRuntime {
  // LED On/Off States
  boolean states[29];
  // LED Blinking
  uint8_t blinkStates[29];
  unsigned long blinkLast;
  unsigned int blinkInterval = 100; // in ms
  uint8_t blinkCount;
  // Update Flags
  boolean updateFlag;
}; 

// Singleton Class
// See : https://forum.arduino.cc/t/using-lcd-class-object-as-static-member/916976/8

class DCI {
  public:
    // Strange singleton code
    DCI(const DCI &) = delete; // I dont understand, but if it works...
    DCI &operator=(const DCI &) = delete; // I dont understand, but if it works...
    static DCI &getInstance();

    // Properties
    // LCD
    LCD lcd;
    // LEDS
    LedRuntime led;

    // Methods 
    void begin();    
    void run();
    

  private:
    // Strange singleton code
    DCI() : lcd(OLED_V2, A13, A12, A14, A8, A9, A10, A11) {}
    
    // Properties  
    int8_t _ledMap[29];
    
    // Methods 
    void _setupPins();
    // LCD
    ////////////////////////////////////////
    void _initLCD();
    void _setupCustomLCDCharacters();
    // LED
    ////////////////////////////////////////
    void _initLED();
    void _updateLeds();
    void _clearStepLeds();
    // Utilities
    void _setupLedMaps(); 
};
extern DCI &display;
#endif

DCI.cpp

#include <Arduino.h>
#include "DCI.h"
/*
 * Strange singleton code
 */
DCI & DCI::getInstance() {
  static DCI instance;
  instance.begin();
  return instance;
}
/*
 * Public Methods
 */
void DCI::begin() {
  // Pin Setup
  this->_setupPins();
  // Hardware I/O Setup
  this->_initLCD();
  this->_initLED();
}
void DCI::run() {
  // Process Led Runtime
  this->_updateLeds();
}

/*
 * Private Methods
 */
void DCI::_setupPins() {
  // LED DRIVER OUTPUT
  DDRD |= (1 << 7); // CLOCK OUTPUT
  PORTD &= ~(1 << 7);
  DDRF |= (1 << 7); // ENABLE OUTPUT
  PORTF |= (1 << 7); // set high -> disabthis->led 
  DDRL |= (1 << 3); // DATA OUTPUT
  PORTL &= ~(1 << 3);
}
// LCD
void DCI::_initLCD() {
  //lcd = lcd;
  // Note: LCD only clears character if printed with single quotes lcd.print(' ');
  this->_setupCustomLCDCharacters();
  this->lcd.home();
  this->lcd.clear();
}
void DCI::_setupCustomLCDCharacters() {
  // Setup Custom Characters for LCD
  this->lcd.command(LCD_FUNCTIONSET | LCD_EUROPEAN_I);
  // USER DEFINED SYMBOLS FOR LCD MENU
  this->lcd.createChar(SYMBOL_LOOP, CHAR_CODE_LOOP);
  this->lcd.createChar(SYMBOL_RECYCLE, CHAR_CODE_RECYCLE);
  this->lcd.createChar(SYMBOL_PL, CHAR_CODE_PL);
  this->lcd.createChar(SYMBOL_AY, CHAR_CODE_AY);
  this->lcd.createChar(SYMBOL_PL2, CHAR_CODE_PL2);
  this->lcd.createChar(SYMBOL_PLAY, CHAR_CODE_PLAY);
}
// LED
void DCI::_initLED() {
  _setupLedMaps();
  // Led States
  for (int i = 0; i < 29; i++) {
    this->led.states[i] = LED_OFF;
  }
  // Blink States
  for (int i = 0; i < 29; i++) {
    this->led.blinkStates[i] = LED_BLINK_OFF;
  }
  this->led.updateFlag = true;
  _updateLeds();
  
  // Show snake to check every step button this->led
  /*
  for (int i = 0; i < 16; i++) {
    this->led.states[i] = LED_ON;
    if (i > 0) {
      this->led.states[i-1] = LED_OFF;
    }
    this->led.updateFlag = true;
    _updateLeds();
    delay(100);
  }
  _clearStepLeds();
  _updateLeds();
  */
}
void DCI::_updateLeds() {
  /*
   * This method bit bangs out data to a multi-channel this->led driver chip.
   */
  // Update Blink count after blinkInterval
  if ((millis() - this->led.blinkLast) > this->led.blinkInterval) {
    this->led.blinkLast = millis();
    if (this->led.blinkCount > 3) this->led.blinkCount = 0;
    this->led.blinkCount++;
    this->led.updateFlag = true;
  }
  if(this->led.updateFlag) {
    int8_t index;
    PORTF &= ~(1 << 7); // enable LED DRIVER
    for (uint8_t i = 0; i < 37; i++) {
      if (i == 0) {
        PORTL |= (1<<3);
      }
      // Shift out LED data     
      if (i > 0 && i < 30) {
        // GET index in states array
        index = this->_ledMap[i-1];
        if (this->led.blinkStates[index] == LED_BLINK_SLOW && (this->led.blinkCount == 1 || this->led.blinkCount == 2) ) {
          PORTL |= (1<<3);
        } else
        if (this->led.blinkStates[index] == LED_BLINK_FAST && (this->led.blinkCount == 1 || this->led.blinkCount == 3) ) {
          PORTL |= (1<<3);
        } else 
        if (this->led.states[index] && this->led.blinkStates[index] == LED_BLINK_OFF) {
          PORTL |= (1<<3);
        } 
      }
      // Clock Pulse
      PORTD |= (1 << 7);
      PORTD &= ~(1 << 7); 
      
      PORTL &= ~(1 << 3);
    }
    PORTF |= (1 << 7); // disable LED DRIVER
    this->led.updateFlag = false;
  }
}
void DCI::_clearStepLeds() {
  for (int i = 0; i < 16; i++) {
    this->led.states[i] = LED_OFF;
  }
  this->led.updateFlag = true;
}
// Utilites
void DCI::_setupLedMaps() {
  // index = channel of the Led Driver
  // value = this->ledStates index for that output channel 
  // 16 Step Button Leds  
  this->_ledMap[16]=0;
  this->_ledMap[15]=1;
  this->_ledMap[14]=2;
  this->_ledMap[13]=3;
  this->_ledMap[12]=4;
  this->_ledMap[10]=5;
  this->_ledMap[9]=6;  
  this->_ledMap[8]=7;  
  this->_ledMap[2]=8;  
  this->_ledMap[11]=9;
  this->_ledMap[17]=10;
  this->_ledMap[0]=11;
  this->_ledMap[1]=12;
  this->_ledMap[3]=13;
  this->_ledMap[4]=14;
  this->_ledMap[5]=15; 
  // Other Buttons
  this->_ledMap[7] = LED_BUTTON_PLAY; // PLAY
  this->_ledMap[6] = LED_BUTTON_SHIFT; // SHIFT
  this->_ledMap[18] = LED_BUTTON_A; // A
  this->_ledMap[19] = LED_BUTTON_B; // B
  this->_ledMap[20] = LED_BUTTON_C; // C
  this->_ledMap[21] = LED_BUTTON_D; // D
  this->_ledMap[22] = LED_BUTTON_E; // E
  this->_ledMap[23] = LED_BUTTON_F; // F
  this->_ledMap[24] = LED_SWITCH; // SWITCH
  this->_ledMap[25] = LED_BAR_1; // BAR 1
  this->_ledMap[26] = LED_BAR_2; // BAR 2
  this->_ledMap[27] = LED_BAR_3; // BAR 3
  this->_ledMap[28] = LED_BAR_4; // BAR 4
}

DCI &display {DCI::getInstance()};

software.ino

/* 
 *  This is the firmware for the Button Matrix and Encoder Controller
 *  of the SEQ-V100 Sequencer.
 */
#include "DCI.h"

#include "seqV100.h"
// Sequencer
SeqV100 seqV100;

// Set Tempo Interrupt Service
ISR(TIMER3_OVF_vect) { 
  seqV100.tempoController.tempoClockInterrupt();
}

void setup() {
  display.lcd.print("hallo");
  // Init SeqV100 instance
  seqV100.begin();
}

void loop() {
  seqV100.run();
}

Thank you so much for your help!!! I am really running out of Energy, these programming knowledge things seem to have no end...

Do I have to set all of the singletons class / object methods to static ? It doesnt work if write static LCD lcd; in the Class definition

-Flub