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

["Stotland, Inga" <inga.stotland@intel.com>]

Hi Jakub,

On Tue, 2020-01-21 at 11:59 +0100, jakub.witowski@silvair.com wrote:
> 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?

I think that the value of CRPL in the config keyfile should correspond
to the maximum CRPL value that the node supports. If an application
specifies anything greater than config value, it will be capped.

As far as changing the value of CRPL dynamically (i.e., in app's
properties), we may allow it, but *only* if the new CRPL value is
greater than the previously specified one (again, subject to be capped
at max).

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

I would strongly prefer not to allow changing CID. Is there a good
reason to change CID ever?
Changing product ID and especially version ID ahould be fine.

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

Agree

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

Best regards,
Inga