mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[Michał Lowas-Rzechonek]

Hi,

When trying to use subscriptions in meshd, I've noticed that
MessageReceived API does not provide the application with destination
address of the deceived message - instead, it only sends a boolean flag
saying if the message was part of a model subscription, or not.

I think this is problematic. There are use cases where a model is
interested in the destination address of a group subscription. For
example:

Imagine a dot-matrix, where each pixel is a mesh node.

Each of these pixels implements two models:
    on element 0, a GenericOnOffServer controlling the light output
    on element 1, a Blinkenlights Server model

Blinkenlights Server extends GenericOnOff Server and GenericOnOff
Client, and on top of that contains a translation table mapping group
address to either 'ON' or 'OFF'.

Now, when Blinkenlights Server receives a GenericOnOff Set message, it
looks up the destination address at the translation table, and sends a
*different* GenericOnOff Set to *its own* element 0, with target value
determined by the translation entry.

This allows users to configure each node in such a way, that sending a
*single* message to a group address causes all pixels to switch to a
preconfigured pattern *at the same time*.

Moreover, at the moment the application can't receive the label of a
virtual address the message is targeted to.


I'd like to discuss the API change, from the current:

	void MessageReceived(uint16 source, uint16 key_index,
					boolean subscription, array{byte} data)

to something like:

	void MessageReceived(uint16 source, uint16 destination,
                    uint16 key_index, array{byte} data)

	void VirtMessageReceived(uint16 source, array{byte}[16] label,
                    uint16 key_index, array{byte} data)

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

Re: mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[Michał Lowas-Rzechonek]

Hello,

On 08/30, Michał Lowas-Rzechonek wrote:
> When trying to use subscriptions in meshd, I've noticed that
> MessageReceived API does not provide the application with destination
> address of the deceived message - instead, it only sends a boolean flag
> saying if the message was part of a model subscription, or not.
> 
> I think this is problematic. There are use cases where a model is
> interested in the destination address of a group subscription. For
> example:

Alright, so it seems that is not as straightforward as I would like it
to be...

First of all, it seems that the specification doesn't *strictly* require
the model to know the value of the destination address of received
messages. The only passage that somewhat relates to that is in Mesh
Models spec v1.0.1, 3.3.2.2.3 "Receiving Generic Delta Set", but at the
moment we think that the requirement about aborting transacations
specified there can be satisfied without knowing the numeric value of a
destination address.

Other things have been neatly summarized by Brian, so I'm gonna respond
as usual.

On 09/04, Gix, Brian wrote:
> Well, what we are trying to balance here is:
>
> 1. Keeping the APIs as simple as possible

Yes, but not simpler...

> 2. Not exposing any information that we don't absolutely need to

And this is the thing I simply don't agree with. Security in mesh is
based on *keys*, not *APIs*.

We discussed access to the keyring before and I kind of agree that
applications don't really need access to key values, as long as various
Import*() methods exist. So keys are covered.

But hiding anything else does not make any sense. Certainly, it doesn't
"increase security" by any means. There is no reason why a model should
not have access to pretty much the whole payload (once it passes all the
layers, subscriptions and bindings are verified etc)., a complete
network layer included.

My primary argument is this: BT mesh is a relatively young standard and
apart from what's in Mesh Models spec we don't see many applications in
the wild. Who knows what the people are going to do with it? One of the
reasons we would like BT mesh to be available as free software is to
enable various people to play with it and invent new, interesting
applications (ideally, they would make it back to SIG and be adopted as
official specification, but that's another story).

In my opinion, keeping that power away from users, because there is no
"official" spec that uses certain information, is harmful.

> 3. Enabling valid use cases.
>
> We also are trying to stay true to the *spirit* of the specifications,
> and so I get a little uncomfortable with arguments like "The spec
> doesn't explicitely disallow it, so we need to support it".

And who gets to decide which use cases are "valid" and "true to the
spirit"?

The two examples I provided are *not* violating the spec in any way.
For the record:
 - a combined server/client sitting on element 1 that receives onoff
   messages and, depending on the destination address, sends a different
   onoff messages to a "regular" onoff server sitting on element 0,
   allowing efficient control over switching scenes involving large
   number of nodes
 - a model that acts as a IPv6 gateway and directly maps virtual
   addresses to IPv6 addresses of nodes living on the other side of the
   gateway

> That said, in this case we are proposing substituting a boolean
> argument (called "subscription", but really just a differentiator
> between a message received as a Unicast message vs a Group or Virtual
> address) with the actual destrination address. If this was as simple
> as a parameter swap, I would say "Go for it", but because of Virtual
> Addresses, it means substituting *two* methods for the one.  Then to
> continue down the rabbit hole, every *App* needs to support both
> methods which increases their complexity as well.

Well, if the application is not using virtual addresses then it doesn't
need to implement the method.

> I remember having this conversation just between Inga and myself when
> designing the interface, which is why we went for the Boolean
> argument...  to keep the API simpler in both the Applications and in
> the Daemon.  But if we are *absolutely sure* that there are valid Use
> Cases that *require* the knowledge of *which* subscription a message
> was received with, then we may have no choice.
>
> Some of the use-cases suggested I believe are invalid...  Multiple
> models on the same element are not allowed to execute the same
> opcode... or at least not ones that change a local state.

Could you elaborate? The two examples above don't require executing the
same opcode on different models.

The first one (in a slightly different form, but the idea is the same)
is already deployed in a production environment.

> This is Inga's area of expertice (both on the App side and the Daemon
> side) so I am inclined to trust her judgement.  But I would *really*
> like to avoid adding yet another MessageRecieved method if possibled.

We could make the address a D-Bus variant. I proposed two methods,
because I find variants somewhat cumbersome, but I have no strong
opinion about that.

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

Re: mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[Michał Lowas-Rzechonek]

On 09/04, Michał Lowas-Rzechonek wrote:
> The two examples I provided are *not* violating the spec in any way.
> For the record:
>  - a combined server/client sitting on element 1 that receives onoff
>    messages and, depending on the destination address, sends a different
>    onoff messages to a "regular" onoff server sitting on element 0,
>    allowing efficient control over switching scenes involving large
>    number of nodes
>  - a model that acts as a IPv6 gateway and directly maps virtual
>    addresses to IPv6 addresses of nodes living on the other side of the
>    gateway

Another one about virtual addresses:

In CANOpen, there is a concept of a "Protocol Data Object" [1].
Basically, the idea is to pack many pieces of information into a
preconfigured format (down to single bits, because CAN frames are even
shorter than mesh ones) - this is known as "PDO Mapping Parameters" -
then send such payloads to a well-known group address.

In static configurations, this allows to decrease the number (and size)
of packets sent by sensor nodes.

Since PDO payloads are *not* self-describing (unlike mesh sensor
messages), the receiving party must be aware of the mapping in order to
parse the data.

In CANOpen, format is determined by the address - in mesh, it could very
well be a virtual label.

[1] https://www.can-cia.org/can-knowledge/canopen/pdo-protocol/

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

Re: mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[Gix, Brian]

On Wed, 2019-09-04 at 21:48 +0200, Michał Lowas-Rzechonek wrote:
> On 09/04, Michał Lowas-Rzechonek wrote:
> > The two examples I provided are *not* violating the spec in any way.
> > For the record:
> >  - a combined server/client sitting on element 1 that receives onoff
> >    messages and, depending on the destination address, sends a different
> >    onoff messages to a "regular" onoff server sitting on element 0,
> >    allowing efficient control over switching scenes involving large
> >    number of nodes
> >  - a model that acts as a IPv6 gateway and directly maps virtual
> >    addresses to IPv6 addresses of nodes living on the other side of the
> >    gateway
> 
> Another one about virtual addresses:
> 
> In CANOpen, there is a concept of a "Protocol Data Object" [1].
> Basically, the idea is to pack many pieces of information into a
> preconfigured format (down to single bits, because CAN frames are even
> shorter than mesh ones) - this is known as "PDO Mapping Parameters" -
> then send such payloads to a well-known group address.
> 
> In static configurations, this allows to decrease the number (and size)
> of packets sent by sensor nodes.
> 
> Since PDO payloads are *not* self-describing (unlike mesh sensor
> messages), the receiving party must be aware of the mapping in order to
> parse the data.
> 
> In CANOpen, format is determined by the address - in mesh, it could very
> well be a virtual label.
> 
> [1] https://www.can-cia.org/can-knowledge/canopen/pdo-protocol/
> 

I think that this is an interesting use of Virtual Addresses, and in addition to this, Mesh Virtual Addresses
have been suggested as a way of addressing IPv6 addressing as well...  However:

1. There is already a way something like this could be used already:  A model could be created that gets
subscribed to the Virtual Addresses that require handling by the node.

2. If such a system proves to be widely requested, daemon support could be added (perhaps under a different
DBus interface) for either or both of IPv6 and "CANOpen".

In any case the ability to create simple mesh Apps with minimal complexity remains intact, and as an added
bonus, the Open Source community (not to mention the Bluetooth Mesh Working Group and larger SIG) can weigh in
on the preferred methodologies.

Re: mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[Stotland, Inga]

[-- Attachment #1: Type: text/plain, Size: 3058 bytes --]

Hi,

On Wed, 2019-09-04 at 13:26 -0700, Gix, Brian wrote:
> On Wed, 2019-09-04 at 21:48 +0200, Michał Lowas-Rzechonek wrote:
> > On 09/04, Michał Lowas-Rzechonek wrote:
> > > The two examples I provided are *not* violating the spec in any
> > > way.
> > > For the record:
> > >  - a combined server/client sitting on element 1 that receives
> > > onoff
> > >    messages and, depending on the destination address, sends a
> > > different
> > >    onoff messages to a "regular" onoff server sitting on element
> > > 0,
> > >    allowing efficient control over switching scenes involving
> > > large
> > >    number of nodes


This sounds like something a vendor model mechanism should handle:
the "mapping" should be understood on both ends: client and server.


> > >  - a model that acts as a IPv6 gateway and directly maps virtual
> > >    addresses to IPv6 addresses of nodes living on the other side
> > > of the
> > >    gateway
> > 
> > Another one about virtual addresses:
> > 
> > In CANOpen, there is a concept of a "Protocol Data Object" [1].
> > Basically, the idea is to pack many pieces of information into a
> > preconfigured format (down to single bits, because CAN frames are
> > even
> > shorter than mesh ones) - this is known as "PDO Mapping Parameters"
> > -
> > then send such payloads to a well-known group address.
> > 
> > In static configurations, this allows to decrease the number (and
> > size)
> > of packets sent by sensor nodes.
> > 
> > Since PDO payloads are *not* self-describing (unlike mesh sensor
> > messages), the receiving party must be aware of the mapping in
> > order to
> > parse the data.
> > 
> > In CANOpen, format is determined by the address - in mesh, it could
> > very
> > well be a virtual label.
> > 
> > [1] https://www.can-cia.org/can-knowledge/canopen/pdo-protocol/
> > 
> 
> I think that this is an interesting use of Virtual Addresses, and in
> addition to this, Mesh Virtual Addresses
> have been suggested as a way of addressing IPv6 addressing as
> well...  However:
> 
> 1. There is already a way something like this could be used
> already:  A model could be created that gets
> subscribed to the Virtual Addresses that require handling by the
> node.
> 
> 2. If such a system proves to be widely requested, daemon support
> could be added (perhaps under a different
> DBus interface) for either or both of IPv6 and "CANOpen".
> 
> In any case the ability to create simple mesh Apps with minimal
> complexity remains intact, and as an added
> bonus, the Open Source community (not to mention the Bluetooth Mesh
> Working Group and larger SIG) can weigh in
> on the preferred methodologies.
> 

My feeling is that the API should be geared towards common case
scenarios (i.e., defined models and such). 

If there is a behavior that absolutely cannot be addressed with the
current API (and use of vendor models), then it has to be
changed/augmented.

As such, I still don't see a compelling reason to do so.



[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3265 bytes --]

Re: mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[michal.lowas-rzechonek]

Hi Inga,

On 09/04, Stotland, Inga wrote:
> > > >  - a combined server/client sitting on element 1 that receives
> > > >  onoff messages and, depending on the destination address, sends
> > > >  a different onoff messages to a "regular" onoff server sitting
> > > >  on element 0, allowing efficient control over switching scenes
> > > >  involving large number of nodes
> This sounds like something a vendor model mechanism should handle:
> the "mapping" should be understood on both ends: client and server.

Well, yes, I've described a vendor model... one that uses destination
group addresses to 'understand' the mapping.

I don't get what other mechanism do you have in mind?

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

Re: mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[michal.lowas-rzechonek]

Hi Brian,

On 09/04, Gix, Brian wrote:
> > Since PDO payloads are *not* self-describing (unlike mesh sensor
> > messages), the receiving party must be aware of the mapping in order to
> > parse the data.
> > 
> > In CANOpen, format is determined by the address - in mesh, it could very
> > well be a virtual label.
> 
> I think that this is an interesting use of Virtual Addresses, and in
> addition to this, Mesh Virtual Addresses have been suggested as a way
> of addressing IPv6 addressing as well...  However:
> 
> 1. There is already a way something like this could be used already:
>    A model could be created that gets subscribed to the Virtual
>    Addresses that require handling by the node.

But the whole idea is that the virtual labels and packing are configured
dynamically (by the commissioner) according to their specific needs.

If a model doesn't receive the *value* of the label, you would need a
separate model for each of the virtuals. This is problematic, because
composition data is immutable, so you wouldn't be able to reconfigure
these mappings without at least re-provisioning the whole network.

> In any case the ability to create simple mesh Apps with minimal
> complexity remains intact, and as an added bonus, the Open Source
> community (not to mention the Bluetooth Mesh Working Group and larger
> SIG) can weigh in on the preferred methodologies.

Aren't *we* the community? :P

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

Re: mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[Michał Lowas-Rzechonek]

Hi Brian,

> Imagine a dot-matrix, where each pixel is a mesh node.
> 
> Each of these pixels implements two models:
>     on element 0, a GenericOnOffServer controlling the light output
>     on element 1, a Blinkenlights Server model
> 
> Blinkenlights Server extends GenericOnOff Server and GenericOnOff
> Client, and on top of that contains a translation table mapping group
> address to either 'ON' or 'OFF'.
> 
> Now, when Blinkenlights Server receives a GenericOnOff Set message, it
> looks up the destination address at the translation table, and sends a
> *different* GenericOnOff Set to *its own* element 0, with target value
> determined by the translation entry.
> 
> This allows users to configure each node in such a way, that sending a
> *single* message to a group address causes all pixels to switch to a
> preconfigured pattern *at the same time*.

Per conversation with Piotr, I'd like to revisit the discussion and
provide more details about our use case for models knowing the
destination address.

Please see a diagram at http://ujeb.se/BmTIW.

The main reason we map scenes using destination addresses is that such a
setup consumes much less unicast addresses.

Assuming that:
 S - number of switches
 B - number of buttons (elements) on a switch
 N - nunber of lamps

With a 'regular' case, number of consumed unicast addresses is
    S*B + N*(B+1)

With the destination mapping, it becomes
    S*B + N*2

Since we typically use 4 button switches (B=4), without translation we
consume unicast address space at a *much* faster rate.

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

Re: mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[Michał Lowas-Rzechonek]

On 09/18, Michał Lowas-Rzechonek wrote:
> Please see a diagram at http://ujeb.se/BmTIW.

If you have trouble accesing the drawing, here are PNGs:

https://drive.google.com/file/d/1zZrmFB7NLcbyR-tPE9ljqxolZIonxXbc/view
https://drive.google.com/file/d/1ntkJGU1SYtgqrmQN9F4PDiWdjYAc0U8o/view

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

Re: mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[Stotland, Inga]

[-- Attachment #1: Type: text/plain, Size: 2287 bytes --]

Hi Michal,

On Wed, 2019-09-18 at 10:52 +0200, Michał Lowas-Rzechonek wrote:
> Hi Brian,
> 
> > Imagine a dot-matrix, where each pixel is a mesh node.
> > 
> > Each of these pixels implements two models:
> >     on element 0, a GenericOnOffServer controlling the light output
> >     on element 1, a Blinkenlights Server model
> > 
> > Blinkenlights Server extends GenericOnOff Server and GenericOnOff
> > Client, and on top of that contains a translation table mapping
> > group
> > address to either 'ON' or 'OFF'.
> > 
> > Now, when Blinkenlights Server receives a GenericOnOff Set message,
> > it
> > looks up the destination address at the translation table, and
> > sends a
> > *different* GenericOnOff Set to *its own* element 0, with target
> > value
> > determined by the translation entry.
> > 
> > This allows users to configure each node in such a way, that
> > sending a
> > *single* message to a group address causes all pixels to switch to
> > a
> > preconfigured pattern *at the same time*.
> 
> Per conversation with Piotr, I'd like to revisit the discussion and
> provide more details about our use case for models knowing the
> destination address.
> 
> Please see a diagram at http://ujeb.se/BmTIW.
> 
> The main reason we map scenes using destination addresses is that
> such a
> setup consumes much less unicast addresses.
> 
> Assuming that:
>  S - number of switches
>  B - number of buttons (elements) on a switch
>  N - nunber of lamps
> 
> With a 'regular' case, number of consumed unicast addresses is
>     S*B + N*(B+1)
> 
> With the destination mapping, it becomes
>     S*B + N*2
> 
> Since we typically use 4 button switches (B=4), without translation
> we
> consume unicast address space at a *much* faster rate.
> 
> reagrds

Okay, this is a good argument for exposing the subscription address in
MessageReceived().
It's better to separate the method into two, e.g. MessageReceived() and
MessageReceivedVirtual().

Then it makes sense to add model subscription array as a dictionary
entry in the UpdateModelConfiguration() as well as for the node
configuration returned when calling Attach() method.
Probably will have to have separate keys: "Groups" and "Virtuals".

Regards,
Inga

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3265 bytes --]

Re: mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[Gix, Brian]

Hi Inga, Michał,

On Wed, 2019-09-25 at 19:02 +0000, Stotland, Inga wrote:
> Hi Michal,
> 
> On Wed, 2019-09-18 at 10:52 +0200, Michał Lowas-Rzechonek wrote:
> > Hi Brian,
> > 
> > > Imagine a dot-matrix, where each pixel is a mesh node.
> > > 
> > > Each of these pixels implements two models:
> > >     on element 0, a GenericOnOffServer controlling the light output
> > >     on element 1, a Blinkenlights Server model
> > > 
> > > Blinkenlights Server extends GenericOnOff Server and GenericOnOff
> > > Client, and on top of that contains a translation table mapping
> > > group
> > > address to either 'ON' or 'OFF'.
> > > 
> > > Now, when Blinkenlights Server receives a GenericOnOff Set message,
> > > it
> > > looks up the destination address at the translation table, and
> > > sends a
> > > *different* GenericOnOff Set to *its own* element 0, with target
> > > value
> > > determined by the translation entry.
> > > 
> > > This allows users to configure each node in such a way, that
> > > sending a
> > > *single* message to a group address causes all pixels to switch to
> > > a
> > > preconfigured pattern *at the same time*.
> > 
> > Per conversation with Piotr, I'd like to revisit the discussion and
> > provide more details about our use case for models knowing the
> > destination address.
> > 
> > Please see a diagram at http://ujeb.se/BmTIW.
> > 
> > The main reason we map scenes using destination addresses is that
> > such a
> > setup consumes much less unicast addresses.
> > 
> > Assuming that:
> >  S - number of switches
> >  B - number of buttons (elements) on a switch
> >  N - nunber of lamps
> > 
> > With a 'regular' case, number of consumed unicast addresses is
> >     S*B + N*(B+1)
> > 
> > With the destination mapping, it becomes
> >     S*B + N*2
> > 
> > Since we typically use 4 button switches (B=4), without translation
> > we
> > consume unicast address space at a *much* faster rate.
> > 
> > reagrds
> 
> Okay, this is a good argument for exposing the subscription address in
> MessageReceived().
> It's better to separate the method into two, e.g. MessageReceived() and
> MessageReceivedVirtual().

I wonder if we could still do this with a single method.  I can think
of 2 methodologies:

1. A simple way that just uses the U16 DST field instead of
the "subscription" boolean (not a 100% reliable differentiator
for Virtuals, but may be sufficient for the use cases given).

2. Replacing the subscription boolean with a u32 "Subscription ID".
A subscription ID value of 0x000000000 would indicate that the
message was received with the Unicast address, and values from
1 - 0xFFFFFFFF mean a Subscription that can be queried for. This
would be accompanied by a new daemon method which could look up
the details of the subscription:

	{dict subcription}  LookupSubscription(u32 Sub_ID)

Both of these methodologies would allow an App to be simpler,
with no added D-Bus Methods required.

With the 2nd methodology, the subscription only needs to be
looked up once (or not at all) to 100% differentiate between
discrete subscriptions.

I *really* do not want an additional mandatory App Method. Most
Apps will be simpler than that, and truely not care to
differentiate between subscriptions...   Particularily Client
based Apps.

> 
> Then it makes sense to add model subscription array as a dictionary
> entry in the UpdateModelConfiguration() as well as for the node
> configuration returned when calling Attach() method.
> Probably will have to have separate keys: "Groups" and "Virtuals".
> 
> Regards,
> Inga

Re: mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[Stotland, Inga]

[-- Attachment #1: Type: text/plain, Size: 5490 bytes --]

Hi Brian, Michal,

Pairing down the cc list since we are down to implementation details.

On Thu, 2019-09-26 at 15:18 +0000, Gix, Brian wrote:
> Hi Inga, Michał,
> 
> On Wed, 2019-09-25 at 19:02 +0000, Stotland, Inga wrote:
> > Hi Michal,
> > 
> > On Wed, 2019-09-18 at 10:52 +0200, Michał Lowas-Rzechonek wrote:
> > > Hi Brian,
> > > 
> > > > Imagine a dot-matrix, where each pixel is a mesh node.
> > > > 
> > > > Each of these pixels implements two models:
> > > >     on element 0, a GenericOnOffServer controlling the light
> > > > output
> > > >     on element 1, a Blinkenlights Server model
> > > > 
> > > > Blinkenlights Server extends GenericOnOff Server and
> > > > GenericOnOff
> > > > Client, and on top of that contains a translation table mapping
> > > > group
> > > > address to either 'ON' or 'OFF'.
> > > > 
> > > > Now, when Blinkenlights Server receives a GenericOnOff Set
> > > > message,
> > > > it
> > > > looks up the destination address at the translation table, and
> > > > sends a
> > > > *different* GenericOnOff Set to *its own* element 0, with
> > > > target
> > > > value
> > > > determined by the translation entry.
> > > > 
> > > > This allows users to configure each node in such a way, that
> > > > sending a
> > > > *single* message to a group address causes all pixels to switch
> > > > to
> > > > a
> > > > preconfigured pattern *at the same time*.
> > > 
> > > Per conversation with Piotr, I'd like to revisit the discussion
> > > and
> > > provide more details about our use case for models knowing the
> > > destination address.
> > > 
> > > Please see a diagram at http://ujeb.se/BmTIW.
> > > 
> > > The main reason we map scenes using destination addresses is that
> > > such a
> > > setup consumes much less unicast addresses.
> > > 
> > > Assuming that:
> > >  S - number of switches
> > >  B - number of buttons (elements) on a switch
> > >  N - nunber of lamps
> > > 
> > > With a 'regular' case, number of consumed unicast addresses is
> > >     S*B + N*(B+1)
> > > 
> > > With the destination mapping, it becomes
> > >     S*B + N*2
> > > 
> > > Since we typically use 4 button switches (B=4), without
> > > translation
> > > we
> > > consume unicast address space at a *much* faster rate.
> > > 
> > > reagrds
> > 
> > Okay, this is a good argument for exposing the subscription address
> > in
> > MessageReceived().
> > It's better to separate the method into two, e.g. MessageReceived()
> > and
> > MessageReceivedVirtual().
> 
> I wonder if we could still do this with a single method.  I can think
> of 2 methodologies:
> 
> 1. A simple way that just uses the U16 DST field instead of
> the "subscription" boolean (not a 100% reliable differentiator
> for Virtuals, but may be sufficient for the use cases given).
> 
> 2. Replacing the subscription boolean with a u32 "Subscription ID".
> A subscription ID value of 0x000000000 would indicate that the
> message was received with the Unicast address, and values from
> 1 - 0xFFFFFFFF mean a Subscription that can be queried for. This
> would be accompanied by a new daemon method which could look up
> the details of the subscription:
> 
> {dict subcription}  LookupSubscription(u32 Sub_ID)
> 
> Both of these methodologies would allow an App to be simpler,
> with no added D-Bus Methods required.
> 
> With the 2nd methodology, the subscription only needs to be
> looked up once (or not at all) to 100% differentiate between
> discrete subscriptions.
> 
> I *really* do not want an additional mandatory App Method. Most
> Apps will be simpler than that, and truely not care to
> differentiate between subscriptions...   Particularily Client
> based Apps.

While I am still in favor of two distinct methods (given choice, I'd
always go with self-explanatory API), one method approach would work as
well if accompanied by detailed and clear description in the mesh-
api.txt doc.

I vote against u16 destination field since there is no reason to
create address space collision even though the chances are small.

A single method "MessageReceived" method can be modified to include a
subscription parameter as:
1) a dictionary with keys "Group" and "Label" (self explanatory if a
bit cumbersome).
or
2) a u32 subscription ID that represents a subscription. This 
requires the daemon to maintain the relationship between "id" and the
actual address. I believe the daemon does this anyway for virtual
labels, but from the API design perspective this is not very clean
IMHO, since it has a whiff of implementation details leaking into user
interface API. 

