Hi Antti,
2011/2/1 Antti Paila <[email protected]>:
> ---
> plugins/nettime.c | 326
> +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 326 insertions(+), 0 deletions(-)
> create mode 100644 plugins/nettime.c
>
> diff --git a/plugins/nettime.c b/plugins/nettime.c
> new file mode 100644
> index 0000000..894dce7
> --- /dev/null
> +++ b/plugins/nettime.c
> @@ -0,0 +1,326 @@
> +/*
> + *
> + * oFono - Open Source Telephony
> + *
> + * Copyright (C) 2008-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 <stdlib.h>
> +#include <glib.h>
> +
> +#define OFONO_API_SUBJECT_TO_CHANGE
> +#include <ofono/plugin.h>
> +#include <ofono/log.h>
> +#include <ofono/nettime.h>
> +#include <ofono/types.h>
> +#include <gdbus.h>
> +#include "ofono.h"
> +
> +#include "common.h"
> +
> +#define TIMED_PATH "/com/meego/time"
> +#define TIMED_SERVICE "com.meego.time"
s/time/timed
> +struct nt_data {
> + gboolean time_available;
> + gboolean time_pending;
> + time_t nw_time_utc;
> + time_t received;
> + int dst;
> + int time_zone;
> + const char *mcc;
> + const char *mnc;
> + unsigned int timed_watch;
> + gboolean timed_present;
> + struct ofono_netreg *netreg;
> + unsigned int netreg_st_watch;
Let's call this just netereg_watch, as there is only one watch that is used.
> +};
> +
> +static void nettime_register(struct ofono_nettime_context *);
> +
> +static gboolean encode_time_format(struct ofono_network_time *time,
> + struct tm *tm)
> +{
> +
> + tm->tm_gmtoff = time->utcoff;
> + tm->tm_isdst = time->dst;
Why are you setting these? The values you are using in struct tm are
UTC. Also, tm_isdst is a boolean, and time->dst can be any of -1, 0,
1, 2.
> + if (time->year < 0)
> + return FALSE;
> +
> + tm->tm_year = time->year - 1900;
> + tm->tm_mon = time->mon - 1;
> + tm->tm_mday = time->mday;
> + tm->tm_hour = time->hour;
> + tm->tm_min = time->min;
> + tm->tm_sec = time->sec;
> +
> + return TRUE;
> +}
> +
> +static time_t get_monotonic_time()
> +{
> + struct timespec ts;
> + clock_gettime(CLOCK_MONOTONIC, &ts);
> + return ts.tv_sec;
> +}
> +
> +static int fill_time_notification(DBusMessage *msg,
> + struct nt_data *ntd)
> +{
> + DBusMessageIter iter, iter_array;
> + int64_t utc;
> +
> + dbus_message_iter_init_append(msg, &iter);
> + dbus_message_iter_open_container(&iter,
> + DBUS_TYPE_ARRAY,
> + "{sv}",
> + &iter_array);
> +
> + if (ntd->time_pending) {
> + if (ntd->time_available) {
> + utc = ntd->nw_time_utc - ntd->received;
I would just store the ofono_network_time struct, and check if year < 0.
> + ofono_dbus_dict_append(&iter_array,
> + "UTC",
> + DBUS_TYPE_INT64,
> + &utc);
> + }
> +
> + ofono_dbus_dict_append(&iter_array,
> + "DST",
> + DBUS_TYPE_INT32,
> + &ntd->dst);
> + ofono_dbus_dict_append(&iter_array,
> + "Timezone",
> + DBUS_TYPE_INT32,
> + &ntd->time_zone);
> + }
> +
> + ofono_dbus_dict_append(&iter_array,
> + "MobileCountryCode",
> + DBUS_TYPE_STRING,
> + &ntd->mcc);
> + ofono_dbus_dict_append(&iter_array,
> + "MobileNetworkCode",
> + DBUS_TYPE_STRING,
> + &ntd->mnc);
> +
> + dbus_message_iter_close_container(&iter, &iter_array);
> + return 0;
> +}
> +
> +static DBusMessage *create_time_notification(
> + struct ofono_nettime_context *context)
> +{
> + DBusMessage *message;
> + struct nt_data *ntd = context->data;
> + const char *path = ofono_modem_get_path(context->modem);
> +
> + if (path == NULL) {
> + ofono_error("Fetching path for modem failed");
> + return NULL;
> + }
> +
> + message = dbus_message_new_method_call(TIMED_SERVICE, TIMED_PATH,
> + "com.meego.NetworkTime", "Notify");
> + if (message == NULL)
> + return NULL;
> +
> + dbus_message_set_no_reply(message, TRUE);
> + fill_time_notification(message, ntd);
> +
> + return message;
> +}
> +
> +static void init_time(struct ofono_nettime_context *context)
> +{
> + struct nt_data *nt_data = g_new0(struct nt_data, 1);
> +
> + nt_data->time_available = FALSE;
> + nt_data->time_pending = FALSE;
> + nt_data->dst = 0;
> + nt_data->time_zone = 0;
That '0' after new actually zeroes out the memory, so these are not necessary.
> + context->data = nt_data;
> +}
> +
> +static int nettime_probe(struct ofono_nettime_context *context)
> +{
> + DBG("Network Time Probe for modem: %p", context->modem);
> +
> + init_time(context);
As the function above reduces to just a g_new0() call and setting it
to context->data, just move all that here.
> + nettime_register(context);
> +
> + return 0;
> +}
> +
> +static void nettime_remove(struct ofono_nettime_context *context)
> +{
> + struct nt_data *ntd = context->data;
> +
> + DBG("Network Time Remove for modem: %p", context->modem);
> + g_dbus_remove_watch(ofono_dbus_get_connection(),
> + ntd->timed_watch);
Just to be safe, you should check that ntd->timed_watch > 0 here. I
think you should also do the same with the netreg_watch.
> + g_free(ntd);
> +}
> +
> +static void notify(int status, int lac, int ci, int tech, const char *mcc,
> + const char *mnc, void *data)
> +{
> + struct ofono_nettime_context *context = data;
> + struct nt_data *ntd = context->data;
> + DBusMessage *message;
> +
> + if (mcc == NULL || mnc == NULL)
> + return;
> +
> + ntd->mcc = mcc;
> + ntd->mnc = mnc;
You're not checking whether MCC has changed since last time.
> + if (ntd->timed_present == FALSE) {
> + DBG("Timed not present. Caching time info");
> + return;
> + }
What are you caching here?
> + message = create_time_notification(context);
> + if (message == NULL) {
> + ofono_error("Failed to create Notification message");
> + return;
> + }
> +
> + g_dbus_send_message(ofono_dbus_get_connection(), message);
> + ntd->time_pending = FALSE;
> +}
Please split the actual D-Bus message sending out to a separate
function. That way, you don't need to call this function with (0, 0,
0, 0, ... arguments all over the place, which looks weird.
> +static void nr_st_watch_destroy(void *data)
> +{
> + struct ofono_nettime_context *context = data;
> + struct nt_data *ntd = context->data;
> + DBG("");
> +
> + ntd->netreg_st_watch = 0;
> +}
Don't give a destroy callback unless you have memory to clean up.
Cheers,
Aki
_______________________________________________
ofono mailing list
[email protected]
http://lists.ofono.org/listinfo/ofono
