fix issues with the discovery feature

If mprisqueeze runs behing a firewall, as no answer can arrive from the
LMS server, it will flood the network with UDP broadcast messages
causing many issues.

This is resolved by sending the UDP broadcast message and waiting for the
answer synchronously.

Somehow related, squeelite also try to discover the LMS server using the
same technic but not using connected mode. This raises another issue
with the firewall. iptables does not recognize a connected state between
a broadcast message and its response.

This is resolved by passing the IP of the server to squeelite.

README.md

@@ -13,24 +13,33 @@ network. To specify a host and a port:
 $ mprisqueeze -H somehost -P 9000
 ```
 
-The default command line for [squeezelite] is: `squeezelite -n SqueezeLite`. It 
-starts [squeezelite] registering itself on [LMS] with the name `SqueezeLite`. 
-To use another name, one can use:
+The default command line for [squeezelite] is:
+
+```
+squeezelite -n {name} -s {server}
+```
+
+Before calling [squeezelite], `mprisqueeze` replaces:
+- `{name}` by the name of the player, `Squeezelite` by default
+- `{server}` by the LMS server IP, either automatically discovered either set 
+  with the `-H` switch
+
+It then starts [squeezelite] registering itself on [LMS] with the name 
+`SqueezeLite`. To use another name, one can use:
 
 ```bash
 $ mprisqueeze -p my-player
 ```
 
 The command to start [squeezelite] can be changed with the last arguments, 
-preceded by a `--`, for example:
+preceded by `--`, for example:
 
 ```bash
-$ mprisqueeze -- squeezelite -f ./squeezelite.log -n {}
+$ mprisqueeze -- squeezelite -f ./squeezelite.log -n {name} -s {server}
 ```
 
-Note that `mprisqueeze` must know the name [squeezelite] will use. Therefore 
-the [squeezelite] command line must contains the string `{}`. It is replaced by 
-the player name when starting the process.
+Note that when using a custom command, both parameters must be present on the 
+command line: `{name}` and `{server}`.
 
 [status]: https://github.com/jecaro/mprisqueeze/actions
 [status-png]: https://github.com/jecaro/mprisqueeze/workflows/CI/badge.svg

src/discover.rs

@@ -1,14 +1,14 @@
 use anyhow::Result;
-use log::info;
+use log::{info, warn};
 use nom::{
     bytes::{self, complete::tag},
     combinator::{flat_map, map, map_res},
     number,
     sequence::{preceded, tuple},
     IResult,
 };
-use std::io::ErrorKind;
-use tokio::net::UdpSocket;
+use std::time::Duration;
+use tokio::{net::UdpSocket, time::timeout};
 
 #[derive(Debug)]
 pub struct Reply {
@@ -26,26 +26,22 @@ pub struct Reply {
 // itself in the next length bytes.
 
 /// Discover the LMS server on the local network
-pub async fn discover() -> Result<Reply> {
+pub async fn discover(reply_timeout: Duration) -> Result<Reply> {
     info!("Discovering LMS server on the local network");
-    let message = "eNAME\0JSON\0UUID\0VERS\0".as_bytes();
 
     let sock = UdpSocket::bind("0.0.0.0:0").await?;
     sock.set_broadcast(true)?;
 
     let mut buf = [0; 1024];
-    loop {
-        let _ = sock.send_to(&message, "255.255.255.255:3483").await?;
 
-        match sock.try_recv(&mut buf) {
-            Err(ref e) if e.kind() == ErrorKind::WouldBlock => {
-                continue;
-            }
-            anything_else => {
-                break anything_else;
-            }
+    loop {
+        let response = timeout(reply_timeout, broasdcast_and_recv(&mut buf, &sock)).await;
+        match response {
+            Ok(Ok(())) => break,
+            Ok(Err(e)) => return Err(e.into()),
+            Err(_) => warn!("Timeout waiting for LMS reply, retrying..."),
         }
-    }?;
+    }
 
     parse_reply(&buf)
         .map(|(_, reply)| {
@@ -58,6 +54,13 @@ pub async fn discover() -> Result<Reply> {
         .map_err(|error| error.to_owned().into())
 }
 
+async fn broasdcast_and_recv(buf: &mut [u8], sock: &UdpSocket) -> Result<()> {
+    let message = "eNAME\0JSON\0UUID\0VERS\0".as_bytes();
+    let _ = sock.send_to(&message, "255.255.255.255:3483").await?;
+    let _ = sock.recv(buf).await?;
+    Ok(())
+}
+
 fn parse_tag<'a>(input: &'a [u8], start_tag: &str) -> IResult<&'a [u8], String> {
     map_res(
         preceded(

src/main.rs

@@ -32,16 +32,25 @@ struct Options {
     )]
     player_timeout: u64,
     #[arg(
-        short = 'T',
+        short = 'd',
         long,
         default_value_t = 3,
         help = "Timeout in seconds for LMS discovery"
     )]
     discover_timeout: u64,
+    #[arg(
+        short = 'r',
+        long,
+        default_value_t = 100,
+        help = "Timeout in milliseconds for LMS to reply to the discovery message"
+    )]
+    discover_reply_timeout: u64,
     #[arg(
         last = true,
-        default_values_t = vec!["squeezelite".to_string(), "-n".to_string(), "{}".to_string()],
-        help = "Player command and arguments. The string '{}' will be replaced with the player name."
+        default_values_t = vec!["squeezelite-pulse".to_string(), "-n".to_string(),
+            "{name}".to_string(), "-s".to_string(), "{server}".to_string()],
+        help = "Player command and arguments. The string '{name}' will be replaced with the player\
+                name, '{server}' with the LMS server name."
     )]
     player_command: Vec<String>,
 }
@@ -70,19 +79,22 @@ async fn wait_for_player(client: &LmsClient, player_name: &str, timeout: u64) ->
 }
 
 /// Start the `squeezelite` process
-fn start_squeezelite(options: &Options) -> Result<Child> {
+fn start_squeezelite(options: &Options, server: &String) -> Result<Child> {
     let (player_command, player_args) = match options.player_command[..] {
         [] => bail!("No player command given"),
         [ref player_command, ref player_args @ ..] => Ok((player_command, player_args)),
     }?;
 
-    // put the player name into the command line arguments
-    if !player_args.iter().any(|arg| arg.contains("{}")) {
-        bail!("Player args must contain the string {{}} to be replaced with the player name");
+    if !player_args.iter().any(|arg| arg.contains("{name}")) {
+        bail!("Player args must contain the string {{name}} to be replaced with the player name");
+    }
+    if !player_args.iter().any(|arg| arg.contains("{server}")) {
+        bail!("Player args must contain the string {{server}} to be replaced with the server name");
     }
     let player_args_with_name = player_args
         .iter()
-        .map(|arg| arg.replace("{}", &options.player_name))
+        .map(|arg| arg.replace("{name}", &options.player_name))
+        .map(|arg| arg.replace("{server}", &server))
         .collect::<Vec<_>>();
 
     info!(
@@ -112,14 +124,18 @@ async fn main() -> Result<()> {
             ..
         } => (hostname.clone(), port),
         _ => {
-            let reply =
-                timeout(Duration::from_secs(options.discover_timeout), discover()).await??;
+            let reply = timeout(
+                Duration::from_secs(options.discover_timeout),
+                discover(Duration::from_millis(options.discover_reply_timeout)),
+            )
+            .await??;
+            println!("Discovered LMS at {}:{}", reply.hostname, reply.port);
             (reply.hostname, reply.port)
         }
     };
 
     // start squeezelite
-    let mut player_process = start_squeezelite(&options)?;
+    let mut player_process = start_squeezelite(&options, &hostname)?;
 
     let result: Result<()> = (|| async {
         // wait for the player to be available