Hi Aleksander,
2017-03-25 19:32 GMT+01:00 Aleksander Morgado <aleksan...@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
>
_______________________________________________
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
