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

Aleksander Morgado aleksander at aleksander.es
Tue Aug 5 04:28:21 PDT 2014


Hey!

Some comments inline.

On Tue, Aug 5, 2014 at 12:43 PM, David McCullough
<david.mccullough at accelecon.com> wrote:
>
> Second revision of Huawei nwtime support.  Takes on feedback from the
> mailing list including helpers,  some basic tests and use of the ^NTCT
> command to determine network time support (^NWTIME).  The helper tests
> need expanding once the approach is confirmed as acceptable.
>
> Signed-off-by:    David McCullough <david.mccullough at accelecon.com>
> ---
>  plugins/huawei/mm-broadband-modem-huawei.c       | 173 ++++++++++++++---------
>  plugins/huawei/mm-modem-helpers-huawei.c         | 145 +++++++++++++++++++
>  plugins/huawei/mm-modem-helpers-huawei.h         |  16 +++
>  plugins/huawei/tests/test-modem-helpers-huawei.c | 129 +++++++++++++++++
>  4 files changed, 394 insertions(+), 69 deletions(-)
>
> diff --git a/plugins/huawei/mm-broadband-modem-huawei.c b/plugins/huawei/mm-broadband-modem-huawei.c
> index 9cbbfb8..84d0215 100644
> --- a/plugins/huawei/mm-broadband-modem-huawei.c
> +++ b/plugins/huawei/mm-broadband-modem-huawei.c
> @@ -108,6 +108,8 @@ struct _MMBroadbandModemHuaweiPrivate {
>      FeatureSupport syscfg_support;
>      FeatureSupport syscfgex_support;
>      FeatureSupport prefmode_support;
> +    FeatureSupport time_support;
> +    FeatureSupport nwtime_support;
>
>      MMModemLocationSource enabled_sources;
>
> @@ -2840,74 +2842,69 @@ get_detailed_registration_state (MMIfaceModemCdma *self,
>  /*****************************************************************************/
>  /* Load network time (Time interface) */
>
> +static MMNetworkTimezone *
> +modem_time_load_network_timezone_finish (MMIfaceModemTime *_self,
> +                                         GAsyncResult *res,
> +                                         GError **error)
> +{
> +    MMBroadbandModemHuawei *self = MM_BROADBAND_MODEM_HUAWEI (_self);
> +    MMNetworkTimezone *tz = NULL;
> +    const gchar *response;
> +    gboolean ret = FALSE;
> +
> +    response = mm_base_modem_at_command_finish (MM_BASE_MODEM (_self), res, error);
> +    if (!response)
> +        return NULL;

How about adding this? This method must only be called when Time
support is available.
g_assert (self->priv->nwtime_support == FEATURE_SUPPORTED ||
self->priv->time_support == FEATURE_SUPPORTED);

> +
> +    if (self->priv->nwtime_support == FEATURE_SUPPORTED)
> +        ret = mm_huawei_parse_nwtime_response (response, NULL, &tz, error);
> +    else if (self->priv->time_support == FEATURE_SUPPORTED)
> +        ret = mm_huawei_parse_time_response (response, NULL, &tz, error);
> +    return ret ? tz : NULL;

Why not just this, and avoid the "ret" variable?
    return tz;


> +}
> +
> +
>  static gchar *
> -modem_time_load_network_time_finish (MMIfaceModemTime *self,
> +modem_time_load_network_time_finish (MMIfaceModemTime *_self,
>                                       GAsyncResult *res,
>                                       GError **error)
>  {
> +    MMBroadbandModemHuawei *self = MM_BROADBAND_MODEM_HUAWEI (_self);
>      const gchar *response;
> -    GRegex *r;
> -    GMatchInfo *match_info = NULL;
> -    GError *match_error = NULL;
> -    guint year, month, day, hour, minute, second;
> -    gchar *result = NULL;
> +    gchar *iso8601 = NULL;
> +    gboolean ret = FALSE;
>
> -    response = mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res, error);
> +    response = mm_base_modem_at_command_finish (MM_BASE_MODEM (_self), res, error);
>      if (!response)
>          return NULL;
>
> -    /* 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,
> -                                 "Couldn't match ^TIME 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 ISO-8601 format date/time string */
> -            result = g_strdup_printf ("%04d/%02d/%02d %02d:%02d:%02d",
> -                                      year, month, day, hour, minute, second);
> -        } else {
> -            g_set_error_literal (error,
> -                                 MM_CORE_ERROR,
> -                                 MM_CORE_ERROR_FAILED,
> -                                 "Failed to parse ^TIME reply");
> -        }
> -    }
> -
> -    if (match_info)
> -        g_match_info_free (match_info);
> -    g_regex_unref (r);
> -    return result;

Same here, I would include:
g_assert (self->priv->nwtime_support == FEATURE_SUPPORTED ||
self->priv->time_support == FEATURE_SUPPORTED);

> +    if (self->priv->nwtime_support == FEATURE_SUPPORTED)
> +        ret = mm_huawei_parse_nwtime_response (response, &iso8601, NULL, error);
> +    else if (self->priv->time_support == FEATURE_SUPPORTED)
> +        ret = mm_huawei_parse_time_response (response, &iso8601, NULL, error);
> +    return ret ? iso8601 : NULL;

And again, just this, avoiding the ret variable:

    return iso8601;

>  }
>
>  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);
> +    const char *command = NULL;
> +    MMBroadbandModemHuawei *self = MM_BROADBAND_MODEM_HUAWEI (_self);
> +
> +    if (self->priv->nwtime_support == FEATURE_SUPPORTED)
> +        command = "^NWTIME?";
> +    else if (self->priv->time_support == FEATURE_SUPPORTED)
> +        command = "^TIME";
> +
> +    if (command)
> +        mm_base_modem_at_command (MM_BASE_MODEM (self),
> +                                  command,
> +                                  3,
> +                                  FALSE,
> +                                  callback,
> +                                  user_data);

This is an async method, and therefore it must *always* have an async
method call inside. If (!command) nothing will be executed and the
async method will never be completed. BUT, yes, this method will only
be called when Time interface support is in place, and therefore it
will logically always have either self->priv->nwtime_support or
self->priv->time_support set to FEATURE_SUPPORTED. In order to
emphasize that, I would therefore rewrite the code so that, instead we
have:

    g_assert (command != NULL);
    mm_base_modem_at_command (MM_BASE_MODEM (self), command, 3....);

This is, no if (command), a g_assert instead.

>  }
>
>  /*****************************************************************************/
> @@ -3487,28 +3484,63 @@ enable_location_gathering (MMIfaceModemLocation *self,
>  /* Check support (Time interface) */
>
>  static gboolean
> -modem_time_check_support_finish (MMIfaceModemTime *self,
> +modem_time_check_support_finish (MMIfaceModemTime *_self,
>                                   GAsyncResult *res,
>                                   GError **error)
>  {
> -    return !g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error);
> +    MMBroadbandModemHuawei *self = MM_BROADBAND_MODEM_HUAWEI (_self);
> +    if (self->priv->nwtime_support == FEATURE_SUPPORTED)
> +        return TRUE;
> +    if (self->priv->time_support == FEATURE_SUPPORTED)
> +        return TRUE;
> +    return FALSE;

Just a side note in case anyone wondered, these check_support()
methods are allowed to return FALSE without error being set.

>  }
>
>  static void
> -modem_time_check_ready (MMBroadbandModem *self,
> +modem_time_check_ready (MMBaseModem *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);
> -
> +    (void)mm_base_modem_at_sequence_finish (self, res, NULL, &error);
> +    g_clear_error (&error);

If you're not interested on the result nor the error, you can just do:
    mm_base_modem_at_sequence_finish (self, res, NULL, NULL);

>      g_simple_async_result_complete (simple);
>      g_object_unref (simple);
>  }
>
> +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, "^NTCT"))
> +            self->priv->nwtime_support = FEATURE_SUPPORTED;
> +        else if (strstr (response, "^TIME"))
> +            self->priv->time_support = FEATURE_SUPPORTED;
> +    } else {
> +        if (strstr (command, "^NTCT"))
> +            self->priv->nwtime_support = FEATURE_NOT_SUPPORTED;
> +        else if (strstr (command, "^TIME"))
> +            self->priv->time_support = FEATURE_NOT_SUPPORTED;
> +    }
> +
> +    return FALSE;
> +}
> +
> +static const MMBaseModemAtCommand time_cmd_sequence[] = {
> +    { "^NTCT?", 3, FALSE, modem_check_time_reply }, /* 3GPP/LTE */
> +    { "^TIME",  3, FALSE, modem_check_time_reply }, /* CDMA */
> +    { NULL }
> +};
> +
>  static void
>  modem_time_check_support (MMIfaceModemTime *self,
>                            GAsyncReadyCallback callback,
> @@ -3521,13 +3553,12 @@ 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);
> +    mm_base_modem_at_sequence (MM_BASE_MODEM (self),
> +                               time_cmd_sequence,
> +                               NULL, /* response_processor_context */
> +                               NULL, /* response_processor_context_free */
> +                               (GAsyncReadyCallback)modem_time_check_ready,
> +                               result);
>  }
>
>  /*****************************************************************************/
> @@ -3714,6 +3745,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->nwtime_support = FEATURE_SUPPORT_UNKNOWN;
> +    self->priv->time_support = FEATURE_SUPPORT_UNKNOWN;
>  }
>
>  static void
> @@ -3845,8 +3878,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
> diff --git a/plugins/huawei/mm-modem-helpers-huawei.c b/plugins/huawei/mm-modem-helpers-huawei.c
> index 656c6de..7dd554b 100644
> --- a/plugins/huawei/mm-modem-helpers-huawei.c
> +++ b/plugins/huawei/mm-modem-helpers-huawei.c
> @@ -1033,3 +1033,148 @@ mm_huawei_parse_syscfgex_response (const gchar *response,
>      g_strfreev (split);
>      return NULL;
>  }
> +
> +/*****************************************************************************/
> +/* ^NWTIME response parser */
> +
> +gboolean mm_huawei_parse_nwtime_response (const gchar *response,
> +                                          gchar **iso8601p,
> +                                          MMNetworkTimezone **tzp,
> +                                          GError **error)
> +{
> +    GRegex *r;
> +    GMatchInfo *match_info = NULL;
> +    GError *match_error = NULL;
> +    guint year = 0, month = 0, day = 0, hour = 0, minute = 0, second = 0, dt = 0;
> +    gint tz = 0;
> +    gboolean ret = TRUE;

I would start with "ret = FALSE", and only set it to TRUE if we're
building correct output to set in 'tzp' or 'iso8601p'.

> +
> +    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 ^NWTIME results: ");
> +        } else {
> +            g_set_error_literal (error,
> +                                 MM_CORE_ERROR,
> +                                 MM_CORE_ERROR_FAILED,
> +                                 "Couldn't match ^NWTIME reply");
> +        }
> +
> +        ret = FALSE;
> +    } else {
> +        /* 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 */
> +                *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");
> +            ret = FALSE;
> +        }
> +    }
> +
> +    if (match_info)
> +        g_match_info_free (match_info);
> +    g_regex_unref (r);
> +
> +    return ret;
> +}
> +
> +/*****************************************************************************/
> +/* ^TIME response parser */
> +
> +gboolean mm_huawei_parse_time_response (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;
> +    gboolean ret = TRUE;

Same here, I would start with "ret = FALSE", and only set it to TRUE
if we're building correct output to set in 'tzp' or 'iso8601p'.

> +
> +    /* 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,
> +                                 "Couldn't match ^TIME reply");
> +        }
> +        ret = FALSE;
> +    } 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 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;

This is not worth a TRUE response; it should be returning FALSE and an
error set to e.g. MM_CORE_ERROR_UNSUPPORTED. Doing this, we make sure
that TRUE is returned *only* when the requested outputs are filled in.
If you do this by setting the error here; also make sure you:
    if (iso8601p)
      g_free (*iso8601p);

The implementation in the huawei plugin will actually  request either
time or timezone, so it should never happen that you really need to
free iso8601p, but well, just in case. If we ever end up needing
requesting both time and timezone at the same time then it may be
actually worth to split the parsers, one for time and another one for
timezone. So I really wouldn't mind never allowing asking for both at
the same time:

g_assert (iso8601p || tzp);        /* at least one */
g_assert (!(iso8601p && tzp)); /* never both */

If you add these to the beginning of the parsers, you can actually
forget the extra free() I'm talking about; your choice.

> +            }
> +        } else {
> +            g_set_error_literal (error,
> +                                 MM_CORE_ERROR,
> +                                 MM_CORE_ERROR_FAILED,
> +                                 "Failed to parse ^TIME reply");
> +            ret = FALSE;
> +        }
> +    }
> +
> +    if (match_info)
> +        g_match_info_free (match_info);
> +    g_regex_unref (r);
> +
> +    return ret;
> +}
> +
> diff --git a/plugins/huawei/mm-modem-helpers-huawei.h b/plugins/huawei/mm-modem-helpers-huawei.h
> index 05894f3..e225e90 100644
> --- a/plugins/huawei/mm-modem-helpers-huawei.h
> +++ b/plugins/huawei/mm-modem-helpers-huawei.h
> @@ -113,4 +113,20 @@ const MMHuaweiSyscfgexCombination *mm_huawei_parse_syscfgex_response (const gcha
>                                                                        const GArray *supported_mode_combinations,
>                                                                        GError **error);
>
> +/*****************************************************************************/
> +/* ^NWTIME response parser */
> +
> +gboolean mm_huawei_parse_nwtime_response (const gchar *response,
> +                                          gchar **iso8601p,
> +                                          MMNetworkTimezone **tzp,
> +                                          GError **error);
> +
> +/*****************************************************************************/
> +/* ^TIME response parser */
> +
> +gboolean mm_huawei_parse_time_response (const gchar *response,
> +                                        gchar **iso8601p,
> +                                        MMNetworkTimezone **tzp,
> +                                        GError **error);
> +
>  #endif  /* MM_MODEM_HELPERS_HUAWEI_H */
> diff --git a/plugins/huawei/tests/test-modem-helpers-huawei.c b/plugins/huawei/tests/test-modem-helpers-huawei.c
> index b74821f..7e79106 100644
> --- a/plugins/huawei/tests/test-modem-helpers-huawei.c
> +++ b/plugins/huawei/tests/test-modem-helpers-huawei.c
> @@ -1002,6 +1002,133 @@ test_syscfgex_response (void)
>  }
>
>  /*****************************************************************************/
> +/* Test ^NWTIME responses */
> +
> +typedef struct {
> +    const gchar *str;
> +    gboolean ret;
> +    gboolean test_iso8601;
> +    gboolean test_tz;
> +    gchar *iso8601;
> +    gint32 offset;
> +    gint32 dst_offset;
> +    gint32 leap_seconds;
> +} NwtimeTest;
> +
> +#define NWT_UNKNOWN    MM_NETWORK_TIMEZONE_LEAP_SECONDS_UNKNOWN
> +
> +static const NwtimeTest nwtime_tests[] = {
> +    { "^NWTIME: 14/08/05,04:00:21+40,00", TRUE, FALSE, FALSE,
> +        "2014-08-05T04:00:21+10:00", 600, 0, NWT_UNKNOWN },
> +    { "^NWTIME: 14/08/05,04:00:21+40,00", TRUE, TRUE, FALSE,
> +        "2014-08-05T04:00:21+10:00", 600, 0, NWT_UNKNOWN },
> +    { "^NWTIME: 14/08/05,04:00:21+40,00", TRUE, FALSE, TRUE,
> +        "2014-08-05T04:00:21+10:00", 600, 0, NWT_UNKNOWN },
> +    { "^NWTIME: 14/08/05,04:00:21+40,00", TRUE, TRUE, TRUE,
> +        "2014-08-05T04:00:21+10:00", 600, 0, NWT_UNKNOWN },
> +    { "^NWTIME: XX/XX/XX,XX:XX:XX+XX,XX", FALSE, FALSE, FALSE,
> +        NULL, NWT_UNKNOWN, NWT_UNKNOWN, NWT_UNKNOWN },
> +    { "^TIME: XX/XX/XX,XX:XX:XX+XX,XX", FALSE, FALSE, FALSE,
> +        NULL, NWT_UNKNOWN, NWT_UNKNOWN, NWT_UNKNOWN },
> +    { "^TIME: 14/08/05,04:00:21+40,00", FALSE, FALSE, FALSE,
> +        NULL, NWT_UNKNOWN, NWT_UNKNOWN, NWT_UNKNOWN },
> +    { NULL, FALSE, FALSE, FALSE, NULL, NWT_UNKNOWN, NWT_UNKNOWN, NWT_UNKNOWN }
> +};
> +
> +static void
> +test_nwtime (void)
> +{
> +    guint i;
> +
> +    for (i = 0; nwtime_tests[i].str; i++) {
> +        GError *error = NULL;
> +        gchar *iso8601;
> +        MMNetworkTimezone *tz;
> +        gboolean ret;
> +
> +        iso8601 = NULL;
> +        tz = NULL;
> +
> +        ret = mm_huawei_parse_nwtime_response (nwtime_tests[i].str,
> +                                               nwtime_tests[i].test_iso8601 ? &iso8601 : NULL,
> +                                               nwtime_tests[i].test_tz ? &tz : NULL,
> +                                               &error);
> +
> +        g_assert (ret == nwtime_tests[i].ret);
> +
> +        if (nwtime_tests[i].test_iso8601)
> +            g_assert_cmpstr (nwtime_tests[i].iso8601, ==, iso8601);
> +
> +        if (nwtime_tests[i].test_tz) {
> +            g_assert (nwtime_tests[i].offset == mm_network_timezone_get_offset (tz));
> +            g_assert (nwtime_tests[i].dst_offset == mm_network_timezone_get_dst_offset (tz));
> +            g_assert (nwtime_tests[i].leap_seconds == mm_network_timezone_get_leap_seconds (tz));
> +        }
> +
> +        if (iso8601)
> +            g_free (iso8601);
> +    }
> +}
> +
> +/*****************************************************************************/
> +/* Test ^TIME responses */
> +
> +typedef struct {
> +    const gchar *str;
> +    gboolean ret;
> +    gboolean test_iso8601;
> +    gboolean test_tz;
> +    gchar *iso8601;
> +} TimeTest;
> +
> +static const TimeTest time_tests[] = {
> +    { "^TIME: XX/XX/XX,XX:XX:XX+XX,XX", FALSE, FALSE, FALSE,
> +        NULL },
> +#if 0 /* no examples for this yet */
> +    { "^TIME: ???????????????????????", TRUE, FALSE, FALSE,
> +        "2014-08-05T03:49:36+10:00" },
> +    { "^TIME: ???????????????????????", TRUE, TRUE, FALSE,
> +        "2014-08-05T03:49:36+10:00" },
> +    { "^TIME: ???????????????????????", TRUE, FALSE, TRUE,
> +        "2014-08-05T03:49:36+10:00" },
> +    { "^TIME: ???????????????????????", TRUE, TRUE, TRUE,
> +        "2014-08-05T03:49:36+10:00" },
> +#endif


Is it that we don't have any examples, or that they were not added yet?



> +    { NULL, FALSE, FALSE, FALSE, NULL }
> +};
> +
> +static void
> +test_time (void)
> +{
> +    guint i;
> +
> +    for (i = 0; time_tests[i].str; i++) {
> +        GError *error = NULL;
> +        gchar *iso8601;
> +        MMNetworkTimezone *tz;
> +        gboolean ret;
> +
> +        iso8601 = NULL;
> +        tz = NULL;
> +
> +        ret = mm_huawei_parse_time_response (time_tests[i].str,
> +                                             time_tests[i].test_iso8601 ? &iso8601 : NULL,
> +                                             time_tests[i].test_tz ? &tz : NULL,
> +                                             &error);
> +
> +        g_assert (ret == time_tests[i].ret);
> +
> +        if (time_tests[i].test_iso8601)
> +            g_assert_cmpstr (time_tests[i].iso8601, ==, iso8601);
> +
> +        g_assert (tz == NULL); /* TZ is not supported */
> +
> +        if (iso8601)
> +            g_free (iso8601);
> +    }
> +}
> +
> +/*****************************************************************************/
>
>  void
>  _mm_log (const char *loc,
> @@ -1039,6 +1166,8 @@ int main (int argc, char **argv)
>      g_test_add_func ("/MM/huawei/syscfg/response", test_syscfg_response);
>      g_test_add_func ("/MM/huawei/syscfgex", test_syscfgex);
>      g_test_add_func ("/MM/huawei/syscfgex/response", test_syscfgex_response);
> +    g_test_add_func ("/MM/huawei/nwtime", test_nwtime);
> +    g_test_add_func ("/MM/huawei/time", test_time);
>
>      return g_test_run ();
>  }
> --
> 1.8.2.rc0
>

-- 
Aleksander
https://aleksander.es


More information about the ModemManager-devel mailing list