serial commands getting lost in the ether

I have an old laser (circa 1980s) which was designed to be controlled by plaintext ascii commands.

I also have an old mac running old software which sends the plaintext ascii. It expects certain reponses from the laser, otherwise it throws an error.

I have replaced the old laser with a new laser (circa 2010). The new laser can be controlled by a DC voltage.

Thus I have an Arduino MEGA 2560 capturing the plaintext commands, responding appropriatly, and setting a DAC to an appropriate level via SPI.

The old laser is designed to ramp up in power, so it received approximately 10 commands over the space of 10 seconds (at 1200 baud).

Unfortunately, about half way through my code seems to loose track and stop ramping. Other than that it works fine.

I was wondering if I could get a second opinion on the code?

The control command is something like "LI=x.xxx" where x.xxx is between 0.000 and 9.999

#include <Wire.h>
#include <Adafruit_MCP4725.h>

Adafruit_MCP4725 dac;

int sensorPinM = A0;
int sensorPinC = A1;
int sensorPinD = A2;

float x = 0.00;
float m = 0.5;
float c = 0.0;
float y = 0.0;
unsigned int DAC = 0;

const int ledPin = 13;      // select the pin for the LED
int tHigh = 150;    // interval to hold high (ms)
int tLow = 2500; // interval to hold low (ms)

boolean ledState = 1;  // variable to store the value coming from the sensor
int echo = 0; // rs-232 echo

unsigned long previousMillis = 0; // length of time of current LED state
unsigned long currentMillis = 0;
unsigned long dt = 0;

String serialBuffer = "";
String value = "0.000";

char CR = char(13);
char LF = char(10);

int inByte;
boolean stringComplete = false;
int tick = 50;

int verbose = 0;

void setup()
{
  //initialise DAC
  dac.begin(0x62);
  dac.setVoltage(0, false);
  
  // initialise serial
  if (verbose) Serial.begin(9600);
  Serial1.begin(1200);
  serialBuffer.reserve(2000);
  
  // initialize LED pin 13 as an output.
  pinMode(ledPin, OUTPUT);
  if (verbose) Serial.println("Booting up...");
}

void loop()
{
  currentMillis = millis();
  dt = currentMillis - previousMillis;
  checkLED();
  digitalWrite(ledPin, ledState);

  if (stringComplete == true) {
    if (verbose) Serial.println(serialBuffer);
    String response = "";
    //queries

    if (serialBuffer.substring(0, 3) == "LI=") {//this should be a number between 1.000 & 9.999
      int startPacket = serialBuffer.indexOf("=")+1;
      int endPacket = serialBuffer.indexOf(CR);
      value = serialBuffer.substring(startPacket, startPacket+4);//endPacket);
      if (verbose) Serial.print("Value: ");
      if (verbose) Serial.println(value);     
      if (isValidNumber(value) == true) {
        x = value.toFloat();
        if (verbose) Serial.print("X: ");
        if (verbose) Serial.println(x);

        DAC = updateDAC();
        if (verbose) Serial.print("DAC: ");
        if (verbose) Serial.println(DAC);
        if (DAC > 4095) DAC = 0; // it should never be larger than 5V. if it is, it's a bug.
        if (DAC < 0) DAC = 0;  // it should never be less than 0v. again, probably bug.
        dac.setVoltage(DAC, false);
        delay(1);
        dac.setVoltage(DAC, false);
        delay(1);

        if (verbose) Serial.print("DAC readback: ");
        if (verbose) Serial.println(analogRead(sensorPinD)*4); //it's only a 10 bit dac - mult by 4 to match 12 bit dac
      }
    }

    else if (serialBuffer == "?LI\r") response = value;
    else if (serialBuffer == "?ID\r") response = "I300";
    else if (serialBuffer == "?LASER\r") response = "2";
    else if (serialBuffer == "?FAULTS\r") response = "System OK";
    else if (serialBuffer == "ECHO=0\r") ;
    else {};

    if (verbose) Serial.println(response);
    Serial1.println(response);
    serialBuffer = "";
    stringComplete = false;

  }
}

int updateDAC()
{
  //we're working in volts.

  //float sensorM = (float)analogRead(sensorPinM);
  //float sensorC = (float)analogRead(sensorPinC);
  float sensorM = 512.0;  //disables external trim pots
  float sensorC = 512.0;  //disables external trim pots
  c = -0.0125 + (1.0 * (sensorM - 512.0) / 1023.0);  // factor of 4 between volts and watts
  m = 0.255 + (1.0 * (sensorC - 512.0) / 1023.0);
  y = m * x + c;
  return (int)((y / 5.0) * 4095.0);
}

void checkLED()
{
  if ((ledState == 0) && ( dt > tLow))
  {
    ledState = 1;
    previousMillis = currentMillis;
    dt = 0;
  }
  if ((ledState == 1) && ( dt > tHigh))
  {
    ledState = 0;
    previousMillis = currentMillis;
    dt = 0;
  }
}

boolean isValidNumber(String str) {
  boolean isNum = false;
  for (byte i = 0; i < str.length(); i++)
  {
    isNum = isDigit(str.charAt(i)) || str.charAt(i) == '.'; //digit or decimal place
    if (!isNum) return false;
  }
  return isNum;
}

