Bug 29377 - Implement Channel.Interface.Messages
Summary: Implement Channel.Interface.Messages
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: rakia (show other bugs)
Version: unspecified
Hardware: Other All
: medium enhancement
Assignee: Mikhail Zabaluev
QA Contact: Telepathy bugs list
URL: http://git.collabora.co.uk/?p=user/ca...
Whiteboard:
Keywords: patch
Depends on:
Blocks: 29376
  Show dependency treegraph
 
Reported: 2010-08-03 05:09 UTC by Guillaume Desmottes
Modified: 2010-11-03 08:30 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description Guillaume Desmottes 2010-08-03 05:09:39 UTC
At some point we'd like to make the Messages interface mandatory (bug # 29376) but sofiasip doesn't implement it yet.
Comment 1 Simon McVittie 2010-10-18 04:24:54 UTC
When implementing this, remember to add the three new immutable properties. (See Bug #30950.)
Comment 2 Guillaume Desmottes 2010-10-26 01:47:37 UTC
Any chance to have this soonish? One of our goal for Empathy 3.0 is to switch to Messages, but we can't easily do it until Messages became mandatory (bug #29376).

AFAIK, sofiasip is the last CM not implementing it.
If no one else has time to do it I can give it a try but I never hacked on tp-sofiasip.
Comment 3 Mikhail Zabaluev 2010-10-26 09:53:06 UTC
(In reply to comment #2)
> AFAIK, sofiasip is the last CM not implementing it.
> If no one else has time to do it I can give it a try but I never hacked on
> tp-sofiasip.

Feel free to go ahead. The text channel code in telepathy-sofiasip is rather basic.
While you are at it, you could try supporting arbitrary MIME types on body payloads, or at least non-multipart cases. But this is completely optional.
Comment 4 Guillaume Desmottes 2010-10-27 03:07:49 UTC
Here is a first version:
http://git.collabora.co.uk/?p=user/cassidy/telepathy-sofiasip;a=shortlog;h=refs/heads/messages-29377

I wasn't able to properly test it because for some reason my SIP provider (voipbuster) refuses to send messages.


test-message.py doesn't pass any more because ListPendingMessages() now returns a delivery report. Is that expected? Should I only create *fail* delivery report when TP_MESSAGE_SENDING_FLAG_REPORT_DELIVERY is set? But then SendError won't be fired...
Comment 5 Will Thompson 2010-10-27 03:42:55 UTC
(In reply to comment #4)
> test-message.py doesn't pass any more because ListPendingMessages() now returns
> a delivery report. Is that expected?

That sounds okay to me.

> Should I only create *fail* delivery
> report when TP_MESSAGE_SENDING_FLAG_REPORT_DELIVERY is set? But then SendError
> won't be fired...

No, you're allowed to signal delivery reports that weren't requested; the flags are really for the cases where you have to do extra work to get the reports.
Comment 6 Mikhail Zabaluev 2010-10-27 04:05:33 UTC
(In reply to comment #4)
> Here is a first version:
> http://git.collabora.co.uk/?p=user/cassidy/telepathy-sofiasip;a=shortlog;h=refs/heads/messages-29377

+  if (content_type == NULL || tp_strdiff (content_type, "text/plain"))

Do we get the "content-type" value normalized? If not, this should be a case-insensitive check.

I should perhaps add extraction of "message-sent" and an appropriate unique token from SIP headers, in a follow-up change.
Comment 7 Mikhail Zabaluev 2010-10-27 04:10:28 UTC
http://git.collabora.co.uk/?p=user/cassidy/telepathy-sofiasip;a=commitdiff;h=a5a7b7909152cde4d7f575ab88ddb41fb3cb3fb5

This part looks fine, except I can't see where the delivery token is exposed in the sent message.
Comment 8 Guillaume Desmottes 2010-10-27 07:38:57 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > test-message.py doesn't pass any more because ListPendingMessages() now returns
> > a delivery report. Is that expected?
> 
> That sounds okay to me.

Cool, I changed the test then.


(In reply to comment #6)
> (In reply to comment #4)
> > Here is a first version:
> > http://git.collabora.co.uk/?p=user/cassidy/telepathy-sofiasip;a=shortlog;h=refs/heads/messages-29377
> 
> +  if (content_type == NULL || tp_strdiff (content_type, "text/plain"))
> 
> Do we get the "content-type" value normalized? If not, this should be a
> case-insensitive check.

I think so; that's how it's done in all the other CM.

> I should perhaps add extraction of "message-sent" and an appropriate unique
> token from SIP headers, in a follow-up change.

Please feel free :)


(In reply to comment #7)
> http://git.collabora.co.uk/?p=user/cassidy/telepathy-sofiasip;a=commitdiff;h=a5a7b7909152cde4d7f575ab88ddb41fb3cb3fb5
> 
> This part looks fine, except I can't see where the delivery token is exposed in
> the sent message.

That's the 4th argument of tp_message_mixin_sent(). My patch was wrongly recorded; it should be clearer now:
http://git.collabora.co.uk/?p=user/cassidy/telepathy-sofiasip;a=commitdiff;h=13285043020ed6bdaa72b30bf65ac9b7d6b0207e
Comment 9 Mikhail Zabaluev 2010-10-27 10:25:45 UTC
(In reply to comment #8)
> > Do we get the "content-type" value normalized? If not, this should be a
> > case-insensitive check.
> 
> I think so; that's how it's done in all the other CM.

The specification does not demand it. Filed as bug #31170.

> > This part looks fine, except I can't see where the delivery token is exposed in
> > the sent message.
> 
> That's the 4th argument of tp_message_mixin_sent(). My patch was wrongly
> recorded; it should be clearer now:
> http://git.collabora.co.uk/?p=user/cassidy/telepathy-sofiasip;a=commitdiff;h=13285043020ed6bdaa72b30bf65ac9b7d6b0207e

OK.
Comment 10 Guillaume Desmottes 2010-11-03 01:28:05 UTC
Did you review the whole branch? Can I merge it?
Comment 11 Mikhail Zabaluev 2010-11-03 01:46:59 UTC
(In reply to comment #10)
> Did you review the whole branch? Can I merge it?

Yes; the only remaining issue (bug #31170) requires a spec decision, so it can be addressed later.
Comment 12 Guillaume Desmottes 2010-11-03 01:53:19 UTC
Merged to master; thanks!
Comment 13 Simon McVittie 2010-11-03 07:22:58 UTC
For the record, this will be in 0.7.0. Mikhail, is there anything blocking a 0.7.0 release?
Comment 14 Mikhail Zabaluev 2010-11-03 08:30:54 UTC
(In reply to comment #13)
> For the record, this will be in 0.7.0. Mikhail, is there anything blocking a
> 0.7.0 release?

I'm going to fix #30538 so we can release something reasonably compliant with the current specification.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.