Hi Denis,

On 02/01/2019 19.36, Denis Kenzior wrote:
On 01/02/2019 05:50 AM, Martin Hundebøll wrote:
Let the user/connection-manager decide what to do with duplicate apn
entries instead of bailing out with an error.
---
  plugins/provision.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

While seemingly trivial, this actually has a huge impact on user experience.  The problem is that there's no good meta-data for the user to make the decision as to which service is the one they want.

Yes. The lack of meta-data makes it difficult to make a good decision. In the case of the mobile-broadband-provider-info the <name> element from the parent provider element would be helpful:

diff --git a/plugins/mbpi.c b/plugins/mbpi.c
index 433f1b55..69ffea8a 100644
--- a/plugins/mbpi.c
+++ b/plugins/mbpi.c
@@ -56,6 +56,7 @@ struct gsm_data {
        GSList *apns;
        gboolean match_found;
        gboolean allow_duplicates;
+       char *current_name;
 };

 struct cdma_data {
@@ -401,6 +402,9 @@ static void gsm_end(GMarkupParseContext *context, const gchar *element_name,
        if (!ap->username || !ap->password)
                ap->auth_method = OFONO_GPRS_AUTH_METHOD_NONE;

+       if (!ap->name && gsm->current_name)
+               ap->name = g_strdup(gsm->current_name);
+
        if (gsm->allow_duplicates == FALSE) {
                GSList *l;

@@ -503,7 +507,12 @@ static void toplevel_gsm_start(GMarkupParseContext *context,
 {
        struct gsm_data *gsm = userdata;

-       if (g_str_equal(element_name, "gsm")) {
+       if (g_str_equal(element_name, "name")) {
+               g_free(gsm->current_name);
+               gsm->current_name = NULL;
+               g_markup_parse_context_push(context, &text_parser,
+                                               &gsm->current_name);
+       } else if (g_str_equal(element_name, "gsm")) {
                gsm->match_found = FALSE;
                g_markup_parse_context_push(context, &gsm_parser, gsm);
        } else if (g_str_equal(element_name, "cdma"))
@@ -514,9 +523,16 @@ static void toplevel_gsm_end(GMarkupParseContext *context,
                                        const gchar *element_name,
                                        gpointer userdata, GError **error)
 {
+       struct gsm_data *gsm = userdata;
+
        if (g_str_equal(element_name, "gsm") ||
-                       g_str_equal(element_name, "cdma"))
+                       g_str_equal(element_name, "cdma") ||
+                       g_str_equal(element_name, "name"))
                g_markup_parse_context_pop(context);
+       else if (g_str_equal(element_name, "provider")) {
+               g_free(gsm->current_name);
+               gsm->current_name = NULL;
+       }
 }

 static const GMarkupParser toplevel_gsm_parser = {


Also, I'm not sure how this impacts connman?  In theory it is supposed to auto-activate the internet context, and if there are multiple one of these, I'm not sure if it gets confused?

I haven't looked at connman (yet). Couldn't connman create an empty internet context if it gets confused? (just like the one ofono creates when provision fails.)

The typical guidance is that if a database contains potential duplicates, then a UI Wizard is needed to guide the user to choose the appropriate one and provision the context that way.

Yes, but wouldn't that require access to the duplicate entries to be able to create the wizard?

Alternatively, provide a provisioning database without conflicts, perhaps specific to the users / operators you're targeting.

That removes the convenience from relying on the mobile-broadband-provider-info maintained by upstream.

--
Kind regards,
Martin Hundebøll
Embedded Linux Consultant

+45 61 65 54 61
[email protected]

Geanix IVS
https://geanix.com
DK39600706
_______________________________________________
ofono mailing list
[email protected]
https://lists.ofono.org/mailman/listinfo/ofono

Reply via email to