Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201029212336.3283410-1-danielwinkler@google.com>
In-Reply-To: <20201029212336.3283410-1-danielwinkler@google.com>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Thu, 29 Oct 2020 15:02:44 -0700
Message-ID: <CABBYNZKN1em3O=WFRNGtmhR2yJ79=7Qa16ym-EzUMKdt=k5naw@mail.gmail.com>
Subject: Re: [Bluez PATCH v5 00/10] Bluetooth: Add new MGMT interface for
 advertising add
To:     Daniel Winkler <danielwinkler@google.com>
Cc:     Luiz Augusto Von Dentz <luiz.von.dentz@intel.com>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        ChromeOS Bluetooth Upstreaming 
        <chromeos-bluetooth-upstreaming@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Daniel,

On Thu, Oct 29, 2020 at 2:27 PM Daniel Winkler <danielwinkler@google.com> w=
rote:
>
> Hi Maintainers,
>
> This patch series defines the new two-call MGMT interface in userspace
> for adding advertising instances. Bluez will detect if kernel supports
> the new MGMT commands, and use them if so. Each new advertising instance
> will be configured by a MGMT call to set advertising parameters,
> followed by a MGMT call to set advertising data. The new data pipeline
> is meant to be unnoticeable from the clients' perspective, with the
> exception of new intervals and tx power support, and new exposed
> advertising manager properties.
>
> All changes have been tested on hatch (extended advertising) and kukui
> (no extended advertising) chromebooks with manual testing verifying
> correctness of parameters/data in btmon traces, and our automated test
> suite of 25 single- and multi-advertising usage scenarios.
>
> V2 of the series puts documentation at the front as requested.
>
> Thank you in advance for your review!
> Daniel Winkler
>
>
> Changes in v5:
> - Changed interval API from jiffies to milliseconds for clarity
> - Changed new dbus endpoints to be experimental
> - Only parse new properties if experimental flag is set
> - Verify capabilities MGMT command is supported before calling it
> - Make SupportedCapabilities experimental
>
> Changes in v4:
> - mgmt-api: moved optional params into 'flags' field
> - mgmt-api: added info for new return parameters for max data size
> - mgmt-api: Move Controller Capabilities MGMT doc into new patch
> - mgmt-api: Tx Power range put into single capabilities entry
> - Moved optional params into flags field
> - Use returned max data length in adv data length check
> - Move tx power range into single capability field
>
> Changes in v3:
> - Removed Tx Power Selected MGMT event
> - Changed Read Security Info cmd to  Read Controller Capabilities
> - Added selected tx power to MGMT params response
> - Removed Tx Power Selected MGMT event from monitor
>
> Changes in v2:
> - Removed extra space in Add Extended Advertising Parameters API
> - Uses btd_has_kernel_features to detect kernel command support
> - Cleaned fail path in add_adv_params_callback
>
> Daniel Winkler (10):
>   doc/advertising-api: update API with new interface
>   doc/mgmt-api: Add new Advertising MGMT interfaces to mgmt-api
>   doc/mgmt-api: Update controller capabilities MGMT command in mgmt-api
>   advertising: Detect if extended advertising mgmt commands are
>     supported
>   advertising: Parse intervals and tx power from adv
>   advertising: Use new mgmt interface for advertising add
>   advertising: Query LE TX range at manager initialization
>   advertising: Expose SupportedCapabilities for advertising
>   client: Add SupportedCapabilities to bluetoothctl
>   monitor: Add new MGMT adv commands and events to monitor
>
>  client/main.c           |   1 +
>  doc/advertising-api.txt |  48 +++++
>  doc/mgmt-api.txt        | 238 +++++++++++++++++++++-
>  lib/mgmt.h              |  46 ++++-
>  monitor/packet.c        |  69 ++++++-
>  src/adapter.c           |   8 +
>  src/adapter.h           |   2 +
>  src/advertising.c       | 428 ++++++++++++++++++++++++++++++++++++++--
>  tools/btmgmt.c          |  12 +-
>  9 files changed, 819 insertions(+), 33 deletions(-)
>
> --
> 2.29.1.341.ge80a0c044ae-goog

[detached HEAD 1cfa60010] advertising: Detect if extended advertising
mgmt commands are supported
 Author: Daniel Winkler <danielwinkler@google.com>
 Date: Thu Oct 29 14:23:29 2020 -0700
 3 files changed, 8 insertions(+)
Executing: make -j12
...
src/adapter.c: In function =E2=80=98read_commands_complete=E2=80=99:
src/adapter.c:9569:8: error: =E2=80=98MGMT_OP_ADD_EXT_ADV_PARAMS=E2=80=99 u=
ndeclared
(first use in this function); did you mean =E2=80=98MGMT_OP_SET_S?
 9569 |   case MGMT_OP_ADD_EXT_ADV_PARAMS:
      |        ^~~~~~~~~~~~~~~~~~~~~~~~~~
      |        MGMT_OP_SET_SCAN_PARAMS


Please make sure that each individual patch do compile properly, this
ensures the patches can be bisected/reverted cleanly, also it looks
the last patch of the series do need a rebase since I just applied a
few patches to monitor.

--=20
Luiz Augusto von Dentz