* [PATCH 0/4 v4] Network Time Plugin @ 2011-02-01 14:49 Antti Paila 2011-02-01 14:49 ` [PATCH 1/4] nettime: Network time plugin implementation Antti Paila ` (3 more replies) 0 siblings, 4 replies; 12+ messages in thread From: Antti Paila @ 2011-02-01 14:49 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 966 bytes --] This series of patches introduces the network time part of the NITZ feature as outlined in 3GPP spec 22.042. The plugin is for delivering network indicated time information to timed process which is responsible for maintaining the system time. The delivery is achieved by timed implementing an interface with a method that is called by the nettime plugin with time related info as a parameter of the method. Antti Paila (4): nettime: Network time plugin implementation nettime: Makefile.am modification nettime: Documentation nettime: Mock Timed for testing Makefile.am | 9 +- doc/network-time-api.txt | 36 +++++ plugins/nettime.c | 326 ++++++++++++++++++++++++++++++++++++++++++++++ test/test-nettime | 35 +++++ 4 files changed, 404 insertions(+), 2 deletions(-) create mode 100644 doc/network-time-api.txt create mode 100644 plugins/nettime.c create mode 100755 test/test-nettime ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/4] nettime: Network time plugin implementation 2011-02-01 14:49 [PATCH 0/4 v4] Network Time Plugin Antti Paila @ 2011-02-01 14:49 ` Antti Paila 2011-02-03 15:17 ` Aki Niemi 2011-02-07 18:58 ` Marcel Holtmann 2011-02-01 14:49 ` [PATCH 2/4] nettime: Makefile.am modification Antti Paila ` (2 subsequent siblings) 3 siblings, 2 replies; 12+ messages in thread From: Antti Paila @ 2011-02-01 14:49 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 8432 bytes --] --- 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" + +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; + +}; + +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; + + 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; + 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; + + 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); + 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); + 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; + + if (ntd->timed_present == FALSE) { + DBG("Timed not present. Caching time info"); + return; + } + + 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; +} + +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; +} + +static void nettime_info_received(struct ofono_nettime_context *context, + struct ofono_network_time *info) +{ + struct tm t; + struct nt_data *ntd = context->data; + const char *mcc, *mnc; + + DBG("Network time notification received, modem: %p", + context->modem); + + if (info == NULL) + return; + + ntd->received = get_monotonic_time(); + ntd->time_pending = TRUE; + + ntd->time_available = encode_time_format(info, &t); + if (ntd->time_available == TRUE) + ntd->nw_time_utc = timegm(&t); + + ntd->dst = info->dst; + ntd->time_zone = info->utcoff; + + ntd->netreg = __ofono_atom_get_data(__ofono_modem_find_atom( + context->modem, OFONO_ATOM_TYPE_NETREG)); + + mcc = ofono_netreg_get_mcc(ntd->netreg); + mnc = ofono_netreg_get_mnc(ntd->netreg); + if ((mcc == NULL) || (mnc == NULL)) { + DBG("Mobile country/network code missing"); + + if (ntd->netreg_st_watch != 0) + return; + + ntd->netreg_st_watch = __ofono_netreg_add_status_watch( + ntd->netreg, notify, + context, nr_st_watch_destroy); + } else { + notify(0, 0, 0, 0, mcc, mnc, context); + } + +} + +static struct ofono_nettime_driver nettime_driver = { + .name = "Network Time", + .probe = nettime_probe, + .remove = nettime_remove, + .info_received = nettime_info_received, +}; + +static int nettime_init(void) +{ + return ofono_nettime_driver_register(&nettime_driver); +} + +static void nettime_exit(void) +{ + ofono_nettime_driver_unregister(&nettime_driver); +} + +static void timed_connect(DBusConnection *connection, void *user_data) +{ + struct ofono_nettime_context *context = user_data; + struct nt_data *ntd = context->data; + const char *mcc, *mnc; + + DBG(""); + + ntd->timed_present = TRUE; + mcc = ofono_netreg_get_mcc(ntd->netreg); + mnc = ofono_netreg_get_mnc(ntd->netreg); + + notify(0, 0, 0, 0, mcc, mnc, context); +} + +static void timed_disconnect(DBusConnection *connection, void *user_data) +{ + struct ofono_nettime_context *context = user_data; + struct nt_data *ntd = context->data; + + DBG(""); + + ntd->timed_present = FALSE; +} + +static void nettime_register(struct ofono_nettime_context *context) +{ + DBusConnection *conn; + struct nt_data *ntd = context->data; + + DBG("Registering Network time for modem %s" , + ofono_modem_get_path(context->modem)); + + conn = ofono_dbus_get_connection(); + + ntd->timed_watch = g_dbus_add_service_watch(conn, TIMED_SERVICE, + timed_connect, timed_disconnect, + context, NULL); +} + +OFONO_PLUGIN_DEFINE(nettime, "Network Time Plugin", + VERSION, OFONO_PLUGIN_PRIORITY_DEFAULT, + nettime_init, nettime_exit) + -- 1.7.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] nettime: Network time plugin implementation 2011-02-01 14:49 ` [PATCH 1/4] nettime: Network time plugin implementation Antti Paila @ 2011-02-03 15:17 ` Aki Niemi 2011-02-07 18:58 ` Marcel Holtmann 1 sibling, 0 replies; 12+ messages in thread From: Aki Niemi @ 2011-02-03 15:17 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 9366 bytes --] Hi Antti, 2011/2/1 Antti Paila <antti.paila@nokia.com>: > --- > 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] nettime: Network time plugin implementation 2011-02-01 14:49 ` [PATCH 1/4] nettime: Network time plugin implementation Antti Paila 2011-02-03 15:17 ` Aki Niemi @ 2011-02-07 18:58 ` Marcel Holtmann 2011-02-08 12:25 ` Antti Paila 1 sibling, 1 reply; 12+ messages in thread From: Marcel Holtmann @ 2011-02-07 18:58 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 9078 bytes --] Hi Antti, > plugins/nettime.c | 326 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 326 insertions(+), 0 deletions(-) > create mode 100644 plugins/nettime.c I would prefer if we call this nokia-timed.c or in case this actually becomes default part of MeeGo, them maybe meego-timed.c. I would be also fine with {nokia,meego}-nettime.c. Just calling it nettime.c is too generic. It needs to be clear what this is for. > +#define TIMED_PATH "/com/meego/time" > +#define TIMED_SERVICE "com.meego.time" So the intention is to convert the Nokia timed into a MeeGo specific daemon? I would have expected to see com.nokia.timed here. > +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; Why do you bother with these here. You might better just reference the netreg atom. The memory is only valid if netreg atom is present. > + unsigned int timed_watch; > + gboolean timed_present; > + struct ofono_netreg *netreg; > + unsigned int netreg_st_watch; > + > +}; > + > +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; > + > + 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; > + 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; > + > + 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); Please just don't bother with putting this in separate function. It only has one caller and I prefer the context->data allocation being in the probe() callback directly. > + nettime_register(context); Same for this one. Also I don't see the point of the forward declaration. > +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); > + g_free(ntd); > +} You can avoid certain checks here, but then you need to have clear error handling in probe() callback. > +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; > + > + if (ntd->timed_present == FALSE) { > + DBG("Timed not present. Caching time info"); > + return; > + } > + > + 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; > +} > + > +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; > +} > + > +static void nettime_info_received(struct ofono_nettime_context *context, > + struct ofono_network_time *info) > +{ > + struct tm t; > + struct nt_data *ntd = context->data; > + const char *mcc, *mnc; > + > + DBG("Network time notification received, modem: %p", > + context->modem); > + > + if (info == NULL) > + return; > + > + ntd->received = get_monotonic_time(); > + ntd->time_pending = TRUE; > + > + ntd->time_available = encode_time_format(info, &t); > + if (ntd->time_available == TRUE) > + ntd->nw_time_utc = timegm(&t); > + > + ntd->dst = info->dst; > + ntd->time_zone = info->utcoff; > + > + ntd->netreg = __ofono_atom_get_data(__ofono_modem_find_atom( > + context->modem, OFONO_ATOM_TYPE_NETREG)); > + > + mcc = ofono_netreg_get_mcc(ntd->netreg); > + mnc = ofono_netreg_get_mnc(ntd->netreg); > + if ((mcc == NULL) || (mnc == NULL)) { > + DBG("Mobile country/network code missing"); > + > + if (ntd->netreg_st_watch != 0) > + return; > + > + ntd->netreg_st_watch = __ofono_netreg_add_status_watch( > + ntd->netreg, notify, > + context, nr_st_watch_destroy); > + } else { > + notify(0, 0, 0, 0, mcc, mnc, context); > + } > + > +} > + > +static struct ofono_nettime_driver nettime_driver = { > + .name = "Network Time", > + .probe = nettime_probe, > + .remove = nettime_remove, > + .info_received = nettime_info_received, > +}; > + > +static int nettime_init(void) > +{ > + return ofono_nettime_driver_register(&nettime_driver); > +} > + > +static void nettime_exit(void) > +{ > + ofono_nettime_driver_unregister(&nettime_driver); > +} So in general the plugin init/exit functions should be last in the source code. Just above the OFONO_PLUGIN_DEFINE. That way it is consistent and a lot easier to read. Also the generic nettime_* namespacing might be better done as nokia_timed_* or just short timed_*. > +static void timed_connect(DBusConnection *connection, void *user_data) > +{ > + struct ofono_nettime_context *context = user_data; > + struct nt_data *ntd = context->data; > + const char *mcc, *mnc; > + > + DBG(""); > + > + ntd->timed_present = TRUE; > + mcc = ofono_netreg_get_mcc(ntd->netreg); > + mnc = ofono_netreg_get_mnc(ntd->netreg); > + > + notify(0, 0, 0, 0, mcc, mnc, context); > +} > + > +static void timed_disconnect(DBusConnection *connection, void *user_data) > +{ > + struct ofono_nettime_context *context = user_data; > + struct nt_data *ntd = context->data; > + > + DBG(""); > + > + ntd->timed_present = FALSE; > +} > + > +static void nettime_register(struct ofono_nettime_context *context) > +{ > + DBusConnection *conn; > + struct nt_data *ntd = context->data; > + > + DBG("Registering Network time for modem %s" , > + ofono_modem_get_path(context->modem)); > + > + conn = ofono_dbus_get_connection(); > + > + ntd->timed_watch = g_dbus_add_service_watch(conn, TIMED_SERVICE, > + timed_connect, timed_disconnect, > + context, NULL); > +} Please reorder the functions a bit better. That makes the whole code more readable and gives us a lot more benefit in the long term. I only wanna see forward declaration if they are 100% unavoidable. > + > +OFONO_PLUGIN_DEFINE(nettime, "Network Time Plugin", > + VERSION, OFONO_PLUGIN_PRIORITY_DEFAULT, > + nettime_init, nettime_exit) > + Regards Marcel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] nettime: Network time plugin implementation 2011-02-07 18:58 ` Marcel Holtmann @ 2011-02-08 12:25 ` Antti Paila 2011-02-08 15:46 ` Marcel Holtmann 0 siblings, 1 reply; 12+ messages in thread From: Antti Paila @ 2011-02-08 12:25 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 1558 bytes --] Hi Marcel, On Mon, 2011-02-07 at 10:58 -0800, ext Marcel Holtmann wrote: > Hi Antti, > > > plugins/nettime.c | 326 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 files changed, 326 insertions(+), 0 deletions(-) > > create mode 100644 plugins/nettime.c > > I would prefer if we call this nokia-timed.c or in case this actually > becomes default part of MeeGo, them maybe meego-timed.c. I would be also > fine with {nokia,meego}-nettime.c. > > Just calling it nettime.c is too generic. It needs to be clear what this > is for. > > > +#define TIMED_PATH "/com/meego/time" > > +#define TIMED_SERVICE "com.meego.time" > > So the intention is to convert the Nokia timed into a MeeGo specific > daemon? I would have expected to see com.nokia.timed here. My understanding is that this will be Meego specific plugin since timed is part of Meego. Hence, the service 'com.meego.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; > > Why do you bother with these here. You might better just reference the > netreg atom. The memory is only valid if netreg atom is present. Timed expects to receive the mcc and mnc in the time notification. However, if the mcc and mnc don't change we don't resend the information. These fields contain the mnc and mcc that oFono sent in the previous time notification. Best Regards, Antti ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] nettime: Network time plugin implementation 2011-02-08 12:25 ` Antti Paila @ 2011-02-08 15:46 ` Marcel Holtmann 0 siblings, 0 replies; 12+ messages in thread From: Marcel Holtmann @ 2011-02-08 15:46 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 1796 bytes --] Hi Antti, > > > plugins/nettime.c | 326 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > > > 1 files changed, 326 insertions(+), 0 deletions(-) > > > create mode 100644 plugins/nettime.c > > > > I would prefer if we call this nokia-timed.c or in case this actually > > becomes default part of MeeGo, them maybe meego-timed.c. I would be also > > fine with {nokia,meego}-nettime.c. > > > > Just calling it nettime.c is too generic. It needs to be clear what this > > is for. > > > > > +#define TIMED_PATH "/com/meego/time" > > > +#define TIMED_SERVICE "com.meego.time" > > > > So the intention is to convert the Nokia timed into a MeeGo specific > > daemon? I would have expected to see com.nokia.timed here. > > My understanding is that this will be Meego specific plugin since timed > is part of Meego. Hence, the service 'com.meego.timed'. this is fine with me, but so far the timed.git tree has nothing alike this mentioned. It currently still uses com.nokia.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; > > > > Why do you bother with these here. You might better just reference the > > netreg atom. The memory is only valid if netreg atom is present. > > Timed expects to receive the mcc and mnc in the time notification. > However, if the mcc and mnc don't change we don't resend the > information. These fields contain the mnc and mcc that oFono sent > in the previous time notification. But this const pointer points to memory inside netreg. It will never change. It always points to the same address. Regards Marcel ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/4] nettime: Makefile.am modification 2011-02-01 14:49 [PATCH 0/4 v4] Network Time Plugin Antti Paila 2011-02-01 14:49 ` [PATCH 1/4] nettime: Network time plugin implementation Antti Paila @ 2011-02-01 14:49 ` Antti Paila 2011-02-01 14:49 ` [PATCH 3/4] nettime: Documentation Antti Paila 2011-02-01 14:49 ` [PATCH 4/4] nettime: Mock Timed for testing Antti Paila 3 siblings, 0 replies; 12+ messages in thread From: Antti Paila @ 2011-02-01 14:49 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 1147 bytes --] --- Makefile.am | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Makefile.am b/Makefile.am index a38fcb9..2d54025 100644 --- a/Makefile.am +++ b/Makefile.am @@ -337,6 +337,9 @@ builtin_modules += example_provision builtin_sources += examples/provision.c endif +builtin_modules += nettime +builtin_sources += plugins/nettime.c + builtin_modules += smart_messaging builtin_sources += plugins/smart-messaging.c @@ -404,7 +407,8 @@ doc_files = doc/overview.txt doc/ofono-paper.txt doc/release-faq.txt \ doc/phonebook-api.txt doc/radio-settings-api.txt \ doc/sim-api.txt doc/stk-api.txt \ doc/audio-settings-api.txt doc/text-telephony-api.txt \ - doc/calypso-modem.txt doc/message-api.txt + doc/calypso-modem.txt doc/message-api.txt \ + doc/network-time-api.txt test_scripts = test/backtrace \ @@ -478,7 +482,8 @@ test_scripts = test/backtrace \ test/cdma-dial-number \ test/cdma-hangup \ test/disable-call-forwarding \ - test/list-messages + test/list-messages \ + test/test-nettime if TEST testdir = $(pkglibdir)/test -- 1.7.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/4] nettime: Documentation 2011-02-01 14:49 [PATCH 0/4 v4] Network Time Plugin Antti Paila 2011-02-01 14:49 ` [PATCH 1/4] nettime: Network time plugin implementation Antti Paila 2011-02-01 14:49 ` [PATCH 2/4] nettime: Makefile.am modification Antti Paila @ 2011-02-01 14:49 ` Antti Paila 2011-02-01 14:54 ` Jeevaka.Badrappan 2011-02-07 19:00 ` Marcel Holtmann 2011-02-01 14:49 ` [PATCH 4/4] nettime: Mock Timed for testing Antti Paila 3 siblings, 2 replies; 12+ messages in thread From: Antti Paila @ 2011-02-01 14:49 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 1332 bytes --] --- doc/network-time-api.txt | 36 ++++++++++++++++++++++++++++++++++++ 1 files changed, 36 insertions(+), 0 deletions(-) create mode 100644 doc/network-time-api.txt diff --git a/doc/network-time-api.txt b/doc/network-time-api.txt new file mode 100644 index 0000000..9133a73 --- /dev/null +++ b/doc/network-time-api.txt @@ -0,0 +1,36 @@ +Network time hierarchy +====================== + +Interface com.meego.NetworkTime +Object path [variable] + +Methods void Notify(dict info) + + Notifies the service of current time and date + as notified by the cellular network. The info + argument contains a dictionary with the + following possible keys: + + int64 UTC [optional] + Network time in seconds from epoch + normalized to device boot time. + Reveicing entity obtains current real + time by adding the value from monotonic + clock e.g. + clock_gettime(CLOCK_MONOTONIC,...). + + int32 Timezone [optional] + Current timezone offset in seconds from + UTC. + + int32 DST [optional] + Current daylight saving setting in + hours. + + string MobileCountryCode + The Mobile country code of the + current network operator. + + string MobileNetworkCode + The Mobile network code of the + current network operator. -- 1.7.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* RE: [PATCH 3/4] nettime: Documentation 2011-02-01 14:49 ` [PATCH 3/4] nettime: Documentation Antti Paila @ 2011-02-01 14:54 ` Jeevaka.Badrappan 2011-02-01 19:58 ` Aki Niemi 2011-02-07 19:00 ` Marcel Holtmann 1 sibling, 1 reply; 12+ messages in thread From: Jeevaka.Badrappan @ 2011-02-01 14:54 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 1416 bytes --] Hi Antti, ofono-bounces(a)ofono.org wrote: > --- > doc/network-time-api.txt | 36 ++++++++++++++++++++++++++++++++++++ > 1 files changed, 36 insertions(+), 0 deletions(-) create > mode 100644 doc/network-time-api.txt > > diff --git a/doc/network-time-api.txt > b/doc/network-time-api.txt new file mode 100644 index > 0000000..9133a73 --- /dev/null +++ b/doc/network-time-api.txt > @@ -0,0 +1,36 @@ > +Network time hierarchy > +====================== > + > +Interface com.meego.NetworkTime > +Object path [variable] > + > +Methods void Notify(dict info) > + > + Notifies the service of current time and date > + as notified by the cellular network. The info > + argument contains a dictionary with the > + following possible keys: > + > + int64 UTC [optional] > + Network time in seconds from epoch > + normalized to device boot time. > + Reveicing entity obtains current real > + time by adding the value from monotonic > + clock e.g. > + clock_gettime(CLOCK_MONOTONIC,...). > + > + int32 Timezone [optional] > + Current timezone offset in seconds from > + UTC. > + > + int32 DST [optional] > + Current daylight saving setting in > + hours. few hours back I delivered a patch for converting the DST from hours to seconds in ofono driver code. are we going to take that into consideration? Aki?? Regards, Jeevaka ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 3/4] nettime: Documentation 2011-02-01 14:54 ` Jeevaka.Badrappan @ 2011-02-01 19:58 ` Aki Niemi 0 siblings, 0 replies; 12+ messages in thread From: Aki Niemi @ 2011-02-01 19:58 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 529 bytes --] Hi Jeevaka, On Tue, 2011-02-01 at 16:54 +0200, ext Jeevaka.Badrappan(a)elektrobit.com wrote: > > + int32 DST [optional] > > + Current daylight saving setting in > > + hours. > > few hours back I delivered a patch for converting the DST from hours to > seconds in ofono driver code. > are we going to take that into consideration? Ah, well. Might as well then change the dst to be hours. Would you mind resubmitting those patches, plus one that fixes struct ofono_network_time as well? Cheers, Aki ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] nettime: Documentation 2011-02-01 14:49 ` [PATCH 3/4] nettime: Documentation Antti Paila 2011-02-01 14:54 ` Jeevaka.Badrappan @ 2011-02-07 19:00 ` Marcel Holtmann 1 sibling, 0 replies; 12+ messages in thread From: Marcel Holtmann @ 2011-02-07 19:00 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 769 bytes --] Hi Antti, > doc/network-time-api.txt | 36 ++++++++++++++++++++++++++++++++++++ > 1 files changed, 36 insertions(+), 0 deletions(-) > create mode 100644 doc/network-time-api.txt > > diff --git a/doc/network-time-api.txt b/doc/network-time-api.txt > new file mode 100644 > index 0000000..9133a73 > --- /dev/null > +++ b/doc/network-time-api.txt > @@ -0,0 +1,36 @@ > +Network time hierarchy > +====================== > + > +Interface com.meego.NetworkTime > +Object path [variable] the object path is not really variable. However this part of the documentation should be really inside timed. Since timed is exposing this interface in the first place. And we are moving from com.nokia.timed towards com.meego.timed? Regards Marcel ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/4] nettime: Mock Timed for testing 2011-02-01 14:49 [PATCH 0/4 v4] Network Time Plugin Antti Paila ` (2 preceding siblings ...) 2011-02-01 14:49 ` [PATCH 3/4] nettime: Documentation Antti Paila @ 2011-02-01 14:49 ` Antti Paila 3 siblings, 0 replies; 12+ messages in thread From: Antti Paila @ 2011-02-01 14:49 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 1238 bytes --] --- test/test-nettime | 35 +++++++++++++++++++++++++++++++++++ 1 files changed, 35 insertions(+), 0 deletions(-) create mode 100755 test/test-nettime diff --git a/test/test-nettime b/test/test-nettime new file mode 100755 index 0000000..908a5db --- /dev/null +++ b/test/test-nettime @@ -0,0 +1,35 @@ +#!/usr/bin/python + +import gobject +import dbus +import sys +import time +import dbus.service +import dbus.mainloop.glib + + +class NetworkTime(dbus.service.Object): + def __init__(self): + busName = dbus.service.BusName('com.meego.time', + bus = dbus.SystemBus()) + dbus.service.Object.__init__(self, busName, '/com/meego/time') + @dbus.service.method(dbus_interface="com.meego.NetworkTime", + in_signature="a{sv}", out_signature="") + def Notify(self, arg): + print arg + print + print "Time from mobile: %d" % arg["UTC"] + print "DST: %d" % arg["DST"] + print "Timezone: %d" % arg["Timezone"] + print "MNC: %s" % arg["MobileNetworkCode"] + print "MCC: %s" % arg["MobileCountryCode"] + +if __name__ == '__main__': + dbus.mainloop.glib.DBusGMainLoop(set_as_default=True) + agent = NetworkTime() + + mainloop = gobject.MainLoop() + mainloop.run() + + + -- 1.7.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2011-02-08 15:46 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-02-01 14:49 [PATCH 0/4 v4] Network Time Plugin Antti Paila 2011-02-01 14:49 ` [PATCH 1/4] nettime: Network time plugin implementation Antti Paila 2011-02-03 15:17 ` Aki Niemi 2011-02-07 18:58 ` Marcel Holtmann 2011-02-08 12:25 ` Antti Paila 2011-02-08 15:46 ` Marcel Holtmann 2011-02-01 14:49 ` [PATCH 2/4] nettime: Makefile.am modification Antti Paila 2011-02-01 14:49 ` [PATCH 3/4] nettime: Documentation Antti Paila 2011-02-01 14:54 ` Jeevaka.Badrappan 2011-02-01 19:58 ` Aki Niemi 2011-02-07 19:00 ` Marcel Holtmann 2011-02-01 14:49 ` [PATCH 4/4] nettime: Mock Timed for testing Antti Paila
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.