[PATCH] telit: implement the network time interface for Telit modems

Aleksander Morgado aleksander at aleksander.es
Wed Mar 11 11:41:55 PDT 2015


Hey,

On Wed, Mar 11, 2015 at 6:49 PM, Jason Simmons <jsimmons at chromium.org> wrote:
> Add support for querying the real-time clock to the Telit plugin.
> Tested with a Telit HE910 modem.
> ---
>  plugins/telit/mm-broadband-modem-telit.c | 174
> ++++++++++++++++++++++++++++++-
>  1 file changed, 173 insertions(+), 1 deletion(-)
>
> diff --git a/plugins/telit/mm-broadband-modem-telit.c
> b/plugins/telit/mm-broadband-modem-telit.c
> index 5e1dbff..429a735 100644
> --- a/plugins/telit/mm-broadband-modem-telit.c
> +++ b/plugins/telit/mm-broadband-modem-telit.c
> @@ -29,12 +29,15 @@
>  #include "mm-modem-helpers.h"
>  #include "mm-base-modem-at.h"
>  #include "mm-iface-modem.h"
> +#include "mm-iface-modem-time.h"
>  #include "mm-broadband-modem-telit.h"
>
>  static void iface_modem_init (MMIfaceModem *iface);
> +static void iface_modem_time_init (MMIfaceModemTime *iface);
>
>  G_DEFINE_TYPE_EXTENDED (MMBroadbandModemTelit, mm_broadband_modem_telit,
> MM_TYPE_BROADBAND_MODEM, 0,
> -                        G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM,
> iface_modem_init));
> +                        G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM,
> iface_modem_init)
> +                        G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM_TIME,
> iface_modem_time_init));
>
>
> /*****************************************************************************/
>  /* Load access technologies (Modem interface) */
> @@ -175,6 +178,164 @@ load_access_technologies (MMIfaceModem *self,
>  }
>
>
> /*****************************************************************************/
> +/* Load network time (Time interface) */
> +
> +static gboolean
> +parse_cclk_reply (const char *response,
> +                  gchar **iso8601p,
> +                  MMNetworkTimezone **tzp,
> +                  GError **error)

Could you put this in a mm-modem-helpers-telit.c|h set of files in the
telit plugin, and add unit tests? You can check e.g. how that is done
in the Huawei plugin for the ^TIME response parsers.

> +{
> +    GRegex *r;
> +    GMatchInfo *match_info = NULL;
> +    GError *match_error = NULL;
> +    guint year = 0, month = 0, day = 0, hour = 0, minute = 0, second = 0;
> +    gint tz = 0;
> +    gboolean ret = FALSE;
> +
> +    g_assert (iso8601p || tzp); /* at least one */
> +
> +    /* Sample reply: +CCLK: "15/03/05,14:14:26-32" */
> +    r = g_regex_new ("[+]CCLK:
> \"(\\d+)/(\\d+)/(\\d+),(\\d+):(\\d+):(\\d+)([-+]\\d+)\"", 0, 0, NULL);
> +    g_assert (r != NULL);
> +
> +    if (!g_regex_match_full (r, response, -1, 0, 0, &match_info,
> &match_error)) {
> +        if (match_error) {
> +            g_propagate_error (error, match_error);
> +            g_prefix_error (error, "Could not parse +CCLK results: ");
> +        } else {
> +            g_set_error_literal (error,
> +                                 MM_CORE_ERROR,
> +                                 MM_CORE_ERROR_FAILED,
> +                                 "Couldn't match +CCLK reply");
> +        }
> +    } else {
> +        /* Remember that g_match_info_get_match_count() includes match #0
> */
> +        g_assert (g_match_info_get_match_count (match_info) >= 8);
> +
> +        if (mm_get_uint_from_match_info (match_info, 1, &year) &&
> +            mm_get_uint_from_match_info (match_info, 2, &month) &&
> +            mm_get_uint_from_match_info (match_info, 3, &day) &&
> +            mm_get_uint_from_match_info (match_info, 4, &hour) &&
> +            mm_get_uint_from_match_info (match_info, 5, &minute) &&
> +            mm_get_uint_from_match_info (match_info, 6, &second) &&
> +            mm_get_int_from_match_info  (match_info, 7, &tz)) {
> +            /* adjust year */
> +            year += 2000;
> +            /*
> +             * tz = timezone offset in 15 minute intervals
> +             */
> +            if (iso8601p) {
> +                /* Return ISO-8601 format date/time string */
> +                *iso8601p = mm_new_iso8601_time (year, month, day, hour,
> +                                                 minute, second,
> +                                                 TRUE, (tz * 15));
> +            }
> +            if (tzp) {
> +                *tzp = mm_network_timezone_new ();
> +                mm_network_timezone_set_offset (*tzp, tz * 15);
> +            }
> +
> +            ret = TRUE;
> +        } else {
> +            g_set_error_literal (error,
> +                                 MM_CORE_ERROR,
> +                                 MM_CORE_ERROR_FAILED,
> +                                 "Failed to parse +CCLK reply");
> +        }
> +    }
> +
> +    if (match_info)
> +        g_match_info_free (match_info);
> +    g_regex_unref (r);
> +
> +    return ret;
> +}
> +
> +static gchar *
> +modem_time_load_network_time_finish (MMIfaceModemTime *self,
> +                                     GAsyncResult *res,
> +                                     GError **error)
> +{
> +    const gchar *response;
> +    gchar *result = NULL;
> +
> +    response = mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res,
> error);
> +    if (response)
> +        parse_cclk_reply (response, &result, NULL, error);
> +    return result;
> +}
> +
> +static void
> +modem_time_load_network_time (MMIfaceModemTime *self,
> +                              GAsyncReadyCallback callback,
> +                              gpointer user_data)
> +{
> +    mm_base_modem_at_command (MM_BASE_MODEM (self),
> +                              "+CCLK?",
> +                              3,
> +                              FALSE,
> +                              callback,
> +                              user_data);
> +}
> +
> +/*****************************************************************************/
> +/* Load network timezone (Time interface) */
> +
> +static MMNetworkTimezone *
> +modem_time_load_network_timezone_finish (MMIfaceModemTime *self,
> +                                         GAsyncResult *res,
> +                                         GError **error)
> +{
> +    const gchar *response;
> +    MMNetworkTimezone *tz = NULL;
> +
> +    response = mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res,
> NULL);
> +    if (response)
> +        parse_cclk_reply (response, NULL, &tz, error);
> +    return tz;
> +}
> +
> +static void
> +modem_time_load_network_timezone (MMIfaceModemTime *self,
> +                                  GAsyncReadyCallback callback,
> +                                  gpointer user_data)
> +{
> +    mm_base_modem_at_command (MM_BASE_MODEM (self),
> +                              "+CCLK?",
> +                              3,
> +                              FALSE,
> +                              callback,
> +                              user_data);
> +}
> +
> +/*****************************************************************************/
> +/* Check support (Time interface) */
> +
> +static gboolean
> +modem_time_check_support_finish (MMIfaceModemTime *self,
> +                                 GAsyncResult *res,
> +                                 GError **error)
> +{
> +    g_debug("modem_time_check_support_finish");

That previous log message is not useful, please remove it.

> +    return !g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT
> (res), error);

This is wrong. I see that it's also wrong in the novatel plugin, which
does basically the same check.
The correct way to finish() an at_command() operation is to call
at_command_finish(). So, the proper code should be:

    return !!mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res, error);

This is, return TRUE if mm_base_modem_at_command_finish() returned a
valid non-error string.

Fixed that for Novatel:
http://cgit.freedesktop.org/ModemManager/ModemManager/commit/?id=46b2aeae531ea47d3cf3d455305c40694d4f60b7

With this change, we don't assume how
mm_base_modem_at_command_finish() treats the GAsyncResult.

> +}
> +
> +static void
> +modem_time_check_support (MMIfaceModemTime *self,
> +                          GAsyncReadyCallback callback,
> +                          gpointer user_data)
> +{
> +    g_debug("modem_time_check_support");

That previous log message is not useful, please remove it.

> +    mm_base_modem_at_command (MM_BASE_MODEM (self),
> +                              "+CCLK?",
> +                              3,
> +                              TRUE,
> +                              callback,
> +                              user_data);
> +}
> +
> +/*****************************************************************************/
>
>  MMBroadbandModemTelit *
>  mm_broadband_modem_telit_new (const gchar *device,
> @@ -205,6 +366,17 @@ iface_modem_init (MMIfaceModem *iface)
>  }
>
>  static void
> +iface_modem_time_init (MMIfaceModemTime *iface)
> +{
> +    iface->check_support = modem_time_check_support;
> +    iface->check_support_finish = modem_time_check_support_finish;
> +    iface->load_network_time = modem_time_load_network_time;
> +    iface->load_network_time_finish = modem_time_load_network_time_finish;
> +    iface->load_network_timezone = modem_time_load_network_timezone;
> +    iface->load_network_timezone_finish =
> modem_time_load_network_timezone_finish;
> +}
> +
> +static void
>  mm_broadband_modem_telit_class_init (MMBroadbandModemTelitClass *klass)
>  {
>  }
> --
> 2.2.0.rc0.207.ga3a616c
>
>
> _______________________________________________
> ModemManager-devel mailing list
> ModemManager-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
>



-- 
Aleksander
https://aleksander.es


More information about the ModemManager-devel mailing list