SD Card Combined with LCD print function problem

Hi All, First post here, and this is my first arduino project.
I've been cobbling together bits of other people's code I've found via google and managed to make something that works (almost) but I've just run into a problem that I'm hoping someone can explain.

The project is to read data via serial from a car ECU, log that information to an SD card in the form of a CSV file, and display some of it on an 16x2 LCD.
The LCD display has 8 different options of displaying various information. I have a pushbutton that toggles the various displays.

The serial/Modbus interface is working fine, and the logging to CSV also.
The LCD display works mostly ok, except if I uncomment the LCD print lines below, the SD card returns the message 'Error opening output file'
Strangely, case 1 and 2 work fine, and the 'lcd.print(knock,1);' line in case 7 also causes no issues.
However If I uncomment any of the other lines, I get the error with the SD card

case 7:
      {
        lcd.setCursor(0, 0);
        //lcd.print("Knock:");
        lcd.setCursor(0, 1);
        lcd.print(knock,1);
      }
      break;
    case 8:
      {
        lcd.setCursor(0, 0);
        //lcd.print("Speed:");
        lcd.setCursor(0, 1);
        //lcd.print(mvss);
      }
      break;

I will post the full code below - it exceeds the 9000 character limit so may need to split it up

Any pointers or advice would be much appreciated. Its frustrating to get it so close to finished but then need to ask for assistance for what seems like something so basic.
Thanks, Graham

Capture.JPG

Part 1-

#define ENABLE_DEBUG_SERIAL
#define ENABLE_RTC

#ifdef ENABLE_DEBUG_SERIAL
#include <SoftwareSerial.h>
#endif

//set up variables for screen toggling
const int switchPin = 16;
static int hits = 0;
int switchState = 0;
int prevSwitchState = 0;
unsigned long lastDebounceTime = 0;  // the last time the output pin was toggled
unsigned long debounceDelay = 50;    // the debounce time; increase if the output flickers

// RTC
#ifdef ENABLE_RTC
#include <Wire.h>
#include "RTClib.h"
#endif

// SD
#include <SPI.h>
#include <SD.h>

// Modbus
#include <SimpleModbusMaster.h>

//LCD
#include <LiquidCrystal.h>
LiquidCrystal lcd(7, 5, 6, 10, 14 , 15);

//Define Floats for LCD Display
float mv = 0;
float afr = 0;
float knock = 0;
float wat = 0;
float act = 0;
float tps = 0;
float rpm = 0;
float mvss = 0;

// L1 LED pin
// Goes high when logging has successfully started
// Comment out to disable
// #define L1_LED_PIN 16

// Debugging output @ 19200 baud
#ifdef ENABLE_DEBUG_SERIAL
SoftwareSerial debugSerial(8, 9); // RX, TX
#endif

// RTC
#ifdef ENABLE_RTC
RTC_DS1307 rtc;
#endif

// SD
#define CHIP_SELECT 10
File dataFile;

// Modbus
#define BAUD 57600
#define TIMEOUT 100
#define POLLING 10 // The scan rate
#define RETRY_COUNT 0
#define TX_ENABLE_PIN 2 // Not used for RS232 but needed for the library

// The total amount of available memory on the Modbus master to store data
#define TOTAL_NO_OF_REGISTERS 33

// Modbus packet definitions
enum
{
  PACKET1,
  PACKET2,
  PACKET3,
  PACKET4,
  PACKET5,
  PACKET6,
  PACKET7,
  TOTAL_NO_OF_PACKETS // leave this last entry
};

// Create an array of Modbus Packets to be configured
Packet packets[TOTAL_NO_OF_PACKETS];

// Master Modbus register array
unsigned int regs[TOTAL_NO_OF_REGISTERS];

// Keeps track of last processed Modbus request
unsigned int lastSuccessfulRequest = 0;

// Specify order of logging
// regs are 4096-4116 (0-20), 4141 (21), 4152 (22), 4185 (23), 4196-4199 (24-27), 4203 (28), 4215-4218 (29-32)
// Refer to e4xxx_ECU_ModbusRegDefn.xls for details
const int logSequence[] = {0, 1, 7, 2, 3, 4, 5, 6, 8, 10, 11, 9, 20, 12, 16, 22, 23, 21, 24, 25, 26, 27, 28, 13, 14, 15, 17, 18, 19, 29, 30, 31, 32};

void setup()
{
  //LCD Setup
  lcd.begin(16, 2);
  lcd.setCursor(0, 0);
  lcd.write("Adaptronic Data");
  lcd.setCursor(0, 4);
  lcd.write("Monitor");

  // Set up the switch pin as input
  pinMode(switchPin, INPUT);

#ifdef ENABLE_DEBUG_SERIAL
  debugSerial.begin(19200);
#endif

#ifdef ENABLE_RTC
#ifdef AVR
  Wire.begin();
#else
  Wire1.begin(); // Shield I2C pins connect to alt I2C bus on Arduino Due
#endif
  rtc.begin();

  if (!rtc.isrunning()) {
#ifdef ENABLE_DEBUG_SERIAL
    debugSerial.println(F("RTC is NOT running!"));
#endif
    // Following line sets the RTC to the date & time this sketch was compiled
    rtc.adjust(DateTime(F(__DATE__), F(__TIME__)));
  }

  DateTime now = rtc.now();
#endif

#ifdef ENABLE_DEBUG_SERIAL
#ifdef ENABLE_RTC
  debugSerial.println(now.unixtime());
#endif
  debugSerial.println(F("Initialising SD card..."));
#endif
  // Make sure that the default chip select pin is set to
  // output, even if you don't use it:
  pinMode(SS, OUTPUT);

  // See if the card is present and can be initialised:
  if (!SD.begin(CHIP_SELECT)) {
#ifdef ENABLE_DEBUG_SERIAL
    debugSerial.println(F("Card failed, or not present."));
#endif
    // Don't do anything more
    while (1) ;
  }
#ifdef ENABLE_DEBUG_SERIAL
  debugSerial.println(F("Card initialised."));
#endif

  // Open up the file we're going to log to
  // Has to be 8.3 file name
#ifdef ENABLE_RTC
  char dataFileName[11];
  sprintf(dataFileName, "%02d%02d%02d%02d.csv", now.month(), now.day(), now.hour(), now.minute());
  dataFile = SD.open(dataFileName, FILE_WRITE);
#else
  dataFile = SD.open("datalog.csv", FILE_WRITE);
#endif
  if (!dataFile) {
#ifdef ENABLE_DEBUG_SERIAL
    debugSerial.println(F("Error opening output file."));
#endif
    // Wait forever since we can't write data
    while (1) ;
  }

#ifdef ENABLE_DEBUG_SERIAL
#ifdef ENABLE_RTC
  debugSerial.print(F("File opened: "));
  debugSerial.println(dataFileName);
#else
  debugSerial.println(F("File opened."));
#endif
#endif

  // Initialise each packet
  modbus_construct(&packets[PACKET1], 1, READ_HOLDING_REGISTERS, 4096, 21, 0);
  modbus_construct(&packets[PACKET2], 1, READ_HOLDING_REGISTERS, 4141, 1, 21);
  modbus_construct(&packets[PACKET3], 1, READ_HOLDING_REGISTERS, 4152, 1, 22);
  modbus_construct(&packets[PACKET4], 1, READ_HOLDING_REGISTERS, 4185, 1, 23);
  modbus_construct(&packets[PACKET5], 1, READ_HOLDING_REGISTERS, 4196, 4, 24);
  modbus_construct(&packets[PACKET6], 1, READ_HOLDING_REGISTERS, 4203, 1, 28);
  // SSI-4 stuff
  modbus_construct(&packets[PACKET7], 1, READ_HOLDING_REGISTERS, 4215, 4, 29);

  // Initialise the Modbus finite state machine
  modbus_configure(&Serial, BAUD, SERIAL_8N1, TIMEOUT, POLLING, RETRY_COUNT, TX_ENABLE_PIN, packets, TOTAL_NO_OF_PACKETS, regs);

  // Proceed only once a Modbus request has succeeded
  while (packets->successful_requests < 1) {
    modbus_update();
  }

#ifdef L1_LED_PIN
  // We have a successful request, so enable L1 LEDuu
  digitalWrite(L1_LED_PIN, HIGH);
#endif

  // Print the header line
  dataFile.println(F("Time (s),RPM,MAP (kPa),TPS (%),MAT (°C),WT (°C),AuxT (°C),Lambda,Knock,Idle,MVSS (km/h),SVSS (km/h),Batt (V),Trim (%),Inj 1 (ms),Ign 1 (°),Wastegate (%),Pump,Gear,ExtIn,EGT1 (°C),EGT2 (°C),EGT3 (°C),EGT4 (°C),Inj 2 (ms),Inj 3 (ms),Inj 4 (ms),Ign 2 (°),Ign 3 (°),Ign 4 (°),Oil P,Fuel P,Aux P"));
}

void loop()
{

  modbus_update();

  unsigned int successfulRequests = packets->successful_requests;
  // Only run if there is a new successful request
  if (successfulRequests > lastSuccessfulRequest) {
    dataFile.print(millis() / 1000.0, 3);
    dataFile.print(",");
    for (int i = 0; i < TOTAL_NO_OF_REGISTERS; i++) {
      switch (logSequence[i]) {
        // AFR needs to be divided by 2570
        case 5:
          // This will output in AFR instead of Lambda
          dataFile.print(regs[logSequence[i]] / 2570.0, 1);
          afr = (regs[logSequence[i]] / 2570.0);
          //dataFile.print(regs[logSequence[i]]/2570.0/14.7, 2);
          //afr=(regs[logSequence[i]]/2570.0/14.7);
          break;
        // Knock needs to be divided by 256
        case 6:
          dataFile.print(regs[logSequence[i]] / 256);
          knock = (regs[logSequence[i]] / 256);
          break;
        // Battery needs to be divided by 10
        case 9:
          dataFile.print(regs[logSequence[i]] / 10.0, 1);
          mv = (regs[logSequence[i]] / 10.0);
          break;
        // Injector needs to be divided by 1500
        case 12:
        case 13:
        case 14:
        case 15:
          dataFile.print(regs[logSequence[i]] / 1500.0, 3);
          break;
        // Timing needs to be divided by 5
        case 16:
        case 17:
        case 18:
        case 19:
          dataFile.print((int)regs[logSequence[i]] / 5.0, 1);
          break;
        // Everything else is untouched
        default:
          dataFile.print((int)regs[logSequence[i]]);
      }

      // Add comma or newline
      if (i == TOTAL_NO_OF_REGISTERS - 1) {
        dataFile.println();
      } else {
        dataFile.print(",");
      }
    }

    lastSuccessfulRequest = successfulRequests;
    dataFile.flush();
  }

Part 2 -

  // check the status of the switch

  int InState = digitalRead(switchPin);

  if (InState != prevSwitchState) {
    // reset the debouncing timer
    lastDebounceTime = millis();
  }

  if ((millis() - lastDebounceTime) > debounceDelay) {
    // whatever the reading is at, it's been there for longer
    // than the debounce delay, so take it as the actual current state:

    // if the button state has changed:
    if (InState != switchState) {
      switchState = InState;

      if (switchState == HIGH) {
        hits = hits + 1;
        lcd.clear();
      }
    }
  }

  // save the reading.
  prevSwitchState = InState;


  switch (hits) {
    case 1:
      {
        lcd.setCursor(0, 0);
        lcd.print("Battery Voltage:");
        lcd.setCursor(0, 1);
        lcd.print(mv, 2);
      }
      break;
    case 2:
      {
        lcd.setCursor(0, 0);
        lcd.print("Air/Fuel Ratio:");
        lcd.setCursor(0, 1);
        lcd.print(afr, 2);
      }
      break;
    case 3:
      {
        lcd.setCursor(0, 0);
        //lcd.print("Water Temp:");
        lcd.setCursor(0, 1);
        //lcd.print(wat,1);
      }
      break;
    case 4:
      {
        lcd.setCursor(0, 0);
        //lcd.print("Inlet Air Temp:");
        lcd.setCursor(0, 1);
        //lcd.print(act,1);
      }
      break;
    case 5:
      {
        lcd.setCursor(0, 0);
        //lcd.print("Throttle Pos:");
        lcd.setCursor(0, 1);
        //lcd.print(tps);
      }
      break;
    case 6:
      {
        lcd.setCursor(0, 0);
        //lcd.print("RPM:");
        lcd.setCursor(0, 1);
        //lcd.print(rpm);
      }
      break;
    case 7:
      {
        lcd.setCursor(0, 0);
        //lcd.print("Knock:");
        lcd.setCursor(0, 1);
        lcd.print(knock,1);
      }
      break;
    case 8:
      {
        lcd.setCursor(0, 0);
        //lcd.print("Speed:");
        lcd.setCursor(0, 1);
        //lcd.print(mvss);
      }
      break;
    default:                  {
        hits = 1;
#ifdef ENABLE_DEBUG_SERIAL
        debugSerial.print("Reset ");
#endif

      }


  }
}
LiquidCrystal lcd(7, 5, 6, 10, 14 , 15);
#define CHIP_SELECT 10

