7:26 p.m.
Hi,
These patches will be used to set PPP credentials when CDMA network will need them.
It contains a test script based on create-internet-context script.
Guillaume Zajac (4):
cdma-connman: add prototype to retrieve username and password
cdma-connman: add possibility to set a username and a password
driver cdma-connman: add credential while setting up PPP if required
test: add script to set credentials for cdma connection
drivers/cdmamodem/connman.c | 8 +++++++
include/cdma-connman.h | 4 +++
src/cdma-connman.c | 44 +++++++++++++++++++++++++++++++++++++++++-
test/create-cdma-credentials | 29 +++++++++++++++++++++++++++
4 files changed, 84 insertions(+), 1 deletions(-)
create mode 100755 test/create-cdma-credentials
7:26 p.m.
New subject: [PATCH 1/4] cdma-connman: add prototype to retrieve username and password
---
include/cdma-connman.h | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/include/cdma-connman.h b/include/cdma-connman.h
index 22252e0..1439ea9 100644
--- a/include/cdma-connman.h
+++ b/include/cdma-connman.h
@@ -72,6 +72,10 @@ void ofono_cdma_connman_set_data(struct ofono_cdma_connman *cm,
void *data);
void *ofono_cdma_connman_get_data(struct ofono_cdma_connman *cm);
+const char *ofono_cdma_connman_get_username(struct ofono_cdma_connman *cm);
+
+const char *ofono_cdma_connman_get_password(struct ofono_cdma_connman *cm);
+
#ifdef __cplusplus
}
#endif
--
1.7.1
7:26 p.m.
New subject: [PATCH 2/4] cdma-connman: add possibility to set a username and a password
---
src/cdma-connman.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 43 insertions(+), 1 deletions(-)
diff --git a/src/cdma-connman.c b/src/cdma-connman.c
index 3321b87..7ca5a0a 100644
--- a/src/cdma-connman.c
+++ b/src/cdma-connman.c
@@ -57,6 +57,8 @@ struct ofono_cdma_connman {
const struct ofono_cdma_connman_driver *driver;
void *driver_data;
struct ofono_atom *atom;
+ char *username;
+ char *password;
};
static void cdma_connman_settings_free(struct cdma_connman_settings *settings)
@@ -379,6 +381,8 @@ static DBusMessage *cdma_connman_set_property(DBusConnection *conn,
DBusMessageIter var;
const char *property;
dbus_bool_t value;
+ const char *username;
+ const char *password;
DBG("");
@@ -399,7 +403,7 @@ static DBusMessage *cdma_connman_set_property(DBusConnection *conn,
dbus_message_iter_recurse(&iter, &var);
- if (!strcmp(property, "Powered")) {
+ if (!g_strcmp0(property, "Powered")) {
if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_BOOLEAN)
return __ofono_error_invalid_args(msg);
@@ -421,6 +425,22 @@ static DBusMessage *cdma_connman_set_property(DBusConnection *conn,
cm->driver->deactivate(cm, deactivate_callback, cm);
return dbus_message_new_method_return(msg);
+ } else if (!g_strcmp0(property, "Username")) {
+ if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_STRING)
+ return __ofono_error_invalid_args(msg);
+
+ dbus_message_iter_get_basic(&var, &username);
+ cm->username = g_strdup(username);
+ DBG("username: %s", username);
+ return dbus_message_new_method_return(msg);
+ } else if (!g_strcmp0(property, "Password")) {
+ if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_STRING)
+ return __ofono_error_invalid_args(msg);
+
+ dbus_message_iter_get_basic(&var, &password);
+ cm->password = g_strdup(password);
+ DBG("password: %s", password);
+ return dbus_message_new_method_return(msg);
}
/* TODO: Dormant property. Not yet supported. */
@@ -488,6 +508,12 @@ static void cdma_connman_remove(struct ofono_atom *atom)
if (cm->driver && cm->driver->remove)
cm->driver->remove(cm);
+ if (cm->username)
+ g_free(cm->username);
+
+ if (cm->password)
+ g_free(cm->password);
+
g_free(cm);
}
@@ -568,3 +594,19 @@ void *ofono_cdma_connman_get_data(struct ofono_cdma_connman *cm)
{
return cm->driver_data;
}
+
+const char *ofono_cdma_connman_get_username(struct ofono_cdma_connman *cm)
+{
+ if (cm->username)
+ return cm->username;
+
+ return NULL;
+}
+
+const char *ofono_cdma_connman_get_password(struct ofono_cdma_connman *cm)
+{
+ if (cm->password)
+ return cm->password;
+
+ return NULL;
+}
--
1.7.1
7:26 p.m.
New subject: [PATCH 3/4] driver cdma-connman: add credential while setting up PPP if required
---
drivers/cdmamodem/connman.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/cdmamodem/connman.c b/drivers/cdmamodem/connman.c
index 14c78b1..7f6bf5a 100644
--- a/drivers/cdmamodem/connman.c
+++ b/drivers/cdmamodem/connman.c
@@ -123,9 +123,14 @@ static gboolean setup_ppp(struct ofono_cdma_connman *cm)
{
struct connman_data *cd = ofono_cdma_connman_get_data(cm);
GAtIO *io;
+ const char *username;
+ const char *password;
DBG("");
+ username = ofono_cdma_connman_get_username(cm);
+ password = ofono_cdma_connman_get_password(cm);
+
io = g_at_chat_get_io(cd->chat);
g_at_chat_suspend(cd->chat);
@@ -145,6 +150,9 @@ static gboolean setup_ppp(struct ofono_cdma_connman *cm)
g_at_ppp_set_connect_function(cd->ppp, ppp_connect, cm);
g_at_ppp_set_disconnect_function(cd->ppp, ppp_disconnect, cm);
+ if (username && password)
+ g_at_ppp_set_credentials(cd->ppp, username, password);
+
/* open the ppp connection */
g_at_ppp_open(cd->ppp, io);
--
1.7.1
7:26 p.m.
New subject: [PATCH 4/4] test: add script to set credentials for cdma connection
---
test/create-cdma-credentials | 29 +++++++++++++++++++++++++++++
1 files changed, 29 insertions(+), 0 deletions(-)
create mode 100755 test/create-cdma-credentials
diff --git a/test/create-cdma-credentials b/test/create-cdma-credentials
new file mode 100755
index 0000000..0f52a98
--- /dev/null
+++ b/test/create-cdma-credentials
@@ -0,0 +1,29 @@
+#!/usr/bin/python
+
+import dbus
+import sys
+
+bus = dbus.SystemBus()
+
+manager = dbus.Interface(bus.get_object('org.ofono', '/'),
+ 'org.ofono.Manager')
+
+modems = manager.GetModems()
+
+for path, properties in modems:
+ if "org.ofono.cdma.ConnectionManager" not in
properties["Interfaces"]:
+ continue
+
+ cm = dbus.Interface(bus.get_object('org.ofono', path),
+ 'org.ofono.cdma.ConnectionManager')
+
+ print "Connecting CDMA Packet Data Service on modem %s..." % path
+
+ if len(sys.argv) > 1:
+ cm.SetProperty("Username", (sys.argv[1]))
+ print "Setting Username to %s" % (sys.argv[1])
+
+ if len(sys.argv) > 2:
+ cm.SetProperty("Password", (sys.argv[2]))
+ print "Setting Password to %s" % (sys.argv[2])
+
--
1.7.1
7:48 p.m.
New subject: [PATCH 2/4] cdma-connman: add possibility to set a username and a password
Hi Guillaume,
src/cdma-connman.c | 44
+++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 43 insertions(+), 1 deletions(-)
diff --git a/src/cdma-connman.c b/src/cdma-connman.c
index 3321b87..7ca5a0a 100644
--- a/src/cdma-connman.c
+++ b/src/cdma-connman.c
@@ -57,6 +57,8 @@ struct ofono_cdma_connman {
const struct ofono_cdma_connman_driver *driver;
void *driver_data;
struct ofono_atom *atom;
+ char *username;
+ char *password;
};
I am still not convinced that this is needed. Please provide logs where
it shows that it fails otherwise.
static void cdma_connman_settings_free(struct cdma_connman_settings
*settings)
@@ -379,6 +381,8 @@ static DBusMessage *cdma_connman_set_property(DBusConnection *conn,
DBusMessageIter var;
const char *property;
dbus_bool_t value;
+ const char *username;
+ const char *password;
DBG("");
@@ -399,7 +403,7 @@ static DBusMessage *cdma_connman_set_property(DBusConnection *conn,
dbus_message_iter_recurse(&iter, &var);
- if (!strcmp(property, "Powered")) {
+ if (!g_strcmp0(property, "Powered")) {
Don't bother here. strcmp is just fine since we know both arguments are
not NULL.
Also if you wanna change this, then please have a separate patch for it.
Intermixing of code with style changes is not a good idea. And we have
always fixed style issues separately.
if (dbus_message_iter_get_arg_type(&var) !=
DBUS_TYPE_BOOLEAN)
return __ofono_error_invalid_args(msg);
@@ -421,6 +425,22 @@ static DBusMessage *cdma_connman_set_property(DBusConnection *conn,
cm->driver->deactivate(cm, deactivate_callback, cm);
return dbus_message_new_method_return(msg);
+ } else if (!g_strcmp0(property, "Username")) {
+ if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_STRING)
+ return __ofono_error_invalid_args(msg);
+
+ dbus_message_iter_get_basic(&var, &username);
+ cm->username = g_strdup(username);
+ DBG("username: %s", username);
+ return dbus_message_new_method_return(msg);
+ } else if (!g_strcmp0(property, "Password")) {
+ if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_STRING)
+ return __ofono_error_invalid_args(msg);
+
+ dbus_message_iter_get_basic(&var, &password);
+ cm->password = g_strdup(password);
+ DBG("password: %s", password);
+ return dbus_message_new_method_return(msg);
}
/* TODO: Dormant property. Not yet supported. */
@@ -488,6 +508,12 @@ static void cdma_connman_remove(struct ofono_atom *atom)
if (cm->driver && cm->driver->remove)
cm->driver->remove(cm);
+ if (cm->username)
+ g_free(cm->username);
+
+ if (cm->password)
+ g_free(cm->password);
+
The if check are pointless. g_free (and also free for that matter) does
a NULL check on its parameter.
@@ -568,3 +594,19 @@ void *ofono_cdma_connman_get_data(struct
ofono_cdma_connman *cm)
{
return cm->driver_data;
}
+
+const char *ofono_cdma_connman_get_username(struct ofono_cdma_connman *cm)
+{
+ if (cm->username)
+ return cm->username;
+
+ return NULL;
+}
+
+const char *ofono_cdma_connman_get_password(struct ofono_cdma_connman *cm)
+{
+ if (cm->password)
+ return cm->password;
+
+ return NULL;
+}
What is wrong with just "return cm->password;". This example is a bit
convoluted right now ;)
The next question is where you want the NULL handling. Or do you want it
all. Please think about that a little bit and maybe have a look at what
GPRS is doing.
Regards
Marcel
8:10 p.m.
New subject: [PATCH 2/4] cdma-connman: add possibility to set a username and a password
Hi Marcel,
On 20/07/2011 11:48, Marcel Holtmann wrote:
Hi Guillaume,
> src/cdma-connman.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 43 insertions(+), 1 deletions(-)
>
> diff --git a/src/cdma-connman.c b/src/cdma-connman.c
> index 3321b87..7ca5a0a 100644
> --- a/src/cdma-connman.c
> +++ b/src/cdma-connman.c
> @@ -57,6 +57,8 @@ struct ofono_cdma_connman {
> const struct ofono_cdma_connman_driver *driver;
> void *driver_data;
> struct ofono_atom *atom;
> + char *username;
> + char *password;
> };
I am still not convinced that this is needed. Please provide logs where
it shows that it fails otherwise.
You will find attached an ofono log giving the result in harcoding,
g_at_ppp_set_credentials(cd->ppp, "", ""); before launching PPP
session.
> static void cdma_connman_settings_free(struct cdma_connman_settings *settings)
> @@ -379,6 +381,8 @@ static DBusMessage *cdma_connman_set_property(DBusConnection
*conn,
> DBusMessageIter var;
> const char *property;
> dbus_bool_t value;
> + const char *username;
> + const char *password;
>
> DBG("");
>
> @@ -399,7 +403,7 @@ static DBusMessage *cdma_connman_set_property(DBusConnection
*conn,
>
> dbus_message_iter_recurse(&iter,&var);
>
> - if (!strcmp(property, "Powered")) {
> + if (!g_strcmp0(property, "Powered")) {
Don't bother here. strcmp is just fine since we know both arguments are
not NULL.
Also if you wanna change this, then please have a separate patch for it.
Intermixing of code with style changes is not a good idea. And we have
always fixed style issues separately.
Ok, I will keep strcmp() fro username and password also, and I will
commit g_strcmp0() later if needed.
> if (dbus_message_iter_get_arg_type(&var) !=
DBUS_TYPE_BOOLEAN)
> return __ofono_error_invalid_args(msg);
>
> @@ -421,6 +425,22 @@ static DBusMessage *cdma_connman_set_property(DBusConnection
*conn,
> cm->driver->deactivate(cm, deactivate_callback, cm);
>
> return dbus_message_new_method_return(msg);
> + } else if (!g_strcmp0(property, "Username")) {
> + if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_STRING)
> + return __ofono_error_invalid_args(msg);
> +
> + dbus_message_iter_get_basic(&var,&username);
> + cm->username = g_strdup(username);
> + DBG("username: %s", username);
> + return dbus_message_new_method_return(msg);
> + } else if (!g_strcmp0(property, "Password")) {
> + if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_STRING)
> + return __ofono_error_invalid_args(msg);
> +
> + dbus_message_iter_get_basic(&var,&password);
> + cm->password = g_strdup(password);
> + DBG("password: %s", password);
> + return dbus_message_new_method_return(msg);
> }
>
> /* TODO: Dormant property. Not yet supported. */
> @@ -488,6 +508,12 @@ static void cdma_connman_remove(struct ofono_atom *atom)
> if (cm->driver&& cm->driver->remove)
> cm->driver->remove(cm);
>
> + if (cm->username)
> + g_free(cm->username);
> +
> + if (cm->password)
> + g_free(cm->password);
> +
The if check are pointless. g_free (and also free for that matter) does
a NULL check on its parameter.
Right, I forgot about this null check...
> @@ -568,3 +594,19 @@ void *ofono_cdma_connman_get_data(struct
ofono_cdma_connman *cm)
> {
> return cm->driver_data;
> }
> +
> +const char *ofono_cdma_connman_get_username(struct ofono_cdma_connman *cm)
> +{
> + if (cm->username)
> + return cm->username;
> +
> + return NULL;
> +}
> +
> +const char *ofono_cdma_connman_get_password(struct ofono_cdma_connman *cm)
> +{
> + if (cm->password)
> + return cm->password;
> +
> + return NULL;
> +}
What is wrong with just "return cm->password;". This example is a bit
convoluted right now ;)
The next question is where you want the NULL handling. Or do you want it
all. Please think about that a little bit and maybe have a look at what
GPRS is doing.
Ok
Kind regards,
Guillaume
4078
days inactive
4078
days old
6 comments
2 participants
participants (2)
-
Guillaume Zajac -
Marcel Holtmann