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

Make the web client a Mopidy extension #5

Closed
jodal opened this issue Jun 23, 2014 · 17 comments
Closed

Make the web client a Mopidy extension #5

jodal opened this issue Jun 23, 2014 · 17 comments

Comments

@jodal
Copy link
Contributor

jodal commented Jun 23, 2014

The shortly upcoming Mopidy 0.19 release lets web clients be installed as Mopidy extensions. This makes it possible to install a web client simply by running e.g. pip install Mopidy-Lux and restarting Mopidy. No configuration changes needed. No git clone or download-and-unzip.

Multiple web clients can be installed at the same time, as each client is hosted with a prefix, e.g. Mopidy-Lux on http://localhost:6680/lux/. Mopidy provides a list of all installed web clients on http://localhost:6680/mopidy/.

Mopidy 0.19 hasn't been released yet, but this feature is complete and available in our develop branch. We want web client authors to start working on making all the web clients pip-installable.

The main sources of information is:

We'll be happy to help you get this working nicely. Let us know if you run into problems.

@martijnboland
Copy link
Owner

@jodal I'm running into an error with Mopidy 0.19 from the github develop branch when trying to add tracks to a playlist or retrieving the current tracklist:

DEBUG 2014-06-24 12:17:53,779 [68156:Thread-10] mopidy.http.handlers
Received WebSocket message from 127.0.0.1: u'{"method":"core.tracklist.get_tl_tracks","jsonrpc":"2.0","id":52}'
Traceback (most recent call last):
File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/logging/init.py", line 851, in emit
msg = self.format(record)
File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/logging/init.py", line 724, in format
return fmt.format(record)
File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/logging/init.py", line 464, in format
record.message = record.getMessage()
File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/logging/init.py", line 328, in getMessage
msg = msg % self.args
TypeError: not all arguments converted during string formatting
Logged from file handlers.py, line 106

I followed the instructions for running Mopidy from GitHub. Do I also need to do something with with the extensions?

jodal added a commit to mopidy/mopidy that referenced this issue Jun 24, 2014
@jodal
Copy link
Contributor Author

jodal commented Jun 24, 2014

Should be fixed by mopidy/mopidy@dba6e9b. Thanks!

@martijnboland
Copy link
Owner

Fixed, excellent! Now I could see what went wrong: Angular adds an $$hashkey property to the js objects that isn't recognized when (de)serialized and sent to mopidy. Added a very dirty hack to mopidy.js that does the trick (by skipping serialization of $$hashkey properties), but I'm going to find a different solution that doesn't require patching mopidy.js.

@jodal
Copy link
Contributor Author

jodal commented Jun 24, 2014

Looking at the comment at http://stackoverflow.com/a/23656919/828646, I guess you can't easily do angular.toJson() as the JSON serialization happens inside Mopidy.js. However, you can maybe use ng-repeat="track in tracks by track.uri"? If you tell Angular.js about an unique field in the data, it won't add the $$hashkey property.

@martijnboland
Copy link
Owner

Looks like a neat solution. Let's give it a try.

@jodal
Copy link
Contributor Author

jodal commented Jun 25, 2014

Just remember that the track.uri isn't unique in the tracklist, since you can have multiple instances of the same track in the tracklist. tl_track.tlid is unique within the tracklist, though.

@martijnboland
Copy link
Owner

Can searches and lookups result in duplicate tracks? This is what we bind with ng-repeat, not tl_tracks. So far, everything works fine with the 'track by track.uri' solution.

@jodal
Copy link
Contributor Author

jodal commented Jun 25, 2014

As far as I can remember, there's nothing in core stopping you from having duplicate tracks in searches/lookups, but I don't see a use case for it, so if there you see any duplicates anywhere than should probably be reported as a bug against that backend.

@martijnboland
Copy link
Owner

Created a branch mopidy-0.19 and it should now be possible to install moped from here:

sudo pip install -e git+https://github.com/martijnboland/moped.git@mopidy-0.19#egg=Mopidy-Moped

@jodal jodal mentioned this issue Jun 25, 2014
@jodal
Copy link
Contributor Author

jodal commented Jun 25, 2014

This works great! It is so nice to be able to have the Mopidy client list, Lux, the API explorer and Moped running side by side. This only becomes better and better with every client joining in :-)

I sent a PR (#6) with some small fixes. Each commit is hopefully self-explanatory.

Some other barely related notes:

  • An open issue is what to do with README.rst vs README.md. Python packages just understand plain text or rst, so maybe just merge whatever is relevant from README.md into README.rst? You should be able to include the image in the README using http://docutils.sourceforge.net/docs/ref/rst/directives.html#image
  • In the future, remember that with the setup.py, you can now increase the required Mopidy version when you start using new features in the future. Lets use the dependency management tools we have to their full extent :-)
  • If you set $locationProvider.html5Mode(true) in Angular you get prettier URLs without the /#/ on all browsers that support the History API.
  • Since the Durandal version is deprecated, maybe just branch of a Durandal branch for the history books and remove all remnants of it from the master branch?

@jodal
Copy link
Contributor Author

jodal commented Jun 25, 2014

Found a bug: Playlists can have duplicate tracks, so by track.uri doesn't work:

Error: [ngRepeat:dupes] Duplicates in a repeater are not allowed. Use 'track by' expression to specify unique keys. Repeater: track in playlist.tracks track by track.uri, Duplicate key: spotify:track:5fxWongjIPVkFkIBzK10pH
http://errors.angularjs.org/1.2.18/ngRepeat/dupes?p0=track%20in%20playlist.tracks%20track%20by%20track.uri&p1=spotify%3Atrack%3A5fxWongjIPVkFkIBzK10pH
    at http://localhost:6680/moped/assets/moped-0.3.0.js:12:20112
    at http://localhost:6680/moped/assets/moped-0.3.0.js:15:26570
    at Object.d [as fn] (http://localhost:6680/moped/assets/moped-0.3.0.js:14:10734)
    at k.$digest (http://localhost:6680/moped/assets/moped-0.3.0.js:14:11413)
    at http://localhost:6680/moped/assets/moped-0.3.0.js:14:12729
    at g (http://localhost:6680/moped/assets/moped-0.3.0.js:13:5307)
    at http://localhost:6680/moped/assets/moped-0.3.0.js:13:7030

@martijnboland
Copy link
Owner

Then I'll think of something else to prevent sending $$hashkey properties.

In the near future I will create a Durandal-vs-Angular branch for historic purposes and reorganize the rest, so there's only one Moped client left.

@jodal
Copy link
Contributor Author

jodal commented Jul 22, 2014

Mopidy 0.19 was released yesterday. Would be great to have the mopidy-0.19 branch of Moped merged and get a release up on PyPI.

@martijnboland
Copy link
Owner

Done. The Mopidy-Moped package can be found at: https://pypi.python.org/pypi/Mopidy-Moped/0.3.0. Can someone give it a try and see if it works? Can't test it on the Raspberry PI because there seems to be an issue with 0.19 and Tornado 3.1.

@jodal
Copy link
Contributor Author

jodal commented Jul 23, 2014

It's working for me :-)

@jodal jodal closed this as completed Jul 23, 2014
@jodal
Copy link
Contributor Author

jodal commented Jul 23, 2014

There's some warnings during the installation which is caused by old entries in the MANIFEST.in file. You should probably have a go at running the check-manifest (pip install check-manifest) command and tweak the MANIFEST.in so that the files you want are included and you don't have any exclude rules that doesn't match any files in the repo.

I started updating MANIFEST.in locally, but figured you have to decide what to include in the pip package yourself. The following diff will fix the warnings, but there are still some files not explicitly included or excluded by it:

diff --git a/MANIFEST.in b/MANIFEST.in
index 1560c49..9b130c8 100644
--- a/MANIFEST.in
+++ b/MANIFEST.in
@@ -1,3 +1,4 @@
+include CHANGELOG.md
 include LICENSE
 include MANIFEST.in
 include README.rst
@@ -5,10 +6,8 @@ include README.rst
 recursive-include mopidy_moped ext.conf
 recursive-include mopidy_moped/static *

-exclude README.md
-
-recursive-exclude angular *
 recursive-exclude dist *
-recursive-exclude durandal *
-recursive-exclude html *
+recursive-exclude karma *
 recursive-exclude screenshots *
+recursive-exclude src *
+recursive-exclude vendor *

@martijnboland
Copy link
Owner

The manifest wasn't updated unfortunately after the source reorganization. As far as I can see everything is either included or excluded now. A new version 0.3.1 is on PyPI.

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

No branches or pull requests

2 participants