Arduino ethernet c# client not receiving data

Question

I am having a problem receiving data (a string of text) from an Arduino via a C# Winform app. The sketch on the Arduino basically reply with whatever I send. I am able to send data. What is strange is that if I telnet the device and type any text it responds correctly so it appears the problem is my C# code, however, I can't seem to figure out where I am incorrect.

Here is what I have

public partial class Form1 : Form
{

    System.Net.Sockets.TcpClient clientSocket = new System.Net.Sockets.TcpClient();
    NetworkStream serverStream = default(NetworkStream);
    string readData = null;


    public Form1()
    {
        InitializeComponent();

    }

    private void button1_Click(object sender, EventArgs e)
    {
        byte[] outStream = System.Text.Encoding.ASCII.GetBytes(textBox2.Text);
        serverStream.Write(outStream, 0, outStream.Length);
        serverStream.Flush();
    }


    private void button2_Click(object sender, EventArgs e)
    {

        clientSocket.Connect("192.168.1.177", 5223);
        readData = "Conected Arduino ...";
        msg();
        serverStream = clientSocket.GetStream();
        byte[] outStream = System.Text.Encoding.ASCII.GetBytes(textBox2.Text);
        serverStream.Write(outStream, 0, outStream.Length);
        serverStream.Flush();

        Thread ctThread = new Thread(getData);
        ctThread.Start();
    }



    private void getData()
    {

        while (true)
        {

            while (true)
            {
                serverStream = clientSocket.GetStream();
                int buffSize = clientSocket.ReceiveBufferSize;
                byte[] inStream = new byte[10025];
                serverStream.Read(inStream, 0, buffSize);
                string returndata = System.Text.Encoding.ASCII.GetString(inStream);
                readData = "" + returndata;
                msg();
            }

        }
    }

    private void msg()
    {

        this.BeginInvoke(new Action(() =>
        {
            textBox1.Text = String.Format("{0}{1} >> {2}", textBox1.Text, Environment.NewLine, readData);
        }
    ));
    }


}

Here is part of the Arduino sketch

void loop() {
// wait for a new client:
EthernetClient client = server.available();

// when the client sends the first byte, say hello:
if (client) {
if (!alreadyConnected) {
  // clead out the input buffer:
  client.flush();    
  Serial.println("We have a new client");
  client.println("Hello, client!");
  alreadyConnected = true;
}

if (client.available() > 0) {
  // read the bytes incoming from the client:
  char thisChar = client.read();
  // echo the bytes back to the client:
  //server.write(thisChar);
  // echo the bytes to the server as well:
  //Serial.write(thisChar);
   if (inputPos < maxLength-1) 
   { 
    if (thisChar == '\n') 
        {
            inputString[inputPos] = 0;
                            server.write(inputString);
            Serial.write(inputString);
            inputPos = 0;
        } else {
                // add it to the inputString:
            inputString[inputPos] = thisChar;
            inputPos++;
        }
   } else {
     inputPos = 0;
   }      
}
}
}

Answer 1

Your code has a number of things wrong with it, including what has to be the most common novice error I've seen in any context: you fail to take into account the number of bytes actually read. Plus you have another common variation on the theme, which is that you process the entire receive buffer array as if the whole thing had valid data in it.

Without a way to test, it's hard to know for sure. But at the very least, you should change your receive method to look like this:

private void getData()
{
    serverStream = clientSocket.GetStream();

    while (true)
    {
        byte[] inStream = new byte[10025];
        int bytesRead = serverStream.Read(inStream, 0, inStream.Length);
        readData = System.Text.Encoding.ASCII.GetString(inStream, 0, bytesRead);
        msg();
    }
}

• DON'T pointlessly nest a while (true) loop inside another while (true) loop
• DON'T create a new NetworkStream every time you want to read
• DON'T concern yourself with the Socket.ReceiveBufferSize property value
• DO capture and use the return value from the read operation
• DON'T concatenate the empty string with other strings (even in an iterated scenario, one should be using StringBuilder instead, and here you're not even iterating the concatenation!)

Of course, not all of those flaws are fatal. The biggest issues are the new NetworkStream on each read and the mismanagement of the receive buffer and result value. But really, you should strive for all of the code to be good.

Note that the above merely improves the code. Even the above still has a variation on "the most common novice error I've seen in any context": while it does use the return value from the read operation, it does not do everything with it that it should. In particular:

1. You are not actually guaranteed that you will receive all of the bytes that were sent to you in one read operation. This means your application protocol really should have some way for you to identify within the stream of bytes you're reading where one message ends and the next one starts.
2. When the connection is gracefully closed, the read operation will return 0 as the byte count, at which point your own code is supposed to respond by finishing up whatever writes to the socket you need to (if any) and then gracefully closing your own socket by calling Socket.Shutdown(SocketShutdown.Both) and finally Socket.Close().

But the above should at least help you make forward progress in your project.

One other thing you really should change IMHO which I didn't bother in the above, is that you should not use an instance field (i.e. readData) to pass data when simply passing it as a method parameter would suffice. You should avoid side-effects in your code as much as possible. It will make the code much easier to comprehend and thus to write correctly, among other things.