[PATCH BlueZ] gatttool: Update interactive prompt if connection is lost

[PATCH BlueZ] gatttool: Update interactive prompt if connection is lost

[Jefferson Delfes]

In interactive mode, when connection is lost, the prompt used to remain
in "connected" state. This patch fixes that case, by always showing the
actual connection state.
---
 attrib/interactive.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/attrib/interactive.c b/attrib/interactive.c
index be81424..47ddcf6 100644
--- a/attrib/interactive.c
+++ b/attrib/interactive.c
@@ -56,6 +56,7 @@ struct characteristic_data {
 };
 
 static void cmd_help(int argcp, char **argvp);
+static void cmd_disconnect(int argcp, char **argvp);
 
 enum state {
 	STATE_DISCONNECTED,
@@ -327,6 +328,14 @@ static void cmd_exit(int argcp, char **argvp)
 	g_main_loop_quit(event_loop);
 }
 
+static gboolean channel_watcher(GIOChannel *chan, GIOCondition cond,
+				gpointer user_data)
+{
+	cmd_disconnect(0, NULL);
+
+	return TRUE;
+}
+
 static void cmd_connect(int argcp, char **argvp)
 {
 	if (conn_state != STATE_DISCONNECTED)
@@ -347,6 +356,8 @@ static void cmd_connect(int argcp, char **argvp)
 						opt_mtu, connect_cb);
 	if (iochannel == NULL)
 		set_state(STATE_DISCONNECTED);
+	else
+		g_io_add_watch(iochannel, G_IO_HUP, channel_watcher, NULL);
 }
 
 static void cmd_disconnect(int argcp, char **argvp)
-- 
1.7.9.4

Re: [PATCH BlueZ] gatttool: Update interactive prompt if connection is lost

[Johan Hedberg]

Hi Jefferson,

On Fri, Mar 23, 2012, Jefferson Delfes wrote:
> In interactive mode, when connection is lost, the prompt used to remain
> in "connected" state. This patch fixes that case, by always showing the
> actual connection state.
> ---
>  attrib/interactive.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/attrib/interactive.c b/attrib/interactive.c
> index be81424..47ddcf6 100644
> --- a/attrib/interactive.c
> +++ b/attrib/interactive.c
> @@ -56,6 +56,7 @@ struct characteristic_data {
>  };
>  
>  static void cmd_help(int argcp, char **argvp);
> +static void cmd_disconnect(int argcp, char **argvp);

We should really try to avoid forward declarations when possible and the
cmd_* functions should be reserved for real user entered commands and
not as generic helpers as you're trying to do. So I'd factor out code
from cmd_disconnect into a static helper function somewhere above both
cmd_disconnect and cmd_connect, call it e.g. disconnect_io().

> +static gboolean channel_watcher(GIOChannel *chan, GIOCondition cond,
> +				gpointer user_data)
> +{
> +	cmd_disconnect(0, NULL);
> +
> +	return TRUE;
> +}

Shouldn't this be returning FALSE instead of TRUE so that the watch
doesn't get left hanging around GLibs watch lists wasting memory?

Johan

Re: [PATCH BlueZ] gatttool: Update interactive prompt if connection is lost

[Jefferson Delfes]

Hi Johan.

On Tue, Mar 27, 2012 at 7:50 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>
> Hi Jefferson,
>
> On Fri, Mar 23, 2012, Jefferson Delfes wrote:
> > In interactive mode, when connection is lost, the prompt used to remain
> > in "connected" state. This patch fixes that case, by always showing the
> > actual connection state.
> > ---
> >  attrib/interactive.c |   11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/attrib/interactive.c b/attrib/interactive.c
> > index be81424..47ddcf6 100644
> > --- a/attrib/interactive.c
> > +++ b/attrib/interactive.c
> > @@ -56,6 +56,7 @@ struct characteristic_data {
> >  };
> >
> >  static void cmd_help(int argcp, char **argvp);
> > +static void cmd_disconnect(int argcp, char **argvp);
>
> We should really try to avoid forward declarations when possible and the
> cmd_* functions should be reserved for real user entered commands and
> not as generic helpers as you're trying to do. So I'd factor out code
> from cmd_disconnect into a static helper function somewhere above both
> cmd_disconnect and cmd_connect, call it e.g. disconnect_io().

Ok, I'll do these changes.

> > +static gboolean channel_watcher(GIOChannel *chan, GIOCondition cond,
> > +                             gpointer user_data)
> > +{
> > +     cmd_disconnect(0, NULL);
> > +
> > +     return TRUE;
> > +}
>
> Shouldn't this be returning FALSE instead of TRUE so that the watch
> doesn't get left hanging around GLibs watch lists wasting memory?

Oops! You are right. I didn't know about this. I made some tests in
valgrind now and I did see some still reachable extra bytes.

> Johan

I'll send a replace for this patch.
Thanks.

[PATCHv2 BlueZ] gatttool: Update interactive prompt if connection is lost

[Jefferson Delfes]

In interactive mode, when connection is lost, the prompt used to remain
in "connected" state. This patch fixes that case, by always showing the
actual connection state.
---
 attrib/interactive.c |   39 +++++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/attrib/interactive.c b/attrib/interactive.c
index 5941811..0064ba2 100644
--- a/attrib/interactive.c
+++ b/attrib/interactive.c
@@ -149,6 +149,22 @@ static void connect_cb(GIOChannel *io, GError *err, gpointer user_data)
 	set_state(STATE_CONNECTED);
 }
 
+static void disconnect_io()
+{
+	if (conn_state == STATE_DISCONNECTED)
+		return;
+
+	g_attrib_unref(attrib);
+	attrib = NULL;
+	opt_mtu = 0;
+
+	g_io_channel_shutdown(iochannel, FALSE, NULL);
+	g_io_channel_unref(iochannel);
+	iochannel = NULL;
+
+	set_state(STATE_DISCONNECTED);
+}
+
 static void primary_all_cb(GSList *services, guint8 status, gpointer user_data)
 {
 	GSList *l;
@@ -327,6 +343,14 @@ static void cmd_exit(int argcp, char **argvp)
 	g_main_loop_quit(event_loop);
 }
 
+static gboolean channel_watcher(GIOChannel *chan, GIOCondition cond,
+				gpointer user_data)
+{
+	disconnect_io();
+
+	return FALSE;
+}
+
 static void cmd_connect(int argcp, char **argvp)
 {
 	if (conn_state != STATE_DISCONNECTED)
@@ -347,22 +371,13 @@ static void cmd_connect(int argcp, char **argvp)
 						opt_mtu, connect_cb);
 	if (iochannel == NULL)
 		set_state(STATE_DISCONNECTED);
+	else
+		g_io_add_watch(iochannel, G_IO_HUP, channel_watcher, NULL);
 }
 
 static void cmd_disconnect(int argcp, char **argvp)
 {
-	if (conn_state == STATE_DISCONNECTED)
-		return;
-
-	g_attrib_unref(attrib);
-	attrib = NULL;
-	opt_mtu = 0;
-
-	g_io_channel_shutdown(iochannel, FALSE, NULL);
-	g_io_channel_unref(iochannel);
-	iochannel = NULL;
-
-	set_state(STATE_DISCONNECTED);
+	disconnect_io();
 }
 
 static void cmd_primary(int argcp, char **argvp)
-- 
1.7.9.4

Re: [PATCHv2 BlueZ] gatttool: Update interactive prompt if connection is lost

[Johan Hedberg]

Hi Jefferson,

On Tue, Mar 27, 2012, Jefferson Delfes wrote:
> In interactive mode, when connection is lost, the prompt used to remain
> in "connected" state. This patch fixes that case, by always showing the
> actual connection state.
> ---
>  attrib/interactive.c |   39 +++++++++++++++++++++++++++------------
>  1 file changed, 27 insertions(+), 12 deletions(-)

Applied. Thanks.

Johan