Skip to contentSkip to conversation

gdbus: Add g_dbus_client_get_proxy

PATCH In review Luiz Augusto von Dentz 2 participants 15 messages Jan 11, 2013 – Jan 13, 2013 Bluetooth drivers

lore
Share & tools
lore

Luiz Augusto von Dentz

author maintainer
#0 lore raw

PATCH 1/7 gdbus: Add g_dbus_client_get_proxy

From: Luiz Augusto von Dentz <[email protected]>

g_dbus_client_get_proxy gives the possibilitity to just check if a proxy exist for the given path and interface pair instead of using g_dbus_proxy_new which end up creating a proxy if it doesn't exists which is not always necessary.

---
 gdbus/client.c | 8 ++++----
 gdbus/gdbus.h  | 3 +++
 2 files changed, 7 insertions(+), 4 deletions(-)
gdbus/client.c +4 -4
index c03e3a4..ea56023 100644
@@ -328,8 +328,8 @@ static void get_all_properties(GDBusProxy *proxy)
 	dbus_message_unref(msg);
 }
 
-static GDBusProxy *proxy_lookup(GDBusClient *client, const char *path,
-						const char *interface)
+GDBusProxy *g_dbus_client_get_proxy(GDBusClient *client, const char *path,
+							const char *interface)
 {
 	GList *list;
 

@@ -423,7 +423,7 @@ GDBusProxy *g_dbus_proxy_new(GDBusClient *client, const char *path,
 	if (client == NULL)
 		return NULL;
 
-	proxy = proxy_lookup(client, path, interface);
+	proxy = g_dbus_client_get_proxy(client, path, interface);
 	if (proxy)
 		return g_dbus_proxy_ref(proxy);
 

@@ -841,7 +841,7 @@ static void parse_properties(GDBusClient *client, const char *path,
 	if (g_str_equal(interface, DBUS_INTERFACE_PROPERTIES) == TRUE)
 		return;
 
-	proxy = proxy_lookup(client, path, interface);
+	proxy = g_dbus_client_get_proxy(client, path, interface);
 	if (proxy) {
 		update_properties(proxy, iter);
 		return;
gdbus/gdbus.h +3 -0
index 6f5a012..77bd069 100644
@@ -360,6 +360,9 @@ gboolean g_dbus_client_set_proxy_handlers(GDBusClient *client,
 					GDBusPropertyFunction property_changed,
 					void *user_data);
 
+GDBusProxy *g_dbus_client_get_proxy(GDBusClient *client, const char *path,
+							const char *interface);
+
 #ifdef __cplusplus
 }
 #endif
Signature · 1 line
1.8.0.1
author maintainer
#1 lore raw

PATCH 2/7 gdbus: Add g_dbus_proxy_get_client function

From: Luiz Augusto von Dentz <[email protected]>

This is convenient as some callbacks don't provide the client which the proxy belongs.

---
 gdbus/client.c | 8 ++++++++
 gdbus/gdbus.h  | 1 +
 2 files changed, 9 insertions(+)
gdbus/client.c +8 -0
index ea56023..3c5784b 100644
@@ -478,6 +478,14 @@ const char *g_dbus_proxy_get_interface(GDBusProxy *proxy)
 	return proxy->interface;
 }
 
+GDBusClient *g_dbus_proxy_get_client(GDBusProxy *proxy)
+{
+	if (proxy == NULL)
+		return NULL;
+
+	return proxy->client;
+}
+
 gboolean g_dbus_proxy_get_property(GDBusProxy *proxy, const char *name,
                                                         DBusMessageIter *iter)
 {
gdbus/gdbus.h +1 -0
index 77bd069..83e41c7 100644
@@ -313,6 +313,7 @@ void g_dbus_proxy_unref(GDBusProxy *proxy);
 
 const char *g_dbus_proxy_get_path(GDBusProxy *proxy);
 const char *g_dbus_proxy_get_interface(GDBusProxy *proxy);
+GDBusClient *g_dbus_proxy_get_client(GDBusProxy *proxy);
 
 gboolean g_dbus_proxy_get_property(GDBusProxy *proxy, const char *name,
 							DBusMessageIter *iter);
Signature · 1 line
1.8.0.1
maintainer
#2 lore raw

Re: [PATCH BlueZ 2/7] gdbus: Add g_dbus_proxy_get_client function

Hi Luiz,

Quoted 2 lines

This is convenient as some callbacks don't provide the client which the proxy belongs.

and I did not add that one on purpose. You should know where your proxy belongs to and worst case hand it over via user_data.

Regards

Marcel

author maintainer
#3 lore raw

Re: [PATCH BlueZ 2/7] gdbus: Add g_dbus_proxy_get_client function

Hi Marcel,

Quoted 7 lines · Marcel Holtmann

Hi Luiz,

Quoted 2 lines

This is convenient as some callbacks don't provide the client which the proxy belongs.

and I did not add that one on purpose. You should know where your proxy belongs to and worst case hand it over via user_data.

I understand this as the user_data will always be the client pointer, or a struct containing it, or you have to use a global variable, is that what you really want? The use application has given the client when registering the callbacks, so it is already passing it as context. Now that being said, Im not saying the design utterly broken, quite the contrary it work like a charm, it just could be more convenient and that is what Im trying to achieve with this patch.

--
Luiz Augusto von Dentz

maintainer
#4 lore raw

Re: [PATCH BlueZ 2/7] gdbus: Add g_dbus_proxy_get_client function

Hi Luiz,

Quoted 6 lines
Quoted 5 lines
Quoted 2 lines

This is convenient as some callbacks don't provide the client which the proxy belongs.

and I did not add that one on purpose. You should know where your proxy belongs to and worst case hand it over via user_data.

Quoted 7 lines

I understand this as the user_data will always be the client pointer, or a struct containing it, or you have to use a global variable, is that what you really want? The use application has given the client when registering the callbacks, so it is already passing it as context. Now that being said, Im not saying the design utterly broken, quite the contrary it work like a charm, it just could be more convenient and that is what Im trying to achieve with this patch.

it is a convenience for one special kind of quick-and-dirty hack that normal application will never need. I have made these mistakes in the past and rather have not sneaking up on me again. So lets not do this.

Regards

Marcel

author maintainer
#5 lore raw

PATCH 3/7 unit: Add gdbus/client_get_dict_property

In reply to Luiz Augusto von Dentz

From: Luiz Augusto von Dentz <[email protected]>

---
 unit/test-gdbus-client.c | 147 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 147 insertions(+)
unit/test-gdbus-client.c +147 -0
index 34c714e..dac0cb5 100644
@@ -81,6 +81,8 @@ static struct context *create_context(void)
 	/* Avoid D-Bus library calling _exit() before next test finishes. */
 	dbus_connection_set_exit_on_disconnect(context->dbus_conn, FALSE);
 
+	g_dbus_attach_object_manager(context->dbus_conn);
+
 	return context;
 }
 

@@ -184,6 +186,148 @@ static void client_connect_disconnect(void)
 	destroy_context(context);
 }
 
+static void append_variant(DBusMessageIter *iter, int type, void *val)
+{
+	DBusMessageIter value;
+	char sig[2] = { type, '\0' };
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_VARIANT, sig, &value);
+
+	dbus_message_iter_append_basic(&value, type, val);
+
+	dbus_message_iter_close_container(iter, &value);
+}
+
+static void dict_append_entry(DBusMessageIter *dict, const char *key, int type,
+								void *val)
+{
+	DBusMessageIter entry;
+
+	if (type == DBUS_TYPE_STRING) {
+		const char *str = *((const char **) val);
+		if (str == NULL)
+			return;
+	}
+
+	dbus_message_iter_open_container(dict, DBUS_TYPE_DICT_ENTRY,
+							NULL, &entry);
+
+	dbus_message_iter_append_basic(&entry, DBUS_TYPE_STRING, &key);
+
+	append_variant(&entry, type, val);
+
+	dbus_message_iter_close_container(dict, &entry);
+}
+
+static gboolean get_dict(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	DBusMessageIter dict;
+	const char *string = "value";
+	dbus_bool_t boolean = TRUE;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
+			DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+			DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
+			DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
+
+	dict_append_entry(&dict, "String", DBUS_TYPE_STRING, &string);
+	dict_append_entry(&dict, "Boolean", DBUS_TYPE_BOOLEAN, &boolean);
+
+	dbus_message_iter_close_container(iter, &dict);
+
+	return TRUE;
+}
+
+static void proxy_get_dict(GDBusProxy *proxy, void *user_data)
+{
+	struct context *context = user_data;
+	DBusMessageIter iter, dict, var1, var2, entry1, entry2;
+	const char *string;
+	dbus_bool_t boolean;
+
+	if (g_test_verbose())
+		g_print("proxy %s found\n",
+					g_dbus_proxy_get_interface(proxy));
+
+	g_assert(g_dbus_proxy_get_property(proxy, "Dict", &iter));
+	g_assert(dbus_message_iter_get_arg_type(&iter) == DBUS_TYPE_ARRAY);
+
+	dbus_message_iter_recurse(&iter, &dict);
+	g_assert(dbus_message_iter_get_arg_type(&dict) ==
+							DBUS_TYPE_DICT_ENTRY);
+
+	dbus_message_iter_recurse(&dict, &entry1);
+	g_assert(dbus_message_iter_get_arg_type(&entry1) == DBUS_TYPE_STRING);
+
+	dbus_message_iter_get_basic(&entry1, &string);
+	g_assert(g_strcmp0(string, "String") == 0);
+
+	dbus_message_iter_next(&entry1);
+	g_assert(dbus_message_iter_get_arg_type(&entry1) == DBUS_TYPE_VARIANT);
+
+	dbus_message_iter_recurse(&entry1, &var1);
+	g_assert(dbus_message_iter_get_arg_type(&var1) == DBUS_TYPE_STRING);
+
+	dbus_message_iter_get_basic(&var1, &string);
+	g_assert(g_strcmp0(string, "value") == 0);
+
+	dbus_message_iter_next(&dict);
+	g_assert(dbus_message_iter_get_arg_type(&dict) ==
+							DBUS_TYPE_DICT_ENTRY);
+
+	dbus_message_iter_recurse(&dict, &entry2);
+	g_assert(dbus_message_iter_get_arg_type(&entry2) == DBUS_TYPE_STRING);
+
+	dbus_message_iter_get_basic(&entry2, &string);
+	g_assert(g_strcmp0(string, "Boolean") == 0);
+
+	dbus_message_iter_next(&entry2);
+	g_assert(dbus_message_iter_get_arg_type(&entry2) == DBUS_TYPE_VARIANT);
+
+	dbus_message_iter_recurse(&entry2, &var2);
+	g_assert(dbus_message_iter_get_arg_type(&var2) == DBUS_TYPE_BOOLEAN);
+
+	dbus_message_iter_get_basic(&var2, &boolean);
+	g_assert(boolean == TRUE);
+
+	dbus_message_iter_next(&dict);
+	g_assert(dbus_message_iter_get_arg_type(&dict) == DBUS_TYPE_INVALID);
+
+	g_main_loop_quit(context->main_loop);
+}
+
+static void client_get_dict_property(void)
+{
+	struct context *context = create_context();
+	static const GDBusPropertyTable dict_properties[] = {
+		{ "Dict", "a{sv}", get_dict },
+		{ },
+	};
+
+	if (context == NULL)
+		return;
+
+	g_dbus_register_interface(context->dbus_conn,
+				SERVICE_PATH, SERVICE_NAME,
+				methods, signals, dict_properties,
+				NULL, NULL);
+
+	context->dbus_client = g_dbus_client_new(context->dbus_conn,
+						SERVICE_NAME, SERVICE_PATH);
+
+	g_dbus_client_set_proxy_handlers(context->dbus_client, proxy_get_dict,
+						NULL, NULL,
+						context);
+
+	g_main_loop_run(context->main_loop);
+
+	g_dbus_unregister_interface(context->dbus_conn,
+					SERVICE_PATH, SERVICE_NAME);
+
+	destroy_context(context);
+}
+
 int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);

@@ -193,5 +337,8 @@ int main(int argc, char *argv[])
 	g_test_add_func("/gdbus/client_connect_disconnect",
 						client_connect_disconnect);
 
+	g_test_add_func("/gdbus/client_get_dict_property",
+						client_get_dict_property);
+
 	return g_test_run();
 }
Signature · 1 line
1.8.0.1
author maintainer
#6 lore raw

PATCH 4/7 unit: Add gdbus/client_get_string_property

In reply to Luiz Augusto von Dentz

From: Luiz Augusto von Dentz <[email protected]>

---
 unit/test-gdbus-client.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)
unit/test-gdbus-client.c +65 -0
index dac0cb5..bcdd811 100644
@@ -328,6 +328,68 @@ static void client_get_dict_property(void)
 	destroy_context(context);
 }
 
+static void proxy_get_string(GDBusProxy *proxy, void *user_data)
+{
+	struct context *context = user_data;
+	DBusMessageIter iter;
+	const char *string;
+
+	if (g_test_verbose())
+		g_print("proxy %s found\n",
+					g_dbus_proxy_get_interface(proxy));
+
+	g_assert(g_dbus_proxy_get_property(proxy, "String", &iter));
+	g_assert(dbus_message_iter_get_arg_type(&iter) == DBUS_TYPE_STRING);
+
+	dbus_message_iter_get_basic(&iter, &string);
+	g_assert(g_strcmp0(string, "value") == 0);
+
+	g_dbus_client_unref(context->dbus_client);
+
+	g_main_loop_quit(context->main_loop);
+}
+
+static gboolean get_string(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	const char *string = "value";
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &string);
+
+	return TRUE;
+}
+
+static void client_get_string_property(void)
+{
+	struct context *context = create_context();
+	static const GDBusPropertyTable string_properties[] = {
+		{ "String", "s", get_string },
+		{ },
+	};
+
+	if (context == NULL)
+		return;
+
+	g_dbus_register_interface(context->dbus_conn,
+				SERVICE_PATH, SERVICE_NAME,
+				methods, signals, string_properties,
+				NULL, NULL);
+
+	context->dbus_client = g_dbus_client_new(context->dbus_conn,
+						SERVICE_NAME, SERVICE_PATH);
+
+	g_dbus_client_set_proxy_handlers(context->dbus_client, proxy_get_string,
+						NULL, NULL,
+						context);
+
+	g_main_loop_run(context->main_loop);
+
+	g_dbus_unregister_interface(context->dbus_conn,
+					SERVICE_PATH, SERVICE_NAME);
+
+	destroy_context(context);
+}
+
 int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);

@@ -337,6 +399,9 @@ int main(int argc, char *argv[])
 	g_test_add_func("/gdbus/client_connect_disconnect",
 						client_connect_disconnect);
 
+	g_test_add_func("/gdbus/client_get_string_property",
+						client_get_string_property);
+
 	g_test_add_func("/gdbus/client_get_dict_property",
 						client_get_dict_property);
Signature · 1 line
1.8.0.1
author maintainer
#7 lore raw

PATCH 5/7 unit: Add gdbus/client_get_boolean_property

In reply to Luiz Augusto von Dentz

From: Luiz Augusto von Dentz <[email protected]>

---
 unit/test-gdbus-client.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)
unit/test-gdbus-client.c +66 -0
index bcdd811..e26dd0c 100644
@@ -390,6 +390,69 @@ static void client_get_string_property(void)
 	destroy_context(context);
 }
 
+static void proxy_get_boolean(GDBusProxy *proxy, void *user_data)
+{
+	struct context *context = user_data;
+	DBusMessageIter iter;
+	dbus_bool_t value;
+
+	if (g_test_verbose())
+		g_print("proxy %s found\n",
+					g_dbus_proxy_get_interface(proxy));
+
+	g_assert(g_dbus_proxy_get_property(proxy, "Boolean", &iter));
+	g_assert(dbus_message_iter_get_arg_type(&iter) == DBUS_TYPE_BOOLEAN);
+
+	dbus_message_iter_get_basic(&iter, &value);
+	g_assert(value == TRUE);
+
+	g_dbus_client_unref(context->dbus_client);
+
+	g_main_loop_quit(context->main_loop);
+}
+
+static gboolean get_boolean(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	dbus_bool_t value = TRUE;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &value);
+
+	return TRUE;
+}
+
+static void client_get_boolean_property(void)
+{
+	struct context *context = create_context();
+	static const GDBusPropertyTable boolean_properties[] = {
+		{ "Boolean", "b", get_boolean },
+		{ },
+	};
+
+	if (context == NULL)
+		return;
+
+	g_dbus_register_interface(context->dbus_conn,
+				SERVICE_PATH, SERVICE_NAME,
+				methods, signals, boolean_properties,
+				NULL, NULL);
+
+	context->dbus_client = g_dbus_client_new(context->dbus_conn,
+						SERVICE_NAME, SERVICE_PATH);
+
+	g_dbus_client_set_proxy_handlers(context->dbus_client,
+						proxy_get_boolean,
+						NULL, NULL,
+						context);
+
+	g_main_loop_run(context->main_loop);
+
+	g_dbus_unregister_interface(context->dbus_conn,
+					SERVICE_PATH, SERVICE_NAME);
+
+	destroy_context(context);
+}
+
 int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);

@@ -402,6 +465,9 @@ int main(int argc, char *argv[])
 	g_test_add_func("/gdbus/client_get_string_property",
 						client_get_string_property);
 
+	g_test_add_func("/gdbus/client_get_boolean_property",
+						client_get_boolean_property);
+
 	g_test_add_func("/gdbus/client_get_dict_property",
 						client_get_dict_property);
Signature · 1 line
1.8.0.1
author maintainer
#8 lore raw

PATCH 6/7 unit: Add gdbus/client_get_array_property

In reply to Luiz Augusto von Dentz

From: Luiz Augusto von Dentz <[email protected]>

---
 unit/test-gdbus-client.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)
unit/test-gdbus-client.c +80 -0
index e26dd0c..c3ae4b9 100644
@@ -453,6 +453,83 @@ static void client_get_boolean_property(void)
 	destroy_context(context);
 }
 
+static void proxy_get_array(GDBusProxy *proxy, void *user_data)
+{
+	struct context *context = user_data;
+	DBusMessageIter iter, entry;
+	const char *value1, *value2;
+
+	if (g_test_verbose())
+		g_print("proxy %s found\n",
+					g_dbus_proxy_get_interface(proxy));
+
+	g_assert(g_dbus_proxy_get_property(proxy, "Array", &iter));
+	g_assert(dbus_message_iter_get_arg_type(&iter) == DBUS_TYPE_ARRAY);
+
+	dbus_message_iter_recurse(&iter, &entry);
+	g_assert(dbus_message_iter_get_arg_type(&entry) == DBUS_TYPE_STRING);
+
+	dbus_message_iter_get_basic(&entry, &value1);
+	g_assert(g_strcmp0(value1, "value1") == 0);
+
+	dbus_message_iter_next(&entry);
+	g_assert(dbus_message_iter_get_arg_type(&entry) == DBUS_TYPE_STRING);
+
+	dbus_message_iter_get_basic(&entry, &value2);
+	g_assert(g_strcmp0(value2, "value2") == 0);
+
+	g_dbus_client_unref(context->dbus_client);
+
+	g_main_loop_quit(context->main_loop);
+}
+
+static gboolean get_array(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	const char *value[2] = { "value1", "value2" };
+	DBusMessageIter array;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "s", &array);
+
+	dbus_message_iter_append_basic(&array, DBUS_TYPE_STRING, &value[0]);
+	dbus_message_iter_append_basic(&array, DBUS_TYPE_STRING, &value[1]);
+
+	dbus_message_iter_close_container(iter, &array);
+
+	return TRUE;
+}
+
+static void client_get_array_property(void)
+{
+	struct context *context = create_context();
+	static const GDBusPropertyTable array_properties[] = {
+		{ "Array", "as", get_array },
+		{ },
+	};
+
+	if (context == NULL)
+		return;
+
+	g_dbus_register_interface(context->dbus_conn,
+				SERVICE_PATH, SERVICE_NAME,
+				methods, signals, array_properties,
+				NULL, NULL);
+
+	context->dbus_client = g_dbus_client_new(context->dbus_conn,
+						SERVICE_NAME, SERVICE_PATH);
+
+	g_dbus_client_set_proxy_handlers(context->dbus_client, proxy_get_array,
+						NULL, NULL,
+						context);
+
+	g_main_loop_run(context->main_loop);
+
+	g_dbus_unregister_interface(context->dbus_conn,
+					SERVICE_PATH, SERVICE_NAME);
+
+	destroy_context(context);
+}
+
 int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);

@@ -468,6 +545,9 @@ int main(int argc, char *argv[])
 	g_test_add_func("/gdbus/client_get_boolean_property",
 						client_get_boolean_property);
 
+	g_test_add_func("/gdbus/client_get_array_property",
+						client_get_array_property);
+
 	g_test_add_func("/gdbus/client_get_dict_property",
 						client_get_dict_property);
Signature · 1 line
1.8.0.1
author maintainer
#9 lore raw

PATCH 7/7 unit: Add gdbus/client_get_uint64_property

In reply to Luiz Augusto von Dentz

From: Luiz Augusto von Dentz <[email protected]>

---
 unit/test-gdbus-client.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)
unit/test-gdbus-client.c +66 -0
index c3ae4b9..a777fe3 100644
@@ -530,6 +530,69 @@ static void client_get_array_property(void)
 	destroy_context(context);
 }
 
+static void proxy_get_uint64(GDBusProxy *proxy, void *user_data)
+{
+	struct context *context = user_data;
+	DBusMessageIter iter;
+	guint64 value;
+
+	if (g_test_verbose())
+		g_print("proxy %s found\n",
+					g_dbus_proxy_get_interface(proxy));
+
+	g_assert(g_dbus_proxy_get_property(proxy, "Number", &iter));
+	g_assert(dbus_message_iter_get_arg_type(&iter) == DBUS_TYPE_UINT64);
+
+	dbus_message_iter_get_basic(&iter, &value);
+	g_assert(value == G_MAXUINT64);
+
+	g_dbus_client_unref(context->dbus_client);
+
+	g_main_loop_quit(context->main_loop);
+}
+
+static gboolean get_uint64(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	guint64 value = G_MAXUINT64;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT64, &value);
+
+	return TRUE;
+}
+
+static void client_get_uint64_property(void)
+{
+	struct context *context = create_context();
+	static const GDBusPropertyTable uint64_properties[] = {
+		{ "Number", "t", get_uint64 },
+		{ },
+	};
+
+	if (context == NULL)
+		return;
+
+	g_dbus_register_interface(context->dbus_conn,
+				SERVICE_PATH, SERVICE_NAME,
+				methods, signals, uint64_properties,
+				NULL, NULL);
+
+	context->dbus_client = g_dbus_client_new(context->dbus_conn,
+						SERVICE_NAME, SERVICE_PATH);
+
+	g_dbus_client_set_proxy_handlers(context->dbus_client,
+						proxy_get_uint64,
+						NULL, NULL,
+						context);
+
+	g_main_loop_run(context->main_loop);
+
+	g_dbus_unregister_interface(context->dbus_conn,
+					SERVICE_PATH, SERVICE_NAME);
+
+	destroy_context(context);
+}
+
 int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);

@@ -545,6 +608,9 @@ int main(int argc, char *argv[])
 	g_test_add_func("/gdbus/client_get_boolean_property",
 						client_get_boolean_property);
 
+	g_test_add_func("/gdbus/client_get_uint64_property",
+						client_get_uint64_property);
+
 	g_test_add_func("/gdbus/client_get_array_property",
 						client_get_array_property);
Signature · 1 line
1.8.0.1
maintainer
#10 lore raw
In reply to Luiz Augusto von Dentz

Hi Luiz,

Quoted 4 lines

g_dbus_client_get_proxy gives the possibilitity to just check if a proxy exist for the given path and interface pair instead of using g_dbus_proxy_new which end up creating a proxy if it doesn't exists which is not always necessary.

why would we do that. You get the proxy via the client callbacks for proxy created or proxy removed.

The proxy_new method is for dealing with services that do not have ObjectManager support.

Regards

Marcel

author maintainer
#11 lore raw

Hi Marcel,

Quoted 9 lines · Marcel Holtmann

Hi Luiz,

Quoted 4 lines

g_dbus_client_get_proxy gives the possibilitity to just check if a proxy exist for the given path and interface pair instead of using g_dbus_proxy_new which end up creating a proxy if it doesn't exists which is not always necessary.

why would we do that. You get the proxy via the client callbacks for proxy created or proxy removed.

That way we the user don't have to maintain a list of found proxies, such a list exist and is maintained by GDBusClient.

Quoted 2 lines

The proxy_new method is for dealing with services that do not have ObjectManager support.

Yep, so in one way or the other you are already letting the user application search in the list of proxies maintained by GDBusClient, the only thing this does is to make the proxy_lookup public so the client don't have to maintain its how list only to be able to look back when a proxy point to another, btw the reason proxy_new is not enough is because it may lead to extra calls when the user application may just want to bail out if the proxy is not found.

--
Luiz Augusto von Dentz

maintainer
#12 lore raw

Hi Luiz,

Quoted 10 lines
Quoted 7 lines
Quoted 4 lines

g_dbus_client_get_proxy gives the possibilitity to just check if a proxy exist for the given path and interface pair instead of using g_dbus_proxy_new which end up creating a proxy if it doesn't exists which is not always necessary.

why would we do that. You get the proxy via the client callbacks for proxy created or proxy removed.

That way we the user don't have to maintain a list of found proxies, such a list exist and is maintained by GDBusClient.

see how bluetoothctl does it. Pretty damn simple. So I am not convinced that this is a good idea.

Quoted 10 lines
Quoted 2 lines

The proxy_new method is for dealing with services that do not have ObjectManager support.

Yep, so in one way or the other you are already letting the user application search in the list of proxies maintained by GDBusClient, the only thing this does is to make the proxy_lookup public so the client don't have to maintain its how list only to be able to look back when a proxy point to another, btw the reason proxy_new is not enough is because it may lead to extra calls when the user application may just want to bail out if the proxy is not found.

Still not convinced. What kind of application would this. This seems to be all half thought trough. I am really failing to see the real purpose here. Either you go all the way with ObjectManager and do it the right way or you don't have an object manager, then you need to manually trigger the get all properties.

Regards

Marcel

author maintainer
#13 lore raw

Hi Marcel,

Quoted 27 lines · Marcel Holtmann

Hi Luiz,

Quoted 10 lines
Quoted 7 lines
Quoted 4 lines

g_dbus_client_get_proxy gives the possibilitity to just check if a proxy exist for the given path and interface pair instead of using g_dbus_proxy_new which end up creating a proxy if it doesn't exists which is not always necessary.

