Hi,

> - It adds the possibility to choose which notification you want to
> receive.
> > The idle command can now take no argument (same behavior as before) or
> > several arguments.
> > For example if a client uses command 'idle mixer elapsed', it will only
> > receive notifications when the volume changed or when the elapsed time
> > changed.
>
> Good idea.
>
> Someone proposed that "idle 10" would make MPD respond after 10
> seconds, disregarding whether an event has really happened, and
> probably returns with an empty event set.
>
> Can you make a separate patch of that?  And maybe implement the
> timeout thing if you have time...

I attach a separate patch. What would be the advantage of this timeout
thing? I don't see in which use case it can be useful.


> > - And last, with this patch MPD doesn't leave idle mode anymore when a
> > notification has been done. It means that idle mode will continue until
> > 'noidle' command is sent. I think this will easy client implementation of
> > idle command and reduce a lot the amount of useless traffic. Please tell
> me
> > if you see a major drawback with this behavior.
>
> No, I think the disadvantages of this solution will prevail here.
> Events aren't that often (unless we really implement your 'elapsed'
> event), and after most events, the client has to query the server
> anyway.

Could you list these disadvantages? In fact I don't see any....



>  flags = ~0;
>
Thanks, it's much better like this ;-)

Marc
diff --git a/src/client.c b/src/client.c
index ee98464..fd55b31 100644
--- a/src/client.c
+++ b/src/client.c
@@ -91,6 +91,9 @@ struct client {
 	/** idle flags pending on this client, to be sent as soon as
 	    the client enters "idle" */
 	unsigned idle_flags;
+
+	/** idle flags that the client wants to receive */
+	unsigned idle_subscriptions;
 };
 
 static LIST_HEAD(clients);
@@ -766,16 +769,6 @@ mpd_fprintf void client_printf(struct client *client, const char *fmt, ...)
 	va_end(args);
 }
 
-static const char *const idle_names[] = {
-	"database",
-	"stored_playlist",
-	"playlist",
-	"player",
-	"mixer",
-	"output",
-	"options",
-};
-
 /**
  * Send "idle" response to this client.
  */
@@ -783,6 +776,7 @@ static void
 client_idle_notify(struct client *client)
 {
 	unsigned flags, i;
+	const char *const* idle_names;
 
 	assert(client->idle_waiting);
 	assert(client->idle_flags != 0);
@@ -791,10 +785,9 @@ client_idle_notify(struct client *client)
 	client->idle_flags = 0;
 	client->idle_waiting = false;
 
-	for (i = 0; i < sizeof(idle_names) / sizeof(idle_names[0]); ++i) {
-		assert(idle_names[i] != NULL);
-
-		if (flags & (1 << i))
+	idle_names = idle_get_names();
+	for (i = 0; idle_names[i]; ++i) {
+		if (flags & (1 << i) & client->idle_subscriptions)
 			client_printf(client, "changed: %s\n",
 				      idle_names[i]);
 	}
@@ -814,20 +807,22 @@ void client_manager_idle_add(unsigned flags)
 			continue;
 
 		client->idle_flags |= flags;
-		if (client->idle_waiting) {
+		if (client->idle_waiting
+		    && (client->idle_flags & client->idle_subscriptions)) {
 			client_idle_notify(client);
 			client_write_output(client);
 		}
 	}
 }
 
-bool client_idle_wait(struct client *client)
+bool client_idle_wait(struct client *client, unsigned flags)
 {
 	assert(!client->idle_waiting);
 
 	client->idle_waiting = true;
+	client->idle_subscriptions = flags;
 
-	if (client->idle_flags != 0) {
+	if (client->idle_flags & client->idle_subscriptions) {
 		client_idle_notify(client);
 		return true;
 	} else
diff --git a/src/client.h b/src/client.h
index e6661e2..9c70318 100644
--- a/src/client.h
+++ b/src/client.h
@@ -78,6 +78,6 @@ void client_manager_idle_add(unsigned flags);
  * sent immediately and "true" is returned".  If no, it puts the
  * client into waiting mode and returns false.
  */
-bool client_idle_wait(struct client *client);
+bool client_idle_wait(struct client *client, unsigned flags);
 
 #endif
diff --git a/src/command.c b/src/command.c
index dc71f1d..6a9489d 100644
--- a/src/command.c
+++ b/src/command.c
@@ -39,6 +39,7 @@
 #include "tag_print.h"
 #include "path.h"
 #include "os_compat.h"
+#include "idle.h"
 
 #define COMMAND_STATUS_VOLUME           "volume"
 #define COMMAND_STATUS_STATE            "state"
@@ -1253,8 +1254,28 @@ static enum command_return
 handle_idle(struct client *client,
 	    mpd_unused int argc, mpd_unused char *argv[])
 {
+        unsigned flags = 0, j;
+        int i;
+        const char *const* idle_names;
+
+        idle_names = idle_get_names();
+        for (i = 1; i < argc; ++i) {
+                if (!argv[i])
+                        continue;
+
+                for (j = 0; idle_names[j]; ++j) {
+                        if (!strcasecmp(argv[i], idle_names[j])) {
+                                flags |= (1 << j);
+                        }
+                }
+        }
+
+        /* No argument means that the client wants to receive everything */
+        if (flags == 0)
+                flags = ~0;
+
 	/* enable "idle" mode on this client */
-	client_idle_wait(client);
+	client_idle_wait(client, flags);
 
 	/* return value is "1" so the caller won't print "OK" */
 	return 1;
@@ -1280,7 +1301,7 @@ static const struct command commands[] = {
 	{ "disableoutput", PERMISSION_ADMIN, 1, 1, handle_disableoutput },
 	{ "enableoutput", PERMISSION_ADMIN, 1, 1, handle_enableoutput },
 	{ "find", PERMISSION_READ, 2, -1, handle_find },
-	{ "idle", PERMISSION_READ, 0, 0, handle_idle },
+	{ "idle", PERMISSION_READ, 0, -1, handle_idle },
 	{ "kill", PERMISSION_ADMIN, -1, -1, handle_kill },
 	{ "list", PERMISSION_READ, 1, -1, handle_list },
 	{ "listall", PERMISSION_READ, 0, 1, handle_listall },
diff --git a/src/idle.c b/src/idle.c
index c779d0a..884086a 100644
--- a/src/idle.c
+++ b/src/idle.c
@@ -30,6 +30,18 @@
 static unsigned idle_flags;
 static pthread_mutex_t idle_mutex = PTHREAD_MUTEX_INITIALIZER;
 
+static const char *const idle_names[] = {
+	"database",
+	"stored_playlist",
+	"playlist",
+	"player",
+	"mixer",
+	"output",
+	"options",
+	"elapsed",
+        NULL
+};
+
 void
 idle_add(unsigned flags)
 {
@@ -54,3 +66,9 @@ idle_get(void)
 
 	return flags;
 }
+
+const char*const*
+idle_get_names(void)
+{
+        return idle_names;
+}
diff --git a/src/idle.h b/src/idle.h
index 69756b1..5079f09 100644
--- a/src/idle.h
+++ b/src/idle.h
@@ -61,4 +61,10 @@ idle_add(unsigned flags);
 unsigned
 idle_get(void);
 
+/**
+ * Get idle names
+ */
+const char*const*
+idle_get_names(void);
+
 #endif
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team

Reply via email to