ARMmbed / wifi-x-nucleo-idw01m1

X-NUCLEO-IDW0xx1 Wi-Fi Driver
3 stars 8 forks source link

x-nucleo-idw01m1: UDP socket recvfrom doesn't block #12

Closed juhaylinen closed 6 years ago

juhaylinen commented 6 years ago

I have a test that sends UDP packets to echo server and reads incoming packets back. Incoming packets are received in a separate thread.

When socket is set to blocking mode, it seems that recvfrom() call doesn't block and returns 0 several times before the packet is received. Similar behaviour is seen when socket is set to non-blocking mode. Instead of returning NSAPI_ERROR_WOULD_BLOCK, recvfrom() call returns 0 several times.

Board: NUCLEO_F401RE Shield: IDW01M1 Compiler: GCC_ARM

Steps to reproduce

mbed new x-nucleo-test
cd x-nucleo-test
mbed add wifi-x-nucleo-idw01m1
mbed target NUCLEO_F401RE
mbed toolchain GCC_ARM

Create main.cpp

#include "mbed.h"

#include "SpwfSAInterface.h"
SpwfSAInterface wifi(D8, D2);

bool running;
#define BUF_SIZE 500

static void generate_RFC_864_pattern(size_t offset, uint8_t *buf,  size_t len)
{
    while (len--) {
        if (offset % 74 == 72)
            *buf++ = '\r';
        else if (offset % 74 == 73)
            *buf++ = '\n';
        else
            *buf++ = ' ' + (offset%74 + offset/74) % 95 ;
        offset++;
    }
}

static bool check_RFC_864_pattern(void *buffer, size_t len, size_t offset)
{
    void *buf = malloc(len);
    if (!buf)
        return false;
    generate_RFC_864_pattern(offset, (uint8_t*)buf, len);
    bool match = memcmp(buf, buffer, len) == 0;
    if (!match) {
        printf("Pattern check failed\r\nWAS:%.*s\r\nREF:%.*s\r\n", len, (char*)buffer, len, (char*)buf);
    }
    free(buf);
    return match;
}

static void udp_receiver_thread(UDPSocket *socket)
{
    printf("Thread: started\n\r");
    SocketAddress addr;
    int ret = 0;
    void *buf = malloc(BUF_SIZE);

    while (running) {
        ret = socket->recvfrom(&addr, (uint8_t*)buf, BUF_SIZE);
        if (ret >= 0) {
            printf("Thread: received %d\r\n", ret);
            if (ret > 0) {
                if (!check_RFC_864_pattern((uint8_t*)buf, ret, 0)) {
                    free(buf);
                    return;
                }
            }
         } else if (ret == NSAPI_ERROR_WOULD_BLOCK) {
             printf("Thread: got NSAPI_ERROR_WOULD_BLOCK. Waiting..\r\n");
             wait(0.1);
         } else {
            printf("Thread: ERROR! UDPSocket::recvfrom() %d\r\n", ret);
            free(buf);
            return;
        }
    }
    free(buf);
}

int main()
{
    uint8_t buf[BUF_SIZE];

    printf("\r\nConnecting...\n\r");

    int ret = wifi.connect("SSID", "PASSWORD", NSAPI_SECURITY_WPA_WPA2);
    if (ret != 0) {
        printf("Failed to connect!\n");
        return 0;
    }
    printf("Connected to WiFi\n\r");

    printf("Creating UDP socket\n\r");
    UDPSocket udp_socket(&wifi);
    udp_socket.set_blocking(true);

    // Create receiving thread
    Thread udp_thread;
    running = true;
    udp_thread.start(callback(udp_receiver_thread, &udp_socket));

    printf("Sending packet to echo server\n\r");
    generate_RFC_864_pattern(0, (uint8_t*)buf, BUF_SIZE);

    ret = udp_socket.sendto("echo.mbedcloudtesting.com", 7, (uint8_t*)buf, BUF_SIZE);
    if (ret < 0) {
        printf("Error socket.sendto returned %d\n", ret);
        return 0;
    }
    printf("Sent %d bytes\n\r", ret);

    wait(4);

    printf("Sending packet to echo server\n\r");
    ret = udp_socket.sendto("echo.mbedcloudtesting.com", 7, (uint8_t*)buf, 256);
    if (ret < 0) {
        printf("Error socket.sendto returned %d\n", ret);
        return 0;
    }
    printf("Sent %d bytes\n\r", ret);

    wait(1);

    running = false;
    udp_thread.terminate();

    printf("Closing socket ..\n\r");
    ret = udp_socket.close();
    if(ret) {
        printf("socket.close() returned %d\n", ret);
        return 0;
    }

    printf("Closed\n\r");

    wifi.disconnect();
    return 0;
}

