MQTT_Connect() does not sent CONNECT packet in collaborative mode

Hi. I noticed a problem in coreMQTT in the MQTT_Connect() function, part of core_mqtt.h and I’m not sure who to address.

In core_mqtt.c, we find the following snippet:

if( status == MQTTSuccess )
{
    bytesSent = sendPacket( pContext,
                            pContext->networkBuffer.pBuffer,
                            packetSize );

    if( bytesSent < ( int32_t ) packetSize )
    {
        LogError( ( "Transport send failed for CONNECT packet." ) );
        status = MQTTSendFailed;
    }
    else
    {
        LogDebug( ( "Sent %ld bytes of CONNECT packet.",
                    ( long int ) bytesSent ) );
    }
}

/* Read CONNACK from transport layer. */
if( status == MQTTSuccess )
{
    status = receiveConnack( pContext,
                             timeoutMs,
                             pConnectInfo->cleanSession,
                             &incomingPacket,
                             pSessionPresent );
}

This snippet sends a CONNECT packet to the MQTT broker, and expects to read a CONNACK packet. The problem is that the CONNECT packet is not sent, it is only added to the IP task send queue. When in collaborative mode (configUSE_PREEMPTION == 0), there is no way the CONNECT packet can be sent before receiveConnack() is called, so the MQTT_Connect() will always return MQTTNoDataAvailable.

This problem was hidden before, because the network_transport demo code (Plaintext_FreeRTOS_recv() for plaintext use) in FreeRTOS 10.4.1 called FreeRTOS_recv() to read the packets , which has a timeout that allows CONNECT to be sent to the broker. With the FreeRTOS 202012.00 release, this demo code now calls FreeRTOS_recvcount() instead, which breaks functionality.

The fix I implemented in my code is to add taskYIELD() like so in core_mqtt.c:

...
taskYIELD();
/* Read CONNACK from transport layer. */
...

I’m not sure if this is a case for a pull request or not, nor where, because coreMQTT() has not changed, but at the same time, it does not seem to work when configUSE_PREEMPTION == 0 with the latest demo code (which IMHO is of better quality than the previous demo), so I think there IS a problem with the library, unless I’m missing something obvious.

What is the best approach to solve the issue in this case? I’m not familiar with this kind of ambiguity in open source projects.

Thanks for reporting this. The problem with adding the taskYIELD() where you have is that taskYIELD() is FreeRTOS specific, whereas we are trying to make coreMQTT RTOS agnostic - in fact you don’t need threading at all (there is also this project that implements a multithreading agent although it is not complete at the time of writing). Would it be possible to add the yield to the transport implementation instead?

Thanks Richard for the input. I have added the fix to the transport interface and created a pull request for it at #491.

Thank you for your contribution. We will look at the PR and merge it.