diff mbox series

[BlueZ,v6,3/8] adapter: remove quality report from experimental features

Message ID 20220526112456.2488536-3-josephsih@chromium.org (mailing list archive)
State New, archived
Headers show
Series [BlueZ,v6,1/8] doc: Introduce the quality report command and event | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/checkpatch success Checkpatch PASS
tedd_an/gitlint success Gitlint PASS

Commit Message

Joseph Hwang May 26, 2022, 11:24 a.m. UTC 
The quality report feature is now enabled through
MGMT_OP_SET_QUALITY_REPORT instead of through the experimental
features.

---

Changes in v6:
- Fixed a patch conflict.

Changes in v5:
- Move is_quality_report_supported() implementation to next patch.
  The function does not belong to this patch.

Changes in v4:
- Move forward this patch in the patch series so that this
  command patch is prior to the quality report event patches.

Changes in v3:
- This is a new patch that enables the quality report feature via
  MGMT_OP_SET_QUALITY_REPORT.

 src/adapter.c | 14 --------------
 src/adapter.h |  1 -
 2 files changed, 15 deletions(-)

Comments

Luiz Augusto von Dentz May 26, 2022, 8:31 p.m. UTC | #1 
Hi Joseph,

On Thu, May 26, 2022 at 4:25 AM Joseph Hwang <josephsih@chromium.org> wrote:
>
> The quality report feature is now enabled through
> MGMT_OP_SET_QUALITY_REPORT instead of through the experimental
> features.
>
> ---
>
> Changes in v6:
> - Fixed a patch conflict.
>
> Changes in v5:
> - Move is_quality_report_supported() implementation to next patch.
>   The function does not belong to this patch.
>
> Changes in v4:
> - Move forward this patch in the patch series so that this
>   command patch is prior to the quality report event patches.
>
> Changes in v3:
> - This is a new patch that enables the quality report feature via
>   MGMT_OP_SET_QUALITY_REPORT.
>
>  src/adapter.c | 14 --------------
>  src/adapter.h |  1 -
>  2 files changed, 15 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index f7faaa263..2ceea6e1c 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -120,13 +120,6 @@ static const struct mgmt_exp_uuid le_simult_central_peripheral_uuid = {
>         .str = "671b10b5-42c0-4696-9227-eb28d1b049d6"
>  };
>
> -/* 330859bc-7506-492d-9370-9a6f0614037f */
> -static const struct mgmt_exp_uuid quality_report_uuid = {
> -       .val = { 0x7f, 0x03, 0x14, 0x06, 0x6f, 0x9a, 0x70, 0x93,
> -               0x2d, 0x49, 0x06, 0x75, 0xbc, 0x59, 0x08, 0x33 },
> -       .str = "330859bc-7506-492d-9370-9a6f0614037f"
> -};
> -
>  /* 15c0a148-c273-11ea-b3de-0242ac130004 */
>  static const struct mgmt_exp_uuid rpa_resolution_uuid = {
>         .val = { 0x04, 0x00, 0x13, 0xac, 0x42, 0x02, 0xde, 0xb3,
> @@ -9621,12 +9614,6 @@ static void le_simult_central_peripheral_func(struct btd_adapter *adapter,
>                                 (void *)le_simult_central_peripheral_uuid.val);
>  }
>
> -static void quality_report_func(struct btd_adapter *adapter, uint8_t action)
> -{
> -       if (action)
> -               queue_push_tail(adapter->exps, (void *)quality_report_uuid.val);
> -}
> -
>  static void set_rpa_resolution_complete(uint8_t status, uint16_t len,
>                                         const void *param, void *user_data)
>  {
> @@ -9703,7 +9690,6 @@ static const struct exp_feat {
>         EXP_FEAT(EXP_FEAT_DEBUG, &debug_uuid, exp_debug_func),
>         EXP_FEAT(EXP_FEAT_LE_SIMULT_ROLES, &le_simult_central_peripheral_uuid,
>                  le_simult_central_peripheral_func),
> -       EXP_FEAT(EXP_FEAT_BQR, &quality_report_uuid, quality_report_func),
>         EXP_FEAT(EXP_FEAT_RPA_RESOLUTION, &rpa_resolution_uuid,
>                 rpa_resolution_func),
>         EXP_FEAT(EXP_FEAT_CODEC_OFFLOAD, &codec_offload_uuid,
> diff --git a/src/adapter.h b/src/adapter.h
> index 688ed51c6..3d53a962d 100644
> --- a/src/adapter.h
> +++ b/src/adapter.h
> @@ -257,7 +257,6 @@ bool btd_le_connect_before_pairing(void);
>  enum experimental_features {
>         EXP_FEAT_DEBUG                  = 1 << 0,
>         EXP_FEAT_LE_SIMULT_ROLES        = 1 << 1,
> -       EXP_FEAT_BQR                    = 1 << 2,
>         EXP_FEAT_RPA_RESOLUTION         = 1 << 3,
>         EXP_FEAT_CODEC_OFFLOAD          = 1 << 4,
>  };

We can't remove existing experimental features since there could be
kernels which don't support MGMT_OP_SET_QUALITY_REPORT, so this will
need to stay for as long as we support such kernels versions.

> --
> 2.36.1.124.g0e6072fb45-goog
>
Joseph Hwang May 27, 2022, 3:13 a.m. UTC | #2 
Thanks Luiz for the comment! Does it mean that bluez should 1) try to
use MGMT_OP_SET_QUALITY_REPORT; and if it failed, use the experimental
feature EXP_FEAT_BQR instead?

Regards,
Joseph


On Fri, May 27, 2022 at 4:31 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Joseph,
>
> On Thu, May 26, 2022 at 4:25 AM Joseph Hwang <josephsih@chromium.org> wrote:
> >
> > The quality report feature is now enabled through
> > MGMT_OP_SET_QUALITY_REPORT instead of through the experimental
> > features.
> >
> > ---
> >
> > Changes in v6:
> > - Fixed a patch conflict.
> >
> > Changes in v5:
> > - Move is_quality_report_supported() implementation to next patch.
> >   The function does not belong to this patch.
> >
> > Changes in v4:
> > - Move forward this patch in the patch series so that this
> >   command patch is prior to the quality report event patches.
> >
> > Changes in v3:
> > - This is a new patch that enables the quality report feature via
> >   MGMT_OP_SET_QUALITY_REPORT.
> >
> >  src/adapter.c | 14 --------------
> >  src/adapter.h |  1 -
> >  2 files changed, 15 deletions(-)
> >
> > diff --git a/src/adapter.c b/src/adapter.c
> > index f7faaa263..2ceea6e1c 100644
> > --- a/src/adapter.c
> > +++ b/src/adapter.c
> > @@ -120,13 +120,6 @@ static const struct mgmt_exp_uuid le_simult_central_peripheral_uuid = {
> >         .str = "671b10b5-42c0-4696-9227-eb28d1b049d6"
> >  };
> >
> > -/* 330859bc-7506-492d-9370-9a6f0614037f */
> > -static const struct mgmt_exp_uuid quality_report_uuid = {
> > -       .val = { 0x7f, 0x03, 0x14, 0x06, 0x6f, 0x9a, 0x70, 0x93,
> > -               0x2d, 0x49, 0x06, 0x75, 0xbc, 0x59, 0x08, 0x33 },
> > -       .str = "330859bc-7506-492d-9370-9a6f0614037f"
> > -};
> > -
> >  /* 15c0a148-c273-11ea-b3de-0242ac130004 */
> >  static const struct mgmt_exp_uuid rpa_resolution_uuid = {
> >         .val = { 0x04, 0x00, 0x13, 0xac, 0x42, 0x02, 0xde, 0xb3,
> > @@ -9621,12 +9614,6 @@ static void le_simult_central_peripheral_func(struct btd_adapter *adapter,
> >                                 (void *)le_simult_central_peripheral_uuid.val);
> >  }
> >
> > -static void quality_report_func(struct btd_adapter *adapter, uint8_t action)
> > -{
> > -       if (action)
> > -               queue_push_tail(adapter->exps, (void *)quality_report_uuid.val);
> > -}
> > -
> >  static void set_rpa_resolution_complete(uint8_t status, uint16_t len,
> >                                         const void *param, void *user_data)
> >  {
> > @@ -9703,7 +9690,6 @@ static const struct exp_feat {
> >         EXP_FEAT(EXP_FEAT_DEBUG, &debug_uuid, exp_debug_func),
> >         EXP_FEAT(EXP_FEAT_LE_SIMULT_ROLES, &le_simult_central_peripheral_uuid,
> >                  le_simult_central_peripheral_func),
> > -       EXP_FEAT(EXP_FEAT_BQR, &quality_report_uuid, quality_report_func),
> >         EXP_FEAT(EXP_FEAT_RPA_RESOLUTION, &rpa_resolution_uuid,
> >                 rpa_resolution_func),
> >         EXP_FEAT(EXP_FEAT_CODEC_OFFLOAD, &codec_offload_uuid,
> > diff --git a/src/adapter.h b/src/adapter.h
> > index 688ed51c6..3d53a962d 100644
> > --- a/src/adapter.h
> > +++ b/src/adapter.h
> > @@ -257,7 +257,6 @@ bool btd_le_connect_before_pairing(void);
> >  enum experimental_features {
> >         EXP_FEAT_DEBUG                  = 1 << 0,
> >         EXP_FEAT_LE_SIMULT_ROLES        = 1 << 1,
> > -       EXP_FEAT_BQR                    = 1 << 2,
> >         EXP_FEAT_RPA_RESOLUTION         = 1 << 3,
> >         EXP_FEAT_CODEC_OFFLOAD          = 1 << 4,
> >  };
>
> We can't remove existing experimental features since there could be
> kernels which don't support MGMT_OP_SET_QUALITY_REPORT, so this will
> need to stay for as long as we support such kernels versions.
>
> > --
> > 2.36.1.124.g0e6072fb45-goog
> >
>
>
> --
> Luiz Augusto von Dentz
Luiz Augusto von Dentz May 30, 2022, 9:02 p.m. UTC | #3 
Hi Joseph,

On Thu, May 26, 2022 at 8:13 PM Joseph Hwang <josephsih@google.com> wrote:
>
> Thanks Luiz for the comment! Does it mean that bluez should 1) try to
> use MGMT_OP_SET_QUALITY_REPORT; and if it failed, use the experimental
> feature EXP_FEAT_BQR instead?

Yep, in fact you should probably keep the existing code as it since it
only set BQR if the kernel supports it anyway, that said how you are
planning to support MGMT_OP_SET_QUALITY_REPORT since that is not
enabled via UUID even if we can detect its support by the kernel Ive
assumed it wouldn't be enabled all the time or that safe to be always
enabled?

> Regards,
> Joseph
>
>
> On Fri, May 27, 2022 at 4:31 AM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Joseph,
> >
> > On Thu, May 26, 2022 at 4:25 AM Joseph Hwang <josephsih@chromium.org> wrote:
> > >
> > > The quality report feature is now enabled through
> > > MGMT_OP_SET_QUALITY_REPORT instead of through the experimental
> > > features.
> > >
> > > ---
> > >
> > > Changes in v6:
> > > - Fixed a patch conflict.
> > >
> > > Changes in v5:
> > > - Move is_quality_report_supported() implementation to next patch.
> > >   The function does not belong to this patch.
> > >
> > > Changes in v4:
> > > - Move forward this patch in the patch series so that this
> > >   command patch is prior to the quality report event patches.
> > >
> > > Changes in v3:
> > > - This is a new patch that enables the quality report feature via
> > >   MGMT_OP_SET_QUALITY_REPORT.
> > >
> > >  src/adapter.c | 14 --------------
> > >  src/adapter.h |  1 -
> > >  2 files changed, 15 deletions(-)
> > >
> > > diff --git a/src/adapter.c b/src/adapter.c
> > > index f7faaa263..2ceea6e1c 100644
> > > --- a/src/adapter.c
> > > +++ b/src/adapter.c
> > > @@ -120,13 +120,6 @@ static const struct mgmt_exp_uuid le_simult_central_peripheral_uuid = {
> > >         .str = "671b10b5-42c0-4696-9227-eb28d1b049d6"
> > >  };
> > >
> > > -/* 330859bc-7506-492d-9370-9a6f0614037f */
> > > -static const struct mgmt_exp_uuid quality_report_uuid = {
> > > -       .val = { 0x7f, 0x03, 0x14, 0x06, 0x6f, 0x9a, 0x70, 0x93,
> > > -               0x2d, 0x49, 0x06, 0x75, 0xbc, 0x59, 0x08, 0x33 },
> > > -       .str = "330859bc-7506-492d-9370-9a6f0614037f"
> > > -};
> > > -
> > >  /* 15c0a148-c273-11ea-b3de-0242ac130004 */
> > >  static const struct mgmt_exp_uuid rpa_resolution_uuid = {
> > >         .val = { 0x04, 0x00, 0x13, 0xac, 0x42, 0x02, 0xde, 0xb3,
> > > @@ -9621,12 +9614,6 @@ static void le_simult_central_peripheral_func(struct btd_adapter *adapter,
> > >                                 (void *)le_simult_central_peripheral_uuid.val);
> > >  }
> > >
> > > -static void quality_report_func(struct btd_adapter *adapter, uint8_t action)
> > > -{
> > > -       if (action)
> > > -               queue_push_tail(adapter->exps, (void *)quality_report_uuid.val);
> > > -}
> > > -
> > >  static void set_rpa_resolution_complete(uint8_t status, uint16_t len,
> > >                                         const void *param, void *user_data)
> > >  {
> > > @@ -9703,7 +9690,6 @@ static const struct exp_feat {
> > >         EXP_FEAT(EXP_FEAT_DEBUG, &debug_uuid, exp_debug_func),
> > >         EXP_FEAT(EXP_FEAT_LE_SIMULT_ROLES, &le_simult_central_peripheral_uuid,
> > >                  le_simult_central_peripheral_func),
> > > -       EXP_FEAT(EXP_FEAT_BQR, &quality_report_uuid, quality_report_func),
> > >         EXP_FEAT(EXP_FEAT_RPA_RESOLUTION, &rpa_resolution_uuid,
> > >                 rpa_resolution_func),
> > >         EXP_FEAT(EXP_FEAT_CODEC_OFFLOAD, &codec_offload_uuid,
> > > diff --git a/src/adapter.h b/src/adapter.h
> > > index 688ed51c6..3d53a962d 100644
> > > --- a/src/adapter.h
> > > +++ b/src/adapter.h
> > > @@ -257,7 +257,6 @@ bool btd_le_connect_before_pairing(void);
> > >  enum experimental_features {
> > >         EXP_FEAT_DEBUG                  = 1 << 0,
> > >         EXP_FEAT_LE_SIMULT_ROLES        = 1 << 1,
> > > -       EXP_FEAT_BQR                    = 1 << 2,
> > >         EXP_FEAT_RPA_RESOLUTION         = 1 << 3,
> > >         EXP_FEAT_CODEC_OFFLOAD          = 1 << 4,
> > >  };
> >
> > We can't remove existing experimental features since there could be
> > kernels which don't support MGMT_OP_SET_QUALITY_REPORT, so this will
> > need to stay for as long as we support such kernels versions.
> >
> > > --
> > > 2.36.1.124.g0e6072fb45-goog
> > >
> >
> >
> > --
> > Luiz Augusto von Dentz
>
>
>
> --
>
> Joseph Shyh-In Hwang
> Email: josephsih@google.com
diff mbox series

Patch

diff --git a/src/adapter.c b/src/adapter.c
index f7faaa263..2ceea6e1c 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -120,13 +120,6 @@  static const struct mgmt_exp_uuid le_simult_central_peripheral_uuid = {
 	.str = "671b10b5-42c0-4696-9227-eb28d1b049d6"
 };
 
-/* 330859bc-7506-492d-9370-9a6f0614037f */
-static const struct mgmt_exp_uuid quality_report_uuid = {
-	.val = { 0x7f, 0x03, 0x14, 0x06, 0x6f, 0x9a, 0x70, 0x93,
-		0x2d, 0x49, 0x06, 0x75, 0xbc, 0x59, 0x08, 0x33 },
-	.str = "330859bc-7506-492d-9370-9a6f0614037f"
-};
-
 /* 15c0a148-c273-11ea-b3de-0242ac130004 */
 static const struct mgmt_exp_uuid rpa_resolution_uuid = {
 	.val = { 0x04, 0x00, 0x13, 0xac, 0x42, 0x02, 0xde, 0xb3,
@@ -9621,12 +9614,6 @@  static void le_simult_central_peripheral_func(struct btd_adapter *adapter,
 				(void *)le_simult_central_peripheral_uuid.val);
 }
 
-static void quality_report_func(struct btd_adapter *adapter, uint8_t action)
-{
-	if (action)
-		queue_push_tail(adapter->exps, (void *)quality_report_uuid.val);
-}
-
 static void set_rpa_resolution_complete(uint8_t status, uint16_t len,
 					const void *param, void *user_data)
 {
@@ -9703,7 +9690,6 @@  static const struct exp_feat {
 	EXP_FEAT(EXP_FEAT_DEBUG, &debug_uuid, exp_debug_func),
 	EXP_FEAT(EXP_FEAT_LE_SIMULT_ROLES, &le_simult_central_peripheral_uuid,
 		 le_simult_central_peripheral_func),
-	EXP_FEAT(EXP_FEAT_BQR, &quality_report_uuid, quality_report_func),
 	EXP_FEAT(EXP_FEAT_RPA_RESOLUTION, &rpa_resolution_uuid,
 		rpa_resolution_func),
 	EXP_FEAT(EXP_FEAT_CODEC_OFFLOAD, &codec_offload_uuid,
diff --git a/src/adapter.h b/src/adapter.h
index 688ed51c6..3d53a962d 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -257,7 +257,6 @@  bool btd_le_connect_before_pairing(void);
 enum experimental_features {
 	EXP_FEAT_DEBUG			= 1 << 0,
 	EXP_FEAT_LE_SIMULT_ROLES	= 1 << 1,
-	EXP_FEAT_BQR			= 1 << 2,
 	EXP_FEAT_RPA_RESOLUTION		= 1 << 3,
 	EXP_FEAT_CODEC_OFFLOAD		= 1 << 4,
 };