[PATCH BlueZ v2] mesh: Add deferral of Attach() and Leave() if busy

[PATCH BlueZ v2] mesh: Add deferral of Attach() and Leave() if busy

[Brian Gix]

We require the successful return of JoinComplete() method before
handling subsequent Attach() or Leave() method calls. To simplify the
construction of Applications, we will accept one of these calls up to 1
second prior to receiving the final return status of JoinComplete,
which tells us that the Application is ready to use the node.

If the node is still not ready after the deferral, Attach and/or Leave
will fail.
---
 mesh/mesh.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 2 deletions(-)

diff --git a/mesh/mesh.c b/mesh/mesh.c
index ab2393deb..bc170371d 100644
--- a/mesh/mesh.c
+++ b/mesh/mesh.c
@@ -104,6 +104,10 @@ static struct l_queue *pending_queue;
 
 static const char *storage_dir;
 
+/* Forward static decalrations */
+static void def_attach(struct l_timeout *timeout, void *user_data);
+static void def_leave(struct l_timeout *timeout, void *user_data);
+
 static bool simple_match(const void *a, const void *b)
 {
 	return a == b;
@@ -634,12 +638,26 @@ static struct l_dbus_message *attach_call(struct l_dbus *dbus,
 	uint64_t token;
 	const char *app_path, *sender;
 	struct l_dbus_message *pending_msg;
+	struct mesh_node *node;
 
 	l_debug("Attach");
 
 	if (!l_dbus_message_get_arguments(msg, "ot", &app_path, &token))
 		return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL);
 
+	node = node_find_by_token(token);
+	if (!node)
+		return dbus_error(msg, MESH_ERROR_NOT_FOUND, "Attach failed");
+
+	if (node_is_busy(node)) {
+		if (user_data)
+			return dbus_error(msg, MESH_ERROR_BUSY, NULL);
+
+		/* Try once more in 1 second */
+		l_timeout_create(1, def_attach, l_dbus_message_ref(msg), NULL);
+		return NULL;
+	}
+
 	sender = l_dbus_message_get_sender(msg);
 
 	pending_msg = l_dbus_message_ref(msg);
@@ -650,6 +668,19 @@ static struct l_dbus_message *attach_call(struct l_dbus *dbus,
 	return NULL;
 }
 
+static void def_attach(struct l_timeout *timeout, void *user_data)
+{
+	struct l_dbus *dbus = dbus_get_bus();
+	struct l_dbus_message *msg = user_data;
+	struct l_dbus_message *reply;
+
+	l_timeout_remove(timeout);
+
+	reply = attach_call(dbus, msg, (void *) true);
+	l_dbus_send(dbus, reply);
+	l_dbus_message_unref(msg);
+}
+
 static struct l_dbus_message *leave_call(struct l_dbus *dbus,
 						struct l_dbus_message *msg,
 						void *user_data)
@@ -666,14 +697,33 @@ static struct l_dbus_message *leave_call(struct l_dbus *dbus,
 	if (!node)
 		return dbus_error(msg, MESH_ERROR_NOT_FOUND, NULL);
 
-	if (node_is_busy(node))
-		return dbus_error(msg, MESH_ERROR_BUSY, NULL);
+	if (node_is_busy(node)) {
+		if (user_data)
+			return dbus_error(msg, MESH_ERROR_BUSY, NULL);
+
+		/* Try once more in 1 second */
+		l_timeout_create(1, def_leave, l_dbus_message_ref(msg), NULL);
+		return NULL;
+	}
 
 	node_remove(node);
 
 	return l_dbus_message_new_method_return(msg);
 }
 
+static void def_leave(struct l_timeout *timeout, void *user_data)
+{
+	struct l_dbus *dbus = dbus_get_bus();
+	struct l_dbus_message *msg = user_data;
+	struct l_dbus_message *reply;
+
+	l_timeout_remove(timeout);
+
+	reply = leave_call(dbus, msg, (void *) true);
+	l_dbus_send(dbus, reply);
+	l_dbus_message_unref(msg);
+}
+
 static void create_join_complete_reply_cb(struct l_dbus_message *msg,
 								void *user_data)
 {
-- 
2.25.4

Re: [PATCH BlueZ v2] mesh: Add deferral of Attach() and Leave() if busy

[Michał Lowas-Rzechonek]

On 06/16, Brian Gix wrote:
> We require the successful return of JoinComplete() method before
> handling subsequent Attach() or Leave() method calls. To simplify the
> construction of Applications, we will accept one of these calls up to 1
> second prior to receiving the final return status of JoinComplete,
> which tells us that the Application is ready to use the node.
> 
> If the node is still not ready after the deferral, Attach and/or Leave
> will fail.

Nice, I think this solution is simpler than the patch I came up with.

> ---
>  mesh/mesh.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 52 insertions(+), 2 deletions(-)
> 
> diff --git a/mesh/mesh.c b/mesh/mesh.c
> index ab2393deb..bc170371d 100644
> --- a/mesh/mesh.c
> +++ b/mesh/mesh.c
> @@ -104,6 +104,10 @@ static struct l_queue *pending_queue;
>  
>  static const char *storage_dir;
>  
> +/* Forward static decalrations */
> +static void def_attach(struct l_timeout *timeout, void *user_data);
> +static void def_leave(struct l_timeout *timeout, void *user_data);
> +
>  static bool simple_match(const void *a, const void *b)
>  {
>  	return a == b;
> @@ -634,12 +638,26 @@ static struct l_dbus_message *attach_call(struct l_dbus *dbus,
>  	uint64_t token;
>  	const char *app_path, *sender;
>  	struct l_dbus_message *pending_msg;
> +	struct mesh_node *node;
>  
>  	l_debug("Attach");
>  
>  	if (!l_dbus_message_get_arguments(msg, "ot", &app_path, &token))
>  		return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL);
>  
> +	node = node_find_by_token(token);
> +	if (!node)
> +		return dbus_error(msg, MESH_ERROR_NOT_FOUND, "Attach failed");
> +
> +	if (node_is_busy(node)) {
> +		if (user_data)
> +			return dbus_error(msg, MESH_ERROR_BUSY, NULL);
> +
> +		/* Try once more in 1 second */
> +		l_timeout_create(1, def_attach, l_dbus_message_ref(msg), NULL);
> +		return NULL;
> +	}
> +
>  	sender = l_dbus_message_get_sender(msg);
>  
>  	pending_msg = l_dbus_message_ref(msg);
> @@ -650,6 +668,19 @@ static struct l_dbus_message *attach_call(struct l_dbus *dbus,
>  	return NULL;
>  }
>  
> +static void def_attach(struct l_timeout *timeout, void *user_data)
> +{
> +	struct l_dbus *dbus = dbus_get_bus();
> +	struct l_dbus_message *msg = user_data;
> +	struct l_dbus_message *reply;
> +
> +	l_timeout_remove(timeout);
> +
> +	reply = attach_call(dbus, msg, (void *) true);

Why not pass NULL as the 3rd argument? We register the interface with
empty user data, so this would be an equivalent.

> +	l_dbus_send(dbus, reply);
> +	l_dbus_message_unref(msg);
> +}
> +
>  static struct l_dbus_message *leave_call(struct l_dbus *dbus,
>  						struct l_dbus_message *msg,
>  						void *user_data)
> @@ -666,14 +697,33 @@ static struct l_dbus_message *leave_call(struct l_dbus *dbus,
>  	if (!node)
>  		return dbus_error(msg, MESH_ERROR_NOT_FOUND, NULL);
>  
> -	if (node_is_busy(node))
> -		return dbus_error(msg, MESH_ERROR_BUSY, NULL);
> +	if (node_is_busy(node)) {
> +		if (user_data)
> +			return dbus_error(msg, MESH_ERROR_BUSY, NULL);
> +
> +		/* Try once more in 1 second */
> +		l_timeout_create(1, def_leave, l_dbus_message_ref(msg), NULL);
> +		return NULL;
> +	}
>  
>  	node_remove(node);
>  
>  	return l_dbus_message_new_method_return(msg);
>  }
>  
> +static void def_leave(struct l_timeout *timeout, void *user_data)
> +{
> +	struct l_dbus *dbus = dbus_get_bus();
> +	struct l_dbus_message *msg = user_data;
> +	struct l_dbus_message *reply;
> +
> +	l_timeout_remove(timeout);
> +
> +	reply = leave_call(dbus, msg, (void *) true);

Same here.

> +	l_dbus_send(dbus, reply);
> +	l_dbus_message_unref(msg);
> +}
> +
>  static void create_join_complete_reply_cb(struct l_dbus_message *msg,
>  								void *user_data)
>  {
> -- 
> 2.25.4
> 

-- 
Michał Lowas-Rzechonek <michal.lowas-rzechonek@silvair.com>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND

Re: [PATCH BlueZ v2] mesh: Add deferral of Attach() and Leave() if busy

[Gix, Brian]

Applied
On Tue, 2020-06-16 at 11:14 -0700, Brian Gix wrote:
> We require the successful return of JoinComplete() method before
> handling subsequent Attach() or Leave() method calls. To simplify the
> construction of Applications, we will accept one of these calls up to 1
> second prior to receiving the final return status of JoinComplete,
> which tells us that the Application is ready to use the node.
> 
> If the node is still not ready after the deferral, Attach and/or Leave
> will fail.
> ---
>  mesh/mesh.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 52 insertions(+), 2 deletions(-)
> 
> diff --git a/mesh/mesh.c b/mesh/mesh.c
> index ab2393deb..bc170371d 100644
> --- a/mesh/mesh.c
> +++ b/mesh/mesh.c
> @@ -104,6 +104,10 @@ static struct l_queue *pending_queue;
>  
>  static const char *storage_dir;
>  
> +/* Forward static decalrations */
> +static void def_attach(struct l_timeout *timeout, void *user_data);
> +static void def_leave(struct l_timeout *timeout, void *user_data);
> +
>  static bool simple_match(const void *a, const void *b)
>  {
>  	return a == b;
> @@ -634,12 +638,26 @@ static struct l_dbus_message *attach_call(struct l_dbus *dbus,
>  	uint64_t token;
>  	const char *app_path, *sender;
>  	struct l_dbus_message *pending_msg;
> +	struct mesh_node *node;
>  
>  	l_debug("Attach");
>  
>  	if (!l_dbus_message_get_arguments(msg, "ot", &app_path, &token))
>  		return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL);
>  
> +	node = node_find_by_token(token);
> +	if (!node)
> +		return dbus_error(msg, MESH_ERROR_NOT_FOUND, "Attach failed");
> +
> +	if (node_is_busy(node)) {
> +		if (user_data)
> +			return dbus_error(msg, MESH_ERROR_BUSY, NULL);
> +
> +		/* Try once more in 1 second */
> +		l_timeout_create(1, def_attach, l_dbus_message_ref(msg), NULL);
> +		return NULL;
> +	}
> +
>  	sender = l_dbus_message_get_sender(msg);
>  
>  	pending_msg = l_dbus_message_ref(msg);
> @@ -650,6 +668,19 @@ static struct l_dbus_message *attach_call(struct l_dbus *dbus,
>  	return NULL;
>  }
>  
> +static void def_attach(struct l_timeout *timeout, void *user_data)
> +{
> +	struct l_dbus *dbus = dbus_get_bus();
> +	struct l_dbus_message *msg = user_data;
> +	struct l_dbus_message *reply;
> +
> +	l_timeout_remove(timeout);
> +
> +	reply = attach_call(dbus, msg, (void *) true);
> +	l_dbus_send(dbus, reply);
> +	l_dbus_message_unref(msg);
> +}
> +
>  static struct l_dbus_message *leave_call(struct l_dbus *dbus,
>  						struct l_dbus_message *msg,
>  						void *user_data)
> @@ -666,14 +697,33 @@ static struct l_dbus_message *leave_call(struct l_dbus *dbus,
>  	if (!node)
>  		return dbus_error(msg, MESH_ERROR_NOT_FOUND, NULL);
>  
> -	if (node_is_busy(node))
> -		return dbus_error(msg, MESH_ERROR_BUSY, NULL);
> +	if (node_is_busy(node)) {
> +		if (user_data)
> +			return dbus_error(msg, MESH_ERROR_BUSY, NULL);
> +
> +		/* Try once more in 1 second */
> +		l_timeout_create(1, def_leave, l_dbus_message_ref(msg), NULL);
> +		return NULL;
> +	}
>  
>  	node_remove(node);
>  
>  	return l_dbus_message_new_method_return(msg);
>  }
>  
> +static void def_leave(struct l_timeout *timeout, void *user_data)
> +{
> +	struct l_dbus *dbus = dbus_get_bus();
> +	struct l_dbus_message *msg = user_data;
> +	struct l_dbus_message *reply;
> +
> +	l_timeout_remove(timeout);
> +
> +	reply = leave_call(dbus, msg, (void *) true);
> +	l_dbus_send(dbus, reply);
> +	l_dbus_message_unref(msg);
> +}
> +
>  static void create_join_complete_reply_cb(struct l_dbus_message *msg,
>  								void *user_data)
>  {

Re: [PATCH BlueZ v2] mesh: Add deferral of Attach() and Leave() if busy

[michal.lowas-rzechonek]

On 06/18, Gix, Brian wrote:
> Applied

I have a vague impression that my comments about (void*) true have been
ignored ;)

-- 
Michał Lowas-Rzechonek <michal.lowas-rzechonek@silvair.com>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND

Re: [PATCH BlueZ v2] mesh: Add deferral of Attach() and Leave() if busy

[Gix, Brian]

Hi Michał,

On Thu, 2020-06-18 at 17:43 +0200, michal.lowas-rzechonek@silvair.com wrote:
> On 06/18, Gix, Brian wrote:
> > Applied
> 
> I have a vague impression that my comments about (void*) true have been
> ignored ;)

Sorry, I thought I had responded to this...

I am using the user_data for the Attach and Leave call (which the dbus system always passes as NULL) as the
indication that the timed deferral call is just that...   Otherwise if we *never* get the JoinComplete
response, we will forever loop repeatedly re-deferring the Attach/Leave.

If we have already deferred the Attach/Leave, I still want to respond "Busy" to the App...  because the App is
doing something wrong....   It owes us a response. 

Re: [PATCH BlueZ v2] mesh: Add deferral of Attach() and Leave() if busy

[michal.lowas-rzechonek]

Brian,

On 06/18, Gix, Brian wrote:
> I am using the user_data for the Attach and Leave call (which the dbus
> system always passes as NULL) as the indication that the timed
> deferral call is just that...   Otherwise if we *never* get the
> JoinComplete response, we will forever loop repeatedly re-deferring
> the Attach/Leave.

Ah, right. I missed that part. Thanks.

-- 
Michał Lowas-Rzechonek <michal.lowas-rzechonek@silvair.com>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND