PDA

View Full Version : Adding a player parameter



Aesculus
2010-01-24, 19:06
Can a plugin add a parameter to a player that can be seen by other plugins without knowing the origin? ie the parameter could be queried by any program or plugin in the system by the player itself versus as a parameter of a given plugin.

We would like to be able to create a parameter that can be exchanged, or at least viewed by various components that is plugin independent.

For example a parameter that announces to interested programs that at least one plugin has verified that it will be able to respond to volume changes in the player even when the user has the volume set to 100% fixed.

peterw
2010-01-24, 21:52
Can a plugin add a parameter to a player that can be seen by other plugins without knowing the origin? ie the parameter could be queried by any program or plugin in the system by the player itself versus as a parameter of a given plugin.

We would like to be able to create a parameter that can be exchanged, or at least viewed by various components that is plugin independent.

For example a parameter that announces to interested programs that at least one plugin has verified that it will be able to respond to volume changes in the player even when the user has the volume set to 100% fixed.

#1: nested CLI commands

One option would be to devise a new CLI query command. Any plugin could implement it via addDispatch. And if a plugin saw that some other plugin had registered that command and did not control the volume for the player in question, it should gracefully call the other registered implementation -- for instance, somebody might have both Denon-specific and Yamaha-specific RS232 plugins loaded, and use each for different Squeezeboxes. Maybe the command would look like

00:04:20:11:22:33 hasexternalvolumecontrol ?
with the response of "0" for "no", "1" for yes, and maybe "+/-" for a player using something like IR Blaster that can take "up"/"down" changes but cannot accept exact volumes. Or maybe there should be a separate query to differentiate between the precise control Denon allows and the up/down control of IR Blaster, e.g.

00:04:20:11:22:33 preciseexternalvolume ?
and plugins like DenonAVP Control and DenonSerial would indicate "1" for both (and iPeng would give those players normal UI treatment). A player for which preciseexternalvolume was 0 might have simple Up/Down controls in software like iPeng but not have nicer features like relative volume change with sync group partners. If an app like iPeng got CLI errors (as with SBS systems that didn't have external volume control plugins installed), it would treat those as equivalent to "0". Apps like iPeng should only need to query once per player per invocation of the control app -- if a user enabled or disabled Denon AVP Control, it's reasonable to expect he should restart iPeng.

#2: agreed-upon client prefs

Alternately, you could simply set a preference in the "server" space, e.g.

preferences('server')->client($client)->set('x-external-volume','1');
preferences('server')->client($client)->set('x-external-precise-volume','1');
but the trick would be that you should probably unset that from a shutdownPlugin() method in case the user restarted SBS with your plugin disabled --but only for players for which you control external volume!

Or we could agree on a new preference namespace and do something like

preferences('volumecontrol')->client($client)->set('external','1');
preferences('volumecontrol')->client($client)->set('precise','1');


#3: notifications

A third option would be a twist on the first. Is it easy for iPeng to subscribe to notifications from SBS? If so, we could agree on a CLI command to request external volume info, e.g. iPeng could send a single CLI command like

getexternalvolumeinfo
and each plugin like ours would use the notifyFromArray() API to give information about any players for which it controlled external amp volume, e.g.

Slim::Control::Request::notifyFromArray($client, ['externalvolume', 1, 1]);
might signify that $client had external volume control despite the 100% locking (the first "1") and that its volume could be controlled precisely (the second "1", e.g. mixer volume 50 vs. mixer volume +10). iPeng would subscribe to 'externalvolume' notifications and process any it got after sending the 'getexternalvolume' command. This 3rd option might be the easiest for plugin developers, and least likely to have problems if a user had a mix of different external volume control plugins, as all plugins could share pretty much the same code for implementing 'getexternalvolume'.

Aesculus
2010-01-24, 22:10
And the last even easier for us to implement but not necessarily for Pippin is that he add a setting for a player through his settings page that states it should still process volume controls even though the volume is locked at 100%. That only solves the problem for iPeng though and it puts all the pressure on Pippin and the user to manage it.

I am worried that without some way to back out settings (as you have described above) then one plugin could add a setting and not have a way to remove it (it would have to be done during the clearCallback? routine) without effecting someone elses rights to the same feature (for example someone replaced my plugin with yours and I took the parameter away when I was disabled).

pippin
2010-01-25, 01:38
Well, I kind of like #3 but would especially prefer it if IRBlaster could buy into this, too.

The most simple solution is of course #2, but as Aesculus pointed out, that does have some issues with reverting the setting. Even today I did have several occasions (not just you :) ) where people had left-over IRBlaster settings in their prefs after uninstalling IRBlaster so iPeng erroneously uses that control.

I believe the UNDERLYING issue here is a broken state model.
The player reports a state to the client (iPeng; state = "you can't control my volume") while in between a plugin gets in the way and tries to do exactly that (control the volume).
In a proper state model, the server would report an effective state, independent of which kind of flag is set on the player side so that you can hook into that logic.
After all: what is the sense of this player state (digitalVolumeControl = 0) if all clients just ignore it? After all, this occupies quite a bit of bandwidth since it's sent every 10s to each Squeezeplay based client plus every 30s to the WebUI. Pretty much for something supposed to be ignored.

So I still believe the _real_ solution would be a way to get into the state model and modify that very state before it gets sent to the client (controller).

Aesculus
2010-01-25, 11:29
Well, I kind of like #3 but would especially prefer it if IRBlaster could buy into this, too.



I am ready to do #3 and its sounds like Peter is too. 2 out of 3 ain't bad :-)

Perhaps you can influence IRBLASTER
Felix Mueller <felix(dot)mueller(at)gwendesign(dot)com>
to join in too.

peterw
2010-01-25, 21:06
two changes:
1) the notification must use an existing CLI command name, so the notification here uses the same name as the CLI command that's used to request info
2) I added another argument to the notifications -- a string naming the plugin that provides the external volume control

Obviously usingDenonSerial() is a method I wrote (already had it) to report true if a given player used my plugin for external amp/volume control, and you'll need to replace this with your own logic.

If this works OK, I'd be happy to take a stab at a patch for IR Blaster -- hopefully Felix will be amenable to including that in a new release. If this doesn't work well, no need to bother him. :-)


# --------------------------------------- external volume indication code -------------------------------
# at the top of Plugin.pm, define a scalar to hold a pointer to the getexternalcolumeinfo CLI command
# my $getexternalvolumeinfoCoderef;
# set $getexternalvolumeinfoCoderef in the initPlugin() method with a line like the following:
# $getexternalvolumeinfoCoderef = Slim::Control::Request::addDispatch(['getexternalvolumeinfo'],[0, 0, 0, \&getexternalvolumeinfoCLI]);

sub getexternalvolumeinfoCLI {
my @args = @_;
&reportOnOurPlayers();
if ( defined($getexternalvolumeinfoCoderef) ) {
# chain to the next implementation
return &$getexternalvolumeinfoCoderef(@args);
}
# else we're authoritative
my $request = $args[0];
$request->setStatusDone();
}

sub reportOnOurPlayers() {
# loop through all currently attached players
foreach my $client (Slim::Player::Client::clients()) {
if (&usingDenonSerial($client) ) {
# using our volume control, report on our capabilities
$log->debug("Note that ".$client->name()." uses us for external volume control");
Slim::Control::Request::notifyFromArray($client, ['getexternalvolumeinfo', 1, 1, string(&getDisplayName())]);
# ^ uses software-controlled external volume
# despite SB output being fixed at 100%
# ^ can set precise volumes (vs relative changes)
# ^ string identifying who provides this control
}
}
}


# --------------------------------------- external volume indication code -------------------------------

peterw
2010-01-25, 21:23
Oh, and I just published DenonSerial version 0.1.29 to my test repository, http://www.tux.org/~peterw/slim/slim7/repodata-test.xml, with this code in place. I haven't tested it with an Ethernet-equipped Denon, but it should work with your gear if you set Amplifier Connection Type to "Network address" and put your Denon's IP in the Amplifier Address field. Watch the poll interval numbers; it looks like those might not be defaulting to sane values. The defaults should be 2 for active, and 5 for idle. Good news is that the code (in one spot intentionally, and in another accidentally) seems to treat poll frequency 0 as "do not poll".

Actually, I'd love to hear if it works OK with your real gear. I've tested my lesser amp using a netbook running serial port software, but don't have access to a nicer, Ethernet-equipped Denon amp. :-)

Aesculus
2010-01-25, 21:25
OK. If Pippin sends us a beta iPeng we can give it a try.

Do you think we need to have an option for people to ask us to support this? ie in my last attempt I had a setting to indicate they wanted to support volume passthrough.

Aesculus
2010-01-25, 21:58
Oh, and I just published DenonSerial version 0.1.29 to my test repository, http://www.tux.org/~peterw/slim/slim7/repodata-test.xml, with this code in place. I haven't tested it with an Ethernet-equipped Denon, but it should work with your gear if you set Amplifier Connection Type to "Network address" and put your Denon's IP in the Amplifier Address field. Watch the poll interval numbers; it looks like those might not be defaulting to sane values. The defaults should be 2 for active, and 5 for idle. Good news is that the code (in one spot intentionally, and in another accidentally) seems to treat poll frequency 0 as "do not poll".

Actually, I'd love to hear if it works OK with your real gear. I've tested my lesser amp using a netbook running serial port software, but don't have access to a nicer, Ethernet-equipped Denon amp. :-)

ON - worked


0027: [10-01-25 20:52:40.5687] Plugins::DenonSerial::Plugin::closeTCPSocket (628) closing TCP connection for IP:192.168.1.102
0026: [10-01-25 20:52:38.5769] Plugins::DenonSerial::Plugin::processResponses (733) for 00:04:20:16:5f:a0, command "MV?", process response "MVMAX 705"
0025: [10-01-25 20:52:38.5689] Plugins::DenonSerial::Plugin::processResponses (733) for 00:04:20:16:5f:a0, command "MV?", process response "MV45"
0024: [10-01-25 20:52:38.2886] Plugins::DenonSerial::Plugin::getResponse (513) for 00:04:20:16:5f:a0, command "MV?", got response "MVMAX 705" from remote host
0023: [10-01-25 20:52:38.2794] Plugins::DenonSerial::Plugin::getResponse (513) for 00:04:20:16:5f:a0, command "MV?", got response "MV45" from remote host
0022: [10-01-25 20:52:38.2702] Plugins::DenonSerial::Plugin::getResponse (480) for 00:04:20:16:5f:a0, indicate we're done with the current command since we're reading responses, queue length now 1
0021: [10-01-25 20:52:38.2619] Plugins::DenonSerial::Plugin::processCommandFromQu eue (409) look for response for 00:04:20:16:5f:a0 command MV?
0020: [10-01-25 20:52:37.9917] Plugins::DenonSerial::Plugin::processCommandFromQu eue (447) not done with queue
0019: [10-01-25 20:52:37.9835] Plugins::DenonSerial::Plugin::sendCommand (886) for 00:04:20:16:5f:a0, indicate we're working on a command, queue length now 1
0018: [10-01-25 20:52:37.9727] Plugins::DenonSerial::Plugin::sendCommand (815) send command: (connection IP, address 192.168.1.102, command MV?)
0017: [10-01-25 20:52:37.9644] Plugins::DenonSerial::Plugin::processCommandFromQu eue (436) for 00:04:20:16:5f:a0 , send command MV?, queue length 1
0016: [10-01-25 20:52:37.8311] Plugins::DenonSerial::Plugin::processCommandFromQu eue (447) not done with queue
0015: [10-01-25 20:52:37.8227] Plugins::DenonSerial::Plugin::processResponses (733) for 00:04:20:16:5f:a0, command "PWON", process response "ZMON"
0014: [10-01-25 20:52:37.8143] Plugins::DenonSerial::Plugin::processResponses (733) for 00:04:20:16:5f:a0, command "PWON", process response "PWON"
0013: [10-01-25 20:52:37.7148] Plugins::DenonSerial::Plugin::getResponse (513) for 00:04:20:16:5f:a0, command "PWON", got response "ZMON" from remote host
0012: [10-01-25 20:52:37.4667] Plugins::DenonSerial::Plugin::getResponse (513) for 00:04:20:16:5f:a0, command "PWON", got response "PWON" from remote host
0011: [10-01-25 20:52:37.4556] Plugins::DenonSerial::Plugin::getResponse (480) for 00:04:20:16:5f:a0, indicate we're done with the current command since we're reading responses, queue length now 2
0010: [10-01-25 20:52:37.4472] Plugins::DenonSerial::Plugin::processCommandFromQu eue (409) look for response for 00:04:20:16:5f:a0 command PWON
0009: [10-01-25 20:52:37.0667] Plugins::DenonSerial::Plugin::processCommandFromQu eue (447) not done with queue
0008: [10-01-25 20:52:37.0588] Plugins::DenonSerial::Plugin::sendCommand (886) for 00:04:20:16:5f:a0, indicate we're working on a command, queue length now 2
0007: [10-01-25 20:52:37.0448] Plugins::DenonSerial::Plugin::sendCommand (842) opening control socket to 192.168.1.102:23
0006: [10-01-25 20:52:37.0342] Plugins::DenonSerial::Plugin::sendCommand (818) send command: (connection IP, address 192.168.1.102, command PWON)
0005: [10-01-25 20:52:37.0259] Plugins::DenonSerial::Plugin::processCommandFromQu eue (436) for 00:04:20:16:5f:a0 , send command PWON, queue length 2
0004: [10-01-25 20:52:35.7411] Plugins::DenonSerial::Plugin::enqueue (380) run MV? for 00:04:20:16:5f:a0 at next convenience
0003: [10-01-25 20:52:35.7303] Plugins::DenonSerial::Plugin::enqueue (384) indicate that processCommandFromQueue() should run
0002: [10-01-25 20:52:35.7211] Plugins::DenonSerial::Plugin::enqueue (380) run PWON for 00:04:20:16:5f:a0 at next convenience
0001: [10-01-25 20:52:35.7114] Plugins::DenonSerial::Plugin::turnAmpOn (1133) turning amp on for Family Room

Aesculus
2010-01-25, 21:58
OFF - did not work


