From 06568248fc20007559a6a707f33b6addf6cb62f2 Mon Sep 17 00:00:00 2001 From: John Maguire Date: Tue, 15 Jan 2013 13:05:43 +0100 Subject: [PATCH] Tidy up some remote control protobuf usage and style quirks. --- .../remotecontrolmessages.proto | 80 ++++--------- src/networkremote/incomingdataparser.cpp | 48 +++----- src/networkremote/incomingdataparser.h | 6 +- src/networkremote/outgoingdatacreator.cpp | 105 +++++++++--------- src/networkremote/outgoingdatacreator.h | 10 +- 5 files changed, 100 insertions(+), 149 deletions(-) diff --git a/ext/libclementine-remote/remotecontrolmessages.proto b/ext/libclementine-remote/remotecontrolmessages.proto index 3bff5496f..0c542193a 100644 --- a/ext/libclementine-remote/remotecontrolmessages.proto +++ b/ext/libclementine-remote/remotecontrolmessages.proto @@ -10,7 +10,7 @@ enum MsgType { REQUEST_PLAYLIST_SONGS = 4; CHANGE_SONG = 5; SET_VOLUME = 6; - + // Messages send by both PLAY = 20; PLAYPAUSE = 21; @@ -23,10 +23,10 @@ enum MsgType { // Either set by client or clementine REPEAT = 27; RANDOM = 28; - + // Messages send from server to client - INFOS = 40; - CURRENT_METAINFOS = 41; + INFO = 40; + CURRENT_METAINFO = 41; PLAYLISTS = 42; PLAYLIST_SONGS = 43; ENGINE_STATE_CHANGED = 44; @@ -64,7 +64,7 @@ message Playlist { optional string name = 2; optional int32 item_count = 3; optional bool active = 4; - + // The songs are only sent when the client requests them. // See src/remotecontrol/outgoingdatacreator.cpp for more info repeated SongMetadata songs = 10; @@ -86,23 +86,6 @@ enum ShuffleMode { Shuffle_Albums = 3; } -// Message with unknown content -// Shouldn't be used anywhere -message Unknown { -} - -// This message is sent when a client connects to clementine -message Connect { -} - -// This message is sent when a client disconnects from clementine -message Disconnect { -} - -// Client requests all playlists -message RequestPlaylists { -} - // A Client requests songs from a specific playlist message RequestPlaylistSongs { optional int32 id = 1; @@ -121,15 +104,6 @@ message RequestSetVolume { optional int32 volume = 1; } -// Controlmessages -message Play {} -message PlayPause {} -message Pause {} -message Stop {} -message Next {} -message Previous {} -message ShufflePlaylist {} - // Repeat and Random messages message Repeat { optional RepeatMode repeat_mode = 1; @@ -138,11 +112,12 @@ message Repeat { message Shuffle { optional ShuffleMode shuffle_mode = 1; } - + // Response from server -// General infos +// General info message ResponseClementineInfo { optional string version = 1; + optional EngineState state = 2; } // The current song played @@ -158,7 +133,6 @@ message ResponsePlaylists { // A list of songs in a playlist message ResponsePlaylistSongs { optional Playlist requested_playlist = 1; - repeated SongMetadata song = 2; } // The current state of the play engine @@ -166,33 +140,19 @@ message ResponseEngineStateChanged { optional EngineState state = 1; } -// Empty Keep Alive telegram -message KeepAlive {} - // The message itself message Message { optional int32 version = 1 [default=1]; - optional MsgType msgType = 2 [default=UNKNOWN]; // What data is in the message? - - optional Unknown unknown = 10; - optional Connect connect = 11; - optional Disconnect disconnect = 12; - optional RequestPlaylists request_playlist = 13; - optional RequestPlaylistSongs request_playlist_songs = 14; - optional RequestChangeSong request_change_song = 15; - optional RequestSetVolume request_set_volume = 16; - optional Play play = 17; - optional PlayPause play_pause = 18; - optional Pause pause = 19; - optional Stop stop = 20; - optional Next next = 21; - optional Previous previous = 22; - optional ShufflePlaylist shuffle_playlist = 23; - optional Repeat repeat = 24; - optional Shuffle shuffle = 25; - optional ResponseClementineInfo response_clementine_info = 26; - optional ResponseCurrentMetadata response_current_metadata = 27; - optional ResponsePlaylists response_playlists = 28; - optional ResponsePlaylistSongs response_playlist_songs = 29; - optional ResponseEngineStateChanged response_engine_state_changed = 30; + optional MsgType type = 2 [default=UNKNOWN]; // What data is in the message? + + optional RequestPlaylistSongs request_playlist_songs = 10; + optional RequestChangeSong request_change_song = 11; + optional RequestSetVolume request_set_volume = 12; + optional Repeat repeat = 13; + optional Shuffle shuffle = 14; + optional ResponseClementineInfo response_clementine_info = 15; + optional ResponseCurrentMetadata response_current_metadata = 16; + optional ResponsePlaylists response_playlists = 17; + optional ResponsePlaylistSongs response_playlist_songs = 18; + optional ResponseEngineStateChanged response_engine_state_changed = 19; } diff --git a/src/networkremote/incomingdataparser.cpp b/src/networkremote/incomingdataparser.cpp index 0b153fb44..ffc81d6f1 100644 --- a/src/networkremote/incomingdataparser.cpp +++ b/src/networkremote/incomingdataparser.cpp @@ -65,17 +65,17 @@ void IncomingDataParser::Parse(const QByteArray& data) { } // Now check what's to do - switch (msg.msgtype()) { - case pb::remote::CONNECT: emit SendClementineInfos(); + switch (msg.type()) { + case pb::remote::CONNECT: emit SendClementineInfo(); emit SendFirstData(); break; case pb::remote::DISCONNECT: close_connection_ = true; break; case pb::remote::REQUEST_PLAYLISTS: emit SendAllPlaylists(); break; - case pb::remote::REQUEST_PLAYLIST_SONGS: GetPlaylistSongs(&msg); + case pb::remote::REQUEST_PLAYLIST_SONGS: GetPlaylistSongs(msg); break; - case pb::remote::SET_VOLUME: emit SetVolume(msg.volume()); + case pb::remote::SET_VOLUME: emit SetVolume(msg.request_set_volume().volume()); break; case pb::remote::PLAY: emit Play(); break; @@ -87,47 +87,29 @@ void IncomingDataParser::Parse(const QByteArray& data) { break; case pb::remote::NEXT: emit Next(); break; - case pb::remote::PREV: emit Previous(); + case pb::remote::PREVIOUS: emit Previous(); break; - case pb::remote::CHANGE_SONG: ChangeSong(&msg); + case pb::remote::CHANGE_SONG: ChangeSong(msg); break; - case pb::remote::TOGGLE_SHUFFLE: emit ShuffleCurrent(); - break; + case pb::remote::SHUFFLE_PLAYLIST: emit ShuffleCurrent(); + break; default: break; } } -void IncomingDataParser::GetPlaylistSongs(pb::remote::Message* msg) { - // Check if we got a playlist - if (msg->playlists_size() == 0) - { - return; - } - - // Get the first entry and send the songs - pb::remote::Playlist playlist = msg->playlists(0); - emit SendPlaylistSongs(playlist.id()); +void IncomingDataParser::GetPlaylistSongs(const pb::remote::Message& msg) { + emit SendPlaylistSongs(msg.request_playlist_songs().id()); } -void IncomingDataParser::ChangeSong(pb::remote::Message* msg) { - // Check if we got a song - if (msg->playlists_size() == 0) { - return; - } - +void IncomingDataParser::ChangeSong(const pb::remote::Message& msg) { // Get the first entry and check if there is a song - pb::remote::Playlist playlist = msg->playlists(0); - if (playlist.songs_size() == 0) { - return; - } - - pb::remote::SongMetadata song = playlist.songs(0); + const pb::remote::RequestChangeSong& request = msg.request_change_song(); // Check if we need to change the playlist - if (playlist.id() != app_->playlist_manager()->active_id()) { - emit SetActivePlaylist(playlist.id()); + if (request.playlist_id() != app_->playlist_manager()->active_id()) { + emit SetActivePlaylist(request.playlist_id()); } // Play the selected song - emit PlayAt(song.index(), Engine::Manual, false); + emit PlayAt(request.song_index(), Engine::Manual, false); } diff --git a/src/networkremote/incomingdataparser.h b/src/networkremote/incomingdataparser.h index c930ef5b8..908618052 100644 --- a/src/networkremote/incomingdataparser.h +++ b/src/networkremote/incomingdataparser.h @@ -17,7 +17,7 @@ public slots: void Parse(const QByteArray& pb_data); signals: - void SendClementineInfos(); + void SendClementineInfo(); void SendFirstData(); void SendAllPlaylists(); void SendPlaylistSongs(int id); @@ -37,8 +37,8 @@ private: Application* app_; bool close_connection_; - void GetPlaylistSongs(pb::remote::Message* msg); - void ChangeSong(pb::remote::Message* msg); + void GetPlaylistSongs(const pb::remote::Message& msg); + void ChangeSong(const pb::remote::Message& msg); }; #endif // INCOMINGDATAPARSER_H diff --git a/src/networkremote/outgoingdatacreator.cpp b/src/networkremote/outgoingdatacreator.cpp index 79895bde7..da3a9fae4 100644 --- a/src/networkremote/outgoingdatacreator.cpp +++ b/src/networkremote/outgoingdatacreator.cpp @@ -59,23 +59,23 @@ void OutgoingDataCreator::SendDataToClients(pb::remote::Message* msg) { } } -void OutgoingDataCreator::SendClementineInfos() { +void OutgoingDataCreator::SendClementineInfo() { // Create the general message and set the message type pb::remote::Message msg; - msg.set_msgtype(pb::remote::INFOS); + msg.set_type(pb::remote::INFO); // Now add the message specific data - SetEngineState(&msg); + pb::remote::ResponseClementineInfo* info = + msg.mutable_response_clementine_info(); + SetEngineState(info); QString version = QString("%1 %2").arg(QCoreApplication::applicationName(), QCoreApplication::applicationVersion()); - pb::remote::ClementineInfo *info = msg.mutable_info(); info->set_version(version.toAscii()); - SendDataToClients(&msg); } -void OutgoingDataCreator::SetEngineState(pb::remote::Message *msg) { +void OutgoingDataCreator::SetEngineState(pb::remote::ResponseClementineInfo* msg) { switch(app_->player()->GetState()) { case Engine::Idle: msg->set_state(pb::remote::Idle); break; @@ -90,31 +90,33 @@ void OutgoingDataCreator::SetEngineState(pb::remote::Message *msg) { void OutgoingDataCreator::SendAllPlaylists() { // Get all Playlists - QList playlists = app_->playlist_manager()->GetAllPlaylists(); - QListIterator i(playlists); + QList app_playlists = app_->playlist_manager()->GetAllPlaylists(); int active_playlist = app_->playlist_manager()->active_id(); // Create message pb::remote::Message msg; - msg.set_msgtype(pb::remote::PLAYLISTS); + msg.set_type(pb::remote::PLAYLISTS); - while(i.hasNext()) { + pb::remote::ResponsePlaylists* playlists = msg.mutable_response_playlists(); + + QListIterator it(app_playlists); + while(it.hasNext()) { // Get the next Playlist - Playlist* p = i.next(); + Playlist* p = it.next(); QString playlist_name = app_->playlist_manager()->GetPlaylistName(p->id()); // Create a new playlist - pb::remote::Playlist* playlist = msg.add_playlists(); + pb::remote::Playlist* playlist = playlists->add_playlist(); playlist->set_name(playlist_name.toStdString()); playlist->set_id(p->id()); playlist->set_active((p->id() == active_playlist)); - // TODO: Fill in the song metadata here. + playlist->set_item_count(p->rowCount()); } SendDataToClients(&msg); } -void OutgoingDataCreator::ActiveChanged(Playlist *) { +void OutgoingDataCreator::ActiveChanged(Playlist*) { // When a playlist was changed, send the new list SendAllPlaylists(); } @@ -140,35 +142,40 @@ void OutgoingDataCreator::CurrentSongChanged(const Song& song, const QString& ur if (!clients_->empty()) { // Create the message pb::remote::Message msg; - msg.set_msgtype(pb::remote::CURRENT_METAINFOS); + msg.set_type(pb::remote::CURRENT_METAINFO); // If there is no song, create an empty node, otherwise fill it with data int i = app_->playlist_manager()->active()->current_row(); - CreateSong(msg.mutable_currentsong(), ¤t_song_, &uri, i); + CreateSong( + current_song_, uri, i, + msg.mutable_response_current_metadata()->mutable_song_metadata()); SendDataToClients(&msg); } } -void OutgoingDataCreator::CreateSong(pb::remote::SongMetadata* song_metadata, - Song* song, const QString* artUri, int index) { - if (song->is_valid()) { - song_metadata->set_id(song->id()); +void OutgoingDataCreator::CreateSong( + const Song& song, + const QString& art_uri, + const int index, + pb::remote::SongMetadata* song_metadata) { + if (song.is_valid()) { + song_metadata->set_id(song.id()); song_metadata->set_index(index); - song_metadata->set_title( DataCommaSizeFromQString(song->PrettyTitle())); - song_metadata->set_artist(DataCommaSizeFromQString(song->artist())); - song_metadata->set_album( DataCommaSizeFromQString(song->album())); - song_metadata->set_albumartist(DataCommaSizeFromQString(song->albumartist())); - song_metadata->set_pretty_length(DataCommaSizeFromQString(song->PrettyLength())); - song_metadata->set_genre(DataCommaSizeFromQString(song->genre())); - song_metadata->set_pretty_year(DataCommaSizeFromQString(song->PrettyYear())); - song_metadata->set_track(song->track()); - song_metadata->set_disc(song->disc()); - song_metadata->set_playcount(song->playcount()); + song_metadata->set_title(DataCommaSizeFromQString(song.PrettyTitle())); + song_metadata->set_artist(DataCommaSizeFromQString(song.artist())); + song_metadata->set_album(DataCommaSizeFromQString(song.album())); + song_metadata->set_albumartist(DataCommaSizeFromQString(song.albumartist())); + song_metadata->set_pretty_length(DataCommaSizeFromQString(song.PrettyLength())); + song_metadata->set_genre(DataCommaSizeFromQString(song.genre())); + song_metadata->set_pretty_year(DataCommaSizeFromQString(song.PrettyYear())); + song_metadata->set_track(song.track()); + song_metadata->set_disc(song.disc()); + song_metadata->set_playcount(song.playcount()); // Append coverart - if (!artUri->isEmpty()) { - QImage orig(QUrl(*artUri).toLocalFile()); + if (!art_uri.isEmpty()) { + QImage orig(QUrl(art_uri).toLocalFile()); QImage small; // Check if we resize the image if (orig.width() > 1000) { @@ -184,9 +191,7 @@ void OutgoingDataCreator::CreateSong(pb::remote::SongMetadata* song_metadata, small.save(&buf, "JPG"); // Append the Data in the protocol buffer - song_metadata->set_art(data.data(), data.size()); - - buf.close(); + song_metadata->set_art(data.constData(), data.size()); } } } @@ -195,8 +200,8 @@ void OutgoingDataCreator::CreateSong(pb::remote::SongMetadata* song_metadata, void OutgoingDataCreator::VolumeChanged(int volume) { // Create the message pb::remote::Message msg; - msg.set_msgtype(pb::remote::SET_VOLUME); - msg.set_volume(volume); + msg.set_type(pb::remote::SET_VOLUME); + msg.mutable_request_set_volume()->set_volume(volume); SendDataToClients(&msg); } @@ -208,23 +213,23 @@ void OutgoingDataCreator::SendPlaylistSongs(int id) { return; } - SongList song_list = playlist->GetAllSongs(); - QListIterator i(song_list); - // Create the message and the playlist pb::remote::Message msg; - msg.set_msgtype(pb::remote::PLAYLIST_SONGS); + msg.set_type(pb::remote::PLAYLIST_SONGS); // Create a new playlist - pb::remote::Playlist* pb_playlist = msg.add_playlists(); + pb::remote::Playlist* pb_playlist = + msg.mutable_response_playlist_songs()->mutable_requested_playlist(); pb_playlist->set_id(id); // Send all songs int index = 0; - while(i.hasNext()) { - Song song = i.next(); + SongList song_list = playlist->GetAllSongs(); + QListIterator it(song_list); + while(it.hasNext()) { + Song song = it.next(); QString art = song.art_automatic(); pb::remote::SongMetadata* pb_song = pb_playlist->add_songs(); - CreateSong(pb_song, &song, &art, index); + CreateSong(song, art, index, pb_song); ++index; } SendDataToClients(&msg); @@ -247,13 +252,13 @@ void OutgoingDataCreator::StateChanged(Engine::State state) { pb::remote::Message msg; switch (state) { - case Engine::Playing: msg.set_msgtype(pb::remote::PLAY); + case Engine::Playing: msg.set_type(pb::remote::PLAY); break; - case Engine::Paused: msg.set_msgtype(pb::remote::PAUSE); + case Engine::Paused: msg.set_type(pb::remote::PAUSE); break; - case Engine::Empty: msg.set_msgtype(pb::remote::STOP); // Empty is called when player stopped + case Engine::Empty: msg.set_type(pb::remote::STOP); // Empty is called when player stopped break; - default: msg.set_msgtype(pb::remote::STOP); + default: msg.set_type(pb::remote::STOP); break; }; @@ -262,6 +267,6 @@ void OutgoingDataCreator::StateChanged(Engine::State state) { void OutgoingDataCreator::SendKeepAlive() { pb::remote::Message msg; - msg.set_msgtype(pb::remote::KEEP_ALIVE); + msg.set_type(pb::remote::KEEP_ALIVE); SendDataToClients(&msg); } diff --git a/src/networkremote/outgoingdatacreator.h b/src/networkremote/outgoingdatacreator.h index 9ec9280e1..d27d19b1e 100644 --- a/src/networkremote/outgoingdatacreator.h +++ b/src/networkremote/outgoingdatacreator.h @@ -24,7 +24,7 @@ public: void SetClients(QList* clients); public slots: - void SendClementineInfos(); + void SendClementineInfo(); void SendAllPlaylists(); void SendFirstData(); void SendPlaylistSongs(int id); @@ -46,8 +46,12 @@ private: int keep_alive_timeout_; void SendDataToClients(pb::remote::Message* msg); - void SetEngineState(pb::remote::Message* msg); - void CreateSong(pb::remote::SongMetadata* song_metadata, Song* song, const QString* art_uri, int index); + void SetEngineState(pb::remote::ResponseClementineInfo* msg); + void CreateSong( + const Song& song, + const QString& art_uri, + const int index, + pb::remote::SongMetadata* song_metadata); }; #endif // OUTGOINGDATACREATOR_H