No matter which approach is chosen, the subscriptions must be included
in the model configuration dictionary for UpdateModelConfiguration()
and in the corresponding dictionary for node configuration returned on
successful execution of Attach().

If we go with a single method and a u32 subscription id, the
disctionary representation of the subscriptions should be a pair {id,
actual subscription address}. This way there is no need for an
additional daemon method.

Michal, any comments?

> 
> > Then it makes sense to add model subscription array as a dictionary
> > entry in the UpdateModelConfiguration() as well as for the node
> > configuration returned when calling Attach() method.
> > Probably will have to have separate keys: "Groups" and "Virtuals".
> > 

Regards,
Inga



[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3265 bytes --]

Re: mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[Gix, Brian]

Hi Inga,

On Thu, 2019-09-26 at 20:41 +0000, Stotland, Inga wrote:
> Hi Brian, Michal,
> 
> Pairing down the cc list since we are down to implementation details.
> 
> On Thu, 2019-09-26 at 15:18 +0000, Gix, Brian wrote:
> > Hi Inga, Michał,
> > 
> > On Wed, 2019-09-25 at 19:02 +0000, Stotland, Inga wrote:
> > > Hi Michal,
> > > 
> > > On Wed, 2019-09-18 at 10:52 +0200, Michał Lowas-Rzechonek wrote:
> > > > Hi Brian,
> > > > 
> > > > > Imagine a dot-matrix, where each pixel is a mesh node.
> > > > > 
> > > > > Each of these pixels implements two models:
> > > > >     on element 0, a GenericOnOffServer controlling the light
> > > > > output
> > > > >     on element 1, a Blinkenlights Server model
> > > > > 
> > > > > Blinkenlights Server extends GenericOnOff Server and
> > > > > GenericOnOff
> > > > > Client, and on top of that contains a translation table mapping
> > > > > group
> > > > > address to either 'ON' or 'OFF'.
> > > > > 
> > > > > Now, when Blinkenlights Server receives a GenericOnOff Set
> > > > > message,
> > > > > it
> > > > > looks up the destination address at the translation table, and
> > > > > sends a
> > > > > *different* GenericOnOff Set to *its own* element 0, with
> > > > > target
> > > > > value
> > > > > determined by the translation entry.
> > > > > 
> > > > > This allows users to configure each node in such a way, that
> > > > > sending a
> > > > > *single* message to a group address causes all pixels to switch
> > > > > to
> > > > > a
> > > > > preconfigured pattern *at the same time*.
> > > > 
> > > > Per conversation with Piotr, I'd like to revisit the discussion
> > > > and
> > > > provide more details about our use case for models knowing the
> > > > destination address.
> > > > 
> > > > Please see a diagram at http://ujeb.se/BmTIW.
> > > > 
> > > > The main reason we map scenes using destination addresses is that
> > > > such a
> > > > setup consumes much less unicast addresses.
> > > > 
> > > > Assuming that:
> > > >  S - number of switches
> > > >  B - number of buttons (elements) on a switch
> > > >  N - nunber of lamps
> > > > 
> > > > With a 'regular' case, number of consumed unicast addresses is
> > > >     S*B + N*(B+1)
> > > > 
> > > > With the destination mapping, it becomes
> > > >     S*B + N*2
> > > > 
> > > > Since we typically use 4 button switches (B=4), without
> > > > translation
> > > > we
> > > > consume unicast address space at a *much* faster rate.
> > > > 
> > > > reagrds
> > > 
> > > Okay, this is a good argument for exposing the subscription address
> > > in
> > > MessageReceived().
> > > It's better to separate the method into two, e.g. MessageReceived()
> > > and
> > > MessageReceivedVirtual().
> > 
> > I wonder if we could still do this with a single method.  I can think
> > of 2 methodologies:
> > 
> > 1. A simple way that just uses the U16 DST field instead of
> > the "subscription" boolean (not a 100% reliable differentiator
> > for Virtuals, but may be sufficient for the use cases given).
> > 
> > 2. Replacing the subscription boolean with a u32 "Subscription ID".
> > A subscription ID value of 0x000000000 would indicate that the
> > message was received with the Unicast address, and values from
> > 1 - 0xFFFFFFFF mean a Subscription that can be queried for. This
> > would be accompanied by a new daemon method which could look up
> > the details of the subscription:
> > 
> > {dict subcription}  LookupSubscription(u32 Sub_ID)
> > 
> > Both of these methodologies would allow an App to be simpler,
> > with no added D-Bus Methods required.
> > 
> > With the 2nd methodology, the subscription only needs to be
> > looked up once (or not at all) to 100% differentiate between
> > discrete subscriptions.
> > 
> > I *really* do not want an additional mandatory App Method. Most
> > Apps will be simpler than that, and truely not care to
> > differentiate between subscriptions...   Particularily Client
> > based Apps.
> 
> While I am still in favor of two distinct methods (given choice, I'd
> always go with self-explanatory API), one method approach would work as
> well if accompanied by detailed and clear description in the mesh-
> api.txt doc.
> 
> I vote against u16 destination field since there is no reason to
> create address space collision even though the chances are small.
> 
> A single method "MessageReceived" method can be modified to include a
> subscription parameter as:
> 1) a dictionary with keys "Group" and "Label" (self explanatory if a
> bit cumbersome).

If this use case is chosen, is it easy enough for an App implementation
of this method to *ignore* the dictionary if the subscription information
is not important to that model? (Again:  Thinking of simple applications
that just want to know whether to respond to the message or not because
it was or wasn't sent to the Unicast address)

> or
> 2) a u32 subscription ID that represents a subscription. This 
> requires the daemon to maintain the relationship between "id" and the
> actual address. I believe the daemon does this anyway for virtual
> labels, but from the API design perspective this is not very clean
> IMHO, since it has a whiff of implementation details leaking into user
> interface API. 
> 
> No matter which approach is chosen, the subscriptions must be included
> in the model configuration dictionary for UpdateModelConfiguration()
> and in the corresponding dictionary for node configuration returned on
> successful execution of Attach().
> 
> If we go with a single method and a u32 subscription id, the
> disctionary representation of the subscriptions should be a pair {id,
> actual subscription address}. This way there is no need for an
> additional daemon method.

We have avoided dictionaries specifically to avoid mandating that
Applications need to have logic to parse them...  And with this,
they would have to parse the dictionary on *every message received*.

Again, I believe that 99% of applications will *not* be interested
in the specifics of the sunscription/group address info except to 
differentiate between Unicast and non-Unicast addressing.

That is why I am in favor of *non* complex arguments on the
MessageReceived() method.  An additional daemon method that returns
a dictionary only when asked (which even with scenes will be rare or
only once a session per subscription) will create less overall dbus
traffic.

> 
> Michal, any comments?
> 
> > > Then it makes sense to add model subscription array as a dictionary
> > > entry in the UpdateModelConfiguration() as well as for the node
> > > configuration returned when calling Attach() method.
> > > Probably will have to have separate keys: "Groups" and "Virtuals".
> > > 
> 
> Regards,
> Inga
> 
> 

Re: mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[Stotland, Inga]

