search

hankhank10 / music-screen-api

Display the playing Sonos track in real time on an e-ink display - also includes functionality for last.fm
120 stars 17 forks source link

Merge dev branch to master #24

Closed jjlawren closed 4 years ago

jjlawren commented 4 years ago

I think the script is in a good state and ready to merge for general use. A quick summary of high-level changes:

User-facing features:

Backend improvements:

Since I don't have an e-ink display, the go_sonos.py script has not been updated and still relies on a legacy copy of sonos_user_data.py.

jjlawren commented 4 years ago

Would be great to get feedback on the current state of dev from those that have previously submitted issues against it (@habra-cadabra @rebrook @FerretVW @v1nc3lx @rdrivas123). Have any of you pulled the latest changes and made sure that things still work smoothly?

habra-cadabra commented 4 years ago

I think dev is in great shape. it's been running really well for me. only just pulled the very latest changes, but everything else has been solid.

FerretVW commented 4 years ago

dev is working great for me on 4 separate displays (all 4" square Hyperpixel displays running on Pi3As). I have made some slight tweaks on my side to better suit my wants but only after running vanilla dev for a few days, I see no issues merging this with master. Last code pull from me was the 27th, have not tested new changes since then but can pull them tonight.

jjlawren commented 4 years ago

Thanks. Have either of you tested the new show_details_timeout config option? It was a bit finicky to implement and could perhaps use another set of eyes on the behavior.

It'll temporarily show the detailed view on track changes before reverting back to the full-screen album art. I didn't like having to choose one or the other to display at all times. Check the sonos_settings.py.example file for details if you're interested in helping test it out.

rdrivas123 commented 4 years ago

I just pulled the latest dev, I will try it out and let you know how it goes. The last pull i did everything has been working without any issues, I am actually waiting for a second Hyperpixel to show up so i can make another unit for my home. =)

habra-cadabra commented 4 years ago

oh neat, no I haven't tried that feature yet. I'll set it up now and report back shortly.

edit: tried it, skipped a few songs, looks like it's working well. turned off the artist/album detail because it gets a little cluttered with all the info there, but having the track name temporarily is great.

rdrivas123 commented 4 years ago

I pulled the latest dev, working fine for me, one thing is I have the rectangle Hyperpixel, it was working fine but now it is off on the display, I am assuming because of the difference in screen resolutions. Is there a way to change the resolution to scale to my display?

I ordered a square hyperpixel for my next unit but this one will still be used.

jjlawren commented 4 years ago

@rdrivas123 I have an idea of how to make this work on both HyperPixel screen dimensions but haven't had time to play with it as of yet. It might just work for now by adjusting these values: https://github.com/hankhank10/music-screen-api/blob/b29f2c0ec32b7da1fb0ab3515b249034379b8ff7/display_controller.py#L12-L15

FerretVW commented 4 years ago

Pulled the latest dev release to 2 of mine about 4 hours ago, everything seems to be working as intended with show_detail on track switch. I don't intend to use it that way as I would rather have them on all the time, but for testing purposes it seems like it is good.

rdrivas123 commented 4 years ago

@rdrivas123 I have an idea of how to make this work on both HyperPixel screen dimensions but haven't had time to play with it as of yet. It might just work for now by adjusting these values:

https://github.com/hankhank10/music-screen-api/blob/b29f2c0ec32b7da1fb0ab3515b249034379b8ff7/display_controller.py#L12-L15

Got the album art to adjust to the screen, there is a gray bar at the bottom when i use outside the standard parameters, not a big deal. One thing I did notice when the track title pops up is that it scales the image back outside the 480 width of the rectangle screen, Not sure if there is a way to adjust the text size down?

Everything else is working perfectly.

jjlawren commented 4 years ago

@rdrivas123 share what values you adjusted and I can try to reproduce tomorrow.

rdrivas123 commented 4 years ago

@rdrivas123 share what values you adjusted and I can try to reproduce tomorrow.

SCREEN_W = 480 SCREEN_H = 720 THUMB_W = 600 THUMB_H = 600

rebrook commented 4 years ago

Everything working well from my side on the latest dev

v1nc3lx commented 4 years ago

image got this strange behavior 🤔

jjlawren commented 4 years ago

Interesting. I think I saw something similar a few days ago also when playing on Spotify. Unfortunately I didn't catch it in time to understand why. Was there anything at all in the logs during that time?

Edit: Are you able to reproduce this reliably? I'm thinking it's some inconsistency with either the Sonos/Spotify integration or how node-sonos-http-api handles Spotify image URLs sometimes. Need hard examples to know for sure.

v1nc3lx commented 4 years ago

..in fact, It happened playing on Spotify. I tried to reproduce the behavior and no luck! I set up logs on DEBUG mode and.. wait ;) Thank you for your great work!

v1nc3lx commented 4 years ago

@jjlawren two questions: 1) how comes that "version.hankapi.com" is checked over and over again? 2) display_controller.py : wouldn't be cleaner lowering the number of pixels from art and the trackname (_label_track.place(relx=0.5, y=THUMBH + 10, anchor=tk.N) instead of _label_track.place(relx=0.5, y=THUMBH + 20, anchor=tk.N)) ?

jjlawren commented 4 years ago

@v1nc3lx are these log messages you're seeing? Can you share an example?

Do you have show_artist_and_album enabled? Or are you suggesting when it only displays the track name?

v1nc3lx commented 4 years ago

I have show_artist_and_album enabled

image

This one is with THUMB_H + 20

The next two one is with + 10 image

Can you see album name overlapping trackname?

jjlawren commented 4 years ago

Yes, that's a result of adding the station name at the bottom ("Mellow Mix" in your example). After using it for a few days I think it looks too busy and I'm going to rework it.

v1nc3lx commented 4 years ago

Got it. And what about that "version.hankapi.com" that is checked over and over again?

jjlawren commented 4 years ago

Are these log messages? Can you share some examples with context?

v1nc3lx commented 4 years ago

It's pihole log.

image

jjlawren commented 4 years ago

Do you use the online demaster option?

v1nc3lx commented 4 years ago

Offline option. Anyway I didn't listen 26k tracks during the last 3hrs.. ;)

jjlawren commented 4 years ago

@v1nc3lx I'm not quite sure as I don't have the same DNS queries on my install. Have you set a remote_debug_key in go_sonos_highres.py?

I've pushed a change to the detail display text. See if this layout fixes the overlap issues.

hankhank10 commented 4 years ago

It's pihole log.

image

It's been a while since I looked at this code, but as far as I'm aware there is nothing in the music-screen script that seeks the version number from the version api. Are you also running my vinylemulator NFC project somewhere locally as well, as this does check the version each time on startup.

As you say 26,000 requests is a lot (!) so I suspect that this script is crashing and being restarted by pm2

v1nc3lx commented 4 years ago

@v1nc3lx I'm not quite sure as I don't have the same DNS queries on my install. Have you set a remote_debug_key in go_sonos_highres.py?

no, i haven't set anything.

I've pushed a change to the detail display text. See if this layout fixes the overlap issues.

cool! it's working :)

v1nc3lx commented 4 years ago

It's pihole log.

image

It's been a while since I looked at this code, but as far as I'm aware there is nothing in the music-screen script that seeks the version number from the version api. Are you also running my vinylemulator NFC project somewhere locally as well, as this does check the version each time on startup.

As you say 26,000 requests is a lot (!) so I suspect that this script is crashing and being restarted by pm2

Yes, I'm running your cool vynilemulator on the same hw too. (another great project, mate!) Question: would it crash if the nfc tag reader is disconnected? I didn't check that.

v1nc3lx commented 4 years ago

@hankhank10 you were right: it's vynilemulator crashing+starting over and aver again. now i stopped it and continuous polling of "version.hankapi.com" is over! ;) now i just have to check if it depends on the disconnection of the tag reader and eventually find a way to avoid that when i have to disconnect it while i'm moving it to the mac for tags writing :)

hankhank10 commented 4 years ago

If vinylemulator doesn't find a reader then it will exit (not really a crash as that's the desired outcome, but effect is the same) and if you have it running through PM2 then PM2 will try to restart it on exit. Not sure how to fix this other than recommending you stop it through PM2 before disconnecting the reader. Open to other suggestions on how to handle this.

v1nc3lx commented 4 years ago

.. recommending you stop it through PM2 before disconnecting the reader.

..and that's what I'm doing now. Thanks.

jjlawren commented 4 years ago

@hankhank10 any concerns or objections to merging?

hankhank10 commented 4 years ago

No objections - please go ahead. Great work.

jjlawren commented 4 years ago

For anyone that helped during testing that's still on the dev branch, please switch back to master to ensure you don't have problems updating in the future.