-
Notifications
You must be signed in to change notification settings - Fork 31
Command Lists #73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Command Lists #73
Changes from all commits
e9bbf62
2968621
bc8a28d
6b25ecb
5d9b5e0
4d50771
8fb8177
364e609
b01adf4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,177 @@ | ||
| module MPD::Plugins | ||
|
|
||
| # Batch send multiple commands at once for speed. | ||
| module CommandList | ||
| # Send multiple commands at once. | ||
| # | ||
| # By default, any response from the server is ignored (for speed). | ||
| # To get results, pass +response_type+ as one of the following options: | ||
| # | ||
| # * +:hash+ — a single hash of return values; multiple return values for the same key are grouped in an array | ||
| # * +:values+ — an array of individual values | ||
| # * +:hashes+ — an array of hashes (where value boundaries are guessed based on the first result) | ||
| # * +:songs+ — an array of Song instances from the results | ||
| # * +:playlists+ - an array of Playlist instances from the results | ||
| # | ||
| # Note that each supported command has no return value inside the block. | ||
| # Instead, the block itself returns the array of results. | ||
| # | ||
| # @param [Symbol] response_type the type of responses desired. | ||
| # @return [nil] default behavior. | ||
| # @return [Array] if +response_type+ is +:values+, +:hashes+, +:songs+, or +:playlists+. | ||
| # @return [Hash] if +response_type+ is +:hash+. | ||
| # | ||
| # @example Simple batched control commands | ||
| # @mpd.command_list do | ||
| # stop | ||
| # shuffle | ||
| # save "shuffled" | ||
| # end | ||
| # | ||
| # @example Adding songs to the queue, ignoring the response | ||
| # @mpd.command_list do | ||
| # my_songs.each do |song| | ||
| # add(song) | ||
| # end | ||
| # end | ||
| # | ||
| # @example Adding songs to the queue and getting the song ids | ||
| # ids = @mpd.command_list(:values){ my_songs.each{ |song| addid(song) } } | ||
| # #=> [799,800,801,802,803] | ||
| # | ||
| # ids = @mpd.command_list(:hashes){ my_songs.each{ |song| addid(song) } } | ||
| # #=> [ {:id=>809}, {:id=>810}, {:id=>811}, {:id=>812}, {:id=>813} ] | ||
| # | ||
| # ids = @mpd.command_list(:hash){ my_songs.each{ |song| addid(song) } } | ||
| # #=> { :id=>[804,805,806,807,808] } | ||
| # | ||
| # @example Finding songs matching various genres | ||
| # songs = @mpd.command_list(:songs) do | ||
| # where genre:'Alternative Rock' | ||
| # where genre:'Alt. Rock' | ||
| # where genre:'alt-rock' | ||
| # end | ||
| # | ||
| # @see CommandList::Commands CommandList::Commands for a list of supported commands. | ||
| def command_list(response_type=nil,&commands) | ||
| @mutex.synchronize do | ||
| begin | ||
| socket.puts "command_list_begin" | ||
| CommandList::Commands.new(self).instance_eval(&commands) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, command lists are executed synchronous with the other calls, such as the callback thread. I'm having some concerns that it could block other execution, what do you think about opening another MPD connection, executing, then closing the connection?
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (In a new thread of course) This is ruby though, so maybe it won't make any difference since we lack real concurrency.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the case explicitly where As it is, I find that about half the time I use the result of the command list. Given that we do not know if this would provide any real benefit (both because of no real concurrency, and the overhead of establishing a new connection), and it's a moderate amount of additional code and work, I'd suggest that it's a theoretically good idea that should be investigated in the future separate from this pull request. |
||
| socket.puts "command_list_end" | ||
|
|
||
| # clear the response from the socket, even if we will not parse it | ||
| response = handle_server_response || "" | ||
|
|
||
| case response_type | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, the command list will return multiple responses though, right? One for each command. So the type might vary from command to command
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I honestly think we might be safe to just throw the responses away and return nil, in most cases this will be used for batching anyway
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, when As I mentioned in another comment, about half my current uses of songs = @mpd.command_list(:songs){ uris.each{ |uri| where({file:uri},{strict:true}) } }Yes, the command_list does return multiple responses, and in the 'common' (as I'm imagining them) use cases they will be the same type. There are edge cases where the user might want to send mixed commands with varying return types AND want the return types. That's not currently possible with my current code, since I'm using I decided to use
This would be a little more code than I currently have, and the resulting responses and parsing MIGHT be a little slower, but it would be more DWIM, more robust, and less geeky. Something like: results = mpd.command_list results:true do
addid(song1)
where genre:'rock'
added(song2)
end
p results[0] #=> 42
p results[1] #=> [ <MPD::Song#...>, <MPD::Song#...>, <MPD::Song#...>, ... ]
p results[2] #=> 43I'm not sure if I believe there is a strong need for the intermingling, but I am a fan of things just working as expected (POLS and all that). Does this sound like a good idea to you? |
||
| when :values then response.lines.map{ |line| parse_line(line).last } | ||
| when :hash then build_hash(response) | ||
| when :hashes then build_response(:commandlist,response,true) | ||
| when :songs then build_songs_list parse_response(:listallinfo,response) | ||
| when :playlists then build_playlists parse_response(:listplaylists,response) | ||
| end | ||
| rescue Errno::EPIPE | ||
| reconnect | ||
| retry | ||
| end | ||
| end | ||
| end | ||
| end | ||
|
|
||
| class CommandList::Commands | ||
| def initialize(mpd) | ||
| @mpd = mpd | ||
| end | ||
|
|
||
| include MPD::Plugins::Controls | ||
| include MPD::Plugins::PlaybackOptions | ||
| include MPD::Plugins::Queue | ||
| include MPD::Plugins::Stickers | ||
| include MPD::Plugins::Database | ||
|
|
||
| # List all of the playlists in the database | ||
| def playlists | ||
| send_command(:listplaylists) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I'm not too thrilled about having another implementation of the playlists just for the command list. Do you think it would be possible to reuse the original MPD::Playlist objects? It won't be possible to fetch it and then reuse it in subsequent commands in the list, but that's not possible with these plain method calls either
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All the playlist methods currently use The only way that I can think to re-use the playlist objects would be to use an instance variable set as a flag when the command list was started and mutate the way I'd also have to modify Would you prefer that approach? Having |
||
| end | ||
|
|
||
| # Fetch full details for all songs in a playlist | ||
| # @param [String,Playlist] playlist The string name (or playlist object) to get songs for. | ||
| def songs_in_playlist(playlist) | ||
| send_command(:listplaylistinfo, playlist) | ||
| end | ||
|
|
||
| # Fetch file names only for all songs in a playlist | ||
| # @param [String,Playlist] playlist The string name (or playlist object) to get files for. | ||
| def files_in_playlist(playlist) | ||
| send_command(:listplaylist, playlist) | ||
| end | ||
|
|
||
| # Load the playlist's songs into the queue. | ||
| # | ||
| # Since MPD v0.17 a range can be passed to load only a part of the playlist. | ||
| # @param [String,Playlist] playlist The string name (or playlist object) to load songs from. | ||
| # @param [Range] range The index range of songs to add. | ||
| def load_playlist(playlist, range=nil) | ||
| send_command :load, playlist, range | ||
| end | ||
|
|
||
| # Add a song to the playlist. | ||
| # @param [String,Playlist] playlist The string name (or playlist object) to add to. | ||
| # @param [String,Song] song The string uri (or song object) to add to the playlist | ||
| def add_to_playlist(playlist, song) | ||
| send_command :playlistadd, playlist, song | ||
| end | ||
|
|
||
| # Search for any song that contains +value+ in the +tag+ field | ||
| # and add them to a playlist. | ||
| # Searches are *NOT* case sensitive. | ||
| # | ||
| # @param [String,Playlist] playlist The string name (or playlist object) to add to. | ||
| # @param [Symbol] tag Can be any tag supported by MPD, or one of the two special | ||
| # parameters: +:file+ to search by full path (relative to database root), | ||
| # and +:any+ to match against all available tags. | ||
| # @param [String] value The string to search for. | ||
| def searchadd_to_playlist(playlist, tag, value) | ||
| send_command :searchaddpl, playlist, tag, value | ||
| end | ||
|
|
||
| # Remove all songs from a playlist. | ||
| # @param [String,Playlist] playlist The string name (or playlist object) to clear. | ||
| def clear_playlist(playlist) | ||
| send_command :playlistclear, playlist | ||
| end | ||
|
|
||
| # Delete song at +index+ from a playlist. | ||
| # @param [String,Playlist] playlist The string name (or playlist object) to affect. | ||
| # @param [Integer] index The index of the song to remove. | ||
| def remove_from_playlist(playlist, index) | ||
| send_command :playlistdelete, playlist, index | ||
| end | ||
|
|
||
| # Move a song with +song_id+ in a playlist to a new +index+. | ||
| # @param [String,Playlist] playlist The string name (or playlist object) to affect. | ||
| # @param [Integer] songid The +id+ of the song to move. | ||
| # @param [Integer] index The index to move the song to. | ||
| def reorder_playlist(playlist, song_id, index) | ||
| send_command :playlistmove, playlist, song_id, songpos | ||
| end | ||
|
|
||
| # Rename a playlist to +new_name+. | ||
| # @param [String,Playlist] playlist The string name (or playlist object) to rename. | ||
| # @param [String] new_name The new name for the playlist. | ||
| def rename_playlist(playlist,new_name) | ||
| send_command :rename, playlist, new_name | ||
| end | ||
|
|
||
| # Delete a playlist from the disk. | ||
| # @param [String,Playlist] playlist The string name (or playlist object) to delete. | ||
| def destroy_playlist(playlist) | ||
| send_command :rm, playlist | ||
| end | ||
|
|
||
| private | ||
| def send_command(command,*args) | ||
| @mpd.send(:socket).puts @mpd.send(:convert_command, command, *args) | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| class MPD | ||
| VERSION = '0.3.4' | ||
| VERSION = '0.4.0' | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| command_list_begin | ||
| clear | ||
| addid test1.mp3 | ||
| addid "test 2.mp3" | ||
| addid test3.mp3 | ||
| command_list_end | ||
| --putsabove--getsbelow-- | ||
| Id: 107 | ||
| Id: 108 | ||
| Id: 109 | ||
| OK |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| command_list_begin | ||
| listplaylists | ||
| command_list_end | ||
| --putsabove--getsbelow-- | ||
| playlist: Mix Rock Alt Electric | ||
| Last-Modified: 2015-11-23T15:58:51Z | ||
| playlist: SNBRN | ||
| Last-Modified: 2016-01-26T00:25:52Z | ||
| playlist: Enya-esque | ||
| Last-Modified: 2015-11-18T16:19:12Z | ||
| playlist: RecentNice | ||
| Last-Modified: 2015-12-01T15:52:38Z | ||
| playlist: Dancetown | ||
| Last-Modified: 2015-11-18T16:19:26Z | ||
| playlist: Piano | ||
| Last-Modified: 2015-11-18T16:17:13Z | ||
| playlist: Thump | ||
| Last-Modified: 2015-11-20T15:32:30Z | ||
| playlist: Smooth Town | ||
| Last-Modified: 2015-11-20T15:54:49Z | ||
| OK |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Playlists can be passed as params in command lists methods where a playlist name is required.