I2C 20x4 LCD and I2C DAC not compatible in the same sketch?

I have a project that is using a timer interrupt, ADC interrupt, a 20x4 LCD over I2C using the NewLiquidCrystal library, and a DAC also over I2C. I have had projects that use 2 or 3 of these but not all at once. This is a first. Using pieces from the past projects as modules, I brought in the relevant sections one piece at a time and ensured each was working before adding the next. I hit a snag in the process. I added some functionality to tie the ADC, timer, and display together and it appears the system just continuously resets (based on the fact the display just rotates between the splash screen and a blank screen). Undoing the last change did not help. So I build several versions of the program installing the code modules from scratch in different orders the problem presented again. My current final determination is adding the DAC write causes it to fail.

#include <Wire.h>
#include <LiquidCrystal_I2C.h>

#define DEBUG_MODE 4

LiquidCrystal_I2C lcd(0x27, 2, 1, 0, 4, 5, 6, 7, 3, POSITIVE);

#define STOPPED 0
#define COMPLETE 1
#define RUNNING 2
volatile byte Run_Status;

#define adc_sens 1.0*5.0/1024.0 // voltage value of one ADC step
#define BATTERY_PIN 0
#define CURRENT_PIN 1
int adcReading;
boolean adcStarted = false;
boolean adcDone = false;
byte adc_pin;

unsigned int vin;
unsigned int battery_voltage, current_draw;
float load = 1.0;

typedef struct elapsed_time_struct
{
  unsigned int hour;
  unsigned int minute;
  unsigned int second;
  unsigned int msec;
} Elapsed_Time;
Elapsed_Time run_time;

volatile byte sec_pulse;

char line_text_0[20], line_text_1[20], line_text_2[20], line_text_3[20];

ISR(TIMER2_COMPA_vect)
{
  static unsigned int msec_count = 0;
  static byte count = 0;

  msec_count++;
  if (msec_count >= 100)
  {
    count++;
    sec_pulse = count;
    if (count >= 4)
    {
      count = 0;
    }
    msec_count -= 100;
  }
}

ISR (ADC_vect)
{
  byte low, high;

  low = ADCL;
  high = ADCH;

  adcReading = (high << 8) | low;
  adcDone = true;
}  // end of ADC_vect

void clear_display()
 {
  char line_text[17];
  sprintf(line_text, "%16s", "");
  lcd.setCursor(0, 0);            // move to the begining of the first line
  lcd.print(line_text);
  lcd.setCursor(0, 1);            // move to the begining of the first line
  lcd.print(line_text);
 }

void Reset_Counts()
 {
  run_time.hour = 0;
  run_time.minute = 0;
  run_time.second = 0;
  run_time.msec = 0;
 }

void DAC_Write(float Output_Value)
 {
  int DAC_Value;
  byte b1, b2;

  DAC_Value = (Output_Value / 5.0) * 4096;
  if (DAC_Value > 4096)
  {
    DAC_Value = 4096;
  }

  //  DAC_Value=2048;
  b1 = byte(DAC_Value / 16);
  b2 = byte(DAC_Value % 16);

  if (DEBUG_MODE == 4)
  {
    Serial.print ("Output Value:");
    Serial.print (Output_Value);
    Serial.println();

    Serial.print ("DAC Value:");
    Serial.print (DAC_Value);
    Serial.println();
  }

  Wire.beginTransmission(0x60);
  Wire.write(64);
  Wire.write(b1);
  Wire.write(b2 << 4); // Needed twice, since the 4 lowest bits (of 12) are in the fourth byte
  Wire.endTransmission();
 }

void display_status(char line_text[])
{
  char temp_text[20];

  switch (Run_Status)
  {
    case STOPPED :
      sprintf(temp_text, "STOPPED");
      break;
    case COMPLETE :
      sprintf(temp_text, "COMPLETE");
      break;
    case RUNNING :
      sprintf(temp_text, "RUNNING");
      break;
    default :
      break;
  }
  sprintf(line_text, "%-9s %02d:%02d:%02d", temp_text, int(run_time.hour), int(run_time.minute), int(run_time.second));
  sprintf(line_text, "%-20s", line_text);
}

void display_voltage(char line_text[])
{
  sprintf(line_text, "Voltage: %2d.%02d V", int(battery_voltage / 100), int(battery_voltage % 100));
  sprintf(line_text, "%-20s", line_text);
}

void display_current(char line_text[])
{
  sprintf(line_text, "Current: %2d.%02d mA", int(current_draw / 100), int(current_draw % 100));
  sprintf(line_text, "%-20s", line_text);
}

void display_capacity(char line_text[])
{
  sprintf(line_text, "Rating : %2d.%02d mA-Hr", int(run_time.minute), int(run_time.second));
  sprintf(line_text, "%-20s", line_text);
}

void setup()
{
  unsigned long current_time;
  current_time = millis();

  Serial.begin (115200);

  lcd.begin(20, 4);              // start the library
  lcd.setCursor(0, 0);            // set the LCD cursor position
  lcd.print("Test Sketch");
  lcd.setCursor(0, 1);            // set the LCD cursor position
  lcd.print("19Feb2017");

  // Setup Timer 2 interrupt at 1mSec
  TCCR2A = 0;          // normal operation
  TCCR2B = bit(WGM12) | bit(CS10) | bit (CS11);   // CTC, scale to clock / 1024
  OCR2A =  249;       // compare A register value (1000 * clock speed / 1024)
  TIMSK2 = bit (OCIE1A);             // interrupt on Compare A Match

  Reset_Counts();
  Run_Status = STOPPED;
  vin = 0;
  sec_pulse = 0;

  Wire.begin();
  adc_pin = BATTERY_PIN;

//  DAC_Write(2.0);

  do
  { } while (millis() - current_time < 5000);
  clear_display();
  lcd.setCursor(0, 0);            // set the LCD cursor position
}
  
