Issues when comparing strings coming from Serial.read()

I've been experiencing quite some issues when I try to compare strings coming from the Serial.read().
I'd like to receive commands from a PC, treat them and send an acknowledgement when the it is finished.
The command passed to the Arduino is in the following form: ABC12345 whereas ABC describes the type of command and the number behind is the parameter to be passed. The command are always the same and therefore I need to split the string received into the command part and the value part. And this is the point where it gets strange..

I tried different methods and I couldn't make it work.

Here some code snipplets I've tried so far:

void parseAndExecuteCommand(String command) {
  String singleCommand = String(command.substring(0,3));
  
  if (singleCommand == "ABC"){
    delay(100);  
    Serial.println("ABC1");
   
  } else if(singleCommand == "BCD"){ 
    delay(100);  
    Serial.println("BCD1");
  } ...

Or something like that:

void parseAndExecuteCommand(char* command) {

  if (strncmp(command, "ABC", 3) == 0){
    delay(100);  
    Serial.println("ABC1");

  } else if(strncmp(command, "BCD", 3) == 0){ 
    delay(100);  
    Serial.println("BCD1");  
 } ...

Unfortunately, nothing is working. I can print the information received (and even the isolated command) and it looks fine but I can't compare them. I've also tried to trim the strings before comparing them, but that made no difference.
Do you know where the issue could be?

Thanks in advance, Manuel

Can you post your whole code, the second method should work, but I'd need to see the whole file

Thanks tobyb121, here is the full code. Hope this helps! Thank you.

unsigned int nextStateChange;
unsigned int cycle_number=0;

void setup() {
  Serial.begin(9600);
  Serial.println("READY");  
}

void loop() {
  long int currentTime;
  int inbyte;
  
  currentTime = millis();
  if(currentTime>=nextStateChange) {
    
    /* some additional code that needs to
     * be in the main loop.
     */
    
    nextStateChange = currentTime+1000l;
  }
  
  checkInput();
}


void checkInput() {
  int inbyte;
  static char incomingBuffer[128]; //static
  static char bufPosition=0;
  
  if(Serial.available()>0) {
    // Read only one character per call
    inbyte = Serial.read();
    if(inbyte==10) {
      incomingBuffer[bufPosition]='\0'; // NULL terminate the string
      bufPosition=0; // Prepare for next command
      
      parseAndExecuteCommand(incomingBuffer);
    }
    else {
      incomingBuffer[bufPosition]=(char)inbyte;
      bufPosition++;
      if(bufPosition==128) {
        Serial.println("ERROR Command Overflow");
        bufPosition=0;
      }
    }
  }
}

/* This routine parses and 
 * executes any command received. 
 */
 
void parseAndExecuteCommand(char* command) {
  
  if (strncmp(command, "CHK", 2) == 0){
    delay(100);  
    Serial.println("ACK1");
   
  } else if(strncmp(command, "DND", 3) == 0){ 
    delay(100);  
    Serial.println("DND1");  
  
  } else {
    delay(100); 
    Serial.println("ERR1");  
  }
}

Oddly enough this code worked first time for me, what are you using to send the commands? If you are using the serial monitor in the IDE make sure you've set the line ending to Newline (dropdown box next to baud rate).

EDIT: One issue you may have is that bufPosition is a signed char, which takes a value from -128 to 127, when you increment it in your code on the 127th character it overflows and becomes -128, so you will never detect the overflow and you'll be writing into some random bit of memory. You just need to change its type to be an unsigned char

You're right - the code works. I've actually never tested it directly using the serial monitor in the IDE.
However, my issue still remains when I send out commands from Python.

It should work when I send out the command with an additional "\n" or "\r\n" line feed at the end, doesn't it?
Unfortunately this doesn't work correctly. Do you have any idea?

Regards,
Manuel

It would help if we could see your python code.

One commonly encountered bug in this situation is this: when you open the serial port in python, the Arduino resets and enters the bootloader for a couple seconds to see if you want to download a new program. If you write to the port immediately after opening it, the commands you send will be heard by the bootloader, not your program. Mostly, this looks like the commands are ignored.

To fix this problem, you can delay a couple seconds with time.sleep(2) after opening the serial port, or wait for a signal from Arduino that your program is running and ready to accept input.

Or it may be something else, which is why it would help to see your code.

-br

A few of things ...

You seem to be defining variables in your loop (i.e. over and over again) rather than in setup

  long int currentTime;
  int inbyte;

Indeed you seem to be defining variables in two places - both repeatedly.

And you are defining as an integer the data that's to be received from the serial port - whereas it receives bytes or chars.

What is the test "if(inbyte==10) {" supposed to do? What it actually does is check if the character whose ascii value is 10 has been received.

It might be worth while walking through your code with a pencil and paper.

...R

I can't post the code yet as I'm not home, but will do it tonight.

I try to answer some of the questions.

@billroy: There is definitely no problem there as I wait sending commands until I get an acknowledgement from the Arduino.

@Robin2: I tried to extend the example from the Arduino website (http://arduino.cc/en/Serial/read):

int incomingByte = 0;   // for incoming serial data

void setup() {
        Serial.begin(9600);     // opens serial port, sets data rate to 9600 bps
}

void loop() {

        // send data only when you receive data:
        if (Serial.available() > 0) {
                // read the incoming byte:
                incomingByte = Serial.read();

                // say what you got:
                Serial.print("I received: ");
                Serial.println(incomingByte, DEC);
        }
}

where they also declare it as a integer.
The "if(inbyte==10)" searches for a "new line" (Ascii code 10). Or am I completely wrong?

Best regards,
Manuel

So, I found some time to get my Python code. The code looks not really nice and still needs improvement.

#!/usr/bin/env python

import sys
import Tkinter
import tkMessageBox
import threading
import Queue
import serial
from collections import deque

def main(argv=None):
    if argv is None:
        argv = sys.argv

    commandList = []
    inputCmds = deque()
    outputCmds = deque()
    inputQ = Queue.Queue()
    outputQ = Queue.Queue()
    manager = CommunicationManager(inputQ,outputQ)
    gui = GuiClass(inputQ,outputQ,manager,commandList,inputCmds, outputCmds)

    gui.go()

    return 0

class GuiClass(object):
    def __init__(self,inputQ,outputQ,commManager,commandList,inputCmds,outputCmds):
        self.inputQ = inputQ
        self.outputQ = outputQ
        self.manager = commManager
        self.commandList = commandList
        self.inputCmds = inputCmds
        self.outputCmds = outputCmds
        self.currentCmd = ""

        self.root = Tkinter.Tk()
        
        text_frame = Tkinter.Frame(self.root)
        entry_frame = Tkinter.Frame(self.root)
        
        scrollbar = Tkinter.Scrollbar(text_frame)
        self.text = Tkinter.Text(text_frame,height=5,width=80,wrap=Tkinter.WORD,yscrollcommand=scrollbar.set)
        scrollbar.pack(side=Tkinter.RIGHT,fill=Tkinter.Y)
        self.text.pack(fill=Tkinter.BOTH,expand=True)
        scrollbar.config(command=self.text.yview)

        portLabel = Tkinter.Label(entry_frame, text="Arduino Port: ")
        self.arduinoFile = Tkinter.StringVar()
        portEntry = Tkinter.Entry(entry_frame, textvariable=self.arduinoFile)
        portButton = Tkinter.Button(entry_frame, text="Connect", command=self.connect)
        portLabel.pack(side=Tkinter.LEFT)
        portEntry.pack(side=Tkinter.LEFT,fill=Tkinter.X,expand=True)
        portButton.pack(side=Tkinter.LEFT)

        newTestButton = Tkinter.Button(self.root,text="New Command",command=self.newTest)
        nextTest = Tkinter.Button(self.root, text="Next commands", command=self.nextcommands)
        quitButton = Tkinter.Button(self.root,text="Quit",command=self.root.quit)
        entry_frame.pack(fill=Tkinter.X)
        text_frame.pack(fill=Tkinter.BOTH,expand=True)
        newTestButton.pack(fill=Tkinter.X)
        nextTest.pack(fill=Tkinter.X)
        quitButton.pack(fill=Tkinter.X)
        self.text.configure(state=Tkinter.DISABLED)
        self.firstline=True

        commands1 = ["DND", "DED"]
        commands2 = ["DND", "DFI100", "DFI2000", "DFI", "DED"]
        commands3 = ["DND", "DFI5000", "DED"]
        commandList.append(commands1)
        commandList.append(commands2)
        commandList.append(commands3)


        # Menubar
        menubar = Tkinter.Menu(self.root)
        
        filemenu = Tkinter.Menu(menubar,tearoff=False)
        #filemenu.add_command(label="Open...",command=self.openport)
        filemenu.add_separator()
        filemenu.add_command(label="Quit",command=self.root.quit)

        helpmenu = Tkinter.Menu(menubar,tearoff=False)
        helpmenu.add_command(label="About",command=self.about)

        menubar.add_cascade(label="File",menu=filemenu)
        menubar.add_cascade(label="Help",menu=helpmenu)
        self.root.config(menu=menubar)
        self.root.title("Python Test")

    def go(self):
        self.root.after(100, self.periodic_check)
        self.root.mainloop()

    def connect(self):
        filename = self.arduinoFile.get()
        self.manager.connect(filename)
        self.root.after(2000,self.check_connection)

    def check_connection(self):
        self.outputQ.put("CHK")

    def periodic_check(self):
        try:
            inp = self.inputQ.get_nowait()
            tokens = inp.split()
            if len(tokens)>=1:
                if tokens[0]=="ACK1":
                    self.writeline("Arduino Connected")
                elif tokens[0]=="DND1":
                    self.inputCmds.append("DND")
                elif tokens[0]=="DFI1":
                    self.inputCmds.append("DFI")
                elif tokens[0]=="DED1":
                    self.inputCmds.append("DED")
                elif tokens[0]=="AGP1":
                    self.writeline("GET POSITION")   
                elif tokens[0]=="ASP1":
                    self.writeline("SET POSITION")
                else:
                    self.writeline("Unrecognized response:")
                    self.writeline(inp)
        except Queue.Empty:
            pass

        # first time        
        if (len(self.outputCmds) > 0 and self.currentCmd == ""):
            self.currentCmd = self.outputCmds[0]
            self.outputQ.put(self.outputCmds.popleft())
            self.writeline("send: "+self.currentCmd)
        
        if (len(self.inputCmds) > 0):
            if (self.inputCmds[0] == self.currentCmd):
                self.writeline("acknowledgement: "+ self.currentCmd)
                try:
                    self.inputCmds.popleft()
                except:
                    pass
                if (len(self.outputCmds) > 0):
                    self.currentCmd = self.outputCmds[0]
                    self.outputQ.put(self.outputCmds.popleft())
                    self.writeline("send: "+self.currentCmd)

                elif (len(self.outputCmds) == 0 and len(self.inputCmds) == 0):
                    self.currentCmd = ""
                    self.writeline("Queue empty.")

        self.root.after(100, self.periodic_check)

    def writeline(self,text):
        self.text.configure(state=Tkinter.NORMAL)
        if self.firstline:
            self.firstline=False
        else:
            self.text.insert(Tkinter.END,"\n")

        self.text.insert(Tkinter.END,text)
        self.text.yview(Tkinter.END)
        self.text.configure(state=Tkinter.DISABLED)
    
    def newTest(self):
        self.outputCmds.append("DND1002001203")
    def nextTest(self):
        self.writeline("Starting sending new commands")
        try:
            mycommands = self.commandList.pop(0)
            for myCommand in mycommands:
                self.outputCmds.append(myCommand)
        except:
            self.writeline("No commands available!")
    def about(self):
        tkMessageBox.showinfo("About",__doc__,parent=self.root)
        return

class CommunicationManager(object):
    def __init__(self,inputQ,outputQ):
        self.inputQ=inputQ
        self.outputQ=outputQ
        self.serialPort=None
        self.inputThread = None
        self.outputThread = None
        self.keepRunning = True
        self.activeConnection = False

    def runInput(self):
        while self.keepRunning:
            try:
                inputline = self.serialPort.readline()
                self.inputQ.put(inputline.rstrip())
            except:
                # This area is reached on connection closing
                pass
        return

    def runOutput(self):
        while self.keepRunning:
            try:
                outputline = self.outputQ.get()
                self.serialPort.write("%s\n"%outputline)
            except:
                # This area is reached on connection closing
                pass

        return

    def connect(self,filename):
        if self.activeConnection:
            self.close()
            self.activeConnection = False
        try:
            self.serialPort = serial.Serial(filename,9600,
                                                parity=serial.PARITY_NONE,
    stopbits=serial.STOPBITS_TWO,
    bytesize=serial.EIGHTBITS)
        except serial.SerialException:
            self.inputQ.put("ERROR Unable to Connect to Serial Port")
            return
        self.keepRunning=True
        self.inputThread = threading.Thread(target=self.runInput)
        self.inputThread.daemon=True
        self.inputThread.start()
        self.outputThread = threading.Thread(target=self.runOutput)
        self.outputThread.daemon=True
        self.outputThread.start()
        self.activeConnection = True

    def close(self):
        self.keepRunning=False;
        self.serialPort.close()
        self.outputQ.put("TERMINATE") # This does not get sent, but stops the outputQ from blocking.
        self.inputThread.join()
        self.outputThread.join()
        self.inputQ.put("IOHALT")

if __name__ == "__main__":
    sys.exit(main())

I suppose that the problem is linked to the self.serialPort.write("%s\n"%outputline) part where I send out the commands to the Arduino.
In the serial monitor everything works fine, but sending out commands from Python only works when they have only 3 characters and no value passed.

Do you have any idea? Thanks in advance,
Manuel

I'm afraid I haven't done anything with python for a few years now, so probably won't be much help.
Try sticking a print(outputline) in your runOutput function to see that it prints the right thing, I would have thought it would, but you never know.
I also noticed that in your run output you do self.outputQ.get(), regardless of whether your output buffer has anything in it to get. I assume this will return a null value the array is empty, but then you append a \n to it and write it to the serial port, so your Arduino is probably getting a lot of blank commands.
I'm not sure if python serial blocks when writing, but you could easily fill the buffer if you're only sending at 9600 baud (that's only 960 characters a second) if you just send continuously from your pc.