[-- Attachment #1: Type: text/plain, Size: 7909 bytes --]

Hi Brian,

On Thu, 2019-09-26 at 23:48 +0000, Gix, Brian wrote:
> Hi Inga,
> 
> On Thu, 2019-09-26 at 20:41 +0000, Stotland, Inga wrote:
> > Hi Brian, Michal,
> > 
> > Pairing down the cc list since we are down to implementation
> > details.
> > 
> > On Thu, 2019-09-26 at 15:18 +0000, Gix, Brian wrote:
> > > Hi Inga, Michał,
> > > 
> > > On Wed, 2019-09-25 at 19:02 +0000, Stotland, Inga wrote:
> > > > Hi Michal,
> > > > 
> > > > On Wed, 2019-09-18 at 10:52 +0200, Michał Lowas-Rzechonek
> > > > wrote:
> > > > > Hi Brian,
> > > > > 
> > > > > > Imagine a dot-matrix, where each pixel is a mesh node.
> > > > > > 
> > > > > > Each of these pixels implements two models:
> > > > > >     on element 0, a GenericOnOffServer controlling the
> > > > > > light
> > > > > > output
> > > > > >     on element 1, a Blinkenlights Server model
> > > > > > 
> > > > > > Blinkenlights Server extends GenericOnOff Server and
> > > > > > GenericOnOff
> > > > > > Client, and on top of that contains a translation table
> > > > > > mapping
> > > > > > group
> > > > > > address to either 'ON' or 'OFF'.
> > > > > > 
> > > > > > Now, when Blinkenlights Server receives a GenericOnOff Set
> > > > > > message,
> > > > > > it
> > > > > > looks up the destination address at the translation table,
> > > > > > and
> > > > > > sends a
> > > > > > *different* GenericOnOff Set to *its own* element 0, with
> > > > > > target
> > > > > > value
> > > > > > determined by the translation entry.
> > > > > > 
> > > > > > This allows users to configure each node in such a way,
> > > > > > that
> > > > > > sending a
> > > > > > *single* message to a group address causes all pixels to
> > > > > > switch
> > > > > > to
> > > > > > a
> > > > > > preconfigured pattern *at the same time*.
> > > > > 
> > > > > Per conversation with Piotr, I'd like to revisit the
> > > > > discussion
> > > > > and
> > > > > provide more details about our use case for models knowing
> > > > > the
> > > > > destination address.
> > > > > 
> > > > > Please see a diagram at http://ujeb.se/BmTIW.
> > > > > 
> > > > > The main reason we map scenes using destination addresses is
> > > > > that
> > > > > such a
> > > > > setup consumes much less unicast addresses.
> > > > > 
> > > > > Assuming that:
> > > > >  S - number of switches
> > > > >  B - number of buttons (elements) on a switch
> > > > >  N - nunber of lamps
> > > > > 
> > > > > With a 'regular' case, number of consumed unicast addresses
> > > > > is
> > > > >     S*B + N*(B+1)
> > > > > 
> > > > > With the destination mapping, it becomes
> > > > >     S*B + N*2
> > > > > 
> > > > > Since we typically use 4 button switches (B=4), without
> > > > > translation
> > > > > we
> > > > > consume unicast address space at a *much* faster rate.
> > > > > 
> > > > > reagrds
> > > > 
> > > > Okay, this is a good argument for exposing the subscription
> > > > address
> > > > in
> > > > MessageReceived().
> > > > It's better to separate the method into two, e.g.
> > > > MessageReceived()
> > > > and
> > > > MessageReceivedVirtual().
> > > 
> > > I wonder if we could still do this with a single method.  I can
> > > think
> > > of 2 methodologies:
> > > 
> > > 1. A simple way that just uses the U16 DST field instead of
> > > the "subscription" boolean (not a 100% reliable differentiator
> > > for Virtuals, but may be sufficient for the use cases given).
> > > 
> > > 2. Replacing the subscription boolean with a u32 "Subscription
> > > ID".
> > > A subscription ID value of 0x000000000 would indicate that the
> > > message was received with the Unicast address, and values from
> > > 1 - 0xFFFFFFFF mean a Subscription that can be queried for. This
> > > would be accompanied by a new daemon method which could look up
> > > the details of the subscription:
> > > 
> > > {dict subcription}  LookupSubscription(u32 Sub_ID)
> > > 
> > > Both of these methodologies would allow an App to be simpler,
> > > with no added D-Bus Methods required.
> > > 
> > > With the 2nd methodology, the subscription only needs to be
> > > looked up once (or not at all) to 100% differentiate between
> > > discrete subscriptions.
> > > 
> > > I *really* do not want an additional mandatory App Method. Most
> > > Apps will be simpler than that, and truely not care to
> > > differentiate between subscriptions...   Particularily Client
> > > based Apps.
> > 
> > While I am still in favor of two distinct methods (given choice,
> > I'd
> > always go with self-explanatory API), one method approach would
> > work as
> > well if accompanied by detailed and clear description in the mesh-
> > api.txt doc.
> > 
> > I vote against u16 destination field since there is no reason to
> > create address space collision even though the chances are small.
> > 
> > A single method "MessageReceived" method can be modified to include
> > a
> > subscription parameter as:
> > 1) a dictionary with keys "Group" and "Label" (self explanatory if
> > a
> > bit cumbersome).
> 
> If this use case is chosen, is it easy enough for an App
> implementation
> of this method to *ignore* the dictionary if the subscription
> information
> is not important to that model? (Again:  Thinking of simple
> applications
> that just want to know whether to respond to the message or not
> because
> it was or wasn't sent to the Unicast address)
> 
> > or
> > 2) a u32 subscription ID that represents a subscription. This
> > requires the daemon to maintain the relationship between "id" and
> > the
> > actual address. I believe the daemon does this anyway for virtual
> > labels, but from the API design perspective this is not very clean
> > IMHO, since it has a whiff of implementation details leaking into
> > user
> > interface API.
> > 
> > No matter which approach is chosen, the subscriptions must be
> > included
> > in the model configuration dictionary for
> > UpdateModelConfiguration()
> > and in the corresponding dictionary for node configuration returned
> > on
> > successful execution of Attach().
> > 
> > If we go with a single method and a u32 subscription id, the
> > disctionary representation of the subscriptions should be a pair
> > {id,
> > actual subscription address}. This way there is no need for an
> > additional daemon method.
> 
> We have avoided dictionaries specifically to avoid mandating that
> Applications need to have logic to parse them...  And with this,
> they would have to parse the dictionary on *every message received*.
> 
> Again, I believe that 99% of applications will *not* be interested
> in the specifics of the sunscription/group address info except to
> differentiate between Unicast and non-Unicast addressing.
> 
> That is why I am in favor of *non* complex arguments on the
> MessageReceived() method.  An additional daemon method that returns
> a dictionary only when asked (which even with scenes will be rare or
> only once a session per subscription) will create less overall dbus
> traffic.
> 
The application is already dealing with dictionaries for the
configuration update.
I strongly feel that subscription addresses (since they are being
exposed in MessageReceived() one way or the other) should be in the
configuration dictionaries.

Also, if an app is not interested in subscripion, it may ignore the
subscription parameter, doesn't matter what form it takes.

> > Michal, any comments?
> > 
> > > > Then it makes sense to add model subscription array as a
> > > > dictionary
> > > > entry in the UpdateModelConfiguration() as well as for the node
> > > > configuration returned when calling Attach() method.
> > > > Probably will have to have separate keys: "Groups" and
> > > > "Virtuals".
> > > > 
> > 
> > Regards,
> > Inga
> > 
> > 

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3265 bytes --]

Re: mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[michal.lowas-rzechonek]

Inga, Brian,

On 09/26, Stotland, Inga wrote:
> While I am still in favor of two distinct methods (given choice, I'd
> always go with self-explanatory API)

