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

Dan Williams dcbw at redhat.com
Wed Jul 30 07:07:11 PDT 2014


On Wed, 2014-07-30 at 08:05 +1000, David McCullough wrote:
> Dan Williams wrote the following:
> > 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
> 
> I haven't seen Bens comments for some reason.

I think he just did a normal reply-all, but I've forwarded his mail to
you anyway.

> > 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.
> 
> 
> No problems,  will update.
> 
> 
> > > +    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.
> 
> 
> No,  nothing I could find to indicate that it supports NWTIME or TIME.
> Until is is registered all combinations I could contrive of the NWTIME
> command return the same results as completely invalid commands.
> 
> I went through the complete command set to see if there was anyway I could
> imply it would be supported but I could not find anything.

Does your device support "AT^NTCT=?" or "AT^NTCT?" ?  Also, what do you
get for "AT^CURC=?" and "AT^CURC?" ? 

> If there was a way in MM to enable time support later then there are
> several options (I am pretty sure the modem sends ^NWTIME updates without
> prompting,  or,  we could just test/enable once registered. 

Yeah, NWTIME is supposed to be one of the unsolicited commands that is
turned on with CURC=1 which MM should set pretty early.  Also, with the
modems you  have, does the time change with each NWTIME request, or does
it stay the same?

Dan

> Cheers,
> Davidm
> 
> 
> 
> 
> > 
> > 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