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
> > 
> >