title: Best practices for HD44780 instruction calls
body: <p>I'm a first time poster, long time forum searcher... thanks for taking a look.</p>
  
  <p>My current embedded project uses a 16x2 LCD controlled by the HD44780 standard controller.  My PIC18 speaks to the LCD via the Adafruit LCD serial backpack (<a href="https://learn.adafruit.com/i2c-spi-lcd-backpack/downloads" rel="nofollow noreferrer">schematic link</a>).  I chose the SPI interface.</p>
  
  <p>The HD44780 is controlled through various instruction writes to 8 data pins (DB0-7), a read/write pin (R/W), a register select pin (RS), and an enable pin (E).  <a href="https://dawes.wordpress.com/2010/01/05/hd44780-instruction-set/" rel="nofollow noreferrer">Link to the instruction set</a>.</p>
  
  <p>The instructions are composed of bits that indicate certain settings... configuration parameters for lack of a better term.</p>
  
  <p>Everything is working as expected, however, my question relates to best practices.  In order to best organize my code for readability and flexibility I have tried to follow the following approach:</p>
  
  <ol>
  <li>Assign each HD44780 configuration parameter via #define directive in the header file</li>
  <li>Build an array of char type for each instruction and load it with the appropriate configuration parameters</li>
  <li><p>Execute instructions by passing the instruction array via pointer and using shift operations to construct the SPI output in the order expected by serial backpack</p>
  
  <p>This is all well and good, however, I have then nagging feeling that storing the instructions as arrays is not the best approach.  I would appreciate any advice on how you would approach this issue with a bent towards clarity and efficiency.</p></li>
  </ol>
  
  <p>My header and .c files are below, main.c is not included but calls LCD_send via the instruction aliases.</p>
  
  <h2>HEADER FILE</h2>
  
  <pre><code>/* 
   * File:   LCD_SPI_16x2.h
   * Author: rbs
   * Comments:
   * Revision history: 
   */
  
  /*Hardware configuration:
   *
   *  Adafruit LCD serial backpack bit configuration:
   *  [DB4 | DB5 | DB6 | DB7 | E | RS | RW | LITE]
   *
   *  HD44780 instruction set bit configuration (4-bit mode):
   *  [RS | RW | DB7 | DB6 | DB5 | DB4]
   * 
   *  HD44780 instruction set bit configuration (8-bit mode):
   *  [RS | RW | DB7 | DB6 | DB5 | DB4 | DB3 | DB2 | DB1 | DB0]
   * 
   *  PIC18 MSSP1 SPI peripheral:
   *  LSB first, clock = FOSC/4, SCK idle = low, TX on SCK low -&gt; high, SDI not used
   */
  
  // This is a guard condition so that contents of this file are not included
  // more than once.  
  #ifndef LCD_SPI_16X2_V2_H
  #define LCD_SPI_16X2_V2_H
  
  #include &lt;xc.h&gt; // include processor files - each processor file is guarded. 
  
  //function prototypes
  //void LCD_init(void);
  void LCD_send(char *cmd);
  void LCD_spi_out(char data);
  //void LCD_send_string(const char * str);
  //void LCD_set_cursr(int row, int col);
  
  #define _E 0x08         //alias for HD44780 ENABLE bit
  
  //HD44780 LCD parameters
  #define _BL  1          //1-&gt; backlight on       0-&gt; backlight off
  
  #define _ID  1          // 1-&gt; increment         0-&gt; decrement
  #define _S   0          // 1-&gt; display shift     0-&gt; no shift
  #define _D   0          // 1-&gt; display on        0-&gt; display off
  #define _C   0          // 1-&gt; cursor on         0-&gt; cursor off
  #define _B   0          // 1-&gt; cursor blinks     0-&gt; cursor does not blink
  #define _SC  0          // 1-&gt; shift display     0-&gt; shift cursor
  #define _RL  0          // 1-&gt; shift to the left 0-&gt; shift to the right
  #define _N   1          // 1-&gt; 2 lines           0-&gt; 1 line
  #define _F   0          // 1-&gt; 5x10 dot font     0-&gt; 5x8 dot font
  #define _DL8 1          // 8-bit mode        
  #define _DL4 0          // 4-bit mode
  
  //HD44780 LCD instruction aliases
  #define _Clear         LCD_send(Clear_display)
  #define _Home          LCD_send(Home)
  #define _Entry_mode    LCD_send(Entry_mode)
  #define _Display_onOff LCD_send(Display_onOff))
  #define _Cursor_shift  LCD_send(Cursor_shift)
  #define _Function8     LCD_send(Function_set_8bit)
  #define _Function4     LCD_send(Function_set_4bit)
  
  //HD44780 LCD instruction set, see format note above
  char Clear_display[10]      = {0,0,0,0,0,0,0,0,0,1};
  char Home[10]               = {0,0,0,0,0,0,0,0,1,0};
  char Entry_mode[10]         = {0,0,0,0,0,0,0,1,_ID,_S};
  char Display_onOff[10]      = {0,0,0,0,0,0,1,_D,_C,_B};
  char Cursor_shift[10]       = {0,0,0,0,0,1,_SC,_RL,0,0};
  char Function_set_8bit[10]  = {0,0,0,0,1,_DL8,_N,_F,0,0};
  char Function_set_4bit[10]  = {0,0,0,0,1,_DL4,_N,_F,0,0};
  char Cursor_pos[10]         = {0,0,1,0,0,0,0,0,0,0};
  
  #endif  /* LCD_SPI_16X2_V2_H */
  </code></pre>
  
  <h2>.C file</h2>
  
  <pre><code>/*
   * File:   LCD_SPI_16x2.c
   * Author: rbs
   *
   * Created on April 4, 2017, 10:00 PM
   */
  
  
  #include &lt;xc.h&gt;
  #include "system_initialize.h"
  #include "LCD_SPI_16x2_v2.h"
  
  
  void LCD_init(){
      //not completed yet
  }
  
  void LCD_send(char *cmd){
   /* Adafruit LCD serial backpack bit configuration:
    *  UPPER:
    *  [DB4 | DB5 | DB6 | DB7 | E | RS | RW | LITE]
    *  LOWER:
    *  [DB0 | DB1 | DB2 | DB3 | E | RS | RW | LITE]
    *  HD44780 incoming instruction configuration:
    *  [RS | RW | DB7 | DB6 | DB5 | DB4 | DB3 | DB2 | DB1 | DB0]
    */
      char Upper = 0x00;
      char Lower = 0x00;
  
      Upper |= (*(cmd))&lt;&lt;2;      //set RS bit
      Upper |= (*(cmd+1))&lt;&lt;1;    //set RW bit
      Upper |= (*(cmd+5))&lt;&lt;7;    //set data bits for upper 4bits of command 
      Upper |= (*(cmd+4))&lt;&lt;6;
      Upper |= (*(cmd+3))&lt;&lt;5;
      Upper |= (*(cmd+2))&lt;&lt;4;
      Upper |= _BL;              //set back light bit
  
      Lower |= (*(cmd))&lt;&lt;2;      //set RS bit
      Lower |= (*(cmd+1))&lt;&lt;1;    //set RW bit
      Lower |= (*(cmd+9))&lt;&lt;7;    //set data bits for lower 4bits of command
      Lower |= (*(cmd+8))&lt;&lt;6;
      Lower |= (*(cmd+7))&lt;&lt;5;
      Lower |= (*(cmd+6))&lt;&lt;4;
      Lower |= _BL;              //set back light bit
  
      LCD_spi_out(Upper);
      LCD_spi_out(Lower);
  }
  
  void LCD_spi_out(char data){
      _CS = 0;                   //chip select = 0
      SSP1BUF = (data|_E);       //send out data with E pin high
      __delay_ms(2);
      _CS = 1;                   //clock data out of LCD backpack shift register
      __delay_ms(2);             //delay for HD44780 to process
  
       _CS = 0;                  //chip select = 0
      SSP1BUF = data;            //send out data with E pin low to set HD44780
      __delay_ms(2);
      _CS = 1;                   //clock data out of LCD backpack shift register
      __delay_ms(2);
  }
  </code></pre>