<div dir="ltr">Argh.<div> </div><div>Stupid mistake. My local branch was tracking the outdated chromium tree, and I never noticed.</div><div> </div></div><div class="gmail_extra"> <div class="gmail_quote">On Wed, Oct 9, 2013 at 1:03 AM, Aleksander Morgado <<a href="mailto:aleksander@lanedo.com" target="_blank">aleksander@lanedo.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On 08/10/13 22:31, Prathmesh Prabhu wrote: > This patch uses the extended error code after a registration failure to > determine if the SIM in the relevant modem is associated with a non-provisioned > account. It uses the standard AT+CEER command, but the implementation is > restricted to the altair plugin. </div>Now I got lost. Your V2 patch contains several of the fixes I suggested in the code review; but this V3 patch doesn't have them... <div class="HOEnZb"><div class="h5"> > --- > plugins/Makefile.am | 13 ++ > plugins/altair/mm-broadband-modem-altair-lte.c | 138 ++++++++++++++++++++- > plugins/altair/mm-modem-helpers-altair-lte.c | 68 ++++++++++ > plugins/altair/mm-modem-helpers-altair-lte.h | 26 ++++ > .../altair/tests/test-modem-helpers-altair-lte.c | 75 +++++++++++ > 5 files changed, 317 insertions(+), 3 deletions(-) > create mode 100644 plugins/altair/mm-modem-helpers-altair-lte.c > create mode 100644 plugins/altair/mm-modem-helpers-altair-lte.h > create mode 100644 plugins/altair/tests/test-modem-helpers-altair-lte.c > > diff --git a/plugins/Makefile.am b/plugins/Makefile.am > index d10c929..d775d5a 100644 > --- a/plugins/Makefile.am > +++ b/plugins/Makefile.am > @@ -348,6 +348,8 @@ libmm_plugin_novatel_la_LDFLAGS = $(PLUGIN_COMMON_LINKER_FLAGS) > > # Altair LTE modem > libmm_plugin_altair_lte_la_SOURCES = \ > + altair/mm-modem-helpers-altair-lte.c \ > + altair/mm-modem-helpers-altair-lte.h \ > altair/mm-plugin-altair-lte.c \ > altair/mm-plugin-altair-lte.h \ > altair/mm-broadband-modem-altair-lte.c \ > @@ -357,6 +359,17 @@ libmm_plugin_altair_lte_la_SOURCES = \ > libmm_plugin_altair_lte_la_CPPFLAGS = $(PLUGIN_COMMON_COMPILER_FLAGS) > libmm_plugin_altair_lte_la_LDFLAGS = $(PLUGIN_COMMON_LINKER_FLAGS) > > +noinst_PROGRAMS += test-modem-helpers-altair-lte > +test_modem_helpers_altair_lte_SOURCES = \ > + altair/mm-modem-helpers-altair-lte.c \ > + altair/mm-modem-helpers-altair-lte.h \ > + altair/tests/test-modem-helpers-altair-lte.c > +test_modem_helpers_altair_lte_CPPFLAGS = \ > + -I$(top_srcdir)/plugins/altair \ > + $(PLUGIN_COMMON_COMPILER_FLAGS) > +test_modem_helpers_altair_lte_LDFLAGS = $(top_builddir)/libmm-glib/<a href="http://libmm-glib.la" target="_blank">libmm-glib.la</a> > + > + > # VIA modem > libmm_plugin_via_la_SOURCES = \ > via/mm-plugin-via.c \ > diff --git a/plugins/altair/mm-broadband-modem-altair-lte.c b/plugins/altair/mm-broadband-modem-altair-lte.c > index 3303cbb..da9510b 100644 > --- a/plugins/altair/mm-broadband-modem-altair-lte.c > +++ b/plugins/altair/mm-broadband-modem-altair-lte.c > @@ -24,6 +24,9 @@ > #include <ctype.h> > > #include "ModemManager.h" > +#define _LIBMM_INSIDE_MM > +#include <libmm-glib.h> > + > #include "mm-base-modem-at.h" > #include "mm-broadband-bearer-altair-lte.h" > #include "mm-broadband-modem-altair-lte.h" > @@ -34,6 +37,7 @@ > #include "mm-iface-modem-messaging.h" > #include "mm-log.h" > #include "mm-modem-helpers.h" > +#include "mm-modem-helpers-altair-lte.h" > #include "mm-serial-parsers.h" > #include "mm-bearer-list.h" > > @@ -465,6 +469,135 @@ reset (MMIfaceModem *self, > } > > /*****************************************************************************/ > +/* Run registration checks (3GPP interface) */ > + > +typedef struct { > + MMBroadbandModemAltairLte *self; > + GSimpleAsyncResult *result; > +} RunRegistrationChecksContext; > + > +static void > +run_registration_checks_context_complete_and_free (RunRegistrationChecksContext *ctx) > +{ > + g_simple_async_result_complete_in_idle (ctx->result); > + g_object_unref (ctx->result); > + g_object_unref (ctx->self); > + g_free (ctx); > +} > + > +static void > +run_registration_checks_subscription_state_ready (MMIfaceModem3gpp *self, > + GAsyncResult *res, > + RunRegistrationChecksContext *ctx) > +{ > + GError *error = NULL; > + const gchar *at_response; > + gchar *ceer_response; > + > + at_response = mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res, &error); > + if (!at_response) { > + g_assert (error); > + g_simple_async_result_take_error (ctx->result, error); > + run_registration_checks_context_complete_and_free (ctx); > + return; > + } > + > + ceer_response = mm_altair_parse_ceer_response (at_response, &error); > + if (!ceer_response) { > + g_assert (error); > + g_simple_async_result_take_error (ctx->result, error); > + run_registration_checks_context_complete_and_free (ctx); > + return; > + } > + > + g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE); > + > + if (g_strcmp0 ("EPS_AND_NON_EPS_SERVICES_NOT_ALLOWED", ceer_response) == 0) { > + mm_dbg ("Registration failed due to unprovisioned SIM."); > + mm_iface_modem_3gpp_update_subscription_state (self, MM_MODEM_3GPP_SUBSCRIPTION_STATE_UNPROVISIONED); > + } else { > + mm_dbg ("Failed to find a better reason for registration failure."); > + } > + > + run_registration_checks_context_complete_and_free (ctx); > + g_free (ceer_response); > +} > + > +static void > +run_registration_checks_ready (MMIfaceModem3gpp *self, > + GAsyncResult *res, > + RunRegistrationChecksContext *ctx) > +{ > + GError *error = NULL; > + gboolean success; > + MMModem3gppRegistrationState registration_state; > + > + g_assert (iface_modem_3gpp_parent->run_registration_checks_finish); > + success = iface_modem_3gpp_parent->run_registration_checks_finish (self, res, &error); > + if (!success) { > + g_assert (error); > + g_simple_async_result_take_error (ctx->result, error); > + run_registration_checks_context_complete_and_free (ctx); > + return; > + } > + > + g_object_get (self, > + MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, &registration_state, > + NULL); > + > + if (registration_state == MM_MODEM_3GPP_REGISTRATION_STATE_HOME || > + registration_state == MM_MODEM_3GPP_REGISTRATION_STATE_ROAMING) { > + mm_dbg ("Registration succeeded: Marking the SIM as provisioned."); > + mm_iface_modem_3gpp_update_subscription_state (self, MM_MODEM_3GPP_SUBSCRIPTION_STATE_PROVISIONED); > + g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE); > + run_registration_checks_context_complete_and_free (ctx); > + return; > + } > + > + mm_dbg ("Registration not successful yet. Checking if SIM is unprovisioned."); > + mm_base_modem_at_command (MM_BASE_MODEM (self), > + "+CEER", > + 6, > + FALSE, > + (GAsyncReadyCallback) run_registration_checks_subscription_state_ready, > + ctx); > +} > + > +static void > +modem_3gpp_run_registration_checks (MMIfaceModem3gpp *self, > + gboolean cs_supported, > + gboolean ps_supported, > + gboolean eps_supported, > + GAsyncReadyCallback callback, > + gpointer user_data) > +{ > + RunRegistrationChecksContext *ctx; > + > + ctx = g_new0 (RunRegistrationChecksContext, 1); > + ctx->self = g_object_ref (MM_BROADBAND_MODEM_ALTAIR_LTE (self)); > + ctx->result = g_simple_async_result_new (G_OBJECT (self), > + callback, > + user_data, > + modem_3gpp_run_registration_checks); > + > + g_assert (iface_modem_3gpp_parent->run_registration_checks); > + iface_modem_3gpp_parent->run_registration_checks (self, > + cs_supported, > + ps_supported, > + eps_supported, > + (GAsyncReadyCallback) run_registration_checks_ready, > + ctx); > +} > + > +static gboolean > +modem_3gpp_run_registration_checks_finish (MMIfaceModem3gpp *self, > + GAsyncResult *res, > + GError **error) > +{ > + return !!mm_base_modem_at_command_full_finish (MM_BASE_MODEM (self), res, error); > +} > + > +/*****************************************************************************/ > /* Register in network (3GPP interface) */ > > static void > @@ -989,7 +1122,6 @@ iface_modem_init (MMIfaceModem *iface) > iface->setup_charset_finish = NULL; > iface->setup_flow_control = NULL; > iface->setup_flow_control_finish = NULL; > - > } > > static void > @@ -1003,7 +1135,6 @@ iface_modem_3gpp_ussd_init (MMIfaceModem3gppUssd *iface) > static void > iface_modem_3gpp_init (MMIfaceModem3gpp *iface) > { > - > iface_modem_3gpp_parent = g_type_interface_peek_parent (iface); > > iface->setup_unsolicited_events = modem_3gpp_setup_unsolicited_events; > @@ -1017,6 +1148,8 @@ iface_modem_3gpp_init (MMIfaceModem3gpp *iface) > > iface->register_in_network = modem_3gpp_register_in_network; > iface->register_in_network_finish = modem_3gpp_register_in_network_finish; > + iface->run_registration_checks = modem_3gpp_run_registration_checks; > + iface->run_registration_checks_finish = modem_3gpp_run_registration_checks_finish; > > /* Scanning is not currently supported */ > iface->scan_networks = NULL; > @@ -1027,7 +1160,6 @@ iface_modem_3gpp_init (MMIfaceModem3gpp *iface) > iface->load_operator_code_finish = modem_3gpp_load_operator_code_finish; > iface->load_operator_name = modem_3gpp_load_operator_name; > iface->load_operator_name_finish = modem_3gpp_load_operator_name_finish; > - > } > > static void > diff --git a/plugins/altair/mm-modem-helpers-altair-lte.c b/plugins/altair/mm-modem-helpers-altair-lte.c > new file mode 100644 > index 0000000..9e5b421 > --- /dev/null > +++ b/plugins/altair/mm-modem-helpers-altair-lte.c > @@ -0,0 +1,68 @@ > +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ > +/* > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details: > + * > + * Copyright (C) 2013 Google Inc. > + * > + */ > + > +#include <string.h> > + > +#include <ModemManager.h> > +#define _LIBMM_INSIDE_MM > +#include <libmm-glib.h> > + > +#include "mm-modem-helpers-altair-lte.h" > + > +/*****************************************************************************/ > +/* +CEER response parser */ > + > +gchar * > +mm_altair_parse_ceer_response (const gchar *response, > + GError **error) > +{ > + GRegex *r; > + GMatchInfo *match_info = NULL; > + gchar *ceer_response = NULL; > + > + > + /* First accept an empty response as the no error case. Sometimes, the only > + * respone to the AT+CEER query is an OK. > + */ > + if (g_strcmp0 ("", response) == 0) { > + return g_strdup (""); > + } > + > + /* The response we are interested in looks so: > + * +CEER: EPS_AND_NON_EPS_SERVICES_NOT_ALLOWED > + */ > + r = g_regex_new ("\\+CEER:\\s*(\\w*)?", > + G_REGEX_RAW, > + 0, NULL); > + g_assert (r != NULL); > + > + if (!g_regex_match (r, response, 0, &match_info)) { > + g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED, "Could not parse +CEER response"); > + g_match_info_free (match_info); > + g_regex_unref (r); > + return NULL; > + } > + > + if (g_match_info_matches (match_info)) { > + ceer_response = mm_get_string_unquoted_from_match_info (match_info, 1); > + if (!ceer_response) > + ceer_response = g_strdup (""); > + } > + > + g_match_info_free (match_info); > + g_regex_unref (r); > + return ceer_response; > +} > diff --git a/plugins/altair/mm-modem-helpers-altair-lte.h b/plugins/altair/mm-modem-helpers-altair-lte.h > new file mode 100644 > index 0000000..dbd641c > --- /dev/null > +++ b/plugins/altair/mm-modem-helpers-altair-lte.h > @@ -0,0 +1,26 @@ > +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ > +/* > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details: > + * > + * Copyright (C) 2013 Google Inc. > + * > + */ > + > +#ifndef MM_MODEM_HELPERS_ALTAIR_H > +#define MM_MODEM_HELPERS_ALTAIR_H > + > +#include <glib.h> > + > +/* +CEER response parser */ > +gchar *mm_altair_parse_ceer_response (const gchar *response, > + GError **error); > + > +#endif /* MM_MODEM_HELPERS_ALTAIR_H */ > diff --git a/plugins/altair/tests/test-modem-helpers-altair-lte.c b/plugins/altair/tests/test-modem-helpers-altair-lte.c > new file mode 100644 > index 0000000..1b3eaa1 > --- /dev/null > +++ b/plugins/altair/tests/test-modem-helpers-altair-lte.c > @@ -0,0 +1,75 @@ > +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ > +/* > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details: > + * > + * Copyright (C) 2013 Google Inc. > + * > + */ > + > +#include <stdarg.h> > +#include <stdio.h> > +#include <glib.h> > +#include <glib-object.h> > +#include <locale.h> > + > +#include "mm-modem-helpers-altair-lte.h" > + > +/*****************************************************************************/ > +/* Test +CEER responses */ > + > +typedef struct { > + const gchar *str; > + const gchar *result; > +} CeerTest; > + > +static const CeerTest ceer_tests[] = { > + { "", "" }, /* Special case, sometimes the response is empty, treat it as a valid response. */ > + { "+CEER:", "" }, > + { "+CEER: EPS_AND_NON_EPS_SERVICES_NOT_ALLOWED", "EPS_AND_NON_EPS_SERVICES_NOT_ALLOWED" }, > + { "+CEER: NO_SUITABLE_CELLS_IN_TRACKING_AREA", "NO_SUITABLE_CELLS_IN_TRACKING_AREA" }, > + { "WRONG RESPONSE", NULL }, > + { NULL, NULL } > +}; > + > +static void > +test_ceer (void) > +{ > + guint i; > + > + for (i = 0; ceer_tests[i].str; ++i) { > + GError *error = NULL; > + gchar *result; > + > + result = mm_altair_parse_ceer_response (ceer_tests[i].str, &error); > + if (ceer_tests[i].result) { > + g_assert (g_strcmp0 (ceer_tests[i].result, result) == 0); > + g_assert (error == NULL); > + g_free (result); > + } > + else { > + g_assert (result == NULL); > + g_assert (error != NULL); > + g_error_free (error); > + } > + } > +} > + > +int main (int argc, char **argv) > +{ > + setlocale (LC_ALL, ""); > + > + g_type_init (); > + g_test_init (&argc, &argv, NULL); > + > + g_test_add_func ("/MM/altair/ceer", test_ceer); > + > + return g_test_run (); > +} > -- </div></div>Aleksander </blockquote></div> </div>