Hi Antti,
2011/2/14 Antti Paila <antti.pa...@nokia.com>:
> ---
> src/cell-info.c | 475
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 475 insertions(+), 0 deletions(-)
> create mode 100644 src/cell-info.c
>
> diff --git a/src/cell-info.c b/src/cell-info.c
> new file mode 100644
> index 0000000..6e70202
> --- /dev/null
> +++ b/src/cell-info.c
> @@ -0,0 +1,475 @@
> +/*
> + *
> + * oFono - Open Source Telephony
> + *
> + * Copyright (C) 2009-2010 Nokia Corporation and/or its subsidiary(-ies).
> + *
> + * 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 <errno.h>
> +
> +#include <glib.h>
> +#include <gdbus.h>
> +
> +#include "ofono.h"
> +
> +#include "common.h"
> +#include "ofono/cell-info.h"
This doesn't belong here; you need to add an include statement for
cell-info.h into src/ofono.h.
> +
> +
Extra empty line here.
> +struct ofono_cell_info {
> + DBusMessage *pending;
> + struct ofono_atom *atom;
> + const struct ofono_cell_info_driver *driver;
> + void *driver_data;
> +};
> +
> +
Extra empty line here.
> +static DBusMessage *ci_get_cells(DBusConnection *, DBusMessage *, void *);
Can this forward declaration be avoided?
> +
> +static GSList *g_drivers = NULL;
> +
> +static GDBusMethodTable ci_methods[] = {
> + { "AquireMeasurement", "", "aa{sv}", ci_get_cells,
> + G_DBUS_METHOD_FLAG_ASYNC },
> + { }
> +};
> +
> +static GDBusSignalTable ci_signals[] = {
> + { }
> +};
> +
> +int ofono_cell_info_driver_register(struct ofono_cell_info_driver *driver)
> +{
> + DBG("driver: %p, name: %s", driver, driver->name);
> +
> + if (driver->probe == NULL)
> + return -EINVAL;
> +
> + g_drivers = g_slist_prepend(g_drivers, (void *) driver);
> +
> + return 0;
> +}
> +
> +void ofono_cell_info_driver_unregister(struct ofono_cell_info_driver *driver)
> +{
> + DBG("driver: %p, name: %s", driver, driver->name);
> +
> + g_drivers = g_slist_remove(g_drivers, (void *) driver);
> +}
> +
> +void ofono_cell_info_remove(struct ofono_cell_info *ci)
> +{
> + __ofono_atom_free(ci->atom);
> +}
> +
> +static void cell_info_unregister(struct ofono_atom *atom)
> +{
> + struct ofono_cell_info *ci = __ofono_atom_get_data(atom);
> + const char *path = __ofono_atom_get_path(ci->atom);
> + DBusConnection *conn = ofono_dbus_get_connection();
> + struct ofono_modem *modem = __ofono_atom_get_modem(ci->atom);
> +
> + ofono_modem_remove_interface(modem, OFONO_CELL_INFO_INTERFACE);
> +
> + if (!g_dbus_unregister_interface(conn, path,
> OFONO_CELL_INFO_INTERFACE))
> + ofono_error("Failed to unregister interface %s",
> + OFONO_CELL_INFO_INTERFACE);
> +}
> +
> +static void cell_info_remove(struct ofono_atom *atom)
> +{
> + struct ofono_cell_info *ci = __ofono_atom_get_data(atom);
> + DBG("atom: %p", atom);
> +
> + if (ci == NULL)
> + return;
> +
> + if (ci->driver && ci->driver->remove)
> + ci->driver->remove(ci);
> +
> + g_free(ci);
> +}
> +
> +void ofono_cell_info_register(struct ofono_cell_info *ci)
> +{
> + DBusConnection *conn = ofono_dbus_get_connection();
> + struct ofono_modem *modem = __ofono_atom_get_modem(ci->atom);
> + const char *path = __ofono_atom_get_path(ci->atom);
> +
> + DBG("Modem: %p", modem);
> +
> + if (!g_dbus_register_interface(conn, path,
> + OFONO_CELL_INFO_INTERFACE,
> + ci_methods, ci_signals, NULL,
> + ci, NULL)) {
> + ofono_error("Could not create %s interface",
> + OFONO_CELL_INFO_INTERFACE);
One too many tabs here.
> +
> + return;
> + }
> +
> + ofono_modem_add_interface(modem, OFONO_CELL_INFO_INTERFACE);
> +
> + __ofono_atom_register(ci->atom, cell_info_unregister);
> +}
> +
> +struct ofono_cell_info *ofono_cell_info_create(struct ofono_modem *modem,
> + unsigned int vendor,
> + const char *driver,
> + void *data)
> +{
Coding style, M4.
> + struct ofono_cell_info *ci;
> + GSList *l;
> +
> + if (driver == NULL)
> + return NULL;
> +
> + ci = g_try_new0(struct ofono_cell_info, 1);
> + if (ci == NULL)
> + return NULL;
> +
> + ci->atom = __ofono_modem_add_atom(modem,
> + OFONO_ATOM_TYPE_CELL_INFO,
> + cell_info_remove, ci);
No need to wrap the line here after modem; the next argument will
still keep it under 80 chars wide.
> + for (l = g_drivers; l; l = l->next) {
> + const struct ofono_cell_info_driver *drv = l->data;
> +
> + if (g_strcmp0(drv->name, driver))
> + continue;
> +
> + if (drv->probe(ci, vendor, data) < 0)
> + continue;
> +
> + ci->driver = drv;
> + break;
> + }
> +
> + return ci;
> +}
> +
> +void *ofono_cell_info_get_data(struct ofono_cell_info *ci)
> +{
> + return ci->driver_data;
> +}
> +
> +void ofono_cell_info_set_data(struct ofono_cell_info *ci, void *cid)
> +{
> + ci->driver_data = cid;
> +}
> +
> +static int append_geran_meta_data(DBusMessageIter *iter,
> + struct ofono_cell_info_results *ci)
Coding style, M4.
> +{
> + DBusMessageIter iter_array;
> + const char *type = "GERAN";
> + const char *mcc = ci->mcc;
> + const char *mnc = ci->mnc;
> +
> + dbus_message_iter_open_container(iter,
> + DBUS_TYPE_ARRAY,
> + "{sv}",
> + &iter_array);
> +
> + ofono_dbus_dict_append(&iter_array,
> + "Type",
> + DBUS_TYPE_STRING,
> + &type);
> + ofono_dbus_dict_append(&iter_array,
> + "MobileNetworkCode",
> + DBUS_TYPE_STRING,
> + &mnc);
> + ofono_dbus_dict_append(&iter_array,
> + "MobileCountryCode",
> + DBUS_TYPE_STRING,
> + &mcc);
> + ofono_dbus_dict_append(&iter_array,
> + "LocationAreaCode",
> + DBUS_TYPE_UINT16,
> + &ci->geran.lac);
> + ofono_dbus_dict_append(&iter_array,
> + "CellId",
> + DBUS_TYPE_UINT16,
> + &ci->geran.ci);
> +
> + if (ci->geran.ta != OFONO_CI_FIELD_TA_UNDEFINED)
> + ofono_dbus_dict_append(&iter_array,
> + "TimingAdvance",
> + DBUS_TYPE_BYTE,
> + &ci->geran.ta);
> +
I think you're being way too aggressive with line wrapping here, just
fill up as much as 80 chars as you can.
> + dbus_message_iter_close_container(iter, &iter_array);
> +
> + return 0;
> +
Extra empty line here.
> +}
> +
> +static void add_geran_neighbor(DBusMessageIter *iter,
> + struct geran_neigh_cell *cell)
Coding style, M4.
> +{
> + DBusMessageIter iter_array;
> +
> + dbus_message_iter_open_container(iter,
> + DBUS_TYPE_ARRAY,
> + "{sv}",
> + &iter_array);
> + ofono_dbus_dict_append(&iter_array,
> + "AbsoluteRadioFrequencyChannelNumber",
> + DBUS_TYPE_UINT16,
> + &cell->arfcn);
> + ofono_dbus_dict_append(&iter_array,
> + "BaseStationIdentityCode",
> + DBUS_TYPE_BYTE,
> + &cell->bsic);
> + ofono_dbus_dict_append(&iter_array,
> + "RXLEV",
> + DBUS_TYPE_BYTE,
> + &cell->rxlev);
> +
Same as above, too aggressive line wraps.
> + dbus_message_iter_close_container(iter, &iter_array);
> +}
> +
> +static int fill_geran_ci(DBusMessageIter *iter,
> + struct ofono_cell_info_results *ci)
Coding style, M4.
> +{
> + int i;
> +
> + append_geran_meta_data(iter, ci);
> +
> + for (i = 0; i < ci->geran.no_cells; ++i)
> + add_geran_neighbor(iter, &ci->geran.nmr[i]);
> +
> + return 0;
> +}
> +
> +static int append_utra_neigh_cell_data(DBusMessageIter *iter,
> + struct cell_measured_results *cmr)
Coding style, M4.
> +{
> + ofono_dbus_dict_append(iter,
> + "ScramblingCode",
> + DBUS_TYPE_UINT16,
> + &cmr->sc);
> +
> + if (cmr->ucid != OFONO_CI_FIELD_UCID_UNDEFINED)
> + ofono_dbus_dict_append(iter,
> + "UniqueCellId",
> + DBUS_TYPE_UINT32,
> + &cmr->ucid);
> +
> + if (cmr->ecn0 != OFONO_CI_FIELD_ECN0_UNDEFINED)
> + ofono_dbus_dict_append(iter,
> + "CPICH-ECN0",
> + DBUS_TYPE_BYTE,
> + &cmr->ecn0);
> +
> + if (cmr->rscp != OFONO_CI_FIELD_RSCP_UNDEFINED)
> + ofono_dbus_dict_append(iter,
> + "CPICH-RSCP",
> + DBUS_TYPE_INT16,
> + &cmr->rscp);
> +
> + if (cmr->pathloss != OFONO_CI_FIELD_PATHLOSS_UNDEFINED)
> + ofono_dbus_dict_append(iter,
> + "Pathloss",
> + DBUS_TYPE_BYTE,
> + &cmr->pathloss);
> +
Same as above about line wrapping. Also, these block's indentation is
one level too deep.
> + return 0;
> +}
> +
> +static int append_utra_neigh_freq_data(DBusMessageIter *iter,
> + struct measured_results_list *mrl)
Coding style, M4.
> +{
> + ofono_dbus_dict_append(iter,
> + "ReceivedSignalStrengthIndicator",
> + DBUS_TYPE_BYTE,
> + &mrl->rssi);
> +
> + ofono_dbus_dict_append(iter,
> + "UARFCN-DL",
> + DBUS_TYPE_UINT16,
> + &mrl->dl_freq);
> +
> + if (mrl->ul_freq != OFONO_CI_FIELD_FREQ_UNDEFINED)
> + ofono_dbus_dict_append(iter,
> + "UARFCN-UL",
> + DBUS_TYPE_UINT16,
> + &mrl->ul_freq);
> +
Same as above.
> + return 0;
> +}
> +
> +static int append_utra_neighbors(DBusMessageIter *iter,
> + struct utran *utran)
> +{
> + DBusMessageIter iter_array;
> + int i, j;
> +
> +
Extra empty line.
> + for (i = 0; i < utran->no_freq; ++i) {
> +
> + for (j = 0; j < utran->mrl[i].no_cells; ++j) {
> + dbus_message_iter_open_container(iter,
> + DBUS_TYPE_ARRAY,
> + "{sv}",
> + &iter_array);
> + append_utra_neigh_freq_data(&iter_array,
> + &utran->mrl[i]);
> + append_utra_neigh_cell_data(&iter_array,
> + &utran->mrl[i].cmr[j]);
> +
> + dbus_message_iter_close_container(iter,
> + &iter_array);
Same as above about line wraps.
> + }
> + }
> +
> + return 0;
> +}
> +
> +static void append_utran_meta_data(DBusMessageIter *iter,
> + struct ofono_cell_info_results *ci)
Coding style, M4.
> +{
> + DBusMessageIter iter_array;
> + const char *type = "UTRA-FDD";
> + const char *mcc = ci->mcc;
> + const char *mnc = ci->mnc;
> +
> + dbus_message_iter_open_container(iter,
> + DBUS_TYPE_ARRAY,
> + "{sv}",
> + &iter_array);
> +
> + ofono_dbus_dict_append(&iter_array,
> + "Type",
> + DBUS_TYPE_STRING,
> + &type);
> + ofono_dbus_dict_append(&iter_array,
> + "MobileNetworkCode",
> + DBUS_TYPE_STRING,
> + &mnc);
> + ofono_dbus_dict_append(&iter_array,
> + "MobileCountryCode",
> + DBUS_TYPE_STRING,
> + &mcc);
> + ofono_dbus_dict_append(&iter_array,
> + "UniqueCellId",
> + DBUS_TYPE_UINT32,
> + &ci->utran.ucid);
> + ofono_dbus_dict_append(&iter_array,
> + "ScramblingCode",
> + DBUS_TYPE_UINT16,
> + &ci->utran.sc);
> + ofono_dbus_dict_append(&iter_array,
> + "UARFCN-DL",
> + DBUS_TYPE_UINT16,
> + &ci->utran.dl_freq);
> +
> + if (ci->utran.ul_freq != OFONO_CI_FIELD_FREQ_UNDEFINED) {
> + ofono_dbus_dict_append(&iter_array,
> + "UARFCN-UL",
> + DBUS_TYPE_UINT16,
> + &ci->utran.ul_freq);
> + }
> +
And line wrapping, as above.
> + dbus_message_iter_close_container(iter, &iter_array);
> +}
> +
> +static int fill_utra_ci(DBusMessageIter *iter,
> + struct ofono_cell_info_results *ci)
> +{
> +
Extra empty line.
> + append_utran_meta_data(iter, ci);
> + append_utra_neighbors(iter, &ci->utran);
> +
> + return 0;
> +}
> +
> +static void ofono_neigh_cell_info_query_cb(const struct ofono_error *error,
> + struct ofono_cell_info_results *ci_results, void *data)
> +{
Coding style, M4. Not completely applicable here, but you could still
fit at least 4 tabs there.
> + struct ofono_cell_info *ci = data;
> + DBusMessage *msg = ci->pending;
> + DBusMessage *reply;
> + DBusMessageIter iter, iter_array;
> + int status;
> +
> + DBG("");
> +
> + if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> + ofono_error("Neighbor cell info query failed");
> + goto error;
> + }
> +
> + reply = dbus_message_new_method_return(msg);
> + if (reply == NULL) {
> + ofono_error("Failed to create response");
> + goto error;
> + }
> +
> + dbus_message_iter_init_append(reply, &iter);
> + dbus_message_iter_open_container(&iter,
> + DBUS_TYPE_ARRAY,
> + "a{sv}",
> + &iter_array);
> +
> + if (ci_results->has_geran_cells == TRUE) {
No need to explicitly call out a boolean.
> + DBG("Geran cell info received");
> +
> + status = fill_geran_ci(&iter_array, ci_results);
> + if (status != 0) {
> + ofono_error("Failed to fill geran info");
> + goto error;
> + }
> +
> + }
> +
> + if (ci_results->has_utran_cells) {
> + DBG("Utran cell info received");
> +
> + status = fill_utra_ci(&iter_array, ci_results);
> + if (status != 0) {
> + ofono_error("Failed to fill utran info");
> + goto error;
> + }
> +
> + }
Add empty line here.
> + dbus_message_iter_close_container(&iter, &iter_array);
> + __ofono_dbus_pending_reply(&msg, reply);
> + return;
> +
> +error:
> + reply = __ofono_error_failed(msg);
> + __ofono_dbus_pending_reply(&msg, reply);
> + return;
> +
Extra empty line here.
> +}
> +
> +static DBusMessage *ci_get_cells(DBusConnection *conn, DBusMessage *msg,
> + void *data)
> +{
> + struct ofono_cell_info *ci = data;
> + DBG("");
> +
> + ci->pending = dbus_message_ref(msg);
> + ci->driver->query(ci, ofono_neigh_cell_info_query_cb,
> ci->driver_data);
> +
> + return NULL;
> +}
Cheers,
Aki
_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
