On Wed, 2012-12-19 at 13:58 +0100, Mikel Astiz wrote:
> From: Mikel Astiz <mikel.astiz at bmw-carit.de>
>
> Use the new interface name if BlueZ 5 has been detected.
> ---
> src/modules/bluetooth/bluetooth-util.c | 52 +++++++++++++++++++++++++---------
> 1 file changed, 38 insertions(+), 14 deletions(-)
>
> diff --git a/src/modules/bluetooth/bluetooth-util.c b/src/modules/bluetooth/bluetooth-util.c
> index 67b3240..0d18c4d 100644
> --- a/src/modules/bluetooth/bluetooth-util.c
> +++ b/src/modules/bluetooth/bluetooth-util.c
> @@ -61,6 +61,30 @@
> " </interface>" \
> "</node>"
>
> +#define BLUEZ_5_ENDPOINT_INTROSPECT_XML \
I think "MEDIA_ENDPOINT_1_INTROSPECT_XML" would be a better name. The
reason is that the introspection data isn't necessarily tied to the
bluez version number - bluez 5 may introduce MediaEndpoint2, or bluez 6
may still use MediaEndpoint1.
> + DBUS_INTROSPECT_1_0_XML_DOCTYPE_DECL_NODE \
> + "<node>" \
> + " <interface name=\"org.bluez.MediaEndpoint1\">" \
> + " <method name=\"SetConfiguration\">" \
> + " <arg name=\"transport\" direction=\"in\" type=\"o\"/>" \
> + " <arg name=\"configuration\" direction=\"in\" type=\"ay\"/>" \
> + " </method>" \
> + " <method name=\"SelectConfiguration\">" \
> + " <arg name=\"capabilities\" direction=\"in\" type=\"ay\"/>" \
> + " <arg name=\"configuration\" direction=\"out\" type=\"ay\"/>" \
> + " </method>" \
> + " <method name=\"ClearConfiguration\">" \
> + " </method>" \
> + " <method name=\"Release\">" \
> + " </method>" \
> + " </interface>" \
> + " <interface name=\"org.freedesktop.DBus.Introspectable\">" \
> + " <method name=\"Introspect\">" \
> + " <arg name=\"data\" type=\"s\" direction=\"out\"/>" \
> + " </method>" \
> + " </interface>" \
> + "</node>"
> +
> typedef enum pa_bluez_version {
> BLUEZ_VERSION_UNKNOWN,
> BLUEZ_VERSION_4,
> @@ -1526,7 +1550,7 @@ static DBusMessage *endpoint_set_configuration(DBusConnection *conn, DBusMessage
> dbus_message_iter_get_basic(&args, &path);
Not a problem of this patch, but it seems that
endpoint_set_configuration() doesn't check that the received message
actually contains the path argument.
>
> if (pa_hashmap_get(y->transports, path)) {
> - pa_log("org.bluez.MediaEndpoint.SetConfiguration: Transport %s is already configured.", path);
> + pa_log("Endpoint SetConfiguration: Transport %s is already configured.", path);
> goto fail;
> }
>
> @@ -1614,9 +1638,8 @@ static DBusMessage *endpoint_set_configuration(DBusConnection *conn, DBusMessage
> return r;
>
> fail:
> - pa_log("org.bluez.MediaEndpoint.SetConfiguration: invalid arguments");
> - pa_assert_se(r = (dbus_message_new_error(m, "org.bluez.MediaEndpoint.Error.InvalidArguments",
> - "Unable to set configuration")));
> + pa_log("Endpoint SetConfiguration: invalid arguments");
> + pa_assert_se(r = (dbus_message_new_error(m, "org.bluez.Error.InvalidArguments", "Unable to set configuration")));
While you're modifying this line, you could also remove the redundant
parentheses around the dbus_message_new_error() call.
> return r;
> }
>
> @@ -1630,7 +1653,7 @@ static DBusMessage *endpoint_clear_configuration(DBusConnection *c, DBusMessage
> dbus_error_init(&e);
>
> if (!dbus_message_get_args(m, &e, DBUS_TYPE_OBJECT_PATH, &path, DBUS_TYPE_INVALID)) {
> - pa_log("org.bluez.MediaEndpoint.ClearConfiguration: %s", e.message);
> + pa_log("Endpoint ClearConfiguration: %s", e.message);
> dbus_error_free(&e);
> goto fail;
> }
> @@ -1655,8 +1678,7 @@ static DBusMessage *endpoint_clear_configuration(DBusConnection *c, DBusMessage
> return r;
>
> fail:
> - pa_assert_se(r = (dbus_message_new_error(m, "org.bluez.MediaEndpoint.Error.InvalidArguments",
> - "Unable to clear configuration")));
> + pa_assert_se(r = (dbus_message_new_error(m, "org.bluez.Error.InvalidArguments", "Unable to clear configuration")));
Extra parentheses here too.
> return r;
> }
>
> @@ -1726,7 +1748,7 @@ static DBusMessage *endpoint_select_configuration(DBusConnection *c, DBusMessage
> dbus_error_init(&e);
>
> if (!dbus_message_get_args(m, &e, DBUS_TYPE_ARRAY, DBUS_TYPE_BYTE, &cap, &size, DBUS_TYPE_INVALID)) {
> - pa_log("org.bluez.MediaEndpoint.SelectConfiguration: %s", e.message);
> + pa_log("Endpoint SelectConfiguration: %s", e.message);
> dbus_error_free(&e);
> goto fail;
> }
> @@ -1823,8 +1845,7 @@ done:
> return r;
>
> fail:
> - pa_assert_se(r = (dbus_message_new_error(m, "org.bluez.MediaEndpoint.Error.InvalidArguments",
> - "Unable to select configuration")));
> + pa_assert_se(r = (dbus_message_new_error(m, "org.bluez.Error.InvalidArguments", "Unable to select configuration")));
Extra parentheses.
--
Tanu