So do I.

> I vote against u16 destination field since there is no reason to
> create address space collision even though the chances are small.
> 
> A single method "MessageReceived" method can be modified to include a
> subscription parameter as:
> 1) a dictionary with keys "Group" and "Label" (self explanatory if a
> bit cumbersome).

If we really need to avoid two separate methods, I think it would be a
bit cleaner to pass this parameter as a D-Bus variant of (u16,
array[16]) instead of a dictionary.

Still, even if we add a method, the application is free *not* to
implement it, since we agreed back in the day that calls to
MessageReceived do not require a response, so any errors would be simply
ignored by the daemon.

> 2) a u32 subscription ID that represents a subscription. This 
> requires the daemon to maintain the relationship between "id" and the
> actual address. I believe the daemon does this anyway for virtual
> labels, but from the API design perspective this is not very clean
> IMHO, since it has a whiff of implementation details leaking into user
> interface API. 

I am very much against adding any kind of traslation here. I think that
maintaining subscription IDs would only complicate things, with no
additional benefit. I think it would also confuse users, as there is no
such thing as 'subscription id' in the spec. Mesh is already complex as
it is.

> No matter which approach is chosen, the subscriptions must be included
> in the model configuration dictionary for UpdateModelConfiguration()
> and in the corresponding dictionary for node configuration returned on
> successful execution of Attach().
>
> > > Then it makes sense to add model subscription array as a dictionary
> > > entry in the UpdateModelConfiguration() as well as for the node
> > > configuration returned when calling Attach() method.
> > > Probably will have to have separate keys: "Groups" and "Virtuals".

Indeed, and we also have two options here:
 - if we decide to provide two MessageReceived methods, I would go with separate
   keys
 - if we go with the variant approach proposed above, I would also
   return an array of variants here

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

Re: mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[Gix, Brian]

Hi Michał, Inga,

On Fri, 2019-09-27 at 10:52 +0200, michal.lowas-rzechonek@silvair.com wrote:
> Inga, Brian,
> 
> On 09/26, Stotland, Inga wrote:
> > While I am still in favor of two distinct methods (given choice, I'd
> > always go with self-explanatory API)
> 
> So do I.
> 
> > I vote against u16 destination field since there is no reason to
> > create address space collision even though the chances are small.
> > 
> > A single method "MessageReceived" method can be modified to include a
> > subscription parameter as:
> > 1) a dictionary with keys "Group" and "Label" (self explanatory if a
> > bit cumbersome).
> 
> If we really need to avoid two separate methods, I think it would be a
> bit cleaner to pass this parameter as a D-Bus variant of (u16,
> array[16]) instead of a dictionary.

Here is an interesting idea...

Is it possible to use a variable sized array for the address?

0 octets -- Destination is the Unicast address
2 octets -- Destination is a network byte ordere (Big Endian) u16
16 octets -- Destination is a 128bit Virtual label

The caveat here is that there would be two variable octet arrays in the message,
but from a D-Bus efficiency standpoint, might be the best we are going to do.

> 

Re: mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[Gix, Brian]

On Fri, 2019-09-27 at 10:52 +0200, michal.lowas-rzechonek@silvair.com wrote:
> Inga, Brian,
> 
> Still, even if we add a method, the application is free *not* to
> implement it, since we agreed back in the day that calls to
> MessageReceived do not require a response, so any errors would be simply
> ignored by the daemon.

This is not an option.

A node does not get to decide which susbscriptions are "valid".  If a Virtual Address subscription is added to
a node, and then a message is sent to that virtua address, the App needs to be able to receive it.

Yes, any discrete message may be lost, but I don't think we have the option of letting *all* virtual addressed
messages to an App to be ignored.  If we add an App API, it will need to be mandatory, which is why I am
against it.

I strongly believe we need:

1. A single method for delivering (non dev key) received messages
2. A method that does not require dictionary parsing

How are we feeling about:
	void MessageReceived(uint16 source, uint16 key_index,
				array{byte} destination, array{byte} data)


Where destination length of:
	0 - Unicast address of element
	2 - Group Address
	16 - Variable Label

I think this fulfills all of our requirements.

Re: mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[Stotland, Inga]

[-- Attachment #1: Type: text/plain, Size: 1886 bytes --]

Hi Brian, 

On Fri, 2019-09-27 at 15:50 +0000, Gix, Brian wrote:
> On Fri, 2019-09-27 at 10:52 +0200, michal.lowas-rzechonek@silvair.com
>  wrote:
> > Inga, Brian,
> > 
> > Still, even if we add a method, the application is free *not* to
> > implement it, since we agreed back in the day that calls to
> > MessageReceived do not require a response, so any errors would be
> > simply
> > ignored by the daemon.
> 
> This is not an option.
> 
> A node does not get to decide which susbscriptions are "valid".  If a
> Virtual Address subscription is added to
> a node, and then a message is sent to that virtua address, the App
> needs to be able to receive it.
> 
> Yes, any discrete message may be lost, but I don't think we have the
> option of letting *all* virtual addressed
> messages to an App to be ignored.  If we add an App API, it will need
> to be mandatory, which is why I am
> against it.
> 
> I strongly believe we need:
> 
> 1. A single method for delivering (non dev key) received messages
> 2. A method that does not require dictionary parsing
> 
> How are we feeling about:
> void MessageReceived(uint16 source, uint16 key_index,
> array{byte} destination, array{byte} data)
> 
> 
> Where destination length of:
> 0 - Unicast address of element
> 2 - Group Address
> 16 - Variable Label
> 
> I think this fulfills all of our requirements.
> 

For a single MessageReceived() method, the cleanest way is to have the
subscription address parameter as a variant (suggested by Michal) or as
a dictionary.
An array introduces an extra consideration of byte ordering for group
addresses.

What I mostly about is that the represenation of the subscription
address in the MessageReceived() method corresponds to the
representation in the configuration dictionaries for the Attach() and
UpdateModelConfiguration() methods.
 

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3265 bytes --]

Re: mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[Gix, Brian]

On Fri, 2019-09-27 at 17:25 +0000, Stotland, Inga wrote:
> Hi Brian, 
> 
> On Fri, 2019-09-27 at 15:50 +0000, Gix, Brian wrote:
> > On Fri, 2019-09-27 at 10:52 +0200, michal.lowas-rzechonek@silvair.com
> >  wrote:
> > > Inga, Brian,
> > > 
> > > Still, even if we add a method, the application is free *not* to
> > > implement it, since we agreed back in the day that calls to
> > > MessageReceived do not require a response, so any errors would be
> > > simply
> > > ignored by the daemon.
> > 
> > This is not an option.
> > 
> > A node does not get to decide which susbscriptions are "valid".  If a
> > Virtual Address subscription is added to
> > a node, and then a message is sent to that virtua address, the App
> > needs to be able to receive it.
> > 
> > Yes, any discrete message may be lost, but I don't think we have the
> > option of letting *all* virtual addressed
> > messages to an App to be ignored.  If we add an App API, it will need
> > to be mandatory, which is why I am
> > against it.
> > 
> > I strongly believe we need:
> > 
> > 1. A single method for delivering (non dev key) received messages
> > 2. A method that does not require dictionary parsing
> > 
> > How are we feeling about:
> > void MessageReceived(uint16 source, uint16 key_index,
> > array{byte} destination, array{byte} data)
> > 
> > 
> > Where destination length of:
> > 0 - Unicast address of element
> > 2 - Group Address
> > 16 - Variable Label
> > 
> > I think this fulfills all of our requirements.
> > 
> 
> For a single MessageReceived() method, the cleanest way is to have the
> subscription address parameter as a variant (suggested by Michal) or as
> a dictionary.
> An array introduces an extra consideration of byte ordering for group
> addresses.

If variants are easy to sort in scripting languages like python3, than I
suppose I could live with this. Would these work like C++ overloading?
Or is this still a two step process of:
1. Determining the u16 vs u128
1.1 unmarshalling the correct type...

What would the signature of the method look like?

> 
> What I mostly about is that the represenation of the subscription
> address in the MessageReceived() method corresponds to the
> representation in the configuration dictionaries for the Attach() and
> UpdateModelConfiguration() methods.

Configuration dictionaries can generally be ignored by simple Apps, so
Dictionary parsing is always optional for Attach() and 
UpdateModelConfiguration(). However even simple Apps need to be able 
to tell the difference between Unicast and Non-Unicast destinations
for receptions, because the rules for responding are generally different...
Which is why the current method has the simple boolean.

Re: mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[michal.lowas-rzechonek]

Brian, Inga,

On 09/27, Gix, Brian wrote:
> > For a single MessageReceived() method, the cleanest way is to have the
> > subscription address parameter as a variant (suggested by Michal) or as
> > a dictionary.
> > An array introduces an extra consideration of byte ordering for group
> > addresses.
> 
> If variants are easy to sort in scripting languages like python3, than I
> suppose I could live with this.

Last time I checked, it was C that had issues with  any type of
'dynamic' typing, so I wouldn't worry about client languages. Handling
D-Bus variants in python is trivial.

> Or is this still a two step process of:
> 1. Determining the u16 vs u128
> 1.1 unmarshalling the correct type...
> 
> What would the signature of the method look like?

Something like this:

	void MessageReceived(uint16 source, uint16 key_index,
					 variant destination, array{byte} data)

and on the Python side, the handling would look somewhat like this
(since Python doesn't do overloading, at all):

class ElementInterface:
	def MessageReceived(source, key_index, destination, data):
	    if type(destination) == dbus.types.UINT16:
	        group_destination = destination
	    elif type(destination) == dbus.types.ARRAY:
	        virtual_destination = uuid.UUID(destination)
	    else:
	        raise DBusError('Unrecoenized destination type')

> > What I mostly about is that the represenation of the subscription
> > address in the MessageReceived() method corresponds to the
> > representation in the configuration dictionaries for the Attach() and
> > UpdateModelConfiguration() methods.

Agreed.

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

Re: mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[Stotland, Inga]

[-- Attachment #1: Type: text/plain, Size: 1923 bytes --]

Hi Michal, Brian,
On Mon, 2019-09-30 at 09:18 +0200, michal.lowas-rzechonek@silvair.com
wrote:
> Brian, Inga,
> 
> On 09/27, Gix, Brian wrote:
> > > For a single MessageReceived() method, the cleanest way is to
> > > have the
> > > subscription address parameter as a variant (suggested by Michal)
> > > or as
> > > a dictionary.
> > > An array introduces an extra consideration of byte ordering for
> > > group
> > > addresses.
> > 
> > If variants are easy to sort in scripting languages like python3,
> > than I
> > suppose I could live with this.
> 
> Last time I checked, it was C that had issues with  any type of
> 'dynamic' typing, so I wouldn't worry about client languages.
> Handling
> D-Bus variants in python is trivial.
> 
> > Or is this still a two step process of:
> > 1. Determining the u16 vs u128
> > 1.1 unmarshalling the correct type...
> > 
> > What would the signature of the method look like?
> 
> Something like this:
> 
> 	void MessageReceived(uint16 source, uint16 key_index,
> 					 variant destination,
> array{byte} data)
> 
> and on the Python side, the handling would look somewhat like this
> (since Python doesn't do overloading, at all):
> 
> class ElementInterface:
> 	def MessageReceived(source, key_index, destination, data):
> 	    if type(destination) == dbus.types.UINT16:
> 	        group_destination = destination
> 	    elif type(destination) == dbus.types.ARRAY:
> 	        virtual_destination = uuid.UUID(destination)
> 	    else:
> 	        raise DBusError('Unrecoenized destination type')
> 
> > > What I mostly about is that the represenation of the subscription
> > > address in the MessageReceived() method corresponds to the
> > > representation in the configuration dictionaries for the Attach()
> > > and
> > > UpdateModelConfiguration() methods.
> 
> Agreed.
> 


I am fine with this version.
Regards,
Inga

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3265 bytes --]

RE: mesh: org.bluez.mesh.Element.MessageReceived method does not provide destination address

[Gix, Brian]

Hi Michał,

Can you submit a patch set with these agreed changes, that includes:
* doc/mesh-api.txt changes
* Source code changes
* test/test-mesh changes that conform to new API


On 30-Sep-2019 Michał wrote:
> > For a single MessageReceived() method, the cleanest way is to have 
> > the subscription address parameter as a variant (suggested by 
> > Michal) or as a dictionary.
> > An array introduces an extra consideration of byte ordering for 
> > group addresses.
> 
> If variants are easy to sort in scripting languages like python3, than 
> I suppose I could live with this.

Last time I checked, it was C that had issues with  any type of 'dynamic' typing, so I wouldn't worry about client languages. Handling D-Bus variants in python is trivial.

> Or is this still a two step process of:
> 1. Determining the u16 vs u128
> 1.1 unmarshalling the correct type...
> 
> What would the signature of the method look like?

Something like this:

	void MessageReceived(uint16 source, uint16 key_index,
					 variant destination, array{byte} data)

and on the Python side, the handling would look somewhat like this (since Python doesn't do overloading, at all):

class ElementInterface:
	def MessageReceived(source, key_index, destination, data):
	    if type(destination) == dbus.types.UINT16:
	        group_destination = destination
	    elif type(destination) == dbus.types.ARRAY:
	        virtual_destination = uuid.UUID(destination)
	    else:
	        raise DBusError('Unrecoenized destination type')

> > What I mostly about is that the represenation of the subscription 
> > address in the MessageReceived() method corresponds to the 
> > representation in the configuration dictionaries for the Attach() 
> > and
> > UpdateModelConfiguration() methods.

Agreed.