From: Johan Hedberg <johan.hedberg@gmail.com>
To: Inga Stotland <inga.stotland@intel.com>
Cc: linux-bluetooth@vger.kernel.org,
Marcel Holtmann <marcel@holtmann.org>,
brian.gix@intel.com
Subject: Re: [PATCH BlueZ] doc: Initial Bluetooth Mesh API
Date: Thu, 8 Nov 2018 12:45:00 +0200 [thread overview]
Message-ID: <4DBB63ED-41C4-4F27-B2BC-1ADB090B1656@gmail.com> (raw)
In-Reply-To: <20181107004950.9153-1-inga.stotland@intel.com>
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’t think we’ve had the habit of versioning well-known bus names so far. Or did you see this somewhere? So I’d 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’t think we have the habit of versioning object paths either. They’re anyway (for the most part) irrelevant - it’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’ve 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)
>
This is a bit hard to read due to the long & complex return value. I’d suggest doing the line break before “Attatch” (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
next prev parent reply other threads:[~2018-11-08 10:45 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-07 0:49 [PATCH BlueZ] doc: Initial Bluetooth Mesh API Inga Stotland
2018-11-08 10:45 ` Johan Hedberg [this message]
2018-11-08 18:46 ` Stotland, Inga
2018-11-10 19:50 ` Johan Hedberg
2018-11-11 10:07 ` Marcel Holtmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4DBB63ED-41C4-4F27-B2BC-1ADB090B1656@gmail.com \
--to=johan.hedberg@gmail.com \
--cc=brian.gix@intel.com \
--cc=inga.stotland@intel.com \
--cc=linux-bluetooth@vger.kernel.org \
--cc=marcel@holtmann.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).