Hi Lucas,
On 11/23/2010 12:04 PM, Lucas De Marchi wrote:
> ---
> Makefile.am | 2 +-
> src/ofono.h | 2 +
> src/text-telephony.c | 341
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 344 insertions(+), 1 deletions(-)
> create mode 100644 src/text-telephony.c
>
> diff --git a/Makefile.am b/Makefile.am
> index fb075a0..ee1313d 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -319,7 +319,7 @@ src_ofonod_SOURCES = $(gdbus_sources) $(builtin_sources)
> src/ofono.ver \
> src/radio-settings.c src/stkutil.h src/stkutil.c \
> src/nettime.c src/stkagent.c src/stkagent.h \
> src/simfs.c src/simfs.h src/audio-settings.c \
> - src/smsagent.c src/smsagent.h
> + src/smsagent.c src/smsagent.h src/text-telephony.c
>
> src_ofonod_LDADD = $(builtin_libadd) @GLIB_LIBS@ @DBUS_LIBS@ @CAPNG_LIBS@
> -ldl
>
> diff --git a/src/ofono.h b/src/ofono.h
> index d1a4bdc..42736bb 100644
> --- a/src/ofono.h
> +++ b/src/ofono.h
> @@ -125,6 +125,7 @@ enum ofono_atom_type {
> OFONO_ATOM_TYPE_AUDIO_SETTINGS = 19,
> OFONO_ATOM_TYPE_STK = 20,
> OFONO_ATOM_TYPE_NETTIME = 21,
> + OFONO_ATOM_TYPE_TEXT_TELEPHONY = 22,
> };
>
> enum ofono_atom_watch_condition {
> @@ -205,6 +206,7 @@ gboolean __ofono_call_settings_is_busy(struct
> ofono_call_settings *cs);
> #include <ofono/gprs-context.h>
> #include <ofono/radio-settings.h>
> #include <ofono/audio-settings.h>
> +#include <ofono/text-telephony.h>
>
> #include <ofono/voicecall.h>
>
> diff --git a/src/text-telephony.c b/src/text-telephony.c
> new file mode 100644
> index 0000000..df87378
> --- /dev/null
> +++ b/src/text-telephony.c
> @@ -0,0 +1,341 @@
> +/*
> + *
> + * oFono - Open Source Telephony
> + *
> + * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
> + * Copyright (C) 2010 Intel Corporation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301
> USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <string.h>
> +#include <stdio.h>
> +#include <errno.h>
> +
> +#include <glib.h>
> +#include <gdbus.h>
> +
> +#include "ofono.h"
> +#include "common.h"
> +
> +#define TEXT_TELEPHONY_FLAG_CACHED 0x1
> +
> +static GSList *g_drivers = NULL;
> +
> +struct ofono_text_telephony {
> + DBusMessage *pending;
> + int flags;
> + ofono_bool_t powered;
> + ofono_bool_t powered_pending;
> + const struct ofono_text_telephony_driver *driver;
> + void *driver_data;
> + struct ofono_atom *atom;
> +};
> +
> +static DBusMessage *tt_get_properties_reply(DBusMessage *msg,
> + struct ofono_text_telephony *tt)
> +{
> + DBusMessage *reply;
> + DBusMessageIter iter;
> + DBusMessageIter dict;
> + dbus_bool_t value;
> +
> + reply = dbus_message_new_method_return(msg);
> + if (!reply)
> + return NULL;
> +
> + dbus_message_iter_init_append(reply, &iter);
> + dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
> + OFONO_PROPERTIES_ARRAY_SIGNATURE,
> + &dict);
> +
> + value = tt->powered;
> + ofono_dbus_dict_append(&dict, "Powered", DBUS_TYPE_BOOLEAN, &value);
As mentioned previously, please use "Enabled" here.
> + dbus_message_iter_close_container(&iter, &dict);
> +
> + return reply;
> +}
> +
> +static void tt_set_powered(struct ofono_text_telephony *tt,
> + ofono_bool_t enable)
> +{
> + DBusConnection *conn = ofono_dbus_get_connection();
> + const char *path = __ofono_atom_get_path(tt->atom);
> + dbus_bool_t value = enable;
> +
> + if (tt->powered == enable)
> + return;
> +
> + ofono_dbus_signal_property_changed(conn, path,
> + OFONO_TEXT_TELEPHONY_INTERFACE,
> + "Powered",
> + DBUS_TYPE_BOOLEAN, &value);
> + tt->powered = enable;
> +}
> +
> +static void tt_set_powered_callback(const struct ofono_error *error,
> + void *data)
> +{
> + struct ofono_text_telephony *tt = data;
> + DBusMessage *reply;
> +
> + if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> + DBG("Error setting powered property");
> +
> + tt->powered_pending = tt->powered;
> +
> + reply = __ofono_error_failed(tt->pending);
> + __ofono_dbus_pending_reply(&tt->pending, reply);
> +
> + return;
> + }
> +
> + reply = dbus_message_new_method_return(tt->pending);
> + __ofono_dbus_pending_reply(&tt->pending, reply);
> +
> + tt_set_powered(tt, tt->powered_pending);
> +}
> +
> +static void tt_send_properties_reply(struct ofono_text_telephony *tt)
> +{
> + DBusMessage *reply;
> +
> + tt->flags |= TEXT_TELEPHONY_FLAG_CACHED;
> +
> + reply = tt_get_properties_reply(tt->pending, tt);
> + __ofono_dbus_pending_reply(&tt->pending, reply);
I actually prefer to fold this function into powered_callback
> +}
> +
> +static void tt_query_powered_callback(const struct ofono_error *error,
> + ofono_bool_t enable, void *data)
> +{
> + struct ofono_text_telephony *tt = data;
> +
> + if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> + DBusMessage *reply;
> +
> + ofono_debug("Error during powered query");
> +
> + reply = __ofono_error_failed(tt->pending);
> + __ofono_dbus_pending_reply(&tt->pending, reply);
> +
> + return;
> + }
> +
> + tt_set_powered(tt, enable);
> + tt_send_properties_reply(tt);
The oFono convention is to reply to the pending D-Bus method call and
then signal the property changed signals.
> +}
> +
> +static DBusMessage *tt_get_properties(DBusConnection *conn,
> + DBusMessage *msg, void *data)
> +{
> + struct ofono_text_telephony *tt = data;
> +
> + if (tt->flags & TEXT_TELEPHONY_FLAG_CACHED)
> + return tt_get_properties_reply(msg, tt);
> +
> + if (tt->pending)
> + return __ofono_error_busy(msg);
> +
> + tt->pending = dbus_message_ref(msg);
> + tt->driver->query_tty(tt, tt_query_powered_callback, tt);
> +
> + return NULL;
> +}
> +
> +static DBusMessage *tt_set_property(DBusConnection *conn, DBusMessage *msg,
> + void *data)
> +{
> + struct ofono_text_telephony *tt = data;
> + DBusMessageIter iter;
> + DBusMessageIter var;
> + const char *property;
> +
> + if (tt->pending)
> + return __ofono_error_busy(msg);
> +
> + if (!dbus_message_iter_init(msg, &iter))
> + return __ofono_error_invalid_args(msg);
> +
> + if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
> + return __ofono_error_invalid_args(msg);
> +
> + dbus_message_iter_get_basic(&iter, &property);
> + dbus_message_iter_next(&iter);
> +
> + if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
> + return __ofono_error_invalid_args(msg);
> +
> + dbus_message_iter_recurse(&iter, &var);
> +
> + if (g_strcmp0(property, "Powered") == 0) {
> + dbus_bool_t value;
> + int target;
> +
> + if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_BOOLEAN)
> + return __ofono_error_invalid_args(msg);
> +
> + dbus_message_iter_get_basic(&var, &value);
> + target = value;
> +
> + if (tt->powered == target)
> + return dbus_message_new_method_return(msg);
> +
> + tt->pending = dbus_message_ref(msg);
> + tt->powered_pending = target;
> +
> + tt->driver->set_tty(tt, target,
> + tt_set_powered_callback, tt);
> + return NULL;
> + }
> +
> + return __ofono_error_invalid_args(msg);
> +}
<snip>
> +int ofono_text_telephony_driver_register(const struct
> ofono_text_telephony_driver *d)
> +{
> + DBG("driver: %p, name: %s", d, d->name);
> +
> + if (!d || !d->probe)
> + return -EINVAL;
> +
> + g_drivers = g_slist_prepend(g_drivers, (void *)d);
> +
> + return 0;
> +}
> +
> +void ofono_text_telephony_driver_unregister(const struct
> ofono_text_telephony_driver *d)
> +{
> + DBG("driver: %p, name: %s", d, d->name);
> +
> + if (!d)
The preferred style is to compare to NULL, e.g. if (d == NULL)
> + return;
> +
> + g_drivers = g_slist_remove(g_drivers, (void *)d);
> +}
> +
<snip>
> +struct ofono_text_telephony *ofono_text_telephony_create(struct ofono_modem
> *modem,
> + unsigned int vendor,
> + const char *driver,
> + void *data)
> +{
> + struct ofono_text_telephony *tt;
> + GSList *l;
Please add a newline here
> + if (!driver)
> + return NULL;
> +
> + tt = g_try_new0(struct ofono_text_telephony, 1);
> + if (!tt)
> + return NULL;
> +
> + tt->atom = __ofono_modem_add_atom(modem, OFONO_ATOM_TYPE_TEXT_TELEPHONY,
> + text_telephony_remove, tt);
> +
> + tt->powered = -1;
I think it is safe to default this to 0 (off).
> +
> + for (l = g_drivers; l; l = l->next) {
> + const struct ofono_text_telephony_driver *drv = l->data;
> +
> + if (g_strcmp0(drv->name, driver) != 0)
> + continue;
> +
> + if (drv->probe(tt, vendor, data) < 0)
> + continue;
> +
> + tt->driver = drv;
> + break;
> + }
> +
> + return tt;
> +}
> +
Otherwise, looks good.
Regards,
-Denis
_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
