Review Request: Make the kded module use async dbus calls

[Martin Klapetek]

> On Aug. 1, 2012, 12:51 p.m., George Kiagiadakis wrote:
> > telepathy-mpris.cpp, lines 80-99
> > <http://git.reviewboard.kde.org/r/105813/diff/1/?file=75589#file75589line80>
> >
> >     This code could be improved a lot. First by just checking the "PlaybackStatus" property instead of all of them and second by caching properties for each player so that you don't have to fetch them all again for each player... I suspect it works, though, but it is a bit fragile imho.

> checking the "PlaybackStatus" property instead of all of them

True, but if the status is "Playing", we need to fetch the song metadata - do another dbus call and handle it. So I fetch it all at once, check for the playback status (again..) and if "Playing", the track data are available right away. That's why I did it like this. But I can change it to fetch only those properties really needed.

> caching properties for each player so that you don't have to fetch them all again for each player

Yeah that's a bit crap. We actually only need the PlaybackStatus and Metadata properties and caching these is no use.

--

I can rework this to check for PlaybackStatus in detectPlayers() and then fetch the track data only if the status is Playing. That way we will query all players for only one property and one more if it is playing. Then I can try changing the onPlayerSignalReceived(..) method to actually be aware which player emitted that signal and only query that one player for the song data.

Would that work?


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105813/#review16762
-----------------------------------------------------------


On Aug. 1, 2012, 12:41 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105813/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2012, 12:41 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> I reworked the kded module to use async dbus calls. Now when it detects a player, it queries for all its properties (to save some roundtrips) and if it finds the player is playing, it sets the song info as the presence.
> 
> 
> Diffs
> -----
> 
>   telepathy-mpris.h de45cec 
>   telepathy-mpris.cpp 8386dd9 
> 
> Diff: http://git.reviewboard.kde.org/r/105813/diff/
> 
> 
> Testing
> -------
> 
> Tested with clementine.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20120801/cd47f0d5/attachment-0001.html>