void loop()
{
  // put your main code here, to run repeatedly:

  switch (sec_pulse)
  {
    case 1 :
       display_status(line_text_0);
        lcd.setCursor(0, 0);            // move to the begining of the first line
        lcd.print(line_text_0);
       sec_pulse = 0;
       adcStarted = false;
      break;
    case 2 :
       display_voltage(line_text_1);
        lcd.setCursor(0, 1);            // move to the begining of the second line
        lcd.print(line_text_1);
       sec_pulse = 0;
       adcStarted = false;
      break;
    case 3 :
       display_current(line_text_2);
        lcd.setCursor(0, 2);            // move to the begining of the second line
        lcd.print(line_text_2);
       sec_pulse = 0;
      break;
    case 4 :
       display_capacity(line_text_3);
        lcd.setCursor(0, 3);            // move to the begining of the second line
        lcd.print(line_text_3);
       sec_pulse = 0;
      break;
    default :
      break;
  }
  if (!adcStarted)
  {
    adcStarted = true;
    // start the conversion
    ADMUX = bit (REFS0) | (adc_pin & 0x07);
    ADCSRA |= bit (ADSC) | bit (ADIE);
  }
  if (adcDone == true)
  {
    adcDone = false;
    vin = (adcReading * 5.0 / 1023.0);
    switch (adc_pin)
    {
      case BATTERY_PIN :
        battery_voltage = (unsigned int) (vin * 100.0);
        adc_pin = CURRENT_PIN;
        break;
      case CURRENT_PIN :
        current_draw = (unsigned int) (vin / load * 100.0);
        adc_pin = BATTERY_PIN;
        break;
      default :
        break;
    }
  }
}

As written, the program works. If the line DAC_Write(2.0) is uncommented, the program continuously resets. What am I missing? I suspect it is some nuance about the wire.h or NewLiquidCrystal library that I’m not familiar with and thus can’t see at this level what the problem is (or the solution is).

Thank you all in advance for your time.

P.S. I used Nick’s I2C Scanner to identify and verify the I2C addresses of the DAC and LCD.

Try:

 //  DAC_Value=2048;
  b1 = highByte(DAC_Vaue);
  b2 = lowByte(DAC_Value);

Response deleted. I did not test the suggestion correctly. I will try the change as written then respond.

756E6C: Try:

 //  DAC_Value=2048;
  b1 = highByte(DAC_Vaue);
  b2 = lowByte(DAC_Value);

The change as described didn't quite work. Turns out b1 is the low byte and b2 the highbyte.

  b1 = lowByte(DAC_Value);
  b2 = highByte(DAC_Value);

But unfortunately that did not fix the problem of the unit constantly resetting. I changed back to a 16x2 I2C LCD and have the same issue. So since the target LCD is 20x4, I have changed back to that one.

I found one issue, the adcReading was supposed to be converted to a 0-5V reading, but it was declared as an int instead of a float. That problem was easy to fix.

sprintf(line_text, "Rating : %2d.%02d mA-Hr", int(run_time.minute), int(run_time.second));

Go through and look at all your strings. This one is 20 characters long. So it won't fit along with the terminating null in your buffer that is defined as only 20 characters long. For a 20 character wide screen, you should have char arrays that are 21 characters wide if you want to use all 20 characters.

Delta_G: sprintf(line_text, "Rating : %2d.%02d mA-Hr", int(run_time.minute), int(run_time.second));

Go through and look at all your strings. This one is 20 characters long. So it won't fit along with the terminating null in your buffer that is defined as only 20 characters long. For a 20 character wide screen, you should have char arrays that are 21 characters wide if you want to use all 20 characters.

Drat. I screwed that up. I changed the array from 17 (for use with the 16x2) to 20. obviously thinking more of the 20x4 than I was of string formats. I changed the array length in the program, but can't test it until later. Do you think that was the problem?

adwsystems: Drat. I screwed that up. I changed the array from 17 (for use with the 16x2) to 20. obviously thinking more of the 20x4 than I was of string formats. I changed the array length in the program, but can't test it until later. Do you think that was the problem?

Anything goes when you write past the end of an array. Try it and tell us.

Jiggy-Ninja: Anything goes when you write past the end of an array. Try it and tell us.

As I recall the there was a piece of malware written once upon a time on the premise of the printf (or sprintf) function writing past the end of the specified memory location.

I have to also add that I was playing with the DAC_Write commented line and the lcd.setCursor function in conjunction with finding the float variable declared as an integer and was able to get the code working. When I changed the tag type I also reorganized some of the variable declarations (to keep related tags together). Afterwards the program seemed to be working, or at least not continuously rebooting. I will have to put the program back from the OP and make all the changes and see it that resolves the issue directly.

I will report back.

adwsystems: As I recall the there was a piece of malware written once upon a time on the premise of the printf (or sprintf) function writing past the end of the specified memory location.

Once upon a time? That should be present tense, not past. Buffer overflows are still an embarrassingly common exploit.

Jiggy-Ninja: Once upon a time? That should be present tense, not past. Buffer overflows are still an embarrassingly common exploit.

I only pass through this line of work. I don't keep up with it on a regular basis. Therefore I did not want to overstate.

I changed the array length and the data type for vin (int to float) and it worked right out of the gate. Though not a problem, I went back on a second pass and changed out the /16 %16 for lowByte and highByte commands (because it is cleaner and more obvious).

Thank you to all that waded through the code to help find my (embarrassing) error.