Custom library makes board hang. Possibly syntax

Hi everyone,

I followed the library tutorial that uses the Morse example to create my own library.
I created my library from a working sketch in Arduino 1.0 that I use on an Uno. The code seems to make the board hang right after I call my library's constructor.
I would much appreciate it if someone can glance over my code and check my C++ syntax. I have tried to compare it to other working libraries with no luck.
Please excuse me for not strictly following the naming conventions but I will read up and fix that once the library is working.
Thanks in advance.
Richard

H-File

#ifndef UniLcdBtnBrd_h
#define UniLcdBtnBrd_h

#include <Arduino.h>
#include <..\Wire\Wire.h>

class UniLcdBtnBrd
{
public: 
 UniLcdBtnBrd();
 UniLcdBtnBrd(char lcdAdd, char btnsAdd);

void setBoardAddresses(char lcdAdd, char btnsAdd);
void setLCDLinesAdd(char add1);
void setLCDLinesAdd(char add1,[color=black][/color] char add2);
void setLCDLinesAdd(char add1, char add2, char add3);
void setLCDLinesAdd(char add1, char add2, char add3, char add4);
void setLCDLineLen(char len);
void initLCD();
void writeLCDChar(char data);
void setLCDAddr(char data);
void writeLCDStr(String data_str, int lineNr);
void clearLCD();
void setLCDDisplay(boolean displayOn, boolean cursorOn,boolean cursorBlink,boolean BLOn);
void initBtns();
char readBtns();
void flashLED();


private:

boolean BLightOn;
char pExpAddLCD;
char pExpAddBtns;
char lcdLine1Add;
char lcdLine2Add;
char lcdLine3Add;
char lcdLine4Add;
char lcdLineLen;

void writeLCDNibble(int data);
void writeLCDByte(char nibbleH,char nibbleL);

};

#endif

CPP File

/*
UniLcdBtnBrd.cpp - library for using the universal lcd and buttons board.
Created by Richard Whittington, 08/05/2012
*/

#include "Arduino.h"
#include <..\Wire\Wire.h>

#include "UniLcdBtnBrd.h"

boolean BLightOn = true;
char pExpAddLCD = 56;
char pExpAddBtns = 57;
char lcdLine1Add = byte(0);
char lcdLine2Add = byte(0);
char lcdLine3Add = byte(0);
char lcdLine4Add = byte(0);
char lcdLineLen = 16;


UniLcdBtnBrd::UniLcdBtnBrd()
{
 flashLED();
 Wire.begin();
 initLCD();
 initBtns();

}
UniLcdBtnBrd::UniLcdBtnBrd(char lcdAdd, char btnsAdd)
{
	flashLED();
	pExpAddLCD = lcdAdd;
	pExpAddBtns = btnsAdd;
	Wire.begin();
  	initLCD();
  	initBtns();
}

void UniLcdBtnBrd::setBoardAddresses(char lcdAdd, char btnsAdd)
{
  pExpAddLCD = lcdAdd;
  pExpAddBtns = btnsAdd;
}

void UniLcdBtnBrd::setLCDLinesAdd(char add1)
{
  lcdLine1Add = add1;
}

void UniLcdBtnBrd::setLCDLinesAdd(char add1, char add2)
{
  lcdLine1Add = add1;
  lcdLine2Add = add2;
}

void UniLcdBtnBrd::setLCDLinesAdd(char add1, char add2, char add3)
{
  lcdLine1Add = add1;
  lcdLine2Add = add2;
  lcdLine3Add = add3;
}

void UniLcdBtnBrd::setLCDLinesAdd(char add1, char add2, char add3, char add4)
{
  lcdLine1Add = add1;
  lcdLine2Add = add2;
  lcdLine3Add = add3;
  lcdLine4Add = add4;
}

void UniLcdBtnBrd::setLCDLineLen(char len)
{
  lcdLineLen = len;
}

