This patch adds the function to enable/disable the quality report
experimental feature in the controller through MGMT_OP_SET_EXP_FEATURE.
A user space process can enable/disable the quality report feature
by sending a property changed signal to the bluetoothd. The bluetoothd
can set up the signal handlers to handle the signal in a file under
plugins/ to call this function.
Note that the bluetoothd calls the experimental feature only when
the quality_report_supported flag is true.
Reviewed-by: Miao-chen Chou <[email protected]>
---
(no changes since v1)
src/adapter.c | 36 ++++++++++++++++++++++++++++++++++++
src/adapter.h | 2 ++
2 files changed, 38 insertions(+)
diff --git a/src/adapter.c b/src/adapter.c
index e2873de46..829d9806b 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -9332,6 +9332,42 @@ static const struct exp_feat {
EXP_FEAT(rpa_resolution_uuid, rpa_resolution_func),
};
+/* A user space process can enable/disable the quality report feature
+ * by sending a property changed signal to the bluetoothd. The bluetoothd
+ * can set up the signal handlers in a file under plugins/ to call
+ * this function.
+ */
+void btd_adapter_update_kernel_quality_report(uint8_t action)
+{
+ struct mgmt_cp_set_exp_feature cp;
+ struct btd_adapter *adapter;
+
+ adapter = btd_adapter_get_default();
+ if (!adapter) {
+ info("No default adapter. Skip enabling quality report.");
+ return;
+ }
+
+ if (!adapter->quality_report_supported) {
+ info("quality report feature not supported.");
+ return;
+ }
+
+ memset(&cp, 0, sizeof(cp));
+ memcpy(cp.uuid, quality_report_uuid, 16);
+
+ cp.action = action;
+ if (cp.action > 1) {
+ error("Unexpected quality report action %u", cp.action);
+ return;
+ }
+
+ mgmt_send(adapter->mgmt, MGMT_OP_SET_EXP_FEATURE, adapter->dev_id,
+ sizeof(cp), &cp, NULL, NULL, NULL);
+ info("update kernel quality report default adapter %d enable %d",
+ adapter->dev_id, cp.action);
+}
+
static void read_exp_features_complete(uint8_t status, uint16_t length,
const void *param, void *user_data)
{
diff --git a/src/adapter.h b/src/adapter.h
index 60b5e3bcc..001f784e4 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -240,3 +240,5 @@ enum kernel_features {
};
bool btd_has_kernel_features(uint32_t feature);
+
+void btd_adapter_update_kernel_quality_report(uint8_t action);
--
2.32.0.288.g62a8d224e6-goog
Hi Joseph,
On Mon, Jun 21, 2021 at 3:23 AM Joseph Hwang <[email protected]> wrote:
>
> This patch adds the function to enable/disable the quality report
> experimental feature in the controller through MGMT_OP_SET_EXP_FEATURE.
>
> A user space process can enable/disable the quality report feature
> by sending a property changed signal to the bluetoothd. The bluetoothd
> can set up the signal handlers to handle the signal in a file under
> plugins/ to call this function.
>
> Note that the bluetoothd calls the experimental feature only when
> the quality_report_supported flag is true.
>
> Reviewed-by: Miao-chen Chou <[email protected]>
> ---
>
> (no changes since v1)
>
> src/adapter.c | 36 ++++++++++++++++++++++++++++++++++++
> src/adapter.h | 2 ++
> 2 files changed, 38 insertions(+)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index e2873de46..829d9806b 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -9332,6 +9332,42 @@ static const struct exp_feat {
> EXP_FEAT(rpa_resolution_uuid, rpa_resolution_func),
> };
>
> +/* A user space process can enable/disable the quality report feature
> + * by sending a property changed signal to the bluetoothd. The bluetoothd
> + * can set up the signal handlers in a file under plugins/ to call
> + * this function.
> + */
> +void btd_adapter_update_kernel_quality_report(uint8_t action)
> +{
> + struct mgmt_cp_set_exp_feature cp;
> + struct btd_adapter *adapter;
> +
> + adapter = btd_adapter_get_default();
> + if (!adapter) {
> + info("No default adapter. Skip enabling quality report.");
> + return;
> + }
> +
> + if (!adapter->quality_report_supported) {
> + info("quality report feature not supported.");
> + return;
> + }
> +
> + memset(&cp, 0, sizeof(cp));
> + memcpy(cp.uuid, quality_report_uuid, 16);
> +
> + cp.action = action;
> + if (cp.action > 1) {
> + error("Unexpected quality report action %u", cp.action);
> + return;
> + }
> +
> + mgmt_send(adapter->mgmt, MGMT_OP_SET_EXP_FEATURE, adapter->dev_id,
> + sizeof(cp), &cp, NULL, NULL, NULL);
> + info("update kernel quality report default adapter %d enable %d",
> + adapter->dev_id, cp.action);
> +}
> +
> static void read_exp_features_complete(uint8_t status, uint16_t length,
> const void *param, void *user_data)
> {
> diff --git a/src/adapter.h b/src/adapter.h
> index 60b5e3bcc..001f784e4 100644
> --- a/src/adapter.h
> +++ b/src/adapter.h
> @@ -240,3 +240,5 @@ enum kernel_features {
> };
>
> bool btd_has_kernel_features(uint32_t feature);
> +
> +void btd_adapter_update_kernel_quality_report(uint8_t action);
It doesn't seem this is being used anywhere, in the patch description
it mentions a plugin is handling this via a signal but we could
actually make the core do that directly, in fact having a plugin
handling posix signals might be a bad idea. If this is something we
don't need to change at runtime I would expect it to be configurable
over main.conf, or are there legitimate reasons to not have the
controller generating these events all the time?
> --
> 2.32.0.288.g62a8d224e6-goog
>
--
Luiz Augusto von Dentz
Please read the replies in the lines below.
Thanks!
On Tue, Jun 22, 2021 at 2:25 AM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Joseph,
>
> On Mon, Jun 21, 2021 at 3:23 AM Joseph Hwang <[email protected]> wrote:
> >
> > This patch adds the function to enable/disable the quality report
> > experimental feature in the controller through MGMT_OP_SET_EXP_FEATURE.
> >
> > A user space process can enable/disable the quality report feature
> > by sending a property changed signal to the bluetoothd. The bluetoothd
> > can set up the signal handlers to handle the signal in a file under
> > plugins/ to call this function.
> >
> > Note that the bluetoothd calls the experimental feature only when
> > the quality_report_supported flag is true.
> >
> > Reviewed-by: Miao-chen Chou <[email protected]>
> > ---
> >
> > (no changes since v1)
> >
> > src/adapter.c | 36 ++++++++++++++++++++++++++++++++++++
> > src/adapter.h | 2 ++
> > 2 files changed, 38 insertions(+)
> >
> > diff --git a/src/adapter.c b/src/adapter.c
> > index e2873de46..829d9806b 100644
> > --- a/src/adapter.c
> > +++ b/src/adapter.c
> > @@ -9332,6 +9332,42 @@ static const struct exp_feat {
> > EXP_FEAT(rpa_resolution_uuid, rpa_resolution_func),
> > };
> >
> > +/* A user space process can enable/disable the quality report feature
> > + * by sending a property changed signal to the bluetoothd. The bluetoothd
> > + * can set up the signal handlers in a file under plugins/ to call
> > + * this function.
> > + */
> > +void btd_adapter_update_kernel_quality_report(uint8_t action)
> > +{
> > + struct mgmt_cp_set_exp_feature cp;
> > + struct btd_adapter *adapter;
> > +
> > + adapter = btd_adapter_get_default();
> > + if (!adapter) {
> > + info("No default adapter. Skip enabling quality report.");
> > + return;
> > + }
> > +
> > + if (!adapter->quality_report_supported) {
> > + info("quality report feature not supported.");
> > + return;
> > + }
> > +
> > + memset(&cp, 0, sizeof(cp));
> > + memcpy(cp.uuid, quality_report_uuid, 16);
> > +
> > + cp.action = action;
> > + if (cp.action > 1) {
> > + error("Unexpected quality report action %u", cp.action);
> > + return;
> > + }
> > +
> > + mgmt_send(adapter->mgmt, MGMT_OP_SET_EXP_FEATURE, adapter->dev_id,
> > + sizeof(cp), &cp, NULL, NULL, NULL);
> > + info("update kernel quality report default adapter %d enable %d",
> > + adapter->dev_id, cp.action);
> > +}
> > +
> > static void read_exp_features_complete(uint8_t status, uint16_t length,
> > const void *param, void *user_data)
> > {
> > diff --git a/src/adapter.h b/src/adapter.h
> > index 60b5e3bcc..001f784e4 100644
> > --- a/src/adapter.h
> > +++ b/src/adapter.h
> > @@ -240,3 +240,5 @@ enum kernel_features {
> > };
> >
> > bool btd_has_kernel_features(uint32_t feature);
> > +
> > +void btd_adapter_update_kernel_quality_report(uint8_t action);
>
> It doesn't seem this is being used anywhere, in the patch description
It is not used elsewhere except in plugins/chromium.c. The function
is placed here so that
other user processes in the Linux community can use it very easily.
Please refer to how it is invoked
in chrome os
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/bluez/current/plugins/chromium.c;l=748?q=btd_adapter_update_kernel_quality_report&ss=chromiumos
This quality report experimental feature
(CONFIG_BT_FEATURE_QUALITY_REPORT) follows what was implemented for
the debug log experimental feature (CONFIG_BT_FEATURE_DEBUG).
> it mentions a plugin is handling this via a signal but we could
> actually make the core do that directly, in fact having a plugin
> handling posix signals might be a bad idea. If this is something we
> don't need to change at runtime I would expect it to be configurable
In chrome os, this quality report experimental feature, just like the
other debug log experimental feature, is required to change at run
time. Both experimental features are enabled/disabled at run time with
the same mechanism -- a user space process sends the dbus signals to
enable/disable the features.
> over main.conf, or are there legitimate reasons to not have the
> controller generating these events all the time?
Another reason not to generate the events all the time is to avoid the
overhead on the BT controller and the stack if the feature is not
required.
>
> > --
> > 2.32.0.288.g62a8d224e6-goog
> >
>
>
> --
> Luiz Augusto von Dentz
--
Joseph Shyh-In Hwang
Email: [email protected]