If a device is capable of waking the host system from suspend, it should
be marked as wake capable. We introduce a new management operation here
to set this property and implement the API needed to call it.
---
doc/mgmt-api.txt | 19 +++++++++++++++
lib/mgmt.h | 8 +++++++
src/adapter.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
src/adapter.h | 2 ++
src/device.c | 15 ++++++++++++
src/device.h | 2 ++
6 files changed, 108 insertions(+)
diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
index 1e59acc54..8a73a9bb9 100644
--- a/doc/mgmt-api.txt
+++ b/doc/mgmt-api.txt
@@ -3047,6 +3047,25 @@ Load Blocked Keys Command
Possible errors: Invalid Parameters
Invalid Index
+Set Wake Capable Command
+===========================
+
+ Command Code: 0x0047
+ Controller Index: <controller id>
+ Command Parameters: Address (6 Octets)
+ Address_Type (1 Octet)
+ Wake Capable (1 Octet)
+ Return Parameters: Address (6 Octets)
+ Address_Type (1 Octet)
+ Wake Capable (1 Octet)
+
+ This command sets whether a bluetooth device is capable of waking the
+ system from suspend. This property is used to set the event filter and
+ LE whitelist when the system enters suspend.
+
+ Possible errors: Failed
+ Invalid Parameters
+ Invalid Index
Command Complete Event
======================
diff --git a/lib/mgmt.h b/lib/mgmt.h
index 276445d0a..da2cce8c5 100644
--- a/lib/mgmt.h
+++ b/lib/mgmt.h
@@ -597,6 +597,12 @@ struct mgmt_blocked_key_info {
struct mgmt_cp_set_blocked_keys {
uint16_t key_count;
struct mgmt_blocked_key_info keys[0];
+
+#define MGMT_OP_SET_WAKE_CAPABLE 0x0047
+#define MGMT_SET_WAKE_CAPABLE_SIZE 8
+struct mgmt_cp_set_wake_capable {
+ struct mgmt_addr_info addr;
+ uint8_t wake_capable;
} __packed;
#define MGMT_EV_CMD_COMPLETE 0x0001
@@ -893,6 +899,8 @@ static const char *mgmt_op[] = {
"Set Appearance",
"Get PHY Configuration",
"Set PHY Configuration",
+ "Set Blocked Keys",
+ "Set Wake Capable",
};
static const char *mgmt_ev[] = {
diff --git a/src/adapter.c b/src/adapter.c
index 329c3ae0b..a35cf1349 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -4685,6 +4685,68 @@ void adapter_whitelist_add(struct btd_adapter *adapter, struct btd_device *dev)
add_whitelist_complete, adapter, NULL);
}
+static void set_wake_capable_complete(uint8_t status, uint16_t length,
+ const void *param, void *user_data)
+{
+ const struct mgmt_cp_set_wake_capable *rp = param;
+ struct btd_adapter *adapter = user_data;
+ struct btd_device *dev;
+
+ char addr[18];
+
+ if (length < sizeof(*rp)) {
+ btd_error(adapter->dev_id,
+ "Too small Set Wake Capable complete event");
+ return;
+ }
+
+ ba2str(&rp->addr.bdaddr, addr);
+
+ dev = btd_adapter_find_device(adapter, &rp->addr.bdaddr,
+ rp->addr.type);
+ if (!dev) {
+ btd_error(adapter->dev_id,
+ "Set Wake Capable complete for unknown device %s",
+ addr);
+ return;
+ }
+
+ if (status != MGMT_STATUS_SUCCESS) {
+ btd_error(adapter->dev_id,
+ "Failed to set wake capable %s(%u) = %d: %s (0x%02x)",
+ addr, rp->addr.type, rp->wake_capable,
+ mgmt_errstr(status), status);
+ return;
+ }
+
+ DBG("Set wake capable complete %s (%u)", addr, rp->addr.type);
+}
+
+void adapter_set_wake_capable(struct btd_adapter* adapter,
+ struct btd_device* dev)
+{
+ struct mgmt_cp_set_wake_capable cp;
+ char addr[18];
+
+ memset(&cp, 0, sizeof(cp));
+ bacpy(&cp.addr.bdaddr, device_get_address(dev));
+ cp.addr.type = btd_device_get_bdaddr_type(dev);
+ cp.wake_capable = device_get_wake_capable(dev);
+
+ ba2strlc(&cp.addr.bdaddr, addr);
+
+ if (!mgmt_send(adapter->mgmt, MGMT_OP_SET_WAKE_CAPABLE, adapter->dev_id,
+ sizeof(cp), &cp, set_wake_capable_complete, adapter,
+ NULL)) {
+ btd_warn(adapter->dev_id,
+ "Could not set wake capable = %u on %s (%u)",
+ cp.wake_capable, addr, cp.addr.type);
+ }
+
+ DBG("Setting %s (%u) to wake capable = %u", addr,
+ cp.addr.type, cp.wake_capable);
+}
+
static void remove_whitelist_complete(uint8_t status, uint16_t length,
const void *param, void *user_data)
{
diff --git a/src/adapter.h b/src/adapter.h
index d0a5253bd..bb63db574 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -221,6 +221,8 @@ void adapter_whitelist_add(struct btd_adapter *adapter,
struct btd_device *dev);
void adapter_whitelist_remove(struct btd_adapter *adapter,
struct btd_device *dev);
+void adapter_set_wake_capable(struct btd_adapter* adapter,
+ struct btd_device* dev);
void btd_adapter_set_oob_handler(struct btd_adapter *adapter,
struct oob_handler *handler);
diff --git a/src/device.c b/src/device.c
index a4fe10980..221f23b09 100644
--- a/src/device.c
+++ b/src/device.c
@@ -189,6 +189,7 @@ struct btd_device {
bool le;
bool pending_paired; /* "Paired" waiting for SDP */
bool svc_refreshed;
+ bool wake_capable; /* Can wake from suspend */
GSList *svc_callbacks;
GSList *eir_uuids;
struct bt_ad *ad;
@@ -1509,6 +1510,20 @@ void device_set_ltk_enc_size(struct btd_device *device, uint8_t enc_size)
bt_att_set_enc_key_size(device->att, device->ltk_enc_size);
}
+bool device_get_wake_capable(struct btd_device *device)
+{
+ return device->wake_capable;
+}
+
+void device_set_wake_capable(struct btd_device *device, bool wake_capable)
+{
+ if (device->wake_capable == wake_capable)
+ return;
+
+ device->wake_capable = wake_capable;
+ adapter_set_wake_capable(device->adapter, device);
+}
+
static void device_set_auto_connect(struct btd_device *device, gboolean enable)
{
char addr[18];
diff --git a/src/device.h b/src/device.h
index 06b100499..44df8e22e 100644
--- a/src/device.h
+++ b/src/device.h
@@ -139,6 +139,8 @@ void device_store_svc_chng_ccc(struct btd_device *device, uint8_t bdaddr_type,
uint16_t value);
void device_load_svc_chng_ccc(struct btd_device *device, uint16_t *ccc_le,
uint16_t *ccc_bredr);
+bool device_get_wake_capable(struct btd_device *device);
+void device_set_wake_capable(struct btd_device *device, bool wake_capable);
typedef void (*disconnect_watch) (struct btd_device *device, gboolean removal,
void *user_data);
--
2.25.0.341.g760bfbb309-goog
HID devices can wake the host from a suspended state. Mark them as wake
capable when they are accepted.
---
profiles/input/device.c | 1 +
profiles/input/hog.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/profiles/input/device.c b/profiles/input/device.c
index 2cb3811c8..a6b0e8053 100644
--- a/profiles/input/device.c
+++ b/profiles/input/device.c
@@ -1381,6 +1381,7 @@ int input_device_register(struct btd_service *service)
}
btd_service_set_user_data(service, idev);
+ device_set_wake_capable(device, true);
return 0;
}
diff --git a/profiles/input/hog.c b/profiles/input/hog.c
index 83c017dcb..8627f3557 100644
--- a/profiles/input/hog.c
+++ b/profiles/input/hog.c
@@ -159,6 +159,7 @@ static int hog_probe(struct btd_service *service)
return -EINVAL;
btd_service_set_user_data(service, dev);
+ device_set_wake_capable(device, true);
return 0;
}
--
2.25.0.341.g760bfbb309-goog
Hi Abhishek,
On Fri, Jan 17, 2020 at 1:28 PM Abhishek Pandit-Subedi
<[email protected]> wrote:
>
> If a device is capable of waking the host system from suspend, it should
> be marked as wake capable. We introduce a new management operation here
> to set this property and implement the API needed to call it.
>
> ---
>
> doc/mgmt-api.txt | 19 +++++++++++++++
> lib/mgmt.h | 8 +++++++
> src/adapter.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
> src/adapter.h | 2 ++
> src/device.c | 15 ++++++++++++
> src/device.h | 2 ++
> 6 files changed, 108 insertions(+)
>
> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
> index 1e59acc54..8a73a9bb9 100644
> --- a/doc/mgmt-api.txt
> +++ b/doc/mgmt-api.txt
> @@ -3047,6 +3047,25 @@ Load Blocked Keys Command
> Possible errors: Invalid Parameters
> Invalid Index
>
> +Set Wake Capable Command
> +===========================
> +
> + Command Code: 0x0047
> + Controller Index: <controller id>
> + Command Parameters: Address (6 Octets)
> + Address_Type (1 Octet)
> + Wake Capable (1 Octet)
> + Return Parameters: Address (6 Octets)
> + Address_Type (1 Octet)
> + Wake Capable (1 Octet)
> +
> + This command sets whether a bluetooth device is capable of waking the
> + system from suspend. This property is used to set the event filter and
> + LE whitelist when the system enters suspend.
> +
> + Possible errors: Failed
> + Invalid Parameters
> + Invalid Index
Please always have the documentation changes as a separated patch so
we can discuss these changes in a separate thread. Btw, Im missing the
D-Bus API changes in order for the application to control this
setting, we could probably consider doing this with auto-connect logic
but I assume this would need user input in order to use devices to
wake their systems, right? Anyway even if we were to do as a separate
setting if the device address is not programmed in the controller
whitelist I fail to see how it would be able to wake the system, so we
need the logic to work together with auto-connect.
For such features it probably make sense to have a cover letter for
the patch-set explaining the feature so we don't try guessing how it
could be used, also this sort of feature may have both security and
power impact, for instance if we don't have a controller capable of
resolving private addresses this may wake up the system for no reason,
in that case I would guess it is better to disable this feature for
devices that do use private addresses, also we may still have some
limitations either way because controllers have limited number of
slots in both the white list and resolving list.
Also, keep in mind the kernel has no persistent storage so we also
will need to store these settings in the storage so may make sense to
allow the command to set a list rather than just going one by one when
i.e. restarting the daemon, doing this via application is probably not
recommended as it may not be active when the daemon is starting.
> Command Complete Event
> ======================
> diff --git a/lib/mgmt.h b/lib/mgmt.h
> index 276445d0a..da2cce8c5 100644
> --- a/lib/mgmt.h
> +++ b/lib/mgmt.h
> @@ -597,6 +597,12 @@ struct mgmt_blocked_key_info {
> struct mgmt_cp_set_blocked_keys {
> uint16_t key_count;
> struct mgmt_blocked_key_info keys[0];
> +
> +#define MGMT_OP_SET_WAKE_CAPABLE 0x0047
> +#define MGMT_SET_WAKE_CAPABLE_SIZE 8
> +struct mgmt_cp_set_wake_capable {
> + struct mgmt_addr_info addr;
> + uint8_t wake_capable;
> } __packed;
>
> #define MGMT_EV_CMD_COMPLETE 0x0001
> @@ -893,6 +899,8 @@ static const char *mgmt_op[] = {
> "Set Appearance",
> "Get PHY Configuration",
> "Set PHY Configuration",
> + "Set Blocked Keys",
> + "Set Wake Capable",
> };
>
> static const char *mgmt_ev[] = {
> diff --git a/src/adapter.c b/src/adapter.c
> index 329c3ae0b..a35cf1349 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -4685,6 +4685,68 @@ void adapter_whitelist_add(struct btd_adapter *adapter, struct btd_device *dev)
> add_whitelist_complete, adapter, NULL);
> }
>
> +static void set_wake_capable_complete(uint8_t status, uint16_t length,
> + const void *param, void *user_data)
> +{
> + const struct mgmt_cp_set_wake_capable *rp = param;
> + struct btd_adapter *adapter = user_data;
> + struct btd_device *dev;
> +
> + char addr[18];
> +
> + if (length < sizeof(*rp)) {
> + btd_error(adapter->dev_id,
> + "Too small Set Wake Capable complete event");
> + return;
> + }
> +
> + ba2str(&rp->addr.bdaddr, addr);
> +
> + dev = btd_adapter_find_device(adapter, &rp->addr.bdaddr,
> + rp->addr.type);
> + if (!dev) {
> + btd_error(adapter->dev_id,
> + "Set Wake Capable complete for unknown device %s",
> + addr);
> + return;
> + }
> +
> + if (status != MGMT_STATUS_SUCCESS) {
> + btd_error(adapter->dev_id,
> + "Failed to set wake capable %s(%u) = %d: %s (0x%02x)",
> + addr, rp->addr.type, rp->wake_capable,
> + mgmt_errstr(status), status);
> + return;
> + }
> +
> + DBG("Set wake capable complete %s (%u)", addr, rp->addr.type);
> +}
> +
> +void adapter_set_wake_capable(struct btd_adapter* adapter,
> + struct btd_device* dev)
> +{
> + struct mgmt_cp_set_wake_capable cp;
> + char addr[18];
> +
> + memset(&cp, 0, sizeof(cp));
> + bacpy(&cp.addr.bdaddr, device_get_address(dev));
> + cp.addr.type = btd_device_get_bdaddr_type(dev);
> + cp.wake_capable = device_get_wake_capable(dev);
> +
> + ba2strlc(&cp.addr.bdaddr, addr);
> +
> + if (!mgmt_send(adapter->mgmt, MGMT_OP_SET_WAKE_CAPABLE, adapter->dev_id,
> + sizeof(cp), &cp, set_wake_capable_complete, adapter,
> + NULL)) {
> + btd_warn(adapter->dev_id,
> + "Could not set wake capable = %u on %s (%u)",
> + cp.wake_capable, addr, cp.addr.type);
> + }
> +
> + DBG("Setting %s (%u) to wake capable = %u", addr,
> + cp.addr.type, cp.wake_capable);
> +}
> +
> static void remove_whitelist_complete(uint8_t status, uint16_t length,
> const void *param, void *user_data)
> {
> diff --git a/src/adapter.h b/src/adapter.h
> index d0a5253bd..bb63db574 100644
> --- a/src/adapter.h
> +++ b/src/adapter.h
> @@ -221,6 +221,8 @@ void adapter_whitelist_add(struct btd_adapter *adapter,
> struct btd_device *dev);
> void adapter_whitelist_remove(struct btd_adapter *adapter,
> struct btd_device *dev);
> +void adapter_set_wake_capable(struct btd_adapter* adapter,
> + struct btd_device* dev);
>
> void btd_adapter_set_oob_handler(struct btd_adapter *adapter,
> struct oob_handler *handler);
> diff --git a/src/device.c b/src/device.c
> index a4fe10980..221f23b09 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -189,6 +189,7 @@ struct btd_device {
> bool le;
> bool pending_paired; /* "Paired" waiting for SDP */
> bool svc_refreshed;
> + bool wake_capable; /* Can wake from suspend */
> GSList *svc_callbacks;
> GSList *eir_uuids;
> struct bt_ad *ad;
> @@ -1509,6 +1510,20 @@ void device_set_ltk_enc_size(struct btd_device *device, uint8_t enc_size)
> bt_att_set_enc_key_size(device->att, device->ltk_enc_size);
> }
>
> +bool device_get_wake_capable(struct btd_device *device)
> +{
> + return device->wake_capable;
> +}
> +
> +void device_set_wake_capable(struct btd_device *device, bool wake_capable)
> +{
> + if (device->wake_capable == wake_capable)
> + return;
> +
> + device->wake_capable = wake_capable;
> + adapter_set_wake_capable(device->adapter, device);
> +}
> +
> static void device_set_auto_connect(struct btd_device *device, gboolean enable)
> {
> char addr[18];
> diff --git a/src/device.h b/src/device.h
> index 06b100499..44df8e22e 100644
> --- a/src/device.h
> +++ b/src/device.h
> @@ -139,6 +139,8 @@ void device_store_svc_chng_ccc(struct btd_device *device, uint8_t bdaddr_type,
> uint16_t value);
> void device_load_svc_chng_ccc(struct btd_device *device, uint16_t *ccc_le,
> uint16_t *ccc_bredr);
> +bool device_get_wake_capable(struct btd_device *device);
> +void device_set_wake_capable(struct btd_device *device, bool wake_capable);
>
> typedef void (*disconnect_watch) (struct btd_device *device, gboolean removal,
> void *user_data);
> --
> 2.25.0.341.g760bfbb309-goog
>
--
Luiz Augusto von Dentz
Hi Abhishek,
On Fri, Jan 17, 2020 at 1:28 PM Abhishek Pandit-Subedi
<[email protected]> wrote:
>
> HID devices can wake the host from a suspended state. Mark them as wake
> capable when they are accepted.
My suggestion is that we do this via application so we let the user
decide if he wants or not to wake up the system, you may still opt-in
to always enable to wake up in your system setting if you choose to,
going this route actually means the user can select not to wake up if
the device is somewhat broken or misbehaving in some respect which is
not that uncommon nowadays.
> ---
>
> profiles/input/device.c | 1 +
> profiles/input/hog.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/profiles/input/device.c b/profiles/input/device.c
> index 2cb3811c8..a6b0e8053 100644
> --- a/profiles/input/device.c
> +++ b/profiles/input/device.c
> @@ -1381,6 +1381,7 @@ int input_device_register(struct btd_service *service)
> }
>
> btd_service_set_user_data(service, idev);
> + device_set_wake_capable(device, true);
>
> return 0;
> }
> diff --git a/profiles/input/hog.c b/profiles/input/hog.c
> index 83c017dcb..8627f3557 100644
> --- a/profiles/input/hog.c
> +++ b/profiles/input/hog.c
> @@ -159,6 +159,7 @@ static int hog_probe(struct btd_service *service)
> return -EINVAL;
>
> btd_service_set_user_data(service, dev);
> + device_set_wake_capable(device, true);
> return 0;
> }
>
> --
> 2.25.0.341.g760bfbb309-goog
>
--
Luiz Augusto von Dentz
Hi Abhishek,
On Fri, Jan 17, 2020 at 2:16 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Abhishek,
>
> On Fri, Jan 17, 2020 at 1:28 PM Abhishek Pandit-Subedi
> <[email protected]> wrote:
> >
> > HID devices can wake the host from a suspended state. Mark them as wake
> > capable when they are accepted.
>
> My suggestion is that we do this via application so we let the user
> decide if he wants or not to wake up the system, you may still opt-in
> to always enable to wake up in your system setting if you choose to,
> going this route actually means the user can select not to wake up if
> the device is somewhat broken or misbehaving in some respect which is
> not that uncommon nowadays.
Will there be any follow up on this or you didn't quite understand
what Im suggesting here?
> > ---
> >
> > profiles/input/device.c | 1 +
> > profiles/input/hog.c | 1 +
> > 2 files changed, 2 insertions(+)
> >
> > diff --git a/profiles/input/device.c b/profiles/input/device.c
> > index 2cb3811c8..a6b0e8053 100644
> > --- a/profiles/input/device.c
> > +++ b/profiles/input/device.c
> > @@ -1381,6 +1381,7 @@ int input_device_register(struct btd_service *service)
> > }
> >
> > btd_service_set_user_data(service, idev);
> > + device_set_wake_capable(device, true);
> >
> > return 0;
> > }
> > diff --git a/profiles/input/hog.c b/profiles/input/hog.c
> > index 83c017dcb..8627f3557 100644
> > --- a/profiles/input/hog.c
> > +++ b/profiles/input/hog.c
> > @@ -159,6 +159,7 @@ static int hog_probe(struct btd_service *service)
> > return -EINVAL;
> >
> > btd_service_set_user_data(service, dev);
> > + device_set_wake_capable(device, true);
> > return 0;
> > }
> >
> > --
> > 2.25.0.341.g760bfbb309-goog
> >
>
>
> --
> Luiz Augusto von Dentz
--
Luiz Augusto von Dentz
Hey Luiz,
Yes, I will be following up. I sent an email on Friday but I think
with html so it may have been lost.
Will resend the patches today with wake_capable exposed via dbus and
not automatically set by the profile.
Abhishek
On Tue, Jan 21, 2020 at 2:15 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Abhishek,
>
> On Fri, Jan 17, 2020 at 2:16 PM Luiz Augusto von Dentz
> <[email protected]> wrote:
> >
> > Hi Abhishek,
> >
> > On Fri, Jan 17, 2020 at 1:28 PM Abhishek Pandit-Subedi
> > <[email protected]> wrote:
> > >
> > > HID devices can wake the host from a suspended state. Mark them as wake
> > > capable when they are accepted.
> >
> > My suggestion is that we do this via application so we let the user
> > decide if he wants or not to wake up the system, you may still opt-in
> > to always enable to wake up in your system setting if you choose to,
> > going this route actually means the user can select not to wake up if
> > the device is somewhat broken or misbehaving in some respect which is
> > not that uncommon nowadays.
>
> Will there be any follow up on this or you didn't quite understand
> what Im suggesting here?
>
> > > ---
> > >
> > > profiles/input/device.c | 1 +
> > > profiles/input/hog.c | 1 +
> > > 2 files changed, 2 insertions(+)
> > >
> > > diff --git a/profiles/input/device.c b/profiles/input/device.c
> > > index 2cb3811c8..a6b0e8053 100644
> > > --- a/profiles/input/device.c
> > > +++ b/profiles/input/device.c
> > > @@ -1381,6 +1381,7 @@ int input_device_register(struct btd_service *service)
> > > }
> > >
> > > btd_service_set_user_data(service, idev);
> > > + device_set_wake_capable(device, true);
> > >
> > > return 0;
> > > }
> > > diff --git a/profiles/input/hog.c b/profiles/input/hog.c
> > > index 83c017dcb..8627f3557 100644
> > > --- a/profiles/input/hog.c
> > > +++ b/profiles/input/hog.c
> > > @@ -159,6 +159,7 @@ static int hog_probe(struct btd_service *service)
> > > return -EINVAL;
> > >
> > > btd_service_set_user_data(service, dev);
> > > + device_set_wake_capable(device, true);
> > > return 0;
> > > }
> > >
> > > --
> > > 2.25.0.341.g760bfbb309-goog
> > >
> >
> >
> > --
> > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz