net: lib: coap_client: check poll() condition before retrying CoAP msg (4c6dd4c7) · Commits · Wenxi XU / zephyr

include/zephyr/net/coap_client.h

+0 −2

Original line number	Diff line number	Diff line
		@@ -108,14 +108,12 @@ struct coap_client {
		int fd;
		struct sockaddr address;
		socklen_t socklen;
		bool response_ready;
		struct k_mutex lock;
		uint8_t send_buf[MAX_COAP_MSG_LEN];
		uint8_t recv_buf[MAX_COAP_MSG_LEN];
		struct coap_client_internal_request requests[CONFIG_COAP_CLIENT_MAX_REQUESTS];
		struct coap_option echo_option;
		bool send_echo;
		int socket_error;
		};
		/** @endcond */

subsys/net/lib/coap/coap_client.c

+83 −121

Original line number	Diff line number	Diff line
		@@ -26,6 +26,13 @@ static int num_clients;
		static K_SEM_DEFINE(coap_client_recv_sem, 0, 1);
		static atomic_t coap_client_recv_active;

		static bool timeout_expired(struct coap_client_internal_request *internal_req);
		static void cancel_requests_with(struct coap_client *client, int error);
		static int recv_response(struct coap_client client, struct coap_packet response, bool *truncated);
		static int handle_response(struct coap_client client, const struct coap_packet response,
		bool response_truncated);


		static int send_request(int sock, const void *buf, size_t len, int flags,
		const struct sockaddr *dest_addr, socklen_t addrlen)
		{
		@@ -127,27 +134,26 @@ static bool has_ongoing_exchange(struct coap_client *client)
		return false;
		}

		static struct coap_client_internal_request get_free_request(struct coap_client client)
		static bool has_timeout_expired(struct coap_client *client)
		{
		for (int i = 0; i < CONFIG_COAP_CLIENT_MAX_REQUESTS; i++) {
		if (client->requests[i].request_ongoing == false &&
		exchange_lifetime_exceeded(&client->requests[i])) {
		return &client->requests[i];
		if (timeout_expired(&client->requests[i])) {
		return true;
		}
		}

		return NULL;
		return false;
		}

		static bool has_ongoing_requests(void)
		static struct coap_client_internal_request get_free_request(struct coap_client client)
		{
		for (int i = 0; i < num_clients; i++) {
		if (has_ongoing_request(clients[i])) {
		return true;
		for (int i = 0; i < CONFIG_COAP_CLIENT_MAX_REQUESTS; i++) {
		if (client->requests[i].request_ongoing == false &&
		exchange_lifetime_exceeded(&client->requests[i])) {
		return &client->requests[i];
		}
		}

		return false;
		return NULL;
		}

		static bool has_ongoing_exchanges(void)
		@@ -498,37 +504,37 @@ static int resend_request(struct coap_client *client,
		return ret;
		}

		static int coap_client_resend_handler(void)
		static void coap_client_resend_handler(struct coap_client *client)
		{
		int ret = 0;

		for (int i = 0; i < num_clients; i++) {
		k_mutex_lock(&clients[i]->lock, K_FOREVER);
		k_mutex_lock(&client->lock, K_FOREVER);

		for (int j = 0; j < CONFIG_COAP_CLIENT_MAX_REQUESTS; j++) {
		if (timeout_expired(&clients[i]->requests[j])) {
		ret = resend_request(clients[i], &clients[i]->requests[j]);
		for (int i = 0; i < CONFIG_COAP_CLIENT_MAX_REQUESTS; i++) {
		if (timeout_expired(&client->requests[i])) {
		ret = resend_request(client, &client->requests[i]);
		if (ret < 0) {
		report_callback_error(&client->requests[i], ret);
		reset_internal_request(&client->requests[i]);
		}
		}

		k_mutex_unlock(&clients[i]->lock);
		}

		return ret;
		k_mutex_unlock(&client->lock);
		}

		static int handle_poll(void)
		{
		int ret = 0;

		while (1) {
		struct zsock_pollfd fds[CONFIG_COAP_CLIENT_MAX_INSTANCES] = {0};
		int nfds = 0;

		/* Use periodic timeouts */
		for (int i = 0; i < num_clients; i++) {
		fds[i].fd = clients[i]->fd;
		fds[i].events = ZSOCK_POLLIN;
		fds[i].events = (has_ongoing_exchange(clients[i]) ? ZSOCK_POLLIN : 0) \|
		(has_timeout_expired(clients[i]) ? ZSOCK_POLLOUT : 0);
		fds[i].revents = 0;
		nfds++;
		}
		@@ -536,49 +542,54 @@ static int handle_poll(void)
		ret = zsock_poll(fds, nfds, COAP_PERIODIC_TIMEOUT);

		if (ret < 0) {
		LOG_ERR("Error in poll:%d", errno);
		errno = 0;
		ret = -errno;
		LOG_ERR("Error in poll:%d", ret);
		return ret;
		} else if (ret == 0) {
		/* Resend all the expired pending messages */
		ret = coap_client_resend_handler();
		return 0;
		}

		if (ret < 0) {
		LOG_ERR("Error resending request: %d", ret);
		for (int i = 0; i < nfds; i++) {
		if (fds[i].revents & ZSOCK_POLLOUT) {
		coap_client_resend_handler(clients[i]);
		}
		if (fds[i].revents & ZSOCK_POLLIN) {
		struct coap_packet response;
		bool response_truncated = false;

		if (!has_ongoing_requests()) {
		return ret;
		k_mutex_lock(&clients[i]->lock, K_FOREVER);

		ret = recv_response(clients[i], &response, &response_truncated);
		if (ret < 0) {
		LOG_ERR("Error receiving response");
		cancel_requests_with(clients[i], -EIO);
		k_mutex_unlock(&clients[i]->lock);
		continue;
		}

		} else {
		for (int i = 0; i < nfds; i++) {
		ret = handle_response(clients[i], &response, response_truncated);
		if (ret < 0) {
		LOG_ERR("Error handling response");
		}

		k_mutex_unlock(&clients[i]->lock);
		}
		if (fds[i].revents & ZSOCK_POLLERR) {
		LOG_ERR("Error in poll for socket %d", fds[i].fd);
		clients[i]->socket_error = -EIO;
		cancel_requests_with(clients[i], -EIO);
		}
		if (fds[i].revents & ZSOCK_POLLHUP) {
		LOG_ERR("Error in poll: POLLHUP for socket %d", fds[i].fd);
		clients[i]->socket_error = -ENOTCONN;
		cancel_requests_with(clients[i], -EIO);
		}
		if (fds[i].revents & ZSOCK_POLLNVAL) {
		LOG_ERR("Error in poll: POLLNVAL - fd %d not open",
		fds[i].fd);
		clients[i]->socket_error = -EINVAL;
		LOG_ERR("Error in poll: POLLNVAL - fd %d not open", fds[i].fd);
		cancel_requests_with(clients[i], -EIO);
		}
		if (fds[i].revents & ZSOCK_POLLIN) {
		clients[i]->response_ready = true;
		}

		}

		return 0;
		}
		}

		return ret;
		}

		static bool token_compare(struct coap_client_internal_request *internal_req,
		const struct coap_packet *resp)
		@@ -895,14 +906,13 @@ static int handle_response(struct coap_client *client, const struct coap_packet
		}
		}
		fail:
		client->response_ready = false;
		if (ret < 0 \|\| !internal_req->is_observe) {
		internal_req->request_ongoing = false;
		}
		return ret;
		}

		void coap_client_cancel_requests(struct coap_client *client)
		static void cancel_requests_with(struct coap_client *client, int error)
		{
		k_mutex_lock(&client->lock, K_FOREVER);

		@@ -914,33 +924,20 @@ void coap_client_cancel_requests(struct coap_client *client)
		* do not reenter it. In that case, the user knows their
		* request was cancelled anyway.
		*/
		report_callback_error(&client->requests[i], -ECANCELED);
		client->requests[i].request_ongoing = false;
		client->requests[i].is_observe = false;
		report_callback_error(&client->requests[i], error);
		reset_internal_request(&client->requests[i]);
		}
		}
		atomic_clear(&coap_client_recv_active);
		k_mutex_unlock(&client->lock);

		/* Wait until after zsock_poll() can time out and return. */
		k_sleep(K_MSEC(COAP_PERIODIC_TIMEOUT));
		}

		static void signal_socket_error(struct coap_client *cli)
		void coap_client_cancel_requests(struct coap_client *client)
		{
		for (int i = 0; i < CONFIG_COAP_CLIENT_MAX_REQUESTS; i++) {
		struct coap_client_internal_request *req = &cli->requests[i];

		if (!req->request_ongoing) {
		continue;
		}

		req->request_ongoing = false;
		if (req->coap_request.cb) {
		req->coap_request.cb(cli->socket_error, 0, NULL, 0,
		true, req->coap_request.user_data);
		}
		}
		cancel_requests_with(client, -ECANCELED);
		/* Wait until after zsock_poll() can time out and return. */
		k_sleep(K_MSEC(COAP_PERIODIC_TIMEOUT));
		}

		void coap_client_recv(void coap_cl, void a, void *b)
		@@ -957,41 +954,6 @@ void coap_client_recv(void coap_cl, void a, void *b)
		goto idle;
		}

		for (int i = 0; i < num_clients; i++) {
		if (clients[i]->response_ready) {
		struct coap_packet response;
		bool response_truncated = false;

		k_mutex_lock(&clients[i]->lock, K_FOREVER);

		ret = recv_response(clients[i], &response, &response_truncated);
		if (ret < 0) {
		LOG_ERR("Error receiving response");
		clients[i]->response_ready = false;
		k_mutex_unlock(&clients[i]->lock);
		if (ret == -EOPNOTSUPP) {
		LOG_ERR("Socket misconfigured.");
		goto idle;
		}
		continue;
		}

		ret = handle_response(clients[i], &response, response_truncated);
		if (ret < 0) {
		LOG_ERR("Error handling response");
		}

		clients[i]->response_ready = false;
		k_mutex_unlock(&clients[i]->lock);
		}

		if (clients[i]->socket_error) {
		signal_socket_error(clients[i]);
		clients[i]->socket_error = 0;
		}

		}

		/* There are more messages coming */
		if (has_ongoing_exchanges()) {
		continue;

tests/net/lib/coap_client/CMakeLists.txt

+1 −0

Original line number	Diff line number	Diff line
		@@ -31,3 +31,4 @@ add_compile_definitions(CONFIG_COAP_CLIENT_MAX_REQUESTS=2)
		add_compile_definitions(CONFIG_COAP_CLIENT_MAX_INSTANCES=2)
		add_compile_definitions(CONFIG_COAP_MAX_RETRANSMIT=4)
		add_compile_definitions(CONFIG_COAP_BACKOFF_PERCENT=200)
		add_compile_definitions(CONFIG_COAP_LOG_LEVEL=4)

tests/net/lib/coap_client/src/main.c

+80 −14

Original line number	Diff line number	Diff line
		@@ -77,7 +77,7 @@ static ssize_t z_impl_zsock_recvfrom_custom_fake(int sock, void *buf, size_t max

		memcpy(buf, ack_data, sizeof(ack_data));

		clear_socket_events();
		clear_socket_events(ZSOCK_POLLIN);

		return sizeof(ack_data);
		}
		@@ -205,7 +205,7 @@ static ssize_t z_impl_zsock_recvfrom_custom_fake_response(int sock, void *buf, s

		memcpy(buf, ack_data, sizeof(ack_data));

		clear_socket_events();
		clear_socket_events(ZSOCK_POLLIN);

		return sizeof(ack_data);
		}
		@@ -247,7 +247,7 @@ static ssize_t z_impl_zsock_recvfrom_custom_fake_unmatching(int sock, void *buf,

		memcpy(buf, ack_data, sizeof(ack_data));

		clear_socket_events();
		clear_socket_events(ZSOCK_POLLIN);

		return sizeof(ack_data);
		}
		@@ -273,7 +273,7 @@ static ssize_t z_impl_zsock_recvfrom_custom_fake_echo(int sock, void *buf, size_
		z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake_response;
		z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_echo;

		clear_socket_events();
		clear_socket_events(ZSOCK_POLLIN);

		return sizeof(ack_data);
		}
		@@ -299,7 +299,7 @@ static ssize_t z_impl_zsock_recvfrom_custom_fake_echo_next_req(int sock, void *b
		z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake_response;
		z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_echo_next_req;

		clear_socket_events();
		clear_socket_events(ZSOCK_POLLIN);

		return sizeof(ack_data);
		}
		@@ -335,6 +335,9 @@ void coap_callback(int16_t code, size_t offset, const uint8_t *payload, size_t l
		{
		LOG_INF("CoAP response callback, %d", code);
		last_response_code = code;
		if (user_data) {
		k_sem_give((struct k_sem *) user_data);
		}
		}

		ZTEST_SUITE(coap_client, NULL, suite_setup, test_setup, NULL, NULL);
		@@ -392,7 +395,7 @@ ZTEST(coap_client, test_resend_request)
		ret = coap_client_req(&client, 0, &address, &client_request, NULL);
		zassert_true(ret >= 0, "Sending request failed, %d", ret);
		k_sleep(K_MSEC(MORE_THAN_ACK_TIMEOUT_MS));
		set_socket_events(ZSOCK_POLLIN);
		set_socket_events(ZSOCK_POLLIN \| ZSOCK_POLLOUT);

		k_sleep(K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS));
		zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response");
		@@ -547,6 +550,7 @@ ZTEST(coap_client, test_no_response)
		client_request.len = strlen(short_payload);

		z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_no_reply;
		set_socket_events(ZSOCK_POLLOUT);

		k_sleep(K_MSEC(1));

		@@ -592,30 +596,35 @@ ZTEST(coap_client, test_multiple_requests)
		{
		int ret = 0;
		int retry = MORE_THAN_EXCHANGE_LIFETIME_MS;
		struct k_sem sem1, sem2;

		struct sockaddr address = {0};
		struct coap_client_request client_request = {
		struct coap_client_request req1 = {
		.method = COAP_METHOD_GET,
		.confirmable = true,
		.path = test_path,
		.fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN,
		.cb = coap_callback,
		.payload = NULL,
		.len = 0
		.payload = short_payload,
		.len = strlen(short_payload),
		.user_data = &sem1
		};
		struct coap_client_request req2 = req1;

		client_request.payload = short_payload;
		client_request.len = strlen(short_payload);
		req2.user_data = &sem2;

		k_sem_init(&sem1, 0, 1);
		k_sem_init(&sem2, 0, 1);

		z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_no_reply;

		k_sleep(K_MSEC(1));

		LOG_INF("Send request");
		ret = coap_client_req(&client, 0, &address, &client_request, NULL);
		ret = coap_client_req(&client, 0, &address, &req1, NULL);
		zassert_true(ret >= 0, "Sending request failed, %d", ret);

		ret = coap_client_req(&client, 0, &address, &client_request, NULL);
		ret = coap_client_req(&client, 0, &address, &req2, NULL);
		zassert_true(ret >= 0, "Sending request failed, %d", ret);

		set_socket_events(ZSOCK_POLLIN);
		@@ -625,8 +634,10 @@ ZTEST(coap_client, test_multiple_requests)
		}
		zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response");

		last_response_code = 0;
		set_socket_events(ZSOCK_POLLIN);
		k_sleep(K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS));
		zassert_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS)));
		zassert_ok(k_sem_take(&sem2, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS)));
		zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response");
		}

		@@ -653,6 +664,7 @@ ZTEST(coap_client, test_unmatching_tokens)
		client_request.len = strlen(short_payload);

		z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake_unmatching;
		set_socket_events(ZSOCK_POLLIN \| ZSOCK_POLLOUT);

		k_sleep(K_MSEC(1));

		@@ -663,3 +675,57 @@ ZTEST(coap_client, test_unmatching_tokens)
		k_sleep(K_MSEC(MORE_THAN_LONG_EXCHANGE_LIFETIME_MS));
		zassert_equal(last_response_code, -ETIMEDOUT, "Unexpected response");
		}

		ZTEST(coap_client, test_multiple_clients)
		{
		int ret;
		int retry = MORE_THAN_EXCHANGE_LIFETIME_MS;
		static struct coap_client client2 = {
		.fd = 2,
		};
		struct k_sem sem1, sem2;
		struct sockaddr address = {0};
		struct coap_client_request req1 = {
		.method = COAP_METHOD_GET,
		.confirmable = true,
		.path = test_path,
		.fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN,
		.cb = coap_callback,
		.payload = short_payload,
		.len = strlen(short_payload),
		.user_data = &sem1
		};
		struct coap_client_request req2 = req1;

		req2.user_data = &sem2;
		req2.payload = long_payload;
		req2.len = strlen(long_payload);

		zassert_ok(k_sem_init(&sem1, 0, 1));
		zassert_ok(k_sem_init(&sem2, 0, 1));

		zassert_ok(coap_client_init(&client2, NULL));

		k_sleep(K_MSEC(1));

		LOG_INF("Sending requests");
		ret = coap_client_req(&client, 1, &address, &req1, NULL);
		zassert_true(ret >= 0, "Sending request failed, %d", ret);

		ret = coap_client_req(&client2, 2, &address, &req2, NULL);
		zassert_true(ret >= 0, "Sending request failed, %d", ret);

		while (last_response_code == 0 && retry > 0) {
		retry--;
		k_sleep(K_MSEC(1));
		}
		set_socket_events(ZSOCK_POLLIN);

		k_sleep(K_SECONDS(1));

		/* ensure we got both responses */
		zassert_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS)));
		zassert_ok(k_sem_take(&sem2, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS)));
		zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response");

		}

tests/net/lib/coap_client/src/stubs.c

+9 −8

Original line number	Diff line number	Diff line
		@@ -29,9 +29,9 @@ void set_socket_events(short events)
		my_events \|= events;
		}

		void clear_socket_events(void)
		void clear_socket_events(short events)
		{
		my_events = 0;
		my_events &= ~events;
		}

		int z_impl_zsock_socket(int family, int type, int proto)
		@@ -41,13 +41,14 @@ int z_impl_zsock_socket(int family, int type, int proto)

		int z_impl_zvfs_poll(struct zvfs_pollfd *fds, int nfds, int poll_timeout)
		{
		LOG_INF("Polling, events %d", my_events);
		int events = 0;
		k_sleep(K_MSEC(1));
		fds->revents = my_events;

		if (my_events) {
		return 1;
		for (int i = 0; i < nfds; i++) {
		fds[i].revents = my_events & (fds[i].events \| ZSOCK_POLLERR \| ZSOCK_POLLHUP);
		if (fds[i].revents) {
		events++;
		}
		}

		return 0;
		return events;
		}

Admin message