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@xxxxxxxxx> wrote: > > 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
