Hi,

This looks reasonable from my point of view.  Two little comments below...

/Jonas


On 12/10/18 15:16, Giacinto Cifelli wrote:
The ofono_lte_default_attach_info now handles also the protocol and the
authentication method, username and password.

Co-authored-by: Martin Baschin <[email protected]>
---
  drivers/atmodem/lte.c | 94 ++++++++++++++++++++++++++++++++++++++-----
  1 file changed, 84 insertions(+), 10 deletions(-)

diff --git a/drivers/atmodem/lte.c b/drivers/atmodem/lte.c
index efa4e5fe..e5af6cd2 100644
--- a/drivers/atmodem/lte.c
+++ b/drivers/atmodem/lte.c
@@ -3,6 +3,7 @@
   *  oFono - Open Source Telephony
   *
   *  Copyright (C) 2017  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2018 Gemalto M2M
   *
   *  This program is free software; you can redistribute it and/or modify
   *  it under the terms of the GNU General Public License version 2 as
@@ -45,10 +46,17 @@ struct lte_driver_data {
        GAtChat *chat;
  };
-static void at_lte_set_default_attach_info_cb(gboolean ok, GAtResult *result,
+struct lte_callbackdata {
+       ofono_lte_cb_t cb;
+       void *data;
+       struct lte_driver_data *ldd;
+       const struct ofono_lte_default_attach_info *info;
+};
+
+static void at_lte_set_auth_cb(gboolean ok, GAtResult *result,
                                                        gpointer user_data)
  {
-       struct cb_data *cbd = user_data;
+       struct lte_callbackdata *cbd = user_data;
        ofono_lte_cb_t cb = cbd->cb;
        struct ofono_error error;
@@ -58,27 +66,93 @@ static void at_lte_set_default_attach_info_cb(gboolean ok, GAtResult *result,
        cb(&error, cbd->data);
  }
+static void at_lte_set_default_attach_info_cb(gboolean ok, GAtResult *result,
+                                                       gpointer user_data)
+{
+       struct lte_callbackdata *cbd = user_data;
+       ofono_lte_cb_t cb = cbd->cb;
+       void *data = cbd->data;
+       struct ofono_error error;
+       char buf[32 + OFONO_GPRS_MAX_USERNAME_LENGTH +
+                                       OFONO_GPRS_MAX_PASSWORD_LENGTH  + 1];
+       size_t buflen = sizeof(buf);
+       size_t len;
+       enum ofono_gprs_auth_method auth_method;
+
+       if (!ok) {
+               g_free(cbd);
+               decode_at_error(&error, g_at_result_final_response(result));
+               cb(&error, data);

You'll want a return here, too.

+       }
+
+       len = snprintf(buf, buflen, "AT+CGAUTH=0,");
+       buflen -= len;
+       auth_method = cbd->info->auth_method;
+
+       /* change the authentication method if the  parameters are invalid */
+       if (!*cbd->info->username || !*cbd->info->password)
+               auth_method = OFONO_GPRS_AUTH_METHOD_NONE;
+
+       switch (auth_method) {
+       case OFONO_GPRS_AUTH_METHOD_PAP:
+               snprintf(buf + len, buflen, "1,\"%s\",\"%s\"",
+                               cbd->info->username, cbd->info->password);
+               break;
+       case OFONO_GPRS_AUTH_METHOD_CHAP:
+               snprintf(buf + len, buflen, "2,\"%s\",\"%s\"",
+                               cbd->info->username, cbd->info->password);
+               break;
+       case OFONO_GPRS_AUTH_METHOD_NONE:
+               snprintf(buf + len, buflen, "0");
+               break;
+       }
+
+       if (g_at_chat_send(cbd->ldd->chat, buf, NULL,
+                       at_lte_set_auth_cb, cbd, g_free) > 0)
+               return;
+
Here you'll leak cbd again.

+       CALLBACK_WITH_FAILURE(cb, data);
+}
+
  static void at_lte_set_default_attach_info(const struct ofono_lte *lte,
                        const struct ofono_lte_default_attach_info *info,
                        ofono_lte_cb_t cb, void *data)
  {
        struct lte_driver_data *ldd = ofono_lte_get_data(lte);
        char buf[32 + OFONO_GPRS_MAX_APN_LENGTH + 1];
-       struct cb_data *cbd = cb_data_new(cb, data);
+       struct lte_callbackdata *cbd = g_new0(struct lte_callbackdata ,1);
+       const char *proto;
+       size_t len;
DBG("LTE config with APN: %s", info->apn); - if (strlen(info->apn) > 0)
-               snprintf(buf, sizeof(buf), "AT+CGDCONT=0,\"IP\",\"%s\"",
-                               info->apn);
-       else
-               snprintf(buf, sizeof(buf), "AT+CGDCONT=0,\"IP\"");
+       cbd->cb = cb;
+       cbd->data = data;
+       cbd->ldd = ldd;
+       cbd->info = info;
+
+       switch (info->proto) {
+       case OFONO_GPRS_PROTO_IPV6:
+               proto = "IPV6";
+               break;
+       case OFONO_GPRS_PROTO_IPV4V6:
+               proto = "IPV4V6";
+               break;
+       case OFONO_GPRS_PROTO_IP:
+               proto = "IP";
+               break;
+       }
+
+       len = snprintf(buf, sizeof(buf), "AT+CGDCONT=0,\"%s\"", proto);
+
+       if (*info->apn)
+               snprintf(buf+len, sizeof(buf)-len, ",\"%s\"", info->apn);
- /* We can't do much in case of failure so don't check response. */
        if (g_at_chat_send(ldd->chat, buf, NULL,
-                       at_lte_set_default_attach_info_cb, cbd, g_free) > 0)
+                       at_lte_set_default_attach_info_cb, cbd, NULL) > 0)
                return;
+ g_free(cbd);
        CALLBACK_WITH_FAILURE(cb, data);
  }

_______________________________________________
ofono mailing list
[email protected]
https://lists.ofono.org/mailman/listinfo/ofono

Reply via email to