On Mon, Apr 20, 2020 at 3:53 PM Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
>
> Hi Miao,
>
> On Fri, Apr 17, 2020 at 8:13 PM Miao-chen Chou <mcchou@xxxxxxxxxxxx> wrote:
> >
> > This describes the following commands.
> > - Add Advertisement Patterns Monitor
> > - Remove Advertisement Monitor
> > - Remove All Advertisement Monitors
> > Note that the content of a monitor can differ based on its type. For now we
> > introduce only pattern-based monitor, so you may find that unlike commands
> > for removing monitor(s), the Add command is tied to a specific type.
> >
> > Signed-off-by: Miao-chen Chou <mcchou@xxxxxxxxxxxx>
>
> For userspace we don't require signed-of-by line.
Ack, it was added by patman tool, I will remove it before sending.
>
> > ---
> >
> > doc/mgmt-api.txt | 68 ++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 68 insertions(+)
> >
> > diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
> > index 39f23c456..fcd281a35 100644
> > --- a/doc/mgmt-api.txt
> > +++ b/doc/mgmt-api.txt
> > @@ -3138,6 +3138,74 @@ Read Security Information Command
> > Invalid Index
> >
> >
> > +Add Advertisement Patterns Monitor Command
> > +=========================================
> > +
> > + Command Code: 0x0049
> > + Controller Index: <controller id>
> > + Command Parameters: Pattern_count (1 Octets)
> > + Pattern1 {
> > + AD_Data_Type (1 Octet)
> > + Index (1 Octet)
>
> What is this index for, it is not very clear why would we need it here
> since we are going to return a index already.
This is the starting position of the given pattern. For instance, if
we'd like to look for the pattern within a Manufacturer Data field
with value {0x01, 0x02, 0x03} starting from index 2. Given the
Manufacturer Data field is {0x00, 0xFF, 0x01, 0x02, 0x03, 0x04}, the
matching would start from octet 0x01, and the result would be a match.
Perhaps I should rename "Index" to "Start_Position" or rename
"Monitor_index" to "Monitor_handle" to avoid confusion.
>
> > + Length (1 Octet)
> > + Value (variable length)
> > + }
> > + Pattern2 { }
> > + ...
> > + Return Parameters: Monitor_Index (8 Octets)
>
> I guess we could call this a handle if the is assigned by the
> controller not the host stack, also it might be more a good idea to
> put some example on how to build the parameter list, etc, to make it
> easier to visualize how these parameters are put together.
"Monitor_Handle" sounds good to me. I will add some examples.
>
> > +
> > + This command is used to add an advertisement monitor whose filtering
> > + conditions are patterns. The kernel would track the number of registered
> > + monitors to determine whether to perform LE scanning while there is
> > + ongoing LE scanning for other intentions, such as auto-reconnection and
> > + discovery session. If the controller supports Microsoft HCI extension,
> > + the kernel would offload the content filtering to the controller in
> > + order to reduce power consumption; otherwise the kernel ignore the
> > + content of the monitor.
> > +
> > + Possible errors: Failed
> > + Busy
> > + Invalid Parameters
> > +
> > +
> > +Remove Advertisement Monitor Command
> > +====================================
> > +
> > + Command Code: 0x004A
> > + Controller Index: <controller id>
> > + Command Parameters: Monitor_Index (8 Octets)
> > + Return Parameters: Monitor_Index (8 Octets)
> > +
> > + This command is used to remove an advertisement monitor. The kernel
> > + would remove the monitor with Monitor_Index and update the LE scanning.
> > + If the controller supports Microsoft HCI extension and the monitor has
> > + been offloaded, the kernel would cancel the offloading; otherwise the
> > + kernel takes no further actions other than removing it from the list.
> > +
> > + Possible errors: Failed
> > + Busy
> > + Invalid Index
> > +
> > +
> > +Remove All Advertisement Monitors Command
> > +=========================================
> > +
> > + Command Code: 0x004B
> > + Controller Index: <controller id>
> > + Command Parameters:
> > + Return Parameters: Num_removed_Monitors (2 Octets)
> > + Monitor_Index[i] (2 Octets)
> > +
> > + This command is used to remove all advertisement monitors. The kernel
> > + would remove all monitors and update the LE scanning if needed. If the
> > + controller supports Microsoft HCI extension the monitors have been
> > + offloaded, the kernel would cancel all offloadings; otherwise the kernel
> > + takes no further actions other than removing all monitors from the list.
> > +
> > + Possible errors: Failed
> > + Busy
>
> I wonder if we could keep this simple and just have a special value
> passed to Remove that would be consider remove all e.g.
> 0x0000000000000000, that way we don't have to allocate yet another
> opcode for removing all, though Im not sure how the values are managed
> in that namespace so it may not be possible allocate one for use with
> remote all logic, the other option would be to have a num_monitors and
> then in case none is provided do the remove all, that actually might
> be useful since if there are multiple application/monitors we can
> remove all monitors from one application without afting the others
> leaving remove all logic just for when bluetoothd is restarted which
> is basically a cleanup.
The intention of this command is indeed to handle the restart of
bluetoothd. It's a good point that we may want to remove all monitors
of an application. Let me try to combine remove and remove all
commands in my next patch.
Thanks,
Miao
