thiagokokada / i3pyblocks

A replacement for i3status, written in Python using asyncio.
MIT License
19 stars 2 forks source link

Create MediaPlayerBlock using a MPRIS2 library #132

Open DesmondTMB opened 2 years ago

DesmondTMB commented 2 years ago

I think it might be beneficial to rewrite the MediaPlayerBlock based on a MPRIS2 library to more easily add features to it.

Features I have in mind are:

DesmondTMB commented 2 years ago

https://github.com/hugosenari/mpris2 looks like it could do the job.

thiagokokada commented 2 years ago

I think it is fine to add this in a separate namespace. They way i3pyblocks is organized is by the main dependency, so I am fine adding a new i3pyblocks.blocks.mpris2 that uses this module instead (while keeping the original MediaPlayerBlock, maybe deprecating it?).

thiagokokada commented 2 years ago

https://github.com/hugosenari/mpris2 looks like it could do the job.

Is there a more maintained alternative for it?

DesmondTMB commented 2 years ago

Is there a more maintained alternative for it?

https://github.com/airtower-luna/mpris-python looks a little more bare bones, but was committed to this year.

thiagokokada commented 2 years ago

https://github.com/airtower-luna/mpris-python looks a little more bare bones, but was committed to this year.

Well, this seems really barebones. If https://github.com/hugosenari/mpris2 works I am fine with it. At least it has no external dependencies, so this is always a plus.

DesmondTMB commented 2 years ago

I am currently looking if I can get around using any dependency after all. Since this project's core is using asyncio, implementing the MediaPlayerBlock using dbus-next seems the more sensible option.

My current pitfalls is that I need two (or more) dbus interfaces where DbusBlock currently only supports one. org.freedesktiop.DBus.Properties which is currently used provides the PropertiesChanged signal, while org.mpris.MediaPlayer2.Player provides methods to control the player (org.mpris.MediaPlayer2 would be useful as well for the Raise method). I am unsure if the best approach would be to implement multi interface support into the DbusBlock or into MediaPlayerBlock itself. Putting it into DbusBlock would probably mean keeping the old methods to prevent breaking anything and then adding new methods like safe_method_call_by_interface and safe_property_set_by_interface which the inheriting blocks could use on self maintained interfaces. Since those methods would be static and not access any class attributes, I am unsure where to put them. An alternative that I can think of would be to implement a DbusMultipleInterfacesBlock that would maintain its interfaces as a dictionary and would re-implement the methods like safe_method_call and wait_interface by requiring a key to the dict

thiagokokada commented 2 years ago

I am currently looking if I can get around using any dependency after all. Since this project's core is using asyncio, implementing the MediaPlayerBlock using dbus-next seems the more sensible option.

My current pitfalls is that I need two (or more) dbus interfaces where DbusBlock currently only supports one. org.freedesktiop.DBus.Properties which is currently used provides the PropertiesChanged signal, while org.mpris.MediaPlayer2.Player provides methods to control the player (org.mpris.MediaPlayer2 would be useful as well for the Raise method). I am unsure if the best approach would be to implement multi interface support into the DbusBlock or into MediaPlayerBlock itself. Putting it into DbusBlock would probably mean keeping the old methods to prevent breaking anything and then adding new methods like safe_method_call_by_interface and safe_property_set_by_interface which the inheriting blocks could use on self maintained interfaces. Since those methods would be static and not access any class attributes, I am unsure where to put them. An alternative that I can think of would be to implement a DbusMultipleInterfacesBlock that would maintain its interfaces as a dictionary and would re-implement the methods like safe_method_call and wait_interface by requiring a key to the dict

One thing I generally do during development of new modules is implementing directly what I want on the final module (so in this case, MediaPlayerBlock), and only try to abstract once I have two or more different usages of this new pattern.

That said, DbusBlock is probably the worse base block in the whole codebase. As it is right now it is impossible to test (so much that the tests are disabled: https://github.com/thiagokokada/i3pyblocks/blob/master/tests/blocks/test_dbus.py#L11 and https://github.com/thiagokokada/i3pyblocks/blob/master/tests/blocks/test_dbus.py#L69). So a from scratch base block would be interesting.

Also yeah, the one property limitation is probably an oversight from my part. It makes the module kinda useless except for very simply contexts. If both KbddBlock and MediaPlayerBlock can be migrated to a new base block I am all from just scrapping the old DbusBlock module and substituting it for a new one.

thiagokokada commented 2 years ago

An alternative that I can think of would be to implement a DbusMultipleInterfacesBlock that would maintain its interfaces as a dictionary and would re-implement the methods like safe_method_call and wait_interface by requiring a key to the dict

This also doesn't seem to be a bad idea actually. I would just suggest another name, maybe DbusesBlock or DbusMultipleBlock.

DesmondTMB commented 2 years ago

An alternative that I can think of would be to implement a DbusMultipleInterfacesBlock that would maintain its interfaces as a dictionary and would re-implement the methods like safe_method_call and wait_interface by requiring a key to the dict

This also doesn't seem to be a bad idea actually. I would just suggest another name, maybe DbusesBlock or DbusMultipleBlock.

I'll try to implement DbusMultipleBlock and then try to implement MediaPlayerBlock with that and see where that leads me

DesmondTMB commented 2 years ago

I am not sure if I am capable of improving the original code by much except removing the one interface limitation

DesmondTMB commented 2 years ago

I have hit a road block, because it seems that in dbus-next there is no way go get possible bus names available on the system, so you need to always know exactly what it is called, which makes a single block for all players impossible. Even using it for firefox, since that allways has an explicit instance as its suffix.

So back to the mpris2 library it is