From 117eb0bc8759858f337c225644471be253ad70dc Mon Sep 17 00:00:00 2001 From: deepikabhavnani Date: Sun, 18 Nov 2018 20:41:03 -0600 Subject: [PATCH] Add socketstats stub functions for unittest and addressed reviews --- TESTS/netsocket/udp/udpsocket_echotest.cpp | 22 ++++-- .../EthernetInterface/unittest.cmake | 1 + .../netsocket/InternetSocket/unittest.cmake | 1 + .../netsocket/NetworkInterface/unittest.cmake | 1 + .../netsocket/NetworkStack/unittest.cmake | 1 + .../netsocket/TCPServer/unittest.cmake | 1 + .../netsocket/TCPSocket/unittest.cmake | 1 + .../netsocket/UDPSocket/unittest.cmake | 1 + UNITTESTS/stubs/SocketStats_Stub.cpp | 61 ++++++++++++++++ features/netsocket/InternetSocket.cpp | 4 +- features/netsocket/SocketStats.cpp | 4 +- features/netsocket/SocketStats.h | 72 +++++++++++++++---- features/netsocket/TCPServer.cpp | 3 +- features/netsocket/TCPSocket.cpp | 11 +-- features/netsocket/TCPSocket.h | 9 --- features/netsocket/UDPSocket.cpp | 11 +-- features/netsocket/UDPSocket.h | 9 --- 17 files changed, 151 insertions(+), 62 deletions(-) create mode 100644 UNITTESTS/stubs/SocketStats_Stub.cpp diff --git a/TESTS/netsocket/udp/udpsocket_echotest.cpp b/TESTS/netsocket/udp/udpsocket_echotest.cpp index 9aebad9655..91a3aef1d6 100644 --- a/TESTS/netsocket/udp/udpsocket_echotest.cpp +++ b/TESTS/netsocket/udp/udpsocket_echotest.cpp @@ -183,18 +183,26 @@ void UDPSOCKET_ECHOTEST_NONBLOCK() } free(stack_mem); -#ifdef MBED_CONF_NSAPI_SOCKET_STATS_ENABLE - TEST_ASSERT_EQUAL(1, fetch_stats()); - TEST_ASSERT_EQUAL(NSAPI_UDP, udp_stats[0].proto); - TEST_ASSERT(udp_stats[0].sent_bytes != 0); - TEST_ASSERT(udp_stats[0].recv_bytes != 0); -#endif - // Packet loss up to 30% tolerated if (packets_sent > 0) { double loss_ratio = 1 - ((double)packets_recv / (double)packets_sent); printf("Packets sent: %d, packets received %d, loss ratio %.2lf\r\n", packets_sent, packets_recv, loss_ratio); TEST_ASSERT_DOUBLE_WITHIN(TOLERATED_LOSS_RATIO, EXPECTED_LOSS_RATIO, loss_ratio); + +#ifdef MBED_CONF_NSAPI_SOCKET_STATS_ENABLE + count = fetch_stats(); + for (j = 0; j < count; j++) { + if ((NSAPI_UDP == udp_stats[j].proto) && (SOCK_OPEN == udp_stats[j].state)) { + TEST_ASSERT(udp_stats[j].sent_bytes != 0); + TEST_ASSERT(udp_stats[j].recv_bytes != 0); + break; + } + } + loss_ratio = 1 - ((double)udp_stats[j].recv_bytes / (double)udp_stats[j].sent_bytes); + printf("Bytes sent: %d, bytes received %d, loss ratio %.2lf\r\n", udp_stats[j].sent_bytes, udp_stats[j].recv_bytes, loss_ratio); + TEST_ASSERT_DOUBLE_WITHIN(TOLERATED_LOSS_RATIO, EXPECTED_LOSS_RATIO, loss_ratio); + +#endif } TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock.close()); } diff --git a/UNITTESTS/features/netsocket/EthernetInterface/unittest.cmake b/UNITTESTS/features/netsocket/EthernetInterface/unittest.cmake index 78a9f1f943..472d5f0a26 100644 --- a/UNITTESTS/features/netsocket/EthernetInterface/unittest.cmake +++ b/UNITTESTS/features/netsocket/EthernetInterface/unittest.cmake @@ -33,4 +33,5 @@ set(unittest-test-sources stubs/stoip4_stub.c stubs/ip4tos_stub.c stubs/NetworkStack_stub.cpp + stubs/SocketStats_Stub.cpp ) diff --git a/UNITTESTS/features/netsocket/InternetSocket/unittest.cmake b/UNITTESTS/features/netsocket/InternetSocket/unittest.cmake index f2f89e1136..ef550912b5 100644 --- a/UNITTESTS/features/netsocket/InternetSocket/unittest.cmake +++ b/UNITTESTS/features/netsocket/InternetSocket/unittest.cmake @@ -25,4 +25,5 @@ set(unittest-test-sources stubs/EventFlags_stub.cpp stubs/stoip4_stub.c stubs/ip4tos_stub.c + stubs/SocketStats_Stub.cpp ) diff --git a/UNITTESTS/features/netsocket/NetworkInterface/unittest.cmake b/UNITTESTS/features/netsocket/NetworkInterface/unittest.cmake index 336e9b106f..fd21980983 100644 --- a/UNITTESTS/features/netsocket/NetworkInterface/unittest.cmake +++ b/UNITTESTS/features/netsocket/NetworkInterface/unittest.cmake @@ -25,4 +25,5 @@ set(unittest-test-sources stubs/nsapi_dns_stub.cpp stubs/EventFlags_stub.cpp features/netsocket/NetworkInterface/test_NetworkInterface.cpp + stubs/SocketStats_Stub.cpp ) diff --git a/UNITTESTS/features/netsocket/NetworkStack/unittest.cmake b/UNITTESTS/features/netsocket/NetworkStack/unittest.cmake index 91ea42d5a2..e495e1c15e 100644 --- a/UNITTESTS/features/netsocket/NetworkStack/unittest.cmake +++ b/UNITTESTS/features/netsocket/NetworkStack/unittest.cmake @@ -28,4 +28,5 @@ set(unittest-test-sources stubs/nsapi_dns_stub.cpp stubs/EventFlags_stub.cpp features/netsocket/NetworkStack/test_NetworkStack.cpp + stubs/SocketStats_Stub.cpp ) diff --git a/UNITTESTS/features/netsocket/TCPServer/unittest.cmake b/UNITTESTS/features/netsocket/TCPServer/unittest.cmake index 942d3aade0..288ec07e93 100644 --- a/UNITTESTS/features/netsocket/TCPServer/unittest.cmake +++ b/UNITTESTS/features/netsocket/TCPServer/unittest.cmake @@ -28,4 +28,5 @@ set(unittest-test-sources stubs/nsapi_dns_stub.cpp stubs/EventFlags_stub.cpp features/netsocket/TCPServer/test_TCPServer.cpp + stubs/SocketStats_Stub.cpp ) diff --git a/UNITTESTS/features/netsocket/TCPSocket/unittest.cmake b/UNITTESTS/features/netsocket/TCPSocket/unittest.cmake index 474a2b158e..abad503b68 100644 --- a/UNITTESTS/features/netsocket/TCPSocket/unittest.cmake +++ b/UNITTESTS/features/netsocket/TCPSocket/unittest.cmake @@ -26,4 +26,5 @@ set(unittest-test-sources stubs/EventFlags_stub.cpp stubs/stoip4_stub.c stubs/ip4tos_stub.c + stubs/SocketStats_Stub.cpp ) diff --git a/UNITTESTS/features/netsocket/UDPSocket/unittest.cmake b/UNITTESTS/features/netsocket/UDPSocket/unittest.cmake index 82d149e086..b4080496bd 100644 --- a/UNITTESTS/features/netsocket/UDPSocket/unittest.cmake +++ b/UNITTESTS/features/netsocket/UDPSocket/unittest.cmake @@ -26,4 +26,5 @@ set(unittest-test-sources stubs/nsapi_dns_stub.cpp stubs/stoip4_stub.c stubs/ip4tos_stub.c + stubs/SocketStats_Stub.cpp ) diff --git a/UNITTESTS/stubs/SocketStats_Stub.cpp b/UNITTESTS/stubs/SocketStats_Stub.cpp new file mode 100644 index 0000000000..cc1fdbab2a --- /dev/null +++ b/UNITTESTS/stubs/SocketStats_Stub.cpp @@ -0,0 +1,61 @@ +/* mbed Microcontroller Library + * Copyright (c) 2018 ARM Limited + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "SocketStats.h" + +int SocketStats::get_entry_position(const Socket *const reference_id) +{ + return 0; +} + +size_t SocketStats::mbed_stats_socket_get_each(mbed_stats_socket_t *stats, size_t count) +{ + return 0; +} + +SocketStats::SocketStats() +{ +} + +void SocketStats::stats_new_socket_entry(const Socket *const reference_id) +{ + return; +} + +void SocketStats::stats_update_socket_state(const Socket *const reference_id, socket_state state) +{ + return; +} + +void SocketStats::stats_update_peer(const Socket *const reference_id, const SocketAddress &peer) +{ + return; +} + +void SocketStats::stats_update_proto(const Socket *const reference_id, nsapi_protocol_t proto) +{ + return; +} + +void SocketStats::stats_update_sent_bytes(const Socket *const reference_id, size_t sent_bytes) +{ + return; +} + +void SocketStats::stats_update_recv_bytes(const Socket *const reference_id, size_t recv_bytes) +{ + return; +} diff --git a/features/netsocket/InternetSocket.cpp b/features/netsocket/InternetSocket.cpp index 5d24f3b1c4..8db00eed8c 100644 --- a/features/netsocket/InternetSocket.cpp +++ b/features/netsocket/InternetSocket.cpp @@ -24,6 +24,7 @@ InternetSocket::InternetSocket() _readers(0), _writers(0), _pending(0), _factory_allocated(false) { + _socket_stats.stats_new_socket_entry(this); } InternetSocket::~InternetSocket() @@ -48,6 +49,7 @@ nsapi_error_t InternetSocket::open(NetworkStack *stack) return err; } + _socket_stats.stats_update_socket_state(this, SOCK_OPEN); _socket = socket; _event = callback(this, &InternetSocket::event); _stack->socket_attach(_socket, Callback::thunk, &_event); @@ -72,7 +74,7 @@ nsapi_error_t InternetSocket::close() _socket = 0; ret = _stack->socket_close(socket); _stack = 0; // Invalidate the stack pointer - otherwise open() fails. - + _socket_stats.stats_update_socket_state(this, SOCK_CLOSED); // Wakeup anything in a blocking operation // on this socket event(); diff --git a/features/netsocket/SocketStats.cpp b/features/netsocket/SocketStats.cpp index df6ea66d89..53a895d279 100644 --- a/features/netsocket/SocketStats.cpp +++ b/features/netsocket/SocketStats.cpp @@ -140,7 +140,7 @@ void SocketStats::stats_update_sent_bytes(const Socket *const reference_id, size #ifdef MBED_CONF_NSAPI_SOCKET_STATS_ENABLE _mutex->lock(); int position = get_entry_position(reference_id); - if ((position >= 0) && (sent_bytes > 0)) { + if ((position >= 0) && ((int32_t)sent_bytes > 0)) { _stats[position].sent_bytes += sent_bytes; } _mutex->unlock(); @@ -152,7 +152,7 @@ void SocketStats::stats_update_recv_bytes(const Socket *const reference_id, size #ifdef MBED_CONF_NSAPI_SOCKET_STATS_ENABLE _mutex->lock(); int position = get_entry_position(reference_id); - if ((position >= 0) && (recv_bytes > 0)) { + if ((position >= 0) && ((int32_t)recv_bytes > 0)) { _stats[position].recv_bytes += recv_bytes; } _mutex->unlock(); diff --git a/features/netsocket/SocketStats.h b/features/netsocket/SocketStats.h index fe53ab71d9..8850b35dd0 100644 --- a/features/netsocket/SocketStats.h +++ b/features/netsocket/SocketStats.h @@ -27,6 +27,12 @@ #define MBED_CONF_NSAPI_SOCKET_STATS_MAX_COUNT 10 #endif +/** Enum of socket states + * + * Can be used to specify current state of socket - Open / Close / Connected / Listen + * + * @enum socket_state + */ typedef enum { SOCK_CLOSED, /**< Socket is closed and does not exist anymore in the system */ SOCK_OPEN, /**< Socket is open, but not associated to any peer address */ @@ -34,6 +40,8 @@ typedef enum { SOCK_LISTEN, /**< Socket is listening for incoming connections */ } socket_state; +/** Structure to parse socket statistics + */ typedef struct { Socket *reference_id; /**< Used for identifying socket */ SocketAddress peer; /**< Last associated peername of this socket (Destination address) */ @@ -51,6 +59,7 @@ typedef struct { class SocketStats { public: +#if !defined(DOXYGEN_ONLY) /** Create an socket statictics object * * Application users must not create class objects. @@ -62,7 +71,7 @@ public: virtual ~SocketStats() { } - +#endif /** * Fill the passed array of structures with the socket statistics for each created socket. * @@ -76,26 +85,62 @@ public: */ static size_t mbed_stats_socket_get_each(mbed_stats_socket_t *stats, size_t count); +#if !defined(DOXYGEN_ONLY) /** Add entry of newly created socket in statistics array. - @Note: Entry in the array will be maintained even after socket is closed. - Entry will be over-written for sockets which were closed first, in case - we socket creation count exceeds `MBED_CONF_NSAPI_SOCKET_STATS_MAX_COUNT`. - */ + * API used by socket (TCP / UDP) layers only, not to be used by application. + * + * @param reference_id Id to identify socket in data array. + * + * @Note: Entry in the array will be maintained even after socket is closed. + * Entry will be over-written for sockets which were closed first, in case + * we socket creation count exceeds `MBED_CONF_NSAPI_SOCKET_STATS_MAX_COUNT`. + * + */ void stats_new_socket_entry(const Socket *const reference_id); - /** Updates the state of socket and along with that records tick_last_change */ + /** Updates the state of socket and along with that records tick_last_change. + * API used by socket (TCP / UDP) layers only, not to be used by application. + * + * @param reference_id Id to identify socket in data array. + * @param state Parameter to update current state of socket. + * + */ void stats_update_socket_state(const Socket *const reference_id, socket_state state); - /** Update the peer information of the socket */ + /** Update the peer information of the socket. + * API used by socket (TCP / UDP) layers only, not to be used by application. + * + * @param reference_id Id to identify socket in data array. + * @param peer Parameter to update destination peer information + * + */ void stats_update_peer(const Socket *const reference_id, const SocketAddress &peer); - /** Update socket protocol */ + /** Update socket protocol. + * API used by socket (TCP / UDP) layers only, not to be used by application. + * + * @param reference_id Id to identify socket in data array. + * @param proto Parameter to update the protocol type of socket. + * + */ void stats_update_proto(const Socket *const reference_id, nsapi_protocol_t proto); - /** Update bytes sent on socket, which is cumulative count per socket */ + /** Update bytes sent on socket, which is cumulative count per socket. + * API used by socket (TCP / UDP) layers only, not to be used by application. + * + * @param reference_id Id to identify socket in data array. + * @param sent_bytes Parameter to append bytes sent over the socket. + * + */ void stats_update_sent_bytes(const Socket *const reference_id, size_t sent_bytes); - /** Update bytes received on socket, which is cumulative count per socket */ + /** Update bytes received on socket, which is cumulative count per socket + * API used by socket (TCP / UDP) layers only, not to be used by application. + * + * @param reference_id Id to identify socket in data array. + * @param recv_bytes Parameter to append bytes received by the socket + * + */ void stats_update_recv_bytes(const Socket *const reference_id, size_t recv_bytes); #ifdef MBED_CONF_NSAPI_SOCKET_STATS_ENABLE @@ -105,10 +150,13 @@ private: static uint32_t _size; /** Internal function to scan the array and get position of element in the list. - This API locks the mutex and the next API call updating the entry in the array - should release the lock */ + * + * @param reference_id Id to identify socket in data array. + * + */ int get_entry_position(const Socket *const reference_id); #endif +#endif }; #endif diff --git a/features/netsocket/TCPServer.cpp b/features/netsocket/TCPServer.cpp index 958912de9f..209052f3e6 100644 --- a/features/netsocket/TCPServer.cpp +++ b/features/netsocket/TCPServer.cpp @@ -20,12 +20,11 @@ using mbed::Callback; TCPServer::TCPServer() { - _socket_stats.stats_new_socket_entry(this); + _socket_stats.stats_update_proto(this, NSAPI_TCP); } TCPServer::~TCPServer() { - _socket_stats.stats_update_socket_state(this, SOCK_CLOSED); } nsapi_error_t TCPServer::accept(TCPSocket *connection, SocketAddress *address) diff --git a/features/netsocket/TCPSocket.cpp b/features/netsocket/TCPSocket.cpp index d2ead4955d..a88fece796 100644 --- a/features/netsocket/TCPSocket.cpp +++ b/features/netsocket/TCPSocket.cpp @@ -20,7 +20,7 @@ TCPSocket::TCPSocket() { - _socket_stats.stats_new_socket_entry(this); + _socket_stats.stats_update_proto(this, NSAPI_TCP); } TCPSocket::TCPSocket(TCPSocket *parent, nsapi_socket_t socket, SocketAddress address) @@ -36,19 +36,10 @@ TCPSocket::TCPSocket(TCPSocket *parent, nsapi_socket_t socket, SocketAddress add TCPSocket::~TCPSocket() { - _socket_stats.stats_update_socket_state(this, SOCK_CLOSED); -} - -nsapi_error_t TCPSocket::close() -{ - _socket_stats.stats_update_socket_state(this, SOCK_CLOSED); - return InternetSocket::close(); } nsapi_protocol_t TCPSocket::get_proto() { - _socket_stats.stats_update_proto(this, NSAPI_TCP); - _socket_stats.stats_update_socket_state(this, SOCK_OPEN); return NSAPI_TCP; } diff --git a/features/netsocket/TCPSocket.h b/features/netsocket/TCPSocket.h index 1f8baae83c..c52a5fc094 100644 --- a/features/netsocket/TCPSocket.h +++ b/features/netsocket/TCPSocket.h @@ -61,15 +61,6 @@ public: */ virtual ~TCPSocket(); - /** Close the socket. - * - * Closes any open connection and deallocates any memory associated - * with the socket. Called from destructor if socket is not closed. - * - * @return NSAPI_ERROR_OK on success, negative error code on failure - */ - virtual nsapi_error_t close(); - /** Override multicast functions to return error for TCP * */ diff --git a/features/netsocket/UDPSocket.cpp b/features/netsocket/UDPSocket.cpp index b0f2275402..415ff315b3 100644 --- a/features/netsocket/UDPSocket.cpp +++ b/features/netsocket/UDPSocket.cpp @@ -20,24 +20,15 @@ UDPSocket::UDPSocket() { - _socket_stats.stats_new_socket_entry(this); + _socket_stats.stats_update_proto(this, NSAPI_UDP); } UDPSocket::~UDPSocket() { - _socket_stats.stats_update_socket_state(this, SOCK_CLOSED); -} - -nsapi_error_t UDPSocket::close() -{ - _socket_stats.stats_update_socket_state(this, SOCK_CLOSED); - return InternetSocket::close(); } nsapi_protocol_t UDPSocket::get_proto() { - _socket_stats.stats_update_proto(this, NSAPI_UDP); - _socket_stats.stats_update_socket_state(this, SOCK_OPEN); return NSAPI_UDP; } diff --git a/features/netsocket/UDPSocket.h b/features/netsocket/UDPSocket.h index db06c3f00f..44025389d2 100644 --- a/features/netsocket/UDPSocket.h +++ b/features/netsocket/UDPSocket.h @@ -59,15 +59,6 @@ public: */ virtual ~UDPSocket(); - /** Close the socket. - * - * Closes any open connection and deallocates any memory associated - * with the socket. Called from destructor if socket is not closed. - * - * @return NSAPI_ERROR_OK on success, negative error code on failure - */ - virtual nsapi_error_t close(); - /** Send data to the specified host and port. * * By default, sendto blocks until data is sent. If socket is set to