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.
v2: Fix test scripts:
- fix API in test-join
- display model subscriptions in test-join and test-mesh
Michał Lowas-Rzechonek (3):
mesh: Fix test-join to include mandatory VendorModels property
mesh: Provide destination address in MessageReceived API
mesh: Inform application about model subscriptions
doc/mesh-api.txt | 32 +++++++++++---
mesh/model.c | 107 ++++++++++++++++++++++++++++++++++++++++++++---
test/test-join | 37 +++++++++++++---
test/test-mesh | 33 ++++++++-------
4 files changed, 179 insertions(+), 30 deletions(-)
--
2.19.1
---
doc/mesh-api.txt | 17 ++++++++++++-----
mesh/model.c | 20 ++++++++++++++++----
test/test-join | 8 ++++----
test/test-mesh | 8 ++++----
4 files changed, 36 insertions(+), 17 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-join b/test/test-join
index 079f71149..fb7b0d640 100644
--- a/test/test-join
+++ b/test/test-join
@@ -268,10 +268,10 @@ 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):
- print('Message Received on Element ', end='')
- print(self.index)
+ in_signature="qqvay", out_signature="")
+ def MessageReceived(self, source, key, destination, data):
+ print('Message Received on Element %d, src=%04x, dst=%s' %
+ self.index, source, destination)
for model in self.models:
model.process_message(source, key, data)
diff --git a/test/test-mesh b/test/test-mesh
index 3c5ded7b3..c67bb65fb 100755
--- a/test/test-mesh
+++ b/test/test-mesh
@@ -556,10 +556,10 @@ 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):
- print('Message Received on Element ', end='')
- print(self.index)
+ in_signature="qqvay", out_signature="")
+ def MessageReceived(self, source, key, destination, data):
+ print('Message Received on Element %d, src=%04x, dst=%s' %
+ self.index, source, destination)
for model in self.models:
model.process_message(source, key, data)
--
2.19.1
---
test/test-join | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/test/test-join b/test/test-join
index cdf92a2f1..079f71149 100644
--- a/test/test-join
+++ b/test/test-join
@@ -232,11 +232,25 @@ class Element(dbus.service.Object):
ids.append(id)
return ids
+ def _get_v_models(self):
+ ids = []
+ for model in self.models:
+ id = model.get_id()
+ v = model.get_vendor()
+ if v != VENDOR_ID_NONE:
+ vendor_id = (v, id)
+ ids.append(vendor_id)
+ return ids
+
def get_properties(self):
+ vendor_models = self._get_v_models()
+ sig_models = self._get_sig_models()
+
return {
MESH_ELEMENT_IFACE: {
'Index': dbus.Byte(self.index),
- 'Models': dbus.Array(self._get_sig_models(), 'q')
+ 'Models': dbus.Array(sig_models, 'q'),
+ 'VendorModels': dbus.Array(vendor_models, '(qq)'),
}
}
--
2.19.1
---
doc/mesh-api.txt | 15 +++++++++
mesh/model.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++-
test/test-join | 13 ++++++++
test/test-mesh | 25 ++++++++------
4 files changed, 128 insertions(+), 12 deletions(-)
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);
}
diff --git a/test/test-join b/test/test-join
index fb7b0d640..6dfb2e8c3 100644
--- a/test/test-join
+++ b/test/test-join
@@ -327,6 +327,19 @@ class Model():
print('Model publication period ', end='')
print(self.pub_period, end='')
print(' ms')
+ if 'Subscriptions' in config:
+ self.print_subscriptions(config.get('Subscriptions'))
+
+ def print_subscriptions(self, subscriptions):
+ print('Model subscriptions ', end='')
+ for sub in subscriptions:
+ if isinstance(sub, int):
+ print('%04x' % sub, end=' ')
+
+ if isinstance(sub, list):
+ label = uuid.UUID(bytes=b''.join(sub))
+ print(label, end=' ')
+ print()
class OnOffServer(Model):
def __init__(self, model_id):
diff --git a/test/test-mesh b/test/test-mesh
index c67bb65fb..5777fcebc 100755
--- a/test/test-mesh
+++ b/test/test-mesh
@@ -128,6 +128,7 @@ import dbus.exceptions
from threading import Timer
import time
+import uuid
try:
from gi.repository import GLib
@@ -628,17 +629,19 @@ class Model():
print('Model publication period ', end='')
print(self.pub_period, end='')
print(' ms')
-
- def print_bindings(self):
- print(set_cyan('Model'), set_cyan('%03x' % self.model_id),
- set_cyan('is bound to: '))
-
- if len(self.bindings) == 0:
- print(set_cyan('** None **'))
- return
-
- for b in self.bindings:
- print(set_green('%03x' % b) + ' ')
+ if 'Subscriptions' in config:
+ print('Model subscriptions ', end='')
+ self.print_subscriptions(config.get('Subscriptions'))
+ print()
+
+ def print_subscriptions(self, subscriptions):
+ for sub in subscriptions:
+ if isinstance(sub, int):
+ print('%04x' % sub, end=' ')
+
+ if isinstance(sub, list):
+ label = uuid.UUID(bytes=b''.join(sub))
+ print(label, end=' ')
########################
# On Off Server Model
--
2.19.1
Hi Michał,
On Mon, 2019-11-25 at 13:31 +0100, Michał Lowas-Rzechonek wrote:
> ---
> doc/mesh-api.txt | 15 +++++++++
> mesh/model.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++-
> test/test-join | 13 ++++++++
> test/test-mesh | 25 ++++++++------
> 4 files changed, 128 insertions(+), 12 deletions(-)
>
> 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);
Could you please fix the style here: space before 'q' and move &grp to
the same line as the rest of the args
> + 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);
> +
Slight change of logic is needed here: first add subscription, and
then, in case of success, notify the app.
The way it is coded now, the app receives the list of subscriptions
that hasn't been updated yet.
> 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);
> }
> diff --git a/test/test-join b/test/test-join
> index fb7b0d640..6dfb2e8c3 100644
> --- a/test/test-join
> +++ b/test/test-join
> @@ -327,6 +327,19 @@ class Model():
> print('Model publication period ', end='')
> print(self.pub_period, end='')
> print(' ms')
> + if 'Subscriptions' in config:
> + self.print_subscriptions(config.get('Subscriptions'))
> +
> + def print_subscriptions(self, subscriptions):
> + print('Model subscriptions ', end='')
> + for sub in subscriptions:
> + if isinstance(sub, int):
> + print('%04x' % sub, end=' ')
> +
> + if isinstance(sub, list):
> + label = uuid.UUID(bytes=b''.join(sub))
> + print(label, end=' ')
> + print()
>
> class OnOffServer(Model):
> def __init__(self, model_id):
> diff --git a/test/test-mesh b/test/test-mesh
> index c67bb65fb..5777fcebc 100755
> --- a/test/test-mesh
> +++ b/test/test-mesh
> @@ -128,6 +128,7 @@ import dbus.exceptions
>
> from threading import Timer
> import time
> +import uuid
>
> try:
> from gi.repository import GLib
> @@ -628,17 +629,19 @@ class Model():
> print('Model publication period ', end='')
> print(self.pub_period, end='')
> print(' ms')
> -
> - def print_bindings(self):
> - print(set_cyan('Model'), set_cyan('%03x' % self.model_id),
> - set_cyan('is bound to: '))
> -
> - if len(self.bindings) == 0:
> - print(set_cyan('** None **'))
> - return
> -
> - for b in self.bindings:
> - print(set_green('%03x' % b) + ' ')
> + if 'Subscriptions' in config:
> + print('Model subscriptions ', end='')
> + self.print_subscriptions(config.get('Subscriptions'))
> + print()
> +
> + def print_subscriptions(self, subscriptions):
> + for sub in subscriptions:
> + if isinstance(sub, int):
> + print('%04x' % sub, end=' ')
> +
> + if isinstance(sub, list):
> + label = uuid.UUID(bytes=b''.join(sub))
> + print(label, end=' ')
>
> ########################
> # On Off Server Model
Otherwise, looks good.
Best regards,
Inga