Serial.Available() doesn't work.

Hi guys, I'm working on an application in C# in which i have to connect to a serial port(arduino).
Now, connection and transmission from arduino to c# runs smoothly, but then at some point i need to write back to the arduino some data. I found this script on arduino's website:

// 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);
        }

Problem is, the "if" condition seems to be always true, even if I didn't send any data from c#'s serial port write method. I tried even not connecting it to the C# program, the condition still remains true and all the instructions I put inside run anyways. Example:

if (Serial.available())
  {
    digitalWrite(ledPinWait, LOW);  //led pins were declared in the same scope. 
        digitalWrite(ledPinOk, HIGH); 
}

All of this runs on the loop().
Basically if I received something back, the leds have to turn on(and the other to turn off) but they do no matter what.
What have I done wrong?
Board: Arduino Uno

you can do like this:

if (Serial.read())
{
  digitalWrite(ledPinWait, LOW);  //led pins were declared in the same scope. 
  digitalWrite(ledPinOk, HIGH); 
}

which will consume what is in the buffer....

there is more to learn here, though.

davicode:
Hi guys, I'm working on an application in C# in which i have to connect to a serial port(arduino).

What have I done wrong?

Most likely you did a programming mistake in that part of the code, which you DO NOT SHOW.

jurs:
Most likely you did a programming mistake in that part of the code, which you DO NOT SHOW.

I didn't show it because even with no serial connection activated (C# program not running)
the arduino still treats the Serial.Available() as true, so i thought the problem would be on the arduino code.

BulldogLowell:
you can do like this:

if (Serial.read())

{
  digitalWrite(ledPinWait, LOW);  //led pins were declared in the same scope.
  digitalWrite(ledPinOk, HIGH);
}




which will consume what is in the buffer....

