Small headset.c fixes

PATCH In review Fabien Chevalier 2 participants 3 messages Oct 14, 2007 – Oct 15, 2007 Bluetooth drivers

lore

Share & tools

lore

Fabien Chevalier

author

2007-10-14 15:39 UTC #0 lore raw

PATCH 1/1 Small headset.c fixes

Hi All,

Please find attached a rather small patch, that fixes some small issues i found while reading headset.c code:
* make String representation of state consistent with the state name
* send Stop() signal only if we transition from playing state, not
playin_in_progress
* remove unused io_id field.

Cheers,

Fabien

headset.c +11 -7

Index: headset.c
===================================================================
@@ -63,14 +63,18 @@
 #define HEADSET_GAIN_SPEAKER 'S'
 #define HEADSET_GAIN_MICROPHONE 'M'
 
-static char *str_state[] = {"DISCONNECTED", "CONNECTING", "CONNECTED",
-				"STREAM_STARTING", "STREAMING"};
+static char *str_state[] = {
+	"HEADSET_STATE_DISCONNECTED", 
+	"HEADSET_STATE_CONNECT_IN_PROGRESS",
+	"HEADSET_STATE_CONNECTED",
+	"HEADSET_STATE_PLAY_IN_PROGRESS",
+	"HEADSET_STATE_PLAYING",
+	};
 
 struct pending_connect {
 	DBusMessage *msg;
 	DBusPendingCall *call;
 	GIOChannel *io;
-	guint io_id;
 	int sock;
 	int err;
 	unsigned int id;

@@ -489,9 +493,9 @@
 			return -err;
 		}
 
-		c->io_id = g_io_add_watch(c->io,
-					G_IO_OUT | G_IO_NVAL | G_IO_ERR | G_IO_HUP,
-					(GIOFunc) sco_connect_cb, device);
+		g_io_add_watch(c->io,
+				G_IO_OUT | G_IO_NVAL | G_IO_ERR | G_IO_HUP,
+				(GIOFunc) sco_connect_cb, device);
 	} else
 		do_callback = TRUE;

@@ -1553,7 +1557,7 @@
 						AUDIO_HEADSET_INTERFACE,
 						"Connected",
 						DBUS_TYPE_INVALID);
-		} else {
+		} else if (hs->state == HEADSET_STATE_PLAYING) {
 			close_sco(dev);
 			dbus_connection_emit_signal(dev->conn, dev->path,
 						AUDIO_HEADSET_INTERFACE,

_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Luiz Augusto von Dentz

maintainer

2007-10-15 01:48 UTC #1 lore raw

Hi Fabien,

Quoted 8 lines · Fabien Chevalier

Hi All,
Please find attached a rather small patch, that fixes some small issues i found while reading headset.c code:
* make String representation of state consistent with the state name
* send Stop() signal only if we transition from playing state, not
playin_in_progress
* remove unused io_id field.

Thought we should not emit Stopped signal please maintain the code for sco cleanup (close_sco), sco connection may fail or disconnected before we got a chance to change the state to playing.

Besides that it seems good to me.

-- =

Luiz Augusto von Dentz
Engenheiro de Computa=E7=E3o

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Fabien Chevalier

author

2007-10-15 09:25 UTC #2 lore raw

Hi Luiz,

Quoted 3 lines

Thought we should not emit Stopped signal please maintain the code for sco cleanup (close_sco), sco connection may fail or disconnected before we got a chance to change the state to playing.

This was bogus indeed, thanks for spotting that ;-)

Quoted 3 lines

Besides that it seems good to me.

Here comes ans updated version of the patch. I moved close_sco to be executed at anytime, it doesn't hurt as close_sco does already check to see if the the sco channel is already opened.

Cheers,

Fabien

headset.c +12 -8

Index: headset.c
===================================================================
@@ -63,14 +63,18 @@
 #define HEADSET_GAIN_SPEAKER 'S'
 #define HEADSET_GAIN_MICROPHONE 'M'
 
-static char *str_state[] = {"DISCONNECTED", "CONNECTING", "CONNECTED",
-				"STREAM_STARTING", "STREAMING"};
+static char *str_state[] = {
+	"HEADSET_STATE_DISCONNECTED", 
+	"HEADSET_STATE_CONNECT_IN_PROGRESS",
+	"HEADSET_STATE_CONNECTED",
+	"HEADSET_STATE_PLAY_IN_PROGRESS",
+	"HEADSET_STATE_PLAYING",
+	};
 
 struct pending_connect {
 	DBusMessage *msg;
 	DBusPendingCall *call;
 	GIOChannel *io;
-	guint io_id;
 	int sock;
 	int err;
 	unsigned int id;

@@ -489,9 +493,9 @@
 			return -err;
 		}
 
-		c->io_id = g_io_add_watch(c->io,
-					G_IO_OUT | G_IO_NVAL | G_IO_ERR | G_IO_HUP,
-					(GIOFunc) sco_connect_cb, device);
+		g_io_add_watch(c->io,
+				G_IO_OUT | G_IO_NVAL | G_IO_ERR | G_IO_HUP,
+				(GIOFunc) sco_connect_cb, device);
 	} else
 		do_callback = TRUE;

@@ -1544,6 +1548,7 @@
 	case HEADSET_STATE_CONNECT_IN_PROGRESS:
 		break;
 	case HEADSET_STATE_CONNECTED:
+		close_sco(dev);
 		if (hs->state < state) {
 			g_io_add_watch(hs->rfcomm,
 				G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,

@@ -1553,8 +1558,7 @@
 						AUDIO_HEADSET_INTERFACE,
 						"Connected",
 						DBUS_TYPE_INVALID);
-		} else {
-			close_sco(dev);
+		} else if (hs->state == HEADSET_STATE_PLAYING) {
 			dbus_connection_emit_signal(dev->conn, dev->path,
 						AUDIO_HEADSET_INTERFACE,
 						"Stopped",

_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel