[RFC v0 1/5] adapter: rename discovery_client to watch_client

[RFC v0 1/5] adapter: rename discovery_client to watch_client

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Rename to a more generic name since we will have different uses for this.
---
 src/adapter.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 5f01edf..0277057 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -105,7 +105,7 @@ static uint8_t mgmt_revision = 0;
 
 static GSList *adapter_drivers = NULL;
 
-struct discovery_client {
+struct watch_client {
 	struct btd_adapter *adapter;
 	char *owner;
 	guint watch;
@@ -1480,7 +1480,7 @@ static void stop_discovery_complete(uint8_t status, uint16_t length,
 
 static int compare_discovery_sender(gconstpointer a, gconstpointer b)
 {
-	const struct discovery_client *client = a;
+	const struct watch_client *client = a;
 	const char *sender = b;
 
 	return g_strcmp0(client->owner, sender);
@@ -1522,7 +1522,7 @@ static gboolean remove_temp_devices(gpointer user_data)
 
 static void discovery_destroy(void *user_data)
 {
-	struct discovery_client *client = user_data;
+	struct watch_client *client = user_data;
 	struct btd_adapter *adapter = client->adapter;
 
 	DBG("owner %s", client->owner);
@@ -1560,7 +1560,7 @@ static void discovery_destroy(void *user_data)
 
 static void discovery_disconnect(DBusConnection *conn, void *user_data)
 {
-	struct discovery_client *client = user_data;
+	struct watch_client *client = user_data;
 	struct btd_adapter *adapter = client->adapter;
 	struct mgmt_cp_stop_discovery cp;
 
@@ -1604,7 +1604,7 @@ static DBusMessage *start_discovery(DBusConnection *conn,
 {
 	struct btd_adapter *adapter = user_data;
 	const char *sender = dbus_message_get_sender(msg);
-	struct discovery_client *client;
+	struct watch_client *client;
 	GSList *list;
 
 	DBG("sender %s", sender);
@@ -1621,7 +1621,7 @@ static DBusMessage *start_discovery(DBusConnection *conn,
 	if (list)
 		return btd_error_busy(msg);
 
-	client = g_new0(struct discovery_client, 1);
+	client = g_new0(struct watch_client, 1);
 
 	client->adapter = adapter;
 	client->owner = g_strdup(sender);
@@ -1648,7 +1648,7 @@ static DBusMessage *stop_discovery(DBusConnection *conn,
 	struct btd_adapter *adapter = user_data;
 	const char *sender = dbus_message_get_sender(msg);
 	struct mgmt_cp_stop_discovery cp;
-	struct discovery_client *client;
+	struct watch_client *client;
 	GSList *list;
 
 	DBG("sender %s", sender);
@@ -4264,7 +4264,7 @@ static void adapter_stop(struct btd_adapter *adapter)
 	cancel_passive_scanning(adapter);
 
 	while (adapter->discovery_list) {
-		struct discovery_client *client;
+		struct watch_client *client;
 
 		client = adapter->discovery_list->data;
 
-- 
1.8.1.4

[RFC v0 2/5] adapter: rename compare_discovery_sender

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

The new name "compare_sender" is more generic and will be used for more
than one purpose.
---
 src/adapter.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 0277057..bb5737e 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1478,7 +1478,7 @@ static void stop_discovery_complete(uint8_t status, uint16_t length,
 	}
 }
 
-static int compare_discovery_sender(gconstpointer a, gconstpointer b)
+static int compare_sender(gconstpointer a, gconstpointer b)
 {
 	const struct watch_client *client = a;
 	const char *sender = b;
@@ -1617,7 +1617,7 @@ static DBusMessage *start_discovery(DBusConnection *conn,
 	 * already started a discovery then return an error.
 	 */
 	list = g_slist_find_custom(adapter->discovery_list, sender,
-						compare_discovery_sender);
+						compare_sender);
 	if (list)
 		return btd_error_busy(msg);
 
@@ -1657,7 +1657,7 @@ static DBusMessage *stop_discovery(DBusConnection *conn,
 		return btd_error_not_ready(msg);
 
 	list = g_slist_find_custom(adapter->discovery_list, sender,
-						compare_discovery_sender);
+						compare_sender);
 	if (!list)
 		return btd_error_failed(msg, "No discovery started");
 
-- 
1.8.1.4

[RFC v0 3/5] doc: add RequestDiscoverable() and ReleaseDiscoverable()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

those methods should be used when the user wants BlueZ to tracks the
clients lifetime and release the discoverable session if the client goes
away without calling ReleaseDiscoverable().
When the last session is dsregistered Discoverable should be back to
False.
---
 doc/adapter-api.txt | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
index 74d235a..bb29795 100644
--- a/doc/adapter-api.txt
+++ b/doc/adapter-api.txt
@@ -35,6 +35,34 @@ Methods		void StartDiscovery()
 					 org.bluez.Error.Failed
 					 org.bluez.Error.NotAuthorized
 
+		void RequestDiscoverable()
+
+			This method starts an adapter Discoverable session,
+			if the Discoverable is False it will be set to True
+			and the D-Bus client who called it will be recorded
+			as the session owner. If it is True we only record
+			the session owner.
+
+			Setting the Discoverable property through the Set()
+			method is discouraged if you are using this method.
+
+			To release a session call ReleaseDiscoverable(). If
+			a client exits without calling it the session will
+			automatically removed.
+
+			Possible errors: org.bluez.Error.NotReady
+					 org.bluez.Error.Failed
+
+		void ReleaseDiscoverable()
+
+			This method removes a Discoverable session. When called
+			it will remove the session and make Discoverable False
+			if session removed is the last one.
+
+			Possible errors: org.bluez.Error.NotReady
+					 org.bluez.Error.Failed
+					 org.bluez.Error.NotAuthorized
+
 		void RemoveDevice(object device)
 
 			This removes the remote device object at the given
@@ -111,6 +139,10 @@ Properties	string Address [readonly]
 
 			For any new adapter this settings defaults to false.
 
+			If your system is using RequestDiscoverable() and
+			ReleaseDiscoverable() to make the adapter discoverable
+			you should not set this property directly.
+
 		boolean Pairable [readwrite]
 
 			Switch an adapter to pairable or non-pairable. This is
@@ -140,6 +172,10 @@ Properties	string Address [readonly]
 			The default value for the discoverable timeout should
 			be 180 seconds (3 minutes).
 
+			If your system is using RequestDiscoverable() and
+			ReleaseDiscoverable() to make the adapter discoverable
+			you should not set this property directly.
+
 		boolean Discovering [readonly]
 
 			Indicates that a device discovery procedure is active.
-- 
1.8.1.4

[RFC v0 4/5] adapter: add RequestDiscoverable() and ReleaseDiscoverable()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

With those two methods BlueZ is now aware of client lifetime and can set
Discoverable back to False if all clients exit or call
ReleaseDiscoverable()
---
 src/adapter.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index bb5737e..c59b526 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -169,6 +169,7 @@ struct btd_adapter {
 	GSList *connections;		/* Connected devices */
 	GSList *devices;		/* Devices structure pointers */
 	GSList *connect_list;		/* Devices to connect when found */
+	GSList *discoverable_list;	/* list fo discoverable sessions */
 	struct btd_device *connect_le;	/* LE device waiting to be connected */
 	sdp_list_t *services;		/* Services associated to adapter */
 
@@ -1699,6 +1700,111 @@ static DBusMessage *stop_discovery(DBusConnection *conn,
 	return dbus_message_new_method_return(msg);
 }
 
+static void discoverable_destroy(void *user_data)
+{
+	struct watch_client *client = user_data;
+	struct btd_adapter *adapter = client->adapter;
+
+	DBG("owner %s", client->owner);
+
+	adapter->discoverable_list = g_slist_remove(adapter->discoverable_list,
+								client);
+
+	g_free(client->owner);
+	g_free(client);
+}
+
+static void discoverable_disconnect(DBusConnection *conn, void *user_data)
+{
+	struct watch_client *client = user_data;
+	struct btd_adapter *adapter = client->adapter;
+
+	DBG("owner %s", client->owner);
+
+	adapter->discoverable_list = g_slist_remove(adapter->discoverable_list,
+								client);
+
+	if (adapter->discoverable_list)
+		return;
+
+	set_discoverable(adapter, 0x00, 0);
+}
+
+static DBusMessage *request_discoverable(DBusConnection *conn,
+					DBusMessage *msg, void *user_data)
+{
+	struct btd_adapter *adapter = user_data;
+	const char *sender = dbus_message_get_sender(msg);
+	struct watch_client *client;
+
+	DBG("sender %s", sender);
+
+	if (!(adapter->current_settings & MGMT_SETTING_POWERED))
+		return btd_error_not_ready(msg);
+
+	/*
+	 * Every client can only start one discoverable session, if the client
+	 * already started a discoverable session then return an error.
+	 */
+	if (g_slist_find_custom(adapter->discoverable_list, sender,
+						compare_sender))
+		return btd_error_busy(msg);
+
+	client = g_new0(struct watch_client, 1);
+
+	client->adapter = adapter;
+	client->owner = g_strdup(sender);
+	client->watch = g_dbus_add_disconnect_watch(dbus_conn, sender,
+						discoverable_disconnect, client,
+						discoverable_destroy);
+
+	adapter->discoverable_list = g_slist_prepend(adapter->discoverable_list,
+								client);
+	if (adapter->current_settings & MGMT_SETTING_DISCOVERABLE)
+		return dbus_message_new_method_return(msg);
+
+	if (!set_discoverable(adapter,0x01, 0))
+		return btd_error_failed(msg, "Failed to set Discoverable");
+
+	return dbus_message_new_method_return(msg);
+}
+
+static DBusMessage *release_discoverable(DBusConnection *conn,
+					DBusMessage *msg, void *user_data)
+{
+	struct btd_adapter *adapter = user_data;
+	const char *sender = dbus_message_get_sender(msg);
+	struct watch_client *client;
+	GSList *list;
+
+	DBG("sender %s", sender);
+
+	if (!(adapter->current_settings & MGMT_SETTING_POWERED))
+		return btd_error_not_ready(msg);
+
+	list = g_slist_find_custom(adapter->discoverable_list, sender,
+						compare_sender);
+	if (!list)
+		return btd_error_failed(msg, "No discovery started");
+
+	client = list->data;
+
+	/*
+	 * The destroy function will cleanup the client information and
+	 * also remove it from the list of discoverable clients.
+	 */
+	g_dbus_remove_watch(dbus_conn, client->watch);
+
+	/* If it is the last session set discoverable to false */
+	if (adapter->discoverable_list)
+		return dbus_message_new_method_return(msg);
+
+	if (!set_discoverable(adapter,0x00, 0))
+		return btd_error_failed(msg, "Failed to set Discoverable");
+
+	return dbus_message_new_method_return(msg);
+}
+
 static gboolean property_get_address(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *user_data)
 {
@@ -2153,6 +2259,10 @@ static DBusMessage *remove_device(DBusConnection *conn,
 static const GDBusMethodTable adapter_methods[] = {
 	{ GDBUS_METHOD("StartDiscovery", NULL, NULL, start_discovery) },
 	{ GDBUS_METHOD("StopDiscovery", NULL, NULL, stop_discovery) },
+	{ GDBUS_METHOD("RequestDiscoverable", NULL, NULL,
+						request_discoverable) },
+	{ GDBUS_METHOD("ReleaseDiscoverable", NULL, NULL,
+						release_discoverable) },
 	{ GDBUS_ASYNC_METHOD("RemoveDevice",
 			GDBUS_ARGS({ "device", "o" }), NULL, remove_device) },
 	{ }
-- 
1.8.1.4

[RFC v0 5/5] test: add requestdiscoverable command to test-adapter

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

---
 test/test-adapter | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/test/test-adapter b/test/test-adapter
index 5deeda4..764330d 100755
--- a/test/test-adapter
+++ b/test/test-adapter
@@ -136,6 +136,14 @@ if (args[0] == "discoverabletimeout"):
 		adapter.Set("org.bluez.Adapter1", "DiscoverableTimeout", to)
 	sys.exit(0)
 
+if (args[0] == "requestdiscoverable"):
+	adapter1 = dbus.Interface(bus.get_object("org.bluez", adapter_path),
+					"org.bluez.Adapter1")
+	adapter1.RequestDiscoverable()
+	time.sleep(20);
+	adapter1.ReleaseDiscoverable()
+	sys.exit(0)
+
 if (args[0] == "discovering"):
 	discovering = adapter.Get("org.bluez.Adapter1", "Discovering")
 	print(discovering)
-- 
1.8.1.4

Re: [RFC v0 4/5] adapter: add RequestDiscoverable() and ReleaseDiscoverable()

[Luiz Augusto von Dentz]

Hi Gustavo,

On Thu, Jun 6, 2013 at 9:18 PM, Gustavo Padovan <gustavo@padovan.org> wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>
> With those two methods BlueZ is now aware of client lifetime and can set
> Discoverable back to False if all clients exit or call
> ReleaseDiscoverable()
> ---

This probably should be handled int the component controlling the
powered state like we used to have with RequestSession or we can have
an agent to the power/policy manager asking if it is ok to power on
the adapter.

Re: [RFC v0 4/5] adapter: add RequestDiscoverable() and ReleaseDiscoverable()

[Gustavo Padovan]

Hi Luiz,

* Luiz Augusto von Dentz <luiz.dentz@gmail.com> [2013-06-06 22:50:32 +0700]:

> Hi Gustavo,
> 
> On Thu, Jun 6, 2013 at 9:18 PM, Gustavo Padovan <gustavo@padovan.org> wrote:
> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> >
> > With those two methods BlueZ is now aware of client lifetime and can set
> > Discoverable back to False if all clients exit or call
> > ReleaseDiscoverable()
> > ---
> 
> This probably should be handled int the component controlling the
> powered state like we used to have with RequestSession or we can have
> an agent to the power/policy manager asking if it is ok to power on
> the adapter.

Not sure if it makes sense to put this in the component that handles power
state (you mean ConnMan here, for example?). How an agent for this can help
us. The problem I'm trying to solve here is, do not let the Discoverable True
for infinite time if the who is using crashes.

	Gustavo

Re: [RFC v0 4/5] adapter: add RequestDiscoverable() and ReleaseDiscoverable()

[Luiz Augusto von Dentz]

Hi Gustavo,

On Thu, Jun 6, 2013 at 10:59 PM, Gustavo Padovan <gustavo@padovan.org> wrote:
> Hi Luiz,
>
> * Luiz Augusto von Dentz <luiz.dentz@gmail.com> [2013-06-06 22:50:32 +0700]:
>
>> Hi Gustavo,
>>
>> On Thu, Jun 6, 2013 at 9:18 PM, Gustavo Padovan <gustavo@padovan.org> wrote:
>> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>> >
>> > With those two methods BlueZ is now aware of client lifetime and can set
>> > Discoverable back to False if all clients exit or call
>> > ReleaseDiscoverable()
>> > ---
>>
>> This probably should be handled int the component controlling the
>> powered state like we used to have with RequestSession or we can have
>> an agent to the power/policy manager asking if it is ok to power on
>> the adapter.
>
> Not sure if it makes sense to put this in the component that handles power
> state (you mean ConnMan here, for example?). How an agent for this can help
> us. The problem I'm trying to solve here is, do not let the Discoverable True
> for infinite time if the who is using crashes.

There is the exact same problem with Powered isn't it? If the
application crashes will stay on, actually with an agent we can always
request the component controlling it e.g. ConnMan to confirm the
action.


--
Luiz Augusto von Dentz

Re: [RFC v0 4/5] adapter: add RequestDiscoverable() and ReleaseDiscoverable()

[Gustavo Padovan]

Hi Luiz,

* Luiz Augusto von Dentz <luiz.dentz@gmail.com> [2013-06-06 23:06:36 +0700]:

> Hi Gustavo,
> 
> On Thu, Jun 6, 2013 at 10:59 PM, Gustavo Padovan <gustavo@padovan.org> wrote:
> > Hi Luiz,
> >
> > * Luiz Augusto von Dentz <luiz.dentz@gmail.com> [2013-06-06 22:50:32 +0700]:
> >
> >> Hi Gustavo,
> >>
> >> On Thu, Jun 6, 2013 at 9:18 PM, Gustavo Padovan <gustavo@padovan.org> wrote:
> >> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> >> >
> >> > With those two methods BlueZ is now aware of client lifetime and can set
> >> > Discoverable back to False if all clients exit or call
> >> > ReleaseDiscoverable()
> >> > ---
> >>
> >> This probably should be handled int the component controlling the
> >> powered state like we used to have with RequestSession or we can have
> >> an agent to the power/policy manager asking if it is ok to power on
> >> the adapter.
> >
> > Not sure if it makes sense to put this in the component that handles power
> > state (you mean ConnMan here, for example?). How an agent for this can help
> > us. The problem I'm trying to solve here is, do not let the Discoverable True
> > for infinite time if the who is using crashes.
> 
> There is the exact same problem with Powered isn't it? If the
> application crashes will stay on, actually with an agent we can always
> request the component controlling it e.g. ConnMan to confirm the
> action.

Yes, Powered has the same problem, but didn't ConnMan itself would need to
register "Powered session" on BlueZ as well. The I see it BlueZ is the session
manager, not ConnMan. In your mind ConnMan would also control things like
Discoverable or Pairable?

	Gustavo

Re: [RFC v0 4/5] adapter: add RequestDiscoverable() and ReleaseDiscoverable()

[Mikel Astiz]

Hi,

On Thu, Jun 6, 2013 at 6:20 PM, Gustavo Padovan <gustavo@padovan.org> wrote:
> Hi Luiz,
>
> * Luiz Augusto von Dentz <luiz.dentz@gmail.com> [2013-06-06 23:06:36 +0700]:
>
>> Hi Gustavo,
>>
>> On Thu, Jun 6, 2013 at 10:59 PM, Gustavo Padovan <gustavo@padovan.org> wrote:
>> > Hi Luiz,
>> >
>> > * Luiz Augusto von Dentz <luiz.dentz@gmail.com> [2013-06-06 22:50:32 +0700]:
>> >
>> >> Hi Gustavo,
>> >>
>> >> On Thu, Jun 6, 2013 at 9:18 PM, Gustavo Padovan <gustavo@padovan.org> wrote:
>> >> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>> >> >
>> >> > With those two methods BlueZ is now aware of client lifetime and can set
>> >> > Discoverable back to False if all clients exit or call
>> >> > ReleaseDiscoverable()
>> >> > ---
>> >>
>> >> This probably should be handled int the component controlling the
>> >> powered state like we used to have with RequestSession or we can have
>> >> an agent to the power/policy manager asking if it is ok to power on
>> >> the adapter.
>> >
>> > Not sure if it makes sense to put this in the component that handles power
>> > state (you mean ConnMan here, for example?). How an agent for this can help
>> > us. The problem I'm trying to solve here is, do not let the Discoverable True
>> > for infinite time if the who is using crashes.
>>
>> There is the exact same problem with Powered isn't it? If the
>> application crashes will stay on, actually with an agent we can always
>> request the component controlling it e.g. ConnMan to confirm the
>> action.
>
> Yes, Powered has the same problem, but didn't ConnMan itself would need to
> register "Powered session" on BlueZ as well. The I see it BlueZ is the session
> manager, not ConnMan. In your mind ConnMan would also control things like
> Discoverable or Pairable?
>

The way I see this there is a difference between Powered and
Pairable/Discoverable.

Powered can be controlled by Connman and that's it. If it crashes, it
should restart and recover.

Pairable/Discoverable are IMO different, and what Gustavo proposes
makes sense to me. A possible improvement would be to request both
pairable and discoverable in a single call.

In the longer term, however, I would be in favor of coupling this with
the registration of the (pairing) agent. The UI would call
RequestPairingMode(object agent) which would do everything together.
The only exceptions here would be the Authorize() and
ConfirmModeChange() methods, which are the only ones in
org.bluez.Agent1 that are unrelated to pairing.

Cheers,
Mikel