0042: [10-01-25 20:55:32.5889] Plugins::DenonSerial::Plugin::closeTCPSocket (628) closing TCP connection for IP:192.168.1.102
0041: [10-01-25 20:55:30.5821] Plugins::DenonSerial::Plugin::processResponses (733) for 00:04:20:16:5f:a0, command "PW?", process response "PWON"
0040: [10-01-25 20:55:30.3014] Plugins::DenonSerial::Plugin::getResponse (513) for 00:04:20:16:5f:a0, command "PW?", got response "PWON" from remote host
0039: [10-01-25 20:55:30.2912] Plugins::DenonSerial::Plugin::getResponse (480) for 00:04:20:16:5f:a0, indicate we're done with the current command since we're reading responses, queue length now 1
0038: [10-01-25 20:55:30.2828] Plugins::DenonSerial::Plugin::processCommandFromQu eue (409) look for response for 00:04:20:16:5f:a0 command PW?
0037: [10-01-25 20:55:29.8858] Plugins::DenonSerial::Plugin::processCommandFromQu eue (447) not done with queue
0036: [10-01-25 20:55:29.8776] Plugins::DenonSerial::Plugin::sendCommand (886) for 00:04:20:16:5f:a0, indicate we're working on a command, queue length now 1
0035: [10-01-25 20:55:29.8669] Plugins::DenonSerial::Plugin::sendCommand (815) send command: (connection IP, address 192.168.1.102, command PW?)
0034: [10-01-25 20:55:29.8585] Plugins::DenonSerial::Plugin::processCommandFromQu eue (436) for 00:04:20:16:5f:a0 , send command PW?, queue length 1
0033: [10-01-25 20:55:29.7369] Plugins::DenonSerial::Plugin::processCommandFromQu eue (447) not done with queue
0032: [10-01-25 20:55:29.7272] Plugins::DenonSerial::Plugin::processResponses (733) for 00:04:20:16:5f:a0, command "SI?", process response "SVSOURCE"
0031: [10-01-25 20:55:29.7160] Plugins::DenonSerial::Plugin::processResponses (733) for 00:04:20:16:5f:a0, command "SI?", process response "SICD"
0030: [10-01-25 20:55:29.7077] Plugins::DenonSerial::Plugin::processResponses (733) for 00:04:20:16:5f:a0, command "SI?", process response "MVMAX 705"
0029: [10-01-25 20:55:29.6863] Plugins::DenonSerial::Plugin::updateSBVolume (332) for Family Room amp volume 45 =~ SB volume 65
0028: [10-01-25 20:55:29.6770] Plugins::DenonSerial::Plugin::processResponses (733) for 00:04:20:16:5f:a0, command "SI?", process response "MV45"
0027: [10-01-25 20:55:29.3063] Plugins::DenonSerial::Plugin::getResponse (513) for 00:04:20:16:5f:a0, command "SI?", got response "SVSOURCE" from remote host
0026: [10-01-25 20:55:29.2969] Plugins::DenonSerial::Plugin::getResponse (513) for 00:04:20:16:5f:a0, command "SI?", got response "SICD" from remote host
0025: [10-01-25 20:55:29.2882] Plugins::DenonSerial::Plugin::getResponse (513) for 00:04:20:16:5f:a0, command "SI?", got response "MVMAX 705" from remote host
0024: [10-01-25 20:55:29.2790] Plugins::DenonSerial::Plugin::getResponse (513) for 00:04:20:16:5f:a0, command "SI?", got response "MV45" from remote host
0023: [10-01-25 20:55:29.2701] Plugins::DenonSerial::Plugin::getResponse (480) for 00:04:20:16:5f:a0, indicate we're done with the current command since we're reading responses, queue length now 2
0022: [10-01-25 20:55:29.2618] Plugins::DenonSerial::Plugin::processCommandFromQu eue (409) look for response for 00:04:20:16:5f:a0 command SI?
0021: [10-01-25 20:55:28.9701] Plugins::DenonSerial::Plugin::processCommandFromQu eue (447) not done with queue
0020: [10-01-25 20:55:28.9619] Plugins::DenonSerial::Plugin::sendCommand (886) for 00:04:20:16:5f:a0, indicate we're working on a command, queue length now 2
0019: [10-01-25 20:55:28.9511] Plugins::DenonSerial::Plugin::sendCommand (815) send command: (connection IP, address 192.168.1.102, command SI?)
0018: [10-01-25 20:55:28.9427] Plugins::DenonSerial::Plugin::processCommandFromQu eue (436) for 00:04:20:16:5f:a0 , send command SI?, queue length 2
0017: [10-01-25 20:55:28.8378] Plugins::DenonSerial::Plugin::processCommandFromQu eue (447) not done with queue
0016: [10-01-25 20:55:28.5578] Plugins::DenonSerial::Plugin::getResponse (480) for 00:04:20:16:5f:a0, indicate we're done with the current command since we're reading responses, queue length now 3
0015: [10-01-25 20:55:28.5494] Plugins::DenonSerial::Plugin::processCommandFromQu eue (409) look for response for 00:04:20:16:5f:a0 command MV?
0014: [10-01-25 20:55:28.4264] Plugins::DenonSerial::Plugin::processCommandFromQu eue (447) not done with queue
0013: [10-01-25 20:55:28.4184] Plugins::DenonSerial::Plugin::sendCommand (886) for 00:04:20:16:5f:a0, indicate we're working on a command, queue length now 3
0012: [10-01-25 20:55:28.4048] Plugins::DenonSerial::Plugin::sendCommand (842) opening control socket to 192.168.1.102:23
0011: [10-01-25 20:55:28.3938] Plugins::DenonSerial::Plugin::sendCommand (815) send command: (connection IP, address 192.168.1.102, command MV?)
0010: [10-01-25 20:55:28.3854] Plugins::DenonSerial::Plugin::processCommandFromQu eue (436) for 00:04:20:16:5f:a0 , send command MV?, queue length 3
0009: [10-01-25 20:55:28.3752] Plugins::DenonSerial::Plugin::enqueue (380) run PW? for 00:04:20:16:5f:a0 at next convenience
0008: [10-01-25 20:55:28.3642] Plugins::DenonSerial::Plugin::enqueue (380) run SI? for 00:04:20:16:5f:a0 at next convenience
0007: [10-01-25 20:55:28.3535] Plugins::DenonSerial::Plugin::enqueue (384) indicate that processCommandFromQueue() should run
0006: [10-01-25 20:55:28.3446] Plugins::DenonSerial::Plugin::enqueue (380) run MV? for 00:04:20:16:5f:a0 at next convenience
0005: [10-01-25 20:55:28.3348] Plugins::DenonSerial::Plugin::pollAmpStatus (1198) checking amp status for Family Room
0004: [10-01-25 20:55:28.3243] Plugins::DenonSerial::Plugin::rescheduleAmpStatusP oll (1023) rescheduling amp status poll
0003: [10-01-25 20:55:28.3155] Plugins::DenonSerial::Plugin::rescheduleAmpStatusP oll (1019) scheduling an amp status poll in 5 minutes
0002: [10-01-25 20:55:14.4892] Plugins::DenonSerial::Plugin::turnAmpOff (1115) leaving amp IP:192.168.1.102 on for player 00:04:20:16:5f:a0
0001: [10-01-25 20:55:14.4758] Plugins::DenonSerial::Plugin::turnAmpOff (1103) turning amp off for Family Room

peterw
2010-01-25, 22:50
OK. If Pippin sends us a beta iPeng we can give it a try.

Do you think we need to have an option for people to ask us to support this? ie in my last attempt I had a setting to indicate they wanted to support volume passthrough.

I'm having trouble imagining why someone would want iPeng & similar apps not to recognize that we could handle external volume commands -- do you think someone might actually want not to have the option of using iPeng, Controller, web UI, etc. to change their amp volume? That they'd want to force themselves to use the knob or Denon IR remote?

Thanks for the logs -- they're very helpful. I'm not worried about power off -- clearly my plugin didn't think it should send PWSTANDBY, and that should be an easily fixed bug. I'm more concerned about the reactions to the MV? and SI? queries. At first glance, it looked like your amp gave no response to the MV? sent around 20:55:28.4048. But volume info shows up in the response to SI? -- which means I probably just didn't wait long enough to get the response to MV?. This should be pretty easy to work around. The default timings are based on my timed observations of RS232 commands; I probably just need to tweak them a bit for TCP-based control. You're very lucky to have async HTTP to work with; making synchronous TCP connections is a bear in the single-threaded SBS app. Everything is a tradeoff between reliability and responsiveness with synchronous I/O. :-(

Thank you!!

Aesculus
2010-01-25, 22:59
MV? does work as I use it and it does take some time. Plus you get other things back than just the volume.



0025: [10-01-25 21:55:19.6595] Plugins::DenonAvpControl::DenonAvpComms::writemsg (299) Sent AVP command request: MV?

0027: [10-01-25 21:55:19.6962] Plugins::DenonAvpControl::DenonAvpComms::_read (347) Buffer read
0028: MV40
MVMAX 675

I need to review your code because I am overunning the protocol at times and I tried to put delays in but they were not successful. I will steal some of your ideas :-)

peterw
2010-01-25, 23:05
MV? does work as I use it and it does take some time. Plus you get other things back than just the volume.

Sometimes the amp behaves like somebody who has no friends. You ask it one simple thing and it bombards you with all the details of everything that's happened since you last called it. User tweaked Audyssey or maybe the gain on the front left speaker? Well, I know you only asked what the current source input is, but let me tell you this other stuff too. At 9600 bits/second. Gee, thanks. ;-)


I need to review your code because I am overunning the protocol at times and I tried to put delays in but they were not successful. I will steal some of your ideas :-)

Please help yourself. I'm heavily relying on the scheduler code that Andy G once recommended to me as a good way to split up small tasks & preserve overall SC/SBS responsiveness.

pippin
2010-01-26, 00:17
two changes:
1) the notification must use an existing CLI command name, so the notification here uses the same name as the CLI command that's used to request info
2) I added another argument to the notifications -- a string naming the plugin that provides the external volume control

Obviously usingDenonSerial() is a method I wrote (already had it) to report true if a given player used my plugin for external amp/volume control, and you'll need to replace this with your own logic.

If this works OK, I'd be happy to take a stab at a patch for IR Blaster -- hopefully Felix will be amenable to including that in a new release. If this doesn't work well, no need to bother him. :-)

[code]# --------------------------------------- external volume indication
Slim::Control::Request::notifyFromArray($client, ['getexternalvolumeinfo', 1, 1, &getDisplayName()]);


So how's this supposed to work on the CLI now? Do I just send a "getexternalvolumeinfo" to the player object? Or does it add a tag to "status"?

pippin
2010-01-26, 00:23
OK, first read, the write...
I have to subscribe to the notification and then send a "getexternalvolume" command to the player and will receive a notification from each plugin that will include the player id for which this is valid, right?

Getting nothing or "getexternalvolume 0 x" means: use "digitalVolumeControl" as is
Getting "getexternalvolume 1 0" means: up/down volume control available
Getting "getexternalvolume 1 1" means: full volume control

I'll add this to an iPeng beta.

Peter, do you have an iThingy? I seem to remember: no?

Aesculus
2010-01-26, 00:28
OK, first read, the write...
I have to subscribe to the notification and then send a "getexternalvolume" command to the player and will receive a notification from each plugin that will include the player id for which this is valid, right?

Getting nothing or "getexternalvolume 0 x" means: use "digitalVolumeControl" as is
Getting "getexternalvolume 1 0" means: up/down volume control available
Getting "getexternalvolume 1 1" means: full volume control

I'll add this to an iPeng beta.

Thats the way I understand it plus we would return the plugin name at the end if you cared.


Peter, do you have an iThingy? I seem to remember: no?

This sounds like a personal question. :)

peterw
2010-01-26, 07:07
Pippin, that's right. I don't think you'd ever see an initial 0 -- all a plugin can know is that *it* doesn't provide external volume control, so it's probably best for plugins only to give info about players they control external volume for. This does mean the user would have to restart iPeng after disabling an external volume control plugin (or changing significant preferences, e.g. disable volume passthrough) in order for iPeng to change its interface, but I think that's reasonable, as that will be a very rare event. And your recollection is correct, I don't have an iPhone/iTouch.

pippin
2010-01-26, 19:17
Peter,

while trying to experiment with this I don't get feedback by the server anymore as soon as I enable your plugin, instead it logs


[10-01-27 03:14:37.5364] Slim::Control::Request::execute (1942) Error: While trying to run function coderef [Plugins::DenonSerial::Plugin::getexternalvolumeinf oCLI]: [Undefined subroutine &Plugins::DenonSerial::Plugin::name called at /var/lib/squeezeboxserver/cache/InstalledPlugins/Plugins/DenonSerial/lib/Plugins/DenonSerial/Plugin.pm line 1354.
]

peterw
2010-01-26, 22:24
while trying to experiment with this I don't get feedback by the server anymore as soon as I enable your plugin, instead it logs

Doh, sorry! Please upgrade to 0.1.32 to have that fixed, which I just tested again on my system with 7.4 trunk.

pippin
2010-01-27, 03:51
Doh, sorry! Please upgrade to 0.1.32 to have that fixed, which I just tested again on my system with 7.4 trunk.

OK, that works now.

A few other observations:
1. The usual "encoding mess", probably not your fault but SBS's: Of the two "1"s you are sending, the first one is being encoded as a string, the second one as a number ("1" vs 1).
I can live with it, SBS always does this, it even sends "text" strings as numbers if they just contain numerals...

2. "getexternalvolumeinfo" is not player specific, right? I'm trying to get my head around how to best capture this without causing too much traffic and without "forgetting" players. Right now, I subscribe to the notification per player and then whenever I see a new player I send a "getexternalvolumeinfo" command which the triggers the notifications. However, I get them for _every_ player, not just the one I send the command to.

Background: I don't just use "listen 1" to get the notification, I subscribe to the individual notification (you can do that through cometd, no idea about the CLI). After doing that, you can trigger the notifications by issuing a "getexternalvolumeinfo" command.

peterw
2010-01-27, 09:49
How about we

