Re: [BlueZ PATCH v2] lib: Add definitions for advertisement monitor features

[Miao-chen Chou <mcchou@chromium.org>]

Hi Luiz,

On Thu, Jun 11, 2020 at 5:54 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Miao,
>
> On Thu, Jun 11, 2020 at 4:27 PM Miao-chen Chou <mcchou@chromium.org> wrote:
> >
> > Hi Luiz,
> >
> > Thanks for checking it in.
> > I just update v3 to address the opcode right after this... Please ignore v3.
> > Also I think there is a need of keeping pattern term to imply the
> > relationship among all patterns within a monitor given the fact that
> > we may need to add other monitor type that the logical relation among
> > conditions can be customized.
>
> Is that supposed to be a difference command? Monitor Added don't seem
> to care what type it is adding, afaik all you can do is add new
> pattern types, or we do intend to have different classes of monitors?
It would be a different command, such as
MGMT_OP_ADD_ADV_FIELDS_MONITOR. In the near future, we may need to
facilitate Android HCI extension where the HCI commands are AD data
type specific. In other words, it only allows the filtering based on
some AD data types with OR or AND logical relation among conditions
within a monitor. I think we haven't come to an agreement how we'd
like to facilitate this though. Since patterns are the content of a
monitor, and what we need here is potentially a new monitor type. So
I'd still argue to keep the pattern term given that we may need a
different monitor type.

> >
> > On Thu, Jun 11, 2020 at 3:38 PM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > Hi Miao,
> > >
> > > On Mon, May 18, 2020 at 8:14 PM Miao-chen Chou <mcchou@chromium.org> wrote:
> > > >
> > > > This adds the following command opcodes, event codes and the corresponding
> > > > structures.
> > > > - MGMT_OP_READ_ADV_MONITOR_FEATURES
> > > > - MGMT_OP_ADD_ADV_PATTERNS_MONITOR
> > > > - MGMT_OP_REMOVE_ADV_MONITOR
> > > > - MGMT_EV_ADV_MONITOR_ADDED
> > > > - MGMT_EV_ADV_MONITOR_REMOVED
> > > > ---
> > > >
> > > > Changes in v2:
> > > > - Fix build failures.
> > > >
> > > >  lib/mgmt.h | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 51 insertions(+)
> > > >
> > > > diff --git a/lib/mgmt.h b/lib/mgmt.h
> > > > index b4fc72069..6d7441ccc 100644
> > > > --- a/lib/mgmt.h
> > > > +++ b/lib/mgmt.h
> > > > @@ -628,6 +628,42 @@ struct mgmt_rp_set_exp_feature {
> > > >         uint32_t flags;
> > > >  } __packed;
> > > >
> > > > +#define MGMT_ADV_MONITOR_FEATURE_MASK_OR_PATTERNS      (1 << 0)
> > > > +
> > > > +#define MGMT_OP_READ_ADV_MONITOR_FEATURES      0x004B
> > > > +struct mgmt_rp_read_adv_monitor_features {
> > > > +       uint32_t supported_features;
> > > > +       uint32_t enabled_features;
> > > > +       uint16_t max_num_handles;
> > > > +       uint8_t max_num_patterns;
> > > > +       uint16_t num_handles;
> > > > +       uint16_t handles[0];
> > > > +}  __packed;
> > > > +
> > > > +struct mgmt_adv_pattern {
> > > > +       uint8_t ad_type;
> > > > +       uint8_t offset;
> > > > +       uint8_t length;
> > > > +       uint8_t value[31];
> > > > +} __packed;
> > > > +
> > > > +#define MGMT_OP_ADD_ADV_PATTERNS_MONITOR       0x004C
> > > > +struct mgmt_cp_add_adv_patterns_monitor {
> > > > +       uint8_t pattern_count;
> > > > +       struct mgmt_adv_pattern patterns[0];
> > > > +} __packed;
> > > > +struct mgmt_rp_add_adv_patterns_monitor {
> > > > +       uint16_t monitor_handle;
> > > > +} __packed;
> > > > +
> > > > +#define MGMT_OP_REMOVE_ADV_MONITOR             0x004D
> > > > +struct mgmt_cp_remove_adv_monitor {
> > > > +       uint16_t monitor_handle;
> > > > +} __packed;
> > > > +struct mgmt_rp_remove_adv_monitor {
> > > > +       uint16_t monitor_handle;
> > > > +} __packed;
> > > > +
> > > >  #define MGMT_EV_CMD_COMPLETE           0x0001
> > > >  struct mgmt_ev_cmd_complete {
> > > >         uint16_t opcode;
> > > > @@ -857,6 +893,16 @@ struct mgmt_ev_exp_feature_changed {
> > > >         uint32_t flags;
> > > >  } __packed;
> > > >
> > > > +#define MGMT_EV_ADV_MONITOR_ADDED      0x0028
> > > > +struct mgmt_ev_adv_monitor_added {
> > > > +       uint16_t monitor_handle;
> > > > +}  __packed;
> > > > +
> > > > +#define MGMT_EV_ADV_MONITOR_REMOVED    0x0029
> > > > +struct mgmt_ev_adv_monitor_removed {
> > > > +       uint16_t monitor_handle;
> > > > +}  __packed;
> > > > +
> > > >  static const char *mgmt_op[] = {
> > > >         "<0x0000>",
> > > >         "Read Version",
> > > > @@ -933,6 +979,9 @@ static const char *mgmt_op[] = {
> > > >         "Read Security Information",                    /* 0x0048 */
> > > >         "Read Experimental Features Information",
> > > >         "Set Experimental Feature",
> > > > +       "Read Advertisement Monitor Features",
> > > > +       "Add Advertisement Patterns Monitor",
> > > > +       "Remove Advertisement Monitor",
> > > >  };
> > > >
> > > >  static const char *mgmt_ev[] = {
> > > > @@ -976,6 +1025,8 @@ static const char *mgmt_ev[] = {
> > > >         "Extended Controller Information Changed",
> > > >         "PHY Configuration Changed",
> > > >         "Experimental Feature Changed",
> > > > +       "Advertisement Monitor Added",                  /* 0x0028 */
> > > > +       "Advertisement Monitor Removed",
> > > >  };
> > > >
> > > >  static const char *mgmt_status[] = {
> > > > --
> > > > 2.26.2
> > > >
> > >
> > > Applied, thanks. Note I did adjust the opcodes so it matches the ones
> > > used in the documentation, I've also dropped the pattern term from the
> > > add command since we can assume a monitor will always be filtering
> > > based on patterns and it is not used for the event.
> > >
> > > --
> > > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz

Regards,
Miao