void UniLcdBtnBrd::initLCD()
{                               //#  E  RW RS D7 D6 D5 D4
  delay(20);
  writeLCDNibble(0b00000011);      //0  0  0  0  0  0  1  1    |thrice Function set (Interface is 8 bits long.)
  delay(20);
  writeLCDNibble(0b00000011);      
  delay(10);
  writeLCDNibble(0b00000011);
  delay(10);
  writeLCDNibble(0b00000010);      //Function set (Set interface to be 4 bits long.)

  writeLCDNibble(0b00000010);
  writeLCDNibble(0b00001000);      //0  0  0  0  N  F  #  #    |Specify the number of display lines and character font
  
  writeLCDNibble(0b00000000);
  writeLCDNibble(0b00001000);      //0  0  0  0  1  0  0  0    |Display off
  
  writeLCDNibble(0b00000000);
  writeLCDNibble(0b00000001);      //0  0  0  0  0  0  0  1    |Display clear
  
  writeLCDNibble(0b00000000);
  writeLCDNibble(0b00000010);      //0  0  0  0  0  1  ID S    |Entry mode set:  Sets cursor move direction and specifies display shift. These operations are performed during data write and read.  

  writeLCDNibble(0b00000000);
  writeLCDNibble(0b00001100);      //0  0  0  0  1  D  C  B    |Display on:  Sets entire display (D) on/off,cursor on/off (C), and blinkingof cursor position character(B). 
}

void UniLcdBtnBrd::writeLCDNibble(int data)
{
  if (BLightOn)
  data = data|0b10000000;

  Wire.beginTransmission(pExpAddLCD);//write nibble 
  Wire.write(data);
  Wire.endTransmission();
  Wire.beginTransmission(pExpAddLCD);//enable High
  if (BLightOn)
    Wire.write(data|0b11000000);
  else
    Wire.write(data|0b01000000); 
  Wire.endTransmission();
  
  Wire.beginTransmission(pExpAddLCD);//enable Low
  if (BLightOn)
    Wire.write(0b00011111&data)|0b10000000;
  else
    Wire.write(0b00011111&data);
  Wire.endTransmission();
}
void UniLcdBtnBrd::writeLCDByte(char nibbleH,char nibbleL)
{
  writeLCDNibble(nibbleH);
  writeLCDNibble(nibbleL);
  Wire.beginTransmission(pExpAddLCD);//enable Low
  if (BLightOn)
  Wire.write(0b10000000);
  else
  Wire.write(byte(0));
  Wire.endTransmission();
}
void UniLcdBtnBrd::writeLCDChar(char data)
{	
  char high=data;
  char low = data;
       low =  0b00001111&low;
       low =  0b00010000|low;
       high = (data>>4);
       high = 0b00001111&high;
       high = 0b00010000|high;
       writeLCDByte(high,low);
}
void UniLcdBtnBrd::setLCDAddr(char data)
{	
  char high=data;
  char low = data;
       low =  0b00001111&low;
       low =  0b00000000|low;
       high = (data>>4);
       high = 0b00001111&high;
       high = 0b00001000|high;
       writeLCDByte(high,low);
}

void UniLcdBtnBrd::writeLCDStr(String data_str, int lineNr)
{  
  char address;
  int i =0;
  
  switch (lineNr)
  {
    case 1:
    address =lcdLine1Add;
    break;
    case 2:
    address =lcdLine2Add;
    break;
    case 3:
    address =lcdLine3Add;
    break;
    case 4:
    address =lcdLine4Add;
    break;
    default:
    address =lcdLine1Add;
    break;
  }
  
  setLCDAddr(address);
  
  while(data_str[i]&&i<lcdLineLen)
  {
    writeLCDChar(data_str[i]);
    i++; 
  }
  while(i<lcdLineLen)
  {
    writeLCDChar(' ');
    i++;    
  }
}
void UniLcdBtnBrd::clearLCD()
{
  writeLCDByte(0x0,0x1);
}
void UniLcdBtnBrd::setLCDDisplay(boolean displayOn, boolean cursorOn,boolean cursorBlink,boolean BLOn)
{  
  char instruction = 0b1000;
  if(displayOn)
  instruction = instruction|0b0100;
  if(cursorOn)
  instruction = instruction|0b0010;
  if(cursorBlink)
  instruction = instruction|0b0001;
  BLightOn = BLOn;
  writeLCDByte(0x0,instruction);
}

