[PATCH] altair-lte: update PCO to handle latest VZW deployment
Dan Williams
dcbw at redhat.com
Fri Jun 6 10:30:14 PDT 2014
On Thu, 2014-06-05 at 17:49 -0700, Thieu Le wrote:
> Update PCO handling code such that it conforms to the latest VZW network
> behavior. This includes updating the way we mark a SIM as provisioned.
> In the old network, only provisioned SIMs can attach to the network.
> Now, unprovisioned SIMs can attach and connect to the network.
I forget where we landed long ago on the hardcoded CIDs, but are we
always guaranteed that on any Altair device, that the IMS and internet
CIDs will always be 1 and 3? Or is this a Verizon-ism? (it does seem
like a Verizon-ism; are we cool with that and we just fix it later
if/when some Altair devices can be used on other networks?)
Comments below...
> ---
> plugins/altair/mm-broadband-modem-altair-lte.c | 51 +++-------------------
> plugins/altair/mm-modem-helpers-altair-lte.c | 30 ++++++++-----
> plugins/altair/mm-modem-helpers-altair-lte.h | 4 +-
> .../altair/tests/test-modem-helpers-altair-lte.c | 30 +++++++++----
> src/mm-broadband-modem.c | 9 +---
> 5 files changed, 49 insertions(+), 75 deletions(-)
>
> diff --git a/plugins/altair/mm-broadband-modem-altair-lte.c b/plugins/altair/mm-broadband-modem-altair-lte.c
> index b71884b..c732c52 100644
> --- a/plugins/altair/mm-broadband-modem-altair-lte.c
> +++ b/plugins/altair/mm-broadband-modem-altair-lte.c
> @@ -1190,33 +1190,15 @@ modem_3gpp_load_subscription_state_finish (MMIfaceModem3gpp *self,
> }
>
> static void
> -altair_load_internet_cid_ready (MMIfaceModem3gpp *self,
> - GAsyncResult *res,
> - LoadSubscriptionStateContext *ctx)
> +altair_get_subscription_state (MMIfaceModem3gpp *self,
> + LoadSubscriptionStateContext *ctx)
> {
> - const gchar *response;
> - GError *error = NULL;
> - guint cid;
> guint pco_value = -1;
> + GError *error = NULL;
> MMModem3gppSubscriptionState subscription_state;
>
> - response = mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res, &error);
> - if (error) {
> - mm_dbg ("Failed to load internet CID.");
> - g_simple_async_result_take_error (ctx->result, error);
> - load_subscription_state_context_complete_and_free (ctx);
> - return;
> - }
> -
> - cid = mm_altair_parse_cid (response, &error);
> - if (error) {
> - g_simple_async_result_take_error (ctx->result, error);
> - load_subscription_state_context_complete_and_free (ctx);
> - return;
> - }
> -
> mm_dbg ("Parsing vendor PCO info: %s", ctx->pco_info);
> - pco_value = mm_altair_parse_vendor_pco_info (ctx->pco_info, cid, &error);
> + pco_value = mm_altair_parse_vendor_pco_info (ctx->pco_info, &error);
> if (error) {
> g_simple_async_result_take_error (ctx->result, error);
> load_subscription_state_context_complete_and_free (ctx);
> @@ -1225,33 +1207,11 @@ altair_load_internet_cid_ready (MMIfaceModem3gpp *self,
> mm_dbg ("PCO value = %d", pco_value);
>
> subscription_state = altair_vzw_pco_value_to_mm_modem_3gpp_subscription_state (pco_value);
> - if (subscription_state == MM_MODEM_3GPP_SUBSCRIPTION_STATE_UNKNOWN) {
> - /* The PCO value is loaded after the modem has successfully registered
> - * with the network. So even if the PCO value is unknown here,
> - * the successful registration indicates a provisioned SIM.
> - */
> - subscription_state = MM_MODEM_3GPP_SUBSCRIPTION_STATE_PROVISIONED;
> - }
> -
> g_simple_async_result_set_op_res_gpointer (ctx->result, GUINT_TO_POINTER (subscription_state), NULL);
> load_subscription_state_context_complete_and_free (ctx);
> }
>
> static void
> -altair_get_subscription_state (MMIfaceModem3gpp *self,
> - LoadSubscriptionStateContext *ctx)
> -{
> - /* Get the latest internet CID first */
> - mm_dbg ("Loading internet CID...");
> - mm_base_modem_at_command (MM_BASE_MODEM (self),
> - "%CGINFO=\"cid\",1",
> - 6,
> - FALSE,
> - (GAsyncReadyCallback)altair_load_internet_cid_ready,
> - ctx);
> -}
> -
> -static void
> altair_load_vendor_pco_info_ready (MMIfaceModem3gpp *self,
> GAsyncResult *res,
> LoadSubscriptionStateContext *ctx)
> @@ -1312,7 +1272,8 @@ altair_get_subscription_state_ready (MMBroadbandModemAltairLte *self,
> }
>
> subscription_state = GPOINTER_TO_UINT (g_simple_async_result_get_op_res_gpointer (G_SIMPLE_ASYNC_RESULT (res)));
> - mm_iface_modem_3gpp_update_subscription_state (MM_IFACE_MODEM_3GPP (self), subscription_state);
> + if (subscription_state != MM_MODEM_3GPP_SUBSCRIPTION_STATE_UNKNOWN)
> + mm_iface_modem_3gpp_update_subscription_state (MM_IFACE_MODEM_3GPP (self), subscription_state);
> }
>
> static void
> diff --git a/plugins/altair/mm-modem-helpers-altair-lte.c b/plugins/altair/mm-modem-helpers-altair-lte.c
> index 90b29e0..2048fc4 100644
> --- a/plugins/altair/mm-modem-helpers-altair-lte.c
> +++ b/plugins/altair/mm-modem-helpers-altair-lte.c
> @@ -23,6 +23,9 @@
>
> #include "mm-modem-helpers-altair-lte.h"
>
> +#define MM_ALTAIR_IMS_PDN_CID 1
> +#define MM_ALTAIR_INTERNET_PDN_CID 3
> +
> /*****************************************************************************/
> /* Bands response parser */
>
> @@ -138,6 +141,13 @@ mm_altair_parse_cid (const gchar *response, GError **error)
> /*****************************************************************************/
> /* %PCOINFO response parser */
>
> +typedef enum {
> + MM_VZW_PCO_PROVISIONED = 0,
> + MM_VZW_PCO_LIMIT_REACHED = 1,
> + MM_VZW_PCO_OUT_OF_DATA = 3,
> + MM_VZW_PCO_UNPROVISIONED = 5
> +} MMVzwPco;
> +
> static guint
> altair_extract_vzw_pco_value (const gchar *pco_payload, GError **error)
> {
> @@ -177,9 +187,7 @@ altair_extract_vzw_pco_value (const gchar *pco_payload, GError **error)
> }
>
> guint
> -mm_altair_parse_vendor_pco_info (const gchar *pco_info,
> - guint cid,
> - GError **error)
> +mm_altair_parse_vendor_pco_info (const gchar *pco_info, GError **error)
> {
> GRegex *regex;
> GMatchInfo *match_info;
> @@ -231,11 +239,6 @@ mm_altair_parse_vendor_pco_info (const gchar *pco_info,
> break;
> }
>
> - if (pco_cid != cid) {
> - g_match_info_next (match_info, error);
> - continue;
> - }
> -
> pco_id = mm_get_string_unquoted_from_match_info (match_info, 3);
> if (!pco_id) {
> g_set_error (error,
> @@ -263,8 +266,15 @@ mm_altair_parse_vendor_pco_info (const gchar *pco_info,
> }
>
> pco_value = altair_extract_vzw_pco_value (pco_payload, error);
> - g_free (pco_payload);
> - break;
> +
> + /* We are only interested in IMS and Internet PDN PCO. */
> + if (pco_cid == MM_ALTAIR_IMS_PDN_CID || pco_cid == MM_ALTAIR_INTERNET_PDN_CID) {
> + g_free (pco_payload);
> + break;
> + }
Won't pco_payload get leaked here? Should probably just leave the
g_free() alone, and just have the 'break' inside the CID check.
> +
> + pco_value = -1;
> + g_match_info_next (match_info, error);
> }
>
> g_match_info_free (match_info);
> diff --git a/plugins/altair/mm-modem-helpers-altair-lte.h b/plugins/altair/mm-modem-helpers-altair-lte.h
> index 56a6e15..b319997 100644
> --- a/plugins/altair/mm-modem-helpers-altair-lte.h
> +++ b/plugins/altair/mm-modem-helpers-altair-lte.h
> @@ -30,8 +30,6 @@ gchar *mm_altair_parse_ceer_response (const gchar *response,
> guint mm_altair_parse_cid (const gchar *response, GError **error);
>
> /* %PCOINFO response parser */
> -guint mm_altair_parse_vendor_pco_info (const gchar *pco_info,
> - guint cid,
> - GError **error);
> +guint mm_altair_parse_vendor_pco_info (const gchar *pco_info, 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
> index 46f70b6..d83d3e5 100644
> --- a/plugins/altair/tests/test-modem-helpers-altair-lte.c
> +++ b/plugins/altair/tests/test-modem-helpers-altair-lte.c
> @@ -103,26 +103,38 @@ test_parse_vendor_pco_info (void)
> guint pco_value;
>
> /* Valid PCO values */
> - pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO: 1,3,FF00,13018400", 3, NULL);
> + pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO: 1,1,FF00,13018400", NULL);
> g_assert (pco_value == 0);
> - pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO: 1,3,FF00,13018403", 3, NULL);
> + pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO: 1,1,FF00,13018403", NULL);
> g_assert (pco_value == 3);
> - pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO: 1,3,FF00,13018405", 3, NULL);
> + pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO: 1,1,FF00,13018405", NULL);
> + g_assert (pco_value == 5);
> + pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO: 1,3,FF00,13018400", NULL);
> + g_assert (pco_value == 0);
> + pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO: 1,3,FF00,13018403", NULL);
> + g_assert (pco_value == 3);
> + pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO: 1,3,FF00,13018405", NULL);
> + g_assert (pco_value == 5);
> + pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO:1,FF00,13018400", NULL);
> + g_assert (pco_value == 0);
> + pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO:1,FF00,13018403", NULL);
> + g_assert (pco_value == 3);
> + pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO:1,FF00,13018405", NULL);
> g_assert (pco_value == 5);
> /* Different container */
> - pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO: 1,3,F000,13018401", 3, NULL);
> + pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO: 1,3,F000,13018401", NULL);
> g_assert (pco_value == -1);
> - /* Different CID */
> - pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO: 1,3,FF00,13018401", 1, NULL);
> + /* Invalid CID */
> + pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO: 1,2,FF00,13018401", NULL);
> g_assert (pco_value == -1);
> /* Different payload */
> - pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO: 1,3,FF00,13018501", 1, NULL);
> + pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO: 1,3,FF00,13018501", NULL);
> g_assert (pco_value == -1);
> /* Bad PCO info */
> - pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO: blah,blah,FF00,13018401", 1, NULL);
> + pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO: blah,blah,FF00,13018401", NULL);
> g_assert (pco_value == -1);
> /* Multiline PCO info */
> - pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO: 1,1,FF00,13018400\r\n%PCOINFO: 1,3,FF00,13018403", 3, NULL);
> + pco_value = mm_altair_parse_vendor_pco_info ("%PCOINFO: 1,2,FF00,13018400\r\n%PCOINFO: 1,3,FF00,13018403", NULL);
> g_assert (pco_value == 3);
> }
>
> diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
> index 9386f0b..4a36f7b 100644
> --- a/src/mm-broadband-modem.c
> +++ b/src/mm-broadband-modem.c
> @@ -3499,16 +3499,9 @@ modem_3gpp_load_subscription_state (MMIfaceModem3gpp *self,
> user_data,
> modem_3gpp_load_subscription_state);
>
> - /* Reloading subscription state only occurs on a successfully registered
> - * modem. (Although the 3GPP interface does not reflect this until after
> - * loading operator information completes.)
> - * By default, we can assume that successful registration implies a
> - * provisioned SIM.
> - */
> - mm_dbg ("Load subscription state: Marking the SIM as provisioned.");
> g_simple_async_result_set_op_res_gpointer (
> result,
> - GUINT_TO_POINTER (MM_MODEM_3GPP_SUBSCRIPTION_STATE_PROVISIONED),
> + GUINT_TO_POINTER (MM_MODEM_3GPP_SUBSCRIPTION_STATE_UNKNOWN),
> NULL);
>
> g_simple_async_result_complete_in_idle (result);
Any idea if this will this be the case for other operators too?
Dan
More information about the ModemManager-devel
mailing list