Commit ed025b2f authored by Benjamin Lindqvist's avatar Benjamin Lindqvist Committed by Anas Nashif
Browse files

net: lib: coap_client: observe-related fixes 



An earlier pull request implementing observe support was merged too
hastily. It had a few issues:

1. The predicate for whether a request should be marked not ongoing was
wrong (it checked ret != 0 instead of ret < 0)
2. Without observes in mind, MID-based deduplication is not a required
feature. Deduplication was handled implicitly - the exchange would get
dropped after the first response anyway, so duplicate responses would
not get matched to anything. But with observes, there are several
responses in an exchange. This commit adds this.
3. Using coap_request_is_observe(&internal_req->request) in the response
handler requires the whole request to stay in scope for the lifetime of
the observation, which I observed was not always the case. Adding an
is_observe bool to the internal struct improved stability significantly.

With these fixes, GETs with observe option works very well.

Signed-off-by: default avatarBenjamin Lindqvist <benjamin@eub.se>
parent 85755dd8

include/zephyr/net/coap_client.h

+6 −3
Original line number Diff line number Diff line
@@ -91,6 +91,10 @@ struct coap_client_internal_request { 
	struct coap_client_request coap_request;

	struct coap_packet request;

	uint8_t request_tag[COAP_TOKEN_MAX_LEN];



	/* For GETs with observe option set */

	bool is_observe;

	int last_response_id;

};



struct coap_client {
@@ -141,9 +145,8 @@ int coap_client_req(struct coap_client *client, int sock, const struct sockaddr 
/**

 * @brief Cancel all current requests.

 *

 * This is intended for canceling long-running requests (e.g. GETs with the OBSERVE option set,

 * or a block transfer) which have gone stale for some reason. It is also intended for responding

 * to network connectivity issues.

 * This is intended for canceling long-running requests (e.g. GETs with the OBSERVE option set)

 * which has gone stale for some reason.

 *

 * @param client Client instance.

 */

subsys/net/lib/coap/coap_client.c

+13 −1
Original line number Diff line number Diff line
@@ -66,6 +66,7 @@ static void reset_internal_request(struct coap_client_internal_request *request) 
{

	request->offset = 0;

	request->last_id = 0;

	request->last_response_id = -1;

	reset_block_contexts(request);

}
@@ -369,6 +370,7 @@ int coap_client_req(struct coap_client *client, int sock, const struct sockaddr 
		}



		coap_pending_cycle(&internal_req->pending);

		internal_req->is_observe = coap_request_is_observe(&internal_req->request);

	}



	ret = send_request(sock, internal_req->request.data, internal_req->request.offset, 0,
@@ -659,6 +661,7 @@ static int handle_response(struct coap_client *client, const struct coap_packet 
	/* CON, NON_CON and piggybacked ACK need to match the token with original request */

	uint16_t payload_len;

	uint8_t response_code = coap_header_get_code(response);

	uint16_t response_id = coap_header_get_id(response);

	const uint8_t *payload = coap_packet_get_payload(response, &payload_len);



	/* Separate response coming */
@@ -675,6 +678,14 @@ static int handle_response(struct coap_client *client, const struct coap_packet 
		return 1;

	}



	/* MID-based deduplication */

	if (response_id == internal_req->last_response_id) {

		LOG_WRN("Duplicate MID, dropping");

		goto fail;

	}



	internal_req->last_response_id = response_id;



	/* Received echo option */

	if (find_echo_option(response, &client->echo_option)) {

		 /* Resend request with echo option */
@@ -833,7 +844,7 @@ static int handle_response(struct coap_client *client, const struct coap_packet 
	}

fail:

	client->response_ready = false;

	if (ret != 0 || !coap_request_is_observe(&internal_req->request)) {

	if (ret < 0 || !internal_req->is_observe) {

		internal_req->request_ongoing = false;

	}

	return ret;
@@ -851,6 +862,7 @@ void coap_client_cancel_requests(struct coap_client *client) 
			 */

			report_callback_error(&client->requests[i], -ECANCELED);

			client->requests[i].request_ongoing = false;

			client->requests[i].is_observe = false;

		}

	}

	atomic_clear(&coap_client_recv_active);

CRA Git | Maintained and supported by SUSTech CRA and CCSE