void UniLcdBtnBrd::initBtns()
{
  Wire.beginTransmission(pExpAddBtns);
  Wire.write((byte)0);
  Wire.endTransmission();
 }

char readBtns()
{
  char data;
  Wire.requestFrom(pExpAddBtns, 1);    // request 1 byte from slave device
  data = Wire.read(); // receive a byte as character
  return data;
}

Moderator edit: Code rendered legible, colour- and italic-free by correct use of tags.

UniLcdBtnBrd.zip (2.46 KB)

Do you have any idea when your constructor is called? Do you know if it is called before or after Wire has been constructed? Before or after Wire is ready to do anything?

Where is the flashLED() method? Is the LED ready to be flashed when your constructor is called?

The initLCD() function assumes that Wire is ready to work. Do you know for a fact that it is?

I doubt that you know when your constructor is called, relative to the other constructors. Did you notice, for instance, that there is a Wire.begin() method, where the real work is done? Your class needs a begin() method, and (nearly) all stuff being done by the constructor needs to be moved to the begin() method.

Hi Paul,

Thank you for your reply.

I made my own circuit board that uses i2c to communicate to a standard LCD and some push buttons. (I tried to include a photo but it didn't work, anyway)
The library takes strings and characters and writes them to the LCD as necessary.

I should have removed the void flashLED(); from the H-File. Anything referencing flashLED should not be there.
I was just trying to find the problem.

I am not sure how to check that Wire is ready as the whole library relies on it.

Where should I rather add the Wire.begin() method as I already put it in my "UniLcdBtnBrd" constructor?

" Your class needs a begin() method, all stuff being done by the constructor needs to be moved to the begin() method."
I'm not sure what to do here... Must I move some of the code around into a "begin()" method.

Thanks for your input!
Richard

Must I move some of the code around into a "begin()" method.

Yes.

(nearly) all stuff being done by the constructor needs to be moved to the begin() method.

Have you tried debugging with Serial.print's?

Throw out a Serial.print('a') at the start, and b,c,d,e,f,g etc when you enter each function. See where your alphabet stops. (Careful using descriptive strings unless you F() them due to memory constraints). Once you find the function call that is causing the issue, print all the variables inside there. Also, verify that you're not running out of SRAM. More often than not, reboots are from running out of memory.

Also, simplifying your code will help you debug it some more. You've got a lot of "unnecessary" clutter and redundant statements that is making everything look at lot more complicated than it needs to be.

It's very easy to make a mistake when dealing with binary numbers and its tedious counting zeros to verify offsets. Sticking to hex tends to be a little bit more programmer friendly in the long run. It doesn't take too long to remember the bit patterns.

Regardless of coding practices, just start dumping checkpoints to the Serial port until you can see exactly where the issue is. If it's inside the Wire library, add dumps inside there too.

Fair warning: Don't copy and paste the below. I don't know the rest of your program, and made assumptions about variables that may or may not be true. It's merely an example.

For example:

void UniLcdBtnBrd::writeLCDNibble(int data)
{
  if (BLightOn)
  data = data|0b10000000;

  Wire.beginTransmission(pExpAddLCD);//write nibble 
  Wire.write(data);
  Wire.endTransmission();
  Wire.beginTransmission(pExpAddLCD);//enable High
  if (BLightOn)
    Wire.write(data|0b11000000);
  else
    Wire.write(data|0b01000000); 
  Wire.endTransmission();
  
  Wire.beginTransmission(pExpAddLCD);//enable Low
  if (BLightOn)
    Wire.write(0b00011111&data)|0b10000000;
  else
    Wire.write(0b00011111&data);
  Wire.endTransmission();
}
....

Is the same as:

void UniLcdBtnBrd::writeLCDNibble(int data)
{
  if (BLightOn)
  {
    data|=0x80;
  }

  Wire.beginTransmission(pExpAddLCD);//write nibble 
  Wire.write(data);
  Wire.endTransmission();

  Wire.beginTransmission(pExpAddLCD);//enable High
  Wire.write(data|0x40); 
  Wire.endTransmission();
  
  Wire.beginTransmission(pExpAddLCD);//enable Low
  Wire.write(0x9F & data)
  Wire.endTransmission();
}
...

Hi tgm1175,

Thanks for your reply.

My issue lies in converting my working sketch to a library. I do not have a firm understanding of how c++ classes should be structured. The code itself does work when in a sketch.

I do admit the coding is cluttered and I will clean it up as soon as I can get the library working.

Thank you for your input.

Regards,
Richard

Hi PaulS,

Once again thanks.

I will attempt adding the begin() method when I'm back at home tonight.

Regards,
Richard

I have added the begin() method.
I have add #include "Wire.h" in the sketch where I am using the library. This seems odd as I have already included it in the library itself.

In the CPP-File I have to redefine this variable char _pExpAddBtns = 57; even though it is already in the H-File. Why is this?

The library is working despite the few questions I have.
Thank you for the help.

Here is the completed H-File

#ifndef UniLcdBtnBrd_h
#define UniLcdBtnBrd_h

#include <..\Wire\Wire.h>
#include <Arduino.h>


class UniLcdBtnBrd
{
  public: 
   UniLcdBtnBrd();
   UniLcdBtnBrd(char lcdAdd, char btnsAdd);

   void setBoardAddresses(char lcdAdd, char btnsAdd);
   void setLcdLinesAdd(char add1);
   void setLcdLinesAdd(char add1, char add2);
   void setLcdLinesAdd(char add1, char add2, char add3);
   void setLcdLinesAdd(char add1, char add2, char add3, char add4);
   void setLcdLineLen(char len);
   void initLcd();
   void writeLcdChar(char data);
   void setLcdAddr(char data);
   void writeLcdStr(String data_str, int lineNr);
   void clearLcd();
   void setLcdDisplay(boolean displayOn, boolean cursorOn,boolean cursorBlink,boolean BLOn);
   void initBtns();
   char readBtns();
   void begin();

  private:
   boolean _bLightOn;
   char _pExpAddLcd;
   
   char _lcdLine1Add;
   char _lcdLine2Add;
   char _lcdLine3Add;
   char _lcdLine4Add;
   char _lcdLineLen;
   char _pExpAddBtns;

   void writeLcdNibble(char data);
   void writeLcdByte(char nibbleH,char nibbleL);
};

#endif

Here is the completed CPP-File

/*
UniLcdBtnBrd.cpp - library for using the universal lcd and buttons board.
Created by Richard Whittington, 08/05/2012
*/

#include "..\Wire\Wire.h"
#include "Arduino.h"
#include "UniLcdBtnBrd.h"

char _pExpAddBtns = 57;//I have added this in the H-File. Why do I have to add it here too?

UniLcdBtnBrd::UniLcdBtnBrd()
{
   _bLightOn = true;
   _pExpAddLcd = 56;
   _pExpAddBtns = 57;
   _lcdLine1Add = byte(0);
   _lcdLine2Add = byte(0);
   _lcdLine3Add = byte(0);
   _lcdLine4Add = byte(0);
   _lcdLineLen = 16;
}
UniLcdBtnBrd::UniLcdBtnBrd(char lcdAdd, char btnsAdd)
{
   boolean _bLightOn = true;
   _pExpAddLcd = lcdAdd;
   _pExpAddBtns = btnsAdd;
   _lcdLine1Add = byte(0);
   _lcdLine2Add = byte(0);
   _lcdLine3Add = byte(0);
   _lcdLine4Add = byte(0);
   _lcdLineLen = 16;
}

void UniLcdBtnBrd::begin()
{
	Wire.begin();
  	initLcd();
  	initBtns();
	setLcdDisplay(true,true,true,true);
}

void UniLcdBtnBrd::setBoardAddresses(char lcdAdd, char btnsAdd)
{
  _pExpAddLcd = lcdAdd;
  _pExpAddBtns = btnsAdd;
}

void UniLcdBtnBrd::setLcdLinesAdd(char add1)
{
  _lcdLine1Add = add1;
}

void UniLcdBtnBrd::setLcdLinesAdd(char add1, char add2)
{
  _lcdLine1Add = add1;
  _lcdLine2Add = add2;
}

void UniLcdBtnBrd::setLcdLinesAdd(char add1, char add2, char add3)
{
  _lcdLine1Add = add1;
  _lcdLine2Add = add2;
  _lcdLine3Add = add3;
}

void UniLcdBtnBrd::setLcdLinesAdd(char add1, char add2, char add3, char add4)
{
  _lcdLine1Add = add1;
  _lcdLine2Add = add2;
  _lcdLine3Add = add3;
  _lcdLine4Add = add4;
}

void UniLcdBtnBrd::setLcdLineLen(char len)
{
  _lcdLineLen = len;
}

void UniLcdBtnBrd::initLcd()
{                               //#  E  RW RS D7 D6 D5 D4
  delay(20);
  writeLcdNibble(0b00000011);      //0  0  0  0  0  0  1  1    |thrice Function set (Interface is 8 bits long.)
  delay(20);
  writeLcdNibble(0b00000011);      
  delay(10);
  writeLcdNibble(0b00000011);
  delay(10);
  writeLcdNibble(0b00000010);      //Function set (Set interface to be 4 bits long.)

  writeLcdNibble(0b00000010);
  writeLcdNibble(0b00001000);      //0  0  0  0  N  F  #  #    |Specify the number of display lines and character font
  
  writeLcdNibble(0b00000000);
  writeLcdNibble(0b00001000);      //0  0  0  0  1  0  0  0    |Display off
  
  writeLcdNibble(0b00000000);
  writeLcdNibble(0b00000001);      //0  0  0  0  0  0  0  1    |Display clear
  
  writeLcdNibble(0b00000000);
  writeLcdNibble(0b00000010);      //0  0  0  0  0  1  ID S    |Entry mode set:  Sets cursor move direction and specifies display shift. These operations are performed during data write and read.  

  writeLcdNibble(0b00000000);
  writeLcdNibble(0b00001100);      //0  0  0  0  1  D  C  B    |Display on:  Sets entire display (D) on/off,cursor on/off (C), and blinkingof cursor position character(B). 
}

void UniLcdBtnBrd::writeLcdNibble(char data)
{
  if (_bLightOn)
  data = data|0b10000000;

  Wire.beginTransmission(_pExpAddLcd);//write nibble 
  Wire.write(data);
  Wire.endTransmission();
  Wire.beginTransmission(_pExpAddLcd);//enable High
  if (_bLightOn)
    Wire.write(data|0b11000000);
  else
    Wire.write(data|0b01000000); 
  Wire.endTransmission();
  
  Wire.beginTransmission(_pExpAddLcd);//enable Low
  if (_bLightOn)
    Wire.write(0b00011111&data)|0b10000000;
  else
    Wire.write(0b00011111&data);
  Wire.endTransmission();
}
void UniLcdBtnBrd::writeLcdByte(char nibbleH,char nibbleL)
{
  writeLcdNibble(nibbleH);
  writeLcdNibble(nibbleL);
  Wire.beginTransmission(_pExpAddLcd);//enable Low
  if (_bLightOn)
  Wire.write(0b10000000);
  else
  Wire.write(byte(0));
  Wire.endTransmission();
}
void UniLcdBtnBrd::writeLcdChar(char data)
{	
  char high=data;
  char low = data;
       low =  0b00001111&low;
       low =  0b00010000|low;
       high = (data>>4);
       high = 0b00001111&high;
       high = 0b00010000|high;
       writeLcdByte(high,low);
}
void UniLcdBtnBrd::setLcdAddr(char data)
{	
  char high=data;
  char low = data;
       low =  0b00001111&low;
       low =  0b00000000|low;
       high = (data>>4);
       high = 0b00001111&high;
       high = 0b00001000|high;
       writeLcdByte(high,low);
}

void UniLcdBtnBrd::writeLcdStr(String data_str, int lineNr)
{  
  char address;
  int i =0;
  
  switch (lineNr)
  {
    case 1:
    address =_lcdLine1Add;
    break;
    case 2:
    address =_lcdLine2Add;
    break;
    case 3:
    address =_lcdLine3Add;
    break;
    case 4:
    address =_lcdLine4Add;
    break;
    default:
    address =_lcdLine1Add;
    break;
  }
  
  setLcdAddr(address);
  
  while(data_str[i]&&i<_lcdLineLen)
  {
    writeLcdChar(data_str[i]);
    i++; 
  }
  while(i<_lcdLineLen)
  {
    writeLcdChar(' ');
    i++;    
  }
}
void UniLcdBtnBrd::clearLcd()
{
  writeLcdByte(0x0,0x1);
}
void UniLcdBtnBrd::setLcdDisplay(boolean displayOn, boolean cursorOn,boolean cursorBlink,boolean BLOn)
{  
  char instruction = 0b1000;
  if(displayOn)
  instruction = instruction|0b0100;
  if(cursorOn)
  instruction = instruction|0b0010;
  if(cursorBlink)
  instruction = instruction|0b0001;
  _bLightOn = BLOn;
  writeLcdByte(0x0,instruction);
}

void UniLcdBtnBrd::initBtns()
{
  Wire.beginTransmission(_pExpAddBtns);
  Wire.write((byte)0);
  Wire.endTransmission();
 }

char readBtns()
{
  char data;
  Wire.requestFrom(_pExpAddBtns, 1);    // request 1 byte from slave device
  data = Wire.read(); // receive a byte as character
  return data;
}

Here is an example of how to use the Library with the LCD

#include "Wire.h"
#include "UniLcdBtnBrd.h"
UniLcdBtnBrd brd1 = UniLcdBtnBrd();
void setup() 
{
  brd1.begin();
  brd1.setLcdLinesAdd(0,0x40);
  brd1.writeLcdStr("     Hello      ",1);
  brd1.writeLcdStr("     World!     ",2);
}
void loop() 
{
  // put your main code here, to run repeatedly: 
}

UniLcdBtnBrd.zip (2.98 KB)

I have add #include "Wire.h" in the sketch where I am using the library. This seems odd as I have already included it in the library itself.

The reason has to do with how the IDE decides what needs to be compiled. The sketch is scanned for include statements. Only the libraries referenced by the sketch as compiled/linked to.

This keeps you from creating a library that uses a library that uses a library that uses a library that the user of your library does not have. A quick glance at an example sketch shows all the libraries that yours is dependent on.

In the CPP-File I have to redefine this variable char _pExpAddBtns = 57; even though it is already in the H-File. Why is this?

You should not have needed to do that. Doing so is actually wrong. You need to define what happened that caused you to think you needed to do that, so we can help you resolve the issue properly.

Hi PaulS,

You are totally right. In my haste to finish the code I forgot to add the readBtns() method as part of the UniLcdBtnBrd class this:

char UniLcdBtnBrd::readBtns()
{
....
}

All is working as expected. Once again, thanks for the input.

Richard.