void serialEvent1() {
  while (Serial1.available() > 0) {
    char inChar = (char)Serial1.read(); // get the new byte:
    serialBuffer += inChar;
    if (inChar == CR) {
      stringComplete = true;
      break;
    }
    if (verbose) Serial.print(inChar);
    if (inChar == LF) break;
  }
}

}

edit: full code added

/*

a whole bunch of constants etc
*/

You wouldn't happen to have deleted a bunch of code that you don't fully understand? Then the problem is probably in that place.

Please give us all the code so we can at least compile it, even if we don't have your hardware to test it with.

MorganS:
You wouldn't happen to have deleted a bunch of code that you don't fully understand?

nope, just hiding my smelly socks...

updated original post with full code.

  serialBuffer.reserve(2000);

That seems like an awfully big chunk of memory to reserve for a command string that is usually 8 bytes or less.

The serial event which fills that buffer never checks if it's getting close to that limit anyway. If there is some major corruption on the line and the carriage-return is never sensed, then this can fill up all available memory. Put some check in the buffer-filling function so that if the buffer is longer than the longest imaginable command, it should discard that buffer and start again.

MorganS:
That seems like an awfully big chunk of memory to reserve for a command string that is usually 8 bytes or less.

The serial event which fills that buffer never checks if it's getting close to that limit anyway. If there is some major corruption on the line and the carriage-return is never sensed, then this can fill up all available memory. Put some check in the buffer-filling function so that if the buffer is longer than the longest imaginable command, it should discard that buffer and start again.

buffer set to 64 bytes, bufferLength increment and a check/reset added to serialEvent1. issue persists...

You aren't using the SoftwareSerial library: don't #include it.

This won't be the problem but it's messy seeing this kind of stuff there when it shouldn't be.

Why is checkLED() resetting previousMillis? You should rename that function to describe what it actually does. checkLEDandResetPrevMillisandDT()

It looks like that might have been a leftover from a blink-without-delay on the LED but now it's scrambling the global variable. I'm not sure. It may be doing something useful but that's just too many side-effects for one small "check" function.

int updateDAC()

{
  //we're working in volts.

//float sensorM = (float)analogRead(sensorPinM);
  //float sensorC = (float)analogRead(sensorPinC);
  float sensorM = 512.0;  //disables external trim pots
  float sensorC = 512.0;  //disables external trim pots
  c = -0.0125 + (1.0 * (sensorM - 512.0) / 1023.0);  // factor of 4 between volts and watts
  m = 0.255 + (1.0 * (sensorC - 512.0) / 1023.0);
  y = m * x + c;
  return (int)((y / 5.0) * 4095.0);
}

This seems to be a bit weird. It doesn't do what its name says. It also seems to be run just after x was set by the main code and it modifies three other global variables.

I would make x a parameter of this function. Right now, it doesn't seem to use any other input variables. Make all the others local to this function. Then rename it to "calculateDAC" or something which implies it isn't stomping on other globals.

So, in verbose mode this seems to echo commands as they're received char-by-char. Then it interprets a 'complete' command which is terminated with a CR. LF is ignored always.

Is it still receiving the chars and printing them when it "loses track"? What responses is it giving at that point? does the ?LI command return a valid-looking value?

I havent seen any errors in verbose mode.

At one point I was thinking perhaps it was a timeout issue on Serial when I had nothing connected, but I completely removed all verbose code and it didnt make a difference.

This one has been going on for a couple of weeks now... I've re-written and shuffled the code a fair bit, hence the issues above.

I'll clean up the code and re-log.

Also, thanks for putting in your time, it's much appreciated.

Is see that the mac expects various responses after it sends query style commands like "?..". I'm just wondering what response (if any) it expects after a regular "LI=..." type command? I notice that you are still issuing a Serial1.println("") response in that case.

If that were my project I'd start with just the basic serial comms and no laser involved. Just make sure I can correctly receive whatever the mac sends (preferably as simple char* strings) and just echo them to serial monitor. Once that works then start on the code to parse them.

BTW. I know the old laser expected 1200 baud. But now that it has gone you can probably up that to 9600 or more and make the response a bit snappier. Does the mac software allow you to set the baud rate, or is it also fixed at 1200?

stuart0:
Is see that the mac expects various responses after it sends query style commands like "?..". I'm just wondering what response (if any) it expects after a regular "LI=..." type command? I notice that you are still sending a Serial.println("") response in that case.

If that were my project I'd start with just the basic serial comms and no laser involved. Just make sure I can correctly receive whatever the mac sends (preferably as simple char* strings) and then echo them to serial monitor. Once that works then start on the code to parse them.

BTW. I know the old laser expected 1200 baud. But now that it has gone you can probably up that to 9600 or more and make the response a bit snappier. Does the mac software allow you to set the baud rate, or is it also fixed at 1200?

The laser returns a CR LF after a successful command such as "LI=". I think the software is happy with the CR. I assume this as the the mac throws a tantrum if the correct response isnt received.

All serial comms was confirmed by my supervisor prior to hooking up the DAC. This was partially because the wall behind the laser isnt very thick.

Unfortunately the old software cannot be modified. Not can the signal path. In fact the mac doesnt even use serial - it's a parallel bus thing which is converted to serial by an intermediate (hardware) converter.

I would rewrite without using the String class. You can get memory fragmentation, and code that stops working after a while is a classic symptom of that. It looks like your commands are short. Work out what the longest one will be and just use a static buffer.