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: <20200429203031.BlueZ.v5.1.If9f6be992cbaeaa35423de29da6db28675b35fcc@changeid>
 <6FD8666C-39AD-4D41-9BBF-7E1371FDFFA6@holtmann.org>
In-Reply-To: <6FD8666C-39AD-4D41-9BBF-7E1371FDFFA6@holtmann.org>
From:   Miao-chen Chou <mcchou@chromium.org>
Date:   Thu, 7 May 2020 14:21:29 -0700
Message-ID: <CABmPvSF_HVxDJPh7hdFtDAK4HTH-EYkYGwtCaaFLuQzpBB+R=g@mail.gmail.com>
Subject: Re: [BlueZ PATCH v5] doc: Describe the new Advertisement Monitor support
To:     Marcel Holtmann <marcel@holtmann.org>
Cc:     Bluetooth Kernel Mailing List <linux-bluetooth@vger.kernel.org>,
        Alain Michaud <alainm@chromium.org>,
        Michael Sun <michaelfsun@google.com>,
        Luiz Augusto von Dentz <luiz.von.dentz@intel.com>,
        Yoni Shavit <yshavit@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Marcel,

On Wed, May 6, 2020 at 4:05 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Miao-chen,
>
> > This describes the following commands and event.
> > - Read Advertisement Monitor Features command
> > - Add Advertisement Patterns Monitor command
> > - Remove Advertisement Monitor command
> > - Advertisement Monitor Added event
> > - Advertisement Monitor Removed event
> > Note that the content of a monitor can differ based on its type. For no=
w we
> > introduce only pattern-based monitor, so you may find that unlike the
> > command of removing monitor(s), the Add command is tied to a specific t=
ype.
> > ---
> >
> > Changes in v5:
> > - Add more fields in the return value of Read Advertisement Monitor
> > Features command.
> > - Refine the description of commands and events.
> >
> > Changes in v4:
> > - In Read Advertisement Monitor Features command, rename
> > Adopted_Features to Enabled_Features.
> >
> > Changes in v3:
> > - Remove Advertisement Monitor can perform the removal of one monitor
> > or all monitors.
> > - Add Read Advertisement Monitor Features command.
> > - Add Advertisement Monitor Added event and dvertisement Monitor Remove=
d
> > event.
> >
> > Changes in v2:
> > - Combine commands to remove one monitor and remove all monitors. The
> > refined command takes multiple handles and an extra field to indicate
> > whether to remove all monitors.
> >
> > doc/mgmt-api.txt | 147 +++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 147 insertions(+)
> >
> > diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
> > index 39f23c456..4aa755c45 100644
> > --- a/doc/mgmt-api.txt
> > +++ b/doc/mgmt-api.txt
> > @@ -3138,6 +3138,125 @@ Read Security Information Command
> >                               Invalid Index
> >
> >
> > +Read Advertisement Monitor Features Command
> > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> > +
> > +     Command Code:           0x0049
> > +     Controller Index:       <controller id>
> > +     Command Parameters:
> > +     Return Parameters:      Supported_Features (4 octets)
> > +                             Enabled_Features (4 octets)
> > +                             Max_Num_Handles (2 octets)
> > +                             Max_Num_Patterns (1 octet)
> > +                             Num_Handles (2 octets)
> > +                             Handle1 (2 octets)
> > +                             Handle2 (2 octets)
> > +                             ...
>
>                                 Handle[i] (2 Octets)
>
> And we have been using Octet(s) and not octet(s). The document should be =
pretty much consistent with its style. If you see mistakes, please send pat=
ches to correct it.
I will correct these in v6.
>
> > +
> > +     This command is used to read the advertisement monitor features
> > +     supported by the controller and stack. Supported_Features lists a=
ll
> > +     related features supported by the controller while Enabled_Featur=
es
> > +     lists the ones currently used by the kernel.
> > +
> > +     Supported_Features and Enabled_Features are bitmasks with current=
ly
> > +     the following available bits:
> > +
> > +             1       Advertisement content monitoring based on Microso=
ft
> > +                     HCI extension.
>
> Advertising content monitoring based on pattern. Lets leave Microsoft out=
 of it.
Ack.
>
> > +
> > +     Max_Num_Handles indicates the maximum number of supported
> > +     advertisement monitors. Note that the actual number of supported
> > +     ones might be less depending on the limitation of the controller.
> > +
> > +     Max_Num_Pattern indicates the maximum number of supported pattern=
s
> > +     in an advertisement patterns monitor. Note that the actual number
> > +     of supported ones might be less depending on the limitation of th=
e
> > +     controller.
> > +
> > +     Num_Handles indicates the number of added advertisement monitors,
> > +     and it is followed by a list of handles.
> > +
> > +     This command can be used when the controller is not powered.
> > +
> > +
> > +Add Advertisement Patterns Monitor Command
> > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> > +
> > +     Command Code:           0x004A
> > +     Controller Index:       <controller id>
> > +     Command Parameters:     Pattern_count (1 Octets)
>
> Pattern_Count please.
>
> > +                             Pattern1 {
> > +                                     AD_Data_Type (1 Octet)
>
> Just call it Type and explain in the description that it is the AD Type.
>
> > +                                     Offset (1 Octet)
> > +                                     Length (1 Octet)
> > +                                     Value (variable length)
>
> We normally give a range and not =E2=80=9Cvariable length=E2=80=9D. I won=
der if we should just use a fixed size field here.
Sounds good to me.
>
> > +                             }
> > +                             Pattern2 { }
> > +                             ...
> > +     Return Parameters:      Monitor_Handle (2 Octets)
> > +
> > +     This command is used to add an advertisement monitor whose
> > +     filtering conditions are patterns. The kernel will trigger scanni=
ng
> > +     if there is at least one monitor added. If the controller support=
s
> > +     advertisement filtering, the kernel would offload the content
> > +     filtering to the controller in order to reduce power consumption;
> > +     otherwise the kernel ignores the content of the monitor. Note tha=
t
> > +     if the there are more than one patterns, OR logic would applied
> > +     among patterns during filtering. In other words, any advertisemen=
t
> > +     matching at least one pattern in a given monitor would be
> > +     considered as a match.
> > +
> > +     A pattern contains the following fields.
> > +             AD_Data_Type    Advertising Data Type. The possible value=
s
> > +                             are defined in Core Specification
> > +                             Supplement.
> > +             Offset          The start index where pattern matching
> > +                             shall be performed with in the AD data.
> > +             Length          The length of the pattern value in bytes.
> > +             Value           The value of the pattern in bytes.
> > +
> > +     Here is an example of a pattern.
> > +             {
> > +                     0x16, // Service Data - 16-bit UUID
> > +                     0x02, // Skip the UUID part.
> > +                     0x04,
> > +                     {0x11, 0x22, 0x33, 0x44},
> > +             }
> > +
> > +     This command can be used when the controller is not powered and
> > +     all settings will be programmed once powered.
> > +
> > +     Possible errors:        Failed
> > +                             Busy
> > +                             No Resources
> > +                             Invalid Parameters
> > +
> > +
> > +Remove Advertisement Monitor Command
> > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> > +
> > +     Command Code:           0x004B
> > +     Controller Index:       <controller id>
> > +     Command Parameters:     Monitor_Handle (2 Octets)
> > +     Return Parameters:
>
> Include Monitor_Handle in the return value. The events are not send to th=
e caller and so the caller somehow needs to be able to map that response if=
 they have to.
>
I will add it in v6.
> > +
> > +     This command is used to remove advertisement monitor(s). The kern=
el
> > +     would remove the monitor(s) with Monitor_Handle and update the LE
> > +     scanning.
> > +
> > +     When the Monitor_Handle is set to zero, then all previously added
> > +     handles will be removed.
> > +
> > +     Removing a monitor while it is being added will be ignored.
> > +
> > +     This command can be used when the controller is not powered and
> > +     all settings will be programmed once powered.
> > +
> > +     Possible errors:        Failed
> > +                             Busy
> > +                             Invalid Index
> > +
> > +
> > Command Complete Event
> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> >
> > @@ -4020,3 +4139,31 @@ PHY Configuration Changed Event
> >       one through which the change was triggered.
> >
> >       Refer Get PHY Configuration command for PHYs parameter.
> > +
> > +
> > +Advertisement Monitor Added Event
> > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> > +
> > +     Event Code:             0x0027
> > +     Controller Index:       <controller id>
> > +     Event Parameters:       Monitor_Handle (2 Octets)
> > +
> > +     This event indicates that an advertisement monitor has been added
> > +     using the Add Advertisement Monitor command.
> > +
> > +     The event will only be sent to management sockets other than the
> > +     one through which the command was sent.
> > +
> > +
> > +Advertisement Monitor Removed Event
> > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> > +
> > +     Event Code:             0x0028
> > +     Controller Index:       <controller id>
> > +     Event Parameters:       Monitor_Handle (2 Octets)
> > +
> > +     This event indicates that an advertisement monitor has been remov=
ed
> > +     using the Remove Advertisement Monitor command.
> > +
> > +     The event will only be sent to management sockets other than the
> > +     one through which the command was sent.

Thanks,
Miao