Skip to content
This repository has been archived by the owner on Dec 4, 2020. It is now read-only.

Implemented MPRIS support #10

Merged
merged 1 commit into from
Jul 25, 2015
Merged

Implemented MPRIS support #10

merged 1 commit into from
Jul 25, 2015

Conversation

jck
Copy link
Contributor

@jck jck commented Jul 25, 2015

@twolfson
Copy link
Owner

I am having trouble getting this to compile on Linux Mint 17.1 Cinnamon 64-bit:

> dbus@0.2.11 install /home/todd/github/google-music-electron/node_modules/mpris-service/node_modules/dbus
> node-gyp configure build

make: Entering directory `/home/todd/github/google-music-electron/node_modules/mpris-service/node_modules/dbus/build'
  CXX(target) Release/obj.target/dbus/src/dbus.o
../src/dbus.cc: In function ‘void NodeDBus::method_callback(DBusPendingCall*, void*)’:
../src/dbus.cc:47:61: error: conversion from ‘v8::Handle<v8::Value>’ to non-scalar type ‘v8::Local<v8::Value>’ requested
   Local<Value> result = Decoder::DecodeMessage(reply_message);
                                                             ^
../src/dbus.cc:51:3: error: conversion from ‘v8::Handle<v8::Value>’ to non-scalar type ‘v8::Local<v8::Value>’ requested
   };
   ^
../src/dbus.cc: In function ‘v8::Handle<v8::Value> NodeDBus::ParseIntrospectSource(const v8::Arguments&)’:
../src/dbus.cc:270:4: error: return-statement with no value, in function returning ‘v8::Handle<v8::Value>’ [-fpermissive]
    return;
    ^
make: *** [Release/obj.target/dbus/src/dbus.o] Error 1
make: Leaving directory `/home/todd/github/google-music-electron/node_modules/mpris-service/node_modules/dbus/build'
gyp ERR! build error 
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/usr/local/lib/node_modules/npm/node_modules/node-gyp/lib/build.js:269:23)
gyp ERR! stack     at ChildProcess.emit (events.js:98:17)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (child_process.js:820:12)
gyp ERR! System Linux 3.13.0-37-generic
gyp ERR! command "node" "/usr/local/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "configure" "build"
gyp ERR! cwd /home/todd/github/google-music-electron/node_modules/mpris-service/node_modules/dbus
gyp ERR! node -v v0.10.40
gyp ERR! node-gyp -v v1.0.3
gyp ERR! not ok 

Any ideas what's wrong?

Outside of that, from my development in spritesmith I have learned that:

  • People usually are confused when they see warning messages upon installation
  • People dislike downloading things they aren't using

As a result, it's usually a good idea to avoid optionalDependencies. What are your thoughts on creating a new command or menu item to handle this installation? Inside the application, we can do a try/catch to optionally load the mrpis module.

google-music-electron install-mpris

@jck
Copy link
Contributor Author

jck commented Jul 25, 2015

It looks like the dbus module compilation is broken on node v0.10.x : Shouqun/node-dbus#97

This is my first time writing node.js code, so I have no idea about best practices for the dependencies. Feel free to make any changes you see fit.

@twolfson
Copy link
Owner

Ah, nice. That is definitely the same error. I am going to try out using the patched version (it has been landed but not yet published).

@jck
Copy link
Contributor Author

jck commented Jul 25, 2015

You'll need to run electron-rebuild after npm install since node-dbus is a native module. Perhaps this should be added to the documentation or the new install command?

@twolfson
Copy link
Owner

It's installing now but I am realizing that we want to be building under electron via electron-rebuild as you have done.

@twolfson
Copy link
Owner

Yep =P

@twolfson
Copy link
Owner

Woot, got it running. Looks awesome =)

selection_149

@twolfson
Copy link
Owner

There's actually a bunch more info we should be able to provide:

  • Album art
  • Playback time

but this is great. Thanks for the work on this =)

I am going to take a little bit and tweak the PR to make it fit with the "extra command" we discussed.

@jck
Copy link
Contributor Author

jck commented Jul 25, 2015

Nice! We can implement the remaining methods(art, volume, seeking) so that all the information is accessible from the widget.

@twolfson twolfson merged commit 9ffde4e into twolfson:master Jul 25, 2015
@twolfson
Copy link
Owner

Alright, I have added in the install-mpris command and documented it. This was released in 1.15.0.

As for the dependencies, I relocated them to an mprisDependencies key which we load in during the install-mpris command.

The full diff can be found here:

1.14.1...1.15.0

I am going to see about adding in the album art/playback time now =)

@jck
Copy link
Contributor Author

jck commented Jul 25, 2015

Awesome. Thanks!

@twolfson
Copy link
Owner

Added more MPRIS functionality:

  • Track duration -- although it doesn't seem to work properly on Cinnamon =(
  • Album art
  • Raise/focus window
  • Exit application

@twolfson
Copy link
Owner

Thanks again for adding this awesome feature =)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants