[PATCH 2/2] Improve support for network time on Huawei modules

Dan Williams dcbw at redhat.com
Tue Jul 29 12:35:14 PDT 2014


On Tue, 2014-07-29 at 09:35 +1000, David McCullough wrote:
> CDMA modems already had this support, extended to work with 3G
> and LTE modem modules like the MU609 and MU909.
> 
> Unfortunately we can only check the network time once we have an active
> network.  The errors for unsupported commands and no active network are
> the same (error 100).

Thanks for the patch!  Besides what Ben said, I only have a few
comments:

> ---
>  plugins/huawei/mm-broadband-modem-huawei.c | 309 +++++++++++++++++++++++------
>  1 file changed, 249 insertions(+), 60 deletions(-)
> 
> diff --git a/plugins/huawei/mm-broadband-modem-huawei.c b/plugins/huawei/mm-broadband-modem-huawei.c
> index 9cbbfb8..64061d4 100644
> --- a/plugins/huawei/mm-broadband-modem-huawei.c
> +++ b/plugins/huawei/mm-broadband-modem-huawei.c
> @@ -74,6 +74,12 @@ typedef enum {
>      FEATURE_SUPPORTED
>  } FeatureSupport;
>  
> +typedef enum {
> +    TIME_SUPPORT_TRY_ALL,

I would use TIME_SUPPORT_UNKNOWN here to match FEATURE_SUPPORT_UNKNOWN.

> +    TIME_SUPPORT_USE_NWTIME,
> +    TIME_SUPPORT_USE_TIME
> +} TimeSupport;
> +
>  struct _MMBroadbandModemHuaweiPrivate {
>      /* Regex for signal quality related notifications */
>      GRegex *rssi_regex;
> @@ -109,6 +115,8 @@ struct _MMBroadbandModemHuaweiPrivate {
>      FeatureSupport syscfgex_support;
>      FeatureSupport prefmode_support;
>  
> +    TimeSupport time_support;
> +
>      MMModemLocationSource enabled_sources;
>  
>      GArray *syscfg_supported_modes;
> @@ -2840,74 +2848,267 @@ get_detailed_registration_state (MMIfaceModemCdma *self,
>  /*****************************************************************************/
>  /* Load network time (Time interface) */
>  
> -static gchar *
> -modem_time_load_network_time_finish (MMIfaceModemTime *self,
> -                                     GAsyncResult *res,
> -                                     GError **error)
> +static void
> +modem_nwtime_parse (const gchar *response,
> +              gchar **iso8601p,
> +              MMNetworkTimezone **tzp,
> +              GError **error)
>  {
> -    const gchar *response;
>      GRegex *r;
>      GMatchInfo *match_info = NULL;
>      GError *match_error = NULL;
> -    guint year, month, day, hour, minute, second;
> -    gchar *result = NULL;
> -
> -    response = mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res, error);
> -    if (!response)
> -        return NULL;
> +    guint year = 0, month = 0, day = 0, hour = 0, minute = 0, second = 0, dt = 0;
> +    gint tz = 0;
>  
> -    /* Already in ISO-8601 format, but verify just to be sure */
> -    r = g_regex_new ("\\^TIME:\\s*(\\d+)/(\\d+)/(\\d+)\\s*(\\d+):(\\d+):(\\d*)$", 0, 0, NULL);
> +    r = g_regex_new ("\\^NWTIME:\\s*(\\d+)/(\\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 ^TIME results: ");
> +            g_prefix_error (error, "Could not parse ^NWTIME results: ");
>          } else {
>              g_set_error_literal (error,
>                                   MM_CORE_ERROR,
>                                   MM_CORE_ERROR_FAILED,
> -                                 "Couldn't match ^TIME reply");
> +                                 "Couldn't match ^NWTIME reply");
>          }
> -    } else {
> -        /* Remember that g_match_info_get_match_count() includes match #0 */
> -        g_assert (g_match_info_get_match_count (match_info) >= 7);
> -
> -        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)) {
> +        return;
> +    }
> +    /* Remember that g_match_info_get_match_count() includes match #0 */
> +    g_assert (g_match_info_get_match_count (match_info) >= 9);
> +
> +    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) &&
> +        mm_get_uint_from_match_info (match_info, 8, &dt)) {
> +        /* adjust year */
> +        if (year < 100)
> +            year += 2000;
> +        /*
> +         * tz = timezone offset in 15 minute intervals
> +         * dt = daylight adjustment, 0 = none, 1 = 1 hour, 2 = 2 hours
> +         *      other values are marked reserved.
> +         */
> +        if (iso8601p) {
>              /* Return ISO-8601 format date/time string */
> -            result = g_strdup_printf ("%04d/%02d/%02d %02d:%02d:%02d",
> -                                      year, month, day, hour, minute, second);
> +            *iso8601p = mm_new_iso8601_time (year, month, day, hour,
> +                                             minute, second,
> +                                             TRUE, (tz * 15) + (dt * 60));
> +        }
> +        if (tzp) {
> +            *tzp = mm_network_timezone_new ();
> +            mm_network_timezone_set_offset (*tzp, tz * 15);
> +            mm_network_timezone_set_dst_offset (*tzp, dt * 60);
> +        }
> +    } else {
> +        g_set_error_literal (error,
> +                             MM_CORE_ERROR,
> +                             MM_CORE_ERROR_FAILED,
> +                             "Failed to parse ^NWTIME reply");
> +        return;
> +    }
> +
> +    if (match_info)
> +        g_match_info_free (match_info);
> +    g_regex_unref (r);
> +
> +    return;
> +}
> +
> +static void
> +modem_time_parse (const gchar *response,
> +              gchar **iso8601p,
> +              MMNetworkTimezone **tzp,
> +              GError **error)
> +{
> +    GRegex *r;
> +    GMatchInfo *match_info = NULL;
> +    GError *match_error = NULL;
> +    guint year, month, day, hour, minute, second;
> +
> +    /* Already in ISO-8601 format, but verify just to be sure */
> +    r = g_regex_new ("\\^TIME:\\s*(\\d+)/(\\d+)/(\\d+)\\s*(\\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 ^TIME results: ");
>          } else {
>              g_set_error_literal (error,
>                                   MM_CORE_ERROR,
>                                   MM_CORE_ERROR_FAILED,
> -                                 "Failed to parse ^TIME reply");
> +                                 "Couldn't match ^TIME reply");
>          }
> +        return;
> +    }
> +    /* Remember that g_match_info_get_match_count() includes match #0 */
> +    g_assert (g_match_info_get_match_count (match_info) >= 7);
> +
> +    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)) {
> +        /* Return ISO-8601 format date/time string */
> +        if (iso8601p)
> +            *iso8601p = mm_new_iso8601_time (year, month, day, hour,
> +                                             minute, second, FALSE, 0);
> +        if (tzp) {
> +            /* not implemented/available for this modem ?
> +             * *tzp = mm_network_timezone_new ();
> +             * mm_network_timezone_set_offset (*tzp, 0); no TZ
> +             * mm_network_timezone_set_dst_offset (*tzp, dt * 60);
> +             */
> +            *tzp = NULL;
> +        }
> +    } else {
> +        g_set_error_literal (error,
> +                             MM_CORE_ERROR,
> +                             MM_CORE_ERROR_FAILED,
> +                             "Failed to parse ^TIME reply");
> +        return;
>      }
>  
>      if (match_info)
>          g_match_info_free (match_info);
>      g_regex_unref (r);
> -    return result;
> +
> +    return;
> +}
> +
> +static gchar *
> +modem_time_load_network_time_finish (MMIfaceModemTime *_self,
> +                                     GAsyncResult *res,
> +                                     GError **error)
> +{
> +    MMBroadbandModemHuawei *self = MM_BROADBAND_MODEM_HUAWEI (_self);
> +    gchar *iso8601p = NULL;
> +    GVariant *cmd_result;
> +    const gchar *response;
> +
> +    cmd_result = mm_base_modem_at_sequence_full_finish (MM_BASE_MODEM (_self),
> +                                                        res, NULL, error);
> +    if (!cmd_result) {
> +        /* We'll assume we can retry a bit later */
> +        g_set_error (error,
> +                     MM_CORE_ERROR,
> +                     MM_CORE_ERROR_RETRY,
> +                     "Retry");
> +        return NULL;
> +    }
> +
> +    response = g_variant_get_string (cmd_result, NULL);
> +
> +    if (self->priv->time_support == TIME_SUPPORT_USE_NWTIME)
> +        modem_nwtime_parse(response, &iso8601p, NULL, error);
> +    else if (self->priv->time_support == TIME_SUPPORT_USE_TIME)
> +        modem_time_parse(response, &iso8601p, NULL, error);
> +
> +    return iso8601p;
>  }
>  
> +static MMNetworkTimezone *
> +modem_time_load_network_timezone_finish (MMIfaceModemTime *_self,
> +                                         GAsyncResult *res,
> +                                         GError **error)
> +{
> +    MMBroadbandModemHuawei *self = MM_BROADBAND_MODEM_HUAWEI (_self);
> +    MMNetworkTimezone *tz = NULL;
> +    GVariant *cmd_result;
> +    const gchar *response;
> +
> +    cmd_result = mm_base_modem_at_sequence_full_finish (MM_BASE_MODEM (_self),
> +                                                        res, NULL, error);
> +    if (!cmd_result) {
> +        /* We'll assume we can retry a bit later */

Ugh.  I wish there was a better answer to this.  Does your modem reply
to "AT^NWTIME=?" at all, especially before being registered?  Two I
tried don't (E3276/hilink chipset, E398/qualcomm), but they don't
support NWTIME even when registered either.

Dan

> +        g_set_error (error,
> +                     MM_CORE_ERROR,
> +                     MM_CORE_ERROR_RETRY,
> +                     "Retry");
> +        return NULL;
> +    }
> +
> +    response = g_variant_get_string (cmd_result, NULL);
> +
> +    if (self->priv->time_support == TIME_SUPPORT_USE_NWTIME)
> +        modem_nwtime_parse(response, NULL, &tz, error);
> +    else if (self->priv->time_support == TIME_SUPPORT_USE_TIME)
> +        modem_time_parse(response, NULL, &tz, error);
> +
> +    return tz;
> +}
> +
> +static gboolean
> +modem_check_time_reply (MMBaseModem *_self,
> +                  gpointer none,
> +                  const gchar *command,
> +                  const gchar *response,
> +                  gboolean last_command,
> +                  const GError *error,
> +                  GVariant **result,
> +                  GError **result_error)
> +{
> +    MMBroadbandModemHuawei *self = MM_BROADBAND_MODEM_HUAWEI (_self);
> +
> +    if (!error) {
> +        if (strstr (response, "^NWTIME")) {
> +            self->priv->time_support = TIME_SUPPORT_USE_NWTIME;
> +            *result = g_variant_new_string(response);
> +        } else if (strstr (response, "^TIME")) {
> +            self->priv->time_support = TIME_SUPPORT_USE_TIME;
> +            *result = g_variant_new_string(response);
> +        }
> +    }
> +
> +    /* Stop sequence if we have an answer */
> +    return self->priv->time_support != TIME_SUPPORT_TRY_ALL ? TRUE : FALSE;
> +}
> +
> +static const MMBaseModemAtCommand time_cmd_sequence_all[] = {
> +    { "^NWTIME?", 3, FALSE, modem_check_time_reply }, /* 3GPP/LTE */
> +    { "^TIME",    3, FALSE, modem_check_time_reply }, /* CDMA */
> +    { NULL }
> +};
> +
> +static const MMBaseModemAtCommand time_cmd_sequence_nwtime[] = {
> +    { "^NWTIME?", 3, FALSE, modem_check_time_reply }, /* 3GPP/LTE */
> +    { NULL }
> +};
> +
> +static const MMBaseModemAtCommand time_cmd_sequence_time[] = {
> +    { "^TIME",    3, FALSE, modem_check_time_reply }, /* CDMA */
> +    { NULL }
> +};
> +
>  static void
> -modem_time_load_network_time (MMIfaceModemTime *self,
> +modem_time_load_network_time_or_zone (MMIfaceModemTime *_self,
>                                GAsyncReadyCallback callback,
>                                gpointer user_data)
>  {
> -    mm_base_modem_at_command (MM_BASE_MODEM (self),
> -                              "^TIME",
> -                              3,
> -                              FALSE,
> -                              callback,
> -                              user_data);
> +    MMBroadbandModemHuawei *self = MM_BROADBAND_MODEM_HUAWEI (_self);
> +    const MMBaseModemAtCommand *seq;
> +
> +    if (self->priv->time_support == TIME_SUPPORT_USE_NWTIME)
> +        seq = time_cmd_sequence_nwtime;
> +    else if (self->priv->time_support == TIME_SUPPORT_USE_TIME)
> +        seq = time_cmd_sequence_time;
> +    else
> +        seq = time_cmd_sequence_all;
> +
> +    mm_base_modem_at_sequence (
> +        MM_BASE_MODEM (_self),
> +        seq,
> +        NULL, /* response_processor_context */
> +        NULL, /* response_processor_context_free */
> +        callback,
> +        user_data);
>  }
>  
>  /*****************************************************************************/
> @@ -3491,22 +3692,11 @@ modem_time_check_support_finish (MMIfaceModemTime *self,
>                                   GAsyncResult *res,
>                                   GError **error)
>  {
> -    return !g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error);
> -}
> -
> -static void
> -modem_time_check_ready (MMBroadbandModem *self,
> -                        GAsyncResult *res,
> -                        GSimpleAsyncResult *simple)
> -{
> -    GError *error = NULL;
> -
> -    mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res, &error);
> -    if (error)
> -        g_simple_async_result_take_error (simple, error);
> -
> -    g_simple_async_result_complete (simple);
> -    g_object_unref (simple);
> +    /* We need to assume Huawei devices always support fetching the time
> +     * since they report error 100 if not yet connected to the network.
> +     * Do our best to runtime determine the access method later.
> +     */
> +    return TRUE;
>  }
>  
>  static void
> @@ -3521,13 +3711,8 @@ modem_time_check_support (MMIfaceModemTime *self,
>                                          user_data,
>                                          modem_time_check_support);
>  
> -    /* Only CDMA devices support this at the moment */
> -    mm_base_modem_at_command (MM_BASE_MODEM (self),
> -                              "^TIME",
> -                              3,
> -                              TRUE,
> -                              (GAsyncReadyCallback)modem_time_check_ready,
> -                              result);
> +    g_simple_async_result_complete_in_idle (result);
> +    g_object_unref (result);
>  }
>  
>  /*****************************************************************************/
> @@ -3714,6 +3899,8 @@ mm_broadband_modem_huawei_init (MMBroadbandModemHuawei *self)
>      self->priv->syscfg_support = FEATURE_SUPPORT_UNKNOWN;
>      self->priv->syscfgex_support = FEATURE_SUPPORT_UNKNOWN;
>      self->priv->prefmode_support = FEATURE_SUPPORT_UNKNOWN;
> +
> +    self->priv->time_support = TIME_SUPPORT_TRY_ALL;
>  }
>  
>  static void
> @@ -3845,8 +4032,10 @@ 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 = modem_time_load_network_time_or_zone;
>      iface->load_network_time_finish = modem_time_load_network_time_finish;
> +    iface->load_network_timezone = modem_time_load_network_time_or_zone;
> +    iface->load_network_timezone_finish = modem_time_load_network_timezone_finish;
>  }
>  
>  static void




More information about the ModemManager-devel mailing list