define the command as accepting tagged arguments, so you could send a command like 'getexternalvolumeinfo player:00%3a04%3a20%3a11%3a22%3a33' to tell the plugins to only report on the player 00:04:20:11:22:33, or plain old 'getexternalvolumeinfo' to get info about all players
use tagged data with string values for the notifications instead of 1/0, e.g. "relative:true", "precise:true", "plugin:MyPluginName"? This would allow us to extend the concept to cover other volume-like capabilities, e.g. the Denon plugins might want to advertise their ability to set dynamic range compression (let's not worry now about how iPeng would request "night mode"!)


Would that second idea really be an improvement, or just extra, over-engineered hassle for everyone?

pippin
2010-01-27, 10:41
Would that second idea really be an improvement, or just extra, over-engineered hassle for everyone?

Wouldn't have phrased it that way :)

I'll try some client-side optimization first, maybe I can catch this by waiting half a second and aggregating some calls.

In most cases it's not _that_ big an issue anyway. There's a bad habit in iPeng in that it requests all player's status twice on startup (haven't found out, why, some fairly complex notification schemes behind this) and then right now I get a growing (by square) number of notifications (1 player: 3 notifications; 2 Players: 16 notifications, 3 players 27 ...)
The notifications are small so usually not a big thing. But then, with 20 players and 1200 notifications...

I think IF I can't get around this, I would prefer something else: to send the player id as a tag with the notification.
That way, I could subscribe to 'getexternalvolumeinfo' on the server level and just send the command every time the list of players changes. Although this would definitely be less elegant (breach of encapsulation).

I kind of like the tagged answers, yet do we need this? I was close to proposing a bit-mask (that's what I use internally) but then I remembered we are dealing with Perl...

peterw
2010-01-27, 11:38
Would you prefer not to use notifications? I thought notifications would simplify things -- you'd send 2 commands at startup and get 2 + N responses (where N = players with external volume control). [Frankly, I hadn't thought about the new player scenario.] Would you prefer a more typical CLI model that requires you to ask about specific players?

Bitmasks would work, though I prefer named tags simply because it's easier to add new tags than decide on what value to assign to a new feature to express in the bitmasked value. And of course I'm happy to add a tagged response value to identify the player. In the text/telnet CLI it's obvious what player a notification applies to, that's why I didn't already do so. But if there's value for you, great, let's add that.

Aesculus
2010-01-27, 11:47
How about we


use tagged data with string values for the notifications instead of 1/0, e.g. "relative:true", "precise:true", "plugin:MyPluginName"? This would allow us to extend the concept to cover other volume-like capabilities, e.g. the Denon plugins might want to advertise their ability to set dynamic range compression (let's not worry now about how iPeng would request "night mode"!)


Would that second idea really be an improvement, or just extra, over-engineered hassle for everyone?
My latest plugin version (1.5) does this via the player settings menus. See:
http://code.google.com/p/denonavpcontrol/wiki/HowToUse

pippin
2010-01-27, 11:57
Would you prefer not to use notifications? I thought notifications would simplify things -- you'd send 2 commands at startup and get 2 + N responses (where N = players with external volume control). [Frankly, I hadn't thought about the new player scenario.] Would you prefer a more typical CLI model that requires you to ask about specific players?

Bitmasks would work, though I prefer named tags simply because it's easier to add new tags than decide on what value to assign to a new feature to express in the bitmasked value. And of course I'm happy to add a tagged response value to identify the player. In the text/telnet CLI it's obvious what player a notification applies to, that's why I didn't already do so. But if there's value for you, great, let's add that.

Hm, let me try a bit first.
Knowing the player is not the issue - it's similarly obvious, actually it's much better as it is now from that perspective.

Tagged values are probably a better idea, though. Makes parsing data more straightforward.

Also, I believe a notification is generally fine, I think the main issue here is the way I handle players and subscriptions. It's all nicely encapsulated but due to that a bit redundant on the feedback side. I simply hadn't expected commands sent to one player to go through to others.

I think I will keep the subscription on the player side by sync the command on the server side to try to only send one per player count change incident. That should be fine then.

Aesculus
2010-01-29, 13:21
Would you prefer not to use notifications? I thought notifications would simplify things -- you'd send 2 commands at startup and get 2 + N responses (where N = players with external volume control). [Frankly, I hadn't thought about the new player scenario.] Would you prefer a more typical CLI model that requires you to ask about specific players?

Bitmasks would work, though I prefer named tags simply because it's easier to add new tags than decide on what value to assign to a new feature to express in the bitmasked value. And of course I'm happy to add a tagged response value to identify the player. In the text/telnet CLI it's obvious what player a notification applies to, that's why I didn't already do so. But if there's value for you, great, let's add that.

Peter: I am getting errors on the statement:


Slim::Control::Request::notifyFromArray($client, ['getexternalvolumeinfo', 1, 1, string(&getDisplayName())]);

Errors:


0013: frame 8: main::main (/opt/ssods4/var/home/SqueezeboxServer/slimserver.pl line 1065)
0012: frame 7: main::idle (/opt/ssods4/var/home/SqueezeboxServer/slimserver.pl line 574)
0011: frame 6: Slim::Control::Request::checkNotifications (/opt/ssods4/var/home/SqueezeboxServer/slimserver.pl line 605)
0010: frame 5: Slim::Control::Request::notify (/share/MD0_DATA/.qpkg/SSOTS/var/home/SqueezeboxServer/Slim/Control/Request.pm line 856)
0009: frame 4: (eval) (/share/MD0_DATA/.qpkg/SSOTS/var/home/SqueezeboxServer/Slim/Control/Request.pm line 2105)
0008: frame 3: Slim::Web::Cometd::__ANON__ (/share/MD0_DATA/.qpkg/SSOTS/var/home/SqueezeboxServer/Slim/Control/Request.pm line 2105)
0007: frame 2: Slim::Web::Cometd::requestCallback (/share/MD0_DATA/.qpkg/SSOTS/var/home/SqueezeboxServer/Slim/Web/Cometd.pm line 733)
0006: frame 1: Slim::Control::Request::renderAsArray (/share/MD0_DATA/.qpkg/SSOTS/var/home/SqueezeboxServer/Slim/Web/Cometd.pm line 871)
0005: frame 0: Slim::Utils::Log::logBacktrace (/share/MD0_DATA/.qpkg/SSOTS/var/home/SqueezeboxServer/Slim/Control/Request.pm line 2303)
0004:
0003: [10-01-29 12:16:07.2362] Slim::Control::Request::renderAsArray (2303) Backtrace:
0002: [10-01-29 12:16:07.2255] Slim::Control::Request::renderAsArray (2303) Error: request should set useIxHashes in Slim::Control::Request->new()

Any ideas?

peterw
2010-01-29, 16:14
Any ideas?

Do you have a &getDisplayName() function that returns a valid string token (e.g. my &getDisplayName() returns something like 'PLUGIN_DENONSERIAL' which is defined in strings.txt). I was trying to make that fairly universal, as getDisplayName() used to be a required method in Plugin.pm, so many of us keep implementing it. It's probably been replaced by an element in install.xml, so you might just want to hard-code that.

pippin, I'd love to switch to tagged values. Please let me know if you'd like a new build. I figured the iPad announcement might be keeping you busy. :-)

I haven't looked at Chris' code, but I had an idea about night mode. Jive menus are supposed to have unique IDs. I wonder if Chris could register menu IDs like 'Plugins.DenonAVPControl.00:04:20:11:22:33' for the root of a given player's settings menu, and then return with his notfications a tagged value with that menu ID. iPeng could then know to add some little settings icon for that player, and load/show that menu if the user requested the extended settings for that player/amp.

Aesculus
2010-01-30, 00:48
Do you have a &getDisplayName() function that returns a valid string token (e.g. my &getDisplayName() returns something like 'PLUGIN_DENONSERIAL' which is defined in strings.txt). I was trying to make that fairly universal, as getDisplayName() used to be a required method in Plugin.pm, so many of us keep implementing it. It's probably been replaced by an element in install.xml, so you might just want to hard-code that.


I have a getDisplayName() routine so thats not it.

From a Google search:

useIxHashes indicates that the response is expected to serialised on the cli and so order of params and results should be maintained using IxHashes.

Not sure what IxHashes are.

pippin
2010-01-30, 14:19
pippin, I'd love to switch to tagged values. Please let me know if you'd like a new build. I figured the iPad announcement might be keeping you busy. :-)

A lot of things keeping me busy right now :)
But yes, please do one and maybe define some tags before that :)


I haven't looked at Chris' code, but I had an idea about night mode. Jive menus are supposed to have unique IDs. I wonder if Chris could register menu IDs like 'Plugins.DenonAVPControl.00:04:20:11:22:33' for the root of a given player's settings menu, and then return with his notfications a tagged value with that menu ID. iPeng could then know to add some little settings icon for that player, and load/show that menu if the user requested the extended settings for that player/amp.
Hm, you mean to only have it present on activated players?
I know Chris has somehow managed to do this after lots of trying but don't remember, how. His menus now show up in the player context menu in iPeng (iPeng shows anything that is a sub menu of the playersettings menu).

peterw
2010-01-30, 16:05
Hm, you mean to only have it present on activated players?

Yes. I was thinking about the volume slider for a player using Denon AVP Control. Since there's more to volume than just the gain setting, I thought it might be cool to have an icon by the volume slider for a player using Denon AVP Control for quick access to things like night mode dynamic range compression.

Aesculus
2010-01-30, 16:28
Yes. I was thinking about the volume slider for a player using Denon AVP Control. Since there's more to volume than just the gain setting, I thought it might be cool to have an icon by the volume slider for a player using Denon AVP Control for quick access to things like night mode dynamic range compression.

Right now on my ver 1.5 of the pluging they have to go to the Player Settings of iPeng (hold down touch on the player) and then they see the Plugin player settings. They have access to all the Denon options for audio in 5 menus.

I don't use many of these myself. I am kind of a set it and forget it guy and have defaults set for each input type. But I don't think its too hard to access the players settings.

There is one too many menus though it does not make any difference in the number of steps. I originally wanted to have my audio settings under the player audio menus but could never get that to work. Now they are under a Denon AVP/AVR Control menu inside the player settings. Same number of presses and steps, but I thought it would have made more sense to have my audio settings in with the other player audio settings.

peterw
2010-01-30, 16:42
I don't use many of these myself. I am kind of a set it and forget it guy and have defaults set for each input type. But I don't think its too hard to access the players settings.


OK, nevermind then. :-)

peterw
2010-02-01, 23:04
A lot of things keeping me busy right now :)
But yes, please do one and maybe define some tags before that :)

OK, version 0.1.33 now gives tagged responses. I don't know how you're parsing these, and if this matters, but I'm just returning strings with "name:value" info. With the telnet/CLI interface, it looks the same as when using the proper addResult API with normal CLI responses, but it might come across differently via comet/json. New Perl code:


# at the top of Plugin.pm, define a scalar to hold a pointer to the getexternalcolumeinfo CLI command
# my $getexternalvolumeinfoCoderef;
# set $getexternalvolumeinfoCoderef in the initPlugin() method with a line like the following:
# $getexternalvolumeinfoCoderef = Slim::Control::Request::addDispatch(['getexternalvolumeinfo'],[0, 0, 0, \&getexternalvolumeinfoCLI]);

sub getexternalvolumeinfoCLI {
my @args = @_;
&reportOnOurPlayers();
if ( defined($getexternalvolumeinfoCoderef) ) {
# chain to the next implementation
return &$getexternalvolumeinfoCoderef(@args);
}
# else we're authoritative
my $request = $args[0];
$request->setStatusDone();
}

sub reportOnOurPlayers() {
# loop through all currently attached players
foreach my $client (Slim::Player::Client::clients()) {
if (&usingDenonSerial($client) ) {
# using our volume control, report on our capabilities
$log->debug("Note that ".$client->name()." uses us for external volume control");
Slim::Control::Request::notifyFromArray($client, ['getexternalvolumeinfo', 'relative:1', 'precise:1', 'plugin:DenonSerial']);
# precise:1 can set exact volume
# relative:1 can make relative volume changes
# plugin:DenonSerial this plugin's name
}
}
}

