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

Wrong D-Bus Object Path #1103

Closed
ThomasFrans opened this issue Mar 28, 2023 · 8 comments · Fixed by #1107
Closed

Wrong D-Bus Object Path #1103

ThomasFrans opened this issue Mar 28, 2023 · 8 comments · Fixed by #1107
Labels
bug Something isn't working

Comments

@ThomasFrans
Copy link
Contributor

ThomasFrans commented Mar 28, 2023

Describe the bug
I'm not sure if this is a bug with ncspot or with d-spy but d-spy does correctly report the mpris implementation for FireFox, so I think it's an issue with the introspection implementation of ncspot. The actual object path can correctly be used at /org/mpris/MediaPlayer2, but the introspection returns /org/mpris/MediaPlayer2/org/mpris/MediaPlayer2 as object path. At least that's what I think this is (I just started playing around with D-Bus today, that's how I noticed it).

To Reproduce

  1. Open ncspot
  2. Open d-spy
  3. Go to the ncspot service
  4. Look at the object path (compared to the one reported by other media players and the one from the mpris specification)

Expected behavior
I think it should report /org/mpris/MediaPlayer2.

Screenshots
image

System (please complete the following information):

  • OS: Arch Linux
  • Terminal: Alacritty
  • Version: 12.0
  • Installed from: Pacman
@ThomasFrans ThomasFrans added the bug Something isn't working label Mar 28, 2023
@hrkfdn
Copy link
Owner

hrkfdn commented Mar 30, 2023

Yeah, can definitely reproduce this. I have spent the whole evening trying to spot the culprit but I can't find it. I've compared it to other MPRIS DBus implementations but no idea how the object path ends up like that 😕

@ThomasFrans
Copy link
Contributor Author

Same thing here. I've spent about 2 hours looking into this and I can't find what it is. It's such a weird issue because it literally adds the same name twice. If you edit the name, it takes the new name and adds it twice (I thought maybe dbus-rs was doing some magic by using the .desktop file and one of the paths was automatically generated, but that isn't true). Maybe mark this one as help wanted?

hrkfdn added a commit that referenced this issue Mar 30, 2023
The initial DBus implementation was getting harder to maintain and `zbus` offers
some nice convenience features that should make our MPRIS implementation
cleaner.

For now this only implements the `org.mpris.MediaPlayer2` interface which does
not do much.

Should help with #1103
@hrkfdn
Copy link
Owner

hrkfdn commented Mar 30, 2023

I just took this as a cue to redo the implementation 😄

It was on my list anyway and zbus looks quite nice!

hrkfdn added a commit that referenced this issue Mar 30, 2023
The initial DBus implementation was getting harder to maintain and `zbus` offers
some nice convenience features that should make our MPRIS implementation
cleaner.

For now this only implements the `org.mpris.MediaPlayer2` interface which does
not do much.

Should help with #1103
hrkfdn added a commit that referenced this issue Mar 30, 2023
The initial DBus implementation was getting harder to maintain and `zbus` offers
some nice convenience features that should make our MPRIS implementation
cleaner.

For now this only implements the `org.mpris.MediaPlayer2` interface which does
not do much.

Should help with #1103
@ThomasFrans
Copy link
Contributor Author

I was going to add that I considered rewriting the mpris code in the previous comment 😄 It could use a rewrite indeed.

Anyways, I've heard that there are problems with zbus on the BSD's. I don't know the specifics of it, but it might be worth looking into that...

@hrkfdn
Copy link
Owner

hrkfdn commented Mar 30, 2023

Oh thanks, that's a very good point!

Edit: Just checked and there are 2 issues related to FreeBSD that were closed. Are you referring to those or did you find anything else?

@ThomasFrans
Copy link
Contributor Author

It might have been those then. Like I said, I don't know any specific details. Someone on another project where I wanted to propose a feature using D-Bus asked me not to use zbus as it had problems on BSD. That's a half year ago I think.

@ThomasFrans
Copy link
Contributor Author

Just for reference: Builditluc/wiki-tui#49 (comment)

(Haven't implemented that search provider yet... 😬 )

@hrkfdn
Copy link
Owner

hrkfdn commented Mar 30, 2023

Hmm okay, thanks. I was able to build the crate on my OpenBSD machine just now. Maybe the issue was fixed or it only happens on NetBSD.

@hrkfdn hrkfdn linked a pull request Apr 1, 2023 that will close this issue
hrkfdn added a commit that referenced this issue Apr 1, 2023
The initial DBus implementation was getting harder to maintain and `zbus` offers
some nice convenience features that should make our MPRIS implementation
cleaner.

For now this only implements the `org.mpris.MediaPlayer2` interface which does
not do much.

Should help with #1103
hrkfdn added a commit that referenced this issue Apr 1, 2023
* Rewrite MPRIS implementation using zbus

The initial DBus implementation was getting harder to maintain and `zbus` offers
some nice convenience features that should make our MPRIS implementation
cleaner.

For now this only implements the `org.mpris.MediaPlayer2` interface which does
not do much.

Should help with #1103

* Implement MPRIS properties

- `PlaybackStatus`
- `PlaybackRate`
- `Volume` (get/set)

* Implement remaining player properties/functions

* Emit signal for changed properties on track change
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants