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
_______________________________________________
ofono mailing list
[email protected]
http://lists.ofono.org/listinfo/ofono
