[PATCH BlueZ 0/4] Sixaxis Plugin, almost there?

[PATCH BlueZ 0/4] Sixaxis Plugin, almost there?

[Antonio Ospite]

Hi,

I hope we are getting there, this is another update to the sixaxis plugin.

The first patch is the same as always, removal of old cruft which I think 
could go in even right now.


The second patch is the sixaxis plugin with some updates:
  
  - Fixed various style issues (long lines, indentation)
  - Fixed some style issue pointed out by checkpatch.pl from linux, 
    namely it suggested not to initialize static variables.
  - Fixed issue pointed out by Vinicius Costa Gomez:
      * remove double newlines
      * move defines and static variables on the top
      * Add a comment about why the timeout is needed in monitor_event()
  - Use manager_get_default_adapter_id() as suggested by Bastien
  - Make return values more meaningful
  - Set the master bdaddr in the controller only when it is different 
    from the BT adapter bdaddr, this is how the PS3 does it (thanks Jim 
    for the dumps), perhaps to avoid unnecessary writes to some eeprom.

I left in the calloc() calls for now, maybe later I'll split the plugin 
in a sixaxis-specific part and a bluez-specific part, but for now I 
think we can live with that. I also left the return codes called "ret" 
because they didn't fit the "err" semantic, as I am checking for < 0.


The third patch is about linking UDEV_LIBS only when needed, embedded 
people might like this but I was not sure and I left is a separate patch 
for an easier review.


The forth patch makes the plugin wait for actual events (that is: PS button 
has been pressed) before setting the leds, this is how the PS3 behaves, and 
happens to cure the problem of setting the second led when connecting the 
controller via USB when it is working over BT already, in fact the controller 
keep sending events over BT even after it is connected via USB after 
association, and so the USB connection event will not set the second led on 
ever. The trick works with one controller, but I think this will mess up 
numbering with multiple controller connected in some mixed order. So again 
this change is sent in its own patch to be more visible.

For those of you who can't get the plugin to work, please try cleaning 
up your /var/lib/bluetooth dir.


NOTES:

  - Testing is needed with multiple BT adapters and/or multiple 
    controllers, but I haven't got the hardware for that yet.

  - Arc Riley suggested to use more generic name for the plugin if it is 
    going to support the Move too, something like "sony-controllers" 
    maybe, but I don't mind leaving it called sixaxis for historical 
    reasons (and laziness reasons too), what do you think?


Thanks,
   Antonio

Antonio Ospite (4):
  Remove input/sixpair.c
  Add sixaxis plugin: USB pairing and LEDs settings
  Link to udev only when needed
  plugins/sixaxis: Wait for the PS button before setting the LEDs

 Makefile.am       |   11 +-
 acinclude.m4      |   10 +
 input/sixpair.c   |  299 --------------------------
 plugins/sixaxis.c |  599 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 619 insertions(+), 300 deletions(-)
 delete mode 100644 input/sixpair.c
 create mode 100644 plugins/sixaxis.c

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[PATCH BlueZ 0/4] Sixaxis Plugin, almost there?

[Antonio Ospite]

Hi,

I hope we are getting there, this is another update to the sixaxis plugin.

The first patch is the same as always, removal of old cruft which I think 
could go in even right now.


The second patch is the sixaxis plugin with some updates:
  
  - Fixed various style issues (long lines, indentation)
  - Fixed some style issue pointed out by checkpatch.pl from linux, 
    namely it suggested not to initialize static variables.
  - Fixed issue pointed out by Vinicius Costa Gomez:
      * remove double newlines
      * move defines and static variables on the top
      * Add a comment about why the timeout is needed in monitor_event()
  - Use manager_get_default_adapter_id() as suggested by Bastien
  - Make return values more meaningful
  - Set the master bdaddr in the controller only when it is different 
    from the BT adapter bdaddr, this is how the PS3 does it (thanks Jim 
    for the dumps), perhaps to avoid unnecessary writes to some eeprom.

I left in the calloc() calls for now, maybe later I'll split the plugin 
in a sixaxis-specific part and a bluez-specific part, but for now I 
think we can live with that. I also left the return codes called "ret" 
because they didn't fit the "err" semantic, as I am checking for < 0.


The third patch is about linking UDEV_LIBS only when needed, embedded 
people might like this but I was not sure and I left is a separate patch 
for an easier review.


The forth patch makes the plugin wait for actual events (that is: PS button 
has been pressed) before setting the leds, this is how the PS3 behaves, and 
happens to cure the problem of setting the second led when connecting the 
controller via USB when it is working over BT already, in fact the controller 
keep sending events over BT even after it is connected via USB after 
association, and so the USB connection event will not set the second led on 
ever. The trick works with one controller, but I think this will mess up 
numbering with multiple controller connected in some mixed order. So again 
this change is sent in its own patch to be more visible.

For those of you who can't get the plugin to work, please try cleaning 
up your /var/lib/bluetooth dir.


NOTES:

  - Testing is needed with multiple BT adapters and/or multiple 
    controllers, but I haven't got the hardware for that yet.

  - Arc Riley suggested to use more generic name for the plugin if it is 
    going to support the Move too, something like "sony-controllers" 
    maybe, but I don't mind leaving it called sixaxis for historical 
    reasons (and laziness reasons too), what do you think?


Thanks,
   Antonio

Antonio Ospite (4):
  Remove input/sixpair.c
  Add sixaxis plugin: USB pairing and LEDs settings
  Link to udev only when needed
  plugins/sixaxis: Wait for the PS button before setting the LEDs

 Makefile.am       |   11 +-
 acinclude.m4      |   10 +
 input/sixpair.c   |  299 --------------------------
 plugins/sixaxis.c |  599 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 619 insertions(+), 300 deletions(-)
 delete mode 100644 input/sixpair.c
 create mode 100644 plugins/sixaxis.c

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[PATCH BlueZ 1/4] Remove input/sixpair.c

[Antonio Ospite]

This is the old way to associate to a Sixaxis controller and it is not
even compiled in, so we can safely get rid of it.
---
 input/sixpair.c |  299 -------------------------------------------------------
 1 files changed, 0 insertions(+), 299 deletions(-)
 delete mode 100644 input/sixpair.c

diff --git a/input/sixpair.c b/input/sixpair.c
deleted file mode 100644
index 5c58b9b..0000000
--- a/input/sixpair.c
+++ /dev/null
@@ -1,299 +0,0 @@
-/* To compile
- * gcc -g -Wall -I../src -I../lib/ -I../include -DSTORAGEDIR=\"/var/lib/bluetooth\" -o sixpair sixpair.c ../src/storage.c ../common/libhelper.a -I../common `pkg-config --libs --cflags glib-2.0 libusb-1.0` -lbluetooth
- */
-
-#include <unistd.h>
-#include <stdio.h>
-#include <inttypes.h>
-
-#include <sdp.h>
-#include <bluetooth/bluetooth.h>
-#include <bluetooth/sdp_lib.h>
-#include <glib.h>
-#include <libusb.h>
-
-#include "storage.h"
-
-/* Vendor and product ID for the Sixaxis PS3 controller */
-#define VENDOR 0x054c
-#define PRODUCT 0x0268
-
-#define PS3_PNP_RECORD "3601920900000A000100000900013503191124090004350D35061901000900113503190011090006350909656E09006A0901000900093508350619112409010009000D350F350D350619010009001335031900110901002513576972656C65737320436F6E74726F6C6C65720901012513576972656C65737320436F6E74726F6C6C6572090102251B536F6E7920436F6D707574657220456E7465727461696E6D656E740902000901000902010901000902020800090203082109020428010902052801090206359A35980822259405010904A101A102850175089501150026FF00810375019513150025013500450105091901291381027501950D0600FF8103150026FF0005010901A10075089504350046FF0009300931093209358102C0050175089527090181027508953009019102750895300901B102C0A1028502750895300901B102C0A10285EE750895300901B102C0A10285EF750895300901B102C0C0090207350835060904090901000902082800090209280109020A280109020B09010009020C093E8009020D280009020E2800"
-
-gboolean option_get_master = TRUE;
-char *option_master= NULL;
-gboolean option_store_info = TRUE;
-const char *option_device = NULL;
-gboolean option_quiet = FALSE;
-
-const GOptionEntry options[] = {
-	{ "get-master", '\0', 0, G_OPTION_ARG_NONE, &option_get_master, "Get currently set master address", NULL },
-	{ "set-master", '\0', 0, G_OPTION_ARG_STRING, &option_master, "Set master address (\"auto\" for automatic)", NULL },
-	{ "store-info", '\0', 0, G_OPTION_ARG_NONE, &option_store_info, "Store the HID info into the input database", NULL },
-	{ "device", '\0', 0, G_OPTION_ARG_STRING, &option_device, "Only handle one device (default, all supported", NULL },
-	{ "quiet", 'q', 0, G_OPTION_ARG_NONE, &option_quiet, "Quieten the output", NULL },
-	{ NULL }
-};
-
-static gboolean
-show_master (libusb_device_handle *devh, int itfnum)
-{
-	unsigned char msg[8];
-	int res;
-
-	res = libusb_control_transfer (devh,
-				       LIBUSB_ENDPOINT_IN | LIBUSB_REQUEST_TYPE_CLASS | LIBUSB_RECIPIENT_INTERFACE,
-				       0x01, 0x03f5, itfnum,
-				       (void*) msg, sizeof(msg),
-				       5000);
-
-	if (res < 0) {
-		g_warning ("Getting the master Bluetooth address failed");
-		return FALSE;
-	}
-	g_print ("Current Bluetooth master: %02X:%02X:%02X:%02X:%02X:%02X\n",
-		 msg[2], msg[3], msg[4], msg[5], msg[6], msg[7]);
-
-	return TRUE;
-}
-
-static char *
-get_bdaddr (libusb_device_handle *devh, int itfnum)
-{
-	unsigned char msg[17];
-	char *address;
-	int res;
-
-	res = libusb_control_transfer (devh,
-				       LIBUSB_ENDPOINT_IN | LIBUSB_REQUEST_TYPE_CLASS | LIBUSB_RECIPIENT_INTERFACE,
-				       0x01, 0x03f2, itfnum,
-				       (void*) msg, sizeof(msg),
-				       5000);
-
-	if (res < 0) {
-		g_warning ("Getting the device Bluetooth address failed");
-		return NULL;
-	}
-
-	address = g_strdup_printf ("%02X:%02X:%02X:%02X:%02X:%02X",
-				   msg[4], msg[5], msg[6], msg[7], msg[8], msg[9]);
-
-	if (option_quiet == FALSE) {
-		g_print ("Device Bluetooth address: %s\n", address);
-	}
-
-	return address;
-}
-
-static gboolean
-set_master_bdaddr (libusb_device_handle *devh, int itfnum, char *host)
-{
-	unsigned char msg[8];
-	int mac[6];
-	int res;
-
-	if (sscanf(host, "%X:%X:%X:%X:%X:%X",
-		   &mac[0],&mac[1],&mac[2],&mac[3],&mac[4],&mac[5]) != 6) {
-		return FALSE;
-	}
-
-	msg[0] = 0x01;
-	msg[1] = 0x00;
-	msg[2] = mac[0];
-	msg[3] = mac[1];
-	msg[4] = mac[2];
-	msg[5] = mac[3];
-	msg[6] = mac[4];
-	msg[7] = mac[5];
-
-	res = libusb_control_transfer (devh,
-				       LIBUSB_ENDPOINT_OUT | LIBUSB_REQUEST_TYPE_CLASS | LIBUSB_RECIPIENT_INTERFACE,
-				       0x09, 0x03f5, itfnum,
-				       (void*) msg, sizeof(msg),
-				       5000);
-
-	if (res < 0) {
-		g_warning ("Setting the master Bluetooth address failed");
-		return FALSE;
-	}
-
-	return TRUE;
-}
-
-static char *
-get_host_bdaddr (void)
-{
-	FILE *f;
-	int mac[6];
-
-	//FIXME use dbus to get the default adapter
-
-	f = popen("hcitool dev", "r");
-
-	if (f == NULL) {
-		//FIXME
-		return NULL;
-	}
-	if (fscanf(f, "%*s\n%*s %X:%X:%X:%X:%X:%X",
-		   &mac[0],&mac[1],&mac[2],&mac[3],&mac[4],&mac[5]) != 6) {
-		//FIXME
-		return NULL;
-	}
-
-	return g_strdup_printf ("%02X:%02X:%02X:%02X:%02X:%02X", mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]);
-}
-
-static int
-handle_device (libusb_device *dev, struct libusb_config_descriptor *cfg, int itfnum, const struct libusb_interface_descriptor *alt)
-{
-	libusb_device_handle *devh;
-	int res, retval;
-
-	retval = -1;
-
-	if (libusb_open (dev, &devh) < 0) {
-		g_warning ("Can't open device");
-		goto bail;
-	}
-	libusb_detach_kernel_driver (devh, itfnum);
-
-	res = libusb_claim_interface (devh, itfnum);
-	if (res < 0) {
-		g_warning ("Can't claim interface %d", itfnum);
-		goto bail;
-	}
-
-	if (option_get_master != FALSE) {
-		if (show_master (devh, itfnum) == FALSE)
-			goto bail;
-		retval = 0;
-	}
-
-	if (option_master != NULL) {
-		if (strcmp (option_master, "auto") == 0) {
-			g_free (option_master);
-			option_master = get_host_bdaddr ();
-			if (option_master == NULL) {
-				g_warning ("Can't get bdaddr from default device");
-				retval = -1;
-				goto bail;
-			}
-		}
-	} else {
-		option_master = get_host_bdaddr ();
-		if (option_master == NULL) {
-			g_warning ("Can't get bdaddr from default device");
-			retval = -1;
-			goto bail;
-		}
-	}
-
-	if (option_store_info != FALSE) {
-		sdp_record_t *rec;
-		char *device;
-		bdaddr_t dst, src;
-
-		device = get_bdaddr (devh, itfnum);
-		if (device == NULL) {
-			retval = -1;
-			goto bail;
-		}
-
-		rec = record_from_string (PS3_PNP_RECORD);
-		store_record(option_master, device, rec);
-		write_trust(option_master, device, "[all]", TRUE);
-		store_device_id(option_master, device, 0xffff, 0x054c, 0x0268, 0);
-		str2ba(option_master, &src);
-		str2ba(device, &dst);
-		write_device_profiles(&src, &dst, "");
-		write_device_name(&src, &dst, "PLAYSTATION(R)3 Controller");
-		sdp_record_free(rec);
-
-		if (set_master_bdaddr (devh, itfnum, option_master) == FALSE) {
-			retval = -1;
-			goto bail;
-		}
-	}
-
-bail:
-	libusb_release_interface (devh, itfnum);
-	res = libusb_attach_kernel_driver(devh, itfnum);
-	if (res < 0) {
-		//FIXME sometimes the kernel tells us ENOENT, but succeeds anyway...
-		g_warning ("Reattaching the driver failed: %d", res);
-	}
-	if (devh != NULL)
-		libusb_close (devh);
-
-	return retval;
-}
-
-int main (int argc, char **argv)
-{
-	GOptionContext *context;
-	GError *error = NULL;
-	libusb_device **list;
-	ssize_t num_devices, i;
-
-	context = g_option_context_new ("- Manage Sixaxis PS3 controllers");
-	g_option_context_add_main_entries (context, options, NULL);
-	if (g_option_context_parse (context, &argc, &argv, &error) == FALSE) {
-		g_warning ("Couldn't parse command-line options: %s", error->message);
-		return 1;
-	}
-
-	/* Check that the passed bdaddr is correct */
-	if (option_master != NULL && strcmp (option_master, "auto") != 0) {
-		//FIXME check bdaddr
-	}
-
-	libusb_init (NULL);
-
-	/* Find device(s) */
-	num_devices = libusb_get_device_list (NULL, &list);
-	if (num_devices < 0) {
-		g_warning ("libusb_get_device_list failed");
-		return 1;
-	}
-
-	for (i = 0; i < num_devices; i++) {
-		struct libusb_config_descriptor *cfg;
-		libusb_device *dev = list[i];
-		struct libusb_device_descriptor desc;
-		guint8 j;
-
-		if (libusb_get_device_descriptor (dev, &desc) < 0) {
-			g_warning ("libusb_get_device_descriptor failed");
-			continue;
-		}
-
-		/* Here we check for the supported devices */
-		if (desc.idVendor != VENDOR || desc.idProduct != PRODUCT)
-			continue;
-
-		/* Look for the interface number that interests us */
-		for (j = 0; j < desc.bNumConfigurations; j++) {
-			struct libusb_config_descriptor *config;
-			guint8 k;
-
-			libusb_get_config_descriptor (dev, j, &config);
-
-			for (k = 0; k < config->bNumInterfaces; k++) {
-				const struct libusb_interface *itf = &config->interface[k];
-				int l;
-
-				for (l = 0; l < itf->num_altsetting ; l++) {
-					struct libusb_interface_descriptor alt;
-
-					alt = itf->altsetting[l];
-					if (alt.bInterfaceClass == 3) {
-						handle_device (dev, cfg, l, &alt);
-					}
-				}
-			}
-		}
-	}
-
-	return 0;
-}
-
-- 
1.7.5.4

[PATCH BlueZ 1/4] Remove input/sixpair.c

[Antonio Ospite]

This is the old way to associate to a Sixaxis controller and it is not
even compiled in, so we can safely get rid of it.
---
 input/sixpair.c |  299 -------------------------------------------------------
 1 files changed, 0 insertions(+), 299 deletions(-)
 delete mode 100644 input/sixpair.c

diff --git a/input/sixpair.c b/input/sixpair.c
deleted file mode 100644
index 5c58b9b..0000000
--- a/input/sixpair.c
+++ /dev/null
@@ -1,299 +0,0 @@
-/* To compile
- * gcc -g -Wall -I../src -I../lib/ -I../include -DSTORAGEDIR=\"/var/lib/bluetooth\" -o sixpair sixpair.c ../src/storage.c ../common/libhelper.a -I../common `pkg-config --libs --cflags glib-2.0 libusb-1.0` -lbluetooth
- */
-
-#include <unistd.h>
-#include <stdio.h>
-#include <inttypes.h>
-
-#include <sdp.h>
-#include <bluetooth/bluetooth.h>
-#include <bluetooth/sdp_lib.h>
-#include <glib.h>
-#include <libusb.h>
-
-#include "storage.h"
-
-/* Vendor and product ID for the Sixaxis PS3 controller */
-#define VENDOR 0x054c
-#define PRODUCT 0x0268
-
-#define PS3_PNP_RECORD "3601920900000A000100000900013503191124090004350D35061901000900113503190011090006350909656E09006A0901000900093508350619112409010009000D350F350D350619010009001335031900110901002513576972656C65737320436F6E74726F6C6C65720901012513576972656C65737320436F6E74726F6C6C6572090102251B536F6E7920436F6D707574657220456E7465727461696E6D656E740902000901000902010901000902020800090203082109020428010902052801090206359A35980822259405010904A101A102850175089501150026FF00810375019513150025013500450105091901291381027501950D0600FF8103150026FF0005010901A10075089504350046FF0009300931093209358102C0050175089527090181027508953009019102750895300901B102C0A1028502750895300901B102C0A10285EE750895300901B102C0A10285EF750895300901B102C0C0090207350835060904090901000902082800090209280109020A280109020B090
 10009020C093E8009020D280009020E2800"
-
-gboolean option_get_master = TRUE;
-char *option_master= NULL;
-gboolean option_store_info = TRUE;
-const char *option_device = NULL;
-gboolean option_quiet = FALSE;
-
-const GOptionEntry options[] = {
-	{ "get-master", '\0', 0, G_OPTION_ARG_NONE, &option_get_master, "Get currently set master address", NULL },
-	{ "set-master", '\0', 0, G_OPTION_ARG_STRING, &option_master, "Set master address (\"auto\" for automatic)", NULL },
-	{ "store-info", '\0', 0, G_OPTION_ARG_NONE, &option_store_info, "Store the HID info into the input database", NULL },
-	{ "device", '\0', 0, G_OPTION_ARG_STRING, &option_device, "Only handle one device (default, all supported", NULL },
-	{ "quiet", 'q', 0, G_OPTION_ARG_NONE, &option_quiet, "Quieten the output", NULL },
-	{ NULL }
-};
-
-static gboolean
-show_master (libusb_device_handle *devh, int itfnum)
-{
-	unsigned char msg[8];
-	int res;
-
-	res = libusb_control_transfer (devh,
-				       LIBUSB_ENDPOINT_IN | LIBUSB_REQUEST_TYPE_CLASS | LIBUSB_RECIPIENT_INTERFACE,
-				       0x01, 0x03f5, itfnum,
-				       (void*) msg, sizeof(msg),
-				       5000);
-
-	if (res < 0) {
-		g_warning ("Getting the master Bluetooth address failed");
-		return FALSE;
-	}
-	g_print ("Current Bluetooth master: %02X:%02X:%02X:%02X:%02X:%02X\n",
-		 msg[2], msg[3], msg[4], msg[5], msg[6], msg[7]);
-
-	return TRUE;
-}
-
-static char *
-get_bdaddr (libusb_device_handle *devh, int itfnum)
-{
-	unsigned char msg[17];
-	char *address;
-	int res;
-
-	res = libusb_control_transfer (devh,
-				       LIBUSB_ENDPOINT_IN | LIBUSB_REQUEST_TYPE_CLASS | LIBUSB_RECIPIENT_INTERFACE,
-				       0x01, 0x03f2, itfnum,
-				       (void*) msg, sizeof(msg),
-				       5000);
-
-	if (res < 0) {
-		g_warning ("Getting the device Bluetooth address failed");
-		return NULL;
-	}
-
-	address = g_strdup_printf ("%02X:%02X:%02X:%02X:%02X:%02X",
-				   msg[4], msg[5], msg[6], msg[7], msg[8], msg[9]);
-
-	if (option_quiet == FALSE) {
-		g_print ("Device Bluetooth address: %s\n", address);
-	}
-
-	return address;
-}
-
-static gboolean
-set_master_bdaddr (libusb_device_handle *devh, int itfnum, char *host)
-{
-	unsigned char msg[8];
-	int mac[6];
-	int res;
-
-	if (sscanf(host, "%X:%X:%X:%X:%X:%X",
-		   &mac[0],&mac[1],&mac[2],&mac[3],&mac[4],&mac[5]) != 6) {
-		return FALSE;
-	}
-
-	msg[0] = 0x01;
-	msg[1] = 0x00;
-	msg[2] = mac[0];
-	msg[3] = mac[1];
-	msg[4] = mac[2];
-	msg[5] = mac[3];
-	msg[6] = mac[4];
-	msg[7] = mac[5];
-
-	res = libusb_control_transfer (devh,
-				       LIBUSB_ENDPOINT_OUT | LIBUSB_REQUEST_TYPE_CLASS | LIBUSB_RECIPIENT_INTERFACE,
-				       0x09, 0x03f5, itfnum,
-				       (void*) msg, sizeof(msg),
-				       5000);
-
-	if (res < 0) {
-		g_warning ("Setting the master Bluetooth address failed");
-		return FALSE;
-	}
-
-	return TRUE;
-}
-
-static char *
-get_host_bdaddr (void)
-{
-	FILE *f;
-	int mac[6];
-
-	//FIXME use dbus to get the default adapter
-
-	f = popen("hcitool dev", "r");
-
-	if (f == NULL) {
-		//FIXME
-		return NULL;
-	}
-	if (fscanf(f, "%*s\n%*s %X:%X:%X:%X:%X:%X",
-		   &mac[0],&mac[1],&mac[2],&mac[3],&mac[4],&mac[5]) != 6) {
-		//FIXME
-		return NULL;
-	}
-
-	return g_strdup_printf ("%02X:%02X:%02X:%02X:%02X:%02X", mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]);
-}
-
-static int
-handle_device (libusb_device *dev, struct libusb_config_descriptor *cfg, int itfnum, const struct libusb_interface_descriptor *alt)
-{
-	libusb_device_handle *devh;
-	int res, retval;
-
-	retval = -1;
-
-	if (libusb_open (dev, &devh) < 0) {
-		g_warning ("Can't open device");
-		goto bail;
-	}
-	libusb_detach_kernel_driver (devh, itfnum);
-
-	res = libusb_claim_interface (devh, itfnum);
-	if (res < 0) {
-		g_warning ("Can't claim interface %d", itfnum);
-		goto bail;
-	}
-
-	if (option_get_master != FALSE) {
-		if (show_master (devh, itfnum) == FALSE)
-			goto bail;
-		retval = 0;
-	}
-
-	if (option_master != NULL) {
-		if (strcmp (option_master, "auto") == 0) {
-			g_free (option_master);
-			option_master = get_host_bdaddr ();
-			if (option_master == NULL) {
-				g_warning ("Can't get bdaddr from default device");
-				retval = -1;
-				goto bail;
-			}
-		}
-	} else {
-		option_master = get_host_bdaddr ();
-		if (option_master == NULL) {
-			g_warning ("Can't get bdaddr from default device");
-			retval = -1;
-			goto bail;
-		}
-	}
-
-	if (option_store_info != FALSE) {
-		sdp_record_t *rec;
-		char *device;
-		bdaddr_t dst, src;
-
-		device = get_bdaddr (devh, itfnum);
-		if (device == NULL) {
-			retval = -1;
-			goto bail;
-		}
-
-		rec = record_from_string (PS3_PNP_RECORD);
-		store_record(option_master, device, rec);
-		write_trust(option_master, device, "[all]", TRUE);
-		store_device_id(option_master, device, 0xffff, 0x054c, 0x0268, 0);
-		str2ba(option_master, &src);
-		str2ba(device, &dst);
-		write_device_profiles(&src, &dst, "");
-		write_device_name(&src, &dst, "PLAYSTATION(R)3 Controller");
-		sdp_record_free(rec);
-
-		if (set_master_bdaddr (devh, itfnum, option_master) == FALSE) {
-			retval = -1;
-			goto bail;
-		}
-	}
-
-bail:
-	libusb_release_interface (devh, itfnum);
-	res = libusb_attach_kernel_driver(devh, itfnum);
-	if (res < 0) {
-		//FIXME sometimes the kernel tells us ENOENT, but succeeds anyway...
-		g_warning ("Reattaching the driver failed: %d", res);
-	}
-	if (devh != NULL)
-		libusb_close (devh);
-
-	return retval;
-}
-
-int main (int argc, char **argv)
-{
-	GOptionContext *context;
-	GError *error = NULL;
-	libusb_device **list;
-	ssize_t num_devices, i;
-
-	context = g_option_context_new ("- Manage Sixaxis PS3 controllers");
-	g_option_context_add_main_entries (context, options, NULL);
-	if (g_option_context_parse (context, &argc, &argv, &error) == FALSE) {
-		g_warning ("Couldn't parse command-line options: %s", error->message);
-		return 1;
-	}
-
-	/* Check that the passed bdaddr is correct */
-	if (option_master != NULL && strcmp (option_master, "auto") != 0) {
-		//FIXME check bdaddr
-	}
-
-	libusb_init (NULL);
-
-	/* Find device(s) */
-	num_devices = libusb_get_device_list (NULL, &list);
-	if (num_devices < 0) {
-		g_warning ("libusb_get_device_list failed");
-		return 1;
-	}
-
-	for (i = 0; i < num_devices; i++) {
-		struct libusb_config_descriptor *cfg;
-		libusb_device *dev = list[i];
-		struct libusb_device_descriptor desc;
-		guint8 j;
-
-		if (libusb_get_device_descriptor (dev, &desc) < 0) {
-			g_warning ("libusb_get_device_descriptor failed");
-			continue;
-		}
-
-		/* Here we check for the supported devices */
-		if (desc.idVendor != VENDOR || desc.idProduct != PRODUCT)
-			continue;
-
-		/* Look for the interface number that interests us */
-		for (j = 0; j < desc.bNumConfigurations; j++) {
-			struct libusb_config_descriptor *config;
-			guint8 k;
-
-			libusb_get_config_descriptor (dev, j, &config);
-
-			for (k = 0; k < config->bNumInterfaces; k++) {
-				const struct libusb_interface *itf = &config->interface[k];
-				int l;
-
-				for (l = 0; l < itf->num_altsetting ; l++) {
-					struct libusb_interface_descriptor alt;
-
-					alt = itf->altsetting[l];
-					if (alt.bInterfaceClass == 3) {
-						handle_device (dev, cfg, l, &alt);
-					}
-				}
-			}
-		}
-	}
-
-	return 0;
-}
-
-- 
1.7.5.4

[PATCH BlueZ 2/4] Add sixaxis plugin: USB pairing and LEDs settings

[Antonio Ospite]

Add a plugin which handles the connection of a Sixaxis device, when a
new hidraw device is connected the plugin:
 - Filters udev events, and select the Sixaxis device
 - Sets LEDs to match the joystick system number (for USB and BT)
 - Sets the Master bluetooth address in the Sixaxis (USB pairing)
 - Adds the device to the database of the current default
   adapter (BT association)

Signed-off-by: Bastien Nocera <hadess@hadess.net>
Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 Makefile.am       |    9 +-
 acinclude.m4      |   10 +
 plugins/sixaxis.c |  593 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 610 insertions(+), 2 deletions(-)
 create mode 100644 plugins/sixaxis.c

diff --git a/Makefile.am b/Makefile.am
index 68380d9..dbe0170 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -224,6 +224,11 @@ builtin_sources += thermometer/main.c \
 			thermometer/thermometer.h thermometer/thermometer.c
 endif
 
+if SIXAXISPLUGIN
+builtin_modules += sixaxis
+builtin_sources += plugins/sixaxis.c
+endif
+
 builtin_modules += hciops mgmtops
 builtin_sources += plugins/hciops.c plugins/mgmtops.c
 
@@ -283,7 +288,7 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
 			src/event.h src/event.c \
 			src/oob.h src/oob.c src/eir.h src/eir.c
 src_bluetoothd_LDADD = lib/libbluetooth.la @GLIB_LIBS@ @DBUS_LIBS@ \
-							@CAPNG_LIBS@ -ldl -lrt
+							@CAPNG_LIBS@ @UDEV_LIBS@ -ldl -lrt
 src_bluetoothd_LDFLAGS = -Wl,--export-dynamic \
 				-Wl,--version-script=$(srcdir)/src/bluetooth.ver
 
@@ -398,7 +403,7 @@ EXTRA_DIST += doc/manager-api.txt \
 
 AM_YFLAGS = -d
 
-AM_CFLAGS = @DBUS_CFLAGS@ @GLIB_CFLAGS@ @CAPNG_CFLAGS@ \
+AM_CFLAGS = @DBUS_CFLAGS@ @GLIB_CFLAGS@ @CAPNG_CFLAGS@ @UDEV_CFLAGS@ \
 		-DBLUETOOTH_PLUGIN_BUILTIN -DPLUGINDIR=\""$(plugindir)"\"
 
 INCLUDES = -I$(builddir)/lib -I$(builddir)/src -I$(srcdir)/src \
diff --git a/acinclude.m4 b/acinclude.m4
index 3cb9459..6176483 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -186,6 +186,7 @@ AC_DEFUN([AC_ARG_BLUEZ], [
 	sndfile_enable=${sndfile_found}
 	hal_enable=no
 	usb_enable=${usb_found}
+	sixaxis_enable=${udev_found}
 	alsa_enable=${alsa_found}
 	gstreamer_enable=${gstreamer_found}
 	audio_enable=yes
@@ -286,6 +287,10 @@ AC_DEFUN([AC_ARG_BLUEZ], [
 		usb_enable=${enableval}
 	])
 
+	AC_ARG_ENABLE(sixaxis, AC_HELP_STRING([--enable-sixaxis], [enable Sixaxis plugin]), [
+		sixaxis_enable=${enableval}
+	])
+
 	AC_ARG_ENABLE(tracer, AC_HELP_STRING([--enable-tracer], [install Tracing daemon]), [
 		tracer_enable=${enableval}
 	])
@@ -385,6 +390,10 @@ AC_DEFUN([AC_ARG_BLUEZ], [
 		AC_DEFINE(HAVE_LIBUSB, 1, [Define to 1 if you have USB library.])
 	fi
 
+	if (test "${sixaxis_enable}" = "yes" && test "${udev_found}" = "yes"); then
+		AC_DEFINE(HAVE_SIXAXIS_PLUGIN, 1, [Define to 1 if you have sixaxis plugin.])
+	fi
+
 	AM_CONDITIONAL(SNDFILE, test "${sndfile_enable}" = "yes" && test "${sndfile_found}" = "yes")
 	AM_CONDITIONAL(USB, test "${usb_enable}" = "yes" && test "${usb_found}" = "yes")
 	AM_CONDITIONAL(SBC, test "${alsa_enable}" = "yes" || test "${gstreamer_enable}" = "yes" ||
@@ -421,4 +430,5 @@ AC_DEFUN([AC_ARG_BLUEZ], [
 	AM_CONDITIONAL(DBUSOOBPLUGIN, test "${dbusoob_enable}" = "yes")
 	AM_CONDITIONAL(WIIMOTEPLUGIN, test "${wiimote_enable}" = "yes")
 	AM_CONDITIONAL(THERMOMETERPLUGIN, test "${thermometer_enable}" = "yes")
+	AM_CONDITIONAL(SIXAXISPLUGIN, test "${sixaxis_enable}" = "yes" && test "${udev_found}" = "yes")
 ])
diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
new file mode 100644
index 0000000..2b0616a
--- /dev/null
+++ b/plugins/sixaxis.c
@@ -0,0 +1,593 @@
+/*
+ * sixaxis plugin: do cable association for Sixaxis controller
+ *
+ * Copyright (C) 2009  Bastien Nocera <hadess@hadess.net>
+ * Copyright (C) 2011  Antonio Ospite <ospite@studenti.unina.it>
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+/*
+ * In the following this terminology is used:
+ *
+ *  - controller: a Sixaxis joypad.
+ *  - adapter: the bluetooth dongle on the host system.
+ *  - adapter_bdaddr: the bdaddr of the bluetooth adapter.
+ *  - device_bdaddr: the bdaddr of the Sixaxis controller.
+ *  - master_bdaddr: the bdaddr of the adapter to be configured into the
+ *    Sixaxis controller
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <errno.h>
+#include <glib.h>
+#include <linux/hidraw.h>
+
+#define LIBUDEV_I_KNOW_THE_API_IS_SUBJECT_TO_CHANGE 1
+#include <libudev.h>
+
+#include "plugin.h"
+#include "log.h"
+#include "adapter.h"
+#include "device.h"
+#include "manager.h"
+#include "storage.h"
+#include "sdp_lib.h"
+
+/* Fallback definitions to compile with older headers */
+#ifndef HIDIOCGFEATURE
+#define HIDIOCGFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x07, len)
+#endif
+
+#ifndef HIDIOCSFEATURE
+#define HIDIOCSFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x06, len)
+#endif
+
+#define BDADDR_STR_SIZE 18 /* strlen("00:00:00:00:00:00") + 1 */
+
+/* Vendor and product ID for the Sixaxis PS3 controller */
+#define VENDOR 0x054c
+#define PRODUCT 0x0268
+#define SIXAXIS_NAME "PLAYSTATION(R)3 Controller"
+#define SIXAXIS_PNP_RECORD "3601920900000A000100000900013503191124090004350D35061901000900113503190011090006350909656E09006A0901000900093508350619112409010009000D350F350D350619010009001335031900110901002513576972656C65737320436F6E74726F6C6C65720901012513576972656C65737320436F6E74726F6C6C6572090102251B536F6E7920436F6D707574657220456E7465727461696E6D656E740902000901000902010901000902020800090203082109020428010902052801090206359A35980822259405010904A101A102850175089501150026FF00810375019513150025013500450105091901291381027501950D0600FF8103150026FF0005010901A10075089504350046FF0009300931093209358102C0050175089527090181027508953009019102750895300901B102C0A1028502750895300901B102C0A10285EE750895300901B102C0A10285EF750895300901B102C0C0090207350835060904090901000902082800090209280109020A280109020B09010009020C093E8009020D280009020E2800"
+#define HID_UUID "00001124-0000-1000-8000-00805f9b34fb"
+
+#define LED_1 (0x01 << 1)
+#define LED_2 (0x01 << 2)
+#define LED_3 (0x01 << 3)
+#define LED_4 (0x01 << 4)
+
+#define LED_STATUS_OFF 0
+#define LED_STATUS_ON  1
+
+static struct udev *ctx;
+static struct udev_monitor *monitor;
+static guint watch_id;
+
+
+static int create_sixaxis_association(struct btd_adapter *adapter,
+					const char *name,
+					const char *address,
+					guint32 vendor_id,
+					guint32 product_id,
+					const char *pnp_record)
+{
+	DBusConnection *conn;
+	sdp_record_t *rec;
+	struct btd_device *device;
+	bdaddr_t src, dst;
+	char srcaddr[18];
+	int ret = 0;
+
+	str2ba(address, &dst);
+	adapter_get_address(adapter, &src);
+	ba2str(&src, srcaddr);
+
+	write_device_name(&dst, &src, (char *) name);
+
+	/* Store the device's SDP record */
+	rec = record_from_string(pnp_record);
+	store_record(srcaddr, address, rec);
+	sdp_record_free(rec);
+
+	/* Set the device id */
+	store_device_id(srcaddr, address, 0xffff, vendor_id, product_id, 0);
+	/* Don't write a profile here,
+	 * it will be updated when the device connects */
+
+	write_trust(srcaddr, address, "[all]", TRUE);
+
+	conn = dbus_bus_get(DBUS_BUS_SYSTEM, NULL);
+	if (conn == NULL) {
+		DBG("Failed to get on the bus");
+		ret = -EPERM;
+		goto fail_dbus;
+	}
+
+	device = adapter_get_device(conn, adapter, address);
+	if (device == NULL) {
+		DBG("Failed to get the device");
+		ret = -ENODEV;
+		goto fail_device;
+	}
+
+	device_set_temporary(device, FALSE);
+	device_set_name(device, name);
+	btd_device_add_uuid(device, HID_UUID);
+
+fail_device:
+	dbus_connection_unref(conn);
+fail_dbus:
+	return ret;
+}
+
+/* Usb cable pairing section */
+static unsigned char *get_feature_report(int fd, uint8_t report_number,
+						unsigned int len)
+{
+	unsigned char *buf;
+	int ret;
+
+	buf = calloc(len, sizeof(*buf));
+	if (buf == NULL) {
+		error("%s:%s() calloc failed", __FILE__, __func__);
+		return NULL;
+	}
+
+	buf[0] = report_number;
+
+	ret = ioctl(fd, HIDIOCGFEATURE(len), buf);
+	if (ret < 0) {
+		error("%s:%s() HIDIOCGFEATURE ret = %d",
+			__FILE__, __func__, ret);
+		free(buf);
+		return NULL;
+	}
+
+	return buf;
+}
+
+static int set_feature_report(int fd, uint8_t *report, int len)
+{
+	int ret;
+
+	ret = ioctl(fd, HIDIOCSFEATURE(len), report);
+	if (ret < 0)
+		error("%s:%s() HIDIOCSFEATURE failed, ret = %d",
+			__FILE__, __func__, ret);
+
+	return ret;
+}
+
+static char *get_device_bdaddr(int fd)
+{
+	unsigned char *buf;
+	char *address;
+
+	buf = get_feature_report(fd, 0xf2, 18);
+	if (buf == NULL) {
+		error("%s:%s() cannot get feature report", __FILE__, __func__);
+		return NULL;
+	}
+
+	address = calloc(BDADDR_STR_SIZE, sizeof(*address));
+	if (address == NULL) {
+		error("%s:%s() calloc failed", __FILE__, __func__);
+		free(buf);
+		return NULL;
+	}
+
+	snprintf(address, BDADDR_STR_SIZE,
+			"%02X:%02X:%02X:%02X:%02X:%02X",
+			buf[4], buf[5], buf[6], buf[7], buf[8], buf[9]);
+
+	free(buf);
+	return address;
+}
+
+static char *get_master_bdaddr(int fd)
+{
+	unsigned char *buf;
+	char *address;
+
+	buf = get_feature_report(fd, 0xf5, 8);
+	if (buf == NULL) {
+		error("%s:%s() cannot get feature report", __FILE__, __func__);
+		return NULL;
+	}
+
+	address = calloc(BDADDR_STR_SIZE, sizeof(*address));
+	if (address == NULL) {
+		error("%s:%s() calloc failed", __FILE__, __func__);
+		free(buf);
+		return NULL;
+	}
+
+	snprintf(address, BDADDR_STR_SIZE,
+			"%02X:%02X:%02X:%02X:%02X:%02X",
+			buf[2], buf[3], buf[4], buf[5], buf[6], buf[7]);
+
+	free(buf);
+	return address;
+}
+
+static int set_master_bdaddr(int fd, char *adapter_bdaddr)
+{
+	uint8_t *report;
+	uint8_t addr[6];
+	int ret;
+
+	ret = sscanf(adapter_bdaddr,
+			"%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx",
+			&addr[0], &addr[1], &addr[2],
+			&addr[3], &addr[4], &addr[5]);
+	if (ret != 6) {
+
+		error("%s:%s() Parsing the bt address failed",
+			__FILE__, __func__);
+		return -EINVAL;
+	}
+
+	report = malloc(8);
+	if (report == NULL) {
+		error("%s:%s() malloc failed", __FILE__, __func__);
+		return -ENOMEM;
+	}
+
+	report[0] = 0xf5;
+	report[1] = 0x01;
+
+	report[2] = addr[0];
+	report[3] = addr[1];
+	report[4] = addr[2];
+	report[5] = addr[3];
+	report[6] = addr[4];
+	report[7] = addr[5];
+
+	ret = set_feature_report(fd, report, 8);
+	if (ret < 0) {
+		error("%s:%s() cannot set feature report",
+			__FILE__, __func__);
+		goto out;
+	}
+
+	DBG("New Master Bluetooth address: %s", adapter_bdaddr);
+
+out:
+	free(report);
+	return ret;
+}
+
+static int sixpair(int fd, struct btd_adapter *adapter)
+{
+	char *device_bdaddr;
+	char *master_bdaddr;
+	char adapter_bdaddr[18];
+	bdaddr_t dst;
+	int ret = 0;
+
+	adapter_get_address(adapter, &dst);
+	ba2str(&dst, adapter_bdaddr);
+	DBG("Adapter bdaddr %s", adapter_bdaddr);
+
+	master_bdaddr = get_master_bdaddr(fd);
+	if (master_bdaddr == NULL) {
+		DBG("Failed to get the Old master Bluetooth address from the device");
+		return -EPERM;
+	}
+
+	/* Only set the master bdaddr when needed, this is how the PS3 does
+	 * it, perhaps to avoid unnecessary writes to some eeprom.
+	 */
+	if (g_strcmp0(master_bdaddr, adapter_bdaddr) != 0) {
+		DBG("Old master Bluetooth address was: %s", master_bdaddr);
+		ret = set_master_bdaddr(fd, adapter_bdaddr);
+		if (ret < 0) {
+			DBG("Failed to set the master Bluetooth address");
+			free(master_bdaddr);
+			return ret;
+		}
+	}
+
+	device_bdaddr = get_device_bdaddr(fd);
+	if (device_bdaddr == NULL) {
+		DBG("Failed to get the Bluetooth address from the device");
+		free(master_bdaddr);
+		return -EPERM;
+	}
+
+	DBG("Device bdaddr %s", device_bdaddr);
+
+	ret = create_sixaxis_association(adapter,
+					SIXAXIS_NAME,
+					device_bdaddr,
+					VENDOR, PRODUCT, SIXAXIS_PNP_RECORD);
+	free(device_bdaddr);
+	free(master_bdaddr);
+	return ret;
+}
+
+/* Led setting section */
+static int set_leds(int fd, unsigned char leds_status[4])
+{
+	int ret;
+
+	/*
+	 * the total time the led is active (0xff means forever)
+	 * |     duty_length: how long a cycle is in deciseconds:
+	 * |     |                              (0 means "blink very fast")
+	 * |     |     ??? (Maybe a phase shift or duty_length multiplier?)
+	 * |     |     |     % of duty_length led is off (0xff means 100%)
+	 * |     |     |     |     % of duty_length led is on (0xff is 100%)
+	 * |     |     |     |     |
+	 * 0xff, 0x27, 0x10, 0x00, 0x32,
+	 */
+	unsigned char leds_report[] = {
+		0x01,
+		0x00, 0x00, 0x00, 0x00, 0x00, /* rumble values TBD */
+		0x00, 0x00, 0x00, 0x00, 0x1e, /* LED_1=0x02, LED_2=0x04 ... */
+		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_4 */
+		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_3 */
+		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_2 */
+		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_1 */
+		0x00, 0x00, 0x00, 0x00, 0x00,
+	};
+
+	int leds = 0;
+	if (leds_status[0])
+		leds |= LED_1;
+	if (leds_status[1])
+		leds |= LED_2;
+	if (leds_status[2])
+		leds |= LED_3;
+	if (leds_status[3])
+		leds |= LED_4;
+
+	leds_report[10] = leds;
+
+	ret = write(fd, leds_report, sizeof(leds_report));
+	if (ret < (ssize_t) sizeof(leds_report))
+		error("%s:%s() Unable to write to hidraw device",
+			__FILE__, __func__);
+
+	return ret;
+}
+
+static int set_controller_number(int fd, unsigned int n)
+{
+	unsigned char leds_status[4] = {0, 0, 0, 0};
+
+	switch (n) {
+	case 0:
+		break;
+	case 1:
+	case 2:
+	case 3:
+	case 4:
+		leds_status[n - 1] = LED_STATUS_ON;
+		break;
+	case 5:
+	case 6:
+	case 7:
+		leds_status[4 - 1] = LED_STATUS_ON;
+		leds_status[n - 4 - 1] = LED_STATUS_ON;
+		break;
+	default:
+		error("%s:%s() Only 7 controllers supported for now",
+			__FILE__, __func__);
+		return -1;
+	}
+
+	return set_leds(fd, leds_status);
+}
+
+
+static inline gboolean is_sixaxis(const char *hid_name)
+{
+	return g_str_has_suffix(hid_name, "PLAYSTATION(R)3 Controller") ||
+		g_str_has_suffix(hid_name,
+			"Sony Computer Entertainment Wireless Controller");
+}
+
+static void handle_device_plug(struct udev_device *udevice)
+{
+	struct udev_device *hid_parent;
+	struct udev_enumerate *enumerate;
+	struct udev_list_entry *devices, *dev_list_entry;
+	const char *hid_name;
+	const char *hid_phys;
+	const char *hidraw_node;
+	unsigned char is_usb = FALSE;
+	int js_num = 0;
+	int fd;
+
+	hid_parent = udev_device_get_parent_with_subsystem_devtype(udevice,
+								"hid", NULL);
+	if (!hid_parent) {
+		error("%s:%s() cannot get parent hid device",
+			__FILE__, __func__);
+		return;
+	}
+
+	hid_name = udev_device_get_property_value(hid_parent, "HID_NAME");
+	DBG("name: %s", hid_name);
+
+	if (!is_sixaxis(hid_name))
+		return;
+
+	DBG("Found a Sixaxis device");
+
+	hidraw_node = udev_device_get_devnode(udevice);
+
+	hid_phys = udev_device_get_property_value(hid_parent, "HID_PHYS");
+
+	/* looking for joysticks */
+	enumerate = udev_enumerate_new(udev_device_get_udev(udevice));
+	udev_enumerate_add_match_sysname(enumerate, "js*");
+	udev_enumerate_scan_devices(enumerate);
+
+	devices = udev_enumerate_get_list_entry(enumerate);
+	udev_list_entry_foreach(dev_list_entry, devices) {
+		const char *devname;
+		struct udev_device *js_dev;
+		struct udev_device *input_parent;
+		const char *input_phys;
+
+		devname = udev_list_entry_get_name(dev_list_entry);
+		js_dev = udev_device_new_from_syspath(udev_device_get_udev(udevice),
+							devname);
+
+		input_parent = udev_device_get_parent_with_subsystem_devtype(js_dev,
+							"input", NULL);
+		if (!input_parent) {
+			error("%s:%s() cannot get parent input device.",
+				__FILE__, __func__);
+			continue;
+		}
+
+		/* check this is the joystick relative to
+		 * the hidraw device above */
+		input_phys = udev_device_get_sysattr_value(input_parent,
+								"phys");
+		if (g_strcmp0(input_phys, hid_phys) == 0) {
+			const char *usb_driver;
+
+			usb_driver = udev_device_get_property_value(js_dev,
+							"ID_USB_DRIVER");
+			js_num = atoi(udev_device_get_sysnum(js_dev)) + 1;
+			DBG("joypad device_num: %d", js_num);
+			DBG("hidraw_node: %s", hidraw_node);
+			DBG("driver: %s", usb_driver);
+
+			if (g_strcmp0(usb_driver, "usbhid") == 0)
+				is_usb = TRUE;
+		}
+
+		udev_device_unref(js_dev);
+	}
+
+	udev_enumerate_unref(enumerate);
+
+	fd = open(hidraw_node, O_RDWR);
+	if (fd < 0) {
+		error("%s:%s() hidraw open", __FILE__, __func__);
+		return;
+	}
+
+	if (is_usb) {
+		struct btd_adapter *adapter;
+
+		/* Look for the default adapter */
+		adapter = manager_get_default_adapter();
+		if (adapter == NULL) {
+			DBG("No adapters, exiting");
+			return;
+		}
+		sixpair(fd, adapter);
+	}
+
+	if (js_num > 0)
+		set_controller_number(fd, js_num);
+
+	close(fd);
+}
+
+static gboolean device_event_idle(struct udev_device *udevice)
+{
+	handle_device_plug(udevice);
+	udev_device_unref(udevice);
+	return FALSE;
+}
+
+static gboolean monitor_event(GIOChannel *source, GIOCondition condition,
+				gpointer data)
+{
+	struct udev_device *udevice;
+
+	udevice = udev_monitor_receive_device(monitor);
+	if (udevice == NULL)
+		goto out;
+	if (g_strcmp0(udev_device_get_action(udevice), "add") != 0) {
+		udev_device_unref(udevice);
+		goto out;
+	}
+
+	/* Give UDEV some time to load kernel modules */
+	g_timeout_add_seconds(1, (GSourceFunc) device_event_idle, udevice);
+
+out:
+	return TRUE;
+}
+
+static int sixaxis_init(void)
+{
+	GIOChannel *channel;
+
+	DBG("Setup Sixaxis cable plugin");
+
+	ctx = udev_new();
+	monitor = udev_monitor_new_from_netlink(ctx, "udev");
+	if (monitor == NULL) {
+		error("%s:%s() Could not get udev monitor",
+			__FILE__, __func__);
+		return -1;
+	}
+
+	/* Listen for newly connected hidraw interfaces */
+	udev_monitor_filter_add_match_subsystem_devtype(monitor,
+							"hidraw", NULL);
+	udev_monitor_enable_receiving(monitor);
+
+	channel = g_io_channel_unix_new(udev_monitor_get_fd(monitor));
+	watch_id = g_io_add_watch(channel, G_IO_IN, monitor_event, NULL);
+	g_io_channel_unref(channel);
+
+	return 0;
+}
+
+static void sixaxis_exit(void)
+{
+	DBG("Cleanup Sixaxis cable plugin");
+
+	if (watch_id != 0) {
+		g_source_remove(watch_id);
+		watch_id = 0;
+	}
+	if (monitor != NULL) {
+		udev_monitor_unref(monitor);
+		monitor = NULL;
+	}
+	if (ctx != NULL) {
+		udev_unref(ctx);
+		ctx = NULL;
+	}
+}
+
+BLUETOOTH_PLUGIN_DEFINE(sixaxis, VERSION,
+			BLUETOOTH_PLUGIN_PRIORITY_DEFAULT,
+			sixaxis_init, sixaxis_exit)
-- 
1.7.5.4

[PATCH BlueZ 2/4] Add sixaxis plugin: USB pairing and LEDs settings

[Antonio Ospite]

Add a plugin which handles the connection of a Sixaxis device, when a
new hidraw device is connected the plugin:
 - Filters udev events, and select the Sixaxis device
 - Sets LEDs to match the joystick system number (for USB and BT)
 - Sets the Master bluetooth address in the Sixaxis (USB pairing)
 - Adds the device to the database of the current default
   adapter (BT association)

Signed-off-by: Bastien Nocera <hadess-0MeiytkfxGOsTnJN9+BGXg@public.gmane.org>
Signed-off-by: Antonio Ospite <ospite-aNJ+ML1ZbiP93QAQaVx+gl6hYfS7NtTn@public.gmane.org>
---
 Makefile.am       |    9 +-
 acinclude.m4      |   10 +
 plugins/sixaxis.c |  593 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 610 insertions(+), 2 deletions(-)
 create mode 100644 plugins/sixaxis.c

diff --git a/Makefile.am b/Makefile.am
index 68380d9..dbe0170 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -224,6 +224,11 @@ builtin_sources += thermometer/main.c \
 			thermometer/thermometer.h thermometer/thermometer.c
 endif
 
+if SIXAXISPLUGIN
+builtin_modules += sixaxis
+builtin_sources += plugins/sixaxis.c
+endif
+
 builtin_modules += hciops mgmtops
 builtin_sources += plugins/hciops.c plugins/mgmtops.c
 
@@ -283,7 +288,7 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
 			src/event.h src/event.c \
 			src/oob.h src/oob.c src/eir.h src/eir.c
 src_bluetoothd_LDADD = lib/libbluetooth.la @GLIB_LIBS@ @DBUS_LIBS@ \
-							@CAPNG_LIBS@ -ldl -lrt
+							@CAPNG_LIBS@ @UDEV_LIBS@ -ldl -lrt
 src_bluetoothd_LDFLAGS = -Wl,--export-dynamic \
 				-Wl,--version-script=$(srcdir)/src/bluetooth.ver
 
@@ -398,7 +403,7 @@ EXTRA_DIST += doc/manager-api.txt \
 
 AM_YFLAGS = -d
 
-AM_CFLAGS = @DBUS_CFLAGS@ @GLIB_CFLAGS@ @CAPNG_CFLAGS@ \
+AM_CFLAGS = @DBUS_CFLAGS@ @GLIB_CFLAGS@ @CAPNG_CFLAGS@ @UDEV_CFLAGS@ \
 		-DBLUETOOTH_PLUGIN_BUILTIN -DPLUGINDIR=\""$(plugindir)"\"
 
 INCLUDES = -I$(builddir)/lib -I$(builddir)/src -I$(srcdir)/src \
diff --git a/acinclude.m4 b/acinclude.m4
index 3cb9459..6176483 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -186,6 +186,7 @@ AC_DEFUN([AC_ARG_BLUEZ], [
 	sndfile_enable=${sndfile_found}
 	hal_enable=no
 	usb_enable=${usb_found}
+	sixaxis_enable=${udev_found}
 	alsa_enable=${alsa_found}
 	gstreamer_enable=${gstreamer_found}
 	audio_enable=yes
@@ -286,6 +287,10 @@ AC_DEFUN([AC_ARG_BLUEZ], [
 		usb_enable=${enableval}
 	])
 
+	AC_ARG_ENABLE(sixaxis, AC_HELP_STRING([--enable-sixaxis], [enable Sixaxis plugin]), [
+		sixaxis_enable=${enableval}
+	])
+
 	AC_ARG_ENABLE(tracer, AC_HELP_STRING([--enable-tracer], [install Tracing daemon]), [
 		tracer_enable=${enableval}
 	])
@@ -385,6 +390,10 @@ AC_DEFUN([AC_ARG_BLUEZ], [
 		AC_DEFINE(HAVE_LIBUSB, 1, [Define to 1 if you have USB library.])
 	fi
 
+	if (test "${sixaxis_enable}" = "yes" && test "${udev_found}" = "yes"); then
+		AC_DEFINE(HAVE_SIXAXIS_PLUGIN, 1, [Define to 1 if you have sixaxis plugin.])
+	fi
+
 	AM_CONDITIONAL(SNDFILE, test "${sndfile_enable}" = "yes" && test "${sndfile_found}" = "yes")
 	AM_CONDITIONAL(USB, test "${usb_enable}" = "yes" && test "${usb_found}" = "yes")
 	AM_CONDITIONAL(SBC, test "${alsa_enable}" = "yes" || test "${gstreamer_enable}" = "yes" ||
@@ -421,4 +430,5 @@ AC_DEFUN([AC_ARG_BLUEZ], [
 	AM_CONDITIONAL(DBUSOOBPLUGIN, test "${dbusoob_enable}" = "yes")
 	AM_CONDITIONAL(WIIMOTEPLUGIN, test "${wiimote_enable}" = "yes")
 	AM_CONDITIONAL(THERMOMETERPLUGIN, test "${thermometer_enable}" = "yes")
+	AM_CONDITIONAL(SIXAXISPLUGIN, test "${sixaxis_enable}" = "yes" && test "${udev_found}" = "yes")
 ])
diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
new file mode 100644
index 0000000..2b0616a
--- /dev/null
+++ b/plugins/sixaxis.c
@@ -0,0 +1,593 @@
+/*
+ * sixaxis plugin: do cable association for Sixaxis controller
+ *
+ * Copyright (C) 2009  Bastien Nocera <hadess-0MeiytkfxGOsTnJN9+BGXg@public.gmane.org>
+ * Copyright (C) 2011  Antonio Ospite <ospite-aNJ+ML1ZbiP93QAQaVx+gl6hYfS7NtTn@public.gmane.org>
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+/*
+ * In the following this terminology is used:
+ *
+ *  - controller: a Sixaxis joypad.
+ *  - adapter: the bluetooth dongle on the host system.
+ *  - adapter_bdaddr: the bdaddr of the bluetooth adapter.
+ *  - device_bdaddr: the bdaddr of the Sixaxis controller.
+ *  - master_bdaddr: the bdaddr of the adapter to be configured into the
+ *    Sixaxis controller
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <errno.h>
+#include <glib.h>
+#include <linux/hidraw.h>
+
+#define LIBUDEV_I_KNOW_THE_API_IS_SUBJECT_TO_CHANGE 1
+#include <libudev.h>
+
+#include "plugin.h"
+#include "log.h"
+#include "adapter.h"
+#include "device.h"
+#include "manager.h"
+#include "storage.h"
+#include "sdp_lib.h"
+
+/* Fallback definitions to compile with older headers */
+#ifndef HIDIOCGFEATURE
+#define HIDIOCGFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x07, len)
+#endif
+
+#ifndef HIDIOCSFEATURE
+#define HIDIOCSFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x06, len)
+#endif
+
+#define BDADDR_STR_SIZE 18 /* strlen("00:00:00:00:00:00") + 1 */
+
+/* Vendor and product ID for the Sixaxis PS3 controller */
+#define VENDOR 0x054c
+#define PRODUCT 0x0268
+#define SIXAXIS_NAME "PLAYSTATION(R)3 Controller"
+#define SIXAXIS_PNP_RECORD "3601920900000A000100000900013503191124090004350D35061901000900113503190011090006350909656E09006A0901000900093508350619112409010009000D350F350D350619010009001335031900110901002513576972656C65737320436F6E74726F6C6C65720901012513576972656C65737320436F6E74726F6C6C6572090102251B536F6E7920436F6D707574657220456E7465727461696E6D656E740902000901000902010901000902020800090203082109020428010902052801090206359A35980822259405010904A101A102850175089501150026FF00810375019513150025013500450105091901291381027501950D0600FF8103150026FF0005010901A10075089504350046FF0009300931093209358102C0050175089527090181027508953009019102750895300901B102C0A1028502750895300901B102C0A10285EE750895300901B102C0A10285EF750895300901B102C0C0090207350835060904090901000902082800090209280109020A280109020
 B09010009020C093E8009020D280009020E2800"
+#define HID_UUID "00001124-0000-1000-8000-00805f9b34fb"
+
+#define LED_1 (0x01 << 1)
+#define LED_2 (0x01 << 2)
+#define LED_3 (0x01 << 3)
+#define LED_4 (0x01 << 4)
+
+#define LED_STATUS_OFF 0
+#define LED_STATUS_ON  1
+
+static struct udev *ctx;
+static struct udev_monitor *monitor;
+static guint watch_id;
+
+
+static int create_sixaxis_association(struct btd_adapter *adapter,
+					const char *name,
+					const char *address,
+					guint32 vendor_id,
+					guint32 product_id,
+					const char *pnp_record)
+{
+	DBusConnection *conn;
+	sdp_record_t *rec;
+	struct btd_device *device;
+	bdaddr_t src, dst;
+	char srcaddr[18];
+	int ret = 0;
+
+	str2ba(address, &dst);
+	adapter_get_address(adapter, &src);
+	ba2str(&src, srcaddr);
+
+	write_device_name(&dst, &src, (char *) name);
+
+	/* Store the device's SDP record */
+	rec = record_from_string(pnp_record);
+	store_record(srcaddr, address, rec);
+	sdp_record_free(rec);
+
+	/* Set the device id */
+	store_device_id(srcaddr, address, 0xffff, vendor_id, product_id, 0);
+	/* Don't write a profile here,
+	 * it will be updated when the device connects */
+
+	write_trust(srcaddr, address, "[all]", TRUE);
+
+	conn = dbus_bus_get(DBUS_BUS_SYSTEM, NULL);
+	if (conn == NULL) {
+		DBG("Failed to get on the bus");
+		ret = -EPERM;
+		goto fail_dbus;
+	}
+
+	device = adapter_get_device(conn, adapter, address);
+	if (device == NULL) {
+		DBG("Failed to get the device");
+		ret = -ENODEV;
+		goto fail_device;
+	}
+
+	device_set_temporary(device, FALSE);
+	device_set_name(device, name);
+	btd_device_add_uuid(device, HID_UUID);
+
+fail_device:
+	dbus_connection_unref(conn);
+fail_dbus:
+	return ret;
+}
+
+/* Usb cable pairing section */
+static unsigned char *get_feature_report(int fd, uint8_t report_number,
+						unsigned int len)
+{
+	unsigned char *buf;
+	int ret;
+
+	buf = calloc(len, sizeof(*buf));
+	if (buf == NULL) {
+		error("%s:%s() calloc failed", __FILE__, __func__);
+		return NULL;
+	}
+
+	buf[0] = report_number;
+
+	ret = ioctl(fd, HIDIOCGFEATURE(len), buf);
+	if (ret < 0) {
+		error("%s:%s() HIDIOCGFEATURE ret = %d",
+			__FILE__, __func__, ret);
+		free(buf);
+		return NULL;
+	}
+
+	return buf;
+}
+
+static int set_feature_report(int fd, uint8_t *report, int len)
+{
+	int ret;
+
+	ret = ioctl(fd, HIDIOCSFEATURE(len), report);
+	if (ret < 0)
+		error("%s:%s() HIDIOCSFEATURE failed, ret = %d",
+			__FILE__, __func__, ret);
+
+	return ret;
+}
+
+static char *get_device_bdaddr(int fd)
+{
+	unsigned char *buf;
+	char *address;
+
+	buf = get_feature_report(fd, 0xf2, 18);
+	if (buf == NULL) {
+		error("%s:%s() cannot get feature report", __FILE__, __func__);
+		return NULL;
+	}
+
+	address = calloc(BDADDR_STR_SIZE, sizeof(*address));
+	if (address == NULL) {
+		error("%s:%s() calloc failed", __FILE__, __func__);
+		free(buf);
+		return NULL;
+	}
+
+	snprintf(address, BDADDR_STR_SIZE,
+			"%02X:%02X:%02X:%02X:%02X:%02X",
+			buf[4], buf[5], buf[6], buf[7], buf[8], buf[9]);
+
+	free(buf);
+	return address;
+}
+
+static char *get_master_bdaddr(int fd)
+{
+	unsigned char *buf;
+	char *address;
+
+	buf = get_feature_report(fd, 0xf5, 8);
+	if (buf == NULL) {
+		error("%s:%s() cannot get feature report", __FILE__, __func__);
+		return NULL;
+	}
+
+	address = calloc(BDADDR_STR_SIZE, sizeof(*address));
+	if (address == NULL) {
+		error("%s:%s() calloc failed", __FILE__, __func__);
+		free(buf);
+		return NULL;
+	}
+
+	snprintf(address, BDADDR_STR_SIZE,
+			"%02X:%02X:%02X:%02X:%02X:%02X",
+			buf[2], buf[3], buf[4], buf[5], buf[6], buf[7]);
+
+	free(buf);
+	return address;
+}
+
+static int set_master_bdaddr(int fd, char *adapter_bdaddr)
+{
+	uint8_t *report;
+	uint8_t addr[6];
+	int ret;
+
+	ret = sscanf(adapter_bdaddr,
+			"%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx",
+			&addr[0], &addr[1], &addr[2],
+			&addr[3], &addr[4], &addr[5]);
+	if (ret != 6) {
+
+		error("%s:%s() Parsing the bt address failed",
+			__FILE__, __func__);
+		return -EINVAL;
+	}
+
+	report = malloc(8);
+	if (report == NULL) {
+		error("%s:%s() malloc failed", __FILE__, __func__);
+		return -ENOMEM;
+	}
+
+	report[0] = 0xf5;
+	report[1] = 0x01;
+
+	report[2] = addr[0];
+	report[3] = addr[1];
+	report[4] = addr[2];
+	report[5] = addr[3];
+	report[6] = addr[4];
+	report[7] = addr[5];
+
+	ret = set_feature_report(fd, report, 8);
+	if (ret < 0) {
+		error("%s:%s() cannot set feature report",
+			__FILE__, __func__);
+		goto out;
+	}
+
+	DBG("New Master Bluetooth address: %s", adapter_bdaddr);
+
+out:
+	free(report);
+	return ret;
+}
+
+static int sixpair(int fd, struct btd_adapter *adapter)
+{
+	char *device_bdaddr;
+	char *master_bdaddr;
+	char adapter_bdaddr[18];
+	bdaddr_t dst;
+	int ret = 0;
+
+	adapter_get_address(adapter, &dst);
+	ba2str(&dst, adapter_bdaddr);
+	DBG("Adapter bdaddr %s", adapter_bdaddr);
+
+	master_bdaddr = get_master_bdaddr(fd);
+	if (master_bdaddr == NULL) {
+		DBG("Failed to get the Old master Bluetooth address from the device");
+		return -EPERM;
+	}
+
+	/* Only set the master bdaddr when needed, this is how the PS3 does
+	 * it, perhaps to avoid unnecessary writes to some eeprom.
+	 */
+	if (g_strcmp0(master_bdaddr, adapter_bdaddr) != 0) {
+		DBG("Old master Bluetooth address was: %s", master_bdaddr);
+		ret = set_master_bdaddr(fd, adapter_bdaddr);
+		if (ret < 0) {
+			DBG("Failed to set the master Bluetooth address");
+			free(master_bdaddr);
+			return ret;
+		}
+	}
+
+	device_bdaddr = get_device_bdaddr(fd);
+	if (device_bdaddr == NULL) {
+		DBG("Failed to get the Bluetooth address from the device");
+		free(master_bdaddr);
+		return -EPERM;
+	}
+
+	DBG("Device bdaddr %s", device_bdaddr);
+
+	ret = create_sixaxis_association(adapter,
+					SIXAXIS_NAME,
+					device_bdaddr,
+					VENDOR, PRODUCT, SIXAXIS_PNP_RECORD);
+	free(device_bdaddr);
+	free(master_bdaddr);
+	return ret;
+}
+
+/* Led setting section */
+static int set_leds(int fd, unsigned char leds_status[4])
+{
+	int ret;
+
+	/*
+	 * the total time the led is active (0xff means forever)
+	 * |     duty_length: how long a cycle is in deciseconds:
+	 * |     |                              (0 means "blink very fast")
+	 * |     |     ??? (Maybe a phase shift or duty_length multiplier?)
+	 * |     |     |     % of duty_length led is off (0xff means 100%)
+	 * |     |     |     |     % of duty_length led is on (0xff is 100%)
+	 * |     |     |     |     |
+	 * 0xff, 0x27, 0x10, 0x00, 0x32,
+	 */
+	unsigned char leds_report[] = {
+		0x01,
+		0x00, 0x00, 0x00, 0x00, 0x00, /* rumble values TBD */
+		0x00, 0x00, 0x00, 0x00, 0x1e, /* LED_1=0x02, LED_2=0x04 ... */
+		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_4 */
+		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_3 */
+		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_2 */
+		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_1 */
+		0x00, 0x00, 0x00, 0x00, 0x00,
+	};
+
+	int leds = 0;
+	if (leds_status[0])
+		leds |= LED_1;
+	if (leds_status[1])
+		leds |= LED_2;
+	if (leds_status[2])
+		leds |= LED_3;
+	if (leds_status[3])
+		leds |= LED_4;
+
+	leds_report[10] = leds;
+
+	ret = write(fd, leds_report, sizeof(leds_report));
+	if (ret < (ssize_t) sizeof(leds_report))
+		error("%s:%s() Unable to write to hidraw device",
+			__FILE__, __func__);
+
+	return ret;
+}
+
+static int set_controller_number(int fd, unsigned int n)
+{
+	unsigned char leds_status[4] = {0, 0, 0, 0};
+
+	switch (n) {
+	case 0:
+		break;
+	case 1:
+	case 2:
+	case 3:
+	case 4:
+		leds_status[n - 1] = LED_STATUS_ON;
+		break;
+	case 5:
+	case 6:
+	case 7:
+		leds_status[4 - 1] = LED_STATUS_ON;
+		leds_status[n - 4 - 1] = LED_STATUS_ON;
+		break;
+	default:
+		error("%s:%s() Only 7 controllers supported for now",
+			__FILE__, __func__);
+		return -1;
+	}
+
+	return set_leds(fd, leds_status);
+}
+
+
+static inline gboolean is_sixaxis(const char *hid_name)
+{
+	return g_str_has_suffix(hid_name, "PLAYSTATION(R)3 Controller") ||
+		g_str_has_suffix(hid_name,
+			"Sony Computer Entertainment Wireless Controller");
+}
+
+static void handle_device_plug(struct udev_device *udevice)
+{
+	struct udev_device *hid_parent;
+	struct udev_enumerate *enumerate;
+	struct udev_list_entry *devices, *dev_list_entry;
+	const char *hid_name;
+	const char *hid_phys;
+	const char *hidraw_node;
+	unsigned char is_usb = FALSE;
+	int js_num = 0;
+	int fd;
+
+	hid_parent = udev_device_get_parent_with_subsystem_devtype(udevice,
+								"hid", NULL);
+	if (!hid_parent) {
+		error("%s:%s() cannot get parent hid device",
+			__FILE__, __func__);
+		return;
+	}
+
+	hid_name = udev_device_get_property_value(hid_parent, "HID_NAME");
+	DBG("name: %s", hid_name);
+
+	if (!is_sixaxis(hid_name))
+		return;
+
+	DBG("Found a Sixaxis device");
+
+	hidraw_node = udev_device_get_devnode(udevice);
+
+	hid_phys = udev_device_get_property_value(hid_parent, "HID_PHYS");
+
+	/* looking for joysticks */
+	enumerate = udev_enumerate_new(udev_device_get_udev(udevice));
+	udev_enumerate_add_match_sysname(enumerate, "js*");
+	udev_enumerate_scan_devices(enumerate);
+
+	devices = udev_enumerate_get_list_entry(enumerate);
+	udev_list_entry_foreach(dev_list_entry, devices) {
+		const char *devname;
+		struct udev_device *js_dev;
+		struct udev_device *input_parent;
+		const char *input_phys;
+
+		devname = udev_list_entry_get_name(dev_list_entry);
+		js_dev = udev_device_new_from_syspath(udev_device_get_udev(udevice),
+							devname);
+
+		input_parent = udev_device_get_parent_with_subsystem_devtype(js_dev,
+							"input", NULL);
+		if (!input_parent) {
+			error("%s:%s() cannot get parent input device.",
+				__FILE__, __func__);
+			continue;
+		}
+
+		/* check this is the joystick relative to
+		 * the hidraw device above */
+		input_phys = udev_device_get_sysattr_value(input_parent,
+								"phys");
+		if (g_strcmp0(input_phys, hid_phys) == 0) {
+			const char *usb_driver;
+
+			usb_driver = udev_device_get_property_value(js_dev,
+							"ID_USB_DRIVER");
+			js_num = atoi(udev_device_get_sysnum(js_dev)) + 1;
+			DBG("joypad device_num: %d", js_num);
+			DBG("hidraw_node: %s", hidraw_node);
+			DBG("driver: %s", usb_driver);
+
+			if (g_strcmp0(usb_driver, "usbhid") == 0)
+				is_usb = TRUE;
+		}
+
+		udev_device_unref(js_dev);
+	}
+
+	udev_enumerate_unref(enumerate);
+
+	fd = open(hidraw_node, O_RDWR);
+	if (fd < 0) {
+		error("%s:%s() hidraw open", __FILE__, __func__);
+		return;
+	}
+
+	if (is_usb) {
+		struct btd_adapter *adapter;
+
+		/* Look for the default adapter */
+		adapter = manager_get_default_adapter();
+		if (adapter == NULL) {
+			DBG("No adapters, exiting");
+			return;
+		}
+		sixpair(fd, adapter);
+	}
+
+	if (js_num > 0)
+		set_controller_number(fd, js_num);
+
+	close(fd);
+}
+
+static gboolean device_event_idle(struct udev_device *udevice)
+{
+	handle_device_plug(udevice);
+	udev_device_unref(udevice);
+	return FALSE;
+}
+
+static gboolean monitor_event(GIOChannel *source, GIOCondition condition,
+				gpointer data)
+{
+	struct udev_device *udevice;
+
+	udevice = udev_monitor_receive_device(monitor);
+	if (udevice == NULL)
+		goto out;
+	if (g_strcmp0(udev_device_get_action(udevice), "add") != 0) {
+		udev_device_unref(udevice);
+		goto out;
+	}
+
+	/* Give UDEV some time to load kernel modules */
+	g_timeout_add_seconds(1, (GSourceFunc) device_event_idle, udevice);
+
+out:
+	return TRUE;
+}
+
+static int sixaxis_init(void)
+{
+	GIOChannel *channel;
+
+	DBG("Setup Sixaxis cable plugin");
+
+	ctx = udev_new();
+	monitor = udev_monitor_new_from_netlink(ctx, "udev");
+	if (monitor == NULL) {
+		error("%s:%s() Could not get udev monitor",
+			__FILE__, __func__);
+		return -1;
+	}
+
+	/* Listen for newly connected hidraw interfaces */
+	udev_monitor_filter_add_match_subsystem_devtype(monitor,
+							"hidraw", NULL);
+	udev_monitor_enable_receiving(monitor);
+
+	channel = g_io_channel_unix_new(udev_monitor_get_fd(monitor));
+	watch_id = g_io_add_watch(channel, G_IO_IN, monitor_event, NULL);
+	g_io_channel_unref(channel);
+
+	return 0;
+}
+
+static void sixaxis_exit(void)
+{
+	DBG("Cleanup Sixaxis cable plugin");
+
+	if (watch_id != 0) {
+		g_source_remove(watch_id);
+		watch_id = 0;
+	}
+	if (monitor != NULL) {
+		udev_monitor_unref(monitor);
+		monitor = NULL;
+	}
+	if (ctx != NULL) {
+		udev_unref(ctx);
+		ctx = NULL;
+	}
+}
+
+BLUETOOTH_PLUGIN_DEFINE(sixaxis, VERSION,
+			BLUETOOTH_PLUGIN_PRIORITY_DEFAULT,
+			sixaxis_init, sixaxis_exit)
-- 
1.7.5.4

Re: [PATCH BlueZ 2/4] Add sixaxis plugin: USB pairing and LEDs settings

[Alan Ott]

On 08/05/2011 10:09 AM, Antonio Ospite wrote:
> Add a plugin which handles the connection of a Sixaxis device, when a
> new hidraw device is connected the plugin:
>  - Filters udev events, and select the Sixaxis device
>  - Sets LEDs to match the joystick system number (for USB and BT)
>  - Sets the Master bluetooth address in the Sixaxis (USB pairing)
>  - Adds the device to the database of the current default
>    adapter (BT association)
>
> Signed-off-by: Bastien Nocera <hadess@hadess.net>
> Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
> ---
>  Makefile.am       |    9 +-
>  acinclude.m4      |   10 +
>  plugins/sixaxis.c |  593 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 610 insertions(+), 2 deletions(-)
>  create mode 100644 plugins/sixaxis.c
>
> diff --git a/Makefile.am b/Makefile.am
> index 68380d9..dbe0170 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -224,6 +224,11 @@ builtin_sources += thermometer/main.c \
>  			thermometer/thermometer.h thermometer/thermometer.c
>  endif
>  
> +if SIXAXISPLUGIN
> +builtin_modules += sixaxis
> +builtin_sources += plugins/sixaxis.c
> +endif
> +
>  builtin_modules += hciops mgmtops
>  builtin_sources += plugins/hciops.c plugins/mgmtops.c
>  
> @@ -283,7 +288,7 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
>  			src/event.h src/event.c \
>  			src/oob.h src/oob.c src/eir.h src/eir.c
>  src_bluetoothd_LDADD = lib/libbluetooth.la @GLIB_LIBS@ @DBUS_LIBS@ \
> -							@CAPNG_LIBS@ -ldl -lrt
> +							@CAPNG_LIBS@ @UDEV_LIBS@ -ldl -lrt
>  src_bluetoothd_LDFLAGS = -Wl,--export-dynamic \
>  				-Wl,--version-script=$(srcdir)/src/bluetooth.ver
>  
> @@ -398,7 +403,7 @@ EXTRA_DIST += doc/manager-api.txt \
>  
>  AM_YFLAGS = -d
>  
> -AM_CFLAGS = @DBUS_CFLAGS@ @GLIB_CFLAGS@ @CAPNG_CFLAGS@ \
> +AM_CFLAGS = @DBUS_CFLAGS@ @GLIB_CFLAGS@ @CAPNG_CFLAGS@ @UDEV_CFLAGS@ \
>  		-DBLUETOOTH_PLUGIN_BUILTIN -DPLUGINDIR=\""$(plugindir)"\"
>  
>  INCLUDES = -I$(builddir)/lib -I$(builddir)/src -I$(srcdir)/src \
> diff --git a/acinclude.m4 b/acinclude.m4
> index 3cb9459..6176483 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -186,6 +186,7 @@ AC_DEFUN([AC_ARG_BLUEZ], [
>  	sndfile_enable=${sndfile_found}
>  	hal_enable=no
>  	usb_enable=${usb_found}
> +	sixaxis_enable=${udev_found}
>  	alsa_enable=${alsa_found}
>  	gstreamer_enable=${gstreamer_found}
>  	audio_enable=yes
> @@ -286,6 +287,10 @@ AC_DEFUN([AC_ARG_BLUEZ], [
>  		usb_enable=${enableval}
>  	])
>  
> +	AC_ARG_ENABLE(sixaxis, AC_HELP_STRING([--enable-sixaxis], [enable Sixaxis plugin]), [
> +		sixaxis_enable=${enableval}
> +	])
> +
>  	AC_ARG_ENABLE(tracer, AC_HELP_STRING([--enable-tracer], [install Tracing daemon]), [
>  		tracer_enable=${enableval}
>  	])
> @@ -385,6 +390,10 @@ AC_DEFUN([AC_ARG_BLUEZ], [
>  		AC_DEFINE(HAVE_LIBUSB, 1, [Define to 1 if you have USB library.])
>  	fi
>  
> +	if (test "${sixaxis_enable}" = "yes" && test "${udev_found}" = "yes"); then
> +		AC_DEFINE(HAVE_SIXAXIS_PLUGIN, 1, [Define to 1 if you have sixaxis plugin.])
> +	fi
> +
>  	AM_CONDITIONAL(SNDFILE, test "${sndfile_enable}" = "yes" && test "${sndfile_found}" = "yes")
>  	AM_CONDITIONAL(USB, test "${usb_enable}" = "yes" && test "${usb_found}" = "yes")
>  	AM_CONDITIONAL(SBC, test "${alsa_enable}" = "yes" || test "${gstreamer_enable}" = "yes" ||
> @@ -421,4 +430,5 @@ AC_DEFUN([AC_ARG_BLUEZ], [
>  	AM_CONDITIONAL(DBUSOOBPLUGIN, test "${dbusoob_enable}" = "yes")
>  	AM_CONDITIONAL(WIIMOTEPLUGIN, test "${wiimote_enable}" = "yes")
>  	AM_CONDITIONAL(THERMOMETERPLUGIN, test "${thermometer_enable}" = "yes")
> +	AM_CONDITIONAL(SIXAXISPLUGIN, test "${sixaxis_enable}" = "yes" && test "${udev_found}" = "yes")
>  ])
> diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
> new file mode 100644
> index 0000000..2b0616a
> --- /dev/null
> +++ b/plugins/sixaxis.c
> @@ -0,0 +1,593 @@
> +/*
> + * sixaxis plugin: do cable association for Sixaxis controller
> + *
> + * Copyright (C) 2009  Bastien Nocera <hadess@hadess.net>
> + * Copyright (C) 2011  Antonio Ospite <ospite@studenti.unina.it>
> + *
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +
> +/*
> + * In the following this terminology is used:
> + *
> + *  - controller: a Sixaxis joypad.
> + *  - adapter: the bluetooth dongle on the host system.
> + *  - adapter_bdaddr: the bdaddr of the bluetooth adapter.
> + *  - device_bdaddr: the bdaddr of the Sixaxis controller.
> + *  - master_bdaddr: the bdaddr of the adapter to be configured into the
> + *    Sixaxis controller
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <stdio.h>
> +#include <stdint.h>
> +#include <stdlib.h>
> +#include <sys/ioctl.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <glib.h>
> +#include <linux/hidraw.h>
> +
> +#define LIBUDEV_I_KNOW_THE_API_IS_SUBJECT_TO_CHANGE 1
> +#include <libudev.h>
> +
> +#include "plugin.h"
> +#include "log.h"
> +#include "adapter.h"
> +#include "device.h"
> +#include "manager.h"
> +#include "storage.h"
> +#include "sdp_lib.h"
> +
> +/* Fallback definitions to compile with older headers */
> +#ifndef HIDIOCGFEATURE
> +#define HIDIOCGFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x07, len)
> +#endif
> +
> +#ifndef HIDIOCSFEATURE
> +#define HIDIOCSFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x06, len)
> +#endif
> +
> +#define BDADDR_STR_SIZE 18 /* strlen("00:00:00:00:00:00") + 1 */
> +
> +/* Vendor and product ID for the Sixaxis PS3 controller */
> +#define VENDOR 0x054c
> +#define PRODUCT 0x0268
> +#define SIXAXIS_NAME "PLAYSTATION(R)3 Controller"
> +#define SIXAXIS_PNP_RECORD "3601920900000A000100000900013503191124090004350D35061901000900113503190011090006350909656E09006A0901000900093508350619112409010009000D350F350D350619010009001335031900110901002513576972656C65737320436F6E74726F6C6C65720901012513576972656C65737320436F6E74726F6C6C6572090102251B536F6E7920436F6D707574657220456E7465727461696E6D656E740902000901000902010901000902020800090203082109020428010902052801090206359A35980822259405010904A101A102850175089501150026FF00810375019513150025013500450105091901291381027501950D0600FF8103150026FF0005010901A10075089504350046FF0009300931093209358102C0050175089527090181027508953009019102750895300901B102C0A1028502750895300901B102C0A10285EE750895300901B102C0A10285EF750895300901B102C0C0090207350835060904090901000902082800090209280109020A280109020B09010009020C093E8009020D280009020E2800"
> +#define HID_UUID "00001124-0000-1000-8000-00805f9b34fb"
> +
> +#define LED_1 (0x01 << 1)
> +#define LED_2 (0x01 << 2)
> +#define LED_3 (0x01 << 3)
> +#define LED_4 (0x01 << 4)
> +
> +#define LED_STATUS_OFF 0
> +#define LED_STATUS_ON  1
> +
> +static struct udev *ctx;
> +static struct udev_monitor *monitor;
> +static guint watch_id;
> +
> +
> +static int create_sixaxis_association(struct btd_adapter *adapter,
> +					const char *name,
> +					const char *address,
> +					guint32 vendor_id,
> +					guint32 product_id,
> +					const char *pnp_record)
> +{
> +	DBusConnection *conn;
> +	sdp_record_t *rec;
> +	struct btd_device *device;
> +	bdaddr_t src, dst;
> +	char srcaddr[18];
> +	int ret = 0;
> +
> +	str2ba(address, &dst);
> +	adapter_get_address(adapter, &src);
> +	ba2str(&src, srcaddr);
> +
> +	write_device_name(&dst, &src, (char *) name);
> +
> +	/* Store the device's SDP record */
> +	rec = record_from_string(pnp_record);
> +	store_record(srcaddr, address, rec);
> +	sdp_record_free(rec);
> +
> +	/* Set the device id */
> +	store_device_id(srcaddr, address, 0xffff, vendor_id, product_id, 0);
> +	/* Don't write a profile here,
> +	 * it will be updated when the device connects */
> +
> +	write_trust(srcaddr, address, "[all]", TRUE);
> +
> +	conn = dbus_bus_get(DBUS_BUS_SYSTEM, NULL);
> +	if (conn == NULL) {
> +		DBG("Failed to get on the bus");
> +		ret = -EPERM;
> +		goto fail_dbus;
> +	}
> +
> +	device = adapter_get_device(conn, adapter, address);
> +	if (device == NULL) {
> +		DBG("Failed to get the device");
> +		ret = -ENODEV;
> +		goto fail_device;
> +	}
> +
> +	device_set_temporary(device, FALSE);
> +	device_set_name(device, name);
> +	btd_device_add_uuid(device, HID_UUID);
> +
> +fail_device:
> +	dbus_connection_unref(conn);
> +fail_dbus:
> +	return ret;
> +}
> +
> +/* Usb cable pairing section */
> +static unsigned char *get_feature_report(int fd, uint8_t report_number,
> +						unsigned int len)
> +{
> +	unsigned char *buf;
> +	int ret;
> +
> +	buf = calloc(len, sizeof(*buf));
> +	if (buf == NULL) {
> +		error("%s:%s() calloc failed", __FILE__, __func__);
> +		return NULL;
> +	}
> +
> +	buf[0] = report_number;
> +
> +	ret = ioctl(fd, HIDIOCGFEATURE(len), buf);
> +	if (ret < 0) {
> +		error("%s:%s() HIDIOCGFEATURE ret = %d",
> +			__FILE__, __func__, ret);
> +		free(buf);
> +		return NULL;
> +	}
> +
> +	return buf;
> +}
> +
> +static int set_feature_report(int fd, uint8_t *report, int len)
> +{
> +	int ret;
> +
> +	ret = ioctl(fd, HIDIOCSFEATURE(len), report);
> +	if (ret < 0)
> +		error("%s:%s() HIDIOCSFEATURE failed, ret = %d",
> +			__FILE__, __func__, ret);
> +
> +	return ret;
> +}
> +
> +static char *get_device_bdaddr(int fd)
> +{
> +	unsigned char *buf;
> +	char *address;
> +
> +	buf = get_feature_report(fd, 0xf2, 18);
> +	if (buf == NULL) {
> +		error("%s:%s() cannot get feature report", __FILE__, __func__);
> +		return NULL;
> +	}
> +
> +	address = calloc(BDADDR_STR_SIZE, sizeof(*address));
> +	if (address == NULL) {
> +		error("%s:%s() calloc failed", __FILE__, __func__);
> +		free(buf);
> +		return NULL;
> +	}
> +
> +	snprintf(address, BDADDR_STR_SIZE,
> +			"%02X:%02X:%02X:%02X:%02X:%02X",
> +			buf[4], buf[5], buf[6], buf[7], buf[8], buf[9]);
> +
> +	free(buf);
> +	return address;
> +}
> +
> +static char *get_master_bdaddr(int fd)
> +{
> +	unsigned char *buf;
> +	char *address;
> +
> +	buf = get_feature_report(fd, 0xf5, 8);
> +	if (buf == NULL) {
> +		error("%s:%s() cannot get feature report", __FILE__, __func__);
> +		return NULL;
> +	}
> +
> +	address = calloc(BDADDR_STR_SIZE, sizeof(*address));
> +	if (address == NULL) {
> +		error("%s:%s() calloc failed", __FILE__, __func__);
> +		free(buf);
> +		return NULL;
> +	}
> +
> +	snprintf(address, BDADDR_STR_SIZE,
> +			"%02X:%02X:%02X:%02X:%02X:%02X",
> +			buf[2], buf[3], buf[4], buf[5], buf[6], buf[7]);
> +
> +	free(buf);
> +	return address;
> +}
> +
> +static int set_master_bdaddr(int fd, char *adapter_bdaddr)
> +{
> +	uint8_t *report;
> +	uint8_t addr[6];
> +	int ret;
> +
> +	ret = sscanf(adapter_bdaddr,
> +			"%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx",
> +			&addr[0], &addr[1], &addr[2],
> +			&addr[3], &addr[4], &addr[5]);
> +	if (ret != 6) {
> +
> +		error("%s:%s() Parsing the bt address failed",
> +			__FILE__, __func__);
> +		return -EINVAL;
> +	}
> +
> +	report = malloc(8);
> +	if (report == NULL) {
> +		error("%s:%s() malloc failed", __FILE__, __func__);
> +		return -ENOMEM;
> +	}
> +
> +	report[0] = 0xf5;
> +	report[1] = 0x01;
> +
> +	report[2] = addr[0];
> +	report[3] = addr[1];
> +	report[4] = addr[2];
> +	report[5] = addr[3];
> +	report[6] = addr[4];
> +	report[7] = addr[5];
> +
> +	ret = set_feature_report(fd, report, 8);
> +	if (ret < 0) {
> +		error("%s:%s() cannot set feature report",
> +			__FILE__, __func__);
> +		goto out;
> +	}
> +
> +	DBG("New Master Bluetooth address: %s", adapter_bdaddr);
> +
> +out:
> +	free(report);
> +	return ret;
> +}
> +
> +static int sixpair(int fd, struct btd_adapter *adapter)
> +{
> +	char *device_bdaddr;
> +	char *master_bdaddr;
> +	char adapter_bdaddr[18];
> +	bdaddr_t dst;
> +	int ret = 0;
> +
> +	adapter_get_address(adapter, &dst);
> +	ba2str(&dst, adapter_bdaddr);
> +	DBG("Adapter bdaddr %s", adapter_bdaddr);
> +
> +	master_bdaddr = get_master_bdaddr(fd);
> +	if (master_bdaddr == NULL) {
> +		DBG("Failed to get the Old master Bluetooth address from the device");
> +		return -EPERM;
> +	}
> +
> +	/* Only set the master bdaddr when needed, this is how the PS3 does
> +	 * it, perhaps to avoid unnecessary writes to some eeprom.
> +	 */
> +	if (g_strcmp0(master_bdaddr, adapter_bdaddr) != 0) {
> +		DBG("Old master Bluetooth address was: %s", master_bdaddr);
> +		ret = set_master_bdaddr(fd, adapter_bdaddr);
> +		if (ret < 0) {
> +			DBG("Failed to set the master Bluetooth address");
> +			free(master_bdaddr);
> +			return ret;
> +		}
> +	}
> +
> +	device_bdaddr = get_device_bdaddr(fd);
> +	if (device_bdaddr == NULL) {
> +		DBG("Failed to get the Bluetooth address from the device");
> +		free(master_bdaddr);
> +		return -EPERM;
> +	}
> +
> +	DBG("Device bdaddr %s", device_bdaddr);
> +
> +	ret = create_sixaxis_association(adapter,
> +					SIXAXIS_NAME,
> +					device_bdaddr,
> +					VENDOR, PRODUCT, SIXAXIS_PNP_RECORD);

You create the association with a hard-coded VID/PID. You match the
device on string name below (not VID/PID), so what if you used the
actual VID/PID of the device for the association instead?


> +	free(device_bdaddr);
> +	free(master_bdaddr);
> +	return ret;
> +}
> +
> +/* Led setting section */
> +static int set_leds(int fd, unsigned char leds_status[4])
> +{
> +	int ret;
> +
> +	/*
> +	 * the total time the led is active (0xff means forever)
> +	 * |     duty_length: how long a cycle is in deciseconds:
> +	 * |     |                              (0 means "blink very fast")
> +	 * |     |     ??? (Maybe a phase shift or duty_length multiplier?)
> +	 * |     |     |     % of duty_length led is off (0xff means 100%)
> +	 * |     |     |     |     % of duty_length led is on (0xff is 100%)
> +	 * |     |     |     |     |
> +	 * 0xff, 0x27, 0x10, 0x00, 0x32,
> +	 */
> +	unsigned char leds_report[] = {
> +		0x01,
> +		0x00, 0x00, 0x00, 0x00, 0x00, /* rumble values TBD */
> +		0x00, 0x00, 0x00, 0x00, 0x1e, /* LED_1=0x02, LED_2=0x04 ... */
> +		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_4 */
> +		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_3 */
> +		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_2 */
> +		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_1 */
> +		0x00, 0x00, 0x00, 0x00, 0x00,
> +	};
> +
> +	int leds = 0;
> +	if (leds_status[0])
> +		leds |= LED_1;
> +	if (leds_status[1])
> +		leds |= LED_2;
> +	if (leds_status[2])
> +		leds |= LED_3;
> +	if (leds_status[3])
> +		leds |= LED_4;
> +
> +	leds_report[10] = leds;

If you are overwriting leds_report[10] unconditionally, why is it
initialized to 0x1e in the leds_report initializer (unless I counted wrong)?




> +
> +	ret = write(fd, leds_report, sizeof(leds_report));
> +	if (ret < (ssize_t) sizeof(leds_report))
> +		error("%s:%s() Unable to write to hidraw device",
> +			__FILE__, __func__);
> +
> +	return ret;
> +}
> +
> +static int set_controller_number(int fd, unsigned int n)
> +{
> +	unsigned char leds_status[4] = {0, 0, 0, 0};
> +
> +	switch (n) {
> +	case 0:
> +		break;
> +	case 1:
> +	case 2:
> +	case 3:
> +	case 4:
> +		leds_status[n - 1] = LED_STATUS_ON;
> +		break;
> +	case 5:
> +	case 6:
> +	case 7:
> +		leds_status[4 - 1] = LED_STATUS_ON;
> +		leds_status[n - 4 - 1] = LED_STATUS_ON;
> +		break;
> +	default:
> +		error("%s:%s() Only 7 controllers supported for now",
> +			__FILE__, __func__);
> +		return -1;
> +	}
> +
> +	return set_leds(fd, leds_status);
> +}
> +
> +
> +static inline gboolean is_sixaxis(const char *hid_name)
> +{
> +	return g_str_has_suffix(hid_name, "PLAYSTATION(R)3 Controller") ||
> +		g_str_has_suffix(hid_name,
> +			"Sony Computer Entertainment Wireless Controller");
> +}

Why do you key on the string name here. This seems problematic,
especially if you want to support things like the PSMove. I'd expect a
table of VID/PIDs or something.

> +
> +static void handle_device_plug(struct udev_device *udevice)
> +{
> +	struct udev_device *hid_parent;
> +	struct udev_enumerate *enumerate;
> +	struct udev_list_entry *devices, *dev_list_entry;
> +	const char *hid_name;
> +	const char *hid_phys;
> +	const char *hidraw_node;
> +	unsigned char is_usb = FALSE;
> +	int js_num = 0;
> +	int fd;
> +
> +	hid_parent = udev_device_get_parent_with_subsystem_devtype(udevice,
> +								"hid", NULL);
> +	if (!hid_parent) {
> +		error("%s:%s() cannot get parent hid device",
> +			__FILE__, __func__);
> +		return;
> +	}
> +
> +	hid_name = udev_device_get_property_value(hid_parent, "HID_NAME");
> +	DBG("name: %s", hid_name);
> +
> +	if (!is_sixaxis(hid_name))
> +		return;
> +
> +	DBG("Found a Sixaxis device");
> +
> +	hidraw_node = udev_device_get_devnode(udevice);
> +
> +	hid_phys = udev_device_get_property_value(hid_parent, "HID_PHYS");
> +
> +	/* looking for joysticks */
> +	enumerate = udev_enumerate_new(udev_device_get_udev(udevice));
> +	udev_enumerate_add_match_sysname(enumerate, "js*");
> +	udev_enumerate_scan_devices(enumerate);
> +
> +	devices = udev_enumerate_get_list_entry(enumerate);
> +	udev_list_entry_foreach(dev_list_entry, devices) {
> +		const char *devname;
> +		struct udev_device *js_dev;
> +		struct udev_device *input_parent;
> +		const char *input_phys;
> +
> +		devname = udev_list_entry_get_name(dev_list_entry);
> +		js_dev = udev_device_new_from_syspath(udev_device_get_udev(udevice),
> +							devname);
> +
> +		input_parent = udev_device_get_parent_with_subsystem_devtype(js_dev,
> +							"input", NULL);
> +		if (!input_parent) {
> +			error("%s:%s() cannot get parent input device.",
> +				__FILE__, __func__);
> +			continue;
> +		}
> +
> +		/* check this is the joystick relative to
> +		 * the hidraw device above */
> +		input_phys = udev_device_get_sysattr_value(input_parent,
> +								"phys");
> +		if (g_strcmp0(input_phys, hid_phys) == 0) {
> +			const char *usb_driver;
> +
> +			usb_driver = udev_device_get_property_value(js_dev,
> +							"ID_USB_DRIVER");
> +			js_num = atoi(udev_device_get_sysnum(js_dev)) + 1;
> +			DBG("joypad device_num: %d", js_num);
> +			DBG("hidraw_node: %s", hidraw_node);
> +			DBG("driver: %s", usb_driver);
> +
> +			if (g_strcmp0(usb_driver, "usbhid") == 0)
> +				is_usb = TRUE;
> +		}
> +
> +		udev_device_unref(js_dev);
> +	}
> +
> +	udev_enumerate_unref(enumerate);
> +
> +	fd = open(hidraw_node, O_RDWR);
> +	if (fd < 0) {
> +		error("%s:%s() hidraw open", __FILE__, __func__);
> +		return;
> +	}
> +
> +	if (is_usb) {
> +		struct btd_adapter *adapter;
> +
> +		/* Look for the default adapter */
> +		adapter = manager_get_default_adapter();
> +		if (adapter == NULL) {
> +			DBG("No adapters, exiting");
> +			return;
> +		}
> +		sixpair(fd, adapter);
> +	}
> +
> +	if (js_num > 0)
> +		set_controller_number(fd, js_num);
> +
> +	close(fd);
> +}
> +
> +static gboolean device_event_idle(struct udev_device *udevice)
> +{
> +	handle_device_plug(udevice);
> +	udev_device_unref(udevice);
> +	return FALSE;
> +}

I'm a little confused why this is called "idle."


Thanks for your work on this, Antonio. Linux working out of the box with
these controllers will be really cool.

Alan.

Re: [PATCH BlueZ 2/4] Add sixaxis plugin: USB pairing and LEDs settings

[Alan Ott]

On 08/05/2011 10:09 AM, Antonio Ospite wrote:
> Add a plugin which handles the connection of a Sixaxis device, when a
> new hidraw device is connected the plugin:
>  - Filters udev events, and select the Sixaxis device
>  - Sets LEDs to match the joystick system number (for USB and BT)
>  - Sets the Master bluetooth address in the Sixaxis (USB pairing)
>  - Adds the device to the database of the current default
>    adapter (BT association)
>
> Signed-off-by: Bastien Nocera <hadess-0MeiytkfxGOsTnJN9+BGXg@public.gmane.org>
> Signed-off-by: Antonio Ospite <ospite-aNJ+ML1ZbiP93QAQaVx+gl6hYfS7NtTn@public.gmane.org>
> ---
>  Makefile.am       |    9 +-
>  acinclude.m4      |   10 +
>  plugins/sixaxis.c |  593 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 610 insertions(+), 2 deletions(-)
>  create mode 100644 plugins/sixaxis.c
>
> diff --git a/Makefile.am b/Makefile.am
> index 68380d9..dbe0170 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -224,6 +224,11 @@ builtin_sources += thermometer/main.c \
>  			thermometer/thermometer.h thermometer/thermometer.c
>  endif
>  
> +if SIXAXISPLUGIN
> +builtin_modules += sixaxis
> +builtin_sources += plugins/sixaxis.c
> +endif
> +
>  builtin_modules += hciops mgmtops
>  builtin_sources += plugins/hciops.c plugins/mgmtops.c
>  
> @@ -283,7 +288,7 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
>  			src/event.h src/event.c \
>  			src/oob.h src/oob.c src/eir.h src/eir.c
>  src_bluetoothd_LDADD = lib/libbluetooth.la @GLIB_LIBS@ @DBUS_LIBS@ \
> -							@CAPNG_LIBS@ -ldl -lrt
> +							@CAPNG_LIBS@ @UDEV_LIBS@ -ldl -lrt
>  src_bluetoothd_LDFLAGS = -Wl,--export-dynamic \
>  				-Wl,--version-script=$(srcdir)/src/bluetooth.ver
>  
> @@ -398,7 +403,7 @@ EXTRA_DIST += doc/manager-api.txt \
>  
>  AM_YFLAGS = -d
>  
> -AM_CFLAGS = @DBUS_CFLAGS@ @GLIB_CFLAGS@ @CAPNG_CFLAGS@ \
> +AM_CFLAGS = @DBUS_CFLAGS@ @GLIB_CFLAGS@ @CAPNG_CFLAGS@ @UDEV_CFLAGS@ \
>  		-DBLUETOOTH_PLUGIN_BUILTIN -DPLUGINDIR=\""$(plugindir)"\"
>  
>  INCLUDES = -I$(builddir)/lib -I$(builddir)/src -I$(srcdir)/src \
> diff --git a/acinclude.m4 b/acinclude.m4
> index 3cb9459..6176483 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -186,6 +186,7 @@ AC_DEFUN([AC_ARG_BLUEZ], [
>  	sndfile_enable=${sndfile_found}
>  	hal_enable=no
>  	usb_enable=${usb_found}
> +	sixaxis_enable=${udev_found}
>  	alsa_enable=${alsa_found}
>  	gstreamer_enable=${gstreamer_found}
>  	audio_enable=yes
> @@ -286,6 +287,10 @@ AC_DEFUN([AC_ARG_BLUEZ], [
>  		usb_enable=${enableval}
>  	])
>  
> +	AC_ARG_ENABLE(sixaxis, AC_HELP_STRING([--enable-sixaxis], [enable Sixaxis plugin]), [
> +		sixaxis_enable=${enableval}
> +	])
> +
>  	AC_ARG_ENABLE(tracer, AC_HELP_STRING([--enable-tracer], [install Tracing daemon]), [
>  		tracer_enable=${enableval}
>  	])
> @@ -385,6 +390,10 @@ AC_DEFUN([AC_ARG_BLUEZ], [
>  		AC_DEFINE(HAVE_LIBUSB, 1, [Define to 1 if you have USB library.])
>  	fi
>  
> +	if (test "${sixaxis_enable}" = "yes" && test "${udev_found}" = "yes"); then
> +		AC_DEFINE(HAVE_SIXAXIS_PLUGIN, 1, [Define to 1 if you have sixaxis plugin.])
> +	fi
> +
>  	AM_CONDITIONAL(SNDFILE, test "${sndfile_enable}" = "yes" && test "${sndfile_found}" = "yes")
>  	AM_CONDITIONAL(USB, test "${usb_enable}" = "yes" && test "${usb_found}" = "yes")
>  	AM_CONDITIONAL(SBC, test "${alsa_enable}" = "yes" || test "${gstreamer_enable}" = "yes" ||
> @@ -421,4 +430,5 @@ AC_DEFUN([AC_ARG_BLUEZ], [
>  	AM_CONDITIONAL(DBUSOOBPLUGIN, test "${dbusoob_enable}" = "yes")
>  	AM_CONDITIONAL(WIIMOTEPLUGIN, test "${wiimote_enable}" = "yes")
>  	AM_CONDITIONAL(THERMOMETERPLUGIN, test "${thermometer_enable}" = "yes")
> +	AM_CONDITIONAL(SIXAXISPLUGIN, test "${sixaxis_enable}" = "yes" && test "${udev_found}" = "yes")
>  ])
> diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
> new file mode 100644
> index 0000000..2b0616a
> --- /dev/null
> +++ b/plugins/sixaxis.c
> @@ -0,0 +1,593 @@
> +/*
> + * sixaxis plugin: do cable association for Sixaxis controller
> + *
> + * Copyright (C) 2009  Bastien Nocera <hadess-0MeiytkfxGOsTnJN9+BGXg@public.gmane.org>
> + * Copyright (C) 2011  Antonio Ospite <ospite-aNJ+ML1ZbiP93QAQaVx+gl6hYfS7NtTn@public.gmane.org>
> + *
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +
> +/*
> + * In the following this terminology is used:
> + *
> + *  - controller: a Sixaxis joypad.
> + *  - adapter: the bluetooth dongle on the host system.
> + *  - adapter_bdaddr: the bdaddr of the bluetooth adapter.
> + *  - device_bdaddr: the bdaddr of the Sixaxis controller.
> + *  - master_bdaddr: the bdaddr of the adapter to be configured into the
> + *    Sixaxis controller
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <stdio.h>
> +#include <stdint.h>
> +#include <stdlib.h>
> +#include <sys/ioctl.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <glib.h>
> +#include <linux/hidraw.h>
> +
> +#define LIBUDEV_I_KNOW_THE_API_IS_SUBJECT_TO_CHANGE 1
> +#include <libudev.h>
> +
> +#include "plugin.h"
> +#include "log.h"
> +#include "adapter.h"
> +#include "device.h"
> +#include "manager.h"
> +#include "storage.h"
> +#include "sdp_lib.h"
> +
> +/* Fallback definitions to compile with older headers */
> +#ifndef HIDIOCGFEATURE
> +#define HIDIOCGFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x07, len)
> +#endif
> +
> +#ifndef HIDIOCSFEATURE
> +#define HIDIOCSFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x06, len)
> +#endif
> +
> +#define BDADDR_STR_SIZE 18 /* strlen("00:00:00:00:00:00") + 1 */
> +
> +/* Vendor and product ID for the Sixaxis PS3 controller */
> +#define VENDOR 0x054c
> +#define PRODUCT 0x0268
> +#define SIXAXIS_NAME "PLAYSTATION(R)3 Controller"
> +#define SIXAXIS_PNP_RECORD "3601920900000A000100000900013503191124090004350D35061901000900113503190011090006350909656E09006A0901000900093508350619112409010009000D350F350D350619010009001335031900110901002513576972656C65737320436F6E74726F6C6C65720901012513576972656C65737320436F6E74726F6C6C6572090102251B536F6E7920436F6D707574657220456E7465727461696E6D656E740902000901000902010901000902020800090203082109020428010902052801090206359A35980822259405010904A101A102850175089501150026FF00810375019513150025013500450105091901291381027501950D0600FF8103150026FF0005010901A10075089504350046FF0009300931093209358102C0050175089527090181027508953009019102750895300901B102C0A1028502750895300901B102C0A10285EE750895300901B102C0A10285EF750895300901B102C0C0090207350835060904090901000902082800090209280109020A2801090
 20B09010009020C093E8009020D280009020E2800"
> +#define HID_UUID "00001124-0000-1000-8000-00805f9b34fb"
> +
> +#define LED_1 (0x01 << 1)
> +#define LED_2 (0x01 << 2)
> +#define LED_3 (0x01 << 3)
> +#define LED_4 (0x01 << 4)
> +
> +#define LED_STATUS_OFF 0
> +#define LED_STATUS_ON  1
> +
> +static struct udev *ctx;
> +static struct udev_monitor *monitor;
> +static guint watch_id;
> +
> +
> +static int create_sixaxis_association(struct btd_adapter *adapter,
> +					const char *name,
> +					const char *address,
> +					guint32 vendor_id,
> +					guint32 product_id,
> +					const char *pnp_record)
> +{
> +	DBusConnection *conn;
> +	sdp_record_t *rec;
> +	struct btd_device *device;
> +	bdaddr_t src, dst;
> +	char srcaddr[18];
> +	int ret = 0;
> +
> +	str2ba(address, &dst);
> +	adapter_get_address(adapter, &src);
> +	ba2str(&src, srcaddr);
> +
> +	write_device_name(&dst, &src, (char *) name);
> +
> +	/* Store the device's SDP record */
> +	rec = record_from_string(pnp_record);
> +	store_record(srcaddr, address, rec);
> +	sdp_record_free(rec);
> +
> +	/* Set the device id */
> +	store_device_id(srcaddr, address, 0xffff, vendor_id, product_id, 0);
> +	/* Don't write a profile here,
> +	 * it will be updated when the device connects */
> +
> +	write_trust(srcaddr, address, "[all]", TRUE);
> +
> +	conn = dbus_bus_get(DBUS_BUS_SYSTEM, NULL);
> +	if (conn == NULL) {
> +		DBG("Failed to get on the bus");
> +		ret = -EPERM;
> +		goto fail_dbus;
> +	}
> +
> +	device = adapter_get_device(conn, adapter, address);
> +	if (device == NULL) {
> +		DBG("Failed to get the device");
> +		ret = -ENODEV;
> +		goto fail_device;
> +	}
> +
> +	device_set_temporary(device, FALSE);
> +	device_set_name(device, name);
> +	btd_device_add_uuid(device, HID_UUID);
> +
> +fail_device:
> +	dbus_connection_unref(conn);
> +fail_dbus:
> +	return ret;
> +}
> +
> +/* Usb cable pairing section */
> +static unsigned char *get_feature_report(int fd, uint8_t report_number,
> +						unsigned int len)
> +{
> +	unsigned char *buf;
> +	int ret;
> +
> +	buf = calloc(len, sizeof(*buf));
> +	if (buf == NULL) {
> +		error("%s:%s() calloc failed", __FILE__, __func__);
> +		return NULL;
> +	}
> +
> +	buf[0] = report_number;
> +
> +	ret = ioctl(fd, HIDIOCGFEATURE(len), buf);
> +	if (ret < 0) {
> +		error("%s:%s() HIDIOCGFEATURE ret = %d",
> +			__FILE__, __func__, ret);
> +		free(buf);
> +		return NULL;
> +	}
> +
> +	return buf;
> +}
> +
> +static int set_feature_report(int fd, uint8_t *report, int len)
> +{
> +	int ret;
> +
> +	ret = ioctl(fd, HIDIOCSFEATURE(len), report);
> +	if (ret < 0)
> +		error("%s:%s() HIDIOCSFEATURE failed, ret = %d",
> +			__FILE__, __func__, ret);
> +
> +	return ret;
> +}
> +
> +static char *get_device_bdaddr(int fd)
> +{
> +	unsigned char *buf;
> +	char *address;
> +
> +	buf = get_feature_report(fd, 0xf2, 18);
> +	if (buf == NULL) {
> +		error("%s:%s() cannot get feature report", __FILE__, __func__);
> +		return NULL;
> +	}
> +
> +	address = calloc(BDADDR_STR_SIZE, sizeof(*address));
> +	if (address == NULL) {
> +		error("%s:%s() calloc failed", __FILE__, __func__);
> +		free(buf);
> +		return NULL;
> +	}
> +
> +	snprintf(address, BDADDR_STR_SIZE,
> +			"%02X:%02X:%02X:%02X:%02X:%02X",
> +			buf[4], buf[5], buf[6], buf[7], buf[8], buf[9]);
> +
> +	free(buf);
> +	return address;
> +}
> +
> +static char *get_master_bdaddr(int fd)
> +{
> +	unsigned char *buf;
> +	char *address;
> +
> +	buf = get_feature_report(fd, 0xf5, 8);
> +	if (buf == NULL) {
> +		error("%s:%s() cannot get feature report", __FILE__, __func__);
> +		return NULL;
> +	}
> +
> +	address = calloc(BDADDR_STR_SIZE, sizeof(*address));
> +	if (address == NULL) {
> +		error("%s:%s() calloc failed", __FILE__, __func__);
> +		free(buf);
> +		return NULL;
> +	}
> +
> +	snprintf(address, BDADDR_STR_SIZE,
> +			"%02X:%02X:%02X:%02X:%02X:%02X",
> +			buf[2], buf[3], buf[4], buf[5], buf[6], buf[7]);
> +
> +	free(buf);
> +	return address;
> +}
> +
> +static int set_master_bdaddr(int fd, char *adapter_bdaddr)
> +{
> +	uint8_t *report;
> +	uint8_t addr[6];
> +	int ret;
> +
> +	ret = sscanf(adapter_bdaddr,
> +			"%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx",
> +			&addr[0], &addr[1], &addr[2],
> +			&addr[3], &addr[4], &addr[5]);
> +	if (ret != 6) {
> +
> +		error("%s:%s() Parsing the bt address failed",
> +			__FILE__, __func__);
> +		return -EINVAL;
> +	}
> +
> +	report = malloc(8);
> +	if (report == NULL) {
> +		error("%s:%s() malloc failed", __FILE__, __func__);
> +		return -ENOMEM;
> +	}
> +
> +	report[0] = 0xf5;
> +	report[1] = 0x01;
> +
> +	report[2] = addr[0];
> +	report[3] = addr[1];
> +	report[4] = addr[2];
> +	report[5] = addr[3];
> +	report[6] = addr[4];
> +	report[7] = addr[5];
> +
> +	ret = set_feature_report(fd, report, 8);
> +	if (ret < 0) {
> +		error("%s:%s() cannot set feature report",
> +			__FILE__, __func__);
> +		goto out;
> +	}
> +
> +	DBG("New Master Bluetooth address: %s", adapter_bdaddr);
> +
> +out:
> +	free(report);
> +	return ret;
> +}
> +
> +static int sixpair(int fd, struct btd_adapter *adapter)
> +{
> +	char *device_bdaddr;
> +	char *master_bdaddr;
> +	char adapter_bdaddr[18];
> +	bdaddr_t dst;
> +	int ret = 0;
> +
> +	adapter_get_address(adapter, &dst);
> +	ba2str(&dst, adapter_bdaddr);
> +	DBG("Adapter bdaddr %s", adapter_bdaddr);
> +
> +	master_bdaddr = get_master_bdaddr(fd);
> +	if (master_bdaddr == NULL) {
> +		DBG("Failed to get the Old master Bluetooth address from the device");
> +		return -EPERM;
> +	}
> +
> +	/* Only set the master bdaddr when needed, this is how the PS3 does
> +	 * it, perhaps to avoid unnecessary writes to some eeprom.
> +	 */
> +	if (g_strcmp0(master_bdaddr, adapter_bdaddr) != 0) {
> +		DBG("Old master Bluetooth address was: %s", master_bdaddr);
> +		ret = set_master_bdaddr(fd, adapter_bdaddr);
> +		if (ret < 0) {
> +			DBG("Failed to set the master Bluetooth address");
> +			free(master_bdaddr);
> +			return ret;
> +		}
> +	}
> +
> +	device_bdaddr = get_device_bdaddr(fd);
> +	if (device_bdaddr == NULL) {
> +		DBG("Failed to get the Bluetooth address from the device");
> +		free(master_bdaddr);
> +		return -EPERM;
> +	}
> +
> +	DBG("Device bdaddr %s", device_bdaddr);
> +
> +	ret = create_sixaxis_association(adapter,
> +					SIXAXIS_NAME,
> +					device_bdaddr,
> +					VENDOR, PRODUCT, SIXAXIS_PNP_RECORD);

You create the association with a hard-coded VID/PID. You match the
device on string name below (not VID/PID), so what if you used the
actual VID/PID of the device for the association instead?


> +	free(device_bdaddr);
> +	free(master_bdaddr);
> +	return ret;
> +}
> +
> +/* Led setting section */
> +static int set_leds(int fd, unsigned char leds_status[4])
> +{
> +	int ret;
> +
> +	/*
> +	 * the total time the led is active (0xff means forever)
> +	 * |     duty_length: how long a cycle is in deciseconds:
> +	 * |     |                              (0 means "blink very fast")
> +	 * |     |     ??? (Maybe a phase shift or duty_length multiplier?)
> +	 * |     |     |     % of duty_length led is off (0xff means 100%)
> +	 * |     |     |     |     % of duty_length led is on (0xff is 100%)
> +	 * |     |     |     |     |
> +	 * 0xff, 0x27, 0x10, 0x00, 0x32,
> +	 */
> +	unsigned char leds_report[] = {
> +		0x01,
> +		0x00, 0x00, 0x00, 0x00, 0x00, /* rumble values TBD */
> +		0x00, 0x00, 0x00, 0x00, 0x1e, /* LED_1=0x02, LED_2=0x04 ... */
> +		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_4 */
> +		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_3 */
> +		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_2 */
> +		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_1 */
> +		0x00, 0x00, 0x00, 0x00, 0x00,
> +	};
> +
> +	int leds = 0;
> +	if (leds_status[0])
> +		leds |= LED_1;
> +	if (leds_status[1])
> +		leds |= LED_2;
> +	if (leds_status[2])
> +		leds |= LED_3;
> +	if (leds_status[3])
> +		leds |= LED_4;
> +
> +	leds_report[10] = leds;

If you are overwriting leds_report[10] unconditionally, why is it
initialized to 0x1e in the leds_report initializer (unless I counted wrong)?




> +
> +	ret = write(fd, leds_report, sizeof(leds_report));
> +	if (ret < (ssize_t) sizeof(leds_report))
> +		error("%s:%s() Unable to write to hidraw device",
> +			__FILE__, __func__);
> +
> +	return ret;
> +}
> +
> +static int set_controller_number(int fd, unsigned int n)
> +{
> +	unsigned char leds_status[4] = {0, 0, 0, 0};
> +
> +	switch (n) {
> +	case 0:
> +		break;
> +	case 1:
> +	case 2:
> +	case 3:
> +	case 4:
> +		leds_status[n - 1] = LED_STATUS_ON;
> +		break;
> +	case 5:
> +	case 6:
> +	case 7:
> +		leds_status[4 - 1] = LED_STATUS_ON;
> +		leds_status[n - 4 - 1] = LED_STATUS_ON;
> +		break;
> +	default:
> +		error("%s:%s() Only 7 controllers supported for now",
> +			__FILE__, __func__);
> +		return -1;
> +	}
> +
> +	return set_leds(fd, leds_status);
> +}
> +
> +
> +static inline gboolean is_sixaxis(const char *hid_name)
> +{
> +	return g_str_has_suffix(hid_name, "PLAYSTATION(R)3 Controller") ||
> +		g_str_has_suffix(hid_name,
> +			"Sony Computer Entertainment Wireless Controller");
> +}

Why do you key on the string name here. This seems problematic,
especially if you want to support things like the PSMove. I'd expect a
table of VID/PIDs or something.

> +
> +static void handle_device_plug(struct udev_device *udevice)
> +{
> +	struct udev_device *hid_parent;
> +	struct udev_enumerate *enumerate;
> +	struct udev_list_entry *devices, *dev_list_entry;
> +	const char *hid_name;
> +	const char *hid_phys;
> +	const char *hidraw_node;
> +	unsigned char is_usb = FALSE;
> +	int js_num = 0;
> +	int fd;
> +
> +	hid_parent = udev_device_get_parent_with_subsystem_devtype(udevice,
> +								"hid", NULL);
> +	if (!hid_parent) {
> +		error("%s:%s() cannot get parent hid device",
> +			__FILE__, __func__);
> +		return;
> +	}
> +
> +	hid_name = udev_device_get_property_value(hid_parent, "HID_NAME");
> +	DBG("name: %s", hid_name);
> +
> +	if (!is_sixaxis(hid_name))
> +		return;
> +
> +	DBG("Found a Sixaxis device");
> +
> +	hidraw_node = udev_device_get_devnode(udevice);
> +
> +	hid_phys = udev_device_get_property_value(hid_parent, "HID_PHYS");
> +
> +	/* looking for joysticks */
> +	enumerate = udev_enumerate_new(udev_device_get_udev(udevice));
> +	udev_enumerate_add_match_sysname(enumerate, "js*");
> +	udev_enumerate_scan_devices(enumerate);
> +
> +	devices = udev_enumerate_get_list_entry(enumerate);
> +	udev_list_entry_foreach(dev_list_entry, devices) {
> +		const char *devname;
> +		struct udev_device *js_dev;
> +		struct udev_device *input_parent;
> +		const char *input_phys;
> +
> +		devname = udev_list_entry_get_name(dev_list_entry);
> +		js_dev = udev_device_new_from_syspath(udev_device_get_udev(udevice),
> +							devname);
> +
> +		input_parent = udev_device_get_parent_with_subsystem_devtype(js_dev,
> +							"input", NULL);
> +		if (!input_parent) {
> +			error("%s:%s() cannot get parent input device.",
> +				__FILE__, __func__);
> +			continue;
> +		}
> +
> +		/* check this is the joystick relative to
> +		 * the hidraw device above */
> +		input_phys = udev_device_get_sysattr_value(input_parent,
> +								"phys");
> +		if (g_strcmp0(input_phys, hid_phys) == 0) {
> +			const char *usb_driver;
> +
> +			usb_driver = udev_device_get_property_value(js_dev,
> +							"ID_USB_DRIVER");
> +			js_num = atoi(udev_device_get_sysnum(js_dev)) + 1;
> +			DBG("joypad device_num: %d", js_num);
> +			DBG("hidraw_node: %s", hidraw_node);
> +			DBG("driver: %s", usb_driver);
> +
> +			if (g_strcmp0(usb_driver, "usbhid") == 0)
> +				is_usb = TRUE;
> +		}
> +
> +		udev_device_unref(js_dev);
> +	}
> +
> +	udev_enumerate_unref(enumerate);
> +
> +	fd = open(hidraw_node, O_RDWR);
> +	if (fd < 0) {
> +		error("%s:%s() hidraw open", __FILE__, __func__);
> +		return;
> +	}
> +
> +	if (is_usb) {
> +		struct btd_adapter *adapter;
> +
> +		/* Look for the default adapter */
> +		adapter = manager_get_default_adapter();
> +		if (adapter == NULL) {
> +			DBG("No adapters, exiting");
> +			return;
> +		}
> +		sixpair(fd, adapter);
> +	}
> +
> +	if (js_num > 0)
> +		set_controller_number(fd, js_num);
> +
> +	close(fd);
> +}
> +
> +static gboolean device_event_idle(struct udev_device *udevice)
> +{
> +	handle_device_plug(udevice);
> +	udev_device_unref(udevice);
> +	return FALSE;
> +}

I'm a little confused why this is called "idle."


Thanks for your work on this, Antonio. Linux working out of the box with
these controllers will be really cool.

Alan.

Re: [PATCH BlueZ 2/4] Add sixaxis plugin: USB pairing and LEDs settings

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 4472 bytes --]

On Tue, 09 Aug 2011 22:24:37 -0400
Alan Ott <alan@signal11.us> wrote:

[...]
> > +	ret = create_sixaxis_association(adapter,
> > +					SIXAXIS_NAME,
> > +					device_bdaddr,
> > +					VENDOR, PRODUCT, SIXAXIS_PNP_RECORD);
> 
> You create the association with a hard-coded VID/PID. You match the
> device on string name below (not VID/PID), so what if you used the
> actual VID/PID of the device for the association instead?
>

Or use statically defined VID/PID to match the device, see below.

[...]
> > +	/*
> > +	 * the total time the led is active (0xff means forever)
> > +	 * |     duty_length: how long a cycle is in deciseconds:
> > +	 * |     |                              (0 means "blink very fast")
> > +	 * |     |     ??? (Maybe a phase shift or duty_length multiplier?)
> > +	 * |     |     |     % of duty_length led is off (0xff means 100%)
> > +	 * |     |     |     |     % of duty_length led is on (0xff is 100%)
> > +	 * |     |     |     |     |
> > +	 * 0xff, 0x27, 0x10, 0x00, 0x32,
> > +	 */
> > +	unsigned char leds_report[] = {
> > +		0x01,
> > +		0x00, 0x00, 0x00, 0x00, 0x00, /* rumble values TBD */
> > +		0x00, 0x00, 0x00, 0x00, 0x1e, /* LED_1=0x02, LED_2=0x04 ... */
> > +		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_4 */
> > +		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_3 */
> > +		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_2 */
> > +		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_1 */
> > +		0x00, 0x00, 0x00, 0x00, 0x00,
> > +	};
> >  
> > +	int leds = 0;
> > +	if (leds_status[0])
> > +		leds |= LED_1;
> > +	if (leds_status[1])
> > +		leds |= LED_2;
> > +	if (leds_status[2])
> > +		leds |= LED_3;
> > +	if (leds_status[3])
> > +		leds |= LED_4;
> > +
> > +	leds_report[10] = leds;
> 
> If you are overwriting leds_report[10] unconditionally, why is it
> initialized to 0x1e in the leds_report initializer (unless I counted wrong)?
>

You are right having it initialized to 0x1e is not necessary at all,
but I'd still keep it to 0x1e in the initializer because this is the
"hardware default", meaning "all leds blinking", I see it as a form of
documentation-inside-code, but if it is confusing I will avoid that. 

[...]
> > +
> > +static inline gboolean is_sixaxis(const char *hid_name)
> > +{
> > +	return g_str_has_suffix(hid_name, "PLAYSTATION(R)3 Controller") ||
> > +		g_str_has_suffix(hid_name,
> > +			"Sony Computer Entertainment Wireless Controller");
> > +}
> 
> Why do you key on the string name here. This seems problematic,
> especially if you want to support things like the PSMove. I'd expect a
> table of VID/PIDs or something.
>

Right, I like this suggestion, I generalized the code a little bit to
handle different devices, see the two follow-up incremental patches, in
the first one I remove static #defines in favor of a structure
representing a device type, in the second one I match devices using
VID/PID from a table of device types.

If the idea looks OK I'll fold those patches in v5, along with the
plugin renaming to "sony-controller", and maybe I'll split the plugin in
several modules too:
  sony-controller.c 	/* udev and bluez stuff */
  sony-controller-hid.c	/* hidraw and device specific stuff */
  sony-controlled-hid.h

[...]
> > +
> > +static gboolean device_event_idle(struct udev_device *udevice)
> > +{
> > +	handle_device_plug(udevice);
> > +	udev_device_unref(udevice);
> > +	return FALSE;
> > +}
> 
> I'm a little confused why this is called "idle."
>

I don't know either, maybe trying to communicate that it is called
asynchronously by glib (via g_timeout_add_seconds())? But even if so
this is not a property of the function itself, so I think we can just
call it device_event().

Ah Bastien, I noticed we are using g_timeout_add_seconds() with the
callback always returning FALSE, so the latter is called only once,
can't we in this case just sleep and then call the function directly?
Isn't the main loop in sixaxis_init() enough to handle multiple
overlapping udev events? Maybe I asked that already... :P

> Thanks for your work on this, Antonio. Linux working out of the box with
> these controllers will be really cool.
> 
> Alan.
>

Regards,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH BlueZ 2/4] Add sixaxis plugin: USB pairing and LEDs settings

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 4500 bytes --]

On Tue, 09 Aug 2011 22:24:37 -0400
Alan Ott <alan-yzvJWuRpmD1zbRFIqnYvSA@public.gmane.org> wrote:

[...]
> > +	ret = create_sixaxis_association(adapter,
> > +					SIXAXIS_NAME,
> > +					device_bdaddr,
> > +					VENDOR, PRODUCT, SIXAXIS_PNP_RECORD);
> 
> You create the association with a hard-coded VID/PID. You match the
> device on string name below (not VID/PID), so what if you used the
> actual VID/PID of the device for the association instead?
>

Or use statically defined VID/PID to match the device, see below.

[...]
> > +	/*
> > +	 * the total time the led is active (0xff means forever)
> > +	 * |     duty_length: how long a cycle is in deciseconds:
> > +	 * |     |                              (0 means "blink very fast")
> > +	 * |     |     ??? (Maybe a phase shift or duty_length multiplier?)
> > +	 * |     |     |     % of duty_length led is off (0xff means 100%)
> > +	 * |     |     |     |     % of duty_length led is on (0xff is 100%)
> > +	 * |     |     |     |     |
> > +	 * 0xff, 0x27, 0x10, 0x00, 0x32,
> > +	 */
> > +	unsigned char leds_report[] = {
> > +		0x01,
> > +		0x00, 0x00, 0x00, 0x00, 0x00, /* rumble values TBD */
> > +		0x00, 0x00, 0x00, 0x00, 0x1e, /* LED_1=0x02, LED_2=0x04 ... */
> > +		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_4 */
> > +		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_3 */
> > +		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_2 */
> > +		0xff, 0x27, 0x10, 0x00, 0x32, /* LED_1 */
> > +		0x00, 0x00, 0x00, 0x00, 0x00,
> > +	};
> >  
> > +	int leds = 0;
> > +	if (leds_status[0])
> > +		leds |= LED_1;
> > +	if (leds_status[1])
> > +		leds |= LED_2;
> > +	if (leds_status[2])
> > +		leds |= LED_3;
> > +	if (leds_status[3])
> > +		leds |= LED_4;
> > +
> > +	leds_report[10] = leds;
> 
> If you are overwriting leds_report[10] unconditionally, why is it
> initialized to 0x1e in the leds_report initializer (unless I counted wrong)?
>

You are right having it initialized to 0x1e is not necessary at all,
but I'd still keep it to 0x1e in the initializer because this is the
"hardware default", meaning "all leds blinking", I see it as a form of
documentation-inside-code, but if it is confusing I will avoid that. 

[...]
> > +
> > +static inline gboolean is_sixaxis(const char *hid_name)
> > +{
> > +	return g_str_has_suffix(hid_name, "PLAYSTATION(R)3 Controller") ||
> > +		g_str_has_suffix(hid_name,
> > +			"Sony Computer Entertainment Wireless Controller");
> > +}
> 
> Why do you key on the string name here. This seems problematic,
> especially if you want to support things like the PSMove. I'd expect a
> table of VID/PIDs or something.
>

Right, I like this suggestion, I generalized the code a little bit to
handle different devices, see the two follow-up incremental patches, in
the first one I remove static #defines in favor of a structure
representing a device type, in the second one I match devices using
VID/PID from a table of device types.

If the idea looks OK I'll fold those patches in v5, along with the
plugin renaming to "sony-controller", and maybe I'll split the plugin in
several modules too:
  sony-controller.c 	/* udev and bluez stuff */
  sony-controller-hid.c	/* hidraw and device specific stuff */
  sony-controlled-hid.h

[...]
> > +
> > +static gboolean device_event_idle(struct udev_device *udevice)
> > +{
> > +	handle_device_plug(udevice);
> > +	udev_device_unref(udevice);
> > +	return FALSE;
> > +}
> 
> I'm a little confused why this is called "idle."
>

I don't know either, maybe trying to communicate that it is called
asynchronously by glib (via g_timeout_add_seconds())? But even if so
this is not a property of the function itself, so I think we can just
call it device_event().

Ah Bastien, I noticed we are using g_timeout_add_seconds() with the
callback always returning FALSE, so the latter is called only once,
can't we in this case just sleep and then call the function directly?
Isn't the main loop in sixaxis_init() enough to handle multiple
overlapping udev events? Maybe I asked that already... :P

> Thanks for your work on this, Antonio. Linux working out of the box with
> these controllers will be really cool.
> 
> Alan.
>

Regards,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH 0/4 incremental 1/2] Generalize controller handling to support different devices

[Antonio Ospite]

Remove hardcoded #defines and put the values in a struct so we can handle 
different device types.

>From the USB dumps I've seen[1], different devices have just different ways to 
get and set the bdaddrs, the pairing algorithm is the same.

[1] http://ps3.jim.sh/sixaxis/dumps/ 

---
 plugins/sixaxis.c |   80 ++++++++++++++++++++++++++++++++++------------------
 1 files changed, 52 insertions(+), 28 deletions(-)

diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
index e08222c..608474f 100644
--- a/plugins/sixaxis.c
+++ b/plugins/sixaxis.c
@@ -70,12 +70,38 @@
 
 #define BDADDR_STR_SIZE 18 /* strlen("00:00:00:00:00:00") + 1 */
 
-/* Vendor and product ID for the Sixaxis PS3 controller */
-#define VENDOR 0x054c
-#define PRODUCT 0x0268
-#define SIXAXIS_NAME "PLAYSTATION(R)3 Controller"
-#define SIXAXIS_PNP_RECORD "3601920900000A000100000900013503191124090004350D35061901000900113503190011090006350909656E09006A0901000900093508350619112409010009000D350F350D350619010009001335031900110901002513576972656C65737320436F6E74726F6C6C65720901012513576972656C65737320436F6E74726F6C6C6572090102251B536F6E7920436F6D707574657220456E7465727461696E6D656E740902000901000902010901000902020800090203082109020428010902052801090206359A35980822259405010904A101A102850175089501150026FF00810375019513150025013500450105091901291381027501950D0600FF8103150026FF0005010901A10075089504350046FF0009300931093209358102C0050175089527090181027508953009019102750895300901B102C0A1028502750895300901B102C0A10285EE750895300901B102C0A10285EF750895300901B102C0C0090207350835060904090901000902082800090209280109020A280109020B09010009020C093E8009020D280009020E2800"
-#define HID_UUID "00001124-0000-1000-8000-00805f9b34fb"
+struct sony_controller {
+	uint16_t vendor_id;
+	uint16_t product_id;
+	char *name;
+	char *pnp_record;
+	char *hid_uuid;
+
+	/* device specific callbacks to get master/device bdaddr and set
+	 * master bdaddr
+	 */
+	char * (*get_device_bdaddr)(int);
+	char * (*get_master_bdaddr)(int);
+	int (*set_master_bdaddr) (int, char *);
+};
+
+static char *sixaxis_get_device_bdaddr(int fd);
+static char *sixaxis_get_master_bdaddr(int fd);
+static int sixaxis_set_master_bdaddr(int fd, char *adapter_bdaddr);
+
+static struct sony_controller controllers[] = {
+	{
+		.vendor_id = 0x054c,
+		.product_id = 0x0268,
+		.name = "PLAYSTATION(R)3 Controller",
+		.pnp_record = "3601920900000A000100000900013503191124090004350D35061901000900113503190011090006350909656E09006A0901000900093508350619112409010009000D350F350D350619010009001335031900110901002513576972656C65737320436F6E74726F6C6C65720901012513576972656C65737320436F6E74726F6C6C6572090102251B536F6E7920436F6D707574657220456E7465727461696E6D656E740902000901000902010901000902020800090203082109020428010902052801090206359A35980822259405010904A101A102850175089501150026FF00810375019513150025013500450105091901291381027501950D0600FF8103150026FF0005010901A10075089504350046FF0009300931093209358102C0050175089527090181027508953009019102750895300901B102C0A1028502750895300901B102C0A10285EE750895300901B102C0A10285EF750895300901B102C0C0090207350835060904090901000902082800090209280109020A280109020B09010009020C093E8009020D280009020E2800",
+		.hid_uuid = "00001124-0000-1000-8000-00805f9b34fb",
+		.get_device_bdaddr = sixaxis_get_device_bdaddr,
+		.get_master_bdaddr = sixaxis_get_master_bdaddr,
+		.set_master_bdaddr = sixaxis_set_master_bdaddr,
+	},
+};
+
 
 #define LED_1 (0x01 << 1)
 #define LED_2 (0x01 << 2)
@@ -90,12 +116,9 @@ static struct udev_monitor *monitor;
 static guint watch_id;
 
 
-static int create_sixaxis_association(struct btd_adapter *adapter,
-					const char *name,
+static int create_controller_association(struct btd_adapter *adapter,
 					const char *address,
-					guint32 vendor_id,
-					guint32 product_id,
-					const char *pnp_record)
+					struct sony_controller *controller)
 {
 	DBusConnection *conn;
 	sdp_record_t *rec;
@@ -108,15 +131,16 @@ static int create_sixaxis_association(struct btd_adapter *adapter,
 	adapter_get_address(adapter, &src);
 	ba2str(&src, srcaddr);
 
-	write_device_name(&dst, &src, (char *) name);
+	write_device_name(&dst, &src, controller->name);
 
 	/* Store the device's SDP record */
-	rec = record_from_string(pnp_record);
+	rec = record_from_string(controller->pnp_record);
 	store_record(srcaddr, address, rec);
 	sdp_record_free(rec);
 
 	/* Set the device id */
-	store_device_id(srcaddr, address, 0xffff, vendor_id, product_id, 0);
+	store_device_id(srcaddr, address, 0xffff, controller->vendor_id,
+			controller->product_id, 0);
 	/* Don't write a profile here,
 	 * it will be updated when the device connects */
 
@@ -137,8 +161,8 @@ static int create_sixaxis_association(struct btd_adapter *adapter,
 	}
 
 	device_set_temporary(device, FALSE);
-	device_set_name(device, name);
-	btd_device_add_uuid(device, HID_UUID);
+	device_set_name(device, controller->name);
+	btd_device_add_uuid(device, controller->hid_uuid);
 
 fail_device:
 	dbus_connection_unref(conn);
@@ -184,7 +208,7 @@ static int set_feature_report(int fd, uint8_t *report, int len)
 	return ret;
 }
 
-static char *get_device_bdaddr(int fd)
+static char *sixaxis_get_device_bdaddr(int fd)
 {
 	unsigned char *buf;
 	char *address;
@@ -210,7 +234,7 @@ static char *get_device_bdaddr(int fd)
 	return address;
 }
 
-static char *get_master_bdaddr(int fd)
+static char *sixaxis_get_master_bdaddr(int fd)
 {
 	unsigned char *buf;
 	char *address;
@@ -236,7 +260,7 @@ static char *get_master_bdaddr(int fd)
 	return address;
 }
 
-static int set_master_bdaddr(int fd, char *adapter_bdaddr)
+static int sixaxis_set_master_bdaddr(int fd, char *adapter_bdaddr)
 {
 	uint8_t *report;
 	uint8_t addr[6];
@@ -282,7 +306,7 @@ out:
 	return ret;
 }
 
-static int sixpair(int fd, struct btd_adapter *adapter)
+static int controller_pair(int fd, struct btd_adapter *adapter, struct sony_controller *controller)
 {
 	char *device_bdaddr;
 	char *master_bdaddr;
@@ -294,7 +318,7 @@ static int sixpair(int fd, struct btd_adapter *adapter)
 	ba2str(&dst, adapter_bdaddr);
 	DBG("Adapter bdaddr %s", adapter_bdaddr);
 
-	master_bdaddr = get_master_bdaddr(fd);
+	master_bdaddr = controller->get_master_bdaddr(fd);
 	if (master_bdaddr == NULL) {
 		DBG("Failed to get the Old master Bluetooth address from the device");
 		return -EPERM;
@@ -305,7 +329,7 @@ static int sixpair(int fd, struct btd_adapter *adapter)
 	 */
 	if (g_strcmp0(master_bdaddr, adapter_bdaddr) != 0) {
 		DBG("Old master Bluetooth address was: %s", master_bdaddr);
-		ret = set_master_bdaddr(fd, adapter_bdaddr);
+		ret = controller->set_master_bdaddr(fd, adapter_bdaddr);
 		if (ret < 0) {
 			DBG("Failed to set the master Bluetooth address");
 			free(master_bdaddr);
@@ -313,7 +337,7 @@ static int sixpair(int fd, struct btd_adapter *adapter)
 		}
 	}
 
-	device_bdaddr = get_device_bdaddr(fd);
+	device_bdaddr = controller->get_device_bdaddr(fd);
 	if (device_bdaddr == NULL) {
 		DBG("Failed to get the Bluetooth address from the device");
 		free(master_bdaddr);
@@ -322,10 +346,7 @@ static int sixpair(int fd, struct btd_adapter *adapter)
 
 	DBG("Device bdaddr %s", device_bdaddr);
 
-	ret = create_sixaxis_association(adapter,
-					SIXAXIS_NAME,
-					device_bdaddr,
-					VENDOR, PRODUCT, SIXAXIS_PNP_RECORD);
+	ret = create_controller_association(adapter, device_bdaddr, controller);
 	free(device_bdaddr);
 	free(master_bdaddr);
 	return ret;
@@ -424,6 +445,7 @@ static void handle_device_plug(struct udev_device *udevice)
 	unsigned char is_usb = FALSE;
 	int js_num = 0;
 	int fd;
+	struct sony_controller *controller;
 
 	hid_parent = udev_device_get_parent_with_subsystem_devtype(udevice,
 								"hid", NULL);
@@ -439,6 +461,8 @@ static void handle_device_plug(struct udev_device *udevice)
 	if (!is_sixaxis(hid_name))
 		return;
 
+	controller = &controllers[0];
+
 	DBG("Found a Sixaxis device");
 
 	hidraw_node = udev_device_get_devnode(udevice);
@@ -507,7 +531,7 @@ static void handle_device_plug(struct udev_device *udevice)
 			DBG("No adapters, exiting");
 			return;
 		}
-		sixpair(fd, adapter);
+		controller_pair(fd, adapter, controller);
 	}
 
 	if (js_num > 0) {
-- 
1.7.5.4

[PATCH 0/4 incremental 1/2] Generalize controller handling to support different devices

[Antonio Ospite]

Remove hardcoded #defines and put the values in a struct so we can handle 
different device types.

>From the USB dumps I've seen[1], different devices have just different ways to 
get and set the bdaddrs, the pairing algorithm is the same.

[1] http://ps3.jim.sh/sixaxis/dumps/ 

---
 plugins/sixaxis.c |   80 ++++++++++++++++++++++++++++++++++------------------
 1 files changed, 52 insertions(+), 28 deletions(-)

diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
index e08222c..608474f 100644
--- a/plugins/sixaxis.c
+++ b/plugins/sixaxis.c
@@ -70,12 +70,38 @@
 
 #define BDADDR_STR_SIZE 18 /* strlen("00:00:00:00:00:00") + 1 */
 
-/* Vendor and product ID for the Sixaxis PS3 controller */
-#define VENDOR 0x054c
-#define PRODUCT 0x0268
-#define SIXAXIS_NAME "PLAYSTATION(R)3 Controller"
-#define SIXAXIS_PNP_RECORD "3601920900000A000100000900013503191124090004350D35061901000900113503190011090006350909656E09006A0901000900093508350619112409010009000D350F350D350619010009001335031900110901002513576972656C65737320436F6E74726F6C6C65720901012513576972656C65737320436F6E74726F6C6C6572090102251B536F6E7920436F6D707574657220456E7465727461696E6D656E740902000901000902010901000902020800090203082109020428010902052801090206359A35980822259405010904A101A102850175089501150026FF00810375019513150025013500450105091901291381027501950D0600FF8103150026FF0005010901A10075089504350046FF0009300931093209358102C0050175089527090181027508953009019102750895300901B102C0A1028502750895300901B102C0A10285EE750895300901B102C0A10285EF750895300901B102C0C0090207350835060904090901000902082800090209280109020A280109020
 B09010009020C093E8009020D280009020E2800"
-#define HID_UUID "00001124-0000-1000-8000-00805f9b34fb"
+struct sony_controller {
+	uint16_t vendor_id;
+	uint16_t product_id;
+	char *name;
+	char *pnp_record;
+	char *hid_uuid;
+
+	/* device specific callbacks to get master/device bdaddr and set
+	 * master bdaddr
+	 */
+	char * (*get_device_bdaddr)(int);
+	char * (*get_master_bdaddr)(int);
+	int (*set_master_bdaddr) (int, char *);
+};
+
+static char *sixaxis_get_device_bdaddr(int fd);
+static char *sixaxis_get_master_bdaddr(int fd);
+static int sixaxis_set_master_bdaddr(int fd, char *adapter_bdaddr);
+
+static struct sony_controller controllers[] = {
+	{
+		.vendor_id = 0x054c,
+		.product_id = 0x0268,
+		.name = "PLAYSTATION(R)3 Controller",
+		.pnp_record = "3601920900000A000100000900013503191124090004350D35061901000900113503190011090006350909656E09006A0901000900093508350619112409010009000D350F350D350619010009001335031900110901002513576972656C65737320436F6E74726F6C6C65720901012513576972656C65737320436F6E74726F6C6C6572090102251B536F6E7920436F6D707574657220456E7465727461696E6D656E740902000901000902010901000902020800090203082109020428010902052801090206359A35980822259405010904A101A102850175089501150026FF00810375019513150025013500450105091901291381027501950D0600FF8103150026FF0005010901A10075089504350046FF0009300931093209358102C0050175089527090181027508953009019102750895300901B102C0A1028502750895300901B102C0A10285EE750895300901B102C0A10285EF750895300901B102C0C0090207350835060904090901000902082800090209280109020A280109020B0901000902
 0C093E8009020D280009020E2800",
+		.hid_uuid = "00001124-0000-1000-8000-00805f9b34fb",
+		.get_device_bdaddr = sixaxis_get_device_bdaddr,
+		.get_master_bdaddr = sixaxis_get_master_bdaddr,
+		.set_master_bdaddr = sixaxis_set_master_bdaddr,
+	},
+};
+
 
 #define LED_1 (0x01 << 1)
 #define LED_2 (0x01 << 2)
@@ -90,12 +116,9 @@ static struct udev_monitor *monitor;
 static guint watch_id;
 
 
-static int create_sixaxis_association(struct btd_adapter *adapter,
-					const char *name,
+static int create_controller_association(struct btd_adapter *adapter,
 					const char *address,
-					guint32 vendor_id,
-					guint32 product_id,
-					const char *pnp_record)
+					struct sony_controller *controller)
 {
 	DBusConnection *conn;
 	sdp_record_t *rec;
@@ -108,15 +131,16 @@ static int create_sixaxis_association(struct btd_adapter *adapter,
 	adapter_get_address(adapter, &src);
 	ba2str(&src, srcaddr);
 
-	write_device_name(&dst, &src, (char *) name);
+	write_device_name(&dst, &src, controller->name);
 
 	/* Store the device's SDP record */
-	rec = record_from_string(pnp_record);
+	rec = record_from_string(controller->pnp_record);
 	store_record(srcaddr, address, rec);
 	sdp_record_free(rec);
 
 	/* Set the device id */
-	store_device_id(srcaddr, address, 0xffff, vendor_id, product_id, 0);
+	store_device_id(srcaddr, address, 0xffff, controller->vendor_id,
+			controller->product_id, 0);
 	/* Don't write a profile here,
 	 * it will be updated when the device connects */
 
@@ -137,8 +161,8 @@ static int create_sixaxis_association(struct btd_adapter *adapter,
 	}
 
 	device_set_temporary(device, FALSE);
-	device_set_name(device, name);
-	btd_device_add_uuid(device, HID_UUID);
+	device_set_name(device, controller->name);
+	btd_device_add_uuid(device, controller->hid_uuid);
 
 fail_device:
 	dbus_connection_unref(conn);
@@ -184,7 +208,7 @@ static int set_feature_report(int fd, uint8_t *report, int len)
 	return ret;
 }
 
-static char *get_device_bdaddr(int fd)
+static char *sixaxis_get_device_bdaddr(int fd)
 {
 	unsigned char *buf;
 	char *address;
@@ -210,7 +234,7 @@ static char *get_device_bdaddr(int fd)
 	return address;
 }
 
-static char *get_master_bdaddr(int fd)
+static char *sixaxis_get_master_bdaddr(int fd)
 {
 	unsigned char *buf;
 	char *address;
@@ -236,7 +260,7 @@ static char *get_master_bdaddr(int fd)
 	return address;
 }
 
-static int set_master_bdaddr(int fd, char *adapter_bdaddr)
+static int sixaxis_set_master_bdaddr(int fd, char *adapter_bdaddr)
 {
 	uint8_t *report;
 	uint8_t addr[6];
@@ -282,7 +306,7 @@ out:
 	return ret;
 }
 
-static int sixpair(int fd, struct btd_adapter *adapter)
+static int controller_pair(int fd, struct btd_adapter *adapter, struct sony_controller *controller)
 {
 	char *device_bdaddr;
 	char *master_bdaddr;
@@ -294,7 +318,7 @@ static int sixpair(int fd, struct btd_adapter *adapter)
 	ba2str(&dst, adapter_bdaddr);
 	DBG("Adapter bdaddr %s", adapter_bdaddr);
 
-	master_bdaddr = get_master_bdaddr(fd);
+	master_bdaddr = controller->get_master_bdaddr(fd);
 	if (master_bdaddr == NULL) {
 		DBG("Failed to get the Old master Bluetooth address from the device");
 		return -EPERM;
@@ -305,7 +329,7 @@ static int sixpair(int fd, struct btd_adapter *adapter)
 	 */
 	if (g_strcmp0(master_bdaddr, adapter_bdaddr) != 0) {
 		DBG("Old master Bluetooth address was: %s", master_bdaddr);
-		ret = set_master_bdaddr(fd, adapter_bdaddr);
+		ret = controller->set_master_bdaddr(fd, adapter_bdaddr);
 		if (ret < 0) {
 			DBG("Failed to set the master Bluetooth address");
 			free(master_bdaddr);
@@ -313,7 +337,7 @@ static int sixpair(int fd, struct btd_adapter *adapter)
 		}
 	}
 
-	device_bdaddr = get_device_bdaddr(fd);
+	device_bdaddr = controller->get_device_bdaddr(fd);
 	if (device_bdaddr == NULL) {
 		DBG("Failed to get the Bluetooth address from the device");
 		free(master_bdaddr);
@@ -322,10 +346,7 @@ static int sixpair(int fd, struct btd_adapter *adapter)
 
 	DBG("Device bdaddr %s", device_bdaddr);
 
-	ret = create_sixaxis_association(adapter,
-					SIXAXIS_NAME,
-					device_bdaddr,
-					VENDOR, PRODUCT, SIXAXIS_PNP_RECORD);
+	ret = create_controller_association(adapter, device_bdaddr, controller);
 	free(device_bdaddr);
 	free(master_bdaddr);
 	return ret;
@@ -424,6 +445,7 @@ static void handle_device_plug(struct udev_device *udevice)
 	unsigned char is_usb = FALSE;
 	int js_num = 0;
 	int fd;
+	struct sony_controller *controller;
 
 	hid_parent = udev_device_get_parent_with_subsystem_devtype(udevice,
 								"hid", NULL);
@@ -439,6 +461,8 @@ static void handle_device_plug(struct udev_device *udevice)
 	if (!is_sixaxis(hid_name))
 		return;
 
+	controller = &controllers[0];
+
 	DBG("Found a Sixaxis device");
 
 	hidraw_node = udev_device_get_devnode(udevice);
@@ -507,7 +531,7 @@ static void handle_device_plug(struct udev_device *udevice)
 			DBG("No adapters, exiting");
 			return;
 		}
-		sixpair(fd, adapter);
+		controller_pair(fd, adapter, controller);
 	}
 
 	if (js_num > 0) {
-- 
1.7.5.4

Re: [PATCH 0/4 incremental 1/2] Generalize controller handling to support different devices

[Alan Ott]

On 08/18/2011 10:22 AM, Antonio Ospite wrote:
> Remove hardcoded #defines and put the values in a struct so we can handle 
> different device types.
>
> From the USB dumps I've seen[1], different devices have just different ways to 
> get and set the bdaddrs, the pairing algorithm is the same.
>
> [1] http://ps3.jim.sh/sixaxis/dumps/ 
>
> ---
>  plugins/sixaxis.c |   80 ++++++++++++++++++++++++++++++++++------------------
>  1 files changed, 52 insertions(+), 28 deletions(-)
>
> diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
> index e08222c..608474f 100644
> --- a/plugins/sixaxis.c
> +++ b/plugins/sixaxis.c
> @@ -70,12 +70,38 @@
>  
>  #define BDADDR_STR_SIZE 18 /* strlen("00:00:00:00:00:00") + 1 */
>  
> -/* Vendor and product ID for the Sixaxis PS3 controller */
> -#define VENDOR 0x054c
> -#define PRODUCT 0x0268
> -#define SIXAXIS_NAME "PLAYSTATION(R)3 Controller"
> -#define SIXAXIS_PNP_RECORD "3601920900000A000100000900013503191124090004350D35061901000900113503190011090006350909656E09006A0901000900093508350619112409010009000D350F350D350619010009001335031900110901002513576972656C65737320436F6E74726F6C6C65720901012513576972656C65737320436F6E74726F6C6C6572090102251B536F6E7920436F6D707574657220456E7465727461696E6D656E740902000901000902010901000902020800090203082109020428010902052801090206359A35980822259405010904A101A102850175089501150026FF00810375019513150025013500450105091901291381027501950D0600FF8103150026FF0005010901A10075089504350046FF0009300931093209358102C0050175089527090181027508953009019102750895300901B102C0A1028502750895300901B102C0A10285EE750895300901B102C0A10285EF750895300901B102C0C0090207350835060904090901000902082800090209280109020A280109020B09010009020C093E8009020D280009020E2800"
> -#define HID_UUID "00001124-0000-1000-8000-00805f9b34fb"
> +struct sony_controller {
> +	uint16_t vendor_id;
> +	uint16_t product_id;
> +	char *name;
> +	char *pnp_record;
> +	char *hid_uuid;
> +
> +	/* device specific callbacks to get master/device bdaddr and set
> +	 * master bdaddr
> +	 */
> +	char * (*get_device_bdaddr)(int);
> +	char * (*get_master_bdaddr)(int);
> +	int (*set_master_bdaddr) (int, char *);
> +};
> +
> +static char *sixaxis_get_device_bdaddr(int fd);
> +static char *sixaxis_get_master_bdaddr(int fd);
> +static int sixaxis_set_master_bdaddr(int fd, char *adapter_bdaddr);
> +
> +static struct sony_controller controllers[] = {
> +	{
> +		.vendor_id = 0x054c,
> +		.product_id = 0x0268,
> +		.name = "PLAYSTATION(R)3 Controller",
> +		.pnp_record = "3601920900000A000100000900013503191124090004350D35061901000900113503190011090006350909656E09006A0901000900093508350619112409010009000D350F350D350619010009001335031900110901002513576972656C65737320436F6E74726F6C6C65720901012513576972656C65737320436F6E74726F6C6C6572090102251B536F6E7920436F6D707574657220456E7465727461696E6D656E740902000901000902010901000902020800090203082109020428010902052801090206359A35980822259405010904A101A102850175089501150026FF00810375019513150025013500450105091901291381027501950D0600FF8103150026FF0005010901A10075089504350046FF0009300931093209358102C0050175089527090181027508953009019102750895300901B102C0A1028502750895300901B102C0A10285EE750895300901B102C0A10285EF750895300901B102C0C0090207350835060904090901000902082800090209280109020A280109020B09010009020C093E8009020D280009020E2800",
> +		.hid_uuid = "00001124-0000-1000-8000-00805f9b34fb",

Where do the pnp_record and hid_uuid come from? It seems a bit odd to
have a giant hard-coded identifier string (pnp), but I'm not an expert
on this stuff, and I'm mostly just curious about it.

> +		.get_device_bdaddr = sixaxis_get_device_bdaddr,
> +		.get_master_bdaddr = sixaxis_get_master_bdaddr,
> +		.set_master_bdaddr = sixaxis_set_master_bdaddr,
> +	},
> +};
> +
>  
>  #define LED_1 (0x01 << 1)
>  #define LED_2 (0x01 << 2)
> @@ -90,12 +116,9 @@ static struct udev_monitor *monitor;
>  static guint watch_id;
>  
>  
> -static int create_sixaxis_association(struct btd_adapter *adapter,
> -					const char *name,
> +static int create_controller_association(struct btd_adapter *adapter,
>  					const char *address,
> -					guint32 vendor_id,
> -					guint32 product_id,
> -					const char *pnp_record)
> +					struct sony_controller *controller)
>  {
>  	DBusConnection *conn;
>  	sdp_record_t *rec;
> @@ -108,15 +131,16 @@ static int create_sixaxis_association(struct btd_adapter *adapter,
>  	adapter_get_address(adapter, &src);
>  	ba2str(&src, srcaddr);
>  
> -	write_device_name(&dst, &src, (char *) name);
> +	write_device_name(&dst, &src, controller->name);
>  
>  	/* Store the device's SDP record */
> -	rec = record_from_string(pnp_record);
> +	rec = record_from_string(controller->pnp_record);
>  	store_record(srcaddr, address, rec);
>  	sdp_record_free(rec);
>  
>  	/* Set the device id */
> -	store_device_id(srcaddr, address, 0xffff, vendor_id, product_id, 0);
> +	store_device_id(srcaddr, address, 0xffff, controller->vendor_id,
> +			controller->product_id, 0);
>  	/* Don't write a profile here,
>  	 * it will be updated when the device connects */
>  
> @@ -137,8 +161,8 @@ static int create_sixaxis_association(struct btd_adapter *adapter,
>  	}
>  
>  	device_set_temporary(device, FALSE);
> -	device_set_name(device, name);
> -	btd_device_add_uuid(device, HID_UUID);
> +	device_set_name(device, controller->name);
> +	btd_device_add_uuid(device, controller->hid_uuid);
>  
>  fail_device:
>  	dbus_connection_unref(conn);
> @@ -184,7 +208,7 @@ static int set_feature_report(int fd, uint8_t *report, int len)
>  	return ret;
>  }
>  
> -static char *get_device_bdaddr(int fd)
> +static char *sixaxis_get_device_bdaddr(int fd)
>  {
>  	unsigned char *buf;
>  	char *address;
> @@ -210,7 +234,7 @@ static char *get_device_bdaddr(int fd)
>  	return address;
>  }
>  
> -static char *get_master_bdaddr(int fd)
> +static char *sixaxis_get_master_bdaddr(int fd)
>  {
>  	unsigned char *buf;
>  	char *address;
> @@ -236,7 +260,7 @@ static char *get_master_bdaddr(int fd)
>  	return address;
>  }
>  
> -static int set_master_bdaddr(int fd, char *adapter_bdaddr)
> +static int sixaxis_set_master_bdaddr(int fd, char *adapter_bdaddr)
>  {
>  	uint8_t *report;
>  	uint8_t addr[6];
> @@ -282,7 +306,7 @@ out:
>  	return ret;
>  }
>  
> -static int sixpair(int fd, struct btd_adapter *adapter)
> +static int controller_pair(int fd, struct btd_adapter *adapter, struct sony_controller *controller)
>  {
>  	char *device_bdaddr;
>  	char *master_bdaddr;
> @@ -294,7 +318,7 @@ static int sixpair(int fd, struct btd_adapter *adapter)
>  	ba2str(&dst, adapter_bdaddr);
>  	DBG("Adapter bdaddr %s", adapter_bdaddr);
>  
> -	master_bdaddr = get_master_bdaddr(fd);
> +	master_bdaddr = controller->get_master_bdaddr(fd);
>  	if (master_bdaddr == NULL) {
>  		DBG("Failed to get the Old master Bluetooth address from the device");
>  		return -EPERM;
> @@ -305,7 +329,7 @@ static int sixpair(int fd, struct btd_adapter *adapter)
>  	 */
>  	if (g_strcmp0(master_bdaddr, adapter_bdaddr) != 0) {
>  		DBG("Old master Bluetooth address was: %s", master_bdaddr);
> -		ret = set_master_bdaddr(fd, adapter_bdaddr);
> +		ret = controller->set_master_bdaddr(fd, adapter_bdaddr);
>  		if (ret < 0) {
>  			DBG("Failed to set the master Bluetooth address");
>  			free(master_bdaddr);
> @@ -313,7 +337,7 @@ static int sixpair(int fd, struct btd_adapter *adapter)
>  		}
>  	}
>  
> -	device_bdaddr = get_device_bdaddr(fd);
> +	device_bdaddr = controller->get_device_bdaddr(fd);
>  	if (device_bdaddr == NULL) {
>  		DBG("Failed to get the Bluetooth address from the device");
>  		free(master_bdaddr);
> @@ -322,10 +346,7 @@ static int sixpair(int fd, struct btd_adapter *adapter)
>  
>  	DBG("Device bdaddr %s", device_bdaddr);
>  
> -	ret = create_sixaxis_association(adapter,
> -					SIXAXIS_NAME,
> -					device_bdaddr,
> -					VENDOR, PRODUCT, SIXAXIS_PNP_RECORD);
> +	ret = create_controller_association(adapter, device_bdaddr, controller);
>  	free(device_bdaddr);
>  	free(master_bdaddr);
>  	return ret;
> @@ -424,6 +445,7 @@ static void handle_device_plug(struct udev_device *udevice)
>  	unsigned char is_usb = FALSE;
>  	int js_num = 0;
>  	int fd;
> +	struct sony_controller *controller;
>  
>  	hid_parent = udev_device_get_parent_with_subsystem_devtype(udevice,
>  								"hid", NULL);
> @@ -439,6 +461,8 @@ static void handle_device_plug(struct udev_device *udevice)
>  	if (!is_sixaxis(hid_name))
>  		return;
>  
> +	controller = &controllers[0];
> +

I'd expect the part above to have a for loop checking over all the
entries in the controller array instead of hard-coding to the first one.
With only one right now, of course it works, but making it a loop now
would make it trivial to add the second device (PSMove?) in the future.

>  	DBG("Found a Sixaxis device");
>  
>  	hidraw_node = udev_device_get_devnode(udevice);
> @@ -507,7 +531,7 @@ static void handle_device_plug(struct udev_device *udevice)
>  			DBG("No adapters, exiting");
>  			return;
>  		}
> -		sixpair(fd, adapter);
> +		controller_pair(fd, adapter, controller);
>  	}
>  
>  	if (js_num > 0) {

Hey, looks good overall. I like it. It seems much cleaner.

Alan.

Re: [PATCH 0/4 incremental 1/2] Generalize controller handling to support different devices

[Alan Ott]

On 08/18/2011 10:22 AM, Antonio Ospite wrote:
> Remove hardcoded #defines and put the values in a struct so we can handle 
> different device types.
>
> From the USB dumps I've seen[1], different devices have just different ways to 
> get and set the bdaddrs, the pairing algorithm is the same.
>
> [1] http://ps3.jim.sh/sixaxis/dumps/ 
>
> ---
>  plugins/sixaxis.c |   80 ++++++++++++++++++++++++++++++++++------------------
>  1 files changed, 52 insertions(+), 28 deletions(-)
>
> diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
> index e08222c..608474f 100644
> --- a/plugins/sixaxis.c
> +++ b/plugins/sixaxis.c
> @@ -70,12 +70,38 @@
>  
>  #define BDADDR_STR_SIZE 18 /* strlen("00:00:00:00:00:00") + 1 */
>  
> -/* Vendor and product ID for the Sixaxis PS3 controller */
> -#define VENDOR 0x054c
> -#define PRODUCT 0x0268
> -#define SIXAXIS_NAME "PLAYSTATION(R)3 Controller"
> -#define SIXAXIS_PNP_RECORD "3601920900000A000100000900013503191124090004350D35061901000900113503190011090006350909656E09006A0901000900093508350619112409010009000D350F350D350619010009001335031900110901002513576972656C65737320436F6E74726F6C6C65720901012513576972656C65737320436F6E74726F6C6C6572090102251B536F6E7920436F6D707574657220456E7465727461696E6D656E740902000901000902010901000902020800090203082109020428010902052801090206359A35980822259405010904A101A102850175089501150026FF00810375019513150025013500450105091901291381027501950D0600FF8103150026FF0005010901A10075089504350046FF0009300931093209358102C0050175089527090181027508953009019102750895300901B102C0A1028502750895300901B102C0A10285EE750895300901B102C0A10285EF750895300901B102C0C0090207350835060904090901000902082800090209280109020A2801090
 20B09010009020C093E8009020D280009020E2800"
> -#define HID_UUID "00001124-0000-1000-8000-00805f9b34fb"
> +struct sony_controller {
> +	uint16_t vendor_id;
> +	uint16_t product_id;
> +	char *name;
> +	char *pnp_record;
> +	char *hid_uuid;
> +
> +	/* device specific callbacks to get master/device bdaddr and set
> +	 * master bdaddr
> +	 */
> +	char * (*get_device_bdaddr)(int);
> +	char * (*get_master_bdaddr)(int);
> +	int (*set_master_bdaddr) (int, char *);
> +};
> +
> +static char *sixaxis_get_device_bdaddr(int fd);
> +static char *sixaxis_get_master_bdaddr(int fd);
> +static int sixaxis_set_master_bdaddr(int fd, char *adapter_bdaddr);
> +
> +static struct sony_controller controllers[] = {
> +	{
> +		.vendor_id = 0x054c,
> +		.product_id = 0x0268,
> +		.name = "PLAYSTATION(R)3 Controller",
> +		.pnp_record = "3601920900000A000100000900013503191124090004350D35061901000900113503190011090006350909656E09006A0901000900093508350619112409010009000D350F350D350619010009001335031900110901002513576972656C65737320436F6E74726F6C6C65720901012513576972656C65737320436F6E74726F6C6C6572090102251B536F6E7920436F6D707574657220456E7465727461696E6D656E740902000901000902010901000902020800090203082109020428010902052801090206359A35980822259405010904A101A102850175089501150026FF00810375019513150025013500450105091901291381027501950D0600FF8103150026FF0005010901A10075089504350046FF0009300931093209358102C0050175089527090181027508953009019102750895300901B102C0A1028502750895300901B102C0A10285EE750895300901B102C0A10285EF750895300901B102C0C0090207350835060904090901000902082800090209280109020A280109020B09010009
 020C093E8009020D280009020E2800",
> +		.hid_uuid = "00001124-0000-1000-8000-00805f9b34fb",

Where do the pnp_record and hid_uuid come from? It seems a bit odd to
have a giant hard-coded identifier string (pnp), but I'm not an expert
on this stuff, and I'm mostly just curious about it.

> +		.get_device_bdaddr = sixaxis_get_device_bdaddr,
> +		.get_master_bdaddr = sixaxis_get_master_bdaddr,
> +		.set_master_bdaddr = sixaxis_set_master_bdaddr,
> +	},
> +};
> +
>  
>  #define LED_1 (0x01 << 1)
>  #define LED_2 (0x01 << 2)
> @@ -90,12 +116,9 @@ static struct udev_monitor *monitor;
>  static guint watch_id;
>  
>  
> -static int create_sixaxis_association(struct btd_adapter *adapter,
> -					const char *name,
> +static int create_controller_association(struct btd_adapter *adapter,
>  					const char *address,
> -					guint32 vendor_id,
> -					guint32 product_id,
> -					const char *pnp_record)
> +					struct sony_controller *controller)
>  {
>  	DBusConnection *conn;
>  	sdp_record_t *rec;
> @@ -108,15 +131,16 @@ static int create_sixaxis_association(struct btd_adapter *adapter,
>  	adapter_get_address(adapter, &src);
>  	ba2str(&src, srcaddr);
>  
> -	write_device_name(&dst, &src, (char *) name);
> +	write_device_name(&dst, &src, controller->name);
>  
>  	/* Store the device's SDP record */
> -	rec = record_from_string(pnp_record);
> +	rec = record_from_string(controller->pnp_record);
>  	store_record(srcaddr, address, rec);
>  	sdp_record_free(rec);
>  
>  	/* Set the device id */
> -	store_device_id(srcaddr, address, 0xffff, vendor_id, product_id, 0);
> +	store_device_id(srcaddr, address, 0xffff, controller->vendor_id,
> +			controller->product_id, 0);
>  	/* Don't write a profile here,
>  	 * it will be updated when the device connects */
>  
> @@ -137,8 +161,8 @@ static int create_sixaxis_association(struct btd_adapter *adapter,
>  	}
>  
>  	device_set_temporary(device, FALSE);
> -	device_set_name(device, name);
> -	btd_device_add_uuid(device, HID_UUID);
> +	device_set_name(device, controller->name);
> +	btd_device_add_uuid(device, controller->hid_uuid);
>  
>  fail_device:
>  	dbus_connection_unref(conn);
> @@ -184,7 +208,7 @@ static int set_feature_report(int fd, uint8_t *report, int len)
>  	return ret;
>  }
>  
> -static char *get_device_bdaddr(int fd)
> +static char *sixaxis_get_device_bdaddr(int fd)
>  {
>  	unsigned char *buf;
>  	char *address;
> @@ -210,7 +234,7 @@ static char *get_device_bdaddr(int fd)
>  	return address;
>  }
>  
> -static char *get_master_bdaddr(int fd)
> +static char *sixaxis_get_master_bdaddr(int fd)
>  {
>  	unsigned char *buf;
>  	char *address;
> @@ -236,7 +260,7 @@ static char *get_master_bdaddr(int fd)
>  	return address;
>  }
>  
> -static int set_master_bdaddr(int fd, char *adapter_bdaddr)
> +static int sixaxis_set_master_bdaddr(int fd, char *adapter_bdaddr)
>  {
>  	uint8_t *report;
>  	uint8_t addr[6];
> @@ -282,7 +306,7 @@ out:
>  	return ret;
>  }
>  
> -static int sixpair(int fd, struct btd_adapter *adapter)
> +static int controller_pair(int fd, struct btd_adapter *adapter, struct sony_controller *controller)
>  {
>  	char *device_bdaddr;
>  	char *master_bdaddr;
> @@ -294,7 +318,7 @@ static int sixpair(int fd, struct btd_adapter *adapter)
>  	ba2str(&dst, adapter_bdaddr);
>  	DBG("Adapter bdaddr %s", adapter_bdaddr);
>  
> -	master_bdaddr = get_master_bdaddr(fd);
> +	master_bdaddr = controller->get_master_bdaddr(fd);
>  	if (master_bdaddr == NULL) {
>  		DBG("Failed to get the Old master Bluetooth address from the device");
>  		return -EPERM;
> @@ -305,7 +329,7 @@ static int sixpair(int fd, struct btd_adapter *adapter)
>  	 */
>  	if (g_strcmp0(master_bdaddr, adapter_bdaddr) != 0) {
>  		DBG("Old master Bluetooth address was: %s", master_bdaddr);
> -		ret = set_master_bdaddr(fd, adapter_bdaddr);
> +		ret = controller->set_master_bdaddr(fd, adapter_bdaddr);
>  		if (ret < 0) {
>  			DBG("Failed to set the master Bluetooth address");
>  			free(master_bdaddr);
> @@ -313,7 +337,7 @@ static int sixpair(int fd, struct btd_adapter *adapter)
>  		}
>  	}
>  
> -	device_bdaddr = get_device_bdaddr(fd);
> +	device_bdaddr = controller->get_device_bdaddr(fd);
>  	if (device_bdaddr == NULL) {
>  		DBG("Failed to get the Bluetooth address from the device");
>  		free(master_bdaddr);
> @@ -322,10 +346,7 @@ static int sixpair(int fd, struct btd_adapter *adapter)
>  
>  	DBG("Device bdaddr %s", device_bdaddr);
>  
> -	ret = create_sixaxis_association(adapter,
> -					SIXAXIS_NAME,
> -					device_bdaddr,
> -					VENDOR, PRODUCT, SIXAXIS_PNP_RECORD);
> +	ret = create_controller_association(adapter, device_bdaddr, controller);
>  	free(device_bdaddr);
>  	free(master_bdaddr);
>  	return ret;
> @@ -424,6 +445,7 @@ static void handle_device_plug(struct udev_device *udevice)
>  	unsigned char is_usb = FALSE;
>  	int js_num = 0;
>  	int fd;
> +	struct sony_controller *controller;
>  
>  	hid_parent = udev_device_get_parent_with_subsystem_devtype(udevice,
>  								"hid", NULL);
> @@ -439,6 +461,8 @@ static void handle_device_plug(struct udev_device *udevice)
>  	if (!is_sixaxis(hid_name))
>  		return;
>  
> +	controller = &controllers[0];
> +

I'd expect the part above to have a for loop checking over all the
entries in the controller array instead of hard-coding to the first one.
With only one right now, of course it works, but making it a loop now
would make it trivial to add the second device (PSMove?) in the future.

>  	DBG("Found a Sixaxis device");
>  
>  	hidraw_node = udev_device_get_devnode(udevice);
> @@ -507,7 +531,7 @@ static void handle_device_plug(struct udev_device *udevice)
>  			DBG("No adapters, exiting");
>  			return;
>  		}
> -		sixpair(fd, adapter);
> +		controller_pair(fd, adapter, controller);
>  	}
>  
>  	if (js_num > 0) {

Hey, looks good overall. I like it. It seems much cleaner.

Alan.

Re: [PATCH 0/4 incremental 1/2] Generalize controller handling to support different devices

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 2653 bytes --]

On Thu, 18 Aug 2011 11:26:38 -0400
Alan Ott <alan@signal11.us> wrote:

> On 08/18/2011 10:22 AM, Antonio Ospite wrote:
> > Remove hardcoded #defines and put the values in a struct so we can handle 
> > different device types.
> >
> > From the USB dumps I've seen[1], different devices have just different ways to 
> > get and set the bdaddrs, the pairing algorithm is the same.
> >
> > [1] http://ps3.jim.sh/sixaxis/dumps/ 
> >
> > ---
> >  plugins/sixaxis.c |   80 ++++++++++++++++++++++++++++++++++------------------
> >  1 files changed, 52 insertions(+), 28 deletions(-)
> >
> > diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
> > index e08222c..608474f 100644
> > --- a/plugins/sixaxis.c
> > +++ b/plugins/sixaxis.c
> > @@ -70,12 +70,38 @@
> >  
[...]
> > +static struct sony_controller controllers[] = {
> > +	{
> > +		.vendor_id = 0x054c,
> > +		.product_id = 0x0268,
> > +		.name = "PLAYSTATION(R)3 Controller",
> > +		.pnp_record = "3601920900000A000100000900013503191124090004350D35061901000900113503190011090006350909656E09006A0901000900093508350619112409010009000D350F350D350619010009001335031900110901002513576972656C65737320436F6E74726F6C6C65720901012513576972656C65737320436F6E74726F6C6C6572090102251B536F6E7920436F6D707574657220456E7465727461696E6D656E740902000901000902010901000902020800090203082109020428010902052801090206359A35980822259405010904A101A102850175089501150026FF00810375019513150025013500450105091901291381027501950D0600FF8103150026FF0005010901A10075089504350046FF0009300931093209358102C0050175089527090181027508953009019102750895300901B102C0A1028502750895300901B102C0A10285EE750895300901B102C0A10285EF750895300901B102C0C0090207350835060904090901000902082800090209280109020A280109020B09010009020C093E8009020D280009020E2800",
> > +		.hid_uuid = "00001124-0000-1000-8000-00805f9b34fb",
> 
> Where do the pnp_record and hid_uuid come from? It seems a bit odd to
> have a giant hard-coded identifier string (pnp), but I'm not an expert
> on this stuff, and I'm mostly just curious about it.
>

Bastien provided those, AFAIR PNP_RECORD is an identifier shown in the
association request when the plugin is not enabled, and it could depend
on the device.

WRT HID_UUID, maybe this is not device specific, it might just tell
"this is a HID device", I need confirmation about that so I can move the
field out of the structure.

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 0/4 incremental 1/2] Generalize controller handling to support different devices

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 1148 bytes --]

On Fri, 19 Aug 2011 21:14:49 +0200
Antonio Ospite <ospite@studenti.unina.it> wrote:

> WRT HID_UUID, maybe this is not device specific, it might just tell
> "this is a HID device", I need confirmation about that so I can move the
> field out of the structure.
>

On a second thought, if this UUID is per device _type_, then it may
still make sense to have it in the struct, in case the PS3 headset will
need a different UUID, I'll just rename the field to just "uuid"
because the HID part is relative to the _value_.

Here's some doc about UUIDs I've found:
http://www.bluetooth.org/Technical/AssignedNumbers/service_discovery.htm

About the PNP record, I can find some info about the "PnP Information"
service in SDP, and I can see something with:
	$ sdptool records --tree <device_bdaddr>

but I don't know how to get the actual value currently used in the
plugin.

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 0/4 incremental 1/2] Generalize controller handling to support different devices

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 3324 bytes --]

On Fri, 19 Aug 2011 22:57:35 +0200
Antonio Ospite <ospite@studenti.unina.it> wrote:

[...]
> About the PNP record, I can find some info about the "PnP Information"
> service in SDP, and I can see something with:
> 	$ sdptool records --tree <device_bdaddr>
> 
> but I don't know how to get the actual value currently used in the
> plugin.
>

The current PNP_RECORD for sixaxis:

3601920900000A000100000900013503191124090004350D350619010009001135031900 \
11090006350909656E09006A0901000900093508350619112409010009000D350F350D35 \
0619010009001335031900110901002513576972656C65737320436F6E74726F6C6C6572 \
0901012513576972656C65737320436F6E74726F6C6C6572090102251B536F6E7920436F \
6D707574657220456E7465727461696E6D656E7409020009010009020109010009020208 \
00090203082109020428010902052801090206359A35980822259405010904A101A10285 \
0175089501150026FF00810375019513150025013500450105091901291381027501950D \
0600FF8103150026FF0005010901A10075089504350046FF0009300931093209358102C0 \
050175089527090181027508953009019102750895300901B102C0A10285027508953009 \
01B102C0A10285EE750895300901B102C0A10285EF750895300901B102C0C00902073508 \
35060904090901000902082800090209280109020A280109020B09010009020C093E8009 \
020D280009020E2800

is just the same info as in the _first_ record shown decoded by:
	$ sdptool records --tree --raw <device_bdaddr>

which is a HumanInterfaceDeviceService record, so maybe it can even be
named HID record, not PNP...

Unless I am still missing something we need to hardcode it because we need
it _before_ the device talks SDP. I wonder if it can be retrieved via USB,
but I doubt it, maybe a trace of the _very_first_ usb connection of a
controller to a PS3 could revel something...

JFYI if we break it down and look at lib/sdp.h for the field types we can
see the structure in the record:

36 0192
09 0000
0A 00010000
09 0001
35 03
19 1124
09 0004
35 0D
35 06
19 0100
09 0011
35 03
19 0011
09 0006
35 09
09 656E
09 006A
09 0100
09 0009
35 08
35 06
19 1124
09 0100
09 000D
35 0F
35 0D
35 06
19 0100
09 0013
35 03
19 0011
09 0100
25 13 576972656C65737320436F6E74726F6C6C6572
      # String: Wireless Controller
09 0101
25 13 576972656C65737320436F6E74726F6C6C6572
      # String: Wireless Controller
09 0102
25 1B 536F6E7920436F6D707574657220456E7465727461696E6D656E74
      # String: Sony Computer Entertainment
09 0200
09 0100
09 0201
09 0100
09 0202
08 00
09 0203
08 21
09 0204
28 01
09 0205
28 01
09 0206
35 9A
35 98
08 22
25 94 05010904A101A102850175089501150026FF008103750195131500250135004501 \
      05091901291381027501950D0600FF8103150026FF0005010901A1007508950435 \
      0046FF0009300931093209358102C0050175089527090181027508953009019102 \
      750895300901B102C0A1028502750895300901B102C0A10285EE750895300901B1 \
      02C0A10285EF750895300901B102C0C0 # HID descriptor
09 0207
35 08
35 06
09 0409
09 0100
09 0208
28 00
09 02
09 2801
09 020A
28 01
09 020B
09 0100
09 020C
09 3E80
09 020D
28 00
09 020E
28 00


Regards,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH 2/2] Match controllers using vendor_id and product_id instead of HID_NAME

[Antonio Ospite]

Maybe controller->name can be filled in from HID_NAME, but for now having it 
hardcoded doesn't hurt.

---
 plugins/sixaxis.c |   42 ++++++++++++++++++++++++++++++------------
 1 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
index 608474f..fc1cd52 100644
--- a/plugins/sixaxis.c
+++ b/plugins/sixaxis.c
@@ -426,12 +426,29 @@ static int set_controller_number(int fd, unsigned int n)
 	return set_leds(fd, leds_status);
 }
 
-
-static inline gboolean is_sixaxis(const char *hid_name)
+static inline struct sony_controller *find_sony_controller(const char *hid_id)
 {
-	return g_str_has_suffix(hid_name, "PLAYSTATION(R)3 Controller") ||
-		g_str_has_suffix(hid_name,
-			"Sony Computer Entertainment Wireless Controller");
+	unsigned int array_size = sizeof(controllers)/sizeof(controllers[0]);
+	unsigned int i;
+	int ret;
+	uint16_t protocol;
+	uint16_t vendor_id;
+	uint16_t product_id;
+
+	ret = sscanf(hid_id, "%hx:%hx:%hx", &protocol, &vendor_id, &product_id);
+	if (ret != 3) {
+		error("%s:%s() Parsing HID_ID failed",
+			__FILE__, __func__);
+		return NULL;
+	}
+
+	for (i = 0; i < array_size; i++) {
+		if (controllers[i].vendor_id == vendor_id &&
+		    controllers[i].product_id == product_id)
+			return &controllers[i];
+	}
+
+	return NULL;
 }
 
 static void handle_device_plug(struct udev_device *udevice)
@@ -439,7 +456,7 @@ static void handle_device_plug(struct udev_device *udevice)
 	struct udev_device *hid_parent;
 	struct udev_enumerate *enumerate;
 	struct udev_list_entry *devices, *dev_list_entry;
-	const char *hid_name;
+	const char *hid_id;
 	const char *hid_phys;
 	const char *hidraw_node;
 	unsigned char is_usb = FALSE;
@@ -455,15 +472,16 @@ static void handle_device_plug(struct udev_device *udevice)
 		return;
 	}
 
-	hid_name = udev_device_get_property_value(hid_parent, "HID_NAME");
-	DBG("name: %s", hid_name);
+	hid_id = udev_device_get_property_value(hid_parent, "HID_ID");
+	DBG("HID_ID: %s", hid_id);
 
-	if (!is_sixaxis(hid_name))
+	controller = find_sony_controller(hid_id);
+	if (!controller) {
+		DBG("No supported controller found");
 		return;
+	}
 
-	controller = &controllers[0];
-
-	DBG("Found a Sixaxis device");
+	DBG("Found a Sony controller: %s", controller->name);
 
 	hidraw_node = udev_device_get_devnode(udevice);
 
-- 
1.7.5.4

Re: [PATCH BlueZ 2/4] Add sixaxis plugin: USB pairing and LEDs settings

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 731 bytes --]

On Thu, 18 Aug 2011 16:13:57 +0200
Antonio Ospite <ospite@studenti.unina.it> wrote:

[...]
> If the idea looks OK I'll fold those patches in v5, along with the
> plugin renaming to "sony-controller", and maybe I'll split the plugin in
> several modules too:
>   sony-controller.c 	/* udev and bluez stuff */
>   sony-controller-hid.c	/* hidraw and device specific stuff */
>   sony-controlled-hid.h
> 

Do you think "controller" fits the PS3 headset as well?

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH BlueZ 2/4] Add sixaxis plugin: USB pairing and LEDs settings

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 763 bytes --]

On Thu, 18 Aug 2011 16:13:57 +0200
Antonio Ospite <ospite-aNJ+ML1ZbiP93QAQaVx+gl6hYfS7NtTn@public.gmane.org> wrote:

[...]
> If the idea looks OK I'll fold those patches in v5, along with the
> plugin renaming to "sony-controller", and maybe I'll split the plugin in
> several modules too:
>   sony-controller.c 	/* udev and bluez stuff */
>   sony-controller-hid.c	/* hidraw and device specific stuff */
>   sony-controlled-hid.h
> 

Do you think "controller" fits the PS3 headset as well?

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH BlueZ 2/4] Add sixaxis plugin: USB pairing and LEDs settings

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 1353 bytes --]

On Fri, 19 Aug 2011 21:57:16 +0200
Antonio Ospite <ospite@studenti.unina.it> wrote:

> On Thu, 18 Aug 2011 16:13:57 +0200
> Antonio Ospite <ospite@studenti.unina.it> wrote:
> 
> [...]
> > If the idea looks OK I'll fold those patches in v5, along with the
> > plugin renaming to "sony-controller", and maybe I'll split the plugin in
> > several modules too:
> >   sony-controller.c 	/* udev and bluez stuff */
> >   sony-controller-hid.c	/* hidraw and device specific stuff */
> >   sony-controlled-hid.h
> > 
> 
> Do you think "controller" fits the PS3 headset as well?
> 

OK, given that:
  - all the possibly supported devices at this time are PS3 peripherals;
  - some of them are not input controllers;
  - the cable pairing/association  mechanism generally does not apply to
    other sony BT hardware;
  - someone imagined a future PS4 controller using the same pairing 
    mechanism;

with Alan we came up with the name:

	playstation-peripheral

for the plugin; do you people like that, or does the variant

	playstation-device

sound better to you?

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH BlueZ 3/4] Link to udev only when needed

[Antonio Ospite]

Don't link bluetoothd to udev unconditionally, but only when it is
needed. For now bluetoothd needs to be linked to udev only when the
sixaxis plugin is enabled.
---
 Makefile.am |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index dbe0170..219ca0f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -288,7 +288,11 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
 			src/event.h src/event.c \
 			src/oob.h src/oob.c src/eir.h src/eir.c
 src_bluetoothd_LDADD = lib/libbluetooth.la @GLIB_LIBS@ @DBUS_LIBS@ \
-							@CAPNG_LIBS@ @UDEV_LIBS@ -ldl -lrt
+							@CAPNG_LIBS@ -ldl -lrt
+if SIXAXISPLUGIN
+src_bluetoothd_LDADD += @UDEV_LIBS@
+endif
+
 src_bluetoothd_LDFLAGS = -Wl,--export-dynamic \
 				-Wl,--version-script=$(srcdir)/src/bluetooth.ver
 
-- 
1.7.5.4

[PATCH BlueZ 3/4] Link to udev only when needed

[Antonio Ospite]

Don't link bluetoothd to udev unconditionally, but only when it is
needed. For now bluetoothd needs to be linked to udev only when the
sixaxis plugin is enabled.
---
 Makefile.am |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index dbe0170..219ca0f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -288,7 +288,11 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
 			src/event.h src/event.c \
 			src/oob.h src/oob.c src/eir.h src/eir.c
 src_bluetoothd_LDADD = lib/libbluetooth.la @GLIB_LIBS@ @DBUS_LIBS@ \
-							@CAPNG_LIBS@ @UDEV_LIBS@ -ldl -lrt
+							@CAPNG_LIBS@ -ldl -lrt
+if SIXAXISPLUGIN
+src_bluetoothd_LDADD += @UDEV_LIBS@
+endif
+
 src_bluetoothd_LDFLAGS = -Wl,--export-dynamic \
 				-Wl,--version-script=$(srcdir)/src/bluetooth.ver
 
-- 
1.7.5.4

Re: [PATCH BlueZ 3/4] Link to udev only when needed

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 1335 bytes --]

On Fri,  5 Aug 2011 16:09:17 +0200
Antonio Ospite <ospite@studenti.unina.it> wrote:

> Don't link bluetoothd to udev unconditionally, but only when it is
> needed. For now bluetoothd needs to be linked to udev only when the
> sixaxis plugin is enabled.

Any comment on this change? If it looks OK I'll fold it in v5.

Thanks,
   Antonio

> ---
>  Makefile.am |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index dbe0170..219ca0f 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -288,7 +288,11 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
>  			src/event.h src/event.c \
>  			src/oob.h src/oob.c src/eir.h src/eir.c
>  src_bluetoothd_LDADD = lib/libbluetooth.la @GLIB_LIBS@ @DBUS_LIBS@ \
> -							@CAPNG_LIBS@ @UDEV_LIBS@ -ldl -lrt
> +							@CAPNG_LIBS@ -ldl -lrt
> +if SIXAXISPLUGIN
> +src_bluetoothd_LDADD += @UDEV_LIBS@
> +endif
> +
>  src_bluetoothd_LDFLAGS = -Wl,--export-dynamic \
>  				-Wl,--version-script=$(srcdir)/src/bluetooth.ver
>  
> -- 
> 1.7.5.4
> 
> 


-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH BlueZ 3/4] Link to udev only when needed

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 1367 bytes --]

On Fri,  5 Aug 2011 16:09:17 +0200
Antonio Ospite <ospite-aNJ+ML1ZbiP93QAQaVx+gl6hYfS7NtTn@public.gmane.org> wrote:

> Don't link bluetoothd to udev unconditionally, but only when it is
> needed. For now bluetoothd needs to be linked to udev only when the
> sixaxis plugin is enabled.

Any comment on this change? If it looks OK I'll fold it in v5.

Thanks,
   Antonio

> ---
>  Makefile.am |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index dbe0170..219ca0f 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -288,7 +288,11 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
>  			src/event.h src/event.c \
>  			src/oob.h src/oob.c src/eir.h src/eir.c
>  src_bluetoothd_LDADD = lib/libbluetooth.la @GLIB_LIBS@ @DBUS_LIBS@ \
> -							@CAPNG_LIBS@ @UDEV_LIBS@ -ldl -lrt
> +							@CAPNG_LIBS@ -ldl -lrt
> +if SIXAXISPLUGIN
> +src_bluetoothd_LDADD += @UDEV_LIBS@
> +endif
> +
>  src_bluetoothd_LDFLAGS = -Wl,--export-dynamic \
>  				-Wl,--version-script=$(srcdir)/src/bluetooth.ver
>  
> -- 
> 1.7.5.4
> 
> 


-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH BlueZ 3/4] Link to udev only when needed

[Marcel Holtmann]

Hi Antonio,

> 
> > Don't link bluetoothd to udev unconditionally, but only when it is
> > needed. For now bluetoothd needs to be linked to udev only when the
> > sixaxis plugin is enabled.
> 
> Any comment on this change? If it looks OK I'll fold it in v5.
> 
> Thanks,
>    Antonio
> 
> > ---
> >  Makefile.am |    6 +++++-
> >  1 files changed, 5 insertions(+), 1 deletions(-)
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index dbe0170..219ca0f 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -288,7 +288,11 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
> >  			src/event.h src/event.c \
> >  			src/oob.h src/oob.c src/eir.h src/eir.c
> >  src_bluetoothd_LDADD = lib/libbluetooth.la @GLIB_LIBS@ @DBUS_LIBS@ \
> > -							@CAPNG_LIBS@ @UDEV_LIBS@ -ldl -lrt
> > +							@CAPNG_LIBS@ -ldl -lrt
> > +if SIXAXISPLUGIN
> > +src_bluetoothd_LDADD += @UDEV_LIBS@
> > +endif
> > +
> >  src_bluetoothd_LDFLAGS = -Wl,--export-dynamic \
> >  				-Wl,--version-script=$(srcdir)/src/bluetooth.ver

I rather have the Sixaxis plugin being an external plugin only then. So
you are not allowed to make this builtin.

Regards

Marcel

Re: [PATCH BlueZ 3/4] Link to udev only when needed

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 1950 bytes --]

On Thu, 18 Aug 2011 16:18:57 -0700
Marcel Holtmann <marcel@holtmann.org> wrote:

> Hi Antonio,
> 
> > 
> > > Don't link bluetoothd to udev unconditionally, but only when it is
> > > needed. For now bluetoothd needs to be linked to udev only when the
> > > sixaxis plugin is enabled.
> > 
> > Any comment on this change? If it looks OK I'll fold it in v5.
> > 
> > Thanks,
> >    Antonio
> > 
> > > ---
> > >  Makefile.am |    6 +++++-
> > >  1 files changed, 5 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/Makefile.am b/Makefile.am
> > > index dbe0170..219ca0f 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -288,7 +288,11 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
> > >  			src/event.h src/event.c \
> > >  			src/oob.h src/oob.c src/eir.h src/eir.c
> > >  src_bluetoothd_LDADD = lib/libbluetooth.la @GLIB_LIBS@ @DBUS_LIBS@ \
> > > -							@CAPNG_LIBS@ @UDEV_LIBS@ -ldl -lrt
> > > +							@CAPNG_LIBS@ -ldl -lrt
> > > +if SIXAXISPLUGIN
> > > +src_bluetoothd_LDADD += @UDEV_LIBS@
> > > +endif
> > > +
> > >  src_bluetoothd_LDFLAGS = -Wl,--export-dynamic \
> > >  				-Wl,--version-script=$(srcdir)/src/bluetooth.ver
> 
> I rather have the Sixaxis plugin being an external plugin only then. So
> you are not allowed to make this builtin.
> 

Marcel, I guess that an _external_ plugin in BlueZ context is one which
is not embedded in the bluetoothd binary but rather distributed as a
shared object, isn't it? Are there examples of such a plugin in bluez
tree I can copy from?

Or do you mean a standalone daemon interacting with bluetoothd via
dbus? AFAIR we discarded this option in the past.

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH BlueZ 3/4] Link to udev only when needed

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 2326 bytes --]

On Fri, 19 Aug 2011 10:58:02 +0200
Antonio Ospite <ospite@studenti.unina.it> wrote:

> On Thu, 18 Aug 2011 16:18:57 -0700
> Marcel Holtmann <marcel@holtmann.org> wrote:
> 
[...]
> > 
> > I rather have the Sixaxis plugin being an external plugin only then. So
> > you are not allowed to make this builtin.
> > 
> 
> Marcel, I guess that an _external_ plugin in BlueZ context is one which
> is not embedded in the bluetoothd binary but rather distributed as a
> shared object, isn't it? Are there examples of such a plugin in bluez
> tree I can copy from?
>

OK, I think I've got it, it is external-dummy, this is how I did it:

---
 Makefile.am |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 4bcff4d..ff33bf0 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -230,8 +230,10 @@ builtin_sources += thermometer/main.c \
 endif
 
 if SIXAXISPLUGIN
-builtin_modules += sixaxis
-builtin_sources += plugins/sixaxis.c
+plugin_LTLIBRARIES += plugins/sixaxis.la
+plugins_sixaxis_la_SOURCES = plugins/sixaxis.c
+plugins_sixaxis_la_LDFLAGS = -module -avoid-version -no-undefined @UDEV_LIBS@
+plugins_sixaxis_la_CFLAGS = -fvisibility=hidden @DBUS_CFLAGS@ @GLIB_CFLAGS@ @UDEV_CFLAGS@
 endif
 
 builtin_modules += hciops mgmtops
@@ -300,7 +302,7 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
 			src/event.h src/event.c \
 			src/oob.h src/oob.c src/eir.h src/eir.c
 src_bluetoothd_LDADD = lib/libbluetooth.la @GLIB_LIBS@ @DBUS_LIBS@ \
-							@CAPNG_LIBS@ @UDEV_LIBS@ -ldl -lrt
+							@CAPNG_LIBS@ -ldl -lrt
 src_bluetoothd_LDFLAGS = -Wl,--export-dynamic \
 				-Wl,--version-script=$(srcdir)/src/bluetooth.ver
 
@@ -415,7 +417,7 @@ EXTRA_DIST += doc/manager-api.txt \
 
 AM_YFLAGS = -d
 
-AM_CFLAGS = @DBUS_CFLAGS@ @GLIB_CFLAGS@ @CAPNG_CFLAGS@ @UDEV_CFLAGS@ \
+AM_CFLAGS = @DBUS_CFLAGS@ @GLIB_CFLAGS@ @CAPNG_CFLAGS@ \
 		-DBLUETOOTH_PLUGIN_BUILTIN -DPLUGINDIR=\""$(plugindir)"\"
 
 INCLUDES = -I$(builddir)/lib -I$(builddir)/src -I$(srcdir)/src \
-- 
1.7.5.4

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH BlueZ 3/4] Link to udev only when needed

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 2385 bytes --]

On Fri, 19 Aug 2011 10:58:02 +0200
Antonio Ospite <ospite-aNJ+ML1ZbiP93QAQaVx+gl6hYfS7NtTn@public.gmane.org> wrote:

> On Thu, 18 Aug 2011 16:18:57 -0700
> Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
> 
[...]
> > 
> > I rather have the Sixaxis plugin being an external plugin only then. So
> > you are not allowed to make this builtin.
> > 
> 
> Marcel, I guess that an _external_ plugin in BlueZ context is one which
> is not embedded in the bluetoothd binary but rather distributed as a
> shared object, isn't it? Are there examples of such a plugin in bluez
> tree I can copy from?
>

OK, I think I've got it, it is external-dummy, this is how I did it:

---
 Makefile.am |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 4bcff4d..ff33bf0 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -230,8 +230,10 @@ builtin_sources += thermometer/main.c \
 endif
 
 if SIXAXISPLUGIN
-builtin_modules += sixaxis
-builtin_sources += plugins/sixaxis.c
+plugin_LTLIBRARIES += plugins/sixaxis.la
+plugins_sixaxis_la_SOURCES = plugins/sixaxis.c
+plugins_sixaxis_la_LDFLAGS = -module -avoid-version -no-undefined @UDEV_LIBS@
+plugins_sixaxis_la_CFLAGS = -fvisibility=hidden @DBUS_CFLAGS@ @GLIB_CFLAGS@ @UDEV_CFLAGS@
 endif
 
 builtin_modules += hciops mgmtops
@@ -300,7 +302,7 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
 			src/event.h src/event.c \
 			src/oob.h src/oob.c src/eir.h src/eir.c
 src_bluetoothd_LDADD = lib/libbluetooth.la @GLIB_LIBS@ @DBUS_LIBS@ \
-							@CAPNG_LIBS@ @UDEV_LIBS@ -ldl -lrt
+							@CAPNG_LIBS@ -ldl -lrt
 src_bluetoothd_LDFLAGS = -Wl,--export-dynamic \
 				-Wl,--version-script=$(srcdir)/src/bluetooth.ver
 
@@ -415,7 +417,7 @@ EXTRA_DIST += doc/manager-api.txt \
 
 AM_YFLAGS = -d
 
-AM_CFLAGS = @DBUS_CFLAGS@ @GLIB_CFLAGS@ @CAPNG_CFLAGS@ @UDEV_CFLAGS@ \
+AM_CFLAGS = @DBUS_CFLAGS@ @GLIB_CFLAGS@ @CAPNG_CFLAGS@ \
 		-DBLUETOOTH_PLUGIN_BUILTIN -DPLUGINDIR=\""$(plugindir)"\"
 
 INCLUDES = -I$(builddir)/lib -I$(builddir)/src -I$(srcdir)/src \
-- 
1.7.5.4

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH BlueZ 3/4] Link to udev only when needed

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 2521 bytes --]

On Fri, 19 Aug 2011 21:08:54 +0200
Antonio Ospite <ospite@studenti.unina.it> wrote:

> On Fri, 19 Aug 2011 10:58:02 +0200
> Antonio Ospite <ospite@studenti.unina.it> wrote:
> 
> > On Thu, 18 Aug 2011 16:18:57 -0700
> > Marcel Holtmann <marcel@holtmann.org> wrote:
> > 
> [...]
> > > 
> > > I rather have the Sixaxis plugin being an external plugin only then. So
> > > you are not allowed to make this builtin.
> > > 
> > 
> > Marcel, I guess that an _external_ plugin in BlueZ context is one which
> > is not embedded in the bluetoothd binary but rather distributed as a
> > shared object, isn't it? Are there examples of such a plugin in bluez
> > tree I can copy from?
> >
> 
> OK, I think I've got it, it is external-dummy, this is how I did it:
> 
> ---
>  Makefile.am |   10 ++++++----
>  1 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 4bcff4d..ff33bf0 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -230,8 +230,10 @@ builtin_sources += thermometer/main.c \
>  endif
>  
>  if SIXAXISPLUGIN
> -builtin_modules += sixaxis
> -builtin_sources += plugins/sixaxis.c
> +plugin_LTLIBRARIES += plugins/sixaxis.la
> +plugins_sixaxis_la_SOURCES = plugins/sixaxis.c
> +plugins_sixaxis_la_LDFLAGS = -module -avoid-version -no-undefined @UDEV_LIBS@
> +plugins_sixaxis_la_CFLAGS = -fvisibility=hidden @DBUS_CFLAGS@ @GLIB_CFLAGS@ @UDEV_CFLAGS@
>  endif
> 

This is not enough, the external plugin can be compiled but I can be used
at runtime because it is using functions from src/storage.c and others,
which seem to be embedded inside bluetoothd only, so I get:

bluetoothd[23459]: Can't load plugin \
  /home/ao2/Proj/PS3/PS3_sixaxis/bluez/plugins/.libs/playstation-peripheral.so: \
  /home/ao2/Proj/PS3/PS3_sixaxis/bluez/plugins/.libs/playstation-peripheral.so: \
  undefined symbol: write_device_name

I could link the needed object files like storage.o and company into the
plugin binary, but definitely this doesn't look OK. Should a shared library
provide those "utility" functions to bluetoothd _and_ external plugins?

This plugin is the first non-trivial external plugin I can see in the tree,
so maybe this problem has never showed up before.

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH BlueZ 3/4] Link to udev only when needed

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 2612 bytes --]

On Fri, 19 Aug 2011 21:08:54 +0200
Antonio Ospite <ospite-aNJ+ML1ZbiP93QAQaVx+gl6hYfS7NtTn@public.gmane.org> wrote:

> On Fri, 19 Aug 2011 10:58:02 +0200
> Antonio Ospite <ospite-aNJ+ML1ZbiP93QAQaVx+gl6hYfS7NtTn@public.gmane.org> wrote:
> 
> > On Thu, 18 Aug 2011 16:18:57 -0700
> > Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
> > 
> [...]
> > > 
> > > I rather have the Sixaxis plugin being an external plugin only then. So
> > > you are not allowed to make this builtin.
> > > 
> > 
> > Marcel, I guess that an _external_ plugin in BlueZ context is one which
> > is not embedded in the bluetoothd binary but rather distributed as a
> > shared object, isn't it? Are there examples of such a plugin in bluez
> > tree I can copy from?
> >
> 
> OK, I think I've got it, it is external-dummy, this is how I did it:
> 
> ---
>  Makefile.am |   10 ++++++----
>  1 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 4bcff4d..ff33bf0 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -230,8 +230,10 @@ builtin_sources += thermometer/main.c \
>  endif
>  
>  if SIXAXISPLUGIN
> -builtin_modules += sixaxis
> -builtin_sources += plugins/sixaxis.c
> +plugin_LTLIBRARIES += plugins/sixaxis.la
> +plugins_sixaxis_la_SOURCES = plugins/sixaxis.c
> +plugins_sixaxis_la_LDFLAGS = -module -avoid-version -no-undefined @UDEV_LIBS@
> +plugins_sixaxis_la_CFLAGS = -fvisibility=hidden @DBUS_CFLAGS@ @GLIB_CFLAGS@ @UDEV_CFLAGS@
>  endif
> 

This is not enough, the external plugin can be compiled but I can be used
at runtime because it is using functions from src/storage.c and others,
which seem to be embedded inside bluetoothd only, so I get:

bluetoothd[23459]: Can't load plugin \
  /home/ao2/Proj/PS3/PS3_sixaxis/bluez/plugins/.libs/playstation-peripheral.so: \
  /home/ao2/Proj/PS3/PS3_sixaxis/bluez/plugins/.libs/playstation-peripheral.so: \
  undefined symbol: write_device_name

I could link the needed object files like storage.o and company into the
plugin binary, but definitely this doesn't look OK. Should a shared library
provide those "utility" functions to bluetoothd _and_ external plugins?

This plugin is the first non-trivial external plugin I can see in the tree,
so maybe this problem has never showed up before.

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH BlueZ 3/4] Link to udev only when needed

[Vinicius Costa Gomes]

Hi Antonio,

On 16:14 Thu 25 Aug, Antonio Ospite wrote:
> On Fri, 19 Aug 2011 21:08:54 +0200
> Antonio Ospite <ospite@studenti.unina.it> wrote:
> 
> > On Fri, 19 Aug 2011 10:58:02 +0200
> > Antonio Ospite <ospite@studenti.unina.it> wrote:
> > 
> > > On Thu, 18 Aug 2011 16:18:57 -0700
> > > Marcel Holtmann <marcel@holtmann.org> wrote:
> > > 
> > [...]
> > > > 
> > > > I rather have the Sixaxis plugin being an external plugin only then. So
> > > > you are not allowed to make this builtin.
> > > > 
> > > 
> > > Marcel, I guess that an _external_ plugin in BlueZ context is one which
> > > is not embedded in the bluetoothd binary but rather distributed as a
> > > shared object, isn't it? Are there examples of such a plugin in bluez
> > > tree I can copy from?
> > >
> > 
> > OK, I think I've got it, it is external-dummy, this is how I did it:
> > 
> > ---
> >  Makefile.am |   10 ++++++----
> >  1 files changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 4bcff4d..ff33bf0 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -230,8 +230,10 @@ builtin_sources += thermometer/main.c \
> >  endif
> >  
> >  if SIXAXISPLUGIN
> > -builtin_modules += sixaxis
> > -builtin_sources += plugins/sixaxis.c
> > +plugin_LTLIBRARIES += plugins/sixaxis.la
> > +plugins_sixaxis_la_SOURCES = plugins/sixaxis.c
> > +plugins_sixaxis_la_LDFLAGS = -module -avoid-version -no-undefined @UDEV_LIBS@
> > +plugins_sixaxis_la_CFLAGS = -fvisibility=hidden @DBUS_CFLAGS@ @GLIB_CFLAGS@ @UDEV_CFLAGS@
> >  endif
> > 
> 
> This is not enough, the external plugin can be compiled but I can be used
> at runtime because it is using functions from src/storage.c and others,
> which seem to be embedded inside bluetoothd only, so I get:
> 
> bluetoothd[23459]: Can't load plugin \
>   /home/ao2/Proj/PS3/PS3_sixaxis/bluez/plugins/.libs/playstation-peripheral.so: \
>   /home/ao2/Proj/PS3/PS3_sixaxis/bluez/plugins/.libs/playstation-peripheral.so: \
>   undefined symbol: write_device_name
> 
> I could link the needed object files like storage.o and company into the
> plugin binary, but definitely this doesn't look OK. Should a shared library
> provide those "utility" functions to bluetoothd _and_ external plugins?
> 
> This plugin is the first non-trivial external plugin I can see in the tree,
> so maybe this problem has never showed up before.

The problem is the visibility of the symbols, src/bluetooth.ver defines
which symbols should[1] be visible to plugins. Every plugin should only
depend on those methods from the "core".

If we want to keep the sixaxis plugin external we need to make the
symbols it need visible, but I wonder if that is right solution.

[1] Seems that with builtin plugins that visibility enforcement doesn't
work, right?

> 
> Thanks,
>    Antonio
> 
> -- 
> Antonio Ospite
> http://ao2.it
> 
> PGP public key ID: 0x4553B001
> 
> A: Because it messes up the order in which people normally read text.
>    See http://en.wikipedia.org/wiki/Posting_style
> Q: Why is top-posting such a bad thing?



Cheers,
-- 
Vinicius

Re: [PATCH BlueZ 3/4] Link to udev only when needed

[Vinicius Costa Gomes]

Hi Antonio,

On 16:14 Thu 25 Aug, Antonio Ospite wrote:
> On Fri, 19 Aug 2011 21:08:54 +0200
> Antonio Ospite <ospite-aNJ+ML1ZbiP93QAQaVx+gl6hYfS7NtTn@public.gmane.org> wrote:
> 
> > On Fri, 19 Aug 2011 10:58:02 +0200
> > Antonio Ospite <ospite-aNJ+ML1ZbiP93QAQaVx+gl6hYfS7NtTn@public.gmane.org> wrote:
> > 
> > > On Thu, 18 Aug 2011 16:18:57 -0700
> > > Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
> > > 
> > [...]
> > > > 
> > > > I rather have the Sixaxis plugin being an external plugin only then. So
> > > > you are not allowed to make this builtin.
> > > > 
> > > 
> > > Marcel, I guess that an _external_ plugin in BlueZ context is one which
> > > is not embedded in the bluetoothd binary but rather distributed as a
> > > shared object, isn't it? Are there examples of such a plugin in bluez
> > > tree I can copy from?
> > >
> > 
> > OK, I think I've got it, it is external-dummy, this is how I did it:
> > 
> > ---
> >  Makefile.am |   10 ++++++----
> >  1 files changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 4bcff4d..ff33bf0 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -230,8 +230,10 @@ builtin_sources += thermometer/main.c \
> >  endif
> >  
> >  if SIXAXISPLUGIN
> > -builtin_modules += sixaxis
> > -builtin_sources += plugins/sixaxis.c
> > +plugin_LTLIBRARIES += plugins/sixaxis.la
> > +plugins_sixaxis_la_SOURCES = plugins/sixaxis.c
> > +plugins_sixaxis_la_LDFLAGS = -module -avoid-version -no-undefined @UDEV_LIBS@
> > +plugins_sixaxis_la_CFLAGS = -fvisibility=hidden @DBUS_CFLAGS@ @GLIB_CFLAGS@ @UDEV_CFLAGS@
> >  endif
> > 
> 
> This is not enough, the external plugin can be compiled but I can be used
> at runtime because it is using functions from src/storage.c and others,
> which seem to be embedded inside bluetoothd only, so I get:
> 
> bluetoothd[23459]: Can't load plugin \
>   /home/ao2/Proj/PS3/PS3_sixaxis/bluez/plugins/.libs/playstation-peripheral.so: \
>   /home/ao2/Proj/PS3/PS3_sixaxis/bluez/plugins/.libs/playstation-peripheral.so: \
>   undefined symbol: write_device_name
> 
> I could link the needed object files like storage.o and company into the
> plugin binary, but definitely this doesn't look OK. Should a shared library
> provide those "utility" functions to bluetoothd _and_ external plugins?
> 
> This plugin is the first non-trivial external plugin I can see in the tree,
> so maybe this problem has never showed up before.

The problem is the visibility of the symbols, src/bluetooth.ver defines
which symbols should[1] be visible to plugins. Every plugin should only
depend on those methods from the "core".

If we want to keep the sixaxis plugin external we need to make the
symbols it need visible, but I wonder if that is right solution.

[1] Seems that with builtin plugins that visibility enforcement doesn't
work, right?

> 
> Thanks,
>    Antonio
> 
> -- 
> Antonio Ospite
> http://ao2.it
> 
> PGP public key ID: 0x4553B001
> 
> A: Because it messes up the order in which people normally read text.
>    See http://en.wikipedia.org/wiki/Posting_style
> Q: Why is top-posting such a bad thing?



Cheers,
-- 
Vinicius

Re: [PATCH BlueZ 3/4] Link to udev only when needed

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 3602 bytes --]

On Thu, 25 Aug 2011 14:06:21 -0300
Vinicius Costa Gomes <vinicius.gomes@openbossa.org> wrote:

[...]
> > > diff --git a/Makefile.am b/Makefile.am
> > > index 4bcff4d..ff33bf0 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -230,8 +230,10 @@ builtin_sources += thermometer/main.c \
> > >  endif
> > >  
> > >  if SIXAXISPLUGIN
> > > -builtin_modules += sixaxis
> > > -builtin_sources += plugins/sixaxis.c
> > > +plugin_LTLIBRARIES += plugins/sixaxis.la
> > > +plugins_sixaxis_la_SOURCES = plugins/sixaxis.c
> > > +plugins_sixaxis_la_LDFLAGS = -module -avoid-version -no-undefined @UDEV_LIBS@
> > > +plugins_sixaxis_la_CFLAGS = -fvisibility=hidden @DBUS_CFLAGS@ @GLIB_CFLAGS@ @UDEV_CFLAGS@
> > >  endif
> > > 
> > 
> > This is not enough, the external plugin can be compiled but I can be used
> > at runtime because it is using functions from src/storage.c and others,
> > which seem to be embedded inside bluetoothd only, so I get:
> > 
> > bluetoothd[23459]: Can't load plugin \
> >   /home/ao2/Proj/PS3/PS3_sixaxis/bluez/plugins/.libs/playstation-peripheral.so: \
> >   /home/ao2/Proj/PS3/PS3_sixaxis/bluez/plugins/.libs/playstation-peripheral.so: \
> >   undefined symbol: write_device_name
> > 
> > I could link the needed object files like storage.o and company into the
> > plugin binary, but definitely this doesn't look OK. Should a shared library
> > provide those "utility" functions to bluetoothd _and_ external plugins?
> > 
> > This plugin is the first non-trivial external plugin I can see in the tree,
> > so maybe this problem has never showed up before.
> 
> The problem is the visibility of the symbols, src/bluetooth.ver defines
> which symbols should[1] be visible to plugins. Every plugin should only
> depend on those methods from the "core".
>

Indeed, adding the needed symbols to the linker script makes the plugin
work:

diff --git a/src/bluetooth.ver b/src/bluetooth.ver
index b71c70d..11fcd33 100644
--- a/src/bluetooth.ver
+++ b/src/bluetooth.ver
@@ -5,6 +5,19 @@
                info;
                error;
                debug;
+               manager_get_default_adapter;
+               adapter_get_address;
+               write_device_name;
+               record_from_string;
+               store_record;
+               sdp_record_free;
+               store_device_id;
+               write_trust;
+               dbus_bus_get;
+               adapter_get_device;
+               device_set_temporary;
+               device_set_name;
+               dbus_connection_unref;
        local:
                *;
 };


> If we want to keep the sixaxis plugin external we need to make the
> symbols it need visible, but I wonder if that is right solution.
>

If there was a public API for plugins to use with well defined prefixes,
the linking scripts could use those, I am thinking to something like:
	sdp_*;
	storage_*;
	device_*;
	dbus_*;

But you bluez people have to tell me if this is the way to go and if
you are willing to do that, of if I'd better have the plugin builtin for
now.

> [1] Seems that with builtin plugins that visibility enforcement doesn't
> work, right?
>

This seems to be the case indeed, a bluiltin plugin can use any (non
static?) symbol linked in the bluetoothd binary.

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH BlueZ 4/4] plugins/sixaxis: Wait for the PS button before setting the LEDs

[Antonio Ospite]

Wait for actual input events, that is PS button has been pressed, before
setting the LEDs to indicate the controller number.
This makes setting LEDs look more like on the PS3.
---
 plugins/sixaxis.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
index 2b0616a..d64ad6d 100644
--- a/plugins/sixaxis.c
+++ b/plugins/sixaxis.c
@@ -511,8 +511,14 @@ static void handle_device_plug(struct udev_device *udevice)
 		sixpair(fd, adapter);
 	}
 
-	if (js_num > 0)
+	if (js_num > 0) {
+		char c;
+
+		/* wait for events before setting leds */
+		if (read(fd, &c, 1) != 1)
+			perror("read error");
 		set_controller_number(fd, js_num);
+	}
 
 	close(fd);
 }
-- 
1.7.5.4

[PATCH BlueZ 4/4] plugins/sixaxis: Wait for the PS button before setting the LEDs

[Antonio Ospite]

Wait for actual input events, that is PS button has been pressed, before
setting the LEDs to indicate the controller number.
This makes setting LEDs look more like on the PS3.
---
 plugins/sixaxis.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
index 2b0616a..d64ad6d 100644
--- a/plugins/sixaxis.c
+++ b/plugins/sixaxis.c
@@ -511,8 +511,14 @@ static void handle_device_plug(struct udev_device *udevice)
 		sixpair(fd, adapter);
 	}
 
-	if (js_num > 0)
+	if (js_num > 0) {
+		char c;
+
+		/* wait for events before setting leds */
+		if (read(fd, &c, 1) != 1)
+			perror("read error");
 		set_controller_number(fd, js_num);
+	}
 
 	close(fd);
 }
-- 
1.7.5.4

Re: [PATCH BlueZ 0/4] Sixaxis Plugin, almost there?

[Anderson Lizardo]

Hi,

On Fri, Aug 5, 2011 at 10:09 AM, Antonio Ospite
<ospite@studenti.unina.it> wrote:
>  - Fixed some style issue pointed out by checkpatch.pl from linux,
>    namely it suggested not to initialize static variables.

Except that... BlueZ code usually initializes static variables (not
sure if it is a specific coding style for BlueZ, or something else)

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil