Re: [PATCH 0/3] Allow to reattach with new composition data

From: "jakub.witowski@silvair.com" <jakub.witowski@silvair.com>
To: "Gix, Brian" <brian.gix@intel.com>
Cc: "linux-bluetooth@vger.kernel.org"
	<linux-bluetooth@vger.kernel.org>,
	"Stotland, Inga" <inga.stotland@intel.com>
Subject: Re: [PATCH 0/3] Allow to reattach with new composition data
Date: Tue, 21 Jan 2020 11:59:27 +0100	[thread overview]
Message-ID: <20200121105927.s2a3avh7tdcnf566@jwitowski2365> (raw)
In-Reply-To: <56b3aa902dee18a45e91d826344085cf1fb3ecca.camel@intel.com>

On 01/20, Gix, Brian wrote:
> Hi Jakub,
> 
> a few things..
> 
> First, the subject line for user space patches should always be [BlueZ PATCH...] to differentiate bluez.git
> patches from kernel patches.
Correct, sorry for that.

> 
> Also:  The title of the patch should always start with the component being modified... in this case 
> "mesh: Allow reattach..." or "tools/mesh: XXXX" for example
> 
Ok

> On Mon, 2020-01-20 at 17:11 +0100, Jakub Witowski wrote:
> > This patch allows the application to modify the CID, PID, VID and CRPL in the composition data.
> 
> This will require some discussion.   Currently we are planning at *least* to make CRPL entirely under the
> control of bluetooth-mesh.conf, because this has a significant daemon impact.

Ok, I don't have a problem with that. I guess this is going to behave in
the same way as feature bits, and CRPL will disappear from Application1
properties altogether?

> 
> The other settings I am not as concerned about...  If the spec really does allow their alteration though, I am
> willing to consider them.
> 

It doesn't explicitly allow the update, but doesn't forbid it
either... These values are application properties, and particularly the
VID can change after software upgrade, (even if element layout doesn't,
that would require re-provisioning etc.). Supporting this is the main
use case.

> > According the Mesh Profile (2.3.4 Elements) the modification of fields other than "Elements" is not
> > prohibited.
> > 
> > Also in my opinion (as you can see in the 1st patch), there is no need to use pointer to the node_composition
> > struct.
> > The static is more clear and less problematic.
> > 
> > Jakub Witowski (3):
> >   mesh: use static node_comp instead of the pointer
> >   mesh: add composition data setter
> >   mesh: allow to reattach with new composition data
> > 
> >  mesh/mesh-config-json.c | 46 +++++++++++++++-----
> >  mesh/mesh-config.h      |  2 +
> >  mesh/node.c             | 96 +++++++++++++++++++++++++----------------
> >  3 files changed, 96 insertions(+), 48 deletions(-)
> > 