mbox series

[Bluez,v2,0/5] Support advertising monitor add pattern with RSSI opcode

Message ID 20210113094905.2787919-1-apusaka@google.com (mailing list archive)
Headers show
Series Support advertising monitor add pattern with RSSI opcode | expand

Message

Archie Pusaka Jan. 13, 2021, 9:49 a.m. UTC
From: Archie Pusaka <apusaka@chromium.org>

Hi linux-bluetooth,

This series of patches adds a new MGMT command for adding a monitor
with RSSI parameter. Changes are focused on passing parameters to
the kernel via btmgmt and bluetoothctl.

PTAL and thanks for your feedback!
Archie

Changes in v2:
Remove trailing period and fix order of mgmt parameter

Archie Pusaka (5):
  lib/mgmt: Adding Add Adv Patterns Monitor RSSI opcode
  src/adv_monitor: add monitor with rssi support for mgmt
  btmgmt: advmon add rssi support
  bluetoothctl: advmon rssi support for mgmt
  monitor: Decode add advmon with RSSI parameter

 client/adv_monitor.c |  90 ++++++++++++------------
 client/adv_monitor.h |   1 +
 client/main.c        |  29 ++++----
 lib/mgmt.h           |  15 ++++
 monitor/packet.c     |  43 ++++++++++--
 src/adv_monitor.c    | 143 +++++++++++++++++++++++++++++---------
 tools/btmgmt.c       | 160 ++++++++++++++++++++++++++++++++++++-------
 7 files changed, 357 insertions(+), 124 deletions(-)

Comments

Luiz Augusto von Dentz Jan. 13, 2021, 5:58 p.m. UTC | #1
Hi Archie,

On Wed, Jan 13, 2021 at 1:49 AM Archie Pusaka <apusaka@google.com> wrote:
>
> From: Archie Pusaka <apusaka@chromium.org>
>
> Hi linux-bluetooth,
>
> This series of patches adds a new MGMT command for adding a monitor
> with RSSI parameter. Changes are focused on passing parameters to
> the kernel via btmgmt and bluetoothctl.
>
> PTAL and thanks for your feedback!
> Archie
>
> Changes in v2:
> Remove trailing period and fix order of mgmt parameter
>
> Archie Pusaka (5):
>   lib/mgmt: Adding Add Adv Patterns Monitor RSSI opcode
>   src/adv_monitor: add monitor with rssi support for mgmt
>   btmgmt: advmon add rssi support
>   bluetoothctl: advmon rssi support for mgmt
>   monitor: Decode add advmon with RSSI parameter
>
>  client/adv_monitor.c |  90 ++++++++++++------------
>  client/adv_monitor.h |   1 +
>  client/main.c        |  29 ++++----
>  lib/mgmt.h           |  15 ++++
>  monitor/packet.c     |  43 ++++++++++--
>  src/adv_monitor.c    | 143 +++++++++++++++++++++++++++++---------
>  tools/btmgmt.c       | 160 ++++++++++++++++++++++++++++++++++++-------
>  7 files changed, 357 insertions(+), 124 deletions(-)
>
> --
> 2.30.0.284.gd98b1dd5eaa7-goog

While this changes seems fine I was going to suggest we split
RSSIThresholdsAndTimers to just RSSI and Timer so one don't have to
set the entire struct if there are not interested in setting a custom
RSSI and/or Timer, the advantage is the user can just omit one or the
other and the daemon will take care of filling in the missing fields
with defaults for the MGMT command. Also we probably just use plain
int16, int16 and uint16, uint16 so we don't use () around it meaning
they are not wrapped as a struct which makes it simpler to parse and
construct.
Archie Pusaka Jan. 14, 2021, 7:48 a.m. UTC | #2
Hi Luiz,

I submitted v3 to incorporate your suggestions. Tell me what you think!

Thanks,
Archie

On Thu, 14 Jan 2021 at 01:58, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Archie,
>
> On Wed, Jan 13, 2021 at 1:49 AM Archie Pusaka <apusaka@google.com> wrote:
> >
> > From: Archie Pusaka <apusaka@chromium.org>
> >
> > Hi linux-bluetooth,
> >
> > This series of patches adds a new MGMT command for adding a monitor
> > with RSSI parameter. Changes are focused on passing parameters to
> > the kernel via btmgmt and bluetoothctl.
> >
> > PTAL and thanks for your feedback!
> > Archie
> >
> > Changes in v2:
> > Remove trailing period and fix order of mgmt parameter
> >
> > Archie Pusaka (5):
> >   lib/mgmt: Adding Add Adv Patterns Monitor RSSI opcode
> >   src/adv_monitor: add monitor with rssi support for mgmt
> >   btmgmt: advmon add rssi support
> >   bluetoothctl: advmon rssi support for mgmt
> >   monitor: Decode add advmon with RSSI parameter
> >
> >  client/adv_monitor.c |  90 ++++++++++++------------
> >  client/adv_monitor.h |   1 +
> >  client/main.c        |  29 ++++----
> >  lib/mgmt.h           |  15 ++++
> >  monitor/packet.c     |  43 ++++++++++--
> >  src/adv_monitor.c    | 143 +++++++++++++++++++++++++++++---------
> >  tools/btmgmt.c       | 160 ++++++++++++++++++++++++++++++++++++-------
> >  7 files changed, 357 insertions(+), 124 deletions(-)
> >
> > --
> > 2.30.0.284.gd98b1dd5eaa7-goog
>
> While this changes seems fine I was going to suggest we split
> RSSIThresholdsAndTimers to just RSSI and Timer so one don't have to
> set the entire struct if there are not interested in setting a custom
> RSSI and/or Timer, the advantage is the user can just omit one or the
> other and the daemon will take care of filling in the missing fields
> with defaults for the MGMT command. Also we probably just use plain
> int16, int16 and uint16, uint16 so we don't use () around it meaning
> they are not wrapped as a struct which makes it simpler to parse and
> construct.
>
> --
> Luiz Augusto von Dentz