* [PATCH 0/4] CDMA-connman add username and password properties
@ 2011-07-20 9:26 Guillaume Zajac
2011-07-20 9:26 ` [PATCH 1/4] cdma-connman: add prototype to retrieve username and password Guillaume Zajac
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Guillaume Zajac @ 2011-07-20 9:26 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 787 bytes --]
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
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/4] cdma-connman: add prototype to retrieve username and password 2011-07-20 9:26 [PATCH 0/4] CDMA-connman add username and password properties Guillaume Zajac @ 2011-07-20 9:26 ` Guillaume Zajac 2011-07-20 9:26 ` [PATCH 2/4] cdma-connman: add possibility to set a username and a password Guillaume Zajac ` (2 subsequent siblings) 3 siblings, 0 replies; 7+ messages in thread From: Guillaume Zajac @ 2011-07-20 9:26 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 634 bytes --] --- 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/4] cdma-connman: add possibility to set a username and a password 2011-07-20 9:26 [PATCH 0/4] CDMA-connman add username and password properties Guillaume Zajac 2011-07-20 9:26 ` [PATCH 1/4] cdma-connman: add prototype to retrieve username and password Guillaume Zajac @ 2011-07-20 9:26 ` Guillaume Zajac 2011-07-20 9:48 ` Marcel Holtmann 2011-07-20 9:26 ` [PATCH 3/4] driver cdma-connman: add credential while setting up PPP if required Guillaume Zajac 2011-07-20 9:26 ` [PATCH 4/4] test: add script to set credentials for cdma connection Guillaume Zajac 3 siblings, 1 reply; 7+ messages in thread From: Guillaume Zajac @ 2011-07-20 9:26 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 2752 bytes --] --- 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/4] cdma-connman: add possibility to set a username and a password 2011-07-20 9:26 ` [PATCH 2/4] cdma-connman: add possibility to set a username and a password Guillaume Zajac @ 2011-07-20 9:48 ` Marcel Holtmann 2011-07-20 10:10 ` Guillaume Zajac 0 siblings, 1 reply; 7+ messages in thread From: Marcel Holtmann @ 2011-07-20 9:48 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 3682 bytes --] 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/4] cdma-connman: add possibility to set a username and a password 2011-07-20 9:48 ` Marcel Holtmann @ 2011-07-20 10:10 ` Guillaume Zajac 0 siblings, 0 replies; 7+ messages in thread From: Guillaume Zajac @ 2011-07-20 10:10 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 4172 bytes --] 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 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: ofono-cdma.log --] [-- Type: text/x-log, Size: 2375 bytes --] ofonod[8735]: plugins/speedupcdma.c:speedupcdma_enable() path is: /dev/ttyUSB0 ofonod[8735]: Modem: > ATE0 +CMEE=1\r ofonod[8735]: plugins/bluetooth.c:manager_properties_cb() ofonod[8735]: plugins/bluetooth.c:parse_adapters() ofonod[8735]: plugins/bluetooth.c:parse_adapters() Calling GetProperties on /org/bluez/1461/hci0 ofonod[8735]: plugins/bluetooth.c:adapter_properties_cb() ofonod[8735]: plugins/bluetooth.c:parse_devices() ofonod[8735]: plugins/bluetooth.c:adapter_properties_cb() Adapter Address: 5C:AC:4C:FF:0C:05, Path: /org/bluez/1461/hci0 ofonod[8735]: Modem: < \r\nOK\r\n ofonod[8735]: Modem: > AT+CFUN=1\r ofonod[8735]: Modem: < \r\nOK\r\n ofonod[8735]: plugins/speedupcdma.c:cfun_enable() ofonod[8735]: src/modem.c:modem_change_state() old state: 0, new state: 1 ofonod[8735]: plugins/speedupcdma.c:speedupcdma_pre_sim() 0x85f7170 ofonod[8735]: src/modem.c:modem_change_state() old state: 1, new state: 2 ofonod[8735]: plugins/speedupcdma.c:speedupcdma_post_sim() 0x85f7170 ofonod[8735]: src/modem.c:modem_change_state() old state: 2, new state: 3 ofonod[8735]: plugins/speedupcdma.c:speedupcdma_post_online() 0x85f7170 ofonod[8735]: src/cdma-connman.c:ofono_cdma_connman_create() ofonod[8735]: drivers/cdmamodem/connman.c:cdma_connman_probe() ofonod[8735]: Modem: > AT&C0\r ofonod[8735]: Modem: < \r\nOK\r\n ofonod[8735]: drivers/cdmamodem/connman.c:at_c0_cb() ok 1 ofonod[8735]: src/cdma-connman.c:ofono_cdma_connman_register() ofonod[8735]: Modem: > AT+GMI\r ofonod[8735]: Modem: < \r\nManufacturer\r\n\r\nOK\r\n ofonod[8735]: Modem: > AT+GMM\r ofonod[8735]: Modem: < \r\nEVDO USB MODEM\r\n\r\nOK\r\n ofonod[8735]: Modem: > AT+GMR\r ofonod[8735]: Modem: < \r\nLCA0017.1.1_M034\r\n\r\nOK\r\n ofonod[8735]: Modem: > AT+GSN\r ofonod[8735]: Modem: < \r\n0x904BC325\r\n\r\nOK\r\n ofonod[8735]: src/cdma-connman.c:cdma_connman_set_property() ofonod[8735]: drivers/cdmamodem/connman.c:cdma_connman_activate() ofonod[8735]: Modem: > ATD#777\r ofonod[8735]: Modem: < \r\nCONNECT 3100000\r\n ofonod[8735]: drivers/cdmamodem/connman.c:atd_cb() ok 1 ofonod[8735]: drivers/cdmamodem/connman.c:setup_ppp() ofonod[8735]: drivers/cdmamodem/connman.c:ppp_disconnect() ofonod[8735]: src/cdma-connman.c:activate_callback() 0x85f2ca8 (null) ofonod[8735]: src/cdma-connman.c:activate_callback() Activating packet data service failed with error: Unknown error type ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/4] driver cdma-connman: add credential while setting up PPP if required 2011-07-20 9:26 [PATCH 0/4] CDMA-connman add username and password properties Guillaume Zajac 2011-07-20 9:26 ` [PATCH 1/4] cdma-connman: add prototype to retrieve username and password Guillaume Zajac 2011-07-20 9:26 ` [PATCH 2/4] cdma-connman: add possibility to set a username and a password Guillaume Zajac @ 2011-07-20 9:26 ` Guillaume Zajac 2011-07-20 9:26 ` [PATCH 4/4] test: add script to set credentials for cdma connection Guillaume Zajac 3 siblings, 0 replies; 7+ messages in thread From: Guillaume Zajac @ 2011-07-20 9:26 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 1059 bytes --] --- 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/4] test: add script to set credentials for cdma connection 2011-07-20 9:26 [PATCH 0/4] CDMA-connman add username and password properties Guillaume Zajac ` (2 preceding siblings ...) 2011-07-20 9:26 ` [PATCH 3/4] driver cdma-connman: add credential while setting up PPP if required Guillaume Zajac @ 2011-07-20 9:26 ` Guillaume Zajac 3 siblings, 0 replies; 7+ messages in thread From: Guillaume Zajac @ 2011-07-20 9:26 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 1138 bytes --] --- 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-07-20 10:10 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-07-20 9:26 [PATCH 0/4] CDMA-connman add username and password properties Guillaume Zajac 2011-07-20 9:26 ` [PATCH 1/4] cdma-connman: add prototype to retrieve username and password Guillaume Zajac 2011-07-20 9:26 ` [PATCH 2/4] cdma-connman: add possibility to set a username and a password Guillaume Zajac 2011-07-20 9:48 ` Marcel Holtmann 2011-07-20 10:10 ` Guillaume Zajac 2011-07-20 9:26 ` [PATCH 3/4] driver cdma-connman: add credential while setting up PPP if required Guillaume Zajac 2011-07-20 9:26 ` [PATCH 4/4] test: add script to set credentials for cdma connection Guillaume Zajac
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox