TCPClient, available() & read() methods, supported scenario?

Troubleshooting
1

Hi,

In the context of one of my projects, a “monitor and command” middleware for embedded projects (http://monitorandcommand.codeplex.com/), I am trying to develop a “Photon/Wiring Client” library
that allow the Photon to send/receive messages to/from the a Server.

I managed to code the “send” part rather easily, and my photon has now been sending temperature and humidity messages to the Server for days.

But I cannot get the “read” part to work. Whatever I try, I cannot get the available() and read() methods to return anything but 0, even though I am 100% sure that the Server sends messages back to the device.

Coming from .NET/NETMF, where I use Threads, I am rather new to Wiring/Photon, and I am now wondering if what I am trying to achieve is feasible with Wiring/Photon.

If I simplify things to the Max, my library defines a mcClient object that references a TCPClient and implements the methods required to send/receive fixed size Messages to/from the Server. What I am trying to achieve is, in the loop() method, to perform periodic work while listening to incoming messages eventually sent by the Server.

My main loop thus look like this :

loop() {

    delay(50); //is it required ?

    if (mcClient.IsConnected()) {

        if (mcClient.ReceivedMessage()) //behind the hood, we use available() here to test if we received anything from the server    
        {
            Message message = mcClient.ReadMessage(); // we get the message from the client
            HandleMessage(message); //We handle the message 
        }

        //Periodically, we do work, like reading sensors and sending messages to the Server
        if (millis() - lastTime > DO_WORK_PERIOD)
        {
            DoWork(); //Behind the hood, we send messages using TCPClient, and it works perfectly
            lastTime = millis();
        }    
    } else {
        delay(RECONNECTION_DELAY);
        ConnectAndRegister(); 
    }
}

Is something like this feasible ? Is my approach the right one ?

I would be glad if anybody could at least send me in the right direction.

Thanks in advance,

Paul

2

The approach is reasonable and it’s definitely feasible. It’s hard to tell what’s wrong without more of the code.

Is this just a typo in your post:

if (!mcClient.IsConnected())

It appears to be executing the code to handle messages only when not connected, instead of when connected.

3

It’s indeed a typo, I corrected in the original message.

Actually, in the above code, just to test the available() method, I went as far as trying this :

if (mcClient.IsConnected()) {

if (mcClient.IsAvailable()) 
{
    Message message = mcClient.ReadMessage(); // we get the message from the client
    HandleMessage(message); //We handle the message 
}

…

where IsAvailable() is :

bool MCClient::IsAvailable() {
return client.available(); //client being an instance of TCPClient.
}

N.B. I also tried to return int instead of bool, or explicitely test available() > 0, not begin sure that the value returned by available() can convert to bool automatically, even though the code compiles.

available() never seems to be > 0 ! Or maybee I am fooling myself at some point, since my C++ skills are rusted.

Anyway, I am glad to read that my approach is feasible/reasonable. I intend to propose the library publicly when it’s ready, but in the meantime, I am ready to share it (where ?) to anyone who wants to review it to help me with the troubleshooting.

Thanks in advance,

Paul

4

I edited the code to reflect the original intent.

void Process() {

    delay(WAIT_MESSAGE_DELAY); //tested with 0 to 50 milliseconds delay, without any effect 

    while (mcClient.HasMessage()) //see below
    {
        message = mcClient.Read(); //see below
        HandleReceivedMessage(message);
        delete message;
        message = NULL;
    }

    //Periodically, we do work
    if (millis() - lastTime > DO_WORK_PERIOD)
    {
        DoWork(); 
        lastTime = millis();
    }
}

bool MCClient::HasMessage() {
    **return (client.available() >= MCClient::BUFFER_SIZE);** //BUFFER_SIZE is the fixed_size of the message
}

Message* MCClient::Read() {    
    Message* message = NULL; 

    if (HasMessage()) {

        unsigned char* messageChars = new unsigned char[MCClient::BUFFER_SIZE]; 
        //int messageLength = client.read(messageChars, MCClient::BUFFER_SIZE); //compiles, but never tested

        while(client.available() > 0) {
            char c = client.read();
            //removed to save space
        }
        //removed to save space 
    }
    
    return message; 
}

Whatever i do, and i tried many different options, client.available() is never > 0, leading me to think that even though the code compile, the way it’s organized leads to an unsupported use of the available() method.

Paul

5

I’m not sure what’s wrong with your code, but here’s a working example:

The Photon main code is in fixedlentcprcv.cpp.

The class to easily handle fixed length messages in a TCPClient stream is in FixedLengthMsgClient.cpp and .h. The .h file contains comments that explain the methods.

There’s also a test server for node.js in server.js. The server exercises a bunch of things including:

  • Sending message one a time
  • Sending a bunch of messages (1-20) at once
  • Sending a message a byte at a time

This should exercise the functionality of the message class.

3 Likes
6

Thanks a lot !

Looking at your code, the first thing that strikes me is that your TCPClient instance is declared in the main file and passed as an argument to the “Client” class. Second, you use SYSTEM_THREAD(ENABLED). Additionally, the FixedLengthMsgClient.cpp file contains approaches I would have not though of by myself.

I am gonna try all this, adapt my code, and I’ll keep ou informed about the results.

Paul

P.S. the fixed size of my messages is set to 512 (was 256). Is there any limitation on the photon about message size ?

7

There should be no problem at 256 or 512 bytes. I’m re-running the test right now at 512 and it seems fine.

1 Like
8

I adapted my code, following the code sample you posted on GitHub, as follows:

SYSTEM_THREAD(ENABLED);
...
TCPClient tcpClient; 
MCClient* mcClient;
Message* message = NULL; 

void setup() {
      ...
      mcClient = new MCClient(&tcpClient);  
      ...      
}

void loop() {
    ...
    //We check for Message data filling the buffer
    int result = mcClient->BufferMessageData(); 
    if (result == MCClient::BUFFER_READY)
    {
        message = mcClient->Read();
        if (message != NULL) {
            HandleReceivedMessage();
            delete message;
            message = NULL;    
        }
    }

    //Periodically, we do some work
    if (millis() - lastTime > DO_WORK_PERIOD)
    {
        DoWork(); 
        lastTime = millis();
    }           
    ...
}

where BufferMessageData() follows what I found in your code sample :

int MCClient::BufferMessageData() {
    
	if (client == NULL) {
		return BUFFER_DISCONNECTED;
	}

	if (client->connected()) {
		**int count = client->read(&buffer[bufferOffset], MCClient::MESSAGE_SIZE - bufferOffset);**
		if (count <= 0) {
		    Serial.printlnf("count=%d", count);
			return (bufferOffset == 0) ? BUFFER_NONE : BUFFER_PARTIAL;
		}

		// New data arrived
		bufferOffset += (size_t)count;
		if (bufferOffset < MCClient::MESSAGE_SIZE) {
			// Still not a full message
			Serial.printlnf("partial offset=%d size=%d", bufferOffset, MCClient::MESSAGE_SIZE);
			return BUFFER_PARTIAL;
		}

		// Got a full message
		bufferOffset = 0;
		return BUFFER_READY;
	}
	else {
		client = NULL;
		return BUFFER_DISCONNECTED;
	}
}

size_t MCClient::MESSAGE_SIZE = 512;

MCClient::MCClient(TCPClient* tcpClient) {
    this->client = tcpClient;    
    buffer = (byte*)malloc(MCClient::MESSAGE_SIZE);
    bufferOffset = 0;
}

The complete code is available @ https://github.com/codeabilitynet/monitorandcommandclientforparticlephoton

The loop runs, the DoWork() method does its work (reading temperature/humidity from a DHT22 and sending messages to the server) every minute, but when the server sends “command” messages back to the Photon, the count variable containing the value returned by client->read(…) is always -1.

I had a look at :

I am stuck there. Any idea why read(), available() and bufferCount() never seem to return anything in this context?

If you look at monitorandcommandclient.ino, you will notice that, before trying to read anything for the socket, my code send several “registration” messages of 512 bytes.

Could it be that the messages sent by the server “overflow” some buffers somewhere?

Paul

9

I’m not seeing a bug in your code. There should be no problem with multiple receiving multiple 512 byte registration messages. In my test code, I sent up to 20 messages (at 512 bytes each) at the Photon at once and it worked fine. TCP is designed to handle this situation. Your problem is quite mysterious!

1 Like
10

The code I took from your sample is working perfectly !

I checked and validated methodically every step of the connection/registration of the Photon to my Server until I reached the BufferMessageData() method and at some point, I saw the buffer receiving the server’s messages through the following line :

Serial.printlnf("partial offset=%d size=%d", bufferOffset, MCClient::MESSAGE_SIZE);

I suspect I introduced some pointer problem somewhere through the multiple changes I did but I am not even sure. My last use of C/C++ dates from the last century and I had forgotten how tricky it could be to work with pointers without an interactive debugger.

I shall now adapt and debug the method that “reads” the buffered message and the first inception of my “Client” will be ready.

Anyway, thank you very much for the support : without your sample and replies, I would have been completely stuck.

Paul

1 Like