Build&Run.

betzw commented 6 years ago

Could you pls. specify which version of the driver you are using?

juhaylinen commented 6 years ago

I have tested with driver versions 1.1.7 and 1.1.6, the result is the same.

betzw commented 6 years ago

Thanks for reporting this. Should be solved in next release of the driver, even if - in my eyes - from the API specs it is not 100% clear what the behavior should be when a reception is performed on a not yet connected socket. Do you know about a document which would clarify this?

betzw commented 6 years ago

Ciao @juhaylinen, I just have published a new version of the Wi-Fi driver, which should address your issue. If you have some time, it would be nice if you can give it a try.

SeppoTakalo commented 6 years ago

API specs it is not 100% clear what the behavior should be when a reception is performed on a not yet connected socket.

UDP sockets are not connected. So there is no defined order in which you need to call recvfrom() or sendto()

After the socket is opened, it should just work.

You should be able to have UDP socket which you never read from. Only receive. Or you should be able to have socket which you never sendto. It is perfectly fine with the protocol, but not always fine with AT-command set which are mostly written only TCP in mind. I'm not sure if this is the case with IDW01

betzw commented 6 years ago

Ciao @SeppoTakalo, sorry for having used with connected the wrong term and such most likely created some confusion. What I just wanted to point out is, that in the mbed API - actually I am referring to what is expressed by the doxygen comments of the respective methods/functions - it is/was not clear to me what the behavior of a call to recvfrom() or recv() should be when there has not yet been any call to sendto()/connect() on the same socket.

P.S.: pls. note that I am not talking about POSIX but only about what mbed specifies!

SeppoTakalo commented 6 years ago

UDPSocket does not have connect() or recv() calls. Those are in TCPSocket

I still cannot see how this causes confusion, there is no mention about any precondition that should happen before the call.

    /** Send a packet over a UDP socket
     *
     *  Sends data to the specified address. Returns the number of bytes
     *  sent from the buffer.
     *
     *  By default, sendto blocks until data is sent. If socket is set to
     *  non-blocking or times out, NSAPI_ERROR_WOULD_BLOCK is returned
     *  immediately.
     *
     *  @param address  The SocketAddress of the remote host
     *  @param data     Buffer of data to send to the host
     *  @param size     Size of the buffer in bytes
     *  @return         Number of sent bytes on success, negative error
     *                  code on failure
     */
    nsapi_size_or_error_t sendto(const SocketAddress &address,
            const void *data, nsapi_size_t size);

    /** Receive a datagram over a UDP socket
     *
     *  Receives a datagram and stores the source address in address if address
     *  is not NULL. Returns the number of bytes written into the buffer. If the
     *  datagram is larger than the buffer, the excess data is silently discarded.
     *
     *  By default, recvfrom blocks until a datagram is received. If socket is set to
     *  non-blocking or times out with no datagram, NSAPI_ERROR_WOULD_BLOCK
     *  is returned.
     *
     *  @param address  Destination for the source address or NULL
     *  @param data     Destination buffer for datagram received from the host
     *  @param size     Size of the buffer in bytes
     *  @return         Number of received bytes on success, negative error
     *                  code on failure
     */
    nsapi_size_or_error_t recvfrom(SocketAddress *address,
            void *data, nsapi_size_t size);

Only precondition that have ever been mentioned is this:

    /** Create an uninitialized socket
     *
     *  Must call open to initialize the socket on a network stack.
     */
    UDPSocket();

So, as long as socket is opened, you can call sendto() or recvfrom() in any order you wish.

Also, knowing what POSIX defines helps, because we try to follow the same behaviour, even that the API is a bit different. That is the only acceptable standard covering Sockets (IEEE Std 1003.1).

juhaylinen commented 6 years ago

After some testing, the issues seems to be resolved. Thanks for the fix.