[PATCH 0/4 v2] Network Time plugin

[PATCH 0/4 v2] Network Time plugin

[Antti Paila]

[-- Attachment #1: Type: text/plain, Size: 762 bytes --]

 This series of patches introduces the network time part of the NITZ feature 
 as outlined in 3GPP spec 22.042. The Network Time plugin has two DBUS 
 interfaces for client applications: notification signal and polling method 
 call. The time information consists of three dictionary entries: 1) time and 
 date in seconds from epoch (renormalized to time indicatation arrival time);
 2) daylight saving time; 3) timezone.
 
 include/dbus.h |    1 +
 plugins/nettime.c | 285 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 Makefile.am |   10 ++++++++--
 test/get-nettime |   25 +++++++++++++++++++++++++
 test/test-nettime |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 doc/network-time-api.txt |   31 +++++++++++++++++++++++++++++++

[PATCH 1/4 v2] plugins: Implementation of Network Time plugin

[Antti Paila]

[-- Attachment #1: Type: text/plain, Size: 8161 bytes --]

---
 include/dbus.h    |    1 +
 plugins/nettime.c |  285 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 286 insertions(+), 0 deletions(-)
 create mode 100644 plugins/nettime.c

diff --git a/include/dbus.h b/include/dbus.h
index 9e29afb..0c48f83 100644
--- a/include/dbus.h
+++ b/include/dbus.h
@@ -45,6 +45,7 @@ extern "C" {
 #define OFONO_MESSAGE_WAITING_INTERFACE "org.ofono.MessageWaiting"
 #define OFONO_NETWORK_REGISTRATION_INTERFACE "org.ofono.NetworkRegistration"
 #define OFONO_NETWORK_OPERATOR_INTERFACE "org.ofono.NetworkOperator"
+#define OFONO_NETWORK_TIME_INTERFACE "org.ofono.NetworkTime"
 #define OFONO_PHONEBOOK_INTERFACE "org.ofono.Phonebook"
 #define OFONO_RADIO_SETTINGS_INTERFACE "org.ofono.RadioSettings"
 #define OFONO_AUDIO_SETTINGS_INTERFACE "org.ofono.AudioSettings"
diff --git a/plugins/nettime.c b/plugins/nettime.c
new file mode 100644
index 0000000..d080582
--- /dev/null
+++ b/plugins/nettime.c
@@ -0,0 +1,285 @@
+/*
+ *
+ *  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 <string.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"
+
+struct nettime_data {
+
+	gboolean time_available;
+	time_t nw_time_utc;
+	time_t received;
+	int dst;
+	int time_zone;
+};
+
+static void nettime_register(struct ofono_nettime_context *);
+
+static gboolean encode_time_format(struct ofono_network_time *time,
+				 struct tm *tm)
+{
+	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;
+	tm->tm_gmtoff = time->utcoff;
+	tm->tm_isdst = time->dst;
+
+	return TRUE;
+}
+
+static time_t get_monotonic_time()
+{
+	struct timespec ts;
+	clock_gettime(CLOCK_MONOTONIC, &ts);
+	return ts.tv_sec;
+}
+
+static int fill_time_changed_signal(DBusMessage *signal,
+		struct nettime_data *ntd)
+{
+	DBusMessageIter iter, iter_array;
+	time_t utc;
+
+	dbus_message_iter_init_append(signal, &iter);
+	dbus_message_iter_open_container(&iter,
+					DBUS_TYPE_ARRAY,
+					"{sv}",
+					&iter_array);
+
+	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,
+				"Timezone",
+				DBUS_TYPE_INT32,
+				&ntd->time_zone);
+	ofono_dbus_dict_append(&iter_array,
+				"DST",
+				DBUS_TYPE_UINT32,
+				&ntd->dst);
+	dbus_message_iter_close_container(&iter, &iter_array);
+	return 0;
+}
+
+static DBusMessage *create_time_changed_signal(
+			struct ofono_nettime_context *context)
+{
+	DBusMessage *signal;
+	struct nettime_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;
+	}
+
+	signal = dbus_message_new_signal(path, OFONO_NETWORK_TIME_INTERFACE,
+					"NetworkTimeChanged");
+	fill_time_changed_signal(signal, ntd);
+
+	return signal;
+}
+
+static void init_time(struct ofono_nettime_context *context)
+{
+	struct nettime_data *nettime_data = g_new0(struct nettime_data, 1);
+
+	nettime_data->time_available = FALSE;
+	nettime_data->dst = 0;
+	nettime_data->time_zone = 0;
+	context->data = nettime_data;
+
+}
+
+static int nettime_probe(struct ofono_nettime_context *context)
+{
+	ofono_debug("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)
+{
+	DBusConnection *conn;
+	const char *path;
+
+	ofono_debug("Network Time Remove for modem: %p", context->modem);
+
+	g_free(context->data);
+
+	conn = ofono_dbus_get_connection();
+	path = ofono_modem_get_path(context->modem);
+
+	if (!g_dbus_unregister_interface(conn,
+					path,
+					OFONO_NETWORK_TIME_INTERFACE))
+		ofono_error("Could not unregister %s interface",
+				OFONO_NETWORK_TIME_INTERFACE);
+
+	ofono_modem_remove_interface(context->modem,
+			OFONO_NETWORK_TIME_INTERFACE);
+}
+
+static void nettime_info_received(struct ofono_nettime_context *context,
+				struct ofono_network_time *info)
+{
+	DBusMessage *nt_signal;
+	struct tm t;
+	struct nettime_data *ntd = context->data;
+
+	if (info == NULL)
+		return;
+
+	ofono_debug("Received a network time notification on modem: %p",
+			context->modem);
+
+	ntd->received = get_monotonic_time();
+
+	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;
+
+	nt_signal = create_time_changed_signal(context);
+	if (nt_signal == NULL) {
+		ofono_error("Failed to create NetworkTimeChanged signal");
+		return;
+	}
+
+	g_dbus_send_message(ofono_dbus_get_connection(), nt_signal);
+}
+
+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 DBusMessage *nettime_get_time(DBusConnection *conn,
+						DBusMessage *msg, void *data)
+{
+	DBusMessage *reply;
+	int status;
+	struct ofono_nettime_context *context = data;
+
+	ofono_debug("Time requested from modem %p", context->modem);
+
+	reply = dbus_message_new_method_return(msg);
+	if (reply == NULL) {
+		ofono_error("Message allocation failed");
+		return NULL;
+	}
+
+	status = fill_time_changed_signal(reply, context->data);
+	if (status != 0) {
+		ofono_error("NetworkTimeChaged signal not filled, status: %d",
+			status);
+		return NULL;
+	}
+
+	return reply;
+}
+
+static GDBusMethodTable nettime_methods[] = {
+	{ "GetNetworkTime", "", "a{sv}", nettime_get_time },
+	{ }
+};
+
+static GDBusSignalTable nettime_signals[] = {
+	{ "NetworkTimeChanged", "a{sv}" },
+	{ }
+};
+
+static void nettime_register(struct ofono_nettime_context *context)
+{
+	DBusConnection *conn;
+	const char *path;
+
+	ofono_debug("Registering Network time for modem %s" ,
+		ofono_modem_get_path(context->modem));
+
+	conn = ofono_dbus_get_connection();
+
+	path = ofono_modem_get_path(context->modem);
+	if (path == NULL) {
+		ofono_error("No path for modem found");
+		return;
+	}
+
+	if (!g_dbus_register_interface(conn, path,
+				OFONO_NETWORK_TIME_INTERFACE,
+				nettime_methods,
+				nettime_signals,
+				NULL, context, NULL)) {
+		ofono_error("Could not create %s interface",
+				OFONO_NETWORK_TIME_INTERFACE);
+		return;
+	}
+
+	ofono_modem_add_interface(context->modem, OFONO_NETWORK_TIME_INTERFACE);
+}
+
+OFONO_PLUGIN_DEFINE(nettime, "Network Time Plugin",
+		VERSION, OFONO_PLUGIN_PRIORITY_DEFAULT,
+		nettime_init, nettime_exit)
+
-- 
1.7.1

[PATCH 2/4 v2] plugins: Enabling nettime plugin in Makefile.am

[Antti Paila]

[-- Attachment #1: Type: text/plain, Size: 1155 bytes --]

---
 Makefile.am |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index cdb3166..70162ec 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -296,6 +296,9 @@ builtin_modules += example_nettime
 builtin_sources += examples/nettime.c
 endif
 
+builtin_modules += nettime
+builtin_sources += plugins/nettime.c
+
 builtin_modules += smart_messaging
 builtin_sources += plugins/smart-messaging.c
 
@@ -358,7 +361,8 @@ doc_files = doc/overview.txt doc/ofono-paper.txt \
 			doc/message-api.txt doc/message-waiting-api.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/audio-settings-api.txt doc/text-telephony-api.txt \
+			doc/network-time-api.txt
 
 
 test_scripts = test/backtrace \
@@ -423,7 +427,9 @@ test_scripts = test/backtrace \
 		test/test-push-notification \
 		test/test-smart-messaging \
 		test/send-vcard \
-		test/set-tty
+		test/set-tty \
+		test/test-nettime \
+		test/get-nettime
 
 if TEST
 testdir = $(pkglibdir)/test
-- 
1.7.1

[PATCH 3/4 v2] plugins: Test scripts for nettime plugin

[Antti Paila]

[-- Attachment #1: Type: text/plain, Size: 2528 bytes --]

---
 test/get-nettime  |   25 +++++++++++++++++++++++++
 test/test-nettime |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 0 deletions(-)
 create mode 100755 test/get-nettime
 create mode 100755 test/test-nettime

diff --git a/test/get-nettime b/test/get-nettime
new file mode 100755
index 0000000..39aa232
--- /dev/null
+++ b/test/get-nettime
@@ -0,0 +1,25 @@
+#!/usr/bin/python
+
+import gobject
+import dbus
+import sys
+import time
+import dbus.mainloop.glib
+
+def time_changed( arg ) :
+	print "--time_changed callback called"
+	if "UTC" in arg :
+		print "UTC Time from mobile: %d" % arg["UTC"]
+	print "DST: %d" % arg["DST"]
+	print "Timezone: %d" % arg["Timezone"]
+	print "UTC time is: %d"  % time.time()
+
+bus = dbus.SystemBus()
+manager = dbus.Interface(bus.get_object('org.ofono','/'), 'org.ofono.Manager')
+path = manager.GetModems()[0][0]
+
+modem_proxy = bus.get_object('org.ofono', path)
+nettime_iface = dbus.Interface(modem_proxy, 'org.ofono.NetworkTime')
+answer = nettime_iface.GetNetworkTime()
+time_changed(answer)
+
diff --git a/test/test-nettime b/test/test-nettime
new file mode 100755
index 0000000..c2d533c
--- /dev/null
+++ b/test/test-nettime
@@ -0,0 +1,46 @@
+#!/usr/bin/python
+
+import gobject
+import dbus
+import sys
+import time
+import dbus.mainloop.glib
+
+
+def time_changed( arg ) :
+	print "Time from mobile: %d" % arg["UTC"]
+	print "DST: %d" % arg["DST"]
+	print "Timezone: %d" % arg["Timezone"]
+	print "UTC is: %d"  % time.time()
+
+
+dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
+
+bus = dbus.SystemBus()
+manager = dbus.Interface(bus.get_object('org.ofono', '/'), 'org.ofono.Manager')
+modems = manager.GetModems()
+path = modems[0][0]
+
+print "Connecting modem %s..." % path
+modem = dbus.Interface(bus.get_object('org.ofono', path),
+						'org.ofono.Modem')
+
+print "Powering up modem %s..." % (path)
+modem.SetProperty("Powered", dbus.Boolean(1))
+
+print "Go Online modem %s..." % (path)
+modem.SetProperty("Online", dbus.Boolean(1))
+print "Go Offline modem %s..." % (path)
+modem.SetProperty("Online", dbus.Boolean(0))
+print "Go Online modem %s..." % (path)
+modem.SetProperty("Online", dbus.Boolean(1))
+
+nettime = dbus.Interface(bus.get_object('org.ofono', path),
+				'org.ofono.NetworkTime')
+nettime.connect_to_signal("NetworkTimeChanged", time_changed)
+
+mainloop = gobject.MainLoop()
+mainloop.run()
+
+
+
-- 
1.7.1

[PATCH 4/4 v2] plugins: Documentation for nettime plugin

[Antti Paila]

[-- Attachment #1: Type: text/plain, Size: 1314 bytes --]

---
 doc/network-time-api.txt |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 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..cb2b9d7
--- /dev/null
+++ b/doc/network-time-api.txt
@@ -0,0 +1,31 @@
+Network time hierarchy
+======================
+
+Service		org.ofono
+Interface	org.ofono.NetworkTime
+Object path	[variable prefix]/{modem0,modem1,...}
+
+Methods		dict GetNetworkTime()
+
+			Returns the network indicated time for the modem object.
+			See the properties section for the content of the dict.
+
+Signals		NetworkTimeChanged(dict)
+
+			This signal indicates the reception of network time.
+			See the properties section for the content of the dict.
+
+Properties	int64 UTC [readonly, optional]
+
+			Network time in seconds from epoch normalized to 
+			indication arrival timestamp. Reveicing entity obtains 
+			real time by adding the current time from monotonic
+			clock e.g. clock_gettime(CLOCK_MONOTONIC,...).
+
+		int32 Timezone [readonly]
+
+			Current timezone offset in seconds from UTC.
+
+		uint32 DST [readonly]
+
+			Current daylight saving setting in hours.
\ No newline at end of file
-- 
1.7.1

Re: [PATCH 0/4 v2] Network Time plugin

[Antti Paila]

[-- Attachment #1: Type: text/plain, Size: 954 bytes --]

Hi,

Could someone review this patch. It has been waiting for review a while
now. Thanks.

-Antti  

On Fri, 2010-12-10 at 09:53 +0200, Antti Paila wrote:
> This series of patches introduces the network time part of the NITZ feature 
>  as outlined in 3GPP spec 22.042. The Network Time plugin has two DBUS 
>  interfaces for client applications: notification signal and polling method 
>  call. The time information consists of three dictionary entries: 1) time and 
>  date in seconds from epoch (renormalized to time indicatation arrival time);
>  2) daylight saving time; 3) timezone.
>  
>  include/dbus.h |    1 +
>  plugins/nettime.c | 285 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  Makefile.am |   10 ++++++++--
>  test/get-nettime |   25 +++++++++++++++++++++++++
>  test/test-nettime |   46 ++++++++++++++++++++++++++++++++++++++++++++++
>  doc/network-time-api.txt |   31 +++++++++++++++++++++++++++++++

Re: [PATCH 0/4 v2] Network Time plugin

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 989 bytes --]

Hi Antti,

please do not top-post on this mailing list.

> Could someone review this patch. It has been waiting for review a while
> now. Thanks.

<snip>

> > This series of patches introduces the network time part of the NITZ feature 
> >  as outlined in 3GPP spec 22.042. The Network Time plugin has two DBUS 
> >  interfaces for client applications: notification signal and polling method 
> >  call. The time information consists of three dictionary entries: 1) time and 
> >  date in seconds from epoch (renormalized to time indicatation arrival time);
> >  2) daylight saving time; 3) timezone.

I am still saying that the plugin should monitor the presence of timed
and then just send a D-Bus message to timed. That saves us the problem
with the timestamping. And in case timed is not running it should just
set the current time directly.

I really prefer that we don't hand out timestamps over the D-Bus API.
That seems wrong to me.

Regards

Marcel

Re: [PATCH 0/4 v2] Network Time plugin

[Aki Niemi]

[-- Attachment #1: Type: text/plain, Size: 831 bytes --]

Hi Marcel,

2010/12/21 Marcel Holtmann <marcel@holtmann.org>:
> I am still saying that the plugin should monitor the presence of timed
> and then just send a D-Bus message to timed. That saves us the problem
> with the timestamping.

There is no problem. If you still think there is a problem, you need
to continue the previous thread on this subject and prove your point.

> And in case timed is not running it should just
> set the current time directly.

You're welcome to write such a plugin -- this is why nettime is a
plugin API after all. It might make sense in some platforms,
especially those that don't have a working monotonic clock...

> I really prefer that we don't hand out timestamps over the D-Bus API.
> That seems wrong to me.

No timestamp is sent. Please read the patches.

Cheers,
Aki

Re: [PATCH 0/4 v2] Network Time plugin

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1047 bytes --]

Hi Aki,

> > I am still saying that the plugin should monitor the presence of timed
> > and then just send a D-Bus message to timed. That saves us the problem
> > with the timestamping.
> 
> There is no problem. If you still think there is a problem, you need
> to continue the previous thread on this subject and prove your point.
> 
> > And in case timed is not running it should just
> > set the current time directly.
> 
> You're welcome to write such a plugin -- this is why nettime is a
> plugin API after all. It might make sense in some platforms,
> especially those that don't have a working monotonic clock...

true, but as long as you guys are proposing an official org.ofono API it
has to fit in a general context of oFono.

> > I really prefer that we don't hand out timestamps over the D-Bus API.
> > That seems wrong to me.
> 
> No timestamp is sent. Please read the patches.

So that got fixed. Is this really enough for you guys to just normalize
the value that you are getting?

Regards

Marcel

Re: [PATCH 0/4 v2] Network Time plugin

[=?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont]

[-- Attachment #1: Type: text/plain, Size: 455 bytes --]

On Tuesday 21 December 2010 17:54:53 ext Aki Niemi, you wrote:
>  It might make sense in some platforms,
> especially those that don't have a working monotonic clock...

That's a stupid requirement. Without a working clock, you can't get things 
like poll(), select() or nanosleep() to work. So glib won't work and so oFono 
won't work either.

That use case is just FUD.

-- 
Rémi Denis-Courmont
Nokia Devices R&D, Maemo Software, Helsinki

Re: [PATCH 0/4 v2] Network Time plugin

[Aki Niemi]

[-- Attachment #1: Type: text/plain, Size: 502 bytes --]

Hi Rémi,

2010/12/21 Rémi Denis-Courmont <remi.denis-courmont@nokia.com>:
> On Tuesday 21 December 2010 17:54:53 ext Aki Niemi, you wrote:
>>  It might make sense in some platforms,
>> especially those that don't have a working monotonic clock...
>
> That's a stupid requirement. Without a working clock, you can't get things
> like poll(), select() or nanosleep() to work. So glib won't work and so oFono
> won't work either.

You obviously missed my <sarcasm> tags. ;)

Cheers,
Aki

Re: [PATCH 0/4 v2] Network Time plugin

[Aki Niemi]

[-- Attachment #1: Type: text/plain, Size: 380 bytes --]

Hi Marcel,

2010/12/21 Marcel Holtmann <marcel@holtmann.org>:
> true, but as long as you guys are proposing an official org.ofono API it
> has to fit in a general context of oFono.

We can stuff this under meego.com instead, no problem.

> So that got fixed. Is this really enough for you guys to just normalize
> the value that you are getting?

Yes.

Cheers,
Aki

Re: [PATCH 0/4 v2] Network Time plugin

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1652 bytes --]

Hi Aki,

> There is no problem. If you still think there is a problem, you need
> to continue the previous thread on this subject and prove your point.
> 
> > And in case timed is not running it should just
> > set the current time directly.
> 
> You're welcome to write such a plugin -- this is why nettime is a
> plugin API after all. It might make sense in some platforms,
> especially those that don't have a working monotonic clock...

actually I spent time thinking about this and the whole time daemon idea
and what are potential input sources.

So the idea of having an oFono D-Bus API to export time information is
just wrong from my point of view. The plugin inside oFono should tell
the time daemon about this. And not the time daemon go out and bother
with additional sources etc.

And if you take the normalized time based on a monotonic clock, such a
plugin that just send a D-Bus message to a time daemon is actually a lot
simpler than exposing a full blown D-Bus interface.

So the plugin just has to store the normalized time in memory. And if a
time daemon is present, then send out an update if needed, otherwise
don't bother.

I really like to see it implemented this way. It is simpler and makes a
lot more sense than sending out D-Bus signals to everybody for time
changes.

In addition there will be more source for time and/or timezone
information. For example the GeoIP country information returned by
ConnMan portal service. We are not exposing a Getter/Changed D-Bus API
from ConnMan for this either. We just will sending these information to
interested parties.

Regards

Marcel

Re: [PATCH 0/4 v2] Network Time plugin

[=?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont]

[-- Attachment #1: Type: text/plain, Size: 1807 bytes --]

On Thursday 23 December 2010 04:44:52 ext Marcel Holtmann, you wrote:
> So the idea of having an oFono D-Bus API to export time information is
> just wrong from my point of view. The plugin inside oFono should tell
> the time daemon about this.

That would be race-prone by design. For all we know, the time daemon might be 
started after oFono (or restarted). It's pretty basic design to have a getter 
and a change signal for any kind of value.

Besides, I find hard-coding The One time daemon in the oFono plugin rather 
silly. It's really nothing but an arbitrary design limitation that would be 
overcome with a clean D-Bus API like we proposed several times.

> And not the time daemon go out and bother
> with additional sources etc.

Last time I checked, NTP clients "go out and bother" to ask the configured NTP 
server, not the other way around. I fail to see reason why oFono should work 
the other way around. I do see several reasons why it should not.

> And if you take the normalized time based on a monotonic clock, such a
> plugin that just send a D-Bus message to a time daemon is actually a lot
> simpler than exposing a full blown D-Bus interface.

It's very marginally simpler: one signal against one signal plus one getter 
that will share much marshalling code with the signal. But that's irrelevant. 
It actually works. The sole signal design is broken.

> So the plugin just has to store the normalized time in memory. And if a
> time daemon is present, then send out an update if needed, otherwise
> don't bother.

Oh? That would actualy be far more complicated, as the plugin would then need 
to track down the presence of the time daemon. I see some contradiction here.

-- 
Rémi Denis-Courmont
Nokia Devices R&D, Maemo Software, Helsinki

Re: [PATCH 0/4 v2] Network Time plugin

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 2519 bytes --]

Hi Remi,

> > So the idea of having an oFono D-Bus API to export time information is
> > just wrong from my point of view. The plugin inside oFono should tell
> > the time daemon about this.
> 
> That would be race-prone by design. For all we know, the time daemon might be 
> started after oFono (or restarted). It's pretty basic design to have a getter 
> and a change signal for any kind of value.

there is no race here. The timed plugin inside oFono can just keep the
time and only send it out when timed starts. You can easily monitor
deamons lifetime with D-Bus.

> Besides, I find hard-coding The One time daemon in the oFono plugin rather 
> silly. It's really nothing but an arbitrary design limitation that would be 
> overcome with a clean D-Bus API like we proposed several times.

It is a plugin that monitors the existence of such a daemon. So we can
have multiple plugins for each daemon. And we can even have them all
active at the same time. So where do you see a problem here?

> > And not the time daemon go out and bother
> > with additional sources etc.
> 
> Last time I checked, NTP clients "go out and bother" to ask the configured NTP 
> server, not the other way around. I fail to see reason why oFono should work 
> the other way around. I do see several reasons why it should not.

You need to tell NTP client (or the ntpd running) the time servers to
use. Check how we do that in ConnMan. we tell ntpd about time servers
and not the other way around.

> > And if you take the normalized time based on a monotonic clock, such a
> > plugin that just send a D-Bus message to a time daemon is actually a lot
> > simpler than exposing a full blown D-Bus interface.
> 
> It's very marginally simpler: one signal against one signal plus one getter 
> that will share much marshalling code with the signal. But that's irrelevant. 
> It actually works. The sole signal design is broken.

I was talking about a method call to timed and not a signal.

> > So the plugin just has to store the normalized time in memory. And if a
> > time daemon is present, then send out an update if needed, otherwise
> > don't bother.
> 
> Oh? That would actualy be far more complicated, as the plugin would then need 
> to track down the presence of the time daemon. I see some contradiction here.

And that is a problem with D-Bus how? With g_dbus_add_service_watch this
is dead simple actually. Simpler than providing a D-Bus interface.

Regards

Marcel

Re: [PATCH 0/4 v2] Network Time plugin

[=?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont]

[-- Attachment #1: Type: text/plain, Size: 3612 bytes --]

This discussion is completely ridiculous at this point.

On Monday 03 January 2011 22:47:36 ext Marcel Holtmann, you wrote:
> > > So the idea of having an oFono D-Bus API to export time information is
> > > just wrong from my point of view. The plugin inside oFono should tell
> > > the time daemon about this.
> > 
> > That would be race-prone by design. For all we know, the time daemon
> > might be started after oFono (or restarted). It's pretty basic design to
> > have a getter and a change signal for any kind of value.
> 
> there is no race here. The timed plugin inside oFono can just keep the
> time and only send it out when timed starts. You can easily monitor
> deamons lifetime with D-Bus.

You just said that you did not want oFono to keep the time reference value. 
This is self-contradictory. Now you admit that oFono does need to keep it.

But then it is far simpler to have a D-Bus getter and a D-Bus signal by any 
sane measure of complexity.

> > Besides, I find hard-coding The One time daemon in the oFono plugin
> > rather silly. It's really nothing but an arbitrary design limitation
> > that would be overcome with a clean D-Bus API like we proposed several
> > times.
> 
> It is a plugin that monitors the existence of such a daemon. So we can
> have multiple plugins for each daemon. And we can even have them all
> active at the same time. So where do you see a problem here?

So that is far more complicated, inextensible and it consumes more CPU (more 
D-Bus interactions). This makes absolutely no sense.

> > > And not the time daemon go out and bother
> > > with additional sources etc.
> > 
> > Last time I checked, NTP clients "go out and bother" to ask the
> > configured NTP server, not the other way around. I fail to see reason
> > why oFono should work the other way around. I do see several reasons why
> > it should not.
> 
> You need to tell NTP client (or the ntpd running) the time servers to
> use. Check how we do that in ConnMan. we tell ntpd about time servers
> and not the other way around.

That's the point: the one who wants to learn the time (the NTP client, or the 
time daemon) is the one ASKING for it from the one who knows (the NTP server 
or oFono). The other direction is just totally backward design.

> > > And if you take the normalized time based on a monotonic clock, such a
> > > plugin that just send a D-Bus message to a time daemon is actually a
> > > lot simpler than exposing a full blown D-Bus interface.
> > 
> > It's very marginally simpler: one signal against one signal plus one
> > getter that will share much marshalling code with the signal. But that's
> > irrelevant. It actually works. The sole signal design is broken.
> 
> I was talking about a method call to timed and not a signal.

It's largelly irrelevant. There's no functional difference between a method 
call without response and a signal.

> > > So the plugin just has to store the normalized time in memory. And if a
> > > time daemon is present, then send out an update if needed, otherwise
> > > don't bother.

> > Oh? That would actualy be far more complicated, as the plugin would then
> > need to track down the presence of the time daemon. I see some
> > contradiction here.
> 
> And that is a problem with D-Bus how? With g_dbus_add_service_watch this
> is dead simple actually. Simpler than providing a D-Bus interface.

I never said this was not feasible. I said this was more complicated and 
disingenuous.

-- 
Rémi Denis-Courmont
Nokia Devices R&D, Maemo Software, Helsinki

Re: [PATCH 0/4 v2] Network Time plugin

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 4840 bytes --]

Hi Remi,

> > > > So the idea of having an oFono D-Bus API to export time information is
> > > > just wrong from my point of view. The plugin inside oFono should tell
> > > > the time daemon about this.
> > > 
> > > That would be race-prone by design. For all we know, the time daemon
> > > might be started after oFono (or restarted). It's pretty basic design to
> > > have a getter and a change signal for any kind of value.
> > 
> > there is no race here. The timed plugin inside oFono can just keep the
> > time and only send it out when timed starts. You can easily monitor
> > deamons lifetime with D-Bus.
> 
> You just said that you did not want oFono to keep the time reference value. 
> This is self-contradictory. Now you admit that oFono does need to keep it.

I said that 3-4 emails early already. It is either that or you just go
ahead and set the system if timed is not running.

> But then it is far simpler to have a D-Bus getter and a D-Bus signal by any 
> sane measure of complexity.

So you did consider the complexity on both sides, ofonod and timed? And
not just looked at one side of picture?

> > > Besides, I find hard-coding The One time daemon in the oFono plugin
> > > rather silly. It's really nothing but an arbitrary design limitation
> > > that would be overcome with a clean D-Bus API like we proposed several
> > > times.
> > 
> > It is a plugin that monitors the existence of such a daemon. So we can
> > have multiple plugins for each daemon. And we can even have them all
> > active at the same time. So where do you see a problem here?
> 
> So that is far more complicated, inextensible and it consumes more CPU (more 
> D-Bus interactions). This makes absolutely no sense.

Are you kidding me here? Where is the more D-Bus interaction going to
happen. It would be nice if you actually start counting both sides of
the equation here.

So you really want timed to monitor the oFono manager interface with
their modems and then the method calls and signals from the time
interface. Please count again what consumes more D-Bus interactions.

> > > > And not the time daemon go out and bother
> > > > with additional sources etc.
> > > 
> > > Last time I checked, NTP clients "go out and bother" to ask the
> > > configured NTP server, not the other way around. I fail to see reason
> > > why oFono should work the other way around. I do see several reasons why
> > > it should not.
> > 
> > You need to tell NTP client (or the ntpd running) the time servers to
> > use. Check how we do that in ConnMan. we tell ntpd about time servers
> > and not the other way around.
> 
> That's the point: the one who wants to learn the time (the NTP client, or the 
> time daemon) is the one ASKING for it from the one who knows (the NTP server 
> or oFono). The other direction is just totally backward design.

Who is telling NTP client where the servers are? Or is the NTP client
just going out fishing for NTP servers.

Maybe we should just turn ofonod into an NTP time source and then let
ntpd just always use that one. Problem solved and nicely integrated with
the desktop world as well.

> > > > And if you take the normalized time based on a monotonic clock, such a
> > > > plugin that just send a D-Bus message to a time daemon is actually a
> > > > lot simpler than exposing a full blown D-Bus interface.
> > > 
> > > It's very marginally simpler: one signal against one signal plus one
> > > getter that will share much marshalling code with the signal. But that's
> > > irrelevant. It actually works. The sole signal design is broken.
> > 
> > I was talking about a method call to timed and not a signal.
> 
> It's largelly irrelevant. There's no functional difference between a method 
> call without response and a signal.

There is a big difference when you look at potential wakeups of other
processes. A method call is always directed, a signal is broadcasted.

> > > > So the plugin just has to store the normalized time in memory. And if a
> > > > time daemon is present, then send out an update if needed, otherwise
> > > > don't bother.
> 
> > > Oh? That would actualy be far more complicated, as the plugin would then
> > > need to track down the presence of the time daemon. I see some
> > > contradiction here.
> > 
> > And that is a problem with D-Bus how? With g_dbus_add_service_watch this
> > is dead simple actually. Simpler than providing a D-Bus interface.
> 
> I never said this was not feasible. I said this was more complicated and 
> disingenuous.

I actually said that it is a lot simpler. If you don't believe me, why
don't you just code both versions of the plugin and then compare them.

And it is also a lot simpler for timed to integrate with this.

Regards

Marcel

Re: [PATCH 0/4 v2] Network Time plugin

[Aki Niemi]

[-- Attachment #1: Type: text/plain, Size: 1272 bytes --]

Hi Marcel,

On Tue, 2011-01-04 at 01:41 -0800, ext Marcel Holtmann wrote:
> > But then it is far simpler to have a D-Bus getter and a D-Bus signal by any 
> > sane measure of complexity.
> 
> So you did consider the complexity on both sides, ofonod and timed? And
> not just looked at one side of picture?

Timed also needs to follow the registration status, namely the MNC/MCC
of the registered network.  This information it needs to be able to find
the correct timezone, as the UTC offset alone only indicates the
geographic longitude for the timezone.

Factor that in to the equation, and timed already needs to enumerate
available modems, call GetProperties, and listen to the
NetworkRegistration interface's PropertyChanged signals.

However, if we refactor the time plugin to also send the MNC/MCC pair --
or better yet, the ISO country code based on MCC or even the actual
timezone from matching zone.tab entry -- then following netreg is no
longer needed.

*Then* I agree a method call is actually a lot simpler from timed point
of view; all it needs to do is implement a single method on some
org.ofono.NetworkTimeConsumer interface and not worry about enumerating
modems via ModemManager or listening on any signals.

Cheers,
Aki

Re: [PATCH 0/4 v2] Network Time plugin

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1652 bytes --]

Hi Aki,

> > > But then it is far simpler to have a D-Bus getter and a D-Bus signal by any 
> > > sane measure of complexity.
> > 
> > So you did consider the complexity on both sides, ofonod and timed? And
> > not just looked at one side of picture?
> 
> Timed also needs to follow the registration status, namely the MNC/MCC
> of the registered network.  This information it needs to be able to find
> the correct timezone, as the UTC offset alone only indicates the
> geographic longitude for the timezone.
> 
> Factor that in to the equation, and timed already needs to enumerate
> available modems, call GetProperties, and listen to the
> NetworkRegistration interface's PropertyChanged signals.
> 
> However, if we refactor the time plugin to also send the MNC/MCC pair --
> or better yet, the ISO country code based on MCC or even the actual
> timezone from matching zone.tab entry -- then following netreg is no
> longer needed.
> 
> *Then* I agree a method call is actually a lot simpler from timed point
> of view; all it needs to do is implement a single method on some
> org.ofono.NetworkTimeConsumer interface and not worry about enumerating
> modems via ModemManager or listening on any signals.

we are doing the country alpha2 matching to MCC already in ConnMan for
the WiFi regulatory enforcement update. I am not sure that I wanna copy
these tables around all the time.

So the struct ofono_nettime_context has already a reference to struct
ofono_modem, so why not add a reference to the struct ofono_netreg and
then you have the MCC/MNC details available inside the plugin.

Regards

Marcel

Re: [PATCH 0/4 v2] Network Time plugin

[=?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont]

[-- Attachment #1: Type: text/plain, Size: 2401 bytes --]

On Tuesday 04 January 2011 23:49:30 ext Marcel Holtmann, you wrote:
> Hi Aki,
> 
> > > > But then it is far simpler to have a D-Bus getter and a D-Bus signal
> > > > by any sane measure of complexity.
> > > 
> > > So you did consider the complexity on both sides, ofonod and timed? And
> > > not just looked at one side of picture?
> > 
> > Timed also needs to follow the registration status, namely the MNC/MCC
> > of the registered network.  This information it needs to be able to find
> > the correct timezone, as the UTC offset alone only indicates the
> > geographic longitude for the timezone.
> > 
> > Factor that in to the equation, and timed already needs to enumerate
> > available modems, call GetProperties, and listen to the
> > NetworkRegistration interface's PropertyChanged signals.
> > 
> > However, if we refactor the time plugin to also send the MNC/MCC pair --
> > or better yet, the ISO country code based on MCC or even the actual
> > timezone from matching zone.tab entry -- then following netreg is no
> > longer needed.
> > 
> > *Then* I agree a method call is actually a lot simpler from timed point
> > of view; all it needs to do is implement a single method on some
> > org.ofono.NetworkTimeConsumer interface and not worry about enumerating
> > modems via ModemManager or listening on any signals.
> 
> we are doing the country alpha2 matching to MCC already in ConnMan for
> the WiFi regulatory enforcement update. I am not sure that I wanna copy
> these tables around all the time.

Mapping MCC should be in oFono if only so not every oFono-using applications 
reinvents the same table. This is perhaps not so relevant for network time, 
but I think oFono should provide the country code for network registration and 
for the SIM card. As an example, this can be used to guess the default locale 
settings when the SIM is first inserted into a device.

In the case of network time, we really want to map the (MCC, UTC offset, DST) 
tuple to a time zone. We do not necessarily have to use any ISO code.

There are some ambiguities though, for instance Honolulu and Unalaska have the 
same offset to UTC (in winter), same country code but are in different time 
zones. Note that this is the only case I know, and it only affects under 10,000 
people.

-- 
Rémi Denis-Courmont
Nokia Devices R&D, Maemo Software, Helsinki

Re: [PATCH 0/4 v2] Network Time plugin

[=?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont]

[-- Attachment #1: Type: text/plain, Size: 502 bytes --]

On Wednesday 05 January 2011 10:29:40 ext Rémi Denis-Courmont, you wrote:
> There are some ambiguities though, for instance Honolulu and Unalaska have
> the same offset to UTC (in winter), same country code but are in different
> time zones. Note that this is the only case I know, and it only affects
> under 10,000 people.

A more common ambiguity would be between Arizona and the other Mountain Time 
United States.

-- 
Rémi Denis-Courmont
Nokia Devices R&D, Maemo Software, Helsinki

Re: [PATCH 0/4 v2] Network Time plugin

[Antti Paila]

[-- Attachment #1: Type: text/plain, Size: 2156 bytes --]

Hi Marcel,

On Tue, 2011-01-04 at 13:49 -0800, ext Marcel Holtmann wrote:
> Hi Aki,
> 
> > > > But then it is far simpler to have a D-Bus getter and a D-Bus signal by any 
> > > > sane measure of complexity.
> > > 
> > > So you did consider the complexity on both sides, ofonod and timed? And
> > > not just looked at one side of picture?
> > 
> > Timed also needs to follow the registration status, namely the MNC/MCC
> > of the registered network.  This information it needs to be able to find
> > the correct timezone, as the UTC offset alone only indicates the
> > geographic longitude for the timezone.
> > 
> > Factor that in to the equation, and timed already needs to enumerate
> > available modems, call GetProperties, and listen to the
> > NetworkRegistration interface's PropertyChanged signals.
> > 
> > However, if we refactor the time plugin to also send the MNC/MCC pair --
> > or better yet, the ISO country code based on MCC or even the actual
> > timezone from matching zone.tab entry -- then following netreg is no
> > longer needed.
> > 
> > *Then* I agree a method call is actually a lot simpler from timed point
> > of view; all it needs to do is implement a single method on some
> > org.ofono.NetworkTimeConsumer interface and not worry about enumerating
> > modems via ModemManager or listening on any signals.
> 
> we are doing the country alpha2 matching to MCC already in ConnMan for
> the WiFi regulatory enforcement update. I am not sure that I wanna copy
> these tables around all the time.
> 
> So the struct ofono_nettime_context has already a reference to struct
> ofono_modem, so why not add a reference to the struct ofono_netreg and
> then you have the MCC/MNC details available inside the plugin.

If I would like to do conversion from mcc to alpha2 in ofono, what would
be your suggestion for doing this. I noticed that in connman you have
mcc.h that contains the mapping array, but in ofono we don't have such
an array. As a temporary solution, would it be feasible to just import
the mcc.h to ofono until a more generic solution is implemented?

Best Regards,
 Antti

Re: [PATCH 0/4 v2] Network Time plugin

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 2732 bytes --]

Hi Antti,

> > > > > But then it is far simpler to have a D-Bus getter and a D-Bus signal by any 
> > > > > sane measure of complexity.
> > > > 
> > > > So you did consider the complexity on both sides, ofonod and timed? And
> > > > not just looked at one side of picture?
> > > 
> > > Timed also needs to follow the registration status, namely the MNC/MCC
> > > of the registered network.  This information it needs to be able to find
> > > the correct timezone, as the UTC offset alone only indicates the
> > > geographic longitude for the timezone.
> > > 
> > > Factor that in to the equation, and timed already needs to enumerate
> > > available modems, call GetProperties, and listen to the
> > > NetworkRegistration interface's PropertyChanged signals.
> > > 
> > > However, if we refactor the time plugin to also send the MNC/MCC pair --
> > > or better yet, the ISO country code based on MCC or even the actual
> > > timezone from matching zone.tab entry -- then following netreg is no
> > > longer needed.
> > > 
> > > *Then* I agree a method call is actually a lot simpler from timed point
> > > of view; all it needs to do is implement a single method on some
> > > org.ofono.NetworkTimeConsumer interface and not worry about enumerating
> > > modems via ModemManager or listening on any signals.
> > 
> > we are doing the country alpha2 matching to MCC already in ConnMan for
> > the WiFi regulatory enforcement update. I am not sure that I wanna copy
> > these tables around all the time.
> > 
> > So the struct ofono_nettime_context has already a reference to struct
> > ofono_modem, so why not add a reference to the struct ofono_netreg and
> > then you have the MCC/MNC details available inside the plugin.

one additional comment here about getting the MCC and MNC (or other atom
details). You can use __ofono_modem_find_atom() to get the netreg atom
from the modem pointer. This means that any nettime plugin already has
all the needed information available.

> If I would like to do conversion from mcc to alpha2 in ofono, what would
> be your suggestion for doing this. I noticed that in connman you have
> mcc.h that contains the mapping array, but in ofono we don't have such
> an array. As a temporary solution, would it be feasible to just import
> the mcc.h to ofono until a more generic solution is implemented?

I don't even like the fact that we imported it in ConnMan. That might
have been a mistake actually. I really prefer to have that on the
filesystem and we just mmap() it. And it is up to the distribution to
provide such a file.

We do a similar thing with OUI for BlueZ, so that is clearly the better
approach here.

Regards

Marcel

Re: [PATCH 0/4 v2] Network Time plugin

[=?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont]

[-- Attachment #1: Type: text/plain, Size: 725 bytes --]

On Friday 07 January 2011 23:05:31 ext Marcel Holtmann, you wrote:
> I don't even like the fact that we imported it in ConnMan. That might
> have been a mistake actually. I really prefer to have that on the
> filesystem and we just mmap() it. And it is up to the distribution to
> provide such a file.
> 
> We do a similar thing with OUI for BlueZ, so that is clearly the better
> approach here.

So I guess there are two different things here:
- whether oFono should parse that file,
- whether oFono should ship that file.

I guess you're arguing that oFono shouldn't ship the file, but it could still 
parse it from a known location.

-- 
Rémi Denis-Courmont
Nokia Devices R&D, Maemo Software, Helsinki