All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv3 0/3] Sixaxis plugin for Bluez
@ 2011-06-08 13:09 Antonio Ospite
  2011-06-08 13:09   ` Antonio Ospite
                   ` (3 more replies)
  0 siblings, 4 replies; 21+ messages in thread
From: Antonio Ospite @ 2011-06-08 13:09 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: Antonio Ospite, Bastien Nocera, linux-input, Jim Paris,
	Ranulf Doswell, Pascal A . Brisset, Marcin Tolysz,
	Christian Birchinger, Filipe Lopes, Alan Ott, Mikko Virkkila,
	Simon Wood

Hi,

it is still not perfect but I don't want to hold this for too long, the 
remaining details can be fixed later.

Patches 1/3 and 2/3 are unchanged. 

Changes since v2 for patch 3/3:

  - Rebased on latest bluez from git.

  - Write the trust file earlier: if we write the trust file before 
    calling adapter_get_device() manual authorization is not asked to 
    the user anymore on the very first connection.

  - Move call to btd_device_add_uuid() inside 
    create_sixaxis_association(): there is no need for a 'struct 
    btd_device *device' variable to be out of that function.

  - Do not use newlines in DBG() calls.

  - Consider also "Sony Computer Entertainment Wireless Controller" when 
    detecting the Sixaxis, I get this identifier sometimes.

  - Add fallback ioctl definition to allow compilation with older kernel 
    headers, I tried running the plugin on older kernels too and it 
    fails gracefully. The configure-time check is now removed, we just check 
    udev to be enabled.

  - Print file and function in error() messages, maybe this can be done 
    globally, but for now I added it locally to know where the failures 
    where happening. 

  - Fix a mem leak in get_feature_report(): free(buf) in the error path.

  - Make set_master_bdaddr() return the int error value instead of a 
    boolean.

  - Don't print the "New Master Bluetooth address" message when we fail 
    to set it.


TODO:

  - When the controller is connected via USB the led is set, but then it 
    goes blinking again waiting for the PS button to be pressed, maybe 
    we should wait for the PS button before setting leds, maybe with a 
    simple blocking read() on the hidraw device.

  - When the controller is connected via USB after it is working over 
    BT, it is seen as a second controller and the second led it turned 
    on, should we force BT disconnection on USB connection?

  - Test with multiple controllers, and/or multiple BT adapters, which I 
    can't do right now.

A remainder, if anyone wants to take a look at USB dumps of pairing and 
led setting, they are here: http://ps3.jim.sh/sixaxis/dumps/
Maybe the gyro initialization is there too.

Thanks,
   Antonio

Antonio Ospite (2):
  Remove input/sixpair.c
  Add sixaxis plugin: USB pairing and LEDs settings

Bastien Nocera (1):
  Re-add manager_get_default_adapter()

 Makefile.am       |    9 +-
 acinclude.m4      |   10 +
 input/sixpair.c   |  299 ------------------------------
 plugins/sixaxis.c |  528 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/manager.c     |    5 +
 src/manager.h     |    1 +
 6 files changed, 551 insertions(+), 301 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?

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

* [PATCHv3 1/3] Remove input/sixpair.c
@ 2011-06-08 13:09   ` Antonio Ospite
  0 siblings, 0 replies; 21+ messages in thread
From: Antonio Ospite @ 2011-06-08 13:09 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: Antonio Ospite, Bastien Nocera, linux-input, Jim Paris,
	Ranulf Doswell, Pascal A . Brisset, Marcin Tolysz,
	Christian Birchinger, Filipe Lopes, Alan Ott, Mikko Virkkila,
	Simon Wood

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.

Signed-off-by: Antonio Ospite <ospite@studenti.unina.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.3


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

* [PATCHv3 1/3] Remove input/sixpair.c
@ 2011-06-08 13:09   ` Antonio Ospite
  0 siblings, 0 replies; 21+ messages in thread
From: Antonio Ospite @ 2011-06-08 13:09 UTC (permalink / raw)
  To: linux-bluetooth-u79uwXL29TY76Z2rM5mHXA
  Cc: Antonio Ospite, Bastien Nocera,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Jim Paris, Ranulf Doswell,
	Pascal A . Brisset, Marcin Tolysz, Christian Birchinger,
	Filipe Lopes, Alan Ott, Mikko Virkkila, Simon Wood

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.

Signed-off-by: Antonio Ospite <ospite-aNJ+ML1ZbiP93QAQaVx+gl6hYfS7NtTn@public.gmane.org>
---
 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.3

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

* [PATCHv3 2/3] Re-add manager_get_default_adapter()
  2011-06-08 13:09 [PATCHv3 0/3] Sixaxis plugin for Bluez Antonio Ospite
  2011-06-08 13:09   ` Antonio Ospite
@ 2011-06-08 13:09 ` Antonio Ospite
  2011-06-09 14:02   ` Bastien Nocera
  2011-06-15 20:53   ` Johan Hedberg
  2011-06-08 13:09   ` Antonio Ospite
  2011-06-08 14:33   ` Bastien Nocera
  3 siblings, 2 replies; 21+ messages in thread
From: Antonio Ospite @ 2011-06-08 13:09 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: Bastien Nocera, linux-input, Jim Paris, Ranulf Doswell,
	Pascal A . Brisset, Marcin Tolysz, Christian Birchinger,
	Filipe Lopes, Alan Ott, Mikko Virkkila, Simon Wood,
	Antonio Ospite

From: Bastien Nocera <hadess@hadess.net>

Signed-off-by: Bastien Nocera <hadess@hadess.net>
Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 src/manager.c |    5 +++++
 src/manager.h |    1 +
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/src/manager.c b/src/manager.c
index e805e0c..254ace4 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -262,6 +262,11 @@ static void manager_set_default_adapter(int id)
 			DBUS_TYPE_INVALID);
 }
 
+struct btd_adapter *manager_get_default_adapter(void)
+{
+	return manager_find_adapter_by_id(default_adapter_id);
+}
+
 static void manager_remove_adapter(struct btd_adapter *adapter)
 {
 	uint16_t dev_id = adapter_get_dev_id(adapter);
diff --git a/src/manager.h b/src/manager.h
index 05c38b3..4f92d2f 100644
--- a/src/manager.h
+++ b/src/manager.h
@@ -35,6 +35,7 @@ void manager_cleanup(DBusConnection *conn, const char *path);
 const char *manager_get_base_path(void);
 struct btd_adapter *manager_find_adapter(const bdaddr_t *sba);
 struct btd_adapter *manager_find_adapter_by_id(int id);
+struct btd_adapter *manager_get_default_adapter(void);
 void manager_foreach_adapter(adapter_cb func, gpointer user_data);
 GSList *manager_get_adapters(void);
 struct btd_adapter *btd_manager_register_adapter(int id);
-- 
1.7.5.3


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

* [PATCHv3 3/3] Add sixaxis plugin: USB pairing and LEDs settings
  2011-06-08 13:09 [PATCHv3 0/3] Sixaxis plugin for Bluez Antonio Ospite
@ 2011-06-08 13:09   ` Antonio Ospite
  2011-06-08 13:09 ` [PATCHv3 2/3] Re-add manager_get_default_adapter() Antonio Ospite
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 21+ messages in thread
From: Antonio Ospite @ 2011-06-08 13:09 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: Antonio Ospite, Bastien Nocera, linux-input, Jim Paris,
	Ranulf Doswell, Pascal A . Brisset, Marcin Tolysz,
	Christian Birchinger, Filipe Lopes, Alan Ott, Mikko Virkkila,
	Simon Wood

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 |  528 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 545 insertions(+), 2 deletions(-)
 create mode 100644 plugins/sixaxis.c

diff --git a/Makefile.am b/Makefile.am
index 4659c80..0d567a9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -202,6 +202,11 @@ builtin_sources += health/hdp_main.c health/hdp_types.h \
 			health/hdp_util.h health/hdp_util.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
 
@@ -253,7 +258,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
 
@@ -370,7 +375,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 d77937b..d8e4ba9 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -183,6 +183,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
@@ -277,6 +278,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}
 	])
@@ -372,6 +377,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" ||
@@ -406,4 +415,5 @@ AC_DEFUN([AC_ARG_BLUEZ], [
 	AM_CONDITIONAL(CONFIGFILES, test "${configfiles_enable}" = "yes")
 	AM_CONDITIONAL(MAEMO6PLUGIN, test "${maemo6_enable}" = "yes")
 	AM_CONDITIONAL(DBUSOOBPLUGIN, test "${dbusoob_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..70f81f4
--- /dev/null
+++ b/plugins/sixaxis.c
@@ -0,0 +1,528 @@
+/*
+ * 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 <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"
+
+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, 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 = -1;
+		goto fail_dbus;
+	}
+
+	device = adapter_get_device(conn, adapter, address);
+	if (device == NULL) {
+		DBG("Failed to get the device");
+		ret = -1;
+		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 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 FALSE;
+	}
+
+	report = malloc(8);
+	if (report == NULL) {
+		error("%s:%s() malloc failed", __FILE__, __func__);
+		return FALSE;
+	}
+
+	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 adapter_bdaddr[18];
+	bdaddr_t dst;
+	int ret = 0;
+
+	adapter_get_address(adapter, &dst);
+	ba2str(&dst, adapter_bdaddr);
+	DBG("Adapter bdaddr %s", adapter_bdaddr);
+
+	ret = set_master_bdaddr(fd, adapter_bdaddr);
+	if (ret < 0) {
+		DBG("Failed to set the master Bluetooth address");
+		return ret;
+	}
+
+	device_bdaddr = get_device_bdaddr(fd);
+	if (device_bdaddr == NULL) {
+		DBG("Failed to get the Bluetooth address from the device");
+		return -1;
+	}
+
+	DBG("Device bdaddr %s", device_bdaddr);
+
+	ret = create_sixaxis_association(adapter,
+	                                 SIXAXIS_NAME,
+	                                 device_bdaddr,
+	                                 VENDOR, PRODUCT, SIXAXIS_PNP_RECORD);
+	free(device_bdaddr);
+	return ret;
+}
+
+
+/* Led setting section */
+#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 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 really fast")
+	 * |     |     ??? (Some form of phase shift or duty_length multiplier?)
+	 * |     |     |     % of duty_length the led is off (0xff means 100%)
+	 * |     |     |     |     % of duty_length the led is on (0xff mean 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 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_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;
+	}
+
+	DBG("name: %s", udev_device_get_property_value(hid_parent, "HID_NAME"));
+
+	if (!(g_str_has_suffix(udev_device_get_property_value(hid_parent, "HID_NAME"),
+	                     "PLAYSTATION(R)3 Controller") ||
+	    g_str_has_suffix(udev_device_get_property_value(hid_parent, "HID_NAME"),
+	                     "Sony Computer Entertainment Wireless Controller"))
+	    )
+		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) {
+			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",
+			     udev_device_get_property_value(js_dev, "ID_USB_DRIVER"));
+
+			if (g_strcmp0(udev_device_get_property_value(js_dev, "ID_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 (js_num > 0)
+		set_controller_number(fd, js_num);
+	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);
+	}
+
+	close(fd);
+}
+
+static gboolean device_event_idle(struct udev_device *udevice)
+{
+	handle_device_plug(udevice);
+	udev_device_unref(udevice);
+	return FALSE;
+}
+
+static struct udev *ctx = NULL;
+static struct udev_monitor *monitor = NULL;
+static guint watch_id = 0;
+
+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;
+	}
+
+	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.3


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

* [PATCHv3 3/3] Add sixaxis plugin: USB pairing and LEDs settings
@ 2011-06-08 13:09   ` Antonio Ospite
  0 siblings, 0 replies; 21+ messages in thread
From: Antonio Ospite @ 2011-06-08 13:09 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: Antonio Ospite, Bastien Nocera, linux-input, Jim Paris,
	Ranulf Doswell, Pascal A . Brisset, Marcin Tolysz,
	Christian Birchinger, Filipe Lopes, Alan Ott, Mikko Virkkila,
	Simon Wood

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 |  528 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 545 insertions(+), 2 deletions(-)
 create mode 100644 plugins/sixaxis.c

diff --git a/Makefile.am b/Makefile.am
index 4659c80..0d567a9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -202,6 +202,11 @@ builtin_sources += health/hdp_main.c health/hdp_types.h \
 			health/hdp_util.h health/hdp_util.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
 
@@ -253,7 +258,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
 
@@ -370,7 +375,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 d77937b..d8e4ba9 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -183,6 +183,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
@@ -277,6 +278,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}
 	])
@@ -372,6 +377,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" ||
@@ -406,4 +415,5 @@ AC_DEFUN([AC_ARG_BLUEZ], [
 	AM_CONDITIONAL(CONFIGFILES, test "${configfiles_enable}" = "yes")
 	AM_CONDITIONAL(MAEMO6PLUGIN, test "${maemo6_enable}" = "yes")
 	AM_CONDITIONAL(DBUSOOBPLUGIN, test "${dbusoob_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..70f81f4
--- /dev/null
+++ b/plugins/sixaxis.c
@@ -0,0 +1,528 @@
+/*
+ * 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 <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"
+
+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, 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 = -1;
+		goto fail_dbus;
+	}
+
+	device = adapter_get_device(conn, adapter, address);
+	if (device == NULL) {
+		DBG("Failed to get the device");
+		ret = -1;
+		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 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 FALSE;
+	}
+
+	report = malloc(8);
+	if (report == NULL) {
+		error("%s:%s() malloc failed", __FILE__, __func__);
+		return FALSE;
+	}
+
+	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 adapter_bdaddr[18];
+	bdaddr_t dst;
+	int ret = 0;
+
+	adapter_get_address(adapter, &dst);
+	ba2str(&dst, adapter_bdaddr);
+	DBG("Adapter bdaddr %s", adapter_bdaddr);
+
+	ret = set_master_bdaddr(fd, adapter_bdaddr);
+	if (ret < 0) {
+		DBG("Failed to set the master Bluetooth address");
+		return ret;
+	}
+
+	device_bdaddr = get_device_bdaddr(fd);
+	if (device_bdaddr == NULL) {
+		DBG("Failed to get the Bluetooth address from the device");
+		return -1;
+	}
+
+	DBG("Device bdaddr %s", device_bdaddr);
+
+	ret = create_sixaxis_association(adapter,
+	                                 SIXAXIS_NAME,
+	                                 device_bdaddr,
+	                                 VENDOR, PRODUCT, SIXAXIS_PNP_RECORD);
+	free(device_bdaddr);
+	return ret;
+}
+
+
+/* Led setting section */
+#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 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 really fast")
+	 * |     |     ??? (Some form of phase shift or duty_length multiplier?)
+	 * |     |     |     % of duty_length the led is off (0xff means 100%)
+	 * |     |     |     |     % of duty_length the led is on (0xff mean 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 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_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;
+	}
+
+	DBG("name: %s", udev_device_get_property_value(hid_parent, "HID_NAME"));
+
+	if (!(g_str_has_suffix(udev_device_get_property_value(hid_parent, "HID_NAME"),
+	                     "PLAYSTATION(R)3 Controller") ||
+	    g_str_has_suffix(udev_device_get_property_value(hid_parent, "HID_NAME"),
+	                     "Sony Computer Entertainment Wireless Controller"))
+	    )
+		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) {
+			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",
+			     udev_device_get_property_value(js_dev, "ID_USB_DRIVER"));
+
+			if (g_strcmp0(udev_device_get_property_value(js_dev, "ID_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 (js_num > 0)
+		set_controller_number(fd, js_num);
+	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);
+	}
+
+	close(fd);
+}
+
+static gboolean device_event_idle(struct udev_device *udevice)
+{
+	handle_device_plug(udevice);
+	udev_device_unref(udevice);
+	return FALSE;
+}
+
+static struct udev *ctx = NULL;
+static struct udev_monitor *monitor = NULL;
+static guint watch_id = 0;
+
+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;
+	}
+
+	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.3


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

* Re: [PATCHv3 0/3] Sixaxis plugin for Bluez
@ 2011-06-08 14:33   ` Bastien Nocera
  0 siblings, 0 replies; 21+ messages in thread
From: Bastien Nocera @ 2011-06-08 14:33 UTC (permalink / raw)
  To: Antonio Ospite
  Cc: linux-bluetooth, linux-input, Jim Paris, Ranulf Doswell,
	Pascal A . Brisset, Marcin Tolysz, Christian Birchinger,
	Filipe Lopes, Alan Ott, Mikko Virkkila, Simon Wood

On Wed, 2011-06-08 at 15:09 +0200, Antonio Ospite wrote:
> 
>   - When the controller is connected via USB the led is set, but then
> it 
>     goes blinking again waiting for the PS button to be pressed,
> maybe 
>     we should wait for the PS button before setting leds, maybe with
> a 
>     simple blocking read() on the hidraw device.

One hard thing to handle is when the device is already in use via USB,
it should automatically connect via Bluetooth when unplugged. I'm not
sure this works right now (I'm a bit hazy on the interaction with the PS
button tbh).

>   - When the controller is connected via USB after it is working over 
>     BT, it is seen as a second controller and the second led it
> turned 
>     on, should we force BT disconnection on USB connection? 

As we can identify the device uniquely through its bdaddr, I'd say yes.


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

* Re: [PATCHv3 0/3] Sixaxis plugin for Bluez
@ 2011-06-08 14:33   ` Bastien Nocera
  0 siblings, 0 replies; 21+ messages in thread
From: Bastien Nocera @ 2011-06-08 14:33 UTC (permalink / raw)
  To: Antonio Ospite
  Cc: linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Jim Paris, Ranulf Doswell,
	Pascal A . Brisset, Marcin Tolysz, Christian Birchinger,
	Filipe Lopes, Alan Ott, Mikko Virkkila, Simon Wood

On Wed, 2011-06-08 at 15:09 +0200, Antonio Ospite wrote:
> 
>   - When the controller is connected via USB the led is set, but then
> it 
>     goes blinking again waiting for the PS button to be pressed,
> maybe 
>     we should wait for the PS button before setting leds, maybe with
> a 
>     simple blocking read() on the hidraw device.

One hard thing to handle is when the device is already in use via USB,
it should automatically connect via Bluetooth when unplugged. I'm not
sure this works right now (I'm a bit hazy on the interaction with the PS
button tbh).

>   - When the controller is connected via USB after it is working over 
>     BT, it is seen as a second controller and the second led it
> turned 
>     on, should we force BT disconnection on USB connection? 

As we can identify the device uniquely through its bdaddr, I'd say yes.

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

* Re: [PATCHv3 3/3] Add sixaxis plugin: USB pairing and LEDs settings
  2011-06-08 13:09   ` Antonio Ospite
@ 2011-06-08 17:52     ` Vinicius Costa Gomes
  -1 siblings, 0 replies; 21+ messages in thread
From: Vinicius Costa Gomes @ 2011-06-08 17:52 UTC (permalink / raw)
  To: Antonio Ospite
  Cc: linux-bluetooth, Bastien Nocera, linux-input, Jim Paris,
	Ranulf Doswell, Pascal A . Brisset, Marcin Tolysz,
	Christian Birchinger, Filipe Lopes, Alan Ott, Mikko Virkkila,
	Simon Wood

Hi Antonio,

On 15:09 Wed 08 Jun, 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)

Just a few (mostly cosmetic) comments.

> 
> 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 |  528 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 545 insertions(+), 2 deletions(-)
>  create mode 100644 plugins/sixaxis.c
> 
> diff --git a/Makefile.am b/Makefile.am
> index 4659c80..0d567a9 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -202,6 +202,11 @@ builtin_sources += health/hdp_main.c health/hdp_types.h \
>  			health/hdp_util.h health/hdp_util.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
>  
> @@ -253,7 +258,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
>  
> @@ -370,7 +375,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 d77937b..d8e4ba9 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -183,6 +183,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
> @@ -277,6 +278,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}
>  	])
> @@ -372,6 +377,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" ||
> @@ -406,4 +415,5 @@ AC_DEFUN([AC_ARG_BLUEZ], [
>  	AM_CONDITIONAL(CONFIGFILES, test "${configfiles_enable}" = "yes")
>  	AM_CONDITIONAL(MAEMO6PLUGIN, test "${maemo6_enable}" = "yes")
>  	AM_CONDITIONAL(DBUSOOBPLUGIN, test "${dbusoob_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..70f81f4
> --- /dev/null
> +++ b/plugins/sixaxis.c
> @@ -0,0 +1,528 @@
> +/*
> + * 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 <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"
> +
> +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)

Use only tabs for indentation.

> +{
> +	DBusConnection *conn;
> +	sdp_record_t *rec;
> +	struct btd_device *device;
> +	bdaddr_t src, dst;
> +	char srcaddr[18];
> +	int ret = 0;

The most common idiom is to call this kind of variable 'err'.

> +
> +	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, 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 = -1;

If possible, use a more descriptive error number, -EPERM perhaps?

> +		goto fail_dbus;
> +	}
> +
> +	device = adapter_get_device(conn, adapter, address);
> +	if (device == NULL) {
> +		DBG("Failed to get the device");
> +		ret = -1;

Same here.

> +		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;
> +}
> +
> +

Extra empty line here.

> +/* Usb cable pairing section */
> +static unsigned char *get_feature_report(int fd, uint8_t report_number, unsigned int len)

Try to keep lines under the 78 characters limit. There are more places like
this, I might have missed some.

> +{
> +	unsigned char *buf;
> +	int ret;
> +
> +	buf = calloc(len, sizeof(*buf));

Usually in BlueZ code we try to use the allocations function provided by
GLib, g_malloc0() in this case.

> +	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));

Same here.

> +	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]);

Just tabs for indentation.

> +
> +	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]);

Same here.

> +	if (ret != 6) {
> +

Extra empty line.

> +		error("%s:%s() Parsing the bt address failed", __FILE__, __func__);
> +		return FALSE;
> +	}
> +
> +	report = malloc(8);

g_malloc0()?

> +	if (report == NULL) {
> +		error("%s:%s() malloc failed", __FILE__, __func__);
> +		return FALSE;
> +	}
> +
> +	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 adapter_bdaddr[18];
> +	bdaddr_t dst;
> +	int ret = 0;

Call this 'err'.

> +
> +	adapter_get_address(adapter, &dst);
> +	ba2str(&dst, adapter_bdaddr);
> +	DBG("Adapter bdaddr %s", adapter_bdaddr);
> +
> +	ret = set_master_bdaddr(fd, adapter_bdaddr);
> +	if (ret < 0) {
> +		DBG("Failed to set the master Bluetooth address");
> +		return ret;
> +	}
> +
> +	device_bdaddr = get_device_bdaddr(fd);
> +	if (device_bdaddr == NULL) {
> +		DBG("Failed to get the Bluetooth address from the device");
> +		return -1;
> +	}
> +
> +	DBG("Device bdaddr %s", device_bdaddr);
> +
> +	ret = create_sixaxis_association(adapter,
> +	                                 SIXAXIS_NAME,
> +	                                 device_bdaddr,
> +	                                 VENDOR, PRODUCT, SIXAXIS_PNP_RECORD);

Spaces for indentation.

> +	free(device_bdaddr);
> +	return ret;
> +}
> +

Extra empty line.

> +
> +/* Led setting section */
> +#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
> +

Move these declarations to the top.

> +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 really fast")
> +	 * |     |     ??? (Some form of phase shift or duty_length multiplier?)
> +	 * |     |     |     % of duty_length the led is off (0xff means 100%)
> +	 * |     |     |     |     % of duty_length the led is on (0xff mean 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 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_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;
> +	}
> +
> +	DBG("name: %s", udev_device_get_property_value(hid_parent, "HID_NAME"));
> +
> +	if (!(g_str_has_suffix(udev_device_get_property_value(hid_parent, "HID_NAME"),
> +	                     "PLAYSTATION(R)3 Controller") ||
> +	    g_str_has_suffix(udev_device_get_property_value(hid_parent, "HID_NAME"),
> +	                     "Sony Computer Entertainment Wireless Controller"))
> +	    )

The coding style here is confusing, and I think that if you add a variable to
store the value of 'udev_get_property_value(hid_parent, "HID_NAME")' things
would look simpler.

> +		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) {
> +			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",
> +			     udev_device_get_property_value(js_dev, "ID_USB_DRIVER"));
> +
> +			if (g_strcmp0(udev_device_get_property_value(js_dev, "ID_USB_DRIVER"),
> +			              "usbhid") == 0)

I guess you already know what I am going to say ;-)

> +				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 (js_num > 0)
> +		set_controller_number(fd, js_num);
> +	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);
> +	}
> +
> +	close(fd);
> +}
> +
> +static gboolean device_event_idle(struct udev_device *udevice)
> +{
> +	handle_device_plug(udevice);
> +	udev_device_unref(udevice);
> +	return FALSE;
> +}
> +
> +static struct udev *ctx = NULL;
> +static struct udev_monitor *monitor = NULL;
> +static guint watch_id = 0;

Move these to the top, globals should be really visible.

> +
> +static gboolean
> +monitor_event(GIOChannel *source,
> +	      GIOCondition condition,
> +	      gpointer data)

These declaration isn't consistent with the rest of the BlueZ coding style.

> +{
> +	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;
> +	}
> +
> +	g_timeout_add_seconds(1, (GSourceFunc) device_event_idle, udevice);

Just a question: why is this delay between finding the device and plug'ing it
is needed? Perhaps a short comment explaining why it is needed would be nice.

> +
> +out:
> +	return TRUE;
> +}
> +
> +

Extra empty line.

> +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.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers,
-- 
Vinicius

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

* Re: [PATCHv3 3/3] Add sixaxis plugin: USB pairing and LEDs settings
@ 2011-06-08 17:52     ` Vinicius Costa Gomes
  0 siblings, 0 replies; 21+ messages in thread
From: Vinicius Costa Gomes @ 2011-06-08 17:52 UTC (permalink / raw)
  To: Antonio Ospite
  Cc: linux-bluetooth, Bastien Nocera, linux-input, Jim Paris,
	Ranulf Doswell, Pascal A . Brisset, Marcin Tolysz,
	Christian Birchinger, Filipe Lopes, Alan Ott, Mikko Virkkila,
	Simon Wood

Hi Antonio,

On 15:09 Wed 08 Jun, 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)

