Hi Giacinto,
+static void at_set_reg_info(const struct ofono_lte *lte,
+ const struct ofono_lte_default_attach_info *info)
{
- struct cb_data *cbd = user_data;
- ofono_lte_cb_t cb = cbd->cb;
- struct ofono_error error;
+ struct lte_driver_data *ldd = ofono_lte_get_data(lte);
+ char buf_cgdcont[32 + OFONO_GPRS_MAX_APN_LENGTH +1];
+ char buf_cgauth[32 + OFONO_GPRS_MAX_USERNAME_LENGTH +
+ OFONO_GPRS_MAX_PASSWORD_LENGTH +1];
Please pay attention to doc/coding-style.txt item M3
+ guint buflen = sizeof(buf_cgauth);
+ enum ofono_gprs_auth_method auth_method;
- DBG("ok %d", ok);
+ if (strlen(info->apn) > 0)
+ snprintf(buf_cgdcont, sizeof(buf_cgdcont),
+ "AT+CGDCONT=0,\"IP\",\"%s\"", info->apn);
+ else
+ snprintf(buf_cgdcont, sizeof(buf_cgdcont),
+ "AT+CGDCONT=0,\"IP\"");
You're not taking IPv4/v6/Dual into account? Why bother adding that
property then?
- decode_at_error(&error, g_at_result_final_response(result));
- cb(&error, cbd->data);
+ if (g_at_chat_send(ldd->chat, buf_cgdcont, NULL, NULL, NULL, NULL) == 0)
+ return;
Uhh, you can't just return here
+
+ snprintf(buf_cgauth, buflen, "AT+CGAUTH=0,");
+ buflen -= strlen(buf_cgauth);
You have way too many unnecessary strlen calls. Refer to 'man snprintf'
(particularly the return value) to understand how these can be avoided.
+
+ auth_method = info->auth_method;
+
+ /*
+ * change the authentication method if the parameters are invalid
+ * for behavior compatibility
+ */
+ if(!*info->username || !*info->password)
+ auth_method = OFONO_GPRS_AUTH_METHOD_NONE;
+
+ switch(auth_method) {
+ case OFONO_GPRS_AUTH_METHOD_PAP:
+ snprintf(buf_cgauth+strlen(buf_cgauth),
+ buflen, "1,\"%s\",\"%s\"",
+ info->username, info->password);
+ break;
+ case OFONO_GPRS_AUTH_METHOD_CHAP:
+ snprintf(buf_cgauth+strlen(buf_cgauth),
+ buflen, "2,\"%s\",\"%s\"",
+ info->username, info->password);
+ break;
+ case OFONO_GPRS_AUTH_METHOD_NONE:
+ snprintf(buf_cgauth+strlen(buf_cgauth), buflen, "0");
+ break;
+ }
+
+ g_at_chat_send(ldd->chat, buf_cgauth, NULL, NULL, NULL, NULL);
Anyway. All this boils down to is a +CGDCONT and a +CGAUTH call
whenever a property changes. And you even take into account
username/password not being valid to override the auth method. So I
really see no point for set_reg_info now?
}
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);
-
- 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\"");
-
- /* 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)
- return;
-
- CALLBACK_WITH_FAILURE(cb, data);
+ CALLBACK_WITH_SUCCESS(cb, data);
So why do we even bother having a driver method that does literally nothing?
}
static gboolean lte_delayed_register(gpointer user_data)
{
- struct ofono_lte *lte = user_data;
-
- ofono_lte_register(lte);
+ ofono_lte_register(user_data);
return FALSE;
}
@@ -129,6 +149,7 @@ static struct ofono_lte_driver driver = {
.probe = at_lte_probe,
.remove = at_lte_remove,
.set_default_attach_info = at_lte_set_default_attach_info,
+ .set_reg_info = at_set_reg_info,
};
void at_lte_init(void)
Regards,
-Denis
_______________________________________________
ofono mailing list
[email protected]
https://lists.ofono.org/mailman/listinfo/ofono