pippin
2010-02-02, 00:17
OK, version 0.1.33 now gives tagged responses. I don't know how you're parsing these, and if this matters, but I'm just returning strings with "name:value" info. With the telnet/CLI interface, it looks the same as when using the proper addResult API with normal CLI responses, but it might come across differently via comet/json. New Perl code:

# at the top of Plugin.pm, define a scalar to hold a pointer to the getexternalcolumeinfo CLI command
# my $getexternalvolumeinfoCoderef;
# set $getexternalvolumeinfoCoderef in the initPlugin() method with a line like the following:
# $getexternalvolumeinfoCoderef = Slim::Control::Request::addDispatch(['getexternalvolumeinfo'],[0, 0, 0, \&getexternalvolumeinfoCLI]);

sub getexternalvolumeinfoCLI {
my @args = @_;
&reportOnOurPlayers();
if ( defined($getexternalvolumeinfoCoderef) ) {
# chain to the next implementation
return &$getexternalvolumeinfoCoderef(@args);
}
# else we're authoritative
my $request = $args[0];
$request->setStatusDone();
}

sub reportOnOurPlayers() {
# loop through all currently attached players
foreach my $client (Slim::Player::Client::clients()) {
if (&usingDenonSerial($client) ) {
# using our volume control, report on our capabilities
$log->debug("Note that ".$client->name()." uses us for external volume control");
Slim::Control::Request::notifyFromArray($client, ['getexternalvolumeinfo', 'relative:true', 'precise:true', 'plugin:DenonSerial']);
# precise:true can set exact volume
# relative:true can make relative volume changes
# plugin:DenonSerial this plugin's name
}
}
}
Ok, I'll give it a try.
One thing: if you use strings you might want to use "tag:1" instead of "tag:true", at least that's what the server does.
I'll support both.

peterw
2010-02-02, 06:30
One thing: if you use strings you might want to use "tag:1" instead of "tag:true", at least that's what the server does.
I'll support both.

Aw, you shouldn't have to do that. I just published 0.1.34, which switches to 1/0 instead of true/false. Better for consistency, better for letting you keep your code tighter.

pippin
2010-02-02, 06:50
Aw, you shouldn't have to do that. I just published 0.1.34, which switches to 1/0 instead of true/false. Better for consistency, better for letting you keep your code tighter.
Thanks!
Now we really have to get Felix to join in, just had another user with some old IRBlaster setting in his preferences that was wrongly detected.

fcm4711
2010-02-02, 15:13
Hey guys

Your suggestion looks reasonable. I'll check out Peter's Perl code sample and how to adapt it for IRBlaster.

Stay tuned
Felix

pippin
2010-02-02, 15:36
Hey guys

Your suggestion looks reasonable. I'll check out Peter's Perl code sample and how to adapt it for IRBlaster.

Stay tuned
Felix

Cool. Thanks!

fcm4711
2010-02-08, 03:50
Hi there

I've adapted Peter's code into IR-Blaster v5.6. You can download / install the new version here: http://www.gwendesign.com/sc/repo/repo_beta.xml (I'll move it to my real repository repo.xml as soon as you give the ok.)

Could you please check whether it is doing what you guys expect?

BTW: A particular player is only listed if volume commands (up and / or down) are selected for that player.

Cheers
Felix

pippin
2010-02-08, 05:56
OK, works for me.

I've added a documentation bug on this to have it included in the CLI doc:
https://bugs.slimdevices.com/show_bug.cgi?id=15657

Chris, Peter, Felix, could you have a look at this?

fcm4711
2010-02-08, 07:26
Hey pippin

Ok, thanks for checking. I moved it to my regular repository: http://www.gwendesign.com/sc/repo/repo.xml

Felix

peterw
2010-02-08, 14:35
OK, works for me.

I've added a documentation bug on this to have it included in the CLI doc:
https://bugs.slimdevices.com/show_bug.cgi?id=15657

Chris, Peter, Felix, could you have a look at this?

Thanks for opening the bug. I think we mainly want 2 things changed in SBS (and maybe the MySB code, too)
1) implement a do-nothing stub routine for getexternalvolumeinfo
2) document the way it's supposed to work

But is that the best way to do this? I'd like to propose another slight change that I think is inline with what we've been doing. SBS has global notifications, right? Notifications without specific players?

How about

1) the SBS code's core implementation of getexternalvolumeinfo does nothing beyond sending a *global* getexternalvolumeinfo notification with *no tagged parameters*.
2) SBS *plugins* could then choose either to intercept the getexternalvolumeinfo command as we have done *or* subscribe to getexternalvolumeinfo notifications. If a plugin subscribed to getexternalvolumeinfo notifications, it would turn around and send player-specific getexternalvolumeinfo notifications with tagged parameters for any players where it's appropriate

This would be 100% compatible with the code Chris, Felix, and I have added and provide two new benefits:

1) Plugins would not need to intercept getexternalvolumeinfo, and if they chose not to intercept getexternalvolumeinfo, there would be a lower chance of anything going wrong. With our current setup, there's a risk that a bug in one plugin might prevent if from chaining the next implementation of getexternalvolumeinfo.
2) Applets could send information. For instance, there might be applet implementations of any of our three plugins for use on SB Touch -- perhaps using its USB port for the RS232 comms for DenonSerial. If the core SBS (and MySB??) code sent "empty"/global getexternalvolumeinfo notifications, then even an applet could see when a control app like iPeng wanted getexternalvolumeinfo info, and could provide that information.

Sound right?

pippin
2010-02-08, 16:17
Sounds right.
Could you add that to the bug?