Just a few (mostly cosmetic) comments.

> 
> 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 |  528 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 545 insertions(+), 2 deletions(-)
>  create mode 100644 plugins/sixaxis.c
> 
> diff --git a/Makefile.am b/Makefile.am
> index 4659c80..0d567a9 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -202,6 +202,11 @@ builtin_sources += health/hdp_main.c health/hdp_types.h \
>  			health/hdp_util.h health/hdp_util.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
>  
> @@ -253,7 +258,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
>  
> @@ -370,7 +375,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 d77937b..d8e4ba9 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -183,6 +183,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
> @@ -277,6 +278,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}
>  	])
> @@ -372,6 +377,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" ||
> @@ -406,4 +415,5 @@ AC_DEFUN([AC_ARG_BLUEZ], [
>  	AM_CONDITIONAL(CONFIGFILES, test "${configfiles_enable}" = "yes")
>  	AM_CONDITIONAL(MAEMO6PLUGIN, test "${maemo6_enable}" = "yes")
>  	AM_CONDITIONAL(DBUSOOBPLUGIN, test "${dbusoob_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..70f81f4
> --- /dev/null
> +++ b/plugins/sixaxis.c
> @@ -0,0 +1,528 @@
> +/*
> + * 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 <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"
> +
> +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)

Use only tabs for indentation.

> +{
> +	DBusConnection *conn;
> +	sdp_record_t *rec;
> +	struct btd_device *device;
> +	bdaddr_t src, dst;
> +	char srcaddr[18];
> +	int ret = 0;

The most common idiom is to call this kind of variable 'err'.

> +
> +	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, 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 = -1;

If possible, use a more descriptive error number, -EPERM perhaps?

> +		goto fail_dbus;
> +	}
> +
> +	device = adapter_get_device(conn, adapter, address);
> +	if (device == NULL) {
> +		DBG("Failed to get the device");
> +		ret = -1;

Same here.

> +		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;
> +}
> +
> +

Extra empty line here.

> +/* Usb cable pairing section */
> +static unsigned char *get_feature_report(int fd, uint8_t report_number, unsigned int len)

Try to keep lines under the 78 characters limit. There are more places like
this, I might have missed some.

> +{
> +	unsigned char *buf;
> +	int ret;
> +
> +	buf = calloc(len, sizeof(*buf));

Usually in BlueZ code we try to use the allocations function provided by
GLib, g_malloc0() in this case.

> +	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));

Same here.

> +	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]);

Just tabs for indentation.

> +
> +	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]);

Same here.

> +	if (ret != 6) {
> +

Extra empty line.

> +		error("%s:%s() Parsing the bt address failed", __FILE__, __func__);
> +		return FALSE;
> +	}
> +
> +	report = malloc(8);

g_malloc0()?

> +	if (report == NULL) {
> +		error("%s:%s() malloc failed", __FILE__, __func__);
> +		return FALSE;
> +	}
> +
> +	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 adapter_bdaddr[18];
> +	bdaddr_t dst;
> +	int ret = 0;

Call this 'err'.

> +
> +	adapter_get_address(adapter, &dst);
> +	ba2str(&dst, adapter_bdaddr);
> +	DBG("Adapter bdaddr %s", adapter_bdaddr);
> +
> +	ret = set_master_bdaddr(fd, adapter_bdaddr);
> +	if (ret < 0) {
> +		DBG("Failed to set the master Bluetooth address");
> +		return ret;
> +	}
> +
> +	device_bdaddr = get_device_bdaddr(fd);
> +	if (device_bdaddr == NULL) {
> +		DBG("Failed to get the Bluetooth address from the device");
> +		return -1;
> +	}
> +
> +	DBG("Device bdaddr %s", device_bdaddr);
> +
> +	ret = create_sixaxis_association(adapter,
> +	                                 SIXAXIS_NAME,
> +	                                 device_bdaddr,
> +	                                 VENDOR, PRODUCT, SIXAXIS_PNP_RECORD);

Spaces for indentation.

> +	free(device_bdaddr);
> +	return ret;
> +}
> +

Extra empty line.

> +
> +/* Led setting section */
> +#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
> +

Move these declarations to the top.

> +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 really fast")
> +	 * |     |     ??? (Some form of phase shift or duty_length multiplier?)
> +	 * |     |     |     % of duty_length the led is off (0xff means 100%)
> +	 * |     |     |     |     % of duty_length the led is on (0xff mean 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 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_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;
> +	}
> +
> +	DBG("name: %s", udev_device_get_property_value(hid_parent, "HID_NAME"));
> +
> +	if (!(g_str_has_suffix(udev_device_get_property_value(hid_parent, "HID_NAME"),
> +	                     "PLAYSTATION(R)3 Controller") ||
> +	    g_str_has_suffix(udev_device_get_property_value(hid_parent, "HID_NAME"),
> +	                     "Sony Computer Entertainment Wireless Controller"))
> +	    )

The coding style here is confusing, and I think that if you add a variable to
store the value of 'udev_get_property_value(hid_parent, "HID_NAME")' things
would look simpler.

> +		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) {
> +			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",
> +			     udev_device_get_property_value(js_dev, "ID_USB_DRIVER"));
> +
> +			if (g_strcmp0(udev_device_get_property_value(js_dev, "ID_USB_DRIVER"),
> +			              "usbhid") == 0)

I guess you already know what I am going to say ;-)

> +				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 (js_num > 0)
> +		set_controller_number(fd, js_num);
> +	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);
> +	}
> +
> +	close(fd);
> +}
> +
> +static gboolean device_event_idle(struct udev_device *udevice)
> +{
> +	handle_device_plug(udevice);
> +	udev_device_unref(udevice);
> +	return FALSE;
> +}
> +
> +static struct udev *ctx = NULL;
> +static struct udev_monitor *monitor = NULL;
> +static guint watch_id = 0;

Move these to the top, globals should be really visible.

> +
> +static gboolean
> +monitor_event(GIOChannel *source,
> +	      GIOCondition condition,
> +	      gpointer data)

These declaration isn't consistent with the rest of the BlueZ coding style.

> +{
> +	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;
> +	}
> +
> +	g_timeout_add_seconds(1, (GSourceFunc) device_event_idle, udevice);

Just a question: why is this delay between finding the device and plug'ing it
is needed? Perhaps a short comment explaining why it is needed would be nice.

> +
> +out:
> +	return TRUE;
> +}
> +
> +

Extra empty line.

> +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.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers,
-- 
Vinicius

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

* Re: [PATCHv3 3/3] Add sixaxis plugin: USB pairing and LEDs settings
  2011-06-08 17:52     ` Vinicius Costa Gomes
  (?)
@ 2011-06-09 13:57     ` Antonio Ospite
  2011-06-09 15:30       ` Anderson Lizardo
  -1 siblings, 1 reply; 21+ messages in thread
From: Antonio Ospite @ 2011-06-09 13:57 UTC (permalink / raw)
  To: Vinicius Costa Gomes
  Cc: linux-bluetooth, Bastien Nocera, linux-input, Jim Paris,
	Ranulf Doswell, Pascal A . Brisset, Marcin Tolysz,
	Christian Birchinger, Filipe Lopes, Alan Ott, Mikko Virkkila,
	Simon Wood

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

On Wed, 8 Jun 2011 14:52:51 -0300
Vinicius Costa Gomes <vinicius.gomes@openbossa.org> wrote:

> Hi Antonio,
> 
> On 15:09 Wed 08 Jun, 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)
> 
> Just a few (mostly cosmetic) comments.
>

OK, it's the classic indentation/alignment issue, I used spaces for
_alignment_ not indentation (not very consistently, I have to admit),
but I am just going to comply to BLueZ style, we don't need any more
time wasted on these debates :)

And I am going to keep lines shorter than 78 chars as requested.
Basically this is what checkpatch.pl from linux complains about.

About the double newlines, I used them to separate logical sections of
the code:
 0. Preamble (license header, includes, defines, etc.)
 1. BT association
 2. Usb cable pairing
 3. Led setting (That's also why some defines were here, I considered
    them "local" to this section)
 4. Udev loop
 5. Plugin framework stuff

They are more visible when the editor folds functions bodies, but I
have no problem about removing them if you like so.

Some further comments below, I left out all the parts where I agree and
I have nothing more to say.

> > +{
> > +	DBusConnection *conn;
> > +	sdp_record_t *rec;
> > +	struct btd_device *device;
> > +	bdaddr_t src, dst;
> > +	char srcaddr[18];
> > +	int ret = 0;
> 
> The most common idiom is to call this kind of variable 'err'.
>

OK on that too, just for the records I tend to use 'err' when I
check != 0 [like in if (err) {...] I use 'ret' when I check for < 0.

[...]
> > +	buf = calloc(len, sizeof(*buf));
> 
> Usually in BlueZ code we try to use the allocations function provided by
> GLib, g_malloc0() in this case.
>

[...]
> > +
> > +	address = calloc(BDADDR_STR_SIZE, sizeof(*address));
> 
> Same here.
>

[...]
> > +		error("%s:%s() Parsing the bt address failed", __FILE__, __func__);
> > +		return FALSE;
> > +	}
> > +
> > +	report = malloc(8);
> 
> g_malloc0()?
> 

Would you mind if I keep these three stdlib ones in? I use this code
verbatim in another program which does not link against Glib.

> > +{
> > +	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;
> > +	}
> > +
> > +	g_timeout_add_seconds(1, (GSourceFunc) device_event_idle, udevice);
> 
> Just a question: why is this delay between finding the device and plug'ing it
> is needed? Perhaps a short comment explaining why it is needed would be nice.
> 

/* Give UDEV some time to load kernel modules */
I guess this is the main reason, fact is that without that it does not
always work: we may get a null ID_USB_DRIVER.

> Cheers,
> -- 
> Vinicius
> 

Thanks for the review,
   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 --]

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

* Re: [PATCHv3 2/3] Re-add manager_get_default_adapter()
  2011-06-08 13:09 ` [PATCHv3 2/3] Re-add manager_get_default_adapter() Antonio Ospite
@ 2011-06-09 14:02   ` Bastien Nocera
  2011-06-09 15:00     ` Antonio Ospite
  2011-06-15 20:53   ` Johan Hedberg
  1 sibling, 1 reply; 21+ messages in thread
From: Bastien Nocera @ 2011-06-09 14:02 UTC (permalink / raw)
  To: Antonio Ospite
  Cc: linux-bluetooth, linux-input, Jim Paris, Ranulf Doswell,
	Pascal A . Brisset, Marcin Tolysz, Christian Birchinger,
	Filipe Lopes, Alan Ott, Mikko Virkkila, Simon Wood

On Wed, 2011-06-08 at 15:09 +0200, Antonio Ospite wrote:
> From: Bastien Nocera <hadess@hadess.net>

Sent out a new patch for that which allows the same functionality, but
returns an ID instead (you can then get the btd_adapter through existing
manager functions).


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

* Re: [PATCHv3 0/3] Sixaxis plugin for Bluez
@ 2011-06-09 14:54     ` Antonio Ospite
  0 siblings, 0 replies; 21+ messages in thread
From: Antonio Ospite @ 2011-06-09 14:54 UTC (permalink / raw)
  To: Bastien Nocera
  Cc: linux-bluetooth, linux-input, Jim Paris, Ranulf Doswell,
	Pascal A . Brisset, Marcin Tolysz, Christian Birchinger,
	Filipe Lopes, Alan Ott, Mikko Virkkila, Simon Wood

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

On Wed, 08 Jun 2011 15:33:42 +0100
Bastien Nocera <hadess@hadess.net> wrote:

> On Wed, 2011-06-08 at 15:09 +0200, Antonio Ospite wrote:
> > 
> >  - When the controller is connected via USB the led is set, but then 
> >  it goes blinking again waiting for the PS button to be pressed, 
> >  maybe we should wait for the PS button before setting leds, maybe  
> >  with a simple blocking read() on the hidraw device.
> 
> One hard thing to handle is when the device is already in use via USB,
> it should automatically connect via Bluetooth when unplugged. I'm not
> sure this works right now (I'm a bit hazy on the interaction with the 
> PS button tbh).
> 

Oh, this works if the user already pressed the PS button when connected 
via USB, it is just the LED setting which is still weak on USB
connection (before and after PS button) because of the PS button
requirement.

BTW Bastien I'd really appreciate a run-time test by other people
before this gets is :)

> >  - When the controller is connected via USB after it is working over 
> >  BT, it is seen as a second controller and the second led it turned 
> >  on, should we force BT disconnection on USB connection? 
> 
> As we can identify the device uniquely through its bdaddr, I'd say 
> yes.
> 

I had done some experiment about that, but I didn't find a good way 
using the bluez API, I'll be back on that after the first iteration
goes in, OK? I guess one problem will be about the numbering of 
/dev/input/jsX, can we disconnect the BT device _before_ the kernel 
enumerates the USB one?

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 --]

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

* Re: [PATCHv3 0/3] Sixaxis plugin for Bluez
@ 2011-06-09 14:54     ` Antonio Ospite
  0 siblings, 0 replies; 21+ messages in thread
From: Antonio Ospite @ 2011-06-09 14:54 UTC (permalink / raw)
  To: Bastien Nocera
  Cc: linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Jim Paris, Ranulf Doswell,
	Pascal A . Brisset, Marcin Tolysz, Christian Birchinger,
	Filipe Lopes, Alan Ott, Mikko Virkkila, Simon Wood

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

On Wed, 08 Jun 2011 15:33:42 +0100
Bastien Nocera <hadess-0MeiytkfxGOsTnJN9+BGXg@public.gmane.org> wrote:

> On Wed, 2011-06-08 at 15:09 +0200, Antonio Ospite wrote:
> > 
> >  - When the controller is connected via USB the led is set, but then 
> >  it goes blinking again waiting for the PS button to be pressed, 
> >  maybe we should wait for the PS button before setting leds, maybe  
> >  with a simple blocking read() on the hidraw device.
> 
> One hard thing to handle is when the device is already in use via USB,
> it should automatically connect via Bluetooth when unplugged. I'm not
> sure this works right now (I'm a bit hazy on the interaction with the 
> PS button tbh).
> 

Oh, this works if the user already pressed the PS button when connected 
via USB, it is just the LED setting which is still weak on USB
connection (before and after PS button) because of the PS button
requirement.

BTW Bastien I'd really appreciate a run-time test by other people
before this gets is :)

> >  - When the controller is connected via USB after it is working over 
> >  BT, it is seen as a second controller and the second led it turned 
> >  on, should we force BT disconnection on USB connection? 
> 
> As we can identify the device uniquely through its bdaddr, I'd say 
> yes.
> 

I had done some experiment about that, but I didn't find a good way 
using the bluez API, I'll be back on that after the first iteration
goes in, OK? I guess one problem will be about the numbering of 
/dev/input/jsX, can we disconnect the BT device _before_ the kernel 
enumerates the USB one?

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 --]

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

* Re: [PATCHv3 2/3] Re-add manager_get_default_adapter()
  2011-06-09 14:02   ` Bastien Nocera
@ 2011-06-09 15:00     ` Antonio Ospite
  2011-06-09 15:10       ` Bastien Nocera
  0 siblings, 1 reply; 21+ messages in thread
From: Antonio Ospite @ 2011-06-09 15:00 UTC (permalink / raw)
  To: Bastien Nocera
  Cc: linux-bluetooth, linux-input, Jim Paris, Ranulf Doswell,
	Pascal A . Brisset, Marcin Tolysz, Christian Birchinger,
	Filipe Lopes, Alan Ott, Mikko Virkkila, Simon Wood

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

On Thu, 09 Jun 2011 15:02:32 +0100
Bastien Nocera <hadess@hadess.net> wrote:

> On Wed, 2011-06-08 at 15:09 +0200, Antonio Ospite wrote:
> > From: Bastien Nocera <hadess@hadess.net>
> 
> Sent out a new patch for that which allows the same functionality, but
> returns an ID instead (you can then get the btd_adapter through existing
> manager functions).

OK, so basically I drop 2/3 and do:
adapter = manager_find_adapter_by_id(manager_get_default_adapter_id());
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?

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

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

* Re: [PATCHv3 2/3] Re-add manager_get_default_adapter()
  2011-06-09 15:00     ` Antonio Ospite
@ 2011-06-09 15:10       ` Bastien Nocera
  0 siblings, 0 replies; 21+ messages in thread
From: Bastien Nocera @ 2011-06-09 15:10 UTC (permalink / raw)
  To: Antonio Ospite
  Cc: linux-bluetooth, linux-input, Jim Paris, Ranulf Doswell,
	Pascal A . Brisset, Marcin Tolysz, Christian Birchinger,
	Filipe Lopes, Alan Ott, Mikko Virkkila, Simon Wood

On Thu, 2011-06-09 at 17:00 +0200, Antonio Ospite wrote:
> On Thu, 09 Jun 2011 15:02:32 +0100
> Bastien Nocera <hadess@hadess.net> wrote:
> 
> > On Wed, 2011-06-08 at 15:09 +0200, Antonio Ospite wrote:
> > > From: Bastien Nocera <hadess@hadess.net>
> > 
> > Sent out a new patch for that which allows the same functionality, but
> > returns an ID instead (you can then get the btd_adapter through existing
> > manager functions).
> 
> OK, so basically I drop 2/3 and do:
> adapter = manager_find_adapter_by_id(manager_get_default_adapter_id());
> right?

Yep. Hopefully my last patch will get in shortly.


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

* Re: [PATCHv3 3/3] Add sixaxis plugin: USB pairing and LEDs settings
  2011-06-09 13:57     ` Antonio Ospite
@ 2011-06-09 15:30       ` Anderson Lizardo
  2011-06-09 15:48           ` Antonio Ospite
  0 siblings, 1 reply; 21+ messages in thread
From: Anderson Lizardo @ 2011-06-09 15:30 UTC (permalink / raw)
  To: Antonio Ospite
  Cc: Vinicius Costa Gomes, linux-bluetooth, Bastien Nocera,
	linux-input, Jim Paris, Ranulf Doswell, Pascal A . Brisset,
	Marcin Tolysz, Christian Birchinger, Filipe Lopes, Alan Ott,
	Mikko Virkkila, Simon Wood

Hi,

On Thu, Jun 9, 2011 at 9:57 AM, Antonio Ospite <ospite@studenti.unina.it> wrote:
> Would you mind if I keep these three stdlib ones in? I use this code
> verbatim in another program which does not link against Glib.

But you use e.g. "g_strcmp0()", GIOChannel etc. How is that?

Also remember sed is your friend :)

My two cents,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

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

* Re: [PATCHv3 3/3] Add sixaxis plugin: USB pairing and LEDs settings
@ 2011-06-09 15:48           ` Antonio Ospite
  0 siblings, 0 replies; 21+ messages in thread
From: Antonio Ospite @ 2011-06-09 15:48 UTC (permalink / raw)
  To: Anderson Lizardo
  Cc: Vinicius Costa Gomes, linux-bluetooth, Bastien Nocera,
	linux-input, Jim Paris, Ranulf Doswell, Pascal A . Brisset,
	Marcin Tolysz, Christian Birchinger, Filipe Lopes, Alan Ott,
	Mikko Virkkila, Simon Wood

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

On Thu, 9 Jun 2011 11:30:56 -0400
Anderson Lizardo <anderson.lizardo@openbossa.org> wrote:

> Hi,
> 
> On Thu, Jun 9, 2011 at 9:57 AM, Antonio Ospite <ospite@studenti.unina.it> wrote:
> > Would you mind if I keep these three stdlib ones in? I use this code
> > verbatim in another program which does not link against Glib.
> 
> But you use e.g. "g_strcmp0()", GIOChannel etc. How is that?
> 
> Also remember sed is your friend :)
> 

I was referring to the "low level" functions:
  get_feature_report()
  set_feature_report()
  get_device_bdaddr()
  set_master_bdaddr()

sorry, I use them in a command line tool which opens the hidraw node
directly without udev.

But as you say, it is not a big deal to substitute them, if you like a
more uniform style in the bluez plugin then I'll comply.

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 --]

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

* Re: [PATCHv3 3/3] Add sixaxis plugin: USB pairing and LEDs settings
@ 2011-06-09 15:48           ` Antonio Ospite
  0 siblings, 0 replies; 21+ messages in thread
From: Antonio Ospite @ 2011-06-09 15:48 UTC (permalink / raw)
  To: Anderson Lizardo
  Cc: Vinicius Costa Gomes, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	Bastien Nocera, linux-input-u79uwXL29TY76Z2rM5mHXA, Jim Paris,
	Ranulf Doswell, Pascal A . Brisset, Marcin Tolysz,
	Christian Birchinger, Filipe Lopes, Alan Ott, Mikko Virkkila,
	Simon Wood

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

On Thu, 9 Jun 2011 11:30:56 -0400
Anderson Lizardo <anderson.lizardo-430g2QfJUUCGglJvpFV4uA@public.gmane.org> wrote:

> Hi,
> 
> On Thu, Jun 9, 2011 at 9:57 AM, Antonio Ospite <ospite-aNJ+ML1ZbiP93QAQaVx+gl6hYfS7NtTn@public.gmane.org> wrote:
> > Would you mind if I keep these three stdlib ones in? I use this code
> > verbatim in another program which does not link against Glib.
> 
> But you use e.g. "g_strcmp0()", GIOChannel etc. How is that?
> 
> Also remember sed is your friend :)
> 

I was referring to the "low level" functions:
  get_feature_report()
  set_feature_report()
  get_device_bdaddr()
  set_master_bdaddr()

sorry, I use them in a command line tool which opens the hidraw node
directly without udev.

But as you say, it is not a big deal to substitute them, if you like a
more uniform style in the bluez plugin then I'll comply.

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 --]

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

* Re: [PATCHv3 3/3] Add sixaxis plugin: USB pairing and LEDs settings
  2011-06-09 15:48           ` Antonio Ospite
  (?)
@ 2011-06-09 15:57           ` Bastien Nocera
  -1 siblings, 0 replies; 21+ messages in thread
From: Bastien Nocera @ 2011-06-09 15:57 UTC (permalink / raw)
  To: Antonio Ospite
  Cc: Anderson Lizardo, Vinicius Costa Gomes, linux-bluetooth,
	linux-input, Jim Paris, Ranulf Doswell, Pascal A . Brisset,
	Marcin Tolysz, Christian Birchinger, Filipe Lopes, Alan Ott,
	Mikko Virkkila, Simon Wood

On Thu, 2011-06-09 at 17:48 +0200, Antonio Ospite wrote:
> On Thu, 9 Jun 2011 11:30:56 -0400
> Anderson Lizardo <anderson.lizardo@openbossa.org> wrote:
> 
> > Hi,
> > 
> > On Thu, Jun 9, 2011 at 9:57 AM, Antonio Ospite <ospite@studenti.unina.it> wrote:
> > > Would you mind if I keep these three stdlib ones in? I use this code
> > > verbatim in another program which does not link against Glib.
> > 
> > But you use e.g. "g_strcmp0()", GIOChannel etc. How is that?
> > 
> > Also remember sed is your friend :)
> > 
> 
> I was referring to the "low level" functions:
>   get_feature_report()
>   set_feature_report()
>   get_device_bdaddr()
>   set_master_bdaddr()
> 
> sorry, I use them in a command line tool which opens the hidraw node
> directly without udev.
> 
> But as you say, it is not a big deal to substitute them, if you like a
> more uniform style in the bluez plugin then I'll comply.

If it makes it easier for testing, split them out in a separate file,
add a header, and a test program in bluez directly.

That'll stop the 2 copies getting out of sync.


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

* Re: [PATCHv3 2/3] Re-add manager_get_default_adapter()
  2011-06-08 13:09 ` [PATCHv3 2/3] Re-add manager_get_default_adapter() Antonio Ospite
  2011-06-09 14:02   ` Bastien Nocera
@ 2011-06-15 20:53   ` Johan Hedberg
  1 sibling, 0 replies; 21+ messages in thread
From: Johan Hedberg @ 2011-06-15 20:53 UTC (permalink / raw)
  To: Antonio Ospite
  Cc: linux-bluetooth, Bastien Nocera, linux-input, Jim Paris,
	Ranulf Doswell, Pascal A . Brisset, Marcin Tolysz,
	Christian Birchinger, Filipe Lopes, Alan Ott, Mikko Virkkila,
	Simon Wood

Hi,

On Wed, Jun 08, 2011, Antonio Ospite wrote:
> From: Bastien Nocera <hadess@hadess.net>
> 
> Signed-off-by: Bastien Nocera <hadess@hadess.net>
> Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
> ---
>  src/manager.c |    5 +++++
>  src/manager.h |    1 +
>  2 files changed, 6 insertions(+), 0 deletions(-)

I've applied this one (with the signed-off-by lines remove which we
don't use and a better explanation) so you can continue with each of
your work in fine-tuning/fixing your patches.

Johan

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

end of thread, other threads:[~2011-06-15 20:53 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-08 13:09 [PATCHv3 0/3] Sixaxis plugin for Bluez Antonio Ospite
2011-06-08 13:09 ` [PATCHv3 1/3] Remove input/sixpair.c Antonio Ospite
2011-06-08 13:09   ` Antonio Ospite
2011-06-08 13:09 ` [PATCHv3 2/3] Re-add manager_get_default_adapter() Antonio Ospite
2011-06-09 14:02   ` Bastien Nocera
2011-06-09 15:00     ` Antonio Ospite
2011-06-09 15:10       ` Bastien Nocera
2011-06-15 20:53   ` Johan Hedberg
2011-06-08 13:09 ` [PATCHv3 3/3] Add sixaxis plugin: USB pairing and LEDs settings Antonio Ospite
2011-06-08 13:09   ` Antonio Ospite
2011-06-08 17:52   ` Vinicius Costa Gomes
2011-06-08 17:52     ` Vinicius Costa Gomes
2011-06-09 13:57     ` Antonio Ospite
2011-06-09 15:30       ` Anderson Lizardo
2011-06-09 15:48         ` Antonio Ospite
2011-06-09 15:48           ` Antonio Ospite
2011-06-09 15:57           ` Bastien Nocera
2011-06-08 14:33 ` [PATCHv3 0/3] Sixaxis plugin for Bluez Bastien Nocera
2011-06-08 14:33   ` Bastien Nocera
2011-06-09 14:54   ` Antonio Ospite
2011-06-09 14:54     ` Antonio Ospite

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.