Ticket #4757 (closed enhancement: fixed)

Opened 7 years ago

Last modified 6 years ago

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@…
Action Needed: finalize Verified: no
Deployments affected: Blocked By:
Blocking: #5079

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

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

Change History

  Changed 7 years ago by gdesmott

  • cc gdesmott added

  Changed 7 years ago by jg

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

  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.

  Changed 7 years ago by jg

  • milestone changed from Never Assigned to Update.2

Changed 7 years ago by gdesmott

  Changed 7 years ago by gdesmott

  • keywords review? added

  Changed 7 years ago by gdesmott

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

  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.

  Changed 7 years ago by morgs

  • keywords review+ added; review? removed

  Changed 6 years ago by gdesmott

I updated the patch and pushed it to a git branch: https://dev.laptop.org/git?p=users/guillaume/presence-service;a=shortlog;h=4757

  Changed 6 years ago by morgs

Still review+ from me.

  Changed 6 years ago by marco

  • keywords 8.0.2:+ added

Is this in?

  Changed 6 years ago by gdesmott

  • next_action set to never set

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

  Changed 6 years ago by gdesmott

  • blocking 5079 added

  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.

  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.

  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:+

  Changed 6 years ago by gdesmott

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

follow-up: ↓ 20   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

  Changed 6 years ago by morgs

Included in release 0.81.3

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.

  Changed 6 years ago by gdesmott

  • next_action changed from never set to finalize

  Changed 6 years ago by gdesmott

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