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:   Wed, 17 Jul 2019 21:47:12 +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>,
        "jakub.witowski@silvair.com" <jakub.witowski@silvair.com>,
        "Gix, Brian" <brian.gix@intel.com>
Subject: Re: [PATCH BlueZ v5 1/4] mesh: Add ImportLocalNode API documentation
Message-ID: <20190717194712.i4dtiwhldq2is2z2@kynes>
Mail-Followup-To: "Stotland, Inga" <inga.stotland@intel.com>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        "jakub.witowski@silvair.com" <jakub.witowski@silvair.com>,
        "Gix, Brian" <brian.gix@intel.com>
References: <20190717083650.26346-1-michal.lowas-rzechonek@silvair.com>
 <20190717083650.26346-2-michal.lowas-rzechonek@silvair.com>
 <915ea1c10883aaf1e4d42c5a749bfda964b54b51.camel@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <915ea1c10883aaf1e4d42c5a749bfda964b54b51.camel@intel.com>
User-Agent: NeoMutt/20180716
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hello,

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.

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.

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

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

I've implemented this validation method on top of this patch-set. I'll
send it as RFC shortly.

regards
-- 
Michał Lowas-Rzechonek <michal.lowas-rzechonek@silvair.com>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND