[PATCH_v2 0/6] CDMA Network registration

[PATCH_v2 0/6] CDMA Network registration

[Guillaume Zajac]

[-- Attachment #1: Type: text/plain, Size: 950 bytes --]

Hi all,

Change log from v1:
	- create new D-Bus message error whenever modem is not registered to
	  the network
	- manage roaming case.
	- notify cdma-connman atom when data connection is lost.
	- remove cdma-netreg status watches.

Kind regards,
Guillaume

Guillaume Zajac (6):
  dbus: Add new D-Bus error message NotRegistered
  doc: Add RoamingAllowed property
  cdma-connman: Rely on roaming and cdma-netreg status to enable data
    call
  cdma-connman: Add public api to notify connection is lost
  cdma-connman: Add public api definition
  cdmamodem: Notify when connection is lost

 doc/cdma-connman-api.txt    |    8 ++++
 drivers/cdmamodem/connman.c |    2 +-
 include/cdma-connman.h      |    2 +
 src/cdma-connman.c          |   83 +++++++++++++++++++++++++++++++++++++++++--
 src/dbus.c                  |    7 ++++
 src/ofono.h                 |    1 +
 6 files changed, 99 insertions(+), 4 deletions(-)

[PATCH_v2 1/6] dbus: Add new D-Bus error message NotRegistered

[Guillaume Zajac]

[-- Attachment #1: Type: text/plain, Size: 1315 bytes --]

---
 src/dbus.c  |    7 +++++++
 src/ofono.h |    1 +
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/src/dbus.c b/src/dbus.c
index a96b595..be2833d 100644
--- a/src/dbus.c
+++ b/src/dbus.c
@@ -349,6 +349,13 @@ DBusMessage *__ofono_error_attach_in_progress(DBusMessage *msg)
 				"GPRS Attach is in progress");
 }
 
+DBusMessage *__ofono_error_not_registered(DBusMessage *msg)
+{
+	return g_dbus_create_error(msg,
+				OFONO_ERROR_INTERFACE ".NotRegistered",
+				"CDMA modem is not registered to the network");
+}
+
 DBusMessage *__ofono_error_canceled(DBusMessage *msg)
 {
 	return g_dbus_create_error(msg, OFONO_ERROR_INTERFACE ".Canceled",
diff --git a/src/ofono.h b/src/ofono.h
index bd45560..bfb534d 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -59,6 +59,7 @@ DBusMessage *__ofono_error_sim_not_ready(DBusMessage *msg);
 DBusMessage *__ofono_error_in_use(DBusMessage *msg);
 DBusMessage *__ofono_error_not_attached(DBusMessage *msg);
 DBusMessage *__ofono_error_attach_in_progress(DBusMessage *msg);
+DBusMessage *__ofono_error_not_registered(DBusMessage *msg);
 DBusMessage *__ofono_error_canceled(DBusMessage *msg);
 DBusMessage *__ofono_error_access_denied(DBusMessage *msg);
 DBusMessage *__ofono_error_emergency_active(DBusMessage *msg);
-- 
1.7.1

[PATCH_v2 2/6] doc: Add RoamingAllowed property

[Guillaume Zajac]

[-- Attachment #1: Type: text/plain, Size: 848 bytes --]

---
 doc/cdma-connman-api.txt |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/doc/cdma-connman-api.txt b/doc/cdma-connman-api.txt
index 48699a3..e4d3d2f 100644
--- a/doc/cdma-connman-api.txt
+++ b/doc/cdma-connman-api.txt
@@ -35,6 +35,14 @@ Properties	boolean Powered [readwrite]
 			Contains whether the connection is dormant.  Will
 			always be false if the connection is not powered.
 
+		boolean RoamingAllowed [readwrite]
+
+			Contains whether data roaming is allowed.  In the off
+			setting, if the packet radio registration state
+			indicates that the modem is roaming, oFono will
+			automatically disable connection and no further
+			connection establishment will be possible.
+
 		string Username [readwrite]
 
 			Holds the username to be used for authentication
-- 
1.7.1

[PATCH_v2 3/6] cdma-connman: Rely on roaming and cdma-netreg status to enable data call

[Guillaume Zajac]

[-- Attachment #1: Type: text/plain, Size: 4693 bytes --]

---
 src/cdma-connman.c |   65 +++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/src/cdma-connman.c b/src/cdma-connman.c
index db8f6c5..42dcca2 100644
--- a/src/cdma-connman.c
+++ b/src/cdma-connman.c
@@ -52,8 +52,11 @@ struct cdma_connman_settings {
 struct ofono_cdma_connman {
 	ofono_bool_t powered;
 	ofono_bool_t dormant;
+	ofono_bool_t roaming_allowed;
 	struct cdma_connman_settings *settings;
 	DBusMessage *pending;
+	struct ofono_cdma_netreg *cdma_netreg;
+	unsigned int cdma_netreg_watch;
 	const struct ofono_cdma_connman_driver *driver;
 	void *driver_data;
 	struct ofono_atom *atom;
@@ -338,6 +341,17 @@ static void cdma_connman_settings_append_properties(
 	dbus_message_iter_close_container(dict, &entry);
 }
 
+ofono_bool_t cdma_connman_netreg_update(struct ofono_cdma_connman *cm)
+{
+	ofono_bool_t registered;
+	int status  = ofono_cdma_netreg_get_status(cm->cdma_netreg);
+
+	registered = status == NETWORK_REGISTRATION_STATUS_REGISTERED;
+
+	return registered || (cm->roaming_allowed &&
+			status == NETWORK_REGISTRATION_STATUS_ROAMING);
+}
+
 static DBusMessage *cdma_connman_get_properties(DBusConnection *conn,
 						DBusMessage *msg, void *data)
 {
@@ -365,6 +379,10 @@ static DBusMessage *cdma_connman_get_properties(DBusConnection *conn,
 	value = cm->dormant;
 	ofono_dbus_dict_append(&dict, "Dormant", DBUS_TYPE_BOOLEAN, &value);
 
+	value = cm->roaming_allowed;
+	ofono_dbus_dict_append(&dict, "RoamingAllowed",
+				DBUS_TYPE_BOOLEAN, &value);
+
 	if (cm->settings)
 		cdma_connman_settings_append_properties(cm, &dict);
 
@@ -450,7 +468,23 @@ static DBusMessage *cdma_connman_set_property(DBusConnection *conn,
 
 	dbus_message_iter_recurse(&iter, &var);
 
-	if (!strcmp(property, "Powered")) {
+	if (!strcmp(property, "RoamingAllowed")) {
+		if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_BOOLEAN)
+			return __ofono_error_invalid_args(msg);
+
+		dbus_message_iter_get_basic(&var, &value);
+
+		if (cm->roaming_allowed == (ofono_bool_t) value)
+			return dbus_message_new_method_return(msg);
+
+		cm->roaming_allowed = value;
+
+		if (cdma_connman_netreg_update(cm) == FALSE &&
+				cm->powered == TRUE)
+			cm->driver->deactivate(cm, deactivate_callback, cm);
+
+		return NULL;
+	} if (!strcmp(property, "Powered")) {
 		if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_BOOLEAN)
 			return __ofono_error_invalid_args(msg);
 
@@ -463,9 +497,11 @@ static DBusMessage *cdma_connman_set_property(DBusConnection *conn,
 				cm->driver->deactivate == NULL)
 			return __ofono_error_not_implemented(msg);
 
+		if (cdma_connman_netreg_update(cm) == FALSE)
+			return __ofono_error_not_registered(msg);
+
 		cm->pending = dbus_message_ref(msg);
 
-		/* TODO: add logic to support CDMA Network Registration */
 		if (value)
 			cm->driver->activate(cm, cm->username, cm->password,
 						activate_callback, cm);
@@ -529,11 +565,18 @@ void ofono_cdma_connman_driver_unregister(
 static void cdma_connman_unregister(struct ofono_atom *atom)
 {
 	DBusConnection *conn = ofono_dbus_get_connection();
+	struct ofono_cdma_connman *cm = __ofono_atom_get_data(atom);
 	struct ofono_modem *modem = __ofono_atom_get_modem(atom);
 	const char *path = __ofono_atom_get_path(atom);
 
 	DBG("");
 
+	if (cm->cdma_netreg_watch) {
+		__ofono_modem_remove_atom_watch(modem, cm->cdma_netreg_watch);
+		cm->cdma_netreg_watch = 0;
+		cm->cdma_netreg = NULL;
+	}
+
 	g_dbus_unregister_interface(conn, path,
 				OFONO_CDMA_CONNECTION_MANAGER_INTERFACE);
 	ofono_modem_remove_interface(modem,
@@ -593,6 +636,20 @@ struct ofono_cdma_connman *ofono_cdma_connman_create(
 	return cm;
 }
 
+static void cdma_netreg_watch(struct ofono_atom *atom,
+				enum ofono_atom_watch_condition cond,
+				void *data)
+{
+	struct ofono_cdma_connman *cm = data;
+
+	if (cond == OFONO_ATOM_WATCH_CONDITION_UNREGISTERED) {
+		cm->cdma_netreg = NULL;
+		return;
+	}
+
+	cm->cdma_netreg = __ofono_atom_get_data(atom);
+}
+
 void ofono_cdma_connman_register(struct ofono_cdma_connman *cm)
 {
 	DBusConnection *conn = ofono_dbus_get_connection();
@@ -613,7 +670,9 @@ void ofono_cdma_connman_register(struct ofono_cdma_connman *cm)
 	ofono_modem_add_interface(modem,
 				OFONO_CDMA_CONNECTION_MANAGER_INTERFACE);
 
-	/* TODO: add watch to support CDMA Network Registration atom */
+	cm->cdma_netreg_watch = __ofono_modem_add_atom_watch(modem,
+					OFONO_ATOM_TYPE_CDMA_NETREG,
+					cdma_netreg_watch, cm, NULL);
 
 	__ofono_atom_register(cm->atom, cdma_connman_unregister);
 }
-- 
1.7.1

[PATCH_v2 4/6] cdma-connman: Add public api to notify connection is lost

[Guillaume Zajac]

[-- Attachment #1: Type: text/plain, Size: 610 bytes --]

---
 include/cdma-connman.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/cdma-connman.h b/include/cdma-connman.h
index b8efe0b..7a115bf 100644
--- a/include/cdma-connman.h
+++ b/include/cdma-connman.h
@@ -64,6 +64,8 @@ int ofono_cdma_connman_driver_register(
 void ofono_cdma_connman_driver_unregister(
 				const struct ofono_cdma_connman_driver *d);
 
+void ofono_cdma_connman_deactivated(struct ofono_cdma_connman *cm);
+
 struct ofono_cdma_connman *ofono_cdma_connman_create(
 						struct ofono_modem *modem,
 						unsigned int vendor,
-- 
1.7.1

[PATCH_v2 5/6] cdma-connman: Add public api definition

[Guillaume Zajac]

[-- Attachment #1: Type: text/plain, Size: 962 bytes --]

---
 src/cdma-connman.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/src/cdma-connman.c b/src/cdma-connman.c
index 42dcca2..f88353a 100644
--- a/src/cdma-connman.c
+++ b/src/cdma-connman.c
@@ -562,6 +562,24 @@ void ofono_cdma_connman_driver_unregister(
 	g_drivers = g_slist_remove(g_drivers, (void *) d);
 }
 
+void ofono_cdma_connman_deactivated(struct ofono_cdma_connman *cm)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path;
+	dbus_bool_t value;
+
+	if (cm == NULL)
+		return;
+
+	cdma_connman_settings_reset(cm);
+	value = FALSE;
+	path = __ofono_atom_get_path(cm->atom);
+
+	ofono_dbus_signal_property_changed(conn, path,
+				OFONO_CDMA_CONNECTION_MANAGER_INTERFACE,
+				"Powered", DBUS_TYPE_BOOLEAN, &value);
+}
+
 static void cdma_connman_unregister(struct ofono_atom *atom)
 {
 	DBusConnection *conn = ofono_dbus_get_connection();
-- 
1.7.1

[PATCH_v2 6/6] cdmamodem: Notify when connection is lost

[Guillaume Zajac]

[-- Attachment #1: Type: text/plain, Size: 574 bytes --]

---
 drivers/cdmamodem/connman.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/cdmamodem/connman.c b/drivers/cdmamodem/connman.c
index 49f60ac..ec17e96 100644
--- a/drivers/cdmamodem/connman.c
+++ b/drivers/cdmamodem/connman.c
@@ -116,7 +116,7 @@ static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user_data)
 		CALLBACK_WITH_SUCCESS(cd->down_cb, cd->cb_data);
 		break;
 	default:
-		/* TODO: Handle network initiated disconnection */
+		ofono_cdma_connman_deactivated(cm);
 		break;
 	}
 
-- 
1.7.1

Re: [PATCH_v2 3/6] cdma-connman: Rely on roaming and cdma-netreg status to enable data call

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 5493 bytes --]

Hi Guillaume,

On 11/02/2011 05:37 AM, Guillaume Zajac wrote:
> ---
>  src/cdma-connman.c |   65 +++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 62 insertions(+), 3 deletions(-)
> 
> diff --git a/src/cdma-connman.c b/src/cdma-connman.c
> index db8f6c5..42dcca2 100644
> --- a/src/cdma-connman.c
> +++ b/src/cdma-connman.c
> @@ -52,8 +52,11 @@ struct cdma_connman_settings {
>  struct ofono_cdma_connman {
>  	ofono_bool_t powered;
>  	ofono_bool_t dormant;
> +	ofono_bool_t roaming_allowed;
>  	struct cdma_connman_settings *settings;
>  	DBusMessage *pending;
> +	struct ofono_cdma_netreg *cdma_netreg;
> +	unsigned int cdma_netreg_watch;
>  	const struct ofono_cdma_connman_driver *driver;
>  	void *driver_data;
>  	struct ofono_atom *atom;
> @@ -338,6 +341,17 @@ static void cdma_connman_settings_append_properties(
>  	dbus_message_iter_close_container(dict, &entry);
>  }
>  
> +ofono_bool_t cdma_connman_netreg_update(struct ofono_cdma_connman *cm)
> +{

I think this function should be named a bit better.  Wouldn't
is_registered be clearer?

> +	ofono_bool_t registered;
> +	int status  = ofono_cdma_netreg_get_status(cm->cdma_netreg);
> +
> +	registered = status == NETWORK_REGISTRATION_STATUS_REGISTERED;
> +
> +	return registered || (cm->roaming_allowed &&
> +			status == NETWORK_REGISTRATION_STATUS_ROAMING);

You seem to be doing quite a bit of work to accomplish your goal.  If
you use __ofono_modem_find_atom then you can drop all the atom watch
bits and make this implementation way easier.

> +}
> +
>  static DBusMessage *cdma_connman_get_properties(DBusConnection *conn,
>  						DBusMessage *msg, void *data)
>  {
> @@ -365,6 +379,10 @@ static DBusMessage *cdma_connman_get_properties(DBusConnection *conn,
>  	value = cm->dormant;
>  	ofono_dbus_dict_append(&dict, "Dormant", DBUS_TYPE_BOOLEAN, &value);
>  
> +	value = cm->roaming_allowed;
> +	ofono_dbus_dict_append(&dict, "RoamingAllowed",
> +				DBUS_TYPE_BOOLEAN, &value);
> +
>  	if (cm->settings)
>  		cdma_connman_settings_append_properties(cm, &dict);
>  
> @@ -450,7 +468,23 @@ static DBusMessage *cdma_connman_set_property(DBusConnection *conn,
>  
>  	dbus_message_iter_recurse(&iter, &var);
>  
> -	if (!strcmp(property, "Powered")) {
> +	if (!strcmp(property, "RoamingAllowed")) {
> +		if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_BOOLEAN)
> +			return __ofono_error_invalid_args(msg);
> +
> +		dbus_message_iter_get_basic(&var, &value);
> +
> +		if (cm->roaming_allowed == (ofono_bool_t) value)
> +			return dbus_message_new_method_return(msg);
> +
> +		cm->roaming_allowed = value;
> +
> +		if (cdma_connman_netreg_update(cm) == FALSE &&
> +				cm->powered == TRUE)
> +			cm->driver->deactivate(cm, deactivate_callback, cm);
> +

Can you please put the RoamingAllowed details in a separate patch
(preferably among the last in the series), this one might need to be
thought about some more.

> +		return NULL;
> +	} if (!strcmp(property, "Powered")) {
>  		if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_BOOLEAN)
>  			return __ofono_error_invalid_args(msg);
>  
> @@ -463,9 +497,11 @@ static DBusMessage *cdma_connman_set_property(DBusConnection *conn,
>  				cm->driver->deactivate == NULL)
>  			return __ofono_error_not_implemented(msg);
>  
> +		if (cdma_connman_netreg_update(cm) == FALSE)
> +			return __ofono_error_not_registered(msg);
> +
>  		cm->pending = dbus_message_ref(msg);
>  
> -		/* TODO: add logic to support CDMA Network Registration */
>  		if (value)
>  			cm->driver->activate(cm, cm->username, cm->password,
>  						activate_callback, cm);
> @@ -529,11 +565,18 @@ void ofono_cdma_connman_driver_unregister(
>  static void cdma_connman_unregister(struct ofono_atom *atom)
>  {
>  	DBusConnection *conn = ofono_dbus_get_connection();
> +	struct ofono_cdma_connman *cm = __ofono_atom_get_data(atom);
>  	struct ofono_modem *modem = __ofono_atom_get_modem(atom);
>  	const char *path = __ofono_atom_get_path(atom);
>  
>  	DBG("");
>  
> +	if (cm->cdma_netreg_watch) {
> +		__ofono_modem_remove_atom_watch(modem, cm->cdma_netreg_watch);
> +		cm->cdma_netreg_watch = 0;
> +		cm->cdma_netreg = NULL;
> +	}
> +
>  	g_dbus_unregister_interface(conn, path,
>  				OFONO_CDMA_CONNECTION_MANAGER_INTERFACE);
>  	ofono_modem_remove_interface(modem,
> @@ -593,6 +636,20 @@ struct ofono_cdma_connman *ofono_cdma_connman_create(
>  	return cm;
>  }
>  
> +static void cdma_netreg_watch(struct ofono_atom *atom,
> +				enum ofono_atom_watch_condition cond,
> +				void *data)
> +{
> +	struct ofono_cdma_connman *cm = data;
> +
> +	if (cond == OFONO_ATOM_WATCH_CONDITION_UNREGISTERED) {
> +		cm->cdma_netreg = NULL;
> +		return;
> +	}
> +
> +	cm->cdma_netreg = __ofono_atom_get_data(atom);
> +}
> +
>  void ofono_cdma_connman_register(struct ofono_cdma_connman *cm)
>  {
>  	DBusConnection *conn = ofono_dbus_get_connection();
> @@ -613,7 +670,9 @@ void ofono_cdma_connman_register(struct ofono_cdma_connman *cm)
>  	ofono_modem_add_interface(modem,
>  				OFONO_CDMA_CONNECTION_MANAGER_INTERFACE);
>  
> -	/* TODO: add watch to support CDMA Network Registration atom */
> +	cm->cdma_netreg_watch = __ofono_modem_add_atom_watch(modem,
> +					OFONO_ATOM_TYPE_CDMA_NETREG,
> +					cdma_netreg_watch, cm, NULL);
>  
>  	__ofono_atom_register(cm->atom, cdma_connman_unregister);
>  }

Regards,
-Denis

Re: [PATCH_v2 5/6] cdma-connman: Add public api definition

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1158 bytes --]

Hi Guillaume,

On 11/02/2011 05:38 AM, Guillaume Zajac wrote:
> ---
>  src/cdma-connman.c |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
> 
> diff --git a/src/cdma-connman.c b/src/cdma-connman.c
> index 42dcca2..f88353a 100644
> --- a/src/cdma-connman.c
> +++ b/src/cdma-connman.c
> @@ -562,6 +562,24 @@ void ofono_cdma_connman_driver_unregister(
>  	g_drivers = g_slist_remove(g_drivers, (void *) d);
>  }
>  
> +void ofono_cdma_connman_deactivated(struct ofono_cdma_connman *cm)
> +{
> +	DBusConnection *conn = ofono_dbus_get_connection();
> +	const char *path;
> +	dbus_bool_t value;
> +
> +	if (cm == NULL)
> +		return;
> +
> +	cdma_connman_settings_reset(cm);
> +	value = FALSE;
> +	path = __ofono_atom_get_path(cm->atom);

Don't you also want to reset cm->powered accordingly?

> +
> +	ofono_dbus_signal_property_changed(conn, path,
> +				OFONO_CDMA_CONNECTION_MANAGER_INTERFACE,
> +				"Powered", DBUS_TYPE_BOOLEAN, &value);
> +}
> +
>  static void cdma_connman_unregister(struct ofono_atom *atom)
>  {
>  	DBusConnection *conn = ofono_dbus_get_connection();

Regards,
-Denis

Re: [PATCH_v2 4/6] cdma-connman: Add public api to notify connection is lost

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 750 bytes --]

Hi Guillaume,

On 11/02/2011 05:37 AM, Guillaume Zajac wrote:
> ---
>  include/cdma-connman.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/include/cdma-connman.h b/include/cdma-connman.h
> index b8efe0b..7a115bf 100644
> --- a/include/cdma-connman.h
> +++ b/include/cdma-connman.h
> @@ -64,6 +64,8 @@ int ofono_cdma_connman_driver_register(
>  void ofono_cdma_connman_driver_unregister(
>  				const struct ofono_cdma_connman_driver *d);
>  
> +void ofono_cdma_connman_deactivated(struct ofono_cdma_connman *cm);
> +
>  struct ofono_cdma_connman *ofono_cdma_connman_create(
>  						struct ofono_modem *modem,
>  						unsigned int vendor,

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH_v2 3/6] cdma-connman: Rely on roaming and cdma-netreg status to enable data call

[Guillaume Zajac]

[-- Attachment #1: Type: text/plain, Size: 5867 bytes --]

Hi Denis,

On 02/11/2011 16:27, Denis Kenzior wrote:
> Hi Guillaume,
>
> On 11/02/2011 05:37 AM, Guillaume Zajac wrote:
>> ---
>>   src/cdma-connman.c |   65 +++++++++++++++++++++++++++++++++++++++++++++++++--
>>   1 files changed, 62 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/cdma-connman.c b/src/cdma-connman.c
>> index db8f6c5..42dcca2 100644
>> --- a/src/cdma-connman.c
>> +++ b/src/cdma-connman.c
>> @@ -52,8 +52,11 @@ struct cdma_connman_settings {
>>   struct ofono_cdma_connman {
>>   	ofono_bool_t powered;
>>   	ofono_bool_t dormant;
>> +	ofono_bool_t roaming_allowed;
>>   	struct cdma_connman_settings *settings;
>>   	DBusMessage *pending;
>> +	struct ofono_cdma_netreg *cdma_netreg;
>> +	unsigned int cdma_netreg_watch;
>>   	const struct ofono_cdma_connman_driver *driver;
>>   	void *driver_data;
>>   	struct ofono_atom *atom;
>> @@ -338,6 +341,17 @@ static void cdma_connman_settings_append_properties(
>>   	dbus_message_iter_close_container(dict,&entry);
>>   }
>>
>> +ofono_bool_t cdma_connman_netreg_update(struct ofono_cdma_connman *cm)
>> +{
> I think this function should be named a bit better.  Wouldn't
> is_registered be clearer?

Yes.

>> +	ofono_bool_t registered;
>> +	int status  = ofono_cdma_netreg_get_status(cm->cdma_netreg);
>> +
>> +	registered = status == NETWORK_REGISTRATION_STATUS_REGISTERED;
>> +
>> +	return registered || (cm->roaming_allowed&&
>> +			status == NETWORK_REGISTRATION_STATUS_ROAMING);
> You seem to be doing quite a bit of work to accomplish your goal.  If
> you use __ofono_modem_find_atom then you can drop all the atom watch
> bits and make this implementation way easier.

I will remove the atom watch and try to find it, if it fails I will 
return FALSE.

>> +}
>> +
>>   static DBusMessage *cdma_connman_get_properties(DBusConnection *conn,
>>   						DBusMessage *msg, void *data)
>>   {
>> @@ -365,6 +379,10 @@ static DBusMessage *cdma_connman_get_properties(DBusConnection *conn,
>>   	value = cm->dormant;
>>   	ofono_dbus_dict_append(&dict, "Dormant", DBUS_TYPE_BOOLEAN,&value);
>>
>> +	value = cm->roaming_allowed;
>> +	ofono_dbus_dict_append(&dict, "RoamingAllowed",
>> +				DBUS_TYPE_BOOLEAN,&value);
>> +
>>   	if (cm->settings)
>>   		cdma_connman_settings_append_properties(cm,&dict);
>>
>> @@ -450,7 +468,23 @@ static DBusMessage *cdma_connman_set_property(DBusConnection *conn,
>>
>>   	dbus_message_iter_recurse(&iter,&var);
>>
>> -	if (!strcmp(property, "Powered")) {
>> +	if (!strcmp(property, "RoamingAllowed")) {
>> +		if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_BOOLEAN)
>> +			return __ofono_error_invalid_args(msg);
>> +
>> +		dbus_message_iter_get_basic(&var,&value);
>> +
>> +		if (cm->roaming_allowed == (ofono_bool_t) value)
>> +			return dbus_message_new_method_return(msg);
>> +
>> +		cm->roaming_allowed = value;
>> +
>> +		if (cdma_connman_netreg_update(cm) == FALSE&&
>> +				cm->powered == TRUE)
>> +			cm->driver->deactivate(cm, deactivate_callback, cm);
>> +
> Can you please put the RoamingAllowed details in a separate patch
> (preferably among the last in the series), this one might need to be
> thought about some more.

Ok I will separate it.

>> +		return NULL;
>> +	} if (!strcmp(property, "Powered")) {
>>   		if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_BOOLEAN)
>>   			return __ofono_error_invalid_args(msg);
>>
>> @@ -463,9 +497,11 @@ static DBusMessage *cdma_connman_set_property(DBusConnection *conn,
>>   				cm->driver->deactivate == NULL)
>>   			return __ofono_error_not_implemented(msg);
>>
>> +		if (cdma_connman_netreg_update(cm) == FALSE)
>> +			return __ofono_error_not_registered(msg);
>> +
>>   		cm->pending = dbus_message_ref(msg);
>>
>> -		/* TODO: add logic to support CDMA Network Registration */
>>   		if (value)
>>   			cm->driver->activate(cm, cm->username, cm->password,
>>   						activate_callback, cm);
>> @@ -529,11 +565,18 @@ void ofono_cdma_connman_driver_unregister(
>>   static void cdma_connman_unregister(struct ofono_atom *atom)
>>   {
>>   	DBusConnection *conn = ofono_dbus_get_connection();
>> +	struct ofono_cdma_connman *cm = __ofono_atom_get_data(atom);
>>   	struct ofono_modem *modem = __ofono_atom_get_modem(atom);
>>   	const char *path = __ofono_atom_get_path(atom);
>>
>>   	DBG("");
>>
>> +	if (cm->cdma_netreg_watch) {
>> +		__ofono_modem_remove_atom_watch(modem, cm->cdma_netreg_watch);
>> +		cm->cdma_netreg_watch = 0;
>> +		cm->cdma_netreg = NULL;
>> +	}
>> +
>>   	g_dbus_unregister_interface(conn, path,
>>   				OFONO_CDMA_CONNECTION_MANAGER_INTERFACE);
>>   	ofono_modem_remove_interface(modem,
>> @@ -593,6 +636,20 @@ struct ofono_cdma_connman *ofono_cdma_connman_create(
>>   	return cm;
>>   }
>>
>> +static void cdma_netreg_watch(struct ofono_atom *atom,
>> +				enum ofono_atom_watch_condition cond,
>> +				void *data)
>> +{
>> +	struct ofono_cdma_connman *cm = data;
>> +
>> +	if (cond == OFONO_ATOM_WATCH_CONDITION_UNREGISTERED) {
>> +		cm->cdma_netreg = NULL;
>> +		return;
>> +	}
>> +
>> +	cm->cdma_netreg = __ofono_atom_get_data(atom);
>> +}
>> +
>>   void ofono_cdma_connman_register(struct ofono_cdma_connman *cm)
>>   {
>>   	DBusConnection *conn = ofono_dbus_get_connection();
>> @@ -613,7 +670,9 @@ void ofono_cdma_connman_register(struct ofono_cdma_connman *cm)
>>   	ofono_modem_add_interface(modem,
>>   				OFONO_CDMA_CONNECTION_MANAGER_INTERFACE);
>>
>> -	/* TODO: add watch to support CDMA Network Registration atom */
>> +	cm->cdma_netreg_watch = __ofono_modem_add_atom_watch(modem,
>> +					OFONO_ATOM_TYPE_CDMA_NETREG,
>> +					cdma_netreg_watch, cm, NULL);
>>
>>   	__ofono_atom_register(cm->atom, cdma_connman_unregister);
>>   }
> Regards,
> -Denis
>

Kind regards,
Guillaume

Re: [PATCH_v2 5/6] cdma-connman: Add public api definition

[Guillaume Zajac]

[-- Attachment #1: Type: text/plain, Size: 1309 bytes --]

Hi Denis,

On 02/11/2011 16:32, Denis Kenzior wrote:
> Hi Guillaume,
>
> On 11/02/2011 05:38 AM, Guillaume Zajac wrote:
>> ---
>>   src/cdma-connman.c |   18 ++++++++++++++++++
>>   1 files changed, 18 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/cdma-connman.c b/src/cdma-connman.c
>> index 42dcca2..f88353a 100644
>> --- a/src/cdma-connman.c
>> +++ b/src/cdma-connman.c
>> @@ -562,6 +562,24 @@ void ofono_cdma_connman_driver_unregister(
>>   	g_drivers = g_slist_remove(g_drivers, (void *) d);
>>   }
>>
>> +void ofono_cdma_connman_deactivated(struct ofono_cdma_connman *cm)
>> +{
>> +	DBusConnection *conn = ofono_dbus_get_connection();
>> +	const char *path;
>> +	dbus_bool_t value;
>> +
>> +	if (cm == NULL)
>> +		return;
>> +
>> +	cdma_connman_settings_reset(cm);
>> +	value = FALSE;
>> +	path = __ofono_atom_get_path(cm->atom);
> Don't you also want to reset cm->powered accordingly?

Yes, I forgot it.

>> +
>> +	ofono_dbus_signal_property_changed(conn, path,
>> +				OFONO_CDMA_CONNECTION_MANAGER_INTERFACE,
>> +				"Powered", DBUS_TYPE_BOOLEAN,&value);
>> +}
>> +
>>   static void cdma_connman_unregister(struct ofono_atom *atom)
>>   {
>>   	DBusConnection *conn = ofono_dbus_get_connection();
> Regards,
> -Denis
>

Kind regards,
Guillaume