Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 27 Sep 2019 10:52:08 +0200
From:   "michal.lowas-rzechonek@silvair.com" 
        <michal.lowas-rzechonek@silvair.com>
To:     "Stotland, Inga" <inga.stotland@intel.com>
Cc:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        "Gix, Brian" <brian.gix@intel.com>
Subject: Re: mesh: org.bluez.mesh.Element.MessageReceived method does not
 provide destination address
Message-ID: <20190927085208.sxy2x5656ci3opo6@mlowasrzechonek2133>
Mail-Followup-To: "Stotland, Inga" <inga.stotland@intel.com>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        "Gix, Brian" <brian.gix@intel.com>
References: <20190830184301.zd3zaqrw7mv6r252@kynes>
 <20190918085239.xhahxoeqjkcrk3bl@mlowasrzechonek2133>
 <3c5858c94b3e08a61c5ff8493f9b00f5f77d0aac.camel@intel.com>
 <4b57242b15ccb44ac123858ce7bdf6e0e526b3e0.camel@intel.com>
 <a0442c7485fef2bfde9bc52283a5c9dfc0b43515.camel@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <a0442c7485fef2bfde9bc52283a5c9dfc0b43515.camel@intel.com>
User-Agent: NeoMutt/20180716
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

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