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

1.0.3 #163

Open
wants to merge 28 commits into
base: master
Choose a base branch
from
Open

1.0.3 #163

wants to merge 28 commits into from

Conversation

jeromeof
Copy link

For version 1.0.3

@jeromeof
Copy link
Author

A couple of minor fixes but allot of changes to the plexamp 'activation' script - which is optional and TBH probably should wait until Volumio has moved to bullseye as it can only activate 'old' plexamp now and that may stop working soon.

@jeromeof
Copy link
Author

@balbuze Is this new PR ok ?

@balbuze
Copy link
Collaborator

balbuze commented Nov 22, 2022

hello!
I would say yes! But the rule is to merge when stable version is release.
So make a volumio plugin submit for this version in beta.
When you think it is good enough for stable release, let me know ;-)
(you can remove i386 from package.json)

@balbuze
Copy link
Collaborator

balbuze commented Nov 22, 2022

I have tested...
When first enable, does not appears being enables (stay red)
once PIN enabled, config page don't load anymore :
image

@balbuze
Copy link
Collaborator

balbuze commented Nov 22, 2022

Nov 22 17:28:32 volumio-rpi4 volumio[823]: info: PlexAmp::connect ECONNREFUSED 127.0.0.1:32400

@jeromeof
Copy link
Author

Thanks - that is very odd - I will see why that might happen!

Once installed - the plugin won't work until it have a connection to a local plex. It uses the Plex PIN to get Authorisation from the cloud to query location of your local plex server(s). It then should save the server connection e.g. something like http://plex:32400 to config.json and should then allow the config screen to present a list of 'music' libraries (I have 3 across 2 different plex servers in my house), it will default to the first music library it finds.

Obviously the server location is somehow coming up as 127.0.0.1 which is strange. I will do some investigation this evening.

@balbuze
Copy link
Collaborator

balbuze commented Nov 22, 2022

because I helped an other user with the plugin dstmmix that need to access to server:port. The problem was my RPI4 connected with WIFI, not eth0.
So now with the code used, it works.
Same thing in FusionDsp to access camilla gui where it was implemented (getIP function)
Not sure in your case if this is the same problem but maybe to be checked...

jeromeof and others added 5 commits November 28, 2022 22:43
…Jerome O'Flaherty's git repo in

mid-May 2023 (git commit 1477ebc from 29-Nov-2022.)

This change replaces the 'net-ping' package with the 'ping' package.  The 'net-ping' package is functionally
superior, but it depends on the 'raw-socket' package, and that suffers from raw-socket issue volumio#70, which causes the
node thread running the ping to suddenly exit. Speculation is that this is caused by garbage collection
in nodejs itself.  This issue remains unresolved for over a year.

The exit of the thread causes the plugin server discovery to fail, causing the any attempt to used Plex
functionality to default to PlexAmp running on 127.0.0.1 (localhost.)  This produces a "server not found"
error unless PlexAmp is locally installed.

The plex.tv URL used to discover server information is also changed from https://plex.tv/pms/servers to
https://plex.tv/pms/resources.   The former link only shows servers that have Remote Access enabled.  Since
many local Plex servers do not have that enabled, it causes server discovery to fail.  Again, this results
in a "server not found" error when no local servers that respond to ICMP ping are found.

Discovered server records are now filtered to a standard JSON structure interface in the plexcloud getServers(),
routine rather than directly exposing the raw output of XML converted to JSON.  The currectly recognized fields are:

{
   "name": "",               # Server name, according to plex.tv (not DNS name)
   "protocol": "",           # Access protocol -- http or https
   "address": "",            # Server IP address
   "port": "",               # Server TCP port number
   "local": ""               # Boolean indicating that the address is local to the host where Plex runs when true,
                             #   or is a Remote Access IP address if false
}

The tests for this plugin used the library name "Ballyoran" to test Plex access.  This is changed to the environment
variable name LIBRARYNAME.

There are also a number of minor code fixes that just prevented the code from fully functioning.

This should allow the plugin to more fully function in line with Jerome O'Flaherty's original intentions.

The plugin setup still chooses the first server it finds.  Allowing users to choose between multiple Plex servers
and how to verify which servers are actually running are the next big issues to be resolved in this code, I think.
Added a list of all albums to the top level Plex screen

In the process of testing the new Albums list, I discovered that the transitions to show album contents were failing.
The failures occurred when looking up a list of related albums to the selected album.  That caused an access
to an undefined property for album information returned by my local Plex server.  The search for related albums
now continues only if the necessary property exists.
While using ICMP ping to detect whether a media server is reachable is a logical choice,
having ICMP ping enabled for a server on the Internet is not absolutely necessary. Cloud
services like Amazon EC2 do not enable ICMP by default for public addresses to avoid
DDoS attacks.  Having ICMP ping enabled for local media servers on private servers is a
safe assumption, but not necessarily for remote access servers.

Changed the ICMP ping to a http header request against the plex http port on the media
server.  This _must_ work to contact the media server.  The request has a 1 second timeout.
The coding is it a little weird because of how Plex works.

The ping is an unauthenticated http request, so a successful request will come back as 401
Unauthorized.  The 'request-promise' library treats 401 as an error, so the error exception
generated by the request must be handled in a 'catch' section.  On the off chance that the
http request comes back as a success in future, any 2XX response is treated as a success.

Beyond that, code in plexpinauth.js is modified to use kew like much of the rest of the code.
This allows for substantial cleanup of the code in getPinAndUpdateConfig in index.js.

Much of the code in getPinAndUpdateConfig and getUIConfig is cleaned up.  It lowers
the nesting levels of both by making better use of promises (.then and .fail) and by
replacing long references to sections in UIConfig.json with useful names.

testPlex.js is modified to test the changes in structure to getPinAndUpdateConfig and
use the http ping.

One problem I discovered after I got multiple media servers and libraries working is that the label for the
Music Library showing what the current saved setting did not work.  This turned out to be because the filter
compares both the content and the type of saved and possible properties of the music library:

var selectedLibrary = libraries.filter((filteredLibrary) => filteredLibrary.hostname === hostname &&
                      filteredLibrary.port === port && filteredLibrary.libraryTitle === libraryName);

The problem turned out to be that port was a number type (despite being loaded from a string) and
filteredLibrary.port was a string type.   The fix: make sure that port is a string type.
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

Successfully merging this pull request may close these issues.

2 participants