[PATCH 8/8] broadband-bearer: once connected, set flow control settings

Daniele Palmas dnlplm at gmail.com
Wed Mar 29 12:23:16 UTC 2017


Hi Aleksander,

2017-03-25 19:32 GMT+01:00 Aleksander Morgado <aleksander at aleksander.es>:
> During modem initialization we detected the flow control settings
> supported by the modem, and selected the best one to use from them,
> notifying it to the device via AT+IFC. The device was therefore
> instructed to use that flow control setting for data transmission in
> the TTY (i.e. not during AT control commands).
>
> The missing thing was to also configure ourselves our end of the
> serial port with the same flow control settings when getting into data
> mode. By doing it ourselves, we avoid requiring any explicit setting
> in pppd for flow control; pppd can assume the flow control settings
> are already the expected ones.
>
> Worth noting that all this setup is completely ignored for TTYs
> exposed directly via USB.
>
> https://bugs.freedesktop.org/show_bug.cgi?id=100394
> ---

I took a look at the whole series and, FWIW the overall logic seems
good to me even though I yet did not find the time for testing this.

Thanks,
Daniele

>  src/mm-broadband-bearer.c | 32 ++++++++++++++++++++++++++++++++
>  src/mm-broadband-modem.c  | 36 +++++++++++++++++++++++++-----------
>  src/mm-broadband-modem.h  |  4 ++++
>  3 files changed, 61 insertions(+), 11 deletions(-)
>
> diff --git a/src/mm-broadband-bearer.c b/src/mm-broadband-bearer.c
> index 9491f5a8..bccee72b 100644
> --- a/src/mm-broadband-bearer.c
> +++ b/src/mm-broadband-bearer.c
> @@ -259,6 +259,14 @@ dial_cdma_ready (MMBaseModem *modem,
>       * connect_succeeded(), we do it right away so that we stop our polling. */
>      mm_port_set_connected (ctx->data, TRUE);
>
> +    /* Configure flow control to use while connected */
> +    if (!mm_port_serial_set_flow_control (MM_PORT_SERIAL (ctx->data),
> +                                          mm_broadband_modem_get_connected_flow_control (MM_BROADBAND_MODEM (modem)),
> +                                          &error)) {
> +        mm_warn ("Couldn't set flow control settings: %s", error->message);
> +        g_clear_error (&error);
> +    }
> +
>      /* Keep port open during connection */
>      ctx->close_data_on_exit = FALSE;
>
> @@ -557,6 +565,8 @@ atd_ready (MMBaseModem *modem,
>             GAsyncResult *res,
>             Dial3gppContext *ctx)
>  {
> +    GError *error = NULL;
> +
>      /* DO NOT check for cancellable here. If we got here without errors, the
>       * bearer is really connected and therefore we need to reflect that in
>       * the state machine. */
> @@ -576,6 +586,14 @@ atd_ready (MMBaseModem *modem,
>          return;
>      }
>
> +    /* Configure flow control to use while connected */
> +    if (!mm_port_serial_set_flow_control (MM_PORT_SERIAL (ctx->dial_port),
> +                                          mm_broadband_modem_get_connected_flow_control (MM_BROADBAND_MODEM (modem)),
> +                                          &error)) {
> +        mm_warn ("Couldn't set flow control settings: %s", error->message);
> +        g_clear_error (&error);
> +    }
> +
>      /* 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. */
> @@ -1452,9 +1470,16 @@ data_flash_cdma_ready (MMPortSerial *data,
>                         DetailedDisconnectContext *ctx)
>  {
>      GError *error = NULL;
> +    GError *flow_control_error = NULL;
>
>      mm_port_serial_flash_finish (data, res, &error);
>
> +    /* Cleanup flow control */
> +    if (!mm_port_serial_set_flow_control (MM_PORT_SERIAL (data), MM_FLOW_CONTROL_NONE, &flow_control_error)) {
> +        mm_dbg ("Couldn't reset flow control settings: %s", flow_control_error->message);
> +        g_clear_error (&flow_control_error);
> +    }
> +
>      /* We kept the serial port open during connection, now we close that open
>       * count */
>      mm_port_serial_close (data);
> @@ -1569,9 +1594,16 @@ data_flash_3gpp_ready (MMPortSerial *data,
>                         DetailedDisconnectContext *ctx)
>  {
>      GError *error = NULL;
> +    GError *flow_control_error = NULL;
>
>      mm_port_serial_flash_finish (data, res, &error);
>
> +    /* Cleanup flow control */
> +    if (!mm_port_serial_set_flow_control (MM_PORT_SERIAL (data), MM_FLOW_CONTROL_NONE, &flow_control_error)) {
> +        mm_dbg ("Couldn't reset flow control settings: %s", flow_control_error->message);
> +        g_clear_error (&flow_control_error);
> +    }
> +
>      /* We kept the serial port open during connection, now we close that open
>       * count */
>      mm_port_serial_close (data);
> diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
> index e7376310..83302f63 100644
> --- a/src/mm-broadband-modem.c
> +++ b/src/mm-broadband-modem.c
> @@ -146,6 +146,7 @@ struct _MMBroadbandModemPrivate {
>      guint modem_cind_max_signal_quality;
>      guint modem_cind_indicator_roaming;
>      guint modem_cind_indicator_service;
> +    MMFlowControl modem_flow_control;
>
>      /*<--- Modem 3GPP interface --->*/
>      /* Properties */
> @@ -3101,17 +3102,20 @@ modem_setup_flow_control_finish (MMIfaceModem  *self,
>  }
>
>  static void
> -ifc_test_ready (MMBaseModem  *self,
> +ifc_test_ready (MMBaseModem  *_self,
>                  GAsyncResult *res,
>                  GTask        *task)
>  {
> -    GError        *error = NULL;
> -    const gchar   *response;
> -    MMFlowControl  mask;
> -    const gchar   *cmd;
> +    MMBroadbandModem *self;
> +    GError           *error = NULL;
> +    const gchar      *response;
> +    MMFlowControl     mask;
> +    const gchar      *cmd;
> +
> +    self = MM_BROADBAND_MODEM (_self);
>
>      /* Completely ignore errors in AT+IFC=? */
> -    response = mm_base_modem_at_command_finish (self, res, &error);
> +    response = mm_base_modem_at_command_finish (_self, res, &error);
>      if (!response)
>          goto out;
>
> @@ -3125,17 +3129,20 @@ ifc_test_ready (MMBaseModem  *self,
>       *  XON/XOFF
>       *  None.
>       */
> -    if (mask & MM_FLOW_CONTROL_RTS_CTS)
> +    if (mask & MM_FLOW_CONTROL_RTS_CTS) {
> +        self->priv->modem_flow_control = MM_FLOW_CONTROL_RTS_CTS;
>          cmd = "+IFC=2,2";
> -    else if (mask & MM_FLOW_CONTROL_XON_XOFF)
> +    } else if (mask & MM_FLOW_CONTROL_XON_XOFF) {
> +        self->priv->modem_flow_control = MM_FLOW_CONTROL_XON_XOFF;
>          cmd = "+IFC=1,1";
> -    else if (mask & MM_FLOW_CONTROL_NONE)
> +    } else if (mask & MM_FLOW_CONTROL_NONE) {
> +        self->priv->modem_flow_control = MM_FLOW_CONTROL_NONE;
>          cmd = "+IFC=0,0";
> -    else
> +    } else
>          g_assert_not_reached ();
>
>      /* Set flow control settings and ignore result */
> -    mm_base_modem_at_command (self, cmd, 3, FALSE, NULL, NULL);
> +    mm_base_modem_at_command (_self, cmd, 3, FALSE, NULL, NULL);
>
>  out:
>      /* Ignore errors */
> @@ -10315,6 +10322,12 @@ mm_broadband_modem_get_current_charset (MMBroadbandModem *self)
>      return self->priv->modem_current_charset;
>  }
>
> +MMFlowControl
> +mm_broadband_modem_get_connected_flow_control (MMBroadbandModem *self)
> +{
> +    return self->priv->modem_flow_control;
> +}
> +
>  gchar *
>  mm_broadband_modem_create_device_identifier (MMBroadbandModem *self,
>                                               const gchar *ati,
> @@ -10635,6 +10648,7 @@ mm_broadband_modem_init (MMBroadbandModem *self)
>      self->priv->current_sms_mem1_storage = MM_SMS_STORAGE_UNKNOWN;
>      self->priv->current_sms_mem2_storage = MM_SMS_STORAGE_UNKNOWN;
>      self->priv->sim_hot_swap_supported = FALSE;
> +    self->priv->modem_flow_control = MM_FLOW_CONTROL_NONE;
>  }
>
>  static void
> diff --git a/src/mm-broadband-modem.h b/src/mm-broadband-modem.h
> index d6b55d9d..9ac4eabd 100644
> --- a/src/mm-broadband-modem.h
> +++ b/src/mm-broadband-modem.h
> @@ -24,6 +24,7 @@
>
>  #include <ModemManager.h>
>
> +#include "mm-modem-helpers.h"
>  #include "mm-charsets.h"
>  #include "mm-base-modem.h"
>
> @@ -109,6 +110,9 @@ gchar *mm_broadband_modem_take_and_convert_to_current_charset (MMBroadbandModem
>
>  MMModemCharset mm_broadband_modem_get_current_charset (MMBroadbandModem *self);
>
> +/* Get the flow control setting to apply when connected, if any */
> +MMFlowControl mm_broadband_modem_get_connected_flow_control (MMBroadbandModem *self);
> +
>  /* Create a unique device identifier string using the ATI and ATI1 replies and some
>   * additional internal info */
>  gchar *mm_broadband_modem_create_device_identifier (MMBroadbandModem *self,
> --
> 2.12.0
>


More information about the ModemManager-devel mailing list