Conflict on pin 10.

Well spotted!
Moved the LCD to pin 17 but still the same fault :frowning:

Im using an Arduino Uno with noname SD card shield,
On compiling I receive the following warning:

Sketch uses 25,580 bytes (79%) of program storage space. Maximum is 32,256 bytes.
Global variables use 1,699 bytes (82%) of dynamic memory, leaving 349 bytes for local variables. Maximum is 2,048 bytes.
Low memory available, stability problems may occur.

Is it possible that I have reached some limitation? I have several other thing in mind to add to this project in the future. Maybe upgrading to a mega would be a good idea but I'm not sure it will help with this problem?

I'm not sure I understand the error condition.

The only time the SD file error is printed is in set up. There have been no calls to any of the lcd print cases.

Are you saying that you see the error in set up dependent upon the compiled or not compiled lines in a loop which is not yet running?

Something I notice but I'm not sure its relevant because the library might handle it, but where's line 4 on a 16x2 display?

//LCD Setup
  lcd.begin(16, 2);
  lcd.setCursor(0, 0);
  lcd.write("Adaptronic Data");
  lcd.setCursor(0, 4);
  lcd.write("Monitor");

cattledog:
I'm not sure I understand the error condition.

The only time the SD file error is printed is in set up. There have been no calls to any of the lcd print cases.

Are you saying that you see the error in set up dependent upon the compiled or not compiled lines in a loop which is not yet running?

Exactly.
If I uncomment the line

//lcd.print(mvss);

Or any other commented out line in the case statements towards the end of the loop, then the SD gives the error :confused:
I've tried it time and time again so its definitely not coincidence.

I also had the same phenomenon yesterday while adding some print.debug() commands to try to solve another issue, these print to softwareserial rather than the LCD.
It did exactly the same where it couldn't open the output file. In that case though i could work around it.

cattledog:
Something I notice but I'm not sure its relevant because the library might handle it, but where's line 4 on a 16x2 display?

//LCD Setup

lcd.begin(16, 2);
  lcd.setCursor(0, 0);
  lcd.write("Adaptronic Data");
  lcd.setCursor(0, 4);
  lcd.write("Monitor");

Good Spot. I'll change it now but I doubt that would cause it.
Edit: No Difference.

Sketch uses 25,580 bytes (79%) of program storage space. Maximum is 32,256 bytes.
Global variables use 1,699 bytes (82%) of dynamic memory, leaving 349 bytes for local variables. Maximum is 2,048 bytes.
Low memory available, stability problems may occur.

I think you are seeing the "stability problems". You might try using SdFat.h instead of SD.h, but I doubt it will be of much help.

Maybe upgrading to a mega would be a good idea

Yes. Especially if you are going to add to the project.

OK, I'll order a Mega anyway.
And I'll try SdFat.h in the mean time.

As an additional point. I tried changing from print() to write() but the result is the same.

Why are you using software serial instead of hardware serial?

hardware serial is used for the actual Modbus connection to the ECU,
software serial is just used for debugging

Another advantage of the mega will be the multiple hardware serial connections.