there is [more to learn here](http://forum.arduino.cc/index.php?topic=396450), though.

Tried, same result.
Point is, i need to use the read to read some bytes incoming, and based on the value of the byte turn on/off different leds.
This way the if seems to be always true and those leds turn on, even tough they are not supposed to because I didn't transmit anything on the serial, so there couldn't be possibly anything to read. Am I wrong?
P.S. I'm reading the article you linked me, maybe it'll help

What is connected to the Arduino and which pins are being used ?

Please post your complete program even though you are sure it has nothing to do with the problem.

UKHeliBob:
What is connected to the Arduino and which pins are being used ?

Please post your complete program even though you are sure it has nothing to do with the problem.

Ok I’ll list everything.
I have an arduino uno, connected to an Adafruit NFC Shield and an RTC.
C# code

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.IO.Ports;
using System.Threading;
using System.Diagnostics;
using System.IO;
using MySql.Data.MySqlClient;
using System.Windows.Forms;

namespace accessgranted
{
    class Program
    {
        static string server, database, uid, password;
        static MySqlConnection conn;
        static StreamWriter sw;
        static SerialPort _serialPort;
        static Thread readThread;
        static string message;
        static string code;
        static int id_gate;
        

        static void Main(string[] args)
        {
            InitializeConnection();
            _serialPort = new SerialPort();
            _serialPort.PortName = SetPortName(_serialPort.PortName);
            _serialPort.BaudRate = 19200;
            _serialPort.ReadTimeout = 500;
            _serialPort.WriteTimeout = 500;

            _serialPort.Open();
            readThread = new Thread(Read);
            readThread.Start();


            
            readThread.Join();
            _serialPort.Close();

        }
        public static string SetPortName(string defaultPortName)
        {
            string portName;

            Console.WriteLine("Available Ports:");
            foreach (string s in SerialPort.GetPortNames())
            {
                Console.WriteLine("   {0}", s);
            }

            Console.Write("Enter COM port value (Default: {0}): ", defaultPortName);
            portName = Console.ReadLine();

            if (portName == "" || !(portName.ToLower()).StartsWith("com"))
            {
                portName = defaultPortName;
            }
            return portName;
        }

        public static void Read()
        {
            
            char ch;

            while (true)
            {
                sw = new StreamWriter("log.txt");
                try
                {
                    code = "";
                    message = "";
                    do
                    {
                        
                        ch = Convert.ToChar(_serialPort.ReadChar());
                        message += ch;
                        Console.Write(ch);
                        if (ch == '!')
                        {
                            Console.WriteLine();
                        }
                    }
                    while (ch != '!');
                    sw.WriteLine(message);
                    
                    for(int i=1;i<15;i++)
                    {
                        if (message[i] == ';')
                            break;
                        code += message[i];
                    }
                    id_gate = 14;
                    if(checkPermission())
                    {
                        Console.WriteLine("Access Granted");
                        byte[] b = BitConverter.GetBytes(1);
                        _serialPort.Write(b,0,4);
                        MessageBox.Show(b[0].ToString());
                        
                    }
                    else
                    {
                        Console.WriteLine("Access Denied, pending authorization...");
                        
                    }
                    
                    System.Threading.Thread.Sleep(1000);
                    
                }
                //catch timeoutexception
                catch (Exception)
                {
                }
                sw.Close();
            }
        }
        public static bool checkPermission()
        {
        }
        
    }
    
}

Arduino code

#include <Adafruit_NFCShield_I2C.h>
#include <stdio.h>
#include <stdlib.h>
#include <stdarg.h>
#include <string.h>
#include <Time.h>
#include <Wire.h>
#include "RTClib.h"
#include "Wire.h"
#define DS1307_ADDRESS 0x68
byte zero = 0x00; //workaround for issue #527

RTC_DS1307 RTC;

#define IRQ   (2)
#define RESET (3)  // Not connected by default on the NFC Shield

Adafruit_NFCShield_I2C nfc(IRQ, RESET);

String memoria = String(); // Buffer to store the returned UID


void setup(void) {

  Wire.begin();                                                     //inizializzo la libreria Wire come Master
  Serial.begin(19200);
  nfc.begin();                                                      // configure board to read RFID tags
  int pin = 13;
  int ledPinWait = 9;
  int ledPinOk = 8;
  int ledPinDenied = 10;
  pinMode(pin, INPUT);                                   // set pin to input
  digitalWrite(pin, HIGH);                 // turn on pullup resistors
  pinMode(ledPinWait, OUTPUT);
  pinMode(ledPinOk, OUTPUT);
  pinMode(ledPinDenied, OUTPUT);
  uint32_t versiondata = nfc.getFirmwareVersion();
  nfc.SAMConfig();
  //setDateTime(); //MUST CONFIGURE IN FUNCTION
}
void loop(void) {
  
  int ledPinWait = 9;
  int ledPinDenied = 8;
  int ledPinOk = 10;
  byte response[1];
  String package;
  String id_gate = String("14");
  String data_ = String();
  char invio[50];
  uint8_t id_user[] = { 0, 0, 0, 0, 0, 0, 0 }; // Buffer to store the returned UID
  boolean success;
  uint8_t uidLength;     // Length of the UID (4 or 7 bytes depending on ISO14443A card type)
  package = "#";
  String app = String();
  success = nfc.readPassiveTargetID(PN532_MIFARE_ISO14443A, &id_user[0], &uidLength);      //leggo il codice RFID
  if (success)                       //se ho letto il codice creo il pacchetto
  {
    for (uint8_t i = 0; i < uidLength; i++)
    {
      package += String(id_user[i], HEX);
    }

    if (memoria != package)
    {
      memoria = package;
      DateTime now = RTC.now();

      data_ = (String)now.year();
      data_ += "/";
      app = (String)now.month();
      if (app == "10" && app == "11" && app == "12")
      {
        data_ += app;
      }
      else
      {
        data_ += "0" + app;
      }
      data_ += "/";
      app = (String)now.day();
      if (app != "1" && app != "2" && app != "3" && app != "4" && app != "5" && app != "6" && app != "7" && app != "8" && app != "9")
      {
        data_ += app;
      }
      else
      {
        data_ += "0" + app;
      }

      data_ += " ";

      app = (String)now.hour();
      if (app != "1" && app != "2" && app != "3" && app != "4" && app != "5" && app != "6" && app != "7" && app != "8" && app != "9")
      {
        data_ += app;
      }
      else
      {
        data_ = data_ + "0" + app;
      }
      data_ += ":";
      app = (String)now.minute();
      if (app != "1" && app != "2" && app != "3" && app != "4" && app != "5" && app != "6" && app != "7" && app != "8" && app != "9")
      {
        data_ += app;
      }
      else
      {
        data_ = data_ + "0" + app;
      }
      data_ += ":";
      app = (String)now.second();
      if (app != "0" && app != "1" && app != "2" && app != "3" && app != "4" && app != "5" && app != "6" && app != "7" && app != "8" && app != "9")
      {
        data_ += app;
      }
      else
      {
        data_ = data_ + "0" + app;
      }

      package += ";";
      package += id_gate;
      package += ";";
      package += data_;
      package += "!";
      package.toCharArray(invio, 50);
      /*Serial.print("\nLunghezza del messaggio:");
        Serial.print(package.length());
        Serial.print("\n");*/
      for (int i = 0; i < package.length(); i++)         //trasmetto il pacchetto con il seriale
      {
        Serial.write(invio[i]);
      }
      digitalWrite(ledPinWait, HIGH);
     
    }
      
  }
  if (Serial.available()>0)
  {
    digitalWrite(ledPinWait, LOW);
        digitalWrite(ledPinOk, HIGH);
    /*switch (response[0],DEC)
    {
      case 1: //ACCESS GRANTED
        digitalWrite(ledPinWait, LOW);
        digitalWrite(ledPinOk, HIGH);
        delay(2500);
        break;
        digitalWrite(ledPinWait, LOW);
        digitalWrite(ledPinDenied, HIGH);
        delay(2500);
      case 2: //ACCESS DENIED

        break;
    }*/

  }

}

void setDateTime() {

  byte second =      30; //0-59
  byte minute =      47; //0-59
  byte hour =        15; //0-23
  byte weekDay =     6; //1-7
  byte monthDay =    20; //1-31
  byte month =       5; //1-12
  byte year  =       17; //0-99

  Wire.beginTransmission(DS1307_ADDRESS);
  Wire.write(zero); //stop Oscillator

  Wire.write(decToBcd(second));
  Wire.write(decToBcd(minute));
  Wire.write(decToBcd(hour));
  Wire.write(decToBcd(weekDay));
  Wire.write(decToBcd(monthDay));
  Wire.write(decToBcd(month));
  Wire.write(decToBcd(year));

  Wire.write(zero); //start

  Wire.endTransmission();

}

byte decToBcd(byte val) {
  // Convert normal decimal numbers to binary coded decimal
  return ( (val / 10 * 16) + (val % 10) );
}

Why do you change the pin numbers on your led's between setup and loop? Why do you redefine them at all? Make those global if they're needed by both functions.

if (app != "1" && app != "2" && app != "3" && app != "4" && app != "5" && app != "6" && app != "7" && app != "8" && app != "9")

What the hell is that? You couldn't think of an easier way to check that app isn't a digit? What are you actually checking for?

Its all really just some old code, not written by me, have to make some changes yet but that part works for now

davicode:
Its all really just some old code, not written by me, have to make some changes yet but that part works for now

OK. If you don't care about your code then neither do I. Good luck!

But I have to ask, when you say that the leds aren't lighting up in that if statement, which leds are you talking about. Do you mean the ones with the pin numbers from setup or the ones with pin numbers from loop? Because you haven't got the same pin numbers in both places. Could you just be looking at the wrong pins here?

But you're probably right. No reason to fix that. Just leave it confusing as hell so it's harder to debug. That's a GREAT idea. It's not like you'll ever be debugging that code... oh wait...

Delta_G:
OK. If you don’t care about your code then neither do I. Good luck!

But I have to ask, when you say that the leds aren’t lighting up in that if statement, which leds are you talking about. Do you mean the ones with the pin numbers from setup or the ones with pin numbers from loop? Because you haven’t got the same pin numbers in both places. Could you just be looking at the wrong pins here?

But you’re probably right. No reason to fix that. Just leave it confusing as hell so it’s harder to debug. That’s a GREAT idea. It’s not like you’ll ever be debugging that code… oh wait…

Delta, it’s not a matter of caring, I care about my code and back when my friends wrote it they made this mess, i know it’s very confusing, that part is needed to compose a package (code+datetime) which will then be sent via serial.I don’t honestly know right now why they did that long piece of code, I’m confused too, but it seems to be working for now and I need to focus on this serial connection matter.
Later i will adjust that too.
Thanks for the help

So you're going to wait until later to fix the obvious stuff? Why do you want to make it harder on yourself? Make the code easier to understand will make it easier to spot your problems when debugging. Really, if I saw something that stupid in a piece of code I just wouldn't use it. Who knows what other bugs they left for you.

Delta_G:
OK. If you don’t care about your code then neither do I. Good luck!

But I have to ask, when you say that the leds aren’t lighting up in that if statement, which leds are you talking about. Do you mean the ones with the pin numbers from setup or the ones with pin numbers from loop? Because you haven’t got the same pin numbers in both places. Could you just be looking at the wrong pins here?

But you’re probably right. No reason to fix that. Just leave it confusing as hell so it’s harder to debug. That’s a GREAT idea. It’s not like you’ll ever be debugging that code… oh wait…

thanks to you I noticed the double declaration, I fixed it and declared 3 global pin variables.
I edited the programand changed those long conditions with if (app < "0" && app> "9")
you were right that was easy.
But still I can’t figure out the serial reading problem

if (app < "0" && app> "9")

That doesn’t say quite the same thing. The old line would have taken ‘0’.

Also note that those need single quotes so they’re characters and not double quotes which makes them strings and blows the whole use of the comparison operators.

I know this might not make a lot of sense to you, but whenever I'm faced with a bug that I can't find, I just start fixing everything I can find until the bug finally exposes himself. Sometimes it works and sometimes it doesn't. But more often than not it helps. So even if these things seem unrelated to the bug you're chasing, fixing them is a good thing and may end up helping us get closer to the bug you want.

Was trying it now and I found that the package wasn't the same. Noticed that not all the conditions excluded the '0', so i added it where needed.

Delta_G:
... even if these things seem unrelated to the bug you're chasing, fixing them is a good thing and may end up helping us get closer to the bug you want.

solid advise

You never call Serial.read() in your Arduino code. If one character is received, even if it’s garbage because of a noisy wire, Serial.available() will always be true.

You should probably avoid using String, especially in that way. I’ll bet you are having long-term stability issues: weird hangs or data after random amounts of time.

Start by printing the pieces instead of making one big String and printing it all at once. After that, you won’t need String. Intead of this mess:

void loop()
{
    ...

      data_ = (String)now.year();
      data_ += "/";
         ...

      package += ";";
      package += id_gate;
      package += ";";
      package += data_;
      package += "!";
      package.toCharArray(invio, 50);
      /*Serial.print("\nLunghezza del messaggio:");
        Serial.print(package.length());
        Serial.print("\n");*/
      for (int i = 0; i < package.length(); i++)         //trasmetto il pacchetto con il seriale
      {
        Serial.write(invio[i]);
      }

…just do this:

const uint8_t id_gate = 14;

void loop()
{
        ...

      Serial.print( ';' );
      Serial.print( id_gate );
      Serial.print( ';' );

      Serial.print( now.year() ); // was in _data String  :P
      Serial.print( '/' );
        ...

      Serial.print( '!' );

That will get rid of most of the String usage. For the id_user, instead of this:

void loop()
{
        ...

  String package;
  String id_gate = String("14");
  String data_ = String();
  char invio[50];

  success = nfc.readPassiveTargetID(PN532_MIFARE_ISO14443A, &id_user[0], &uidLength);      //leggo il codice RFID

  if (success)                       //se ho letto il codice creo il pacchetto
  {
    for (uint8_t i = 0; i < uidLength; i++)
    {
      package += String(id_user[i], HEX);
    }

    if (memoria != package)
    {
      memoria = package;

… just do this:

uint8_t memoria[8];
uint8_t memoriaLength = 0;

void loop()
{
        ...

    success = nfc.readPassiveTargetID(PN532_MIFARE_ISO14443A, &id_user[0],  &uidLength);      //leggo il codice RFID

    if ((memoriaLength != uidLength) ||  // different lengths or 
       (memcmp( id_user, memoria, uidLength ) != 0)) { // different bytes

      memoriaLength = uidLength;
      if (memoriaLength > sizeof(memoria)) // make sure it's not too big
        memoriaLength = sizeof(memoria);
      memset( memoria, id_user, memoriaLength );

        ...

This does a direct comparison of the ID bytes instead of converting them to HEX for comparison. Faster, smaller, etc.

In general, you’ll save a bunch of RAM and program space, and it will run forever. No kidding.

Cheers,
/dev

Thank you all for your time. Thing is, I will be creating the program from scratch using all of your advices and see what happens. Will get rid of those strings. Very much appreciated.