diff --git a/packages/atclient/include/atclient/connection.h b/packages/atclient/include/atclient/connection.h index a9a2ffb2..118245b2 100644 --- a/packages/atclient/include/atclient/connection.h +++ b/packages/atclient/include/atclient/connection.h @@ -1,7 +1,7 @@ /* * * The connection family of types and methods represents a single connection to - * if you want a pure socket representation see net_socket.h. + * if you want a pure socket representation see socket.h. * * At the moment _socket represents a singular tcp socket, but in the future it may be altered * to be a union of different connection types, such as a websocket or other construct. diff --git a/packages/atclient/include/atclient/monitor.h b/packages/atclient/include/atclient/monitor.h index 41e01aef..db57877e 100644 --- a/packages/atclient/include/atclient/monitor.h +++ b/packages/atclient/include/atclient/monitor.h @@ -6,14 +6,10 @@ extern "C" { #include "atclient/atclient.h" #include "atclient/atnotification.h" +#include "atclient/socket.h" #include // IWYU pragma: keep #include -// HACK let's just get it working for now this is so wrong -#ifndef MBEDTLS_ERR_SSL_TIMEOUT -#define MBEDTLS_ERR_SSL_TIMEOUT -37 -#endif - /** * @brief Represents a message received from the monitor connection, typically derived from the prefix of the response * (e.g. "data:ok"'s message type would be "data" = ATCLIENT_MONITOR_MESSAGE_TYPE_DATA_RESPONSE) diff --git a/packages/atclient/include/atclient/socket.h b/packages/atclient/include/atclient/socket.h index bdb15e22..0a205f90 100644 --- a/packages/atclient/include/atclient/socket.h +++ b/packages/atclient/include/atclient/socket.h @@ -10,6 +10,22 @@ extern "C" { #endif +#ifndef ATCLIENT_SSL_TIMEOUT_EXITCODE + +#if defined(ATCLIENT_SOCKET_PROVIDER_MBEDTLS) +#define ATCLIENT_SSL_TIMEOUT_EXITCODE MBEDTLS_ERR_SSL_TIMEOUT + +#elif defined(ATCLIENT_SOCKET_PROVIDER_ARDUINO_BEARSSL) +// Most arduino libraries only use -1 or positive integers +#define ATCLIENT_SSL_TIMEOUT_EXITCODE -101 + +#else +#error "ATCLIENT_ERR_SSL_TIMEOUT is undefined" + +#endif + +#endif + // IWYU pragma: begin_exports // Export the appropriate platform specific struct implementation diff --git a/packages/atclient/include/atclient/socket_mbedtls.h b/packages/atclient/include/atclient/socket_mbedtls.h index e2668bf0..09ef806c 100644 --- a/packages/atclient/include/atclient/socket_mbedtls.h +++ b/packages/atclient/include/atclient/socket_mbedtls.h @@ -1,7 +1,7 @@ -// IWYU pragma: private, include "atclient/net_socket.h" -// IWYU pragma: friend "net_socket_mbedtls.*" -#ifndef ATCLIENT_NET_SOCKET_MBEDTLS_H -#define ATCLIENT_NET_SOCKET_MBEDTLS_H +// IWYU pragma: private, include "atclient/socket.h" +// IWYU pragma: friend "socket_mbedtls.*" +#ifndef ATCLIENT_SOCKET_MBEDTLS_H +#define ATCLIENT_SOCKET_MBEDTLS_H #include #if defined(ATCLIENT_SOCKET_PROVIDER_MBEDTLS) #include @@ -14,7 +14,7 @@ extern "C" { #endif -// Make this type more portable to consume later +// TODO: Make this type more portable to consume later struct atclient_raw_socket { mbedtls_net_context net; }; diff --git a/packages/atclient/include/atclient/socket_shared.h b/packages/atclient/include/atclient/socket_shared.h index 7e7729a0..8d7624ea 100644 --- a/packages/atclient/include/atclient/socket_shared.h +++ b/packages/atclient/include/atclient/socket_shared.h @@ -1,5 +1,5 @@ -// IWYU pragma: private, include "atclient/net_socket.h" -// IWYU pragma: friend "net_socket_mbedtls.*" +// IWYU pragma: private, include "atclient/socket.h" +// IWYU pragma: friend "socket_mbedtls.*" #ifndef ATCLIENT_SOCKET_SHARED_H #define ATCLIENT_SOCKET_SHARED_H #include diff --git a/packages/atclient/src/monitor.c b/packages/atclient/src/monitor.c index 8b34257a..d8282a63 100644 --- a/packages/atclient/src/monitor.c +++ b/packages/atclient/src/monitor.c @@ -113,18 +113,15 @@ int atclient_monitor_read(atclient *monitor_conn, atclient *atclient, atclient_m int ret = atclient_tls_socket_read(&monitor_conn->atserver_connection._socket, &buffer, &buffer_len, atclient_socket_read_until_char('@')); - // TODO: move this later for now it's fine as it should - if (ret == MBEDTLS_ERR_SSL_TIMEOUT) { + if (ret == ATCLIENT_SSL_TIMEOUT_EXITCODE) { // treat a timeout as empty message, non error message->type = ATCLIENT_MONITOR_MESSAGE_TYPE_EMPTY; ret = 0; goto exit; - } - - if (ret <= 0) { // you should reconnect... + } else if (ret != 0) { // you should reconnect... message->type = ATCLIENT_MONITOR_ERROR_READ; message->error_read.error_code = ret; - atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "Read nothing from the monitor connection: %d\n", ret); + atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "Error: monitor exited with code %d\n", ret); goto exit; } diff --git a/packages/atclient/src/socket.c b/packages/atclient/src/socket.c index a2f1710e..0b639485 100644 --- a/packages/atclient/src/socket.c +++ b/packages/atclient/src/socket.c @@ -1,7 +1,5 @@ #include #include -// Most of the implementation for net_socket is platform specific, -// See the other net_socket_*.c files struct atclient_socket_read_options atclient_socket_read_until_char(char until) { return (struct atclient_socket_read_options){ diff --git a/packages/atclient/src/socket_mbedtls.c b/packages/atclient/src/socket_mbedtls.c index 724ca0d4..b79aa223 100644 --- a/packages/atclient/src/socket_mbedtls.c +++ b/packages/atclient/src/socket_mbedtls.c @@ -1,6 +1,7 @@ // These two headers must be included in a specific order #include "atchops/platform.h" // IWYU pragma: keep // Don't move them +#include "atclient/monitor.h" #include "atclient/socket.h" #if defined(ATCLIENT_SOCKET_PROVIDER_MBEDTLS) @@ -273,96 +274,6 @@ int atclient_tls_socket_read(struct atclient_tls_socket *socket, unsigned char * return 4; } } -// int atclient_tls_socket_read_num_bytes(struct atclient_tls_socket *socket, unsigned char **value, size_t *value_len, -// size_t num_bytes) { -// // Assume params have been validated by socket_read -// int ret; -// unsigned char *recv = NULL; -// size_t blocks = 0; // number of allocated blocks -// -// do { -// size_t offset = READ_BLOCK_LEN * blocks; // offset to current block -// // Allocate memory -// unsigned char *temp = realloc(recv, sizeof(unsigned char) * (offset + READ_BLOCK_LEN)); -// if (temp == NULL) { -// atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to allocate receive buffer\n"); -// if (recv != NULL) { -// free(recv); -// } -// return 1; -// } -// recv = temp; // once we ensure realloc was successful we set recv to the new memory -// -// // Read into current block -// size_t pos = 0; // position in current block -// do { -// size_t remaining_for_block; -// if (READ_BLOCK_LEN + offset > num_bytes) { -// // We are in the final block, so only read the amount that will make -// // us reach num_bytes -// remaining_for_block = num_bytes - (offset + pos); -// } else { -// remaining_for_block = READ_BLOCK_LEN - pos; -// } -// -// ret = mbedtls_ssl_read(&socket->ssl, recv + offset + pos, remaining_for_block); -// if (ret > 0) { -// pos += ret; // successful read, increment pos -// -// if (offset + pos == num_bytes) { -// *value = recv; -// *value_len = (offset + pos); -// return 0; // The only return where recv should not be freed -// } -// -// continue; // not done reading yet -// } -// -// // handle non-happy path -// switch (ret) { -// case 0: // connection is closed -// case MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY: // connection is closed -// atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Socket closed while reading: %d\n", ret); -// ret = MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY; // ensure ret val is not 0 -// free(recv); -// return ret; -// case MBEDTLS_ERR_SSL_WANT_READ: // handshake incomplete -// case MBEDTLS_ERR_SSL_WANT_WRITE: // handshake incomplete -// case MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS: // async operation in progress -// case MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS: // crypto operation in progress -// // async error, we need to try again -// break; -// case MBEDTLS_ERR_SSL_TIMEOUT: // treat a timeout as -// -// if (value != NULL) { -// *value = recv; -// } else { -// free(recv); -// } -// if (value_len != NULL) { -// *value_len = (offset + pos); -// } -// return 0; // The only return where recv should not be freed -// // unexpected errors while reading -// default: -// if (ret > 0) { -// atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Unexpected read value %d\n", ret); -// } else { -// char strerr[512]; -// mbedtls_strerror(ret, strerr, 512); -// atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "%s", strerr); -// } -// free(recv); -// return ret; -// } // don't put anything after switch without checking it first -// } while (pos < READ_BLOCK_LEN); -// blocks++; -// } while (blocks < MAX_READ_BLOCKS); -// // We should only arrive at this point if we max out blocks -// // Every other code path should return early -// atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to read within the maximum allowed number of read -// blocks\n"); free(recv); return 1; -// } int atclient_tls_socket_read_until_char(struct atclient_tls_socket *socket, unsigned char **value, size_t *value_len, char until_char) { @@ -409,6 +320,7 @@ int atclient_tls_socket_read_until_char(struct atclient_tls_socket *socket, unsi continue; // continue if not found char yet } // handle non-happy path + atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "Socket read error: %d", ret); switch (ret) { case 0: // connection is closed case MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY: // connection is closed @@ -428,7 +340,7 @@ int atclient_tls_socket_read_until_char(struct atclient_tls_socket *socket, unsi if (timeout_count == MAX_READ_TIMEOUTS) { atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "Failed to read the full message after %d attempts\n", MAX_READ_TIMEOUTS); - return ret; + return ATCLIENT_SSL_TIMEOUT_EXITCODE; } usleep(1000); break; diff --git a/tests/functional_tests/tests/test_atclient_get_atkeys.c b/tests/functional_tests/tests/test_atclient_get_atkeys.c index c949843d..6f6c1213 100644 --- a/tests/functional_tests/tests/test_atclient_get_atkeys.c +++ b/tests/functional_tests/tests/test_atclient_get_atkeys.c @@ -146,7 +146,6 @@ exit: { static int test_4_atclient_get_atkeys_null_regex(atclient *ctx, const char *scan_regex, const bool showhidden) { int ret = 1; - return 0; // NOTE: disabled temporarily DONT LET THIS GET MERGED atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "test_4_atclient_get_atkeys_null_regex\n");