Linux-Bluetooth Archive on lore.kernel.org
 help / color / Atom feed
* [BlueZ PATCH v3 2/5] device: Allow device to be marked as wake capable
@ 2020-01-28 18:43 Abhishek Pandit-Subedi
  2020-01-28 20:29 ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 6+ messages in thread
From: Abhishek Pandit-Subedi @ 2020-01-28 18:43 UTC (permalink / raw)
  To: luiz.dentz, marcel, alainm
  Cc: linux-bluetooth, chromeos-bluetooth-upstreaming, Abhishek Pandit-Subedi

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. We also
add the dbus endpoint to allow the wake capable setting to be
controlled.

---

Changes in v3:
* Added profile_wake_support and made wake_capable dependent on it

Changes in v2:
* Added dbus api "WakeCapable" to set value
* Update device_set_wake_capable to be called by
  adapter_set_wake_capable_complete so we can emit property changed

 lib/mgmt.h    |  9 ++++++
 src/adapter.c | 65 ++++++++++++++++++++++++++++++++++++++++
 src/adapter.h |  2 ++
 src/device.c  | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++
 src/device.h  |  5 ++++
 5 files changed, 164 insertions(+)

diff --git a/lib/mgmt.h b/lib/mgmt.h
index 276445d0a..cf19dd086 100644
--- a/lib/mgmt.h
+++ b/lib/mgmt.h
@@ -599,6 +599,13 @@ struct mgmt_cp_set_blocked_keys {
 	struct mgmt_blocked_key_info keys[0];
 } __packed;
 
+#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_enable;
+} __packed;
+
 #define MGMT_EV_CMD_COMPLETE		0x0001
 struct mgmt_ev_cmd_complete {
 	uint16_t opcode;
@@ -893,6 +900,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..294a9c9e4 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -4685,6 +4685,71 @@ 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_enable,
+			  mgmt_errstr(status), status);
+		return;
+	}
+
+	device_set_wake_capable(dev, rp->wake_enable);
+
+	DBG("Set wake capable complete %s (%u)", addr, rp->addr.type);
+}
+
+void adapter_set_wake_capable(struct btd_adapter* adapter,
+			      struct btd_device* dev,
+			      bool wake_enable)
+{
+	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_enable = device_get_profile_wake_support(dev) && wake_enable;
+
+	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_enable, addr, cp.addr.type);
+	}
+
+	DBG("Setting %s (%u) to wake capable = %u", addr,
+	    cp.addr.type, cp.wake_enable);
+}
+
 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..e990279ed 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, bool wake_enable);
 
 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..7a455e1e9 100644
--- a/src/device.c
+++ b/src/device.c
@@ -189,6 +189,8 @@ struct btd_device {
 	bool		le;
 	bool		pending_paired;		/* "Paired" waiting for SDP */
 	bool		svc_refreshed;
+	bool		profile_wake_support;	/* Profile supports wake */
+	bool		wake_capable;		/* Can wake from suspend */
 	GSList		*svc_callbacks;
 	GSList		*eir_uuids;
 	struct bt_ad	*ad;
@@ -415,6 +417,9 @@ static gboolean store_device_info_cb(gpointer user_data)
 	g_key_file_set_boolean(key_file, "General", "Blocked",
 							device->blocked);
 
+	g_key_file_set_boolean(key_file, "General", "WakeCapable",
+							device->wake_capable);
+
 	if (device->uuids) {
 		GSList *l;
 		int i;
@@ -1318,6 +1323,47 @@ dev_property_advertising_data_exist(const GDBusPropertyTable *property,
 	return bt_ad_has_data(device->ad, NULL);
 }
 
+static gboolean
+dev_property_get_wake_capable(const GDBusPropertyTable *property,
+			     DBusMessageIter *iter, void *data)
+{
+	struct btd_device *device = data;
+	dbus_bool_t wake_capable =
+			device_get_wake_capable(device) ? TRUE : FALSE;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &wake_capable);
+
+	return TRUE;
+}
+
+static void dev_property_set_wake_capable(const GDBusPropertyTable *property,
+					 DBusMessageIter *value,
+					 GDBusPendingPropertySet id, void *data)
+{
+	struct btd_device *device = data;
+	dbus_bool_t b;
+
+	if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_BOOLEAN) {
+		g_dbus_pending_property_error(id,
+					ERROR_INTERFACE ".InvalidArguments",
+					"Invalid arguments in method call");
+		return;
+	}
+
+	dbus_message_iter_get_basic(value, &b);
+
+	adapter_set_wake_capable(device->adapter, device, b == TRUE);
+	g_dbus_pending_property_success(id);
+}
+
+static gboolean dev_property_wake_capable_exist(
+		const GDBusPropertyTable *property, void *data)
+{
+	struct btd_device *device = data;
+
+	return device_get_profile_wake_support(device) ? TRUE : FALSE;
+}
+
 static gboolean disconnect_all(gpointer user_data)
 {
 	struct btd_device *device = user_data;
@@ -1509,6 +1555,34 @@ 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_profile_wake_support(struct btd_device *device)
+{
+	return device->profile_wake_support;
+}
+
+void device_set_profile_wake_support(struct btd_device *device,
+				     bool wake_support)
+{
+	device->profile_wake_support = wake_support;
+	/* WakeCapable is only visible when wake_support is true */
+	g_dbus_emit_property_changed(dbus_conn, device->path, DEVICE_INTERFACE,
+				     "WakeCapable");
+}
+
+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)
+{
+	device->wake_capable = wake_capable;
+
+	store_device_info(device);
+	g_dbus_emit_property_changed(dbus_conn, device->path, DEVICE_INTERFACE,
+				     "WakeCapable");
+}
+
 static void device_set_auto_connect(struct btd_device *device, gboolean enable)
 {
 	char addr[18];
@@ -2779,6 +2853,9 @@ static const GDBusPropertyTable device_properties[] = {
 	{ "AdvertisingData", "a{yv}", dev_property_get_advertising_data,
 				NULL, dev_property_advertising_data_exist,
 				G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "WakeCapable", "b", dev_property_get_wake_capable,
+				dev_property_set_wake_capable,
+				dev_property_wake_capable_exist },
 	{ }
 };
 
@@ -3030,6 +3107,7 @@ static void load_info(struct btd_device *device, const char *local,
 	char *str;
 	gboolean store_needed = FALSE;
 	gboolean blocked;
+	gboolean wake_capable;
 	char **uuids;
 	int source, vendor, product, version;
 	char **techno, **t;
@@ -3141,6 +3219,11 @@ next:
 		btd_device_set_pnpid(device, source, vendor, product, version);
 	}
 
+	/* Mark wake capable */
+	wake_capable = g_key_file_get_boolean(key_file, "General",
+					      "WakeCapable", NULL);
+	adapter_set_wake_capable(device->adapter, device, wake_capable == TRUE);
+
 	if (store_needed)
 		store_device_info(device);
 }
diff --git a/src/device.h b/src/device.h
index 06b100499..43f633e38 100644
--- a/src/device.h
+++ b/src/device.h
@@ -139,6 +139,11 @@ 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_profile_wake_support(struct btd_device *device);
+void device_set_profile_wake_support(struct btd_device *device,
+				     bool wake_support);
+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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [BlueZ PATCH v3 2/5] device: Allow device to be marked as wake capable
  2020-01-28 18:43 [BlueZ PATCH v3 2/5] device: Allow device to be marked as wake capable Abhishek Pandit-Subedi
@ 2020-01-28 20:29 ` Luiz Augusto von Dentz
  2020-01-28 22:12   ` Abhishek Pandit-Subedi
  0 siblings, 1 reply; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2020-01-28 20:29 UTC (permalink / raw)
  To: Abhishek Pandit-Subedi
  Cc: Marcel Holtmann, Alain Michaud, linux-bluetooth,
	chromeos-bluetooth-upstreaming

Hi Abhishek,

On Tue, Jan 28, 2020 at 10:44 AM Abhishek Pandit-Subedi
<abhishekpandit@chromium.org> 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. We also
> add the dbus endpoint to allow the wake capable setting to be
> controlled.
>
> ---
>
> Changes in v3:
> * Added profile_wake_support and made wake_capable dependent on it
>
> Changes in v2:
> * Added dbus api "WakeCapable" to set value
> * Update device_set_wake_capable to be called by
>   adapter_set_wake_capable_complete so we can emit property changed
>
>  lib/mgmt.h    |  9 ++++++
>  src/adapter.c | 65 ++++++++++++++++++++++++++++++++++++++++
>  src/adapter.h |  2 ++
>  src/device.c  | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/device.h  |  5 ++++
>  5 files changed, 164 insertions(+)
>
> diff --git a/lib/mgmt.h b/lib/mgmt.h
> index 276445d0a..cf19dd086 100644
> --- a/lib/mgmt.h
> +++ b/lib/mgmt.h
> @@ -599,6 +599,13 @@ struct mgmt_cp_set_blocked_keys {
>         struct mgmt_blocked_key_info keys[0];
>  } __packed;
>
> +#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_enable;
> +} __packed;
> +
>  #define MGMT_EV_CMD_COMPLETE           0x0001
>  struct mgmt_ev_cmd_complete {
>         uint16_t opcode;
> @@ -893,6 +900,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..294a9c9e4 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -4685,6 +4685,71 @@ 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;

Extra empty line not needed.

> +       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_enable,
> +                         mgmt_errstr(status), status);
> +               return;
> +       }
> +
> +       device_set_wake_capable(dev, rp->wake_enable);
> +
> +       DBG("Set wake capable complete %s (%u)", addr, rp->addr.type);
> +}
> +
> +void adapter_set_wake_capable(struct btd_adapter* adapter,
> +                             struct btd_device* dev,
> +                             bool wake_enable)
> +{
> +       struct mgmt_cp_set_wake_capable cp;
> +       char addr[18];

Shouldn't there be a way to check if the kernel does actually support
the command first? We shouldn't be attempting to use it with kernels
that don't support it and the property should also not appear in that
case, so it is only visible if:

1. Kernel reports that Wake Capable command is supported
2. A driver/plugin/profile calls device_set_wake_support

Also in case those 2 conditions are attended bu the command fails we
should reply with an error since there won't be any PropertiesChanged
to indicate something went wrong.

> +       memset(&cp, 0, sizeof(cp));
> +       bacpy(&cp.addr.bdaddr, device_get_address(dev));
> +       cp.addr.type = btd_device_get_bdaddr_type(dev);
> +       cp.wake_enable = device_get_profile_wake_support(dev) && wake_enable;
> +
> +       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_enable, addr, cp.addr.type);
> +       }
> +
> +       DBG("Setting %s (%u) to wake capable = %u", addr,
> +           cp.addr.type, cp.wake_enable);
> +}
> +
>  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..e990279ed 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, bool wake_enable);
>
>  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..7a455e1e9 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -189,6 +189,8 @@ struct btd_device {
>         bool            le;
>         bool            pending_paired;         /* "Paired" waiting for SDP */
>         bool            svc_refreshed;
> +       bool            profile_wake_support;   /* Profile supports wake */

I guess we can drop the profile term here since that is implicit already.

> +       bool            wake_capable;           /* Can wake from suspend */
>         GSList          *svc_callbacks;
>         GSList          *eir_uuids;
>         struct bt_ad    *ad;
> @@ -415,6 +417,9 @@ static gboolean store_device_info_cb(gpointer user_data)
>         g_key_file_set_boolean(key_file, "General", "Blocked",
>                                                         device->blocked);
>
> +       g_key_file_set_boolean(key_file, "General", "WakeCapable",
> +                                                       device->wake_capable);
> +
>         if (device->uuids) {
>                 GSList *l;
>                 int i;
> @@ -1318,6 +1323,47 @@ dev_property_advertising_data_exist(const GDBusPropertyTable *property,
>         return bt_ad_has_data(device->ad, NULL);
>  }
>
> +static gboolean
> +dev_property_get_wake_capable(const GDBusPropertyTable *property,
> +                            DBusMessageIter *iter, void *data)
> +{
> +       struct btd_device *device = data;
> +       dbus_bool_t wake_capable =
> +                       device_get_wake_capable(device) ? TRUE : FALSE;
> +
> +       dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &wake_capable);
> +
> +       return TRUE;
> +}
> +
> +static void dev_property_set_wake_capable(const GDBusPropertyTable *property,
> +                                        DBusMessageIter *value,
> +                                        GDBusPendingPropertySet id, void *data)
> +{
> +       struct btd_device *device = data;
> +       dbus_bool_t b;
> +
> +       if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_BOOLEAN) {
> +               g_dbus_pending_property_error(id,
> +                                       ERROR_INTERFACE ".InvalidArguments",
> +                                       "Invalid arguments in method call");
> +               return;
> +       }
> +
> +       dbus_message_iter_get_basic(value, &b);
> +
> +       adapter_set_wake_capable(device->adapter, device, b == TRUE);
> +       g_dbus_pending_property_success(id);
> +}
> +
> +static gboolean dev_property_wake_capable_exist(
> +               const GDBusPropertyTable *property, void *data)
> +{
> +       struct btd_device *device = data;
> +
> +       return device_get_profile_wake_support(device) ? TRUE : FALSE;
> +}
> +
>  static gboolean disconnect_all(gpointer user_data)
>  {
>         struct btd_device *device = user_data;
> @@ -1509,6 +1555,34 @@ 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_profile_wake_support(struct btd_device *device)
> +{
> +       return device->profile_wake_support;
> +}
> +
> +void device_set_profile_wake_support(struct btd_device *device,
> +                                    bool wake_support)
> +{
> +       device->profile_wake_support = wake_support;
> +       /* WakeCapable is only visible when wake_support is true */
> +       g_dbus_emit_property_changed(dbus_conn, device->path, DEVICE_INTERFACE,
> +                                    "WakeCapable");
> +}
> +
> +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)
> +{
> +       device->wake_capable = wake_capable;
> +
> +       store_device_info(device);
> +       g_dbus_emit_property_changed(dbus_conn, device->path, DEVICE_INTERFACE,
> +                                    "WakeCapable");
> +}
> +
>  static void device_set_auto_connect(struct btd_device *device, gboolean enable)
>  {
>         char addr[18];
> @@ -2779,6 +2853,9 @@ static const GDBusPropertyTable device_properties[] = {
>         { "AdvertisingData", "a{yv}", dev_property_get_advertising_data,
>                                 NULL, dev_property_advertising_data_exist,
>                                 G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
> +       { "WakeCapable", "b", dev_property_get_wake_capable,
> +                               dev_property_set_wake_capable,
> +                               dev_property_wake_capable_exist },
>         { }
>  };
>
> @@ -3030,6 +3107,7 @@ static void load_info(struct btd_device *device, const char *local,
>         char *str;
>         gboolean store_needed = FALSE;
>         gboolean blocked;
> +       gboolean wake_capable;
>         char **uuids;
>         int source, vendor, product, version;
>         char **techno, **t;
> @@ -3141,6 +3219,11 @@ next:
>                 btd_device_set_pnpid(device, source, vendor, product, version);
>         }
>
> +       /* Mark wake capable */
> +       wake_capable = g_key_file_get_boolean(key_file, "General",
> +                                             "WakeCapable", NULL);
> +       adapter_set_wake_capable(device->adapter, device, wake_capable == TRUE);
> +
>         if (store_needed)
>                 store_device_info(device);
>  }
> diff --git a/src/device.h b/src/device.h
> index 06b100499..43f633e38 100644
> --- a/src/device.h
> +++ b/src/device.h
> @@ -139,6 +139,11 @@ 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_profile_wake_support(struct btd_device *device);
> +void device_set_profile_wake_support(struct btd_device *device,
> +                                    bool wake_support);
> +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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [BlueZ PATCH v3 2/5] device: Allow device to be marked as wake capable
  2020-01-28 20:29 ` Luiz Augusto von Dentz
@ 2020-01-28 22:12   ` Abhishek Pandit-Subedi
  2020-01-29  4:03     ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 6+ messages in thread
From: Abhishek Pandit-Subedi @ 2020-01-28 22:12 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: Marcel Holtmann, Alain Michaud, linux-bluetooth,
	chromeos-bluetooth-upstreaming

Hi Luiz,

On Tue, Jan 28, 2020 at 12:29 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Abhishek,
>
> On Tue, Jan 28, 2020 at 10:44 AM Abhishek Pandit-Subedi
> <abhishekpandit@chromium.org> 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. We also
> > add the dbus endpoint to allow the wake capable setting to be
> > controlled.
> >
> > ---
> >
> > Changes in v3:
> > * Added profile_wake_support and made wake_capable dependent on it
> >
> > Changes in v2:
> > * Added dbus api "WakeCapable" to set value
> > * Update device_set_wake_capable to be called by
> >   adapter_set_wake_capable_complete so we can emit property changed
> >
> >  lib/mgmt.h    |  9 ++++++
> >  src/adapter.c | 65 ++++++++++++++++++++++++++++++++++++++++
> >  src/adapter.h |  2 ++
> >  src/device.c  | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  src/device.h  |  5 ++++
> >  5 files changed, 164 insertions(+)
> >
> > diff --git a/lib/mgmt.h b/lib/mgmt.h
> > index 276445d0a..cf19dd086 100644
> > --- a/lib/mgmt.h
> > +++ b/lib/mgmt.h
> > @@ -599,6 +599,13 @@ struct mgmt_cp_set_blocked_keys {
> >         struct mgmt_blocked_key_info keys[0];
> >  } __packed;
> >
> > +#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_enable;
> > +} __packed;
> > +
> >  #define MGMT_EV_CMD_COMPLETE           0x0001
> >  struct mgmt_ev_cmd_complete {
> >         uint16_t opcode;
> > @@ -893,6 +900,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..294a9c9e4 100644
> > --- a/src/adapter.c
> > +++ b/src/adapter.c
> > @@ -4685,6 +4685,71 @@ 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;
>
> Extra empty line not needed.

My git merging skills have failed me yet again. Will fix in next patch.

>
> > +       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_enable,
> > +                         mgmt_errstr(status), status);
> > +               return;
> > +       }
> > +
> > +       device_set_wake_capable(dev, rp->wake_enable);
> > +
> > +       DBG("Set wake capable complete %s (%u)", addr, rp->addr.type);
> > +}
> > +
> > +void adapter_set_wake_capable(struct btd_adapter* adapter,
> > +                             struct btd_device* dev,
> > +                             bool wake_enable)
> > +{
> > +       struct mgmt_cp_set_wake_capable cp;
> > +       char addr[18];
>
> Shouldn't there be a way to check if the kernel does actually support
> the command first? We shouldn't be attempting to use it with kernels
> that don't support it and the property should also not appear in that
> case, so it is only visible if:
>
> 1. Kernel reports that Wake Capable command is supported
> 2. A driver/plugin/profile calls device_set_wake_support
>
> Also in case those 2 conditions are attended bu the command fails we
> should reply with an error since there won't be any PropertiesChanged
> to indicate something went wrong.

That sounds reasonable. Are all supported management apis reported
somewhere? (I may discover the answer to this just by reading code)

Also, what's a good way to report an error on PropertiesChanged? Just
logging the error (i.e. error("Failed to set wake capable property"))
or actually calling something like g_dbus_emit_property_error (I don't
think this is real; just using an example)?

>
> > +       memset(&cp, 0, sizeof(cp));
> > +       bacpy(&cp.addr.bdaddr, device_get_address(dev));
> > +       cp.addr.type = btd_device_get_bdaddr_type(dev);
> > +       cp.wake_enable = device_get_profile_wake_support(dev) && wake_enable;
> > +
> > +       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_enable, addr, cp.addr.type);
> > +       }
> > +
> > +       DBG("Setting %s (%u) to wake capable = %u", addr,
> > +           cp.addr.type, cp.wake_enable);
> > +}
> > +
> >  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..e990279ed 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, bool wake_enable);
> >
> >  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..7a455e1e9 100644
> > --- a/src/device.c
> > +++ b/src/device.c
> > @@ -189,6 +189,8 @@ struct btd_device {
> >         bool            le;
> >         bool            pending_paired;         /* "Paired" waiting for SDP */
> >         bool            svc_refreshed;
> > +       bool            profile_wake_support;   /* Profile supports wake */
>
> I guess we can drop the profile term here since that is implicit already.

Will change in next patch.

>
> > +       bool            wake_capable;           /* Can wake from suspend */
> >         GSList          *svc_callbacks;
> >         GSList          *eir_uuids;
> >         struct bt_ad    *ad;
> > @@ -415,6 +417,9 @@ static gboolean store_device_info_cb(gpointer user_data)
> >         g_key_file_set_boolean(key_file, "General", "Blocked",
> >                                                         device->blocked);
> >
> > +       g_key_file_set_boolean(key_file, "General", "WakeCapable",
> > +                                                       device->wake_capable);
> > +
> >         if (device->uuids) {
> >                 GSList *l;
> >                 int i;
> > @@ -1318,6 +1323,47 @@ dev_property_advertising_data_exist(const GDBusPropertyTable *property,
> >         return bt_ad_has_data(device->ad, NULL);
> >  }
> >
> > +static gboolean
> > +dev_property_get_wake_capable(const GDBusPropertyTable *property,
> > +                            DBusMessageIter *iter, void *data)
> > +{
> > +       struct btd_device *device = data;
> > +       dbus_bool_t wake_capable =
> > +                       device_get_wake_capable(device) ? TRUE : FALSE;
> > +
> > +       dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &wake_capable);
> > +
> > +       return TRUE;
> > +}
> > +
> > +static void dev_property_set_wake_capable(const GDBusPropertyTable *property,
> > +                                        DBusMessageIter *value,
> > +                                        GDBusPendingPropertySet id, void *data)
> > +{
> > +       struct btd_device *device = data;
> > +       dbus_bool_t b;
> > +
> > +       if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_BOOLEAN) {
> > +               g_dbus_pending_property_error(id,
> > +                                       ERROR_INTERFACE ".InvalidArguments",
> > +                                       "Invalid arguments in method call");
> > +               return;
> > +       }
> > +
> > +       dbus_message_iter_get_basic(value, &b);
> > +
> > +       adapter_set_wake_capable(device->adapter, device, b == TRUE);
> > +       g_dbus_pending_property_success(id);
> > +}
> > +
> > +static gboolean dev_property_wake_capable_exist(
> > +               const GDBusPropertyTable *property, void *data)
> > +{
> > +       struct btd_device *device = data;
> > +
> > +       return device_get_profile_wake_support(device) ? TRUE : FALSE;
> > +}
> > +
> >  static gboolean disconnect_all(gpointer user_data)
> >  {
> >         struct btd_device *device = user_data;
> > @@ -1509,6 +1555,34 @@ 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_profile_wake_support(struct btd_device *device)
> > +{
> > +       return device->profile_wake_support;
> > +}
> > +
> > +void device_set_profile_wake_support(struct btd_device *device,
> > +                                    bool wake_support)
> > +{
> > +       device->profile_wake_support = wake_support;
> > +       /* WakeCapable is only visible when wake_support is true */
> > +       g_dbus_emit_property_changed(dbus_conn, device->path, DEVICE_INTERFACE,
> > +                                    "WakeCapable");
> > +}
> > +
> > +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)
> > +{
> > +       device->wake_capable = wake_capable;
> > +
> > +       store_device_info(device);
> > +       g_dbus_emit_property_changed(dbus_conn, device->path, DEVICE_INTERFACE,
> > +                                    "WakeCapable");
> > +}
> > +
> >  static void device_set_auto_connect(struct btd_device *device, gboolean enable)
> >  {
> >         char addr[18];
> > @@ -2779,6 +2853,9 @@ static const GDBusPropertyTable device_properties[] = {
> >         { "AdvertisingData", "a{yv}", dev_property_get_advertising_data,
> >                                 NULL, dev_property_advertising_data_exist,
> >                                 G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
> > +       { "WakeCapable", "b", dev_property_get_wake_capable,
> > +                               dev_property_set_wake_capable,
> > +                               dev_property_wake_capable_exist },
> >         { }
> >  };
> >
> > @@ -3030,6 +3107,7 @@ static void load_info(struct btd_device *device, const char *local,
> >         char *str;
> >         gboolean store_needed = FALSE;
> >         gboolean blocked;
> > +       gboolean wake_capable;
> >         char **uuids;
> >         int source, vendor, product, version;
> >         char **techno, **t;
> > @@ -3141,6 +3219,11 @@ next:
> >                 btd_device_set_pnpid(device, source, vendor, product, version);
> >         }
> >
> > +       /* Mark wake capable */
> > +       wake_capable = g_key_file_get_boolean(key_file, "General",
> > +                                             "WakeCapable", NULL);
> > +       adapter_set_wake_capable(device->adapter, device, wake_capable == TRUE);
> > +
> >         if (store_needed)
> >                 store_device_info(device);
> >  }
> > diff --git a/src/device.h b/src/device.h
> > index 06b100499..43f633e38 100644
> > --- a/src/device.h
> > +++ b/src/device.h
> > @@ -139,6 +139,11 @@ 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_profile_wake_support(struct btd_device *device);
> > +void device_set_profile_wake_support(struct btd_device *device,
> > +                                    bool wake_support);
> > +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

--
Thanks,
Abhishek

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [BlueZ PATCH v3 2/5] device: Allow device to be marked as wake capable
  2020-01-28 22:12   ` Abhishek Pandit-Subedi
@ 2020-01-29  4:03     ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2020-01-29  4:03 UTC (permalink / raw)
  To: Abhishek Pandit-Subedi
  Cc: Marcel Holtmann, Alain Michaud, linux-bluetooth,
	chromeos-bluetooth-upstreaming

Hi Abhishek,

On Tue, Jan 28, 2020 at 2:12 PM Abhishek Pandit-Subedi
<abhishekpandit@chromium.org> wrote:
>
> Hi Luiz,
>
> On Tue, Jan 28, 2020 at 12:29 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Abhishek,
> >
> > On Tue, Jan 28, 2020 at 10:44 AM Abhishek Pandit-Subedi
> > <abhishekpandit@chromium.org> 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. We also
> > > add the dbus endpoint to allow the wake capable setting to be
> > > controlled.
> > >
> > > ---
> > >
> > > Changes in v3:
> > > * Added profile_wake_support and made wake_capable dependent on it
> > >
> > > Changes in v2:
> > > * Added dbus api "WakeCapable" to set value
> > > * Update device_set_wake_capable to be called by
> > >   adapter_set_wake_capable_complete so we can emit property changed
> > >
> > >  lib/mgmt.h    |  9 ++++++
> > >  src/adapter.c | 65 ++++++++++++++++++++++++++++++++++++++++
> > >  src/adapter.h |  2 ++
> > >  src/device.c  | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  src/device.h  |  5 ++++
> > >  5 files changed, 164 insertions(+)
> > >
> > > diff --git a/lib/mgmt.h b/lib/mgmt.h
> > > index 276445d0a..cf19dd086 100644
> > > --- a/lib/mgmt.h
> > > +++ b/lib/mgmt.h
> > > @@ -599,6 +599,13 @@ struct mgmt_cp_set_blocked_keys {
> > >         struct mgmt_blocked_key_info keys[0];
> > >  } __packed;
> > >
> > > +#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_enable;
> > > +} __packed;
> > > +
> > >  #define MGMT_EV_CMD_COMPLETE           0x0001
> > >  struct mgmt_ev_cmd_complete {
> > >         uint16_t opcode;
> > > @@ -893,6 +900,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..294a9c9e4 100644
> > > --- a/src/adapter.c
> > > +++ b/src/adapter.c
> > > @@ -4685,6 +4685,71 @@ 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;
> >
> > Extra empty line not needed.
>
> My git merging skills have failed me yet again. Will fix in next patch.
>
> >
> > > +       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_enable,
> > > +                         mgmt_errstr(status), status);
> > > +               return;
> > > +       }
> > > +
> > > +       device_set_wake_capable(dev, rp->wake_enable);
> > > +
> > > +       DBG("Set wake capable complete %s (%u)", addr, rp->addr.type);
> > > +}
> > > +
> > > +void adapter_set_wake_capable(struct btd_adapter* adapter,
> > > +                             struct btd_device* dev,
> > > +                             bool wake_enable)
> > > +{
> > > +       struct mgmt_cp_set_wake_capable cp;
> > > +       char addr[18];
> >
> > Shouldn't there be a way to check if the kernel does actually support
> > the command first? We shouldn't be attempting to use it with kernels
> > that don't support it and the property should also not appear in that
> > case, so it is only visible if:
> >
> > 1. Kernel reports that Wake Capable command is supported
> > 2. A driver/plugin/profile calls device_set_wake_support
> >
> > Also in case those 2 conditions are attended bu the command fails we
> > should reply with an error since there won't be any PropertiesChanged
> > to indicate something went wrong.
>
> That sounds reasonable. Are all supported management apis reported
> somewhere? (I may discover the answer to this just by reading code)
>
> Also, what's a good way to report an error on PropertiesChanged? Just
> logging the error (i.e. error("Failed to set wake capable property"))
> or actually calling something like g_dbus_emit_property_error (I don't
> think this is real; just using an example)?

Look for g_dbus_pending_property_error

> >
> > > +       memset(&cp, 0, sizeof(cp));
> > > +       bacpy(&cp.addr.bdaddr, device_get_address(dev));
> > > +       cp.addr.type = btd_device_get_bdaddr_type(dev);
> > > +       cp.wake_enable = device_get_profile_wake_support(dev) && wake_enable;
> > > +
> > > +       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_enable, addr, cp.addr.type);
> > > +       }
> > > +
> > > +       DBG("Setting %s (%u) to wake capable = %u", addr,
> > > +           cp.addr.type, cp.wake_enable);
> > > +}
> > > +
> > >  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..e990279ed 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, bool wake_enable);
> > >
> > >  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..7a455e1e9 100644
> > > --- a/src/device.c
> > > +++ b/src/device.c
> > > @@ -189,6 +189,8 @@ struct btd_device {
> > >         bool            le;
> > >         bool            pending_paired;         /* "Paired" waiting for SDP */
> > >         bool            svc_refreshed;
> > > +       bool            profile_wake_support;   /* Profile supports wake */
> >
> > I guess we can drop the profile term here since that is implicit already.
>
> Will change in next patch.
>
> >
> > > +       bool            wake_capable;           /* Can wake from suspend */
> > >         GSList          *svc_callbacks;
> > >         GSList          *eir_uuids;
> > >         struct bt_ad    *ad;
> > > @@ -415,6 +417,9 @@ static gboolean store_device_info_cb(gpointer user_data)
> > >         g_key_file_set_boolean(key_file, "General", "Blocked",
> > >                                                         device->blocked);
> > >
> > > +       g_key_file_set_boolean(key_file, "General", "WakeCapable",
> > > +                                                       device->wake_capable);
> > > +
> > >         if (device->uuids) {
> > >                 GSList *l;
> > >                 int i;
> > > @@ -1318,6 +1323,47 @@ dev_property_advertising_data_exist(const GDBusPropertyTable *property,
> > >         return bt_ad_has_data(device->ad, NULL);
> > >  }
> > >
> > > +static gboolean
> > > +dev_property_get_wake_capable(const GDBusPropertyTable *property,
> > > +                            DBusMessageIter *iter, void *data)
> > > +{
> > > +       struct btd_device *device = data;
> > > +       dbus_bool_t wake_capable =
> > > +                       device_get_wake_capable(device) ? TRUE : FALSE;
> > > +
> > > +       dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &wake_capable);
> > > +
> > > +       return TRUE;
> > > +}
> > > +
> > > +static void dev_property_set_wake_capable(const GDBusPropertyTable *property,
> > > +                                        DBusMessageIter *value,
> > > +                                        GDBusPendingPropertySet id, void *data)
> > > +{
> > > +       struct btd_device *device = data;
> > > +       dbus_bool_t b;
> > > +
> > > +       if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_BOOLEAN) {
> > > +               g_dbus_pending_property_error(id,
> > > +                                       ERROR_INTERFACE ".InvalidArguments",
> > > +                                       "Invalid arguments in method call");
> > > +               return;
> > > +       }
> > > +
> > > +       dbus_message_iter_get_basic(value, &b);
> > > +
> > > +       adapter_set_wake_capable(device->adapter, device, b == TRUE);
> > > +       g_dbus_pending_property_success(id);
> > > +}
> > > +
> > > +static gboolean dev_property_wake_capable_exist(
> > > +               const GDBusPropertyTable *property, void *data)
> > > +{
> > > +       struct btd_device *device = data;
> > > +
> > > +       return device_get_profile_wake_support(device) ? TRUE : FALSE;
> > > +}
> > > +
> > >  static gboolean disconnect_all(gpointer user_data)
> > >  {
> > >         struct btd_device *device = user_data;
> > > @@ -1509,6 +1555,34 @@ 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_profile_wake_support(struct btd_device *device)
> > > +{
> > > +       return device->profile_wake_support;
> > > +}
> > > +
> > > +void device_set_profile_wake_support(struct btd_device *device,
> > > +                                    bool wake_support)
> > > +{
> > > +       device->profile_wake_support = wake_support;
> > > +       /* WakeCapable is only visible when wake_support is true */
> > > +       g_dbus_emit_property_changed(dbus_conn, device->path, DEVICE_INTERFACE,
> > > +                                    "WakeCapable");
> > > +}
> > > +
> > > +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)
> > > +{
> > > +       device->wake_capable = wake_capable;
> > > +
> > > +       store_device_info(device);
> > > +       g_dbus_emit_property_changed(dbus_conn, device->path, DEVICE_INTERFACE,
> > > +                                    "WakeCapable");
> > > +}
> > > +
> > >  static void device_set_auto_connect(struct btd_device *device, gboolean enable)
> > >  {
> > >         char addr[18];
> > > @@ -2779,6 +2853,9 @@ static const GDBusPropertyTable device_properties[] = {
> > >         { "AdvertisingData", "a{yv}", dev_property_get_advertising_data,
> > >                                 NULL, dev_property_advertising_data_exist,
> > >                                 G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
> > > +       { "WakeCapable", "b", dev_property_get_wake_capable,
> > > +                               dev_property_set_wake_capable,
> > > +                               dev_property_wake_capable_exist },
> > >         { }
> > >  };
> > >
> > > @@ -3030,6 +3107,7 @@ static void load_info(struct btd_device *device, const char *local,
> > >         char *str;
> > >         gboolean store_needed = FALSE;
> > >         gboolean blocked;
> > > +       gboolean wake_capable;
> > >         char **uuids;
> > >         int source, vendor, product, version;
> > >         char **techno, **t;
> > > @@ -3141,6 +3219,11 @@ next:
> > >                 btd_device_set_pnpid(device, source, vendor, product, version);
> > >         }
> > >
> > > +       /* Mark wake capable */
> > > +       wake_capable = g_key_file_get_boolean(key_file, "General",
> > > +                                             "WakeCapable", NULL);
> > > +       adapter_set_wake_capable(device->adapter, device, wake_capable == TRUE);
> > > +
> > >         if (store_needed)
> > >                 store_device_info(device);
> > >  }
> > > diff --git a/src/device.h b/src/device.h
> > > index 06b100499..43f633e38 100644
> > > --- a/src/device.h
> > > +++ b/src/device.h
> > > @@ -139,6 +139,11 @@ 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_profile_wake_support(struct btd_device *device);
> > > +void device_set_profile_wake_support(struct btd_device *device,
> > > +                                    bool wake_support);
> > > +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
>
> --
> Thanks,
> Abhishek



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [BlueZ PATCH v3 2/5] device: Allow device to be marked as wake capable
  2020-01-28  2:05 ` [BlueZ PATCH v3 2/5] device: Allow device " Abhishek Pandit-Subedi
@ 2020-01-28 18:43   ` Abhishek Pandit-Subedi
  0 siblings, 0 replies; 6+ messages in thread
From: Abhishek Pandit-Subedi @ 2020-01-28 18:43 UTC (permalink / raw)
  To: Luiz Augusto von Dentz, Marcel Holtmann, Alain Michaud
  Cc: Bluez mailing list, chromeos-bluetooth-upstreaming

Please ignore the first version of this -- it has a duplicated
function entry (I made a mistake while doing merges between branches
and must have forgotten to compile check right before sending).

Will resend just this email with the fix.

On Mon, Jan 27, 2020 at 6:05 PM Abhishek Pandit-Subedi
<abhishekpandit@chromium.org> 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. We also
> add the dbus endpoint to allow the wake capable setting to be
> controlled.
>
> ---
>
> Changes in v3:
> * Added profile_wake_support and made wake_capable dependent on it
>
> Changes in v2:
> * Added dbus api "WakeCapable" to set value
> * Update device_set_wake_capable to be called by
>   adapter_set_wake_capable_complete so we can emit property changed
>
>  lib/mgmt.h    |   9 ++++
>  src/adapter.c |  65 ++++++++++++++++++++++++++++
>  src/adapter.h |   2 +
>  src/device.c  | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/device.h  |   5 +++
>  5 files changed, 197 insertions(+)
>
> diff --git a/lib/mgmt.h b/lib/mgmt.h
> index 276445d0a..cf19dd086 100644
> --- a/lib/mgmt.h
> +++ b/lib/mgmt.h
> @@ -599,6 +599,13 @@ struct mgmt_cp_set_blocked_keys {
>         struct mgmt_blocked_key_info keys[0];
>  } __packed;
>
> +#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_enable;
> +} __packed;
> +
>  #define MGMT_EV_CMD_COMPLETE           0x0001
>  struct mgmt_ev_cmd_complete {
>         uint16_t opcode;
> @@ -893,6 +900,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..294a9c9e4 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -4685,6 +4685,71 @@ 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_enable,
> +                         mgmt_errstr(status), status);
> +               return;
> +       }
> +
> +       device_set_wake_capable(dev, rp->wake_enable);
> +
> +       DBG("Set wake capable complete %s (%u)", addr, rp->addr.type);
> +}
> +
> +void adapter_set_wake_capable(struct btd_adapter* adapter,
> +                             struct btd_device* dev,
> +                             bool wake_enable)
> +{
> +       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_enable = device_get_profile_wake_support(dev) && wake_enable;
> +
> +       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_enable, addr, cp.addr.type);
> +       }
> +
> +       DBG("Setting %s (%u) to wake capable = %u", addr,
> +           cp.addr.type, cp.wake_enable);
> +}
> +
>  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..e990279ed 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, bool wake_enable);
>
>  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..6b152bb19 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -189,6 +189,8 @@ struct btd_device {
>         bool            le;
>         bool            pending_paired;         /* "Paired" waiting for SDP */
>         bool            svc_refreshed;
> +       bool            profile_wake_support;   /* Profile supports wake */
> +       bool            wake_capable;           /* Can wake from suspend */
>         GSList          *svc_callbacks;
>         GSList          *eir_uuids;
>         struct bt_ad    *ad;
> @@ -415,6 +417,9 @@ static gboolean store_device_info_cb(gpointer user_data)
>         g_key_file_set_boolean(key_file, "General", "Blocked",
>                                                         device->blocked);
>
> +       g_key_file_set_boolean(key_file, "General", "WakeCapable",
> +                                                       device->wake_capable);
> +
>         if (device->uuids) {
>                 GSList *l;
>                 int i;
> @@ -1318,6 +1323,80 @@ dev_property_advertising_data_exist(const GDBusPropertyTable *property,
>         return bt_ad_has_data(device->ad, NULL);
>  }
>
> +static gboolean
> +dev_property_get_wake_capable(const GDBusPropertyTable *property,
> +                            DBusMessageIter *iter, void *data)
> +{
> +       struct btd_device *device = data;
> +       dbus_bool_t wake_capable =
> +                       device_get_wake_capable(device) ? TRUE : FALSE;
> +
> +       dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &wake_capable);
> +
> +       return TRUE;
> +}
> +
> +static void dev_property_set_wake_capable(const GDBusPropertyTable *property,
> +                                        DBusMessageIter *value,
> +                                        GDBusPendingPropertySet id, void *data)
> +{
> +       struct btd_device *device = data;
> +       dbus_bool_t b;
> +
> +       if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_BOOLEAN) {
> +               g_dbus_pending_property_error(id,
> +                                       ERROR_INTERFACE ".InvalidArguments",
> +                                       "Invalid arguments in method call");
> +               return;
> +       }
> +
> +       dbus_message_iter_get_basic(value, &b);
> +
> +       adapter_set_wake_capable(device->adapter, device, b == TRUE);
> +       g_dbus_pending_property_success(id);
> +}
> +
> +static gboolean
> +dev_property_get_wake_capable(const GDBusPropertyTable *property,
> +                            DBusMessageIter *iter, void *data)
> +{
> +       struct btd_device *device = data;
> +       dbus_bool_t wake_capable =
> +                       device_get_wake_capable(device) ? TRUE : FALSE;
> +
> +       dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &wake_capable);
> +
> +       return TRUE;
> +}
> +
> +static void dev_property_set_wake_capable(const GDBusPropertyTable *property,
> +                                        DBusMessageIter *value,
> +                                        GDBusPendingPropertySet id, void *data)
> +{
> +       struct btd_device *device = data;
> +       dbus_bool_t b;
> +
> +       if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_BOOLEAN) {
> +               g_dbus_pending_property_error(id,
> +                                       ERROR_INTERFACE ".InvalidArguments",
> +                                       "Invalid arguments in method call");
> +               return;
> +       }
> +
> +       dbus_message_iter_get_basic(value, &b);
> +
> +       adapter_set_wake_capable(device->adapter, device, b == TRUE);
> +       g_dbus_pending_property_success(id);
> +}
> +
> +static gboolean dev_property_wake_capable_exist(
> +               const GDBusPropertyTable *property, void *data)
> +{
> +       struct btd_device *device = data;
> +
> +       return device_get_profile_wake_support(device) ? TRUE : FALSE;
> +}
> +
>  static gboolean disconnect_all(gpointer user_data)
>  {
>         struct btd_device *device = user_data;
> @@ -1509,6 +1588,34 @@ 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_profile_wake_support(struct btd_device *device)
> +{
> +       return device->profile_wake_support;
> +}
> +
> +void device_set_profile_wake_support(struct btd_device *device,
> +                                    bool wake_support)
> +{
> +       device->profile_wake_support = wake_support;
> +       /* WakeCapable is only visible when wake_support is true */
> +       g_dbus_emit_property_changed(dbus_conn, device->path, DEVICE_INTERFACE,
> +                                    "WakeCapable");
> +}
> +
> +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)
> +{
> +       device->wake_capable = wake_capable;
> +
> +       store_device_info(device);
> +       g_dbus_emit_property_changed(dbus_conn, device->path, DEVICE_INTERFACE,
> +                                    "WakeCapable");
> +}
> +
>  static void device_set_auto_connect(struct btd_device *device, gboolean enable)
>  {
>         char addr[18];
> @@ -2779,6 +2886,9 @@ static const GDBusPropertyTable device_properties[] = {
>         { "AdvertisingData", "a{yv}", dev_property_get_advertising_data,
>                                 NULL, dev_property_advertising_data_exist,
>                                 G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
> +       { "WakeCapable", "b", dev_property_get_wake_capable,
> +                               dev_property_set_wake_capable,
> +                               dev_property_wake_capable_exist },
>         { }
>  };
>
> @@ -3030,6 +3140,7 @@ static void load_info(struct btd_device *device, const char *local,
>         char *str;
>         gboolean store_needed = FALSE;
>         gboolean blocked;
> +       gboolean wake_capable;
>         char **uuids;
>         int source, vendor, product, version;
>         char **techno, **t;
> @@ -3141,6 +3252,11 @@ next:
>                 btd_device_set_pnpid(device, source, vendor, product, version);
>         }
>
> +       /* Mark wake capable */
> +       wake_capable = g_key_file_get_boolean(key_file, "General",
> +                                             "WakeCapable", NULL);
> +       adapter_set_wake_capable(device->adapter, device, wake_capable == TRUE);
> +
>         if (store_needed)
>                 store_device_info(device);
>  }
> diff --git a/src/device.h b/src/device.h
> index 06b100499..43f633e38 100644
> --- a/src/device.h
> +++ b/src/device.h
> @@ -139,6 +139,11 @@ 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_profile_wake_support(struct btd_device *device);
> +void device_set_profile_wake_support(struct btd_device *device,
> +                                    bool wake_support);
> +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
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [BlueZ PATCH v3 2/5] device: Allow device to be marked as wake capable
  2020-01-28  2:05 [BlueZ PATCH v3 0/5] device: Allow devices " Abhishek Pandit-Subedi
@ 2020-01-28  2:05 ` " Abhishek Pandit-Subedi
  2020-01-28 18:43   ` Abhishek Pandit-Subedi
  0 siblings, 1 reply; 6+ messages in thread
From: Abhishek Pandit-Subedi @ 2020-01-28  2:05 UTC (permalink / raw)
  To: luiz.dentz, marcel, alainm
  Cc: linux-bluetooth, chromeos-bluetooth-upstreaming, Abhishek Pandit-Subedi

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. We also
add the dbus endpoint to allow the wake capable setting to be
controlled.

---

Changes in v3:
* Added profile_wake_support and made wake_capable dependent on it

Changes in v2:
* Added dbus api "WakeCapable" to set value
* Update device_set_wake_capable to be called by
  adapter_set_wake_capable_complete so we can emit property changed

 lib/mgmt.h    |   9 ++++
 src/adapter.c |  65 ++++++++++++++++++++++++++++
 src/adapter.h |   2 +
 src/device.c  | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++
 src/device.h  |   5 +++
 5 files changed, 197 insertions(+)

diff --git a/lib/mgmt.h b/lib/mgmt.h
index 276445d0a..cf19dd086 100644
--- a/lib/mgmt.h
+++ b/lib/mgmt.h
@@ -599,6 +599,13 @@ struct mgmt_cp_set_blocked_keys {
 	struct mgmt_blocked_key_info keys[0];
 } __packed;
 
+#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_enable;
+} __packed;
+
 #define MGMT_EV_CMD_COMPLETE		0x0001
 struct mgmt_ev_cmd_complete {
 	uint16_t opcode;
@@ -893,6 +900,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..294a9c9e4 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -4685,6 +4685,71 @@ 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_enable,
+			  mgmt_errstr(status), status);
+		return;
+	}
+
+	device_set_wake_capable(dev, rp->wake_enable);
+
+	DBG("Set wake capable complete %s (%u)", addr, rp->addr.type);
+}
+
+void adapter_set_wake_capable(struct btd_adapter* adapter,
+			      struct btd_device* dev,
+			      bool wake_enable)
+{
+	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_enable = device_get_profile_wake_support(dev) && wake_enable;
+
+	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_enable, addr, cp.addr.type);
+	}
+
+	DBG("Setting %s (%u) to wake capable = %u", addr,
+	    cp.addr.type, cp.wake_enable);
+}
+
 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..e990279ed 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, bool wake_enable);
 
 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..6b152bb19 100644
--- a/src/device.c
+++ b/src/device.c
@@ -189,6 +189,8 @@ struct btd_device {
 	bool		le;
 	bool		pending_paired;		/* "Paired" waiting for SDP */
 	bool		svc_refreshed;
+	bool		profile_wake_support;	/* Profile supports wake */
+	bool		wake_capable;		/* Can wake from suspend */
 	GSList		*svc_callbacks;
 	GSList		*eir_uuids;
 	struct bt_ad	*ad;
@@ -415,6 +417,9 @@ static gboolean store_device_info_cb(gpointer user_data)
 	g_key_file_set_boolean(key_file, "General", "Blocked",
 							device->blocked);
 
+	g_key_file_set_boolean(key_file, "General", "WakeCapable",
+							device->wake_capable);
+
 	if (device->uuids) {
 		GSList *l;
 		int i;
@@ -1318,6 +1323,80 @@ dev_property_advertising_data_exist(const GDBusPropertyTable *property,
 	return bt_ad_has_data(device->ad, NULL);
 }
 
+static gboolean
+dev_property_get_wake_capable(const GDBusPropertyTable *property,
+			     DBusMessageIter *iter, void *data)
+{
+	struct btd_device *device = data;
+	dbus_bool_t wake_capable =
+			device_get_wake_capable(device) ? TRUE : FALSE;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &wake_capable);
+
+	return TRUE;
+}
+
+static void dev_property_set_wake_capable(const GDBusPropertyTable *property,
+					 DBusMessageIter *value,
+					 GDBusPendingPropertySet id, void *data)
+{
+	struct btd_device *device = data;
+	dbus_bool_t b;
+
+	if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_BOOLEAN) {
+		g_dbus_pending_property_error(id,
+					ERROR_INTERFACE ".InvalidArguments",
+					"Invalid arguments in method call");
+		return;
+	}
+
+	dbus_message_iter_get_basic(value, &b);
+
+	adapter_set_wake_capable(device->adapter, device, b == TRUE);
+	g_dbus_pending_property_success(id);
+}
+
+static gboolean
+dev_property_get_wake_capable(const GDBusPropertyTable *property,
+			     DBusMessageIter *iter, void *data)
+{
+	struct btd_device *device = data;
+	dbus_bool_t wake_capable =
+			device_get_wake_capable(device) ? TRUE : FALSE;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &wake_capable);
+
+	return TRUE;
+}
+
+static void dev_property_set_wake_capable(const GDBusPropertyTable *property,
+					 DBusMessageIter *value,
+					 GDBusPendingPropertySet id, void *data)
+{
+	struct btd_device *device = data;
+	dbus_bool_t b;
+
+	if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_BOOLEAN) {
+		g_dbus_pending_property_error(id,
+					ERROR_INTERFACE ".InvalidArguments",
+					"Invalid arguments in method call");
+		return;
+	}
+
+	dbus_message_iter_get_basic(value, &b);
+
+	adapter_set_wake_capable(device->adapter, device, b == TRUE);
+	g_dbus_pending_property_success(id);
+}
+
+static gboolean dev_property_wake_capable_exist(
+		const GDBusPropertyTable *property, void *data)
+{
+	struct btd_device *device = data;
+
+	return device_get_profile_wake_support(device) ? TRUE : FALSE;
+}
+
 static gboolean disconnect_all(gpointer user_data)
 {
 	struct btd_device *device = user_data;
@@ -1509,6 +1588,34 @@ 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_profile_wake_support(struct btd_device *device)
+{
+	return device->profile_wake_support;
+}
+
+void device_set_profile_wake_support(struct btd_device *device,
+				     bool wake_support)
+{
+	device->profile_wake_support = wake_support;
+	/* WakeCapable is only visible when wake_support is true */
+	g_dbus_emit_property_changed(dbus_conn, device->path, DEVICE_INTERFACE,
+				     "WakeCapable");
+}
+
+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)
+{
+	device->wake_capable = wake_capable;
+
+	store_device_info(device);
+	g_dbus_emit_property_changed(dbus_conn, device->path, DEVICE_INTERFACE,
+				     "WakeCapable");
+}
+
 static void device_set_auto_connect(struct btd_device *device, gboolean enable)
 {
 	char addr[18];
@@ -2779,6 +2886,9 @@ static const GDBusPropertyTable device_properties[] = {
 	{ "AdvertisingData", "a{yv}", dev_property_get_advertising_data,
 				NULL, dev_property_advertising_data_exist,
 				G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "WakeCapable", "b", dev_property_get_wake_capable,
+				dev_property_set_wake_capable,
+				dev_property_wake_capable_exist },
 	{ }
 };
 
@@ -3030,6 +3140,7 @@ static void load_info(struct btd_device *device, const char *local,
 	char *str;
 	gboolean store_needed = FALSE;
 	gboolean blocked;
+	gboolean wake_capable;
 	char **uuids;
 	int source, vendor, product, version;
 	char **techno, **t;
@@ -3141,6 +3252,11 @@ next:
 		btd_device_set_pnpid(device, source, vendor, product, version);
 	}
 
+	/* Mark wake capable */
+	wake_capable = g_key_file_get_boolean(key_file, "General",
+					      "WakeCapable", NULL);
+	adapter_set_wake_capable(device->adapter, device, wake_capable == TRUE);
+
 	if (store_needed)
 		store_device_info(device);
 }
diff --git a/src/device.h b/src/device.h
index 06b100499..43f633e38 100644
--- a/src/device.h
+++ b/src/device.h
@@ -139,6 +139,11 @@ 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_profile_wake_support(struct btd_device *device);
+void device_set_profile_wake_support(struct btd_device *device,
+				     bool wake_support);
+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


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-28 18:43 [BlueZ PATCH v3 2/5] device: Allow device to be marked as wake capable Abhishek Pandit-Subedi
2020-01-28 20:29 ` Luiz Augusto von Dentz
2020-01-28 22:12   ` Abhishek Pandit-Subedi
2020-01-29  4:03     ` Luiz Augusto von Dentz
  -- strict thread matches above, loose matches on Subject: below --
2020-01-28  2:05 [BlueZ PATCH v3 0/5] device: Allow devices " Abhishek Pandit-Subedi
2020-01-28  2:05 ` [BlueZ PATCH v3 2/5] device: Allow device " Abhishek Pandit-Subedi
2020-01-28 18:43   ` Abhishek Pandit-Subedi

Linux-Bluetooth Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-bluetooth/0 linux-bluetooth/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-bluetooth linux-bluetooth/ https://lore.kernel.org/linux-bluetooth \
		linux-bluetooth@vger.kernel.org
	public-inbox-index linux-bluetooth

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-bluetooth


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git