DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2266E2089A
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 12.1 \(3445.101.1\))
Subject: Re: [PATCH BlueZ] doc: Initial Bluetooth Mesh API
From:   Johan Hedberg <johan.hedberg@gmail.com>
In-Reply-To: <20181107004950.9153-1-inga.stotland@intel.com>
Date:   Thu, 8 Nov 2018 12:45:00 +0200
Cc:     linux-bluetooth@vger.kernel.org,
        Marcel Holtmann <marcel@holtmann.org>, brian.gix@intel.com
Content-Transfer-Encoding: quoted-printable
Message-Id: <4DBB63ED-41C4-4F27-B2BC-1ADB090B1656@gmail.com>
References: <20181107004950.9153-1-inga.stotland@intel.com>
To:     Inga Stotland <inga.stotland@intel.com>
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Inga,

At first glance this looks quite good to me, just some cosmetic =
comments:

> On 7 Nov 2018, at 2.49, Inga Stotland <inga.stotland@intel.com> wrote:
> +Service	org.bluez.mesh1

I don=E2=80=99t think we=E2=80=99ve had the habit of versioning =
well-known bus names so far. Or did you see this somewhere? So I=E2=80=99d=
 just leave away the version here.

> +Interface	org.bluez.mesh1.Network

The way the existing interfaces do versioning is to have the version at =
the very end, so this should be org.bluez.mesh.Network1

> +Object path	/org/bluez/mesh1

I don=E2=80=99t think we have the habit of versioning object paths =
either. They=E2=80=99re anyway (for the most part) irrelevant - it=E2=80=99=
s the interfaces an object has that defines its type.


> +			PossibleErrors:
> +				org.bluez.mesh1.Error.Failed,
> +				org.bluez.mesh1.Error.Canceled,
> +				org.bluez.mesh1.Error.InvalidArguments
> +				org.bluez.mesh1.Error.Timeout

We=E2=80=99ve never versioned errors either, or did you see this done =
somewhere? So just leave it out from here.

> +		(object node, array{byte, array{(uint16, dict}} =
configuration) Attach(
> +									=
object app_defined_root, uint64 token)
>=20

This is a bit hard to read due to the long & complex return value. I=E2=80=
=99d suggest doing the line break before =E2=80=9CAttatch=E2=80=9D (a =
common C coding-style as well).

> +		void Send(object element_path, uint16_t destination, =
uint16 key_index,
> +								=
array{byte} data)

Minor inconsistency: should be uint16 instead of uint16_t

Johan=