why would we do that. You get the proxy via the client callbacks for proxy created or proxy removed.

That way we the user don't have to maintain a list of found proxies, such a list exist and is maintained by GDBusClient.

see how bluetoothctl does it. Pretty damn simple. So I am not convinced that this is a good idea.

Quoted 10 lines
Quoted 2 lines

The proxy_new method is for dealing with services that do not have ObjectManager support.

Yep, so in one way or the other you are already letting the user application search in the list of proxies maintained by GDBusClient, the only thing this does is to make the proxy_lookup public so the client don't have to maintain its how list only to be able to look back when a proxy point to another, btw the reason proxy_new is not enough is because it may lead to extra calls when the user application may just want to bail out if the proxy is not found.

Quoted 5 lines

Still not convinced. What kind of application would this. This seems to be all half thought trough. I am really failing to see the real purpose here. Either you go all the way with ObjectManager and do it the right way or you don't have an object manager, then you need to manually trigger the get all properties.

Im actually using these functions in an upcoming patch:

static void register_player(GDBusProxy *proxy)
{
...
client = g_dbus_proxy_get_client(proxy);

if (!g_dbus_proxy_get_property(proxy, "Device", &iter))

    return;

dbus_message_iter_get_basic(&iter, &path);

device = g_dbus_client_get_proxy(client, path, "org.bluez.Device1"); if (device == NULL)

    return;

if (!g_dbus_proxy_get_property(device, "Name", &iter))

    return;

...
static void proxy_added(GDBusProxy *proxy, void *user_data) {
...

    } else if (!strcmp(interface, BLUEZ_MEDIA_PLAYER_INTERFACE)) {
         printf("Bluetooth Player %s found\n", g_dbus_proxy_get_path(proxy));
         register_player(proxy);

}
...

With this I don't have to keep any list of found devices what so ever, if I need a proxy I can just query it via g_dbus_client_get_proxy. Now compare this to what bluetoothctl does:

static void proxy_added(GDBusProxy *proxy, void *user_data) {
...

    if (!strcmp(interface, "org.bluez.Device1")) {
        if (device_is_child(proxy, default_ctrl) == TRUE) {
            dev_list = g_list_append(dev_list, proxy);

            print_device(proxy, "NEW");
    } else if (!strcmp(interface, "org.bluez.Adapter1")) {
            ctrl_list = g_list_append(ctrl_list, proxy);

            if (!default_ctrl)
                default_ctrl = proxy;

             print_adapter(proxy, "NEW");

...

So you are keeping the list of proxies while GDBusClient keeps them too, now it maybe useful for bluetoothctl but for mpris-tool it is completely useless, why would I keep a proxy of a device that doesn't contain any player, specially while discovering devices this would just populate with useless temporary devices.

Btw, with these functions we could actually have the following changes to device_is_child:

client/main.c +3 -1
index 9a927a8..1cd6d10 100644
@@ -224,8 +224,10 @@ static void print_uuids(GDBusProxy *proxy)
 
 static gboolean device_is_child(GDBusProxy *device, GDBusProxy *master)
 {
+       GDBusClient *client;
+       GDBusProxy *proxy;
        DBusMessageIter iter;
-       const char *adapter, *path;
+       const char *path;
 
        if (!master)
                return FALSE;

@@ -233,13 +235,13 @@ static gboolean device_is_child(GDBusProxy

*device, GDBusProxy *master)

        if (g_dbus_proxy_get_property(device, "Adapter", &iter) == FALSE)
                return FALSE;
-       dbus_message_iter_get_basic(&iter, &adapter);
-       path = g_dbus_proxy_get_path(master);
+       dbus_message_iter_get_basic(&iter, &path);

- if (!strcmp(path, adapter))
- return TRUE;
+ client = g_dbus_proxy_get_client(device);

- return FALSE;
+ proxy = g_dbus_client_get_proxy(client, path, "org.bluez.Adapter1");
+
+ return proxy == master ? TRUE : FALSE;
}

So we compare the proxies not the path alone.

--
Luiz Augusto von Dentz

maintainer
#14 lore raw

Hi Luiz,

Quoted 78 lines
Quoted 25 lines
Quoted 10 lines
Quoted 7 lines
Quoted 4 lines

g_dbus_client_get_proxy gives the possibilitity to just check if a proxy exist for the given path and interface pair instead of using g_dbus_proxy_new which end up creating a proxy if it doesn't exists which is not always necessary.

why would we do that. You get the proxy via the client callbacks for proxy created or proxy removed.

That way we the user don't have to maintain a list of found proxies, such a list exist and is maintained by GDBusClient.

see how bluetoothctl does it. Pretty damn simple. So I am not convinced that this is a good idea.

Quoted 10 lines
Quoted 2 lines

The proxy_new method is for dealing with services that do not have ObjectManager support.

Yep, so in one way or the other you are already letting the user application search in the list of proxies maintained by GDBusClient, the only thing this does is to make the proxy_lookup public so the client don't have to maintain its how list only to be able to look back when a proxy point to another, btw the reason proxy_new is not enough is because it may lead to extra calls when the user application may just want to bail out if the proxy is not found.

Quoted 5 lines

Still not convinced. What kind of application would this. This seems to be all half thought trough. I am really failing to see the real purpose here. Either you go all the way with ObjectManager and do it the right way or you don't have an object manager, then you need to manually trigger the get all properties.

Im actually using these functions in an upcoming patch:

static void register_player(GDBusProxy *proxy)
{
...
client = g_dbus_proxy_get_client(proxy);

if (!g_dbus_proxy_get_property(proxy, "Device", &iter))

    return;

dbus_message_iter_get_basic(&iter, &path);

device = g_dbus_client_get_proxy(client, path, "org.bluez.Device1"); if (device == NULL)

    return;

if (!g_dbus_proxy_get_property(device, "Name", &iter))

    return;

...
static void proxy_added(GDBusProxy *proxy, void *user_data) {
...

    } else if (!strcmp(interface, BLUEZ_MEDIA_PLAYER_INTERFACE)) {
         printf("Bluetooth Player %s found\n", g_dbus_proxy_get_path(proxy));
         register_player(proxy);

}
...

With this I don't have to keep any list of found devices what so ever, if I need a proxy I can just query it via g_dbus_client_get_proxy. Now compare this to what bluetoothctl does:

static void proxy_added(GDBusProxy *proxy, void *user_data) {
...

    if (!strcmp(interface, "org.bluez.Device1")) {
        if (device_is_child(proxy, default_ctrl) == TRUE) {
            dev_list = g_list_append(dev_list, proxy);

            print_device(proxy, "NEW");
    } else if (!strcmp(interface, "org.bluez.Adapter1")) {
            ctrl_list = g_list_append(ctrl_list, proxy);

            if (!default_ctrl)
                default_ctrl = proxy;

             print_adapter(proxy, "NEW");
Quoted 7 lines

...

So you are keeping the list of proxies while GDBusClient keeps them too, now it maybe useful for bluetoothctl but for mpris-tool it is completely useless, why would I keep a proxy of a device that doesn't contain any player, specially while discovering devices this would just populate with useless temporary devices.

bluetoothctl only keeps the list of proxies that contain an interface it is interested in. And the same could be done for mpris-tool. If you get an object path with a player interface, you remember it, otherwise you just ignore it.

Quoted 21 lines

Btw, with these functions we could actually have the following changes to device_is_child:

client/main.c +3 -1
index 9a927a8..1cd6d10 100644
@@ -224,8 +224,10 @@ static void print_uuids(GDBusProxy *proxy)
 
 static gboolean device_is_child(GDBusProxy *device, GDBusProxy *master)
 {
+       GDBusClient *client;
+       GDBusProxy *proxy;
        DBusMessageIter iter;
-       const char *adapter, *path;
+       const char *path;
 
        if (!master)
                return FALSE;

@@ -233,13 +235,13 @@ static gboolean device_is_child(GDBusProxy
 *device, GDBusProxy *master)

client/main.c static gboolean device_is_child(GDBusProxy · line 241

Quoted 10 lines
client/main.c +2 -4
        if (g_dbus_proxy_get_property(device, "Adapter", &iter) == FALSE)
                return FALSE;
 
-       dbus_message_iter_get_basic(&iter, &adapter);
-       path = g_dbus_proxy_get_path(master);
+       dbus_message_iter_get_basic(&iter, &path);
 
-       if (!strcmp(path, adapter))
-               return TRUE;
+       client = g_dbus_proxy_get_client(device);

And this I have single client, I could just make this global. So I am still not buying into this one.

client/main.c static gboolean device_is_child(GDBusProxy · line 245

Quoted 5 lines
client/main.c +3 -1
-       return FALSE;
+       proxy = g_dbus_client_get_proxy(client, path, "org.bluez.Adapter1");
+
+       return proxy == master ? TRUE : FALSE;
 }

Don't see how this makes it simpler code. Or ends up in less instructions for that matter.

Regards

Marcel