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: <20201001230152.2422908-1-danielwinkler@google.com> <CAP2xMbs9ZeYrCCFOh5dh2Yj5PnrJ1hRyb03b_2aaPN6Q20MMqA@mail.gmail.com>
In-Reply-To: <CAP2xMbs9ZeYrCCFOh5dh2Yj5PnrJ1hRyb03b_2aaPN6Q20MMqA@mail.gmail.com>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Tue, 27 Oct 2020 15:13:38 -0700
Message-ID: <CABBYNZLig6m8gv5X-MmaaQCNongTmgTCd_Q9EDO=tTCC0kR2Lw@mail.gmail.com>
Subject: Re: [Bluez PATCH v4 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>,
        BlueZ <linux-bluetooth@vger.kernel.org>,
        chromeos-bluetooth-upstreaming 
        <chromeos-bluetooth-upstreaming@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Daniel, Marcel,


On Mon, Oct 26, 2020 at 5:09 PM Daniel Winkler <danielwinkler@google.com> wrote:
>
> Hello Maintainers,
>
> Pinging again to see if there is further feedback on this advertising
> patch series. Please let me know if there are any further comments
> that I can address.
>
> Best regards,
> Daniel
>
>
> On Thu, Oct 1, 2020 at 4:01 PM Daniel Winkler <danielwinkler@google.com> wrote:
> >
> > 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 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 |  50 +++++
> >  doc/mgmt-api.txt        | 238 +++++++++++++++++++++++-
> >  lib/mgmt.h              |  46 ++++-
> >  monitor/packet.c        |  69 ++++++-
> >  src/adapter.c           |   4 +
> >  src/adapter.h           |   1 +
> >  src/advertising.c       | 403 ++++++++++++++++++++++++++++++++++++++--
> >  tools/btmgmt.c          |  12 +-
> >  9 files changed, 791 insertions(+), 33 deletions(-)
> >
> > --
> > 2.28.0.709.gb0816b6eb0-goog
> >

Other than the small comments these look good to me.

--
Luiz Augusto von Dentz