Hi Archie, On Wed, Jan 13, 2021 at 1:49 AM Archie Pusaka <apusaka@xxxxxxxxxx> wrote: > > From: Archie Pusaka <apusaka@xxxxxxxxxxxx> > > 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
