LMS-Community/slimserver

"shufflemode" missing from "alarms" query.

CDrummond opened this issue · 4 comments

Material used the "alarms" JSONRPC call to get list of alarms for a player. Currently Material cannot set the shuffle-mode of an alarm. This is due to the fact that "shufflemode" is not part of the alarm details in the "alarms" call.

The following diff fixes this:

diff --git a/Slim/Control/Queries.pm b/Slim/Control/Queries.pm
index df5297b27..ff47ab1f7 100644
--- a/Slim/Control/Queries.pm
+++ b/Slim/Control/Queries.pm
@@ -237,6 +237,7 @@ sub alarmsQuery {
                        $request->addResultLoop($loopname, $cnt, 'repeat', $alarm->repeat());
                        $request->addResultLoop($loopname, $cnt, 'time', $alarm->time());
                        $request->addResultLoop($loopname, $cnt, 'volume', $alarm->volume());
+                       $request->addResultLoop($loopname, $cnt, 'shufflemode', $alarm->shufflemode());
                        $request->addResultLoop($loopname, $cnt, 'url', $alarm->playlist() || 'CURRENT_PLAYLIST');
                        $cnt++;
                }

Is this kind of a PR to add that feature, or is this fixing a regression?

Well, that's your call I guess. To me its a bug as all other parameters are part of this call, just not this one. This parameter is also part of the call to create an alarm.

I was more wondering whether it was a regression, something that existed before, but had been lost somewhere somewhen. I take it that's not the case?

No, I don't think it existed before. When I was looking to add this feature to Material I noted I hard already (some time ago) written code to set shuffle mode - but it was disabled due to not being able to tell what the current mode for the alarm was.

So yes, its been missing for a while - but (IMVHO) it's something that should have been there from the start. However, its such a minor thing that its probably not worth backporting (but is only 1 line, so see no harm).