Re: [PATCH] broadband-bearer: set TTY as connected as soon as ATD replies
On Fri, Mar 24, 2017 at 9:35 PM, Dan Williamswrote: >> When the connection request is done via ATD, flag the port as >> connected as soon as the successful response is received, so that we >> remove our polling on the device right away. >> >> https://bugs.freedesktop.org/show_bug.cgi?id=99541 > > LGTM. Pushed to git master, mm-1-6 and mm-1-4. -- Aleksander https://aleksander.es ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH] broadband-bearer: set TTY as connected as soon as ATD replies
On Fri, 2017-03-24 at 15:03 +0100, Aleksander Morgado wrote: > When the connection request is done via ATD, flag the port as > connected as soon as the successful response is received, so that we > remove our polling on the device right away. > > https://bugs.freedesktop.org/show_bug.cgi?id=99541 LGTM. Dan > --- > > Hey Colin, Dan & everyone, > > This patch tries to minimize the possibility of a race condition > happening when we end up sending AT commands (e.g. signal quality > loading request) between a successful reply to ATD, and the actual > setting of the TTY as 'connected', which is done in an idle. > > This was discussed not long ago: > https://lists.freedesktop.org/archives/modemmanager-devel/2017-Januar > y/003773.html > > Comments? > > --- > src/mm-broadband-bearer.c | 13 +++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/src/mm-broadband-bearer.c b/src/mm-broadband-bearer.c > index f2834b1b..bb4ff567 100644 > --- a/src/mm-broadband-bearer.c > +++ b/src/mm-broadband-bearer.c > @@ -254,7 +254,10 @@ dial_cdma_ready (MMBaseModem *modem, > return; > } > > -/* else... Yuhu! */ > +/* The ATD command has succeeded, and therefore the TTY is in > data mode now. > + * Instead of waiting for setting the port as connected later in > + * connect_succeeded(), we do it right away so that we stop our > polling. */ > +mm_port_set_connected (ctx->data, TRUE); > > /* Keep port open during connection */ > ctx->close_data_on_exit = FALSE; > @@ -573,6 +576,11 @@ atd_ready (MMBaseModem *modem, > return; > } > > +/* The ATD command has succeeded, and therefore the TTY is in > data mode now. > + * Instead of waiting for setting the port as connected later in > + * connect_succeeded(), we do it right away so that we stop our > polling. */ > +mm_port_set_connected (MM_PORT (ctx->dial_port), TRUE); > + > g_simple_async_result_set_op_res_gpointer (ctx->result, > g_object_ref (ctx- > >dial_port), > (GDestroyNotify)g_obj > ect_unref); > @@ -1177,7 +1185,8 @@ connect_succeeded (ConnectContext *ctx, > ctx->self->priv->port = g_object_ref > (mm_bearer_connect_result_peek_data (result)); > ctx->self->priv->connection_type = connection_type; > > -/* Port is connected; update the state */ > +/* Port is connected; update the state. For ATD based > connections, the port > + * may already be set as connected, but no big deal. */ > mm_port_set_connected (ctx->self->priv->port, TRUE); > > /* Set operation result */ > -- > 2.12.0 > ___ > ModemManager-devel mailing list > ModemManager-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
[PATCH] broadband-bearer: set TTY as connected as soon as ATD replies
When the connection request is done via ATD, flag the port as connected as soon as the successful response is received, so that we remove our polling on the device right away. https://bugs.freedesktop.org/show_bug.cgi?id=99541 --- Hey Colin, Dan & everyone, This patch tries to minimize the possibility of a race condition happening when we end up sending AT commands (e.g. signal quality loading request) between a successful reply to ATD, and the actual setting of the TTY as 'connected', which is done in an idle. This was discussed not long ago: https://lists.freedesktop.org/archives/modemmanager-devel/2017-January/003773.html Comments? --- src/mm-broadband-bearer.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/mm-broadband-bearer.c b/src/mm-broadband-bearer.c index f2834b1b..bb4ff567 100644 --- a/src/mm-broadband-bearer.c +++ b/src/mm-broadband-bearer.c @@ -254,7 +254,10 @@ dial_cdma_ready (MMBaseModem *modem, return; } -/* else... Yuhu! */ +/* The ATD command has succeeded, and therefore the TTY is in data mode now. + * Instead of waiting for setting the port as connected later in + * connect_succeeded(), we do it right away so that we stop our polling. */ +mm_port_set_connected (ctx->data, TRUE); /* Keep port open during connection */ ctx->close_data_on_exit = FALSE; @@ -573,6 +576,11 @@ atd_ready (MMBaseModem *modem, return; } +/* The ATD command has succeeded, and therefore the TTY is in data mode now. + * Instead of waiting for setting the port as connected later in + * connect_succeeded(), we do it right away so that we stop our polling. */ +mm_port_set_connected (MM_PORT (ctx->dial_port), TRUE); + g_simple_async_result_set_op_res_gpointer (ctx->result, g_object_ref (ctx->dial_port), (GDestroyNotify)g_object_unref); @@ -1177,7 +1185,8 @@ connect_succeeded (ConnectContext *ctx, ctx->self->priv->port = g_object_ref (mm_bearer_connect_result_peek_data (result)); ctx->self->priv->connection_type = connection_type; -/* Port is connected; update the state */ +/* Port is connected; update the state. For ATD based connections, the port + * may already be set as connected, but no big deal. */ mm_port_set_connected (ctx->self->priv->port, TRUE); /* Set operation result */ -- 2.12.0 ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel