* [PATCH] plugins, mbpi: make mbpi return only APNs of a given type. When provisioning for internect connection, only OFONO_GPRS_CONTEXT_TYPE_INTERNET APNs will be returned. @ 2013-10-11 9:24 Alfonso Sanchez-Beato 2013-10-11 15:50 ` Denis Kenzior 0 siblings, 1 reply; 5+ messages in thread From: Alfonso Sanchez-Beato @ 2013-10-11 9:24 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 3096 bytes --] From: Alfonso Sanchez-Beato <alfonsosanchezbeato@yahoo.es> --- plugins/mbpi.c | 7 ++++++- plugins/mbpi.h | 1 + plugins/provision.c | 3 ++- tools/lookup-apn.c | 3 ++- 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/plugins/mbpi.c b/plugins/mbpi.c index 309e1ed..b3e26fd 100644 --- a/plugins/mbpi.c +++ b/plugins/mbpi.c @@ -53,6 +53,7 @@ enum MBPI_ERROR { struct gsm_data { const char *match_mcc; const char *match_mnc; + enum ofono_gprs_context_type match_type; GSList *apns; gboolean match_found; gboolean allow_duplicates; @@ -370,7 +371,9 @@ static void gsm_end(GMarkupParseContext *context, const gchar *element_name, } } - gsm->apns = g_slist_append(gsm->apns, ap); + if (gsm->match_type == OFONO_GPRS_CONTEXT_TYPE_ANY || + gsm->match_type == ap->type) + gsm->apns = g_slist_append(gsm->apns, ap); } static const GMarkupParser gsm_parser = { @@ -565,6 +568,7 @@ static gboolean mbpi_parse(const GMarkupParser *parser, gpointer userdata, } GSList *mbpi_lookup_apn(const char *mcc, const char *mnc, + enum ofono_gprs_context_type type, gboolean allow_duplicates, GError **error) { struct gsm_data gsm; @@ -573,6 +577,7 @@ GSList *mbpi_lookup_apn(const char *mcc, const char *mnc, memset(&gsm, 0, sizeof(gsm)); gsm.match_mcc = mcc; gsm.match_mnc = mnc; + gsm.match_type = type; gsm.allow_duplicates = allow_duplicates; if (mbpi_parse(&toplevel_gsm_parser, &gsm, error) == FALSE) { diff --git a/plugins/mbpi.h b/plugins/mbpi.h index 64b7ea5..36bbed6 100644 --- a/plugins/mbpi.h +++ b/plugins/mbpi.h @@ -24,6 +24,7 @@ const char *mbpi_ap_type(enum ofono_gprs_context_type type); void mbpi_ap_free(struct ofono_gprs_provision_data *data); GSList *mbpi_lookup_apn(const char *mcc, const char *mnc, + enum ofono_gprs_context_type type, gboolean allow_duplicates, GError **error); char *mbpi_lookup_cdma_provider_name(const char *sid, GError **error); diff --git a/plugins/provision.c b/plugins/provision.c index 99c299e..39e037b 100644 --- a/plugins/provision.c +++ b/plugins/provision.c @@ -50,7 +50,8 @@ static int provision_get_settings(const char *mcc, const char *mnc, DBG("Provisioning for MCC %s, MNC %s, SPN '%s'", mcc, mnc, spn); - apns = mbpi_lookup_apn(mcc, mnc, FALSE, &error); + apns = mbpi_lookup_apn(mcc, mnc, OFONO_GPRS_CONTEXT_TYPE_INTERNET, + FALSE, &error); if (apns == NULL) { if (error != NULL) { ofono_error("%s", error->message); diff --git a/tools/lookup-apn.c b/tools/lookup-apn.c index 884b32a..4654b03 100644 --- a/tools/lookup-apn.c +++ b/tools/lookup-apn.c @@ -42,7 +42,8 @@ static void lookup_apn(const char *match_mcc, const char *match_mnc, g_print("Searching for info for network: %s%s\n", match_mcc, match_mnc); - apns = mbpi_lookup_apn(match_mcc, match_mnc, allow_duplicates, &error); + apns = mbpi_lookup_apn(match_mcc, match_mnc, OFONO_GPRS_CONTEXT_TYPE_ANY, + allow_duplicates, &error); if (apns == NULL) { if (error != NULL) { -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] plugins, mbpi: make mbpi return only APNs of a given type. When provisioning for internect connection, only OFONO_GPRS_CONTEXT_TYPE_INTERNET APNs will be returned. 2013-10-11 9:24 [PATCH] plugins, mbpi: make mbpi return only APNs of a given type. When provisioning for internect connection, only OFONO_GPRS_CONTEXT_TYPE_INTERNET APNs will be returned Alfonso Sanchez-Beato @ 2013-10-11 15:50 ` Denis Kenzior 2013-10-11 16:17 ` Alfonso Sanchez-Beato 0 siblings, 1 reply; 5+ messages in thread From: Denis Kenzior @ 2013-10-11 15:50 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 3875 bytes --] Hi Alfonso, On 10/11/2013 04:24 AM, Alfonso Sanchez-Beato wrote: > From: Alfonso Sanchez-Beato <alfonsosanchezbeato@yahoo.es> > > --- > plugins/mbpi.c | 7 ++++++- > plugins/mbpi.h | 1 + > plugins/provision.c | 3 ++- > tools/lookup-apn.c | 3 ++- Please break this up into three patches, one for mbpi.[ch] changes, one for lookup-apn and one for provision.c. We're not so much concerned with breaking git bisect. Also, please keep the patch title under 72 characters if possible. Sentences should be put into the patch description, not the title. > 4 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/plugins/mbpi.c b/plugins/mbpi.c > index 309e1ed..b3e26fd 100644 > --- a/plugins/mbpi.c > +++ b/plugins/mbpi.c > @@ -53,6 +53,7 @@ enum MBPI_ERROR { > struct gsm_data { > const char *match_mcc; > const char *match_mnc; > + enum ofono_gprs_context_type match_type; > GSList *apns; > gboolean match_found; > gboolean allow_duplicates; > @@ -370,7 +371,9 @@ static void gsm_end(GMarkupParseContext *context, const gchar *element_name, > } > } > > - gsm->apns = g_slist_append(gsm->apns, ap); > + if (gsm->match_type == OFONO_GPRS_CONTEXT_TYPE_ANY || > + gsm->match_type == ap->type) > + gsm->apns = g_slist_append(gsm->apns, ap); > } > > static const GMarkupParser gsm_parser = { > @@ -565,6 +568,7 @@ static gboolean mbpi_parse(const GMarkupParser *parser, gpointer userdata, > } > > GSList *mbpi_lookup_apn(const char *mcc, const char *mnc, > + enum ofono_gprs_context_type type, > gboolean allow_duplicates, GError **error) > { > struct gsm_data gsm; > @@ -573,6 +577,7 @@ GSList *mbpi_lookup_apn(const char *mcc, const char *mnc, > memset(&gsm, 0, sizeof(gsm)); > gsm.match_mcc = mcc; > gsm.match_mnc = mnc; > + gsm.match_type = type; > gsm.allow_duplicates = allow_duplicates; > > if (mbpi_parse(&toplevel_gsm_parser, &gsm, error) == FALSE) { > diff --git a/plugins/mbpi.h b/plugins/mbpi.h > index 64b7ea5..36bbed6 100644 > --- a/plugins/mbpi.h > +++ b/plugins/mbpi.h > @@ -24,6 +24,7 @@ const char *mbpi_ap_type(enum ofono_gprs_context_type type); > void mbpi_ap_free(struct ofono_gprs_provision_data *data); > > GSList *mbpi_lookup_apn(const char *mcc, const char *mnc, > + enum ofono_gprs_context_type type, > gboolean allow_duplicates, GError **error); > > char *mbpi_lookup_cdma_provider_name(const char *sid, GError **error); I'm fine with the above changes, they could be useful I suppose... > diff --git a/plugins/provision.c b/plugins/provision.c > index 99c299e..39e037b 100644 > --- a/plugins/provision.c > +++ b/plugins/provision.c > @@ -50,7 +50,8 @@ static int provision_get_settings(const char *mcc, const char *mnc, > > DBG("Provisioning for MCC %s, MNC %s, SPN '%s'", mcc, mnc, spn); > > - apns = mbpi_lookup_apn(mcc, mnc, FALSE, &error); > + apns = mbpi_lookup_apn(mcc, mnc, OFONO_GPRS_CONTEXT_TYPE_INTERNET, > + FALSE, &error); However, why do you think this is a good idea? Provisioning should provision all context types, not just internet ones. > if (apns == NULL) { > if (error != NULL) { > ofono_error("%s", error->message); > diff --git a/tools/lookup-apn.c b/tools/lookup-apn.c > index 884b32a..4654b03 100644 > --- a/tools/lookup-apn.c > +++ b/tools/lookup-apn.c > @@ -42,7 +42,8 @@ static void lookup_apn(const char *match_mcc, const char *match_mnc, > > g_print("Searching for info for network: %s%s\n", match_mcc, match_mnc); > > - apns = mbpi_lookup_apn(match_mcc, match_mnc, allow_duplicates, &error); > + apns = mbpi_lookup_apn(match_mcc, match_mnc, OFONO_GPRS_CONTEXT_TYPE_ANY, > + allow_duplicates, &error); > > if (apns == NULL) { > if (error != NULL) { > Regards, -Denis ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] plugins, mbpi: make mbpi return only APNs of a given type. When provisioning for internect connection, only OFONO_GPRS_CONTEXT_TYPE_INTERNET APNs will be returned. 2013-10-11 15:50 ` Denis Kenzior @ 2013-10-11 16:17 ` Alfonso Sanchez-Beato 2013-10-11 17:00 ` Denis Kenzior 0 siblings, 1 reply; 5+ messages in thread From: Alfonso Sanchez-Beato @ 2013-10-11 16:17 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 4966 bytes --] Hi Denis, No problem with splitting the patch. The patch is related to the following bug: https://bugs.launchpad.net/ubuntu/+source/ofono/+bug/1222106 So we were obtaining some times settings for the MMS APN instead of the internet one. Maybe the change makes it work only for internet connection, but not for MMS. Would that mean that get_settings() should have a parameter specifying the context type so the user decides? Thanks, Alfonso On Fri, Oct 11, 2013 at 5:50 PM, Denis Kenzior <denkenz@gmail.com> wrote: > Hi Alfonso, > > > On 10/11/2013 04:24 AM, Alfonso Sanchez-Beato wrote: > >> From: Alfonso Sanchez-Beato <alfonsosanchezbeato@yahoo.es> >> >> --- >> plugins/mbpi.c | 7 ++++++- >> plugins/mbpi.h | 1 + >> plugins/provision.c | 3 ++- >> tools/lookup-apn.c | 3 ++- >> > > Please break this up into three patches, one for mbpi.[ch] changes, one > for lookup-apn and one for provision.c. We're not so much concerned with > breaking git bisect. > > Also, please keep the patch title under 72 characters if possible. > Sentences should be put into the patch description, not the title. > > > 4 files changed, 11 insertions(+), 3 deletions(-) >> >> diff --git a/plugins/mbpi.c b/plugins/mbpi.c >> index 309e1ed..b3e26fd 100644 >> --- a/plugins/mbpi.c >> +++ b/plugins/mbpi.c >> @@ -53,6 +53,7 @@ enum MBPI_ERROR { >> struct gsm_data { >> const char *match_mcc; >> const char *match_mnc; >> + enum ofono_gprs_context_type match_type; >> GSList *apns; >> gboolean match_found; >> gboolean allow_duplicates; >> @@ -370,7 +371,9 @@ static void gsm_end(GMarkupParseContext *context, >> const gchar *element_name, >> } >> } >> >> - gsm->apns = g_slist_append(gsm->apns, ap); >> + if (gsm->match_type == OFONO_GPRS_CONTEXT_TYPE_ANY || >> + gsm->match_type == ap->type) >> + gsm->apns = g_slist_append(gsm->apns, ap); >> } >> >> static const GMarkupParser gsm_parser = { >> @@ -565,6 +568,7 @@ static gboolean mbpi_parse(const GMarkupParser >> *parser, gpointer userdata, >> } >> >> GSList *mbpi_lookup_apn(const char *mcc, const char *mnc, >> + enum ofono_gprs_context_type type, >> gboolean allow_duplicates, GError **error) >> { >> struct gsm_data gsm; >> @@ -573,6 +577,7 @@ GSList *mbpi_lookup_apn(const char *mcc, const char >> *mnc, >> memset(&gsm, 0, sizeof(gsm)); >> gsm.match_mcc = mcc; >> gsm.match_mnc = mnc; >> + gsm.match_type = type; >> gsm.allow_duplicates = allow_duplicates; >> >> if (mbpi_parse(&toplevel_gsm_**parser, &gsm, error) == FALSE) { >> diff --git a/plugins/mbpi.h b/plugins/mbpi.h >> index 64b7ea5..36bbed6 100644 >> --- a/plugins/mbpi.h >> +++ b/plugins/mbpi.h >> @@ -24,6 +24,7 @@ const char *mbpi_ap_type(enum ofono_gprs_context_type >> type); >> void mbpi_ap_free(struct ofono_gprs_provision_data *data); >> >> GSList *mbpi_lookup_apn(const char *mcc, const char *mnc, >> + enum ofono_gprs_context_type type, >> gboolean allow_duplicates, GError **error); >> >> char *mbpi_lookup_cdma_provider_**name(const char *sid, GError >> **error); >> > > I'm fine with the above changes, they could be useful I suppose... > > > diff --git a/plugins/provision.c b/plugins/provision.c >> index 99c299e..39e037b 100644 >> --- a/plugins/provision.c >> +++ b/plugins/provision.c >> @@ -50,7 +50,8 @@ static int provision_get_settings(const char *mcc, >> const char *mnc, >> >> DBG("Provisioning for MCC %s, MNC %s, SPN '%s'", mcc, mnc, spn); >> >> - apns = mbpi_lookup_apn(mcc, mnc, FALSE, &error); >> + apns = mbpi_lookup_apn(mcc, mnc, OFONO_GPRS_CONTEXT_TYPE_** >> INTERNET, >> + FALSE, &error); >> > > However, why do you think this is a good idea? Provisioning should > provision all context types, not just internet ones. > > > if (apns == NULL) { >> if (error != NULL) { >> ofono_error("%s", error->message); >> diff --git a/tools/lookup-apn.c b/tools/lookup-apn.c >> index 884b32a..4654b03 100644 >> --- a/tools/lookup-apn.c >> +++ b/tools/lookup-apn.c >> @@ -42,7 +42,8 @@ static void lookup_apn(const char *match_mcc, const >> char *match_mnc, >> >> g_print("Searching for info for network: %s%s\n", match_mcc, >> match_mnc); >> >> - apns = mbpi_lookup_apn(match_mcc, match_mnc, allow_duplicates, >> &error); >> + apns = mbpi_lookup_apn(match_mcc, match_mnc, >> OFONO_GPRS_CONTEXT_TYPE_ANY, >> + allow_duplicates, &error); >> >> if (apns == NULL) { >> if (error != NULL) { >> >> > Regards, > -Denis > [-- Attachment #2: attachment.html --] [-- Type: text/html, Size: 6170 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] plugins, mbpi: make mbpi return only APNs of a given type. When provisioning for internect connection, only OFONO_GPRS_CONTEXT_TYPE_INTERNET APNs will be returned. 2013-10-11 16:17 ` Alfonso Sanchez-Beato @ 2013-10-11 17:00 ` Denis Kenzior 2013-10-14 7:33 ` Alfonso Sanchez-Beato 0 siblings, 1 reply; 5+ messages in thread From: Denis Kenzior @ 2013-10-11 17:00 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 1141 bytes --] Hi Alfonso, No top posting on this mailing list please. > No problem with splitting the patch. > > The patch is related to the following bug: > > https://bugs.launchpad.net/ubuntu/+source/ofono/+bug/1222106 > > So we were obtaining some times settings for the MMS APN instead of the > internet one. Maybe the change makes it work only for internet > connection, but not for MMS. Would that mean that get_settings() should > have a parameter specifying the context type so the user decides? > The mbpi parser will assign a context type to the provisioned context based on the "type" attribute inside mobile-broadband-provider-info. So assuming the parser is working correctly, the context will be labeled accordingly in oFono on the D-Bus API level. The parser works by returning all contexts that match a particular mcc/mnc if and only if no conflicts or multiple matches are found. Since you're saying it picks the 'wrong' one, then I assume the issue is inside your application, not oFono. What does test/list-contexts output look like when you run the provisioning procedure? Regards, -Denis ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] plugins, mbpi: make mbpi return only APNs of a given type. When provisioning for internect connection, only OFONO_GPRS_CONTEXT_TYPE_INTERNET APNs will be returned. 2013-10-11 17:00 ` Denis Kenzior @ 2013-10-14 7:33 ` Alfonso Sanchez-Beato 0 siblings, 0 replies; 5+ messages in thread From: Alfonso Sanchez-Beato @ 2013-10-14 7:33 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 1263 bytes --] Hi Denis, The patch is related to the following bug: >> >> https://bugs.launchpad.net/**ubuntu/+source/ofono/+bug/**1222106<https://bugs.launchpad.net/ubuntu/+source/ofono/+bug/1222106> >> >> So we were obtaining some times settings for the MMS APN instead of the >> internet one. Maybe the change makes it work only for internet >> connection, but not for MMS. Would that mean that get_settings() should >> have a parameter specifying the context type so the user decides? >> >> > The mbpi parser will assign a context type to the provisioned context > based on the "type" attribute inside mobile-broadband-provider-**info. > So assuming the parser is working correctly, the context will be labeled > accordingly in oFono on the D-Bus API level. > > The parser works by returning all contexts that match a particular mcc/mnc > if and only if no conflicts or multiple matches are found. Since you're > saying it picks the 'wrong' one, then I assume the issue is inside your > application, not oFono. > After gathering some more information, I think that the problem comes from different behaviors of connman vs NetworkManager when handling the context. So we need to re-think this on our side. Thanks for your comments, Alfonso [-- Attachment #2: attachment.html --] [-- Type: text/html, Size: 1713 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-10-14 7:33 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-10-11 9:24 [PATCH] plugins, mbpi: make mbpi return only APNs of a given type. When provisioning for internect connection, only OFONO_GPRS_CONTEXT_TYPE_INTERNET APNs will be returned Alfonso Sanchez-Beato 2013-10-11 15:50 ` Denis Kenzior 2013-10-11 16:17 ` Alfonso Sanchez-Beato 2013-10-11 17:00 ` Denis Kenzior 2013-10-14 7:33 ` Alfonso Sanchez-Beato
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.