SMS agent interface

SMS agent interface

[Aki Niemi]

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

Hi all,

This following set of patches introduces an agent interface for
receiving text messages as well as vCard and vCal objects. It grows a
pair of methods on the org.ofono.MessageManager interface for
registering and unregistering such agents.

I've included the patches introducing datagram support to history that
I sent out separately earlier today. I think these should be evaluated
as a bunch, plus, they introduced some refactoring to the sms atom's
dispatch logic that I didn't feel like redoing. ;)

Basically, all text(ish) messages can now be received via the agent
interface on the sms atom, and then further support for dispatching
handlers based on other dst/src port numbers would happen via the
history API.

An example of such a plugin would be a WAP push handler that would
handle the WAP push datagrams exposing an appropriate D-Bus interface
for applications to process them further.

What is still missing is sending support for vCard and vCalendar,
which for consistency should probably be implemented as a pair of
methods on the org.ofono.MessageManager interface.

Cheers,
Aki

--

 Makefile.am         |    2 +-
 doc/message-api.txt |   51 ++++++++
 include/dbus.h      |    1 +
 include/history.h   |    5 +
 src/history.c       |   47 +++++++-
 src/ofono.h         |    8 ++
 src/sms.c           |  187 +++++++++++++++++++++++++---
 src/smsagent.c      |  344 +++++++++++++++++++++++++++++++++++++++++++++++++++
 src/smsagent.h      |   56 +++++++++
 test/sms-agent      |   59 +++++++++
 10 files changed, 741 insertions(+), 19 deletions(-)

[PATCH 1/7] Include datagram support to history API

[Aki Niemi]

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

---
 include/history.h |    5 +++++
 src/history.c     |   47 ++++++++++++++++++++++++++++++++++++++++++++++-
 src/ofono.h       |    8 ++++++++
 3 files changed, 59 insertions(+), 1 deletions(-)

diff --git a/include/history.h b/include/history.h
index c1c4aa1..e661f24 100644
--- a/include/history.h
+++ b/include/history.h
@@ -58,6 +58,11 @@ struct ofono_history_driver {
 				const struct ofono_uuid *uuid, const char *from,
 				const struct tm *remote, const struct tm *local,
 				const char *text);
+	void (*datagram_received)(struct ofono_history_context *context,
+				const struct ofono_uuid *uuid, const char *from,
+				int dst, int src, const struct tm *remote,
+				const struct tm *local,
+				const unsigned char *buffer, unsigned len);
 	void (*sms_send_pending)(struct ofono_history_context *context,
 					const struct ofono_uuid *uuid,
 					const char *to,
diff --git a/src/history.c b/src/history.c
index 0229cf5..2610141 100644
--- a/src/history.c
+++ b/src/history.c
@@ -47,7 +47,15 @@ struct history_call_foreach_data {
 struct history_sms_foreach_data {
 	const struct ofono_uuid *uuid;
 	const char *address;
-	const char *text;
+	union {
+		const char *text;
+		struct {
+			const unsigned char *data;
+			unsigned len;
+			guint16 dst;
+			guint16 src;
+		};
+	};
 	union {
 		struct {
 			const struct tm *remote;
@@ -193,6 +201,43 @@ void __ofono_history_sms_received(struct ofono_modem *modem,
 					history_sms_received, &hfd);
 }
 
+static void history_datagram_received(struct ofono_atom *atom, void *data)
+{
+	struct ofono_history_context *context = __ofono_atom_get_data(atom);
+	struct history_sms_foreach_data *hfd = data;
+
+	if (context->driver->datagram_received == NULL)
+		return;
+
+	context->driver->datagram_received(context, hfd->uuid, hfd->address,
+						hfd->dst, hfd->src,
+						hfd->remote, hfd->local,
+						hfd->data, hfd->len);
+}
+
+void __ofono_history_datagram_received(struct ofono_modem *modem,
+					const struct ofono_uuid *uuid,
+					const char *from, int dst, int src,
+					const struct tm *remote,
+					const struct tm *local,
+					const unsigned char *buffer,
+					unsigned len)
+{
+	struct history_sms_foreach_data hfd;
+
+	hfd.uuid = uuid;
+	hfd.address = from;
+	hfd.dst = dst;
+	hfd.src = src;
+	hfd.remote = remote;
+	hfd.local = local;
+	hfd.data = buffer;
+	hfd.len = len;
+
+	__ofono_modem_foreach_atom(modem, OFONO_ATOM_TYPE_HISTORY,
+					history_datagram_received, &hfd);
+}
+
 static void history_sms_send_pending(struct ofono_atom *atom, void *data)
 {
 	struct ofono_history_context *context = __ofono_atom_get_data(atom);
diff --git a/src/ofono.h b/src/ofono.h
index 41ce011..b068f48 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -311,6 +311,14 @@ void __ofono_history_sms_received(struct ofono_modem *modem,
 					const struct tm *local,
 					const char *text);
 
+void __ofono_history_datagram_received(struct ofono_modem *modem,
+					const struct ofono_uuid *uuid,
+					const char *from, int dst, int src,
+					const struct tm *remote,
+					const struct tm *local,
+					const unsigned char *buffer,
+					unsigned len);
+
 void __ofono_history_sms_send_pending(struct ofono_modem *modem,
 					const struct ofono_uuid *uuid,
 					const char *to,
-- 
1.7.0.4

Re: [PATCH 1/7] Include datagram support to history API

[Denis Kenzior]

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

Hi Aki,

> --- a/src/history.c
> +++ b/src/history.c
> @@ -47,7 +47,15 @@ struct history_call_foreach_data {
>  struct history_sms_foreach_data {
>  	const struct ofono_uuid *uuid;
>  	const char *address;
> -	const char *text;
> +	union {
> +		const char *text;
> +		struct {
> +			const unsigned char *data;
> +			unsigned len;
> +			guint16 dst;
> +			guint16 src;

Are you overflowing the dst/src ports for the 16 bit case by any chance?
 With the 8bit port fix patch, we're actually using 24 bits to store
these values.  There might need to be a comment in the history API about
this.

Are 8 bit ports even used in practice?  If not, then might be that
shifting 8-bit values by 16 is better...

Other than that, I'm happy with this one.

Regards,
-Denis

[PATCH 2/7] Add datagram dispatch to sms atom

[Aki Niemi]

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

---
 src/sms.c |   41 +++++++++++++++++++++++++++--------------
 1 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/src/sms.c b/src/sms.c
index 9bf7feb..e6a2f9c 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -1064,12 +1064,24 @@ static gboolean compute_incoming_msgid(GSList *sms_list,
 	return TRUE;
 }
 
-static void dispatch_app_datagram(struct ofono_sms *sms, int dst, int src,
-					unsigned char *buf, long len)
+static void dispatch_app_datagram(struct ofono_sms *sms,
+					const struct ofono_uuid *uuid,
+					int dst, int src,
+					unsigned char *buf, unsigned len,
+					const struct sms_address *addr,
+					const struct sms_scts *scts)
 {
-	DBG("Got app datagram for dst port: %d, src port: %d",
-			dst, src);
-	DBG("Contents-Len: %ld", len);
+	struct ofono_modem *modem = __ofono_atom_get_modem(sms->atom);
+	const char *sender = sms_address_to_string(addr);
+	time_t ts;
+	struct tm remote;
+	struct tm local;
+
+	ts = sms_scts_to_time(scts, &remote);
+	localtime_r(&ts, &local);
+
+	__ofono_history_datagram_received(modem, uuid, sender, dst, src,
+						&remote, &local, buf, len);
 }
 
 static void dispatch_text_message(struct ofono_sms *sms,
@@ -1141,6 +1153,7 @@ static void sms_dispatch(struct ofono_sms *sms, GSList *sms_list)
 {
 	GSList *l;
 	const struct sms *s;
+	struct ofono_uuid uuid;
 	enum sms_charset uninitialized_var(old_charset);
 	enum sms_class cls;
 	int srcport = -1;
@@ -1214,6 +1227,11 @@ static void sms_dispatch(struct ofono_sms *sms, GSList *sms_list)
 		}
 	}
 
+	if (!compute_incoming_msgid(sms_list, &uuid))
+		return;
+
+	s = sms_list->data;
+
 	/* Handle datagram */
 	if (old_charset == SMS_CHARSET_8BIT) {
 		unsigned char *buf;
@@ -1230,23 +1248,18 @@ static void sms_dispatch(struct ofono_sms *sms, GSList *sms_list)
 		if (!buf)
 			return;
 
-		dispatch_app_datagram(sms, dstport, srcport, buf, len);
+		dispatch_app_datagram(sms, &uuid, dstport, srcport, buf, len,
+					&s->deliver.oaddr, &s->deliver.scts);
 
 		g_free(buf);
 	} else {
-		struct ofono_uuid uuid;
 		char *message = sms_decode_text(sms_list);
 
 		if (!message)
 			return;
 
-		if (compute_incoming_msgid(sms_list, &uuid)) {
-			s = sms_list->data;
-
-			dispatch_text_message(sms, &uuid, message, cls,
-						&s->deliver.oaddr,
-						&s->deliver.scts);
-		}
+		dispatch_text_message(sms, &uuid, message, cls,
+					&s->deliver.oaddr, &s->deliver.scts);
 
 		g_free(message);
 	}
-- 
1.7.0.4

[PATCH 3/7] Add message agent interface documentation

[Aki Niemi]

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

---
 doc/message-api.txt |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/doc/message-api.txt b/doc/message-api.txt
index 9e8d02f..5bdc98d 100644
--- a/doc/message-api.txt
+++ b/doc/message-api.txt
@@ -38,6 +38,29 @@ Methods		dict GetProperties()
 			message could be queued successfully, this method
 			returns an object path to the created Message object.
 
+		void RegisterAgent(object path)
+
+			Registers an agent to handle received messages.
+
+			The object path defines the path of the agent that
+			will be called when either a text message, a vCard
+			object or a vCalendar object is ready to be
+			dispatched.
+
+			Possible Errors: [service].Error.InvalidArguments
+					 [service].Error.InvalidFormat
+					 [service].Error.InUse
+
+		void UnregisterAgent(object path)
+
+			Unregisters an agent.  If no agents exist that
+			can handle an incoming message, it will be
+			silently dropped.
+
+			Possible Errors: [service].Error.InvalidArguments
+					 [service].Error.InvalidFormat
+					 [service].Error.NotFound
+
 Signals		PropertyChanged(string name, variant value)
 
 			This signal indicates a changed value of the given
@@ -113,3 +136,31 @@ Properties	string State
 				"pending",
 				"sent",
 				"failed"
+
+MessageAgent Hierarchy
+===============
+
+Service		unique name
+Interface	org.ofono.MessageAgent
+Object path	freely definable
+
+Methods		void HandleMessage(string message, dict info)
+
+			New incoming SMS received.  Info has Sender
+			LocalSentTime, SentTime and ContentType information.
+			Sender address is given in string format.  LocalSentTime
+			and SentTime are given in string form using
+			ISO8601 format.
+
+			The ContentType parameter is a string that indicates the
+			content type of	the message.  Possible values are:
+
+				"text" - Plain text message
+				"vcard" - vCard object
+				"vcal"  - vCalendar object
+
+		void Release()
+
+			Agent is being released, possibly because of oFono
+			terminating, the MessageManager interface being
+			torn down or modem powering down.
-- 
1.7.0.4

Re: [PATCH 3/7] Add message agent interface documentation

[Denis Kenzior]

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

Hi Aki,

On 09/27/2010 11:54 AM, Aki Niemi wrote:
> ---
>  doc/message-api.txt |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 51 insertions(+), 0 deletions(-)
> 
> diff --git a/doc/message-api.txt b/doc/message-api.txt
> index 9e8d02f..5bdc98d 100644
> --- a/doc/message-api.txt
> +++ b/doc/message-api.txt
> @@ -38,6 +38,29 @@ Methods		dict GetProperties()
>  			message could be queued successfully, this method
>  			returns an object path to the created Message object.
>  
> +		void RegisterAgent(object path)
> +
> +			Registers an agent to handle received messages.
> +
> +			The object path defines the path of the agent that
> +			will be called when either a text message, a vCard
> +			object or a vCalendar object is ready to be
> +			dispatched.
> +
> +			Possible Errors: [service].Error.InvalidArguments
> +					 [service].Error.InvalidFormat
> +					 [service].Error.InUse

So me and Marcel had a very long talk about this.  We are basically fine
using an Agent for text messaging (however, we agreed that it isn't
really necessary, the signals and history cover all realistic use cases).

What we did not like is being able to register multiple agents.  So
instead we decided that using explicit APIs for this is better.  Please
see the detailed proposal in doc/smartmessaging-api.txt and
doc/pushnotification-api.txt.

The SmartMessaging API will handle reception of vCalendars and vCards.
This API also covers sending of these objects.

The PushNotification API will handle WAP PUSH requests.

We'd like to do all of this from a plugin, so the last thing to figure
out is the plumbing for plugins to be able to track the lifetime of the
sms atom and register the interface appropriately.

Regards,
-Denis

Re: [PATCH 3/7] Add message agent interface documentation

[Aki Niemi]

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

On Wed, 2010-09-29 at 04:27 +0200, ext Denis Kenzior wrote:
> Hi Aki,
> 
> On 09/27/2010 11:54 AM, Aki Niemi wrote:
> > ---
> >  doc/message-api.txt |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 51 insertions(+), 0 deletions(-)
> > 
> > diff --git a/doc/message-api.txt b/doc/message-api.txt
> > index 9e8d02f..5bdc98d 100644
> > --- a/doc/message-api.txt
> > +++ b/doc/message-api.txt
> > @@ -38,6 +38,29 @@ Methods		dict GetProperties()
> >  			message could be queued successfully, this method
> >  			returns an object path to the created Message object.
> >  
> > +		void RegisterAgent(object path)
> > +
> > +			Registers an agent to handle received messages.
> > +
> > +			The object path defines the path of the agent that
> > +			will be called when either a text message, a vCard
> > +			object or a vCalendar object is ready to be
> > +			dispatched.
> > +
> > +			Possible Errors: [service].Error.InvalidArguments
> > +					 [service].Error.InvalidFormat
> > +					 [service].Error.InUse
> 
> So me and Marcel had a very long talk about this.  We are basically fine
> using an Agent for text messaging (however, we agreed that it isn't
> really necessary, the signals and history cover all realistic use cases).

So you agree to an agent interface for receiving SMSs only? Because
there is no usable API to history, so you can't offer that as an
alternative.

My preference would've been a single agent for all text(ish) messages,
though.

There's also no need for separate signal handlers and history lookup, if
the agent interface also does spooling, or can otherwise guarantee the
agent always receives the messages.

Also, telepathy-ring won't have any use for call history; call history
is stored by the UIs along with the history of other types of calls. As
for messages, the history interface is not needed if the main message
interface works as it should.

> What we did not like is being able to register multiple agents.  So
> instead we decided that using explicit APIs for this is better.

Explicit API for what? Don't your proposal allow multiple agents as
well?

> Please
> see the detailed proposal in doc/smartmessaging-api.txt and
> doc/pushnotification-api.txt.

Honestly, if this is really a proposal, then shouldn't it have been sent
as an RFC patch to the mailing list? It's a bit hard to comment on it
now.

For new interfaces, I'd prefer all project participants to work
according to the same set of rules.

> The SmartMessaging API will handle reception of vCalendars and vCards.
> This API also covers sending of these objects.

Why array{byte} and not just string?

> The PushNotification API will handle WAP PUSH requests.
> 
> We'd like to do all of this from a plugin, so the last thing to figure
> out is the plumbing for plugins to be able to track the lifetime of the
> sms atom and register the interface appropriately.

Implement them as history plugins?

Cheers,
Aki

Re: [PATCH 3/7] Add message agent interface documentation

[Marcel Holtmann]

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

Hi Aki,

> > >  doc/message-api.txt |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 files changed, 51 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/doc/message-api.txt b/doc/message-api.txt
> > > index 9e8d02f..5bdc98d 100644
> > > --- a/doc/message-api.txt
> > > +++ b/doc/message-api.txt
> > > @@ -38,6 +38,29 @@ Methods		dict GetProperties()
> > >  			message could be queued successfully, this method
> > >  			returns an object path to the created Message object.
> > >  
> > > +		void RegisterAgent(object path)
> > > +
> > > +			Registers an agent to handle received messages.
> > > +
> > > +			The object path defines the path of the agent that
> > > +			will be called when either a text message, a vCard
> > > +			object or a vCalendar object is ready to be
> > > +			dispatched.
> > > +
> > > +			Possible Errors: [service].Error.InvalidArguments
> > > +					 [service].Error.InvalidFormat
> > > +					 [service].Error.InUse
> > 
> > So me and Marcel had a very long talk about this.  We are basically fine
> > using an Agent for text messaging (however, we agreed that it isn't
> > really necessary, the signals and history cover all realistic use cases).
> 
> So you agree to an agent interface for receiving SMSs only? Because
> there is no usable API to history, so you can't offer that as an
> alternative.

please see my comments about the history integration. That is something
we have to tackle pretty soon the right way. Currently it is just
hacking around it.

So we need to have an oFono history plugin that talks to tracker
directly via a Tracker API on the system D-Bus. This needs to be done
similar on how we handle the integration of ConnMan and pacrunner for
the proxy configuration.

The oFono history plugin needs to track when Tracker becomes available
and if, then send the spooled history and keep sending it until Tracker
gets off the bus.

This way we have Tracker having always the history. And Tracker can be
autostarted via D-Bus activation or whatever method when the UI actually
becomes available. And the UI should retrieve SMS and call history
directly from Tracker.

With this design, we can also properly secure the SMS and call history
and ensure that nothing gets into the wrong hands.

And if the preferred storage method is not Tracker, then that is fine as
well. It can be just anther history plugin talking to a different
system.

> My preference would've been a single agent for all text(ish) messages,
> though.
> 
> There's also no need for separate signal handlers and history lookup, if
> the agent interface also does spooling, or can otherwise guarantee the
> agent always receives the messages.

I just don't see text message going directly to the SMS application.
That application has to integrate with the storage.

The UI needs to do too much work if it has to track SMS coming from an
agent, SMS inside the history and have to keep matching them up for the
user to display them nicely.

Even for outgoing SMS the UI can just listen and follow Tracker's
information about the SMS. Same as for incoming ones.

So in the end the signals are just here for some simple easy
demonstration. And we could actually remove them. Beside the
ImmediateMessage one of course. Since that doesn't need to be spooled
anyway. If you miss it then it is too late already anyway.

> Also, telepathy-ring won't have any use for call history; call history
> is stored by the UIs along with the history of other types of calls. As
> for messages, the history interface is not needed if the main message
> interface works as it should.

I think this is wrong. Call history needs to be stored in Tracker and
then either the UI or Telepathy can access it via Tracker.

And yes, the history of SMS is needed. See my comments above.

> > What we did not like is being able to register multiple agents.  So
> > instead we decided that using explicit APIs for this is better.
> 
> Explicit API for what? Don't your proposal allow multiple agents as
> well?

How many applications do you expect to handle vCards and vCalendars. I
see one and that is the contacts application.

Same for WAP Push notifications. That is either the separate WAP Push
daemon or the MMS application/stack.

Having multiple agents sounds nice on paper, but I don't see the real
use case inside a system. You really don't wanna receive a vCard or
vCalendar twice. That would confuse the user.

> > Please
> > see the detailed proposal in doc/smartmessaging-api.txt and
> > doc/pushnotification-api.txt.
> 
> Honestly, if this is really a proposal, then shouldn't it have been sent
> as an RFC patch to the mailing list? It's a bit hard to comment on it
> now.
> 
> For new interfaces, I'd prefer all project participants to work
> according to the same set of rules.

Next time we will. This was an oversight on my part. I was discussing
this with Denis F2F in long details. Going forth and back between
different proposals on the table. This is the one that might sense right
away.

The API is 100% intuitive for developers having to write the vCard and
vCalendar support and also to integrate with WAP Push notifications.
They exactly what to expect and they know exactly what they get.

> > The SmartMessaging API will handle reception of vCalendars and vCards.
> > This API also covers sending of these objects.
> 
> Why array{byte} and not just string?

So if I remember this correctly then we don't know the encoding of the
vCard/vCalendar. For all intense and purposes it could be some weird
encoding that is not UTF-8. And if we don't know the encoding, we can't
convert it. If you tell me that we know the encoding, then we can change
that to a string.

> > The PushNotification API will handle WAP PUSH requests.
> > 
> > We'd like to do all of this from a plugin, so the last thing to figure
> > out is the plumbing for plugins to be able to track the lifetime of the
> > sms atom and register the interface appropriately.
> 
> Implement them as history plugins?

As history plugin or some SMS specific plugin or something else. Doesn't
matter. I am pretty much convinced that we can handle this way better
and cleaner inside plugins than trying to have one fits all D-Bus API.
We just don't have enough information on the use cases of the API to
make it that generic.

We know our use cases and for these use we write plugins that expose a
dedicated API for that use that. That is easy and simpler. Only try to
solve something that you do understand. And right now we have an idea
about Smart Messaging and the WAP Push stuff. Everything else is kinda a
bit blur in form of real use cases and really understanding is
completely.

Regards

Marcel

Re: [PATCH 3/7] Add message agent interface documentation

[Aki Niemi]

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

Hi Marcel,

2010/9/29 Marcel Holtmann <marcel@holtmann.org>:
> I just don't see text message going directly to the SMS application.
> That application has to integrate with the storage.
>
> The UI needs to do too much work if it has to track SMS coming from an
> agent, SMS inside the history and have to keep matching them up for the
> user to display them nicely.
>
> Even for outgoing SMS the UI can just listen and follow Tracker's
> information about the SMS. Same as for incoming ones.
>
> So in the end the signals are just here for some simple easy
> demonstration. And we could actually remove them. Beside the
> ImmediateMessage one of course. Since that doesn't need to be spooled
> anyway. If you miss it then it is too late already anyway.

Telepathy-ring is the agent here. It needs to receive incoming SMSs
and send outgoing ones. It needs to receive vCard and vCalendar
objects as well as sending them. It needs to track outgoing messages,
and map their statuses to Telepathy.

History of messages (and calls) is stored elsewhere, most likely
tracker by the call and messaging framework. What telepathy-ring needs
is a simple and reliable way of talking to oFono. This is what my
patches were trying to provide.

I don't understand what the hard to understand part here is. We won't
use tracker for telepathy-ring to talk to oFono, that is for sure.
Sure, I can rewrite these patches as a plugin, but that doesn't make
it any simpler or better. Just different and more complicated in fact.
For instance, now telepathy-ring would need to register two agents
instead of one.

> And yes, the history of SMS is needed. See my comments above.

No, the agent (telepathy-ring here) doesn't need to use history, if
the agent interface works as it should. That is, messages get queued
if no agent is there to handle them, or something similar to that
effect.

>> > What we did not like is being able to register multiple agents.  So
>> > instead we decided that using explicit APIs for this is better.
>>
>> Explicit API for what? Don't your proposal allow multiple agents as
>> well?
>
> How many applications do you expect to handle vCards and vCalendars. I
> see one and that is the contacts application.

Still, your proposal allows multiple agents to register, same as mine.

> Same for WAP Push notifications. That is either the separate WAP Push
> daemon or the MMS application/stack.
>
> Having multiple agents sounds nice on paper, but I don't see the real
> use case inside a system. You really don't wanna receive a vCard or
> vCalendar twice. That would confuse the user.

If this is the case, then I don't see a real need for the
RegisterAgent() and UnregisterAgent() methods at all. Static
registration by providing configuration files at a well known location
works much better, since then package management can then enforce
singleton status for such agents.

Cheers,
Aki

Re: [PATCH 3/7] Add message agent interface documentation

[Marcel Holtmann]

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

Hi Aki,

> > I just don't see text message going directly to the SMS application.
> > That application has to integrate with the storage.
> >
> > The UI needs to do too much work if it has to track SMS coming from an
> > agent, SMS inside the history and have to keep matching them up for the
> > user to display them nicely.
> >
> > Even for outgoing SMS the UI can just listen and follow Tracker's
> > information about the SMS. Same as for incoming ones.
> >
> > So in the end the signals are just here for some simple easy
> > demonstration. And we could actually remove them. Beside the
> > ImmediateMessage one of course. Since that doesn't need to be spooled
> > anyway. If you miss it then it is too late already anyway.
> 
> Telepathy-ring is the agent here. It needs to receive incoming SMSs
> and send outgoing ones. It needs to receive vCard and vCalendar
> objects as well as sending them. It needs to track outgoing messages,
> and map their statuses to Telepathy.

and how does it track outgoing messages? It can only tack outgoing
messages that it sends itself. How can it track outgoing messages from
other pieces of the system?

> History of messages (and calls) is stored elsewhere, most likely
> tracker by the call and messaging framework. What telepathy-ring needs
> is a simple and reliable way of talking to oFono. This is what my
> patches were trying to provide.
> 
> I don't understand what the hard to understand part here is. We won't
> use tracker for telepathy-ring to talk to oFono, that is for sure.
> Sure, I can rewrite these patches as a plugin, but that doesn't make
> it any simpler or better. Just different and more complicated in fact.
> For instance, now telepathy-ring would need to register two agents
> instead of one.

I don't think that just stuffing everything in telepathy-ring is a good
idea. You can of course do that, but that is not the only piece of
software in the system using oFono.

oFono is not a system where its only purpose it to make Telepathy happy.
It can be used perfectly fine without Telepathy and that is a valid use
case for oFono.

If you wanna bloat Telepathy and telepathy-ring into something huge that
is suppose to do everything, then that is fine, but it is not done at
the expense of oFono and other potential users of the oFono API.

> > And yes, the history of SMS is needed. See my comments above.
> 
> No, the agent (telepathy-ring here) doesn't need to use history, if
> the agent interface works as it should. That is, messages get queued
> if no agent is there to handle them, or something similar to that
> effect.

How do you handle outgoing messages? Especially outgoing messages send
by other pieces in your system.

> >> > What we did not like is being able to register multiple agents.  So
> >> > instead we decided that using explicit APIs for this is better.
> >>
> >> Explicit API for what? Don't your proposal allow multiple agents as
> >> well?
> >
> > How many applications do you expect to handle vCards and vCalendars. I
> > see one and that is the contacts application.
> 
> Still, your proposal allows multiple agents to register, same as mine.

Either way is fine here. This will be figured out later on. Maybe
multiple agents will make actually sense in some cases. We will see.

> > Same for WAP Push notifications. That is either the separate WAP Push
> > daemon or the MMS application/stack.
> >
> > Having multiple agents sounds nice on paper, but I don't see the real
> > use case inside a system. You really don't wanna receive a vCard or
> > vCalendar twice. That would confuse the user.
> 
> If this is the case, then I don't see a real need for the
> RegisterAgent() and UnregisterAgent() methods at all. Static
> registration by providing configuration files at a well known location
> works much better, since then package management can then enforce
> singleton status for such agents.

And that is what we will be not doing. I already told you that we tried
this with BlueZ and it failed horrible. I am not repeating that mistake.
The blind usage of D-Bus activation is not a good idea. In addition you
have the system vs session bus boundaries.

Regards

Marcel

[PATCH 4/7] Add message agent interface definition

[Aki Niemi]

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

---
 include/dbus.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/dbus.h b/include/dbus.h
index 363bae2..bf945f1 100644
--- a/include/dbus.h
+++ b/include/dbus.h
@@ -42,6 +42,7 @@ extern "C" {
 #define OFONO_CONNECTION_MANAGER_INTERFACE "org.ofono.ConnectionManager"
 #define OFONO_MESSAGE_MANAGER_INTERFACE "org.ofono.MessageManager"
 #define OFONO_MESSAGE_INTERFACE "org.ofono.Message"
+#define OFONO_MESSAGE_AGENT_INTERFACE "org.ofono.MessageAgent"
 #define OFONO_MESSAGE_WAITING_INTERFACE "org.ofono.MessageWaiting"
 #define OFONO_NETWORK_REGISTRATION_INTERFACE "org.ofono.NetworkRegistration"
 #define OFONO_NETWORK_OPERATOR_INTERFACE "org.ofono.NetworkOperator"
-- 
1.7.0.4

[PATCH 5/7] Add message agent implementation

[Aki Niemi]

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

---
 Makefile.am    |    2 +-
 src/smsagent.c |  344 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/smsagent.h |   56 +++++++++
 3 files changed, 401 insertions(+), 1 deletions(-)
 create mode 100644 src/smsagent.c
 create mode 100644 src/smsagent.h

diff --git a/Makefile.am b/Makefile.am
index 73de359..0d87006 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -296,7 +296,7 @@ src_ofonod_SOURCES = $(gdbus_sources) $(builtin_sources) src/ofono.ver \
 			src/gprs.c src/idmap.h src/idmap.c \
 			src/radio-settings.c src/stkutil.h src/stkutil.c \
 			src/nettime.c src/stkagent.c src/stkagent.h \
-			src/simfs.c src/simfs.h
+			src/simfs.c src/simfs.h src/smsagent.c src/smsagent.h
 
 src_ofonod_LDADD = $(builtin_libadd) @GLIB_LIBS@ @DBUS_LIBS@ @CAPNG_LIBS@ -ldl
 
diff --git a/src/smsagent.c b/src/smsagent.c
new file mode 100644
index 0000000..dc08e0a
--- /dev/null
+++ b/src/smsagent.c
@@ -0,0 +1,344 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2009-2010 Nokia Corporation and/or its subsidiary(-ies).
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#define _GNU_SOURCE
+#include <stdint.h>
+#include <string.h>
+#include <errno.h>
+
+#include <glib.h>
+#include <gdbus.h>
+
+#include "ofono.h"
+
+#include "common.h"
+#include "smsagent.h"
+
+enum sms_agent_port {
+	SMS_AGENT_PORT_VCARD = 9204,
+	SMS_AGENT_PORT_VCAL = 9205,
+	SMS_AGENT_PORT_VCARD_S = 9206,		/* vCard secure?? */
+	SMS_AGENT_PORT_VCAL_S = 9207,		/* vCal secure?? */
+};
+
+struct sms_agent {
+	char *path;
+	char *name;
+	guint disconnect_watch;
+	sms_agent_remove_cb remove_cb;
+	void *remove_data;
+	GSList *reqs;
+};
+
+struct sms_agent_request {
+	struct sms_agent *agent;
+	DBusMessage *msg;
+	DBusPendingCall *call;
+	sms_agent_dispatch_cb dispatch_cb;
+	void *dispatch_data;
+	ofono_destroy_func destroy;
+};
+
+static const char *sms_agent_port_to_type(int port)
+{
+	switch (port) {
+	case SMS_AGENT_PORT_VCARD:
+	case SMS_AGENT_PORT_VCARD_S:
+		return "vcard";
+
+	case SMS_AGENT_PORT_VCAL:
+	case SMS_AGENT_PORT_VCAL_S:
+		return "vcalendar";
+	}
+
+	return NULL;
+}
+
+static struct sms_agent_request *sms_agent_request_create(struct sms_agent *agent,
+						sms_agent_dispatch_cb cb,
+						void *user_data,
+						ofono_destroy_func destroy)
+{
+	struct sms_agent_request *req;
+
+	req = g_try_new0(struct sms_agent_request, 1);
+	if (!req)
+		return NULL;
+
+	req->agent = agent;
+	req->dispatch_cb = cb;
+	req->dispatch_data = user_data;
+	req->destroy = destroy;
+
+	return req;
+}
+
+static void sms_agent_request_destroy(struct sms_agent_request *req)
+{
+	if (req->msg) {
+		dbus_message_unref(req->msg);
+		req->msg = NULL;
+	}
+
+	if (req->call) {
+		dbus_pending_call_unref(req->call);
+		req->call = NULL;
+	}
+
+	if (req->destroy)
+		req->destroy(req->dispatch_data);
+
+	g_free(req);
+}
+
+static void sms_agent_request_cancel(struct sms_agent_request *req)
+{
+	if (!req->call)
+		return;
+
+	dbus_pending_call_cancel(req->call);
+
+	sms_agent_request_destroy(req);
+}
+
+static void sms_agent_send_noreply(struct sms_agent *agent, const char *method)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	DBusMessage *message;
+
+	message = dbus_message_new_method_call(agent->name, agent->path,
+						OFONO_MESSAGE_AGENT_INTERFACE,
+							method);
+
+	if (!message)
+		return;
+
+	dbus_message_set_no_reply(message, TRUE);
+
+	DBG("Sending: '%s' to '%s' at '%s'", method, agent->name, agent->path);
+	g_dbus_send_message(conn, message);
+}
+
+static inline void sms_agent_send_release(struct sms_agent *agent)
+{
+	sms_agent_send_noreply(agent, "Release");
+}
+
+static void sms_agent_disconnect_cb(DBusConnection *conn, void *data)
+{
+	struct sms_agent *agent = data;
+
+	agent->disconnect_watch = 0;
+
+	if (agent->remove_cb)
+		agent->remove_cb(agent, agent->remove_data);
+}
+
+struct sms_agent *sms_agent_create(const char *path, const char *name,
+					sms_agent_remove_cb cb,
+					void *user_data)
+{
+	struct sms_agent *agent = g_try_new0(struct sms_agent, 1);
+	DBusConnection *conn = ofono_dbus_get_connection();
+
+	if (!agent)
+		return NULL;
+
+	agent->path = g_strdup(path);
+	agent->name = g_strdup(name);
+	agent->remove_cb = cb;
+	agent->remove_data = user_data;
+
+	agent->disconnect_watch = g_dbus_add_disconnect_watch(conn, name,
+							sms_agent_disconnect_cb,
+							agent, NULL);
+	return agent;
+}
+
+void sms_agent_destroy(struct sms_agent *agent)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+
+	if (!agent)
+		return;
+
+	if (agent->disconnect_watch) {
+		sms_agent_send_release(agent);
+
+		g_dbus_remove_watch(conn, agent->disconnect_watch);
+		agent->disconnect_watch = 0;
+	}
+
+	g_slist_foreach(agent->reqs, (GFunc)sms_agent_request_cancel, NULL);
+	g_slist_free(agent->reqs);
+
+	g_free(agent->path);
+	g_free(agent->name);
+	g_free(agent);
+}
+
+ofono_bool_t sms_agent_matches(struct sms_agent *agent, const char *path,
+				const char *name)
+{
+	return (path && strcmp(agent->path, path) == 0)
+		&& (name && strcmp(agent->name, name) == 0);
+}
+
+static enum sms_agent_result sms_agent_check_error(struct sms_agent *agent,
+							DBusMessage *reply)
+{
+	enum sms_agent_result result = SMS_AGENT_RESULT_FAILED;
+	DBusError err;
+
+	dbus_error_init(&err);
+
+	if (!dbus_set_error_from_message(&err, reply))
+		return SMS_AGENT_RESULT_OK;
+
+	if (g_str_equal(err.name, DBUS_ERROR_NO_REPLY))
+		result = SMS_AGENT_RESULT_TIMEOUT;
+
+	ofono_debug("MessageAgent %s replied with error %s, %s",
+			agent->name, err.name, err.message);
+
+	dbus_error_free(&err);
+	return result;
+}
+
+static void sms_agent_dispatch_reply_cb(DBusPendingCall *call, void *data)
+{
+	struct sms_agent_request *req = data;
+	struct sms_agent *agent = req->agent;
+	sms_agent_dispatch_cb cb = req->dispatch_cb;
+	void *dispatch_data = req->dispatch_data;
+	DBusMessage *reply = dbus_pending_call_steal_reply(req->call);
+	enum sms_agent_result result;
+
+	result = sms_agent_check_error(agent, reply);
+
+	agent->reqs = g_slist_remove(agent->reqs, req);
+	sms_agent_request_destroy(req);
+
+	if (cb)
+		cb(agent, result, dispatch_data);
+
+	dbus_message_unref(reply);
+}
+
+static int sms_agent_dispatch(struct sms_agent *agent, const char *from,
+				const struct tm *remote, const struct tm *local,
+				const char *message, const char *content_type,
+				sms_agent_dispatch_cb cb, void *user_data,
+				ofono_destroy_func destroy)
+{
+	struct sms_agent_request *req;
+	DBusConnection *conn = ofono_dbus_get_connection();
+	DBusMessageIter iter;
+	DBusMessageIter dict;
+	char buf[128];
+	const char *str = buf;
+
+	req = sms_agent_request_create(agent, cb, user_data, destroy);
+	if (!req)
+		return -ENOMEM;
+
+	req->msg = dbus_message_new_method_call(agent->name, agent->path,
+						OFONO_MESSAGE_AGENT_INTERFACE,
+						"HandleMessage");
+	if (!req->msg) {
+		sms_agent_request_destroy(req);
+		return -ENOMEM;
+	}
+
+	dbus_message_iter_init_append(req->msg, &iter);
+
+	dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &message);
+
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+					OFONO_PROPERTIES_ARRAY_SIGNATURE,
+					&dict);
+
+	strftime(buf, 127, "%Y-%m-%dT%H:%M:%S%z", local);
+	buf[127] = '\0';
+	ofono_dbus_dict_append(&dict, "LocalSentTime", DBUS_TYPE_STRING, &str);
+
+	strftime(buf, 127, "%Y-%m-%dT%H:%M:%S%z", remote);
+	buf[127] = '\0';
+	ofono_dbus_dict_append(&dict, "SentTime", DBUS_TYPE_STRING, &str);
+
+	ofono_dbus_dict_append(&dict, "Sender", DBUS_TYPE_STRING, &from);
+	ofono_dbus_dict_append(&dict, "ContentType", DBUS_TYPE_STRING,
+				&content_type);
+
+	dbus_message_iter_close_container(&iter, &dict);
+
+	if (!dbus_connection_send_with_reply(conn, req->msg, &req->call, -1)) {
+		ofono_error("Sending D-Bus method failed");
+		sms_agent_request_destroy(req);
+		return -EIO;
+	}
+
+	agent->reqs = g_slist_append(agent->reqs, req);
+
+	dbus_pending_call_set_notify(req->call, sms_agent_dispatch_reply_cb,
+					req, NULL);
+
+	return 0;
+}
+
+int sms_agent_dispatch_text(struct sms_agent *agent, const char *from,
+				const struct tm *remote_sent_time,
+				const struct tm *local_sent_time,
+				const char *message, sms_agent_dispatch_cb cb,
+				void *user_data, ofono_destroy_func destroy)
+{
+	return sms_agent_dispatch(agent, from, remote_sent_time, local_sent_time,
+					message, "text", cb, user_data,
+					destroy);
+}
+
+int sms_agent_dispatch_datagram(struct sms_agent *agent, const char *from,
+				const struct tm *remote_sent_time,
+				const struct tm *local_sent_time,
+				int dst_port, int src_port,
+				const unsigned char *buffer,
+				unsigned int len,
+				sms_agent_dispatch_cb cb, void *user_data,
+				ofono_destroy_func destroy)
+{
+	char message[len + 1];
+	const char *content_type;
+
+	content_type = sms_agent_port_to_type(dst_port);
+	if (!content_type)
+		return -ENOTSUP;
+
+	memcpy(message, buffer, len);
+	message[len] = '\0';
+
+	return sms_agent_dispatch(agent, from, remote_sent_time, local_sent_time,
+					message, content_type, cb, user_data,
+					destroy);
+}
diff --git a/src/smsagent.h b/src/smsagent.h
new file mode 100644
index 0000000..40637d9
--- /dev/null
+++ b/src/smsagent.h
@@ -0,0 +1,56 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2009-2010 Nokia Corporation and/or its subsidiary(-ies).
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+struct sms_agent;
+
+enum sms_agent_result {
+	SMS_AGENT_RESULT_OK = 0,
+	SMS_AGENT_RESULT_FAILED,
+	SMS_AGENT_RESULT_TIMEOUT,
+};
+
+typedef void (*sms_agent_remove_cb) (struct sms_agent *agent, void *data);
+
+typedef void (*sms_agent_dispatch_cb) (struct sms_agent *agent,
+					enum sms_agent_result result,
+					void *data);
+
+struct sms_agent *sms_agent_create(const char *path, const char *name,
+					sms_agent_remove_cb cb, void *data);
+
+void sms_agent_destroy(struct sms_agent *agent);
+
+ofono_bool_t sms_agent_matches(struct sms_agent *agent, const char *path,
+				const char *name);
+
+int sms_agent_dispatch_text(struct sms_agent *agent, const char *from,
+				const struct tm *remote_sent_time,
+				const struct tm *local_sent_time,
+				const char *message, sms_agent_dispatch_cb cb,
+				void *user_data, ofono_destroy_func destroy);
+
+int sms_agent_dispatch_datagram(struct sms_agent *agent, const char *from,
+				const struct tm *remote_sent_time,
+				const struct tm *local_sent_time,
+				int dst_port, int src_port,
+				const unsigned char *buffer, unsigned int len,
+				sms_agent_dispatch_cb cb, void *user_data,
+				ofono_destroy_func destroy);
-- 
1.7.0.4

[PATCH 6/7] Add agent interface to sms atom

[Aki Niemi]

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

---
 src/sms.c |  146 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 143 insertions(+), 3 deletions(-)

diff --git a/src/sms.c b/src/sms.c
index e6a2f9c..d86b5ec 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -38,6 +38,8 @@
 #include "smsutil.h"
 #include "storage.h"
 
+#include "smsagent.h"
+
 #define uninitialized_var(x) x = x
 
 #define MESSAGE_MANAGER_FLAG_CACHED 0x1
@@ -76,6 +78,7 @@ struct ofono_sms {
 	GKeyFile *settings;
 	char *imsi;
 	int bearer;
+	GSList *agents;
 	const struct ofono_sms_driver *driver;
 	void *driver_data;
 	struct ofono_atom *atom;
@@ -631,6 +634,93 @@ static DBusMessage *sms_set_property(DBusConnection *conn, DBusMessage *msg,
 	return __ofono_error_invalid_args(msg);
 }
 
+static void sms_agent_removed(struct sms_agent *agent, void *data)
+{
+	struct ofono_sms *sms = data;
+
+	DBG("Agent %p disconnected without unregistering", agent);
+
+	sms->agents = g_slist_remove(sms->agents, agent);
+
+	sms_agent_destroy(agent);
+}
+
+static DBusMessage *sms_register_agent(DBusConnection *conn,
+					DBusMessage *msg, void *data)
+{
+	struct ofono_sms *sms = data;
+	struct sms_agent *agent;
+	GSList *l;
+	const char *path;
+	const char *name;
+
+	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
+					DBUS_TYPE_INVALID))
+		return __ofono_error_invalid_args(msg);
+
+	if (!__ofono_dbus_valid_object_path(path))
+		return __ofono_error_invalid_format(msg);
+
+	name = dbus_message_get_sender(msg);
+
+	for (l = sms->agents; l; l = l->next) {
+		struct sms_agent *old = l->data;
+
+		if (sms_agent_matches(old, path, name))
+			return __ofono_error_in_use(msg);
+	}
+
+	agent = sms_agent_create(path, name, sms_agent_removed, sms);
+	if (!agent)
+		return __ofono_error_failed(msg);
+
+	DBG("Registering agent %s at %s (%p)", name, path, agent);
+
+	sms->agents = g_slist_append(sms->agents, agent);
+
+	return dbus_message_new_method_return(msg);
+}
+
+static DBusMessage *sms_unregister_agent(DBusConnection *conn,
+						DBusMessage *msg, void *data)
+{
+	struct ofono_sms *sms = data;
+	GSList *l;
+	const char *path;
+	const char *name;
+	int removed = 0;
+
+	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
+					DBUS_TYPE_INVALID))
+		return __ofono_error_invalid_args(msg);
+
+	if (!__ofono_dbus_valid_object_path(path))
+		return __ofono_error_invalid_format(msg);
+
+	name = dbus_message_get_sender(msg);
+
+	DBG("Finding %s at %s", name, path);
+
+	for (l = sms->agents; l; l = l->next) {
+		struct sms_agent *agent = l->data;
+
+		if (!sms_agent_matches(agent, path, name))
+			continue;
+
+		DBG("Unregistering agent %s at %s, (%p)", name, path, agent);
+
+		sms->agents = g_slist_remove(sms->agents, agent);
+		sms_agent_destroy(agent);
+		removed = 1;
+		break;
+	}
+
+	if (!removed)
+		return __ofono_error_not_found(msg);
+
+	return dbus_message_new_method_return(msg);
+}
+
 /*
  * Destroy/release the contents of a 'struct tx_queue_entry'
  *
@@ -1025,6 +1115,8 @@ static GDBusMethodTable sms_manager_methods[] = {
 	{ "SendMessage",      "ss",  "o",             sms_send_message,
 						G_DBUS_METHOD_FLAG_ASYNC },
 	{ "GetMessages",       "",    "a(oa{sv})",    sms_get_messages },
+	{ "RegisterAgent",     "o",   "",             sms_register_agent },
+	{ "UnregisterAgent",   "o",   "",             sms_unregister_agent },
 	{ }
 };
 
@@ -1064,6 +1156,21 @@ static gboolean compute_incoming_msgid(GSList *sms_list,
 	return TRUE;
 }
 
+static void agent_dispatch_cb(struct sms_agent *agent,
+				enum sms_agent_result result,
+				void *data)
+{
+	struct ofono_sms *sms = data;
+
+	if (result == SMS_AGENT_RESULT_OK)
+		return;
+
+	ofono_error("Dispatching agent %p failed, removing", agent);
+
+	sms->agents = g_slist_remove(sms->agents, agent);
+	sms_agent_destroy(agent);
+}
+
 static void dispatch_app_datagram(struct ofono_sms *sms,
 					const struct ofono_uuid *uuid,
 					int dst, int src,
@@ -1076,12 +1183,27 @@ static void dispatch_app_datagram(struct ofono_sms *sms,
 	time_t ts;
 	struct tm remote;
 	struct tm local;
+	GSList *l;
 
 	ts = sms_scts_to_time(scts, &remote);
 	localtime_r(&ts, &local);
 
+	for (l = sms->agents; l; l = l->next) {
+		struct sms_agent *agent = l->data;
+		int ret;
+
+		ret = sms_agent_dispatch_datagram(agent, sender, &remote, &local,
+							dst, src, buf, len,
+							agent_dispatch_cb, sms,
+							NULL);
+		if (ret < 0)
+			ofono_error("Unable to dispatch datagram to agent %p: "
+					"%d (%s)", agent, ret, strerror(-ret));
+	}
+
 	__ofono_history_datagram_received(modem, uuid, sender, dst, src,
 						&remote, &local, buf, len);
+
 }
 
 static void dispatch_text_message(struct ofono_sms *sms,
@@ -1103,6 +1225,7 @@ static void dispatch_text_message(struct ofono_sms *sms,
 	struct tm remote;
 	struct tm local;
 	const char *str = buf;
+	GSList *l;
 
 	if (!message)
 		return;
@@ -1144,9 +1267,23 @@ static void dispatch_text_message(struct ofono_sms *sms,
 
 	g_dbus_send_message(conn, signal);
 
-	if (cls != SMS_CLASS_0)
-		__ofono_history_sms_received(modem, uuid, str,
-						&remote, &local, message);
+	if (cls == SMS_CLASS_0)
+		return;
+
+	for (l = sms->agents; l; l = l->next) {
+		struct sms_agent *agent = l->data;
+		int ret;
+
+		ret = sms_agent_dispatch_text(agent, str, &remote, &local,
+						message, agent_dispatch_cb,
+						sms, NULL);
+		if (ret < 0)
+			ofono_error("Unable to dispatch text to agent %p",
+					agent);
+	}
+
+	__ofono_history_sms_received(modem, uuid, str, &remote, &local,
+					message);
 }
 
 static void sms_dispatch(struct ofono_sms *sms, GSList *sms_list)
@@ -1591,6 +1728,9 @@ static void sms_remove(struct ofono_atom *atom)
 		sms->sr_assembly = NULL;
 	}
 
+	g_slist_foreach(sms->agents, (GFunc)sms_agent_destroy, NULL);
+	g_slist_free(sms->agents);
+
 	g_free(sms);
 }
 
-- 
1.7.0.4

[PATCH 7/7] Add test sms agent implementation

[Aki Niemi]

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

---
 test/sms-agent |   59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 59 insertions(+), 0 deletions(-)
 create mode 100755 test/sms-agent

diff --git a/test/sms-agent b/test/sms-agent
new file mode 100755
index 0000000..58b33c3
--- /dev/null
+++ b/test/sms-agent
@@ -0,0 +1,59 @@
+#!/usr/bin/python
+
+import gobject
+
+import sys
+import dbus
+import dbus.service
+import dbus.mainloop.glib
+
+class SmsAgent(dbus.service.Object):
+
+	def __init__(self, bus, path):
+		dbus.service.Object.__init__(self, bus, path)
+
+		print "Started agent %s at %s" % (bus, path)
+
+	@dbus.service.method("org.ofono.MessageAgent",
+				in_signature="", out_signature="")
+	def Release(self):
+		print "Release, quitting"
+		mainloop.quit()
+
+	@dbus.service.method("org.ofono.MessageAgent",
+				in_signature="sa{sv}", out_signature="")
+	def HandleMessage(self, message, props):
+		print "From: %s" % props["Sender"]
+		print "SentTime: %s" % props["SentTime"]
+		print "LocalSentTime: %s" % props["LocalSentTime"]
+		print "ContentType: %s" % props["ContentType"]
+		print "Text: %s" % message
+
+if __name__ == '__main__':
+	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()
+
+	for path, props in modems:
+
+		if "org.ofono.MessageManager" not in props["Interfaces"]:
+			continue
+
+		sms = dbus.Interface(bus.get_object('org.ofono', path),
+					'org.ofono.MessageManager')
+
+	path = "/test/agent"
+	agent = SmsAgent(bus, path)
+	sms.RegisterAgent(path)
+
+	mainloop = gobject.MainLoop()
+
+	try:
+		mainloop.run()
+	except KeyboardInterrupt:
+		sms.UnregisterAgent(path)
+		mainloop.run()
-- 
1.7.0.4