-
-
Notifications
You must be signed in to change notification settings - Fork 204
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
feat(config): Allow users to set AP port #1420
Conversation
src/spotify.rs
Outdated
let ap_port = cfg.values().ap_port.unwrap_or(0); | ||
if ap_port != 0 { | ||
session_config.ap_port = Some(ap_port); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this intermediate 0
value? Could we also do it like this?
let ap_port = cfg.values().ap_port.unwrap_or(0); | |
if ap_port != 0 { | |
session_config.ap_port = Some(ap_port); | |
} | |
session_config.ap_port = cfg.values().ap_port; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was not sure if that would behave in the correct way (no parameter is set) when the ap-port
option is missing in the config file. If that's the case, please feel free to apply the change, I wont be able in a couple of days.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By default librespot sets ap_port
to None
: https://github.com/librespot-org/librespot/blob/4d35c5ffe222c839d1532d8afac78fe4c37043b3/core/src/config.rs#L33
However, if we wanted to be absolutely sure we could also use an if let
expression here.
Describe your changes
Added a config option
ap-port
to fix the port used to connect tospotify
and pass it tolibrespot
. Fixes issues with restrictive firewalls, after settingap-port = 443
in config file.Issue ticket number and link
Fixes #1404
Checklist before requesting a review
not performance improvements, etc.)