[PATCH 2/2] broadband-modem-mbim: implement initial signal quality loading

[Aleksander Morgado]

On Tue, Apr 24, 2018 at 1:24 AM, Ben Chan <benchan at chromium.org> wrote:
> ModemManager currently relies on unsolicited MBIM_CID_SIGNAL_STATE
> notifications to obtain signal quality updates, and it doesn't query the
> initial signal quality. It's been observed that some MBIM modems issue a
> MBIM_CID_SIGNAL_STATE notification only when there is a notable change
> in RSSI. The signal quality may remain at 0 for quite some time. It's
> more noticeable when simply restarting ModemManager after the modem has
> been initialized and enabled once.
>
> We could simply enable periodic signal check on an MBIM modem, but
> that's less ideal as it may unnecessarily wake the modem up from USB
> selective suspend (unless we use a much longer polling period).
>
> To address the issue, this patch adds the implementation of
> load_signal_quality to MMBroadbandModemMbim so that the signal quality
> is initially polled via a solicited MBIM_CID_SIGNAL_STATE query. To
> avoid the periodic signal check, we set the
> MM_IFACE_MODEM_PERIODIC_SIGNAL_CHECK_DISABLED property to TRUE for
> MMBroadbandModemMbim.
> ---

Pushed to git master, thanks!

>  src/mm-broadband-modem-mbim.c | 80 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 78 insertions(+), 2 deletions(-)
>
> diff --git a/src/mm-broadband-modem-mbim.c b/src/mm-broadband-modem-mbim.c
> index d6562088..09e1ffb7 100644
> --- a/src/mm-broadband-modem-mbim.c
> +++ b/src/mm-broadband-modem-mbim.c
> @@ -1433,6 +1433,79 @@ modem_power_down (MMIfaceModem *self,
>      mbim_message_unref (message);
>  }
>
> +/*****************************************************************************/
> +/* Signal quality loading (Modem interface) */
> +
> +static guint
> +modem_load_signal_quality_finish (MMIfaceModem *self,
> +                                  GAsyncResult *res,
> +                                  GError **error)
> +{
> +    gssize value;
> +
> +    value = g_task_propagate_int (G_TASK (res), error);
> +    return value < 0 ? 0 : value;
> +}
> +
> +static void
> +signal_state_query_ready (MbimDevice *device,
> +                          GAsyncResult *res,
> +                          GTask *task)
> +{
> +    MbimMessage *response;
> +    GError *error = NULL;
> +    guint32 rssi;
> +
> +    response = mbim_device_command_finish (device, res, &error);
> +    if (response &&
> +        mbim_message_response_get_result (response, MBIM_MESSAGE_TYPE_COMMAND_DONE, &error) &&
> +        mbim_message_signal_state_response_parse (
> +            response,
> +            &rssi,
> +            NULL, /* error_rate */
> +            NULL, /* signal_strength_interval */
> +            NULL, /* rssi_threshold */
> +            NULL, /* error_rate_threshold */
> +            &error)) {
> +        guint32 quality;
> +
> +        /* Normalize the quality. 99 means unknown, we default it to 0 */
> +        quality = CLAMP (rssi == 99 ? 0 : rssi, 0, 31) * 100 / 31;
> +
> +        g_task_return_int (task, quality);
> +    } else
> +        g_task_return_error (task, error);
> +
> +    g_object_unref (task);
> +
> +    if (response)
> +        mbim_message_unref (response);
> +}
> +
> +static void
> +modem_load_signal_quality (MMIfaceModem *self,
> +                           GAsyncReadyCallback callback,
> +                           gpointer user_data)
> +{
> +    MbimDevice *device;
> +    MbimMessage *message;
> +    GTask *task;
> +
> +    if (!peek_device (self, &device, callback, user_data))
> +        return;
> +
> +    task = g_task_new (self, NULL, callback, user_data);
> +
> +    message = mbim_message_signal_state_query_new (NULL);
> +    mbim_device_command (device,
> +                         message,
> +                         10,
> +                         NULL,
> +                         (GAsyncReadyCallback)signal_state_query_ready,
> +                         task);
> +    mbim_message_unref (message);
> +}
> +
>  /*****************************************************************************/
>  /* Create Bearer (Modem interface) */
>
> @@ -3305,6 +3378,7 @@ mm_broadband_modem_mbim_new (const gchar *device,
>                           MM_BASE_MODEM_PRODUCT_ID, product_id,
>                           MM_IFACE_MODEM_SIM_HOT_SWAP_SUPPORTED, TRUE,
>                           MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED, FALSE,
> +                         MM_IFACE_MODEM_PERIODIC_SIGNAL_CHECK_DISABLED, TRUE,
>                           NULL);
>  }
>
> @@ -3379,6 +3453,10 @@ iface_modem_init (MMIfaceModem *iface)
>      iface->load_supported_ip_families = modem_load_supported_ip_families;
>      iface->load_supported_ip_families_finish = modem_load_supported_ip_families_finish;
>
> +    /* Additional actions */
> +    iface->load_signal_quality = modem_load_signal_quality;
> +    iface->load_signal_quality_finish = modem_load_signal_quality_finish;
> +
>      /* Unneeded things */
>      iface->modem_after_power_up = NULL;
>      iface->modem_after_power_up_finish = NULL;
> @@ -3388,8 +3466,6 @@ iface_modem_init (MMIfaceModem *iface)
>      iface->setup_flow_control_finish = NULL;
>      iface->setup_charset = NULL;
>      iface->setup_charset_finish = NULL;
> -    iface->load_signal_quality = NULL;
> -    iface->load_signal_quality_finish = NULL;
>      iface->load_access_technologies = NULL;
>      iface->load_access_technologies_finish = NULL;
>
> --
> 2.17.0.484.g0c8726318c-goog
>



-- 
Aleksander
https://aleksander.es