[PATCH BlueZ 0/2] Include destination address in MessageReceived API

[PATCH BlueZ 0/2] Include destination address in MessageReceived API

[Michał Lowas-Rzechonek]

This patchset changes the MessageReceived API, replacing 'subscription'
flag with destination address of received messages.

The application receives destination address as a D-Bus variant,
containing either as a regular 16bit address (unicast/group) or a
16-octet virtual label.

See previous discussion https://marc.info/?t=156719067300001&r=1&w=2 for
rationale.

Michał Lowas-Rzechonek (2):
  mesh: Provide destination address in MessageReceived API
  mesh: Inform application about model subscriptions

 doc/mesh-api.txt |  32 +++++++++++---
 mesh/model.c     | 107 ++++++++++++++++++++++++++++++++++++++++++++---
 test/test-mesh   |   4 +-
 3 files changed, 131 insertions(+), 12 deletions(-)

-- 
2.19.1

[PATCH BlueZ 1/2] mesh: Provide destination address in MessageReceived API

[Michał Lowas-Rzechonek]

---
 doc/mesh-api.txt | 17 ++++++++++++-----
 mesh/model.c     | 20 ++++++++++++++++----
 test/test-mesh   |  4 ++--
 3 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt
index a589616eb..23a958771 100644
--- a/doc/mesh-api.txt
+++ b/doc/mesh-api.txt
@@ -768,7 +768,7 @@ Object path	<app_defined_element_path>
 
 Methods:
 	void MessageReceived(uint16 source, uint16 key_index,
-					boolean subscription, array{byte} data)
+					variant destination, array{byte} data)
 
 		This method is called by bluetooth-meshd daemon when a message
 		arrives addressed to the application.
@@ -781,10 +781,17 @@ Methods:
 		be used by the application when sending a response to this
 		message (in case a response is expected).
 
-		The subscription parameter is a boolean that is set to true if
-		the message is received as a part of the subscription (i.e., the
-		destination is either a well known group address or a virtual
-		label.
+		The destination parameter contains the destination address of
+		received message. Underlying variant types are:
+
+		uint16
+
+			Destination is an unicast address, or a well known
+			group address
+
+		array{byte}
+
+			Destination is a virtual address label
 
 		The data parameter is the incoming message.
 
diff --git a/mesh/model.c b/mesh/model.c
index de271f171..a0c683691 100644
--- a/mesh/model.c
+++ b/mesh/model.c
@@ -823,8 +823,10 @@ static void send_dev_key_msg_rcvd(struct mesh_node *node, uint8_t ele_idx,
 	l_dbus_send(dbus, msg);
 }
 
-static void send_msg_rcvd(struct mesh_node *node, uint8_t ele_idx, bool is_sub,
-					uint16_t src, uint16_t app_idx,
+static void send_msg_rcvd(struct mesh_node *node, uint8_t ele_idx,
+					uint16_t src, uint16_t dst,
+					const struct mesh_virtual *virt,
+					uint16_t app_idx,
 					uint16_t size, const uint8_t *data)
 {
 	struct l_dbus *dbus = dbus_get_bus();
@@ -847,7 +849,17 @@ static void send_msg_rcvd(struct mesh_node *node, uint8_t ele_idx, bool is_sub,
 
 	l_dbus_message_builder_append_basic(builder, 'q', &src);
 	l_dbus_message_builder_append_basic(builder, 'q', &app_idx);
-	l_dbus_message_builder_append_basic(builder, 'b', &is_sub);
+
+	if (virt) {
+		l_dbus_message_builder_enter_variant(builder, "ay");
+		dbus_append_byte_array(builder, virt->label,
+							sizeof(virt->label));
+		l_dbus_message_builder_leave_variant(builder);
+	} else {
+		l_dbus_message_builder_enter_variant(builder, "q");
+		l_dbus_message_builder_append_basic(builder, 'q', &dst);
+		l_dbus_message_builder_leave_variant(builder);
+	}
 
 	dbus_append_byte_array(builder, data, size);
 
@@ -986,7 +998,7 @@ bool mesh_model_rx(struct mesh_node *node, bool szmict, uint32_t seq0,
 		 */
 		if (forward.has_dst && !forward.done) {
 			if ((decrypt_idx & APP_IDX_MASK) == decrypt_idx)
-				send_msg_rcvd(node, i, is_subscription, src,
+				send_msg_rcvd(node, i, src, dst, decrypt_virt,
 						forward.app_idx, forward.size,
 						forward.data);
 			else if (decrypt_idx == APP_IDX_DEV_REMOTE ||
diff --git a/test/test-mesh b/test/test-mesh
index 3c5ded7b3..91467bae7 100755
--- a/test/test-mesh
+++ b/test/test-mesh
@@ -556,8 +556,8 @@ class Element(dbus.service.Object):
 			self.UpdateModelConfiguration(mod_id, config[1])
 
 	@dbus.service.method(MESH_ELEMENT_IFACE,
-					in_signature="qqbay", out_signature="")
-	def MessageReceived(self, source, key, is_sub, data):
+					in_signature="qqvay", out_signature="")
+	def MessageReceived(self, source, key, destination, data):
 		print('Message Received on Element ', end='')
 		print(self.index)
 		for model in self.models:
-- 
2.19.1

[PATCH BlueZ 2/2] mesh: Inform application about model subscriptions

[Michał Lowas-Rzechonek]

---
 doc/mesh-api.txt | 15 +++++++++
 mesh/model.c     | 87 +++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 101 insertions(+), 1 deletion(-)

diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt
index 23a958771..30b7452e2 100644
--- a/doc/mesh-api.txt
+++ b/doc/mesh-api.txt
@@ -100,6 +100,14 @@ Methods:
 					A 16-bit Company ID as defined by the
 					Bluetooth SIG
 
+				array{variant} Subscriptions
+
+					Addresses the model is subscribed to.
+
+					Each address is provided either as
+					uint16 for group addresses, or
+					as array{byte} for virtual labels.
+
 		PossibleErrors:
 			org.bluez.mesh.Error.InvalidArguments
 			org.bluez.mesh.Error.NotFound,
@@ -841,6 +849,13 @@ Methods:
 			A 16-bit Bluetooth-assigned Company Identifier of the
 			vendor as defined by Bluetooth SIG
 
+		array{variant} Subscriptions
+
+			Addresses the model is subscribed to.
+
+			Each address is provided either as uint16 for group
+			addresses, or as array{byte} for virtual labels.
+
 Properties:
 	uint8 Index [read-only]
 
diff --git a/mesh/model.c b/mesh/model.c
index a0c683691..b4e2c0600 100644
--- a/mesh/model.c
+++ b/mesh/model.c
@@ -295,6 +295,71 @@ static void config_update_model_bindings(struct mesh_node *node,
 	l_dbus_send(dbus, msg);
 }
 
+static void append_dict_subs_array(struct l_dbus_message_builder *builder,
+						struct l_queue *subs,
+						struct l_queue *virts,
+						const char *key)
+{
+	const struct l_queue_entry *entry;
+
+	l_dbus_message_builder_enter_dict(builder, "sv");
+	l_dbus_message_builder_append_basic(builder, 's', key);
+	l_dbus_message_builder_enter_variant(builder, "av");
+	l_dbus_message_builder_enter_array(builder, "v");
+
+	if (!subs)
+		goto virts;
+
+	for (entry = l_queue_get_entries(subs); entry; entry = entry->next) {
+		uint16_t grp = L_PTR_TO_UINT(entry->data);
+
+		l_dbus_message_builder_enter_variant(builder, "q");
+		l_dbus_message_builder_append_basic(builder,'q',
+									&grp);
+		l_dbus_message_builder_leave_variant(builder);
+	}
+
+virts:
+	if (!virts)
+		goto done;
+
+	for (entry = l_queue_get_entries(virts); entry; entry = entry->next) {
+		const struct mesh_virtual *virt = entry->data;
+
+		l_dbus_message_builder_enter_variant(builder, "ay");
+		dbus_append_byte_array(builder, virt->label,
+							sizeof(virt->label));
+		l_dbus_message_builder_leave_variant(builder);
+
+	}
+
+done:
+	l_dbus_message_builder_leave_array(builder);
+	l_dbus_message_builder_leave_variant(builder);
+	l_dbus_message_builder_leave_dict(builder);
+}
+
+static void config_update_model_subscriptions(struct mesh_node *node,
+							struct mesh_model *mod)
+{
+	struct l_dbus *dbus = dbus_get_bus();
+	struct l_dbus_message *msg;
+	struct l_dbus_message_builder *builder;
+
+	msg = create_config_update_msg(node, mod->ele_idx, mod->id,
+								&builder);
+	if (!msg)
+		return;
+
+	append_dict_subs_array(builder, mod->subs, mod->virtuals,
+							"Subscriptions");
+
+	l_dbus_message_builder_leave_array(builder);
+	l_dbus_message_builder_finalize(builder);
+	l_dbus_message_builder_destroy(builder);
+	l_dbus_send(dbus, msg);
+}
+
 static void forward_model(void *a, void *b)
 {
 	struct mesh_model *mod = a;
@@ -1381,6 +1446,10 @@ int mesh_model_sub_add(struct mesh_node *node, uint16_t addr, uint32_t id,
 	if (!mod)
 		return status;
 
+	if (!mod->cbs)
+		/* External models */
+		config_update_model_subscriptions(node, mod);
+
 	return add_sub(node_get_net(node), mod, group, b_virt, dst);
 }
 
@@ -1417,7 +1486,7 @@ int mesh_model_sub_ovr(struct mesh_node *node, uint16_t addr, uint32_t id,
 		}
 	}
 
-	status = mesh_model_sub_add(node, addr, id, group, b_virt, dst);
+	status = add_sub(node_get_net(node), mod, group, b_virt, dst);
 
 	if (status != MESH_STATUS_SUCCESS) {
 		/* Adding new group failed, so revert to old lists */
@@ -1440,6 +1509,10 @@ int mesh_model_sub_ovr(struct mesh_node *node, uint16_t addr, uint32_t id,
 		l_queue_destroy(virtuals, unref_virt);
 	}
 
+	if (!mod->cbs)
+		/* External models */
+		config_update_model_subscriptions(node, mod);
+
 	return status;
 }
 
@@ -1475,6 +1548,10 @@ int mesh_model_sub_del(struct mesh_node *node, uint16_t addr, uint32_t id,
 	if (l_queue_remove(mod->subs, L_UINT_TO_PTR(grp)))
 		mesh_net_dst_unreg(node_get_net(node), grp);
 
+	if (!mod->cbs)
+		/* External models */
+		config_update_model_subscriptions(node, mod);
+
 	return MESH_STATUS_SUCCESS;
 }
 
@@ -1497,6 +1574,10 @@ int mesh_model_sub_del_all(struct mesh_node *node, uint16_t addr, uint32_t id)
 	l_queue_clear(mod->subs, NULL);
 	l_queue_clear(mod->virtuals, unref_virt);
 
+	if (!mod->cbs)
+		/* External models */
+		config_update_model_subscriptions(node, mod);
+
 	return MESH_STATUS_SUCCESS;
 }
 
@@ -1691,6 +1772,10 @@ void model_build_config(void *model, void *msg_builder)
 								&period);
 	}
 
+	if (l_queue_length(mod->subs) || l_queue_length(mod->virtuals))
+		append_dict_subs_array(builder, mod->subs, mod->virtuals,
+							"Subscriptions");
+
 	l_dbus_message_builder_leave_array(builder);
 	l_dbus_message_builder_leave_struct(builder);
 }
-- 
2.19.1

Re: [PATCH BlueZ 0/2] Include destination address in MessageReceived API

[Michał Lowas-Rzechonek]

Hi Brian

On 11/18, Michał Lowas-Rzechonek wrote:
> This patchset changes the MessageReceived API, replacing 'subscription'
> flag with destination address of received messages.
> 
> The application receives destination address as a D-Bus variant,
> containing either as a regular 16bit address (unicast/group) or a
> 16-octet virtual label.
> 
> See previous discussion https://marc.info/?t=156719067300001&r=1&w=2 for
> rationale.

Any comments about the patchset? If the change itself looks OK, I'm
happy to improve the patch if needed.

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

Re: [PATCH BlueZ 0/2] Include destination address in MessageReceived API

[Gix, Brian]

Hi Michał,

On Thu, 2019-11-21 at 09:19 +0100, Michał Lowas-Rzechonek wrote:
> Hi Brian
> 
> On 11/18, Michał Lowas-Rzechonek wrote:
> > This patchset changes the MessageReceived API, replacing 'subscription'
> > flag with destination address of received messages.
> > 
> > The application receives destination address as a D-Bus variant,
> > containing either as a regular 16bit address (unicast/group) or a
> > 16-octet virtual label.
> > 
> > See previous discussion https://marc.info/?t=156719067300001&r=1&w=2 for
> > rationale.
> 
> Any comments about the patchset? If the change itself looks OK, I'm
> happy to improve the patch if needed.

We are still reviewing the patch, and intend to apply it. (We accept the rationale).

We need to make sure that all of the existing tools and tests work with the API change.

--Brian

Re: [PATCH BlueZ 0/2] Include destination address in MessageReceived API

[Gix, Brian]

Hi Michał,

On Thu, 2019-11-21 at 09:19 +0100, Michał Lowas-Rzechonek wrote:
> Hi Brian
> 
> On 11/18, Michał Lowas-Rzechonek wrote:
> > This patchset changes the MessageReceived API, replacing 'subscription'
> > flag with destination address of received messages.
> > 
> > The application receives destination address as a D-Bus variant,
> > containing either as a regular 16bit address (unicast/group) or a
> > 16-octet virtual label.
> > 
> > See previous discussion https://marc.info/?t=156719067300001&r=1&w=2 for
> > rationale.
> 
> Any comments about the patchset? If the change itself looks OK, I'm
> happy to improve the patch if needed.


I think we would like the following:

1. Make API changes to MessageReceived() in test/test-join
2. Add printing out Subscriptions property to both test-mesh and test-join

Then, Assuming Inga has no other comments, I think this patch-set is ready for application.