RE: [PATCH BlueZ v5 1/4] mesh: Add ImportLocalNode API documentation

["Gix, Brian" <brian.gix@intel.com>]

Hi Inga & Michal,

> On 07/17, Stotland, Inga wrote:
> > I feel like having "object app_root" is unnecessary and also, creates
> > some gnarly pathways within the code.
> >
> > What exactly is the problem for requiring the composition data to be
> > part of the import_data? It's just weird to say "oh, it may be there
> > or it may be not".
> 
> The main issue lies on the application side. In order to properly Attach(), the
> application must expose element structure via D-Bus.

I understand this concern, but I think I am agreeing with Inga on this one.

At the time of inception, Not only the Prov Data (net key, net idx, iv index, flags, unicast, dev key) must be present, but also so must the "Composition Data"...  all of the data which is returned by the "GET COMPOSITION DATA" opcode.  This data needs to be present as soon as the Provisioner starts querying and setting states on the (new or migrated) fresh node.

An important point to remember here is that *the application does not need to be attached" while the remote Provisioner/ConfigClient is interacting with the nodes Config Server.  One of our Design Goals, in fact, was that the node always exists on the network, regardless of whether the App is attached...  This is why the Config Server model is part of the daemon....   It is always "OnLine".

For this reason, I am supporting Inga here, that the JSON input file should contain everything Jakub has included:
Dev Key
Net Key
Net Index
IV Index
Flags
Unicast 

*And* it should include the pure Composition Data including
CID/VID/PID  (optional .. Can be set with BlueZ defaults)
CRPL  (optional ...  Can be set with internal defaults)
Features (optional ...  missing are created as either "Unsupported" or "Disabled")
array of Elements, containing array of Models. (Mandatory ... Needed for Cfg Server to function)

The application has no need to even exist at this point, as long as it can attach to the token at some point in the future.
But this *does* enable the ability to have a *generic* application that can inject nodes (fully configured, or "New") into the daemon.

But worries that the App might not immediately be able to "Attach" go away.



> If we say that it must also do the same via JSON, to call ImportLocalNode, it
> leads to code duplication on the application side.
> 
> Moreover, the app still needs to be queried via D-Bus to check that the passed
> JSON matches the D-Bus structure - otherwise the app would then fail to
> Attach() and the user would be in deep trouble.

The composition as reflected by the GetManagedObjects() call is "sanity checked" against the internal storage *every* time the App attaches...  I think Inga is concerned with code complexity and bloat to repeat this during ImportLocalNode(), simply as a "second way" attain the composition data.  This is different from the Join() case, in my opinion, where the JSON (or other storage) is being created *totally* from scratch, via the provisioning interaction with the remote Provisioner. In that case, yes... the "owning application" needs to be present on the D-Bus.

 
> > Getting rid of the app_root and mandating the composition to be part
> > of the import_data allows:
> >
> > 1) Avoid checking whether this is a "full" configuration or the
> > "minimal" one
> 
> I'm not convinced that the "full" configuration is even needed. We certaintly
> don't use it in our use case, but it might be required in the future.

We *definitely* need an option for importing/migrating a fully configured node.  Phones are retired and replaced... Workstations are retired and replaced...  Some nodes publications will inevitably need to pick up the "current conversation" with the migrated node, where the old conversation left off. And this will almost certainly be a rare (but important) operation, and a "Utility" application to perform the operation (that does not itself need to have the model/element arrays implemented) will be easier to write and maintain.


> 
> > 2) Efficiently re-use the existing code:
> > Adding an API call like this one may be sufficient
> >
> > mesh_config_import(const char *cfg_dir,
> >                    const uint8_t uuid[16],
> >                    const uint8 *import_data, <import__len>?,
> >                    mesh_config_node_func_t cb,
> >                    void *user_data)
> >
> > We can just re-factor the code that parses and populates a single node
> > from the stored configuration. user_data may contain whatever we need
> > to preserve in order to respond to d-bus call.
> 
> After refactoring node validation to byte-compare composition data, the code
> also becomes significantly simpler, and execution paths for Join(), Attach(),
> CreateNetwork() and ImportLocalNode() converge.

Again, I cannot think of any situations where Join/Attach/Create would ever exist in the absence of the Application.

This is an easy and obvious use case with Import.

--Brian