Skip to content
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

Add MPD support #268

Merged
merged 15 commits into from
Apr 18, 2019
Merged

Add MPD support #268

merged 15 commits into from
Apr 18, 2019

Conversation

minijackson
Copy link
Contributor

@minijackson minijackson commented Apr 16, 2019

This adds a basic MPD module, somewhat based on Polybar's implementation.

Also adds a minimal EditorConfig so editors picks up the codebase's style. (Maybe a .clang_format would be nice too but I don't think I should be the one choosing the style. See here if you're interested)

Clang format configuration added!

Don't merge now I'm currently trying to also add support for options (shuffle, repeat, etc.). I'm putting it here now so I pick up on recommendations/reviews as soon as possible ^^

Features:

  • artist, album-artist, album, title, and date in the label
  • format, format-stopped, and format-disconnected depending on this module's state
  • Event driven updates (so fast!)
  • Automatic reconnect

Implementation details:

  • Uses the official libmpdclient
  • Use the Deleter feature of unique_ptrs to avoid the rule of 5

Todo:

  • Add support for options (shuffle, repeat, etc.)
  • Allow specifying the reconnect interval
  • Add support for icons
  • Add support for pause/play when clicking
  • Add a better sample config with icons in resources/config
  • Update the wiki?
  • Check error handling (the current implementation can throw an exception in a thread which won't be handled, and can make Waybar crash)
  • Allow setting a label when disconnected
  • Periodically fetch elapsed time when a song is playing

Thanks for the great work on Waybar ^^

@minijackson
Copy link
Contributor Author

This was very briefly mentioned on #59, so @xajler might be interested (sorry if you didn't want to be tagged)

@Alexays
Copy link
Owner

Alexays commented Apr 16, 2019

Nice work!
I wanted to do the clang format based on google rules but I forgot about it, if you have time to do it before me do not hesitate ❤️

@xajler
Copy link

xajler commented Apr 17, 2019

This was very briefly mentioned on #59, so @xajler might be interested (sorry if you didn't want to be tagged)

No problem for tagging, thank you very much @minijackson for mpd support, I'm very eager to try it, because I currently have some nasty shell script displaying song and elapsed time of song in waybar.

@minijackson
Copy link
Contributor Author

I currently have some nasty shell script displaying song

I'm exactly the same!

Unfortunately, I'm not really sure how to handle the elapsed time, since the current implementation is event based, and we have to have periodic updates to get the elapsed time. Maybe we should do this in a separated thread, and synchronize the elapsed time only when playing using a mutex? Seems a bit much for that :-/
It would be nice nonetheless, so I'm going to add it to the todolist.

@minijackson
Copy link
Contributor Author

Okay, I think it's pretty much good to merge, now! 🎉

I just need to rebase it on master, which currently have a small conflict on the .clang-format file

So @Alexays should I keep yours or mine ? ^^

@Alexays
Copy link
Owner

Alexays commented Apr 18, 2019

Nice work! 🎉
You can keep yours, but i just don't really like AlignConsecutiveAssignments: true
Also if you can update the wiki it'll be perfect ❤️

@minijackson
Copy link
Contributor Author

Rebase done! You can merge it, I'll update the wiki in the meantime

@Alexays Alexays merged commit 817c428 into Alexays:master Apr 18, 2019
@minijackson minijackson deleted the mpd branch April 18, 2019 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants