Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
juhaylinen opened this issue Apr 16, 2018 · 8 comments
Closed

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

juhaylinen opened this issue Apr 16, 2018 · 8 comments

Comments

@juhaylinen
Copy link
Contributor

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
Copy link
Collaborator

betzw commented Apr 16, 2018

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

@juhaylinen
Copy link
Contributor Author

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

@betzw
Copy link
Collaborator

betzw commented Apr 17, 2018

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
Copy link
Collaborator

betzw commented Apr 17, 2018

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
Copy link
Contributor

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
Copy link
Collaborator

betzw commented Apr 19, 2018

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
Copy link
Contributor

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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants