Opened 7 years ago

Closed 6 years ago

#4757 closed enhancement (fixed)

PS Activity objects: replace GetChannels() with something more useful

Reported by: smcv Owned by: gdesmott
Priority: normal Milestone: 8.2.0 (was Update.2)
Component: presence-service Version:
Keywords: 8.2.0:+ review+ Cc: olpc@…
Blocked By: Blocking: #5079
Deployments affected: Action Needed: finalize
Verified: no

Description

The code added in #4503 really wants to get the type, handle type and handle for each channel, without additional round trips. The ListChannels() method in Telepathy returns this information, but GetChannels() in the PS doesn't.

We should add a method (perhaps call it ListChannels() too?) that returns the connection object path, and an array of structs containing channel object paths, types, handle types and handles. The service name isn't strictly necessary, since Telepathy guarantees that you can derive it from the connection object path, but perhaps we should include that too for clarity.

Once this is implemented, we can remove numerous round trips in sugar.presence.

Attachments (1)

ps-4757.patch (2.5 KB) - added by gdesmott 7 years ago.

Download all attachments as: .zip

Change History (23)

comment:1 Changed 7 years ago by gdesmott

  • Cc gdesmott added

comment:2 Changed 7 years ago by jg

hmmm. Estimates on consequences? Urgency? Particularly on scaling?

comment:3 Changed 7 years ago by smcv

This is a refactoring task; low impact, low risk, low-urgency-but-would-be-nice. There is no effect on scalability.

I expect it to add around 10 lines of code to Presence Service and save between one and three D-Bus round trips per activity shared.

comment:4 Changed 7 years ago by jg

  • Milestone changed from Never Assigned to Update.2

Changed 7 years ago by gdesmott

comment:5 Changed 7 years ago by gdesmott

  • Keywords review? added

comment:6 Changed 7 years ago by gdesmott

See #5079 for the sugar patch using this new API.

comment:7 Changed 7 years ago by smcv

  • Cc olpc@… added; morgs gdesmott removed
  • Owner changed from smcv to gdesmott

Reassigning to gdesmott, Cc the Collabora OLPC team, since I'm not currently working on OLPC.

comment:8 Changed 7 years ago by morgs

  • Keywords review+ added; review? removed

comment:9 Changed 7 years ago by gdesmott

comment:10 Changed 6 years ago by morgs

Still review+ from me.

comment:11 Changed 6 years ago by marco

  • Keywords 8.0.2:+ added

Is this in?

comment:12 Changed 6 years ago by gdesmott

  • Action Needed set to never set

It's not. I'll upgrade the branch and merge it soon.

comment:13 Changed 6 years ago by gdesmott

  • Blocking 5079 added

comment:13 Changed 6 years ago by gdesmott

  • Blocking 5079 removed

I rebase my branch to master: https://dev.laptop.org/git?p=users/guillaume/presence-service;a=shortlog;h=4757-rebased

If no objection, I'll merge it tomorrow.

comment:14 Changed 6 years ago by gdesmott

  • Blocking 5079 added

(In #5079) I rebased my branch to master: https://dev.laptop.org/git?p=users/guillaume/sugar-toolkit;a=shortlog;h=5079-rebased

Will consider merging once #4757 is fixed.

comment:16 Changed 6 years ago by morgs

  • Keywords 8.2.0:+ added; 8.0.2:+ removed

Looks like the keyword should be 8.2.0:+ not 8.0.2:+

comment:17 Changed 6 years ago by gdesmott

Patch merged to PS HEAD. Will be included in the next release.

comment:18 follow-up: Changed 6 years ago by daf

Can you document the new method on on the wiki?

http://wiki.laptop.org/go/Presence_Service_DBus_API

comment:19 Changed 6 years ago by morgs

Included in release 0.81.3

comment:20 in reply to: ↑ 18 Changed 6 years ago by gdesmott

Replying to daf:

Can you document the new method on on the wiki?

http://wiki.laptop.org/go/Presence_Service_DBus_API

Oh, forgot about that. Good catch.

Updated.

comment:21 Changed 6 years ago by gdesmott

  • Action Needed changed from never set to finalize

comment:22 Changed 6 years ago by gdesmott

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.