kaaholst / android-squeezer

Remote control for your Logitech Media Server ("Squeezeserver" etc) and players.
Apache License 2.0
75 stars 15 forks source link

Feature request: Return to the top level menu when switching LMS players #768

Closed SamInPgh closed 1 year ago

SamInPgh commented 2 years ago

Hi, Kurt. I have a request that would be very helpful with regard to the operation of the DenonAvpControl plugin, which inserts its own AVR audio settings submenu under Squeezer's "Settings" menu. As you may know, this menu is only valid for players that are using the plugin to control functions of an attached Denon or Marantz AVR. If the menu is invoked for a player that is not using the plugin, a pop-up message is created informing the user of that fact and the rest of the menu controls are not displayed. However, it the menu is invoked for a player using the plugin and the user has gone into one of the sub-menus, such as the one which allows for adjusting the AVR's channel levels via slider controls, and then switches to another non-plugin player via Squeezer's always-visible drop-down menu control, we are left with a situation where the user is looking at a menu which has no valid context for that player. In order to handle this in the plugin, I would have to add logic in the callback function of every submenu to check that the current player is valid and, if not, display an error message, instead of just doing it in the top menu as it does now. I have noticed that the Squeeze Control app does, in fact, send the user back to the top menu when the player is changed and am hoping that Squeezer could also implement this behavior. As always, any feedback is appreciated. Thanks.

-Sam

kaaholst commented 2 years ago

When you switch player, the current menu is reloaded. Can that be of any use?

SamInPgh commented 2 years ago

No. I see that is happening but, in this case, the current menu has no context for the new player, which does not use the plugin. Reloading the current submenu effectively sidesteps the gatekeeper code in the parent menu that would have prevented the new player from getting there. In order to handle this situation in the plugin, I would have to add code to every submenu (10 of them currently) to detect it and take some sort of remedial action. I believe that this could also be an issue with other context-sensitive menus and, although there are certainly cases where reloading the current menu would make sense, I feel that the safest way to handle the situation in general is to return to the top menu as Squeeze Ctrl does, assuming that there's not a way you can do it as a special case for the plugin.

In any case, it's not a huge deal as there are probably only a few hundred users of the plugin and even fewer that use the AVR audio settings menus with Squeezer (which is a shame because it's a really cool feature). By the way, I recently discovered how to detect which client app is in control during menu processing and so now can tailor the various submenus accordingly. For instance, Squeeze Ctrl can't handle 'slider' controls at all so now I just bypass the preamp and channel level menus completely for those users. And iPeng somehow reverses the roles of 'checkbox' and 'button' controls so I have to tell it that the power button is a 'checkbox' to get it working correctly there. Fun stuff!

kaaholst commented 2 years ago

Yes going to top menu is probably the safest and also easiest to implement.

However from a user perspective it's quite handy to be able to switch player when in the now playing screen, current playlist, alarm list or even when browsed the local music and want to play the current results on a different player. Especially since this functionality is already present in Squeezer.

An alternative could be to explore the nextWindow maybe in combination with goNow functionality of SBS SqueezePlay interface, which Squeezer and I believe most other controllers use.

SamInPgh commented 2 years ago

I see your point regarding the usefulness of keeping the current menu active when switching players in the situations you listed. Is there any way Squeezer could detect that the Denon plugin's menu system is in use when switching players and instead return to either the top or the Settings menu?

I use "nextWindow => refresh" in many of the menus in order to update the window contents after executing a command. As I said, I could add code in each of the ten submenus to make sure that the current player is still valid and take remedial action if it's not but I'd like to avoid that if possible.

On Sun, Jun 26, 2022, 5:32 PM Kurt Aaholst @.***> wrote:

Yes going to top menu is probably the safest and also easiest to implement.

However from a user perspective it's quite handy to be able to switch player when in the now playing screen, current playlist, alarm list or even when browsed the local music and want to play the current results on a different player. Especially since this functionality is already present in Squeezer.

An alternative could be to explore the nextWindow maybe in combination with goNow functionality of SBS SqueezePlay interface https://wiki.slimdevices.com/index.php/SBS_SqueezePlay_interface, which Squeezer and I believe most other controllers use.

— Reply to this email directly, view it on GitHub https://github.com/kaaholst/android-squeezer/issues/768#issuecomment-1166651191, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASZI4RHYRY2WVI3M2HBUGJDVRDD73ANCNFSM5ZFI5XQA . You are receiving this because you authored the thread.Message ID: @.***>

SamInPgh commented 2 years ago

I just realized that there is another more serious problem caused by not returning to the main (or Settings) menu when switching players. If the user is, for example, multiple levels deep in a menu and switches players, it's true that the current menu gets refreshed. However, if the user then backs out to the previous menu via the "back" button, the screen contents are NOT updated or refreshed to reflect the new player. Unlike with the current menu, there is nothing I can do in the plugin to handle this situation, as the menu function does not seem to get invoked in this case. The cached contents of the menu are simply re-displayed. An example of this behavior in the default menu structure can be seen when selecting "Settings->Advanced->Squeezebox Information->xxxx Information", where 'xxxx' is the name of the current player. If the player is switched to 'yyyy', for example, while in that screen and then the user backs up one level to the previous screen, it still shows the name of the previous player, i.e. "xxxx Information". In the case of my plugin menu, the problem is more serious than this because the cached menu will now contain audio settings relating to the AVR connected to the previous player instead of the current one and, as I said, I have no way of controlling or handling that situation --- at least none that I know of.

I hope I have explained the problem clearly and that maybe you can find a way to send the user back to the main Settings menu when switching players while in a submenu of Settings, either through a user option or by default.

Thanks.

kaaholst commented 2 years ago

Yes I know about the parent issue, that's why I mentioned the goNow parameter which provides the possibility to send the user up the hierarchy and even to the home screen. That wouldn't help for example if we are in for example the Now Playing screen but come from multiple levels deep from the home screen.

I think we should go for returning to home screen when switching players, and hope there won't be to many complains. You can try this apk

SamInPgh commented 2 years ago

The new apk works fine. Thank you. Hopefully nobody will complain about the change. I know it's impossible to please everyone but this seems to be the best way to handle the situation safely.

viertelb commented 1 year ago

Yes I know about the parent issue, that's why I mentioned the goNow parameter which provides the possibility to send the user up the hierarchy and even to the home screen. That wouldn't help for example if we are in for example the Now Playing screen but come from multiple levels deep from the home screen.

I think we should go for returning to home screen when switching players, and hope there won't be to many complains. You can try this apk

@kaaholst Sorry, but I would strongly complain. :) It happens often that I want to play something on a player for the kids and happen to be on my own player. When I realize this I already have done a lot of navigation in the app. It is VERY good that the menu stays right where it is instead of changing. There should at least be an option to set it in the settings page.

kaaholst commented 1 year ago

@viertelb Do you have any ideas to how we might autodetect, when it's necessary to go to the main menu. Otherwise, I agree we need an option for this.

SamInPgh commented 1 year ago

I have a possible way to do this without adding an option. During the two months since I opened this issue, I spent some time working with Craig Drummond as he added support for the Denon plugin to the Material Skin client. The problem with switching players while in the settings menu structure wasn't actually an issue there since MS, like all the other client apps I know of, take the user back to the top menu immediately upon switching. However, Craig and I both didn't like the fact that the Denon plugin menu shows up in the Settings menu for all players, regardless of whether the current player uses the Denon plugin. (If the user selects the Denon menu entry for a non-Denon-controlled player, a message is displayed indicating that the menu is not supported for the current player.) This behavior is necessitated by the fact that player-specific plugin menus are not currently supported by LMS. Upon looking at the LMS code and discovering a seemingly unused "player" attribute in the plugin menu structure, I proposed that the plugin maintain an array of client id's ($client->id) containing all players who had registered the Denon plugin menu. Each time the plugin registers the top level plugin menu for a player, it passes a pointer to the current array in the "actions=>go=>player" attribute. So the first player registering passes an array containing only its player id, the second one an array containing both id's, etc. Material Skin makes use of this information when creating its menu structure, only displaying the plugin menu for players whose client id is contained in the array, and it works well. Releases of the plugin beginning with v4.6 contain the code to maintain and pass the pointer to the array in the "actions=>go=>player" attribute of the menu structure used in the registerPluginMenu() call.

My idea would be for Squeezer to also make use of this array, not only to prevent the Denon plugin menu entry from being displayed in the Settings menu for non-plugin players, but also to check against when switching players. For instance, the logic could always take the user to the top menu when switching FROM a player whose client id is in the array, ideally only if the user was currently navigating in the plugin menu's hierarchy. Then again, if Squeezer could determine that the user was currently in the Denon plugin menu when switching players, it wouldn't need to use the array at all. It could simply always go the top menu in that case, since the only way the user would be in that menu would be if it was in the array.

I hope this makes sense. If not, I apologize. If it does, and you think it's not too much of a kluge, I would be happy to work with you on implementing it.

viertelb commented 1 year ago

@viertelb Do you have any ideas to how we might autodetect, when it's necessary to go to the main menu. Otherwise, I agree we need an option for this.

I now read ab out the problem beeing in wrong menu structures after changing players - I did not think about this before and this is of course a problem. But it is not an issue for structures of directories, genres, artists because they should be the same over all players. I will investigate further to check if there could be an easy distinction. Thanks for the input, @SamInPgh

kaaholst commented 1 year ago

I have a possible way to do this without adding an option.

@SamInPgh That looks interesting. I would like to do some experiments. I have followed some instructions I got from Chris once we were debugging an icon issue, so I can see the Denon menus even if I don't have the device.

Then follows some incstructions, which I think maybe obsolete.

Can you help?

SamInPgh commented 1 year ago

Let me give this some thought. Do you recall if those changes gave you access to the entire menu structure? It seems doubtful to me. I will sit down with the code this weekend and see what I can come up with. It shouldn't be too hard to dummy up the logic for one particular sub-menu, which I'm assuming is all you would need for your purposes. Then again, maybe it's simpler than I think.

On Fri, Sep 9, 2022, 3:19 AM Kurt Aaholst @.***> wrote:

I have a possible way to do this without adding an option.

@SamInPgh https://github.com/SamInPgh That looks interesting. I would like to do some experiments. I have followed some instructions I got from Chris once we were debugging an icon issue, so I can see the Denon menus even if I don't have the device.

  • Install V3 of the DenonAvpControl plugin via the Plugins tab in the Settings of the LMS
  • It should prompt you to restart LMS -Go to a player in Settings and choose the DenonAVP/AVRControl pulldown on the player -Enter a fake ip address on your network since it won't find a AVR
  • Make sure Main zone is selected
  • Select: Enable audio settings menus
  • Make sure the Enable Plugin is selected.
  • Select Apply

Then follows some incstructions, which I think maybe obsolete.

  • Now you will need to modify a few lines of code in the plugin on your LMS server
  • Go to where the plugin is installed and edit Plugin.pm
  • Look for line 210 and change 'settingsPlayer' to 'settings'
  • Comment out lines 252-255
  • Note that line 249 is the reference to the main menu icon. Lets start with this one
  • Save the file.
  • Now restart LMS
  • Once LMS is running load Squeezer and turn on the selected player the plugin is assigned to

Can you help?

— Reply to this email directly, view it on GitHub https://github.com/kaaholst/android-squeezer/issues/768#issuecomment-1241598259, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASZI4RG5QAPQZ2VPKEJ2S33V5LQHDANCNFSM5ZFI5XQA . You are receiving this because you were mentioned.Message ID: @.***>

kaaholst commented 1 year ago

Here's the issue I referred to for reference: #686

I don't think I need the full menu structure, I just need enough to test the data in the menu structure we receive for the Denon menus. I also don't need the functionality to actually work.

In the meantime I commented out lines 289-294 so now I can show the top level Denon plugin menu entry in the settings menu. I will start with that.

SamInPgh commented 1 year ago

Sorry for the delay, Kurt. Opening weekend of the NFL (American Football) season, as well as my first time playing in a fantasy football league in 20+ years. Now I remember why I stopped playing. ;-)

Anyway, attached is a copy of Plugin.pm that should give you access to the "Input Source Select" submenu of the main menu. Selecting any other submenu will not work and will likely crash the client. The list of input sources will, of course, be empty. If you need access to any of the other menus or need real data in the Input Source Select menu, let me know. I figured that any of the submenus would work for you. Good luck! (I had to rename perl file because Gitbub doesn't like them.)

Plugin.pm.txt

kaaholst commented 1 year ago

Thanks, this is working fine.

We should be able to make this work, using the player attribute.

I have some comments though.

  1. The attribute is not unused. It is currently only used in the direction from controller to LMS, but is defined also from LMS to controller. It is documented here. Squeezer fills this in when sending commands to LMS. We don't currently read it for menus received from LMS, so nothing should break if you introduce this change. I can't speak for other controllers, so maybe Michael or the forum should be involved in this.

  2. The current player attribute is for a single player, not for an array. It feels hacky to put an array in there. We actually don't need an array to implement this in Squeezer, just the ID of the currently active player. If this is filled in, then it's a player specific command, so the menu is not valid when switching player. This can be generic, i.e. not specific to the Denon plugin, so other menus/plugins could make use of this as well.

  3. Regarding player specific plugin menus, I think this is in fact currently supported in LMS. I believe that's actually what the player attribute is for. In the plugin code, I'm guessing this means you shall not call registerPluginMenu if not enabled for player. This means that the clients don't have to implement not showing the menu.

  4. I was hoping to get Playerid not only in the top level Denon menu, but also in sub menus. This way we could implement that when the user returns to a menu, and the active player has changed, we can move up the windows stack until we reach a menu which is not player specific. Similar to how windowId works.

  5. While it should be possible to determine if we are in the Denon menu using its ID, this again would be a hack. It's much better to check for properties rather than identity, especially because I would have to check for the mysterious “pluginDenonAvpControl_a”. This solution would be less generic and only work for this specific plugin.

SamInPgh commented 1 year ago

Hi Kurt. Could you please look over this rather lengthy discussion between Craig Drummond and myself regarding this issue as it pertains to Material Skin and then let me know what you think. There's a lot of extraneous stuff in there but I'm hoping that the link takes you to the part where we begin discussing this problem. It should be on July 4. Thanks.

https://github.com/CDrummond/lms-material/issues/583#issuecomment-1173477933

kaaholst commented 1 year ago

I will read that (lengthy) tread.

In the meantime, I have made a fix for the issue you describe above. When the user resumes to a previous menu, it is reloaded if the player has changed. This may also take us some way regarding player specific menu trees, like the Denon plugin. You can see it in this apk

SamInPgh commented 1 year ago

Thanks. The fix seems to work for that particular situation. Hopefully, you can find a way to return the user to the top menu when switching players while in the plugin menu.

kaaholst commented 1 year ago

OK, I have now read the linked thread. It seems that, contrary to the documentation, player-specific plugin menus are not currently supported by LMS. This is also seen in this old thread from the forum.

While this should be fixed server side, we need to a workaround for now and for the foreseeable future. I still think that using the player attribute for an array is a hack, however it is necessary to support player-specific-plugins generically, so I think it's justified. Like I said, we currently only use the player attribute when sending commands to LMS. An alternative to use a new attribute for this would also be OK.

To save you the trouble of adding the player array to every sub menu for the Denon plugin, I interpret it like this: If an LMS menu is player specific (i.e. it has the player attribute filled in with a value different from “0”), then all sub menus are player specific to the same player array.

Please try this updated apk.

I didn't implement to return to the home menu, because both Benjamin and I think the current behaviour is convenient. I do however think this fixes the issue.

CC @viertelb

SamInPgh commented 1 year ago

Thanks, Kurt. Good find on Chris's old thread from 13 years ago! I hadn't seen that before.

The new apk is working exactly as you describe. It's nice not to see the Denon menu listed under Settings for players not using the plugin. However, there is still a problem when switching between two players that use the plugin while in a player specific submenu. In that case, the current menu may not be valid for the new player or may depend on data from the previous player. This is because the number and type of submenus can vary depending upon player-specific attributes. I would ask that, in this situation, the user also be returned up the stack, but only to the top plugin menu, avpTop. If the player change is made while in the top plugin menu, nothing needs to be done (as is the current behavior). To paraphrase your changes above:

If this is not possible or too specific to the plugin, the safest way to handle it would be to simplify your change as follows:

While it would be nice to be able to switch between players while staying within the plugin menu structure, it is not necessary.

Thanks again.

kaaholst commented 1 year ago

However, there is still a problem when switching between two players that use the plugin while in a player specific submenu. In that case, the current menu may not be valid for the new player or may depend on data from the previous player. This is because the number and type of submenus can vary depending upon player-specific attributes. I would ask that, in this situation, the user also be returned up the stack, but only to the top plugin menu, avpTop. If the player change is made while in the top plugin menu, nothing needs to be done (as is the current behavior). To paraphrase your changes above:

Makes sense, I probably should have guessed this from the word: player specific!

I can achieve the desired effect if I interpret sub menus from a player specific menu like this:

Please test, and let me know what you think.

One last thing; this also means that if you don't want the default behavior for a sub-menu, you can fill in the player array. If for example the Quick select sub-menu is valid for all players which has enabled the Denon plugin, then put the same player array in Quick select menu item, as for the top level plugin menu.

SamInPgh commented 1 year ago

Well, Kurt. At the risk of jinxing it, I'm going to say that you nailed it perfectly! Everything is working just as you describe and I'm pleased that switching between plugin-specific players within the menu structure goes to the avpTop menu rather than back to Settings. I will continue testing and let you know if I run into any problems but so far it looks great!

Once again, thanks for your continued cooperation and support. It is greatly appreciated.

P.S. There are three sub-menus, including Quick Select, that are valid for all players. I will add the player array to one of them to test that logic and let you know the results.

SamInPgh commented 1 year ago

Okay. Maybe not quite perfect yet. I have run into a problem where, sometimes when switching from one plugin player to another while in a sub-menu, it correctly returns to the avpTop menu but doesn't seem to reload it or, if it does, it uses the old player instead of the new one. The result is that it displays the menu for the old player even though the new player is now active. I haven't detected a pattern yet. It seems to happen randomly. I'll let you know if I can narrow it down.

SamInPgh commented 1 year ago

I have found a way to re-create the above issue about 50% of the time if you want me to run it with debug logging.

kaaholst commented 1 year ago

I would like if you can run with cometd debug, or otherwise determine whether Squeezer sends the command to load the menu with the wrong player or not at all when returning to the avpTop menu.

kaaholst commented 1 year ago

HI @SamInPgh, Are there any news on this one? I am not able to reproduce the issue here, so I need more info to fix this.

SamInPgh commented 1 year ago

Sorry @kaaholst. I got side-tracked with another plugin problem I found while trying to get the doc for you. That one is now solved. Here is a portion of the LMS cometd debug. I tried to narrow it down as much as possible. To set the scene, it starts when I am in the top level (avpTop) menu of Player 1 ("Max2Play_BT"), whose MAC address is "b8:25:eb:c1:71:f0" and I select the "Preamp Volume" submenu. That menu is displayed correctly. I then select Player 2 ("Max2Play"), another player using the plugin whose MAC address is "b8:26:eb:c1:71:f0") from the dropdown player list at the top. At that point, the avpTop menu is again displayed, but still containing the context of Player 1 instead of Player 2. From what I can tell, Squeezer doesn't send the command to load the menu of Player 2 at all. As I mentioned earlier, this sequence sometimes works correctly. If you need me to get a debug of that scenario, let me know. I hope this is helpful.

Thanks.

[22-09-28 17:55:20.8357] Slim::Web::Cometd::handler (149) Cometd request (192.168.1.12:40031): [
  {
    channel => "/slim/request",
    clientId => 38_732_907,
    data => {
          request  => [
                        "b8:25:eb:c1:71:f0",
                        ["avpLvl", 0, 512, "useContextMenu:1", "menu:avpLvl"],
                      ],
          response => "/38732907/slim/request/13",
        },
    id => 57,
  },
]
[22-09-28 17:55:20.8372] Plugins::DenonAvpControl::Plugin::avpLvl (1455) Preamp level menu routine entered...
[22-09-28 17:55:20.8376] Plugins::DenonAvpControl::Plugin::avpLvl (1467) The value of preLevel is: 51
[22-09-28 17:55:20.8378] Slim::Web::Cometd::handleRequest (909) Request for /38732907/slim/request/13 / 57 is not async
[22-09-28 17:55:20.8381] Slim::Web::Cometd::Manager::deliver_events (240) Sending event on channel /38732907/slim/request/13 to 38732907
[22-09-28 17:55:20.8396] Slim::Web::Cometd::Manager::deliver_events (254) Delivering events to 38732907:
[
  {
    channel => "/38732907/slim/request/13",
    data => {
          count     => 2,
          item_loop => [
                         { text => "Value (0-70):  [51]" },
                         {
                           actions     => {
                                            "do" => { cmd => ["avpSetLvl"], params => { valtag => "value" }, player => 0 },
                                          },
                           adjust      => 1,
                           initial     => 51,
                           label       => "Volume",
                           max         => 70,
                           min         => 0,
                           nextWindow  => "refresh",
                           slider      => 1,
                           sliderIcons => "volume",
                         },
                       ],
          offset    => 0,
        },
    ext => { priority => "" },
    id => 57,
  },
]
[22-09-28 17:55:20.8403] Slim::Web::Cometd::sendHTTPResponse (740) Sending Cometd chunk (192.168.1.12:39723):
[{"data":{"count":"2","item_loop":[{"text":"Value (0-70):  [51]"},{"actions":{"do":{"params":{"valtag":"value"},"cmd":["avpSetLvl"],"player":"0"}},"min":"0","max":"70","sliderIcons":"volume","slider":"1","adjust":"1","label":"Volume","initial":"51","nextWindow":"refresh"}],"offset":"0"},"id":"57","channel":"/38732907/slim/request/13","ext":{"priority":""}}]
[22-09-28 17:55:20.8409] Slim::Web::Cometd::sendHTTPResponse (734) Sending Cometd response (192.168.1.12:40031):
HTTP/1.1 200 OK
Cache-Control: no-cache
Pragma: no-cache
Content-Length: 79
Content-Type: application/json
Expires: -1
X-Time-To-Serve: 0.00831103324890137

[{"clientId":"38732907","id":"57","channel":"/slim/request","successful":true}]
[22-09-28 17:55:27.1343] Slim::Web::Cometd::handler (149) Cometd request (192.168.1.12:40031): [
  {
    channel => "/slim/subscribe",
    clientId => 38_732_907,
    data => {
          request  => ["b8:25:eb:c1:71:f0", ["displaystatus", "subscribe:"]],
          response => "/38732907/slim/displaystatus/b8:25:eb:c1:71:f0",
        },
    id => 58,
  },
]
[22-09-28 17:55:27.1354] Slim::Web::Cometd::handleRequest (909) Request for /38732907/slim/displaystatus/b8:25:eb:c1:71:f0 / 58 is not async
[22-09-28 17:55:27.1360] Slim::Web::Cometd::Manager::deliver_events (240) Sending event on channel /38732907/slim/displaystatus/b8:25:eb:c1:71:f0 to 38732907
[22-09-28 17:55:27.1366] Slim::Web::Cometd::Manager::deliver_events (254) Delivering events to 38732907:
[
  {
    channel => "/38732907/slim/displaystatus/b8:25:eb:c1:71:f0",
    data => {},
    ext => { priority => "" },
    id => 58,
  },
]
[22-09-28 17:55:27.1370] Slim::Web::Cometd::sendHTTPResponse (740) Sending Cometd chunk (192.168.1.12:39723):
[{"data":{},"id":"58","channel":"/38732907/slim/displaystatus/b8:25:eb:c1:71:f0","ext":{"priority":""}}]
[22-09-28 17:55:27.1376] Slim::Web::Cometd::sendHTTPResponse (734) Sending Cometd response (192.168.1.12:40031):
HTTP/1.1 200 OK
Cache-Control: no-cache
Pragma: no-cache
Content-Length: 81
Content-Type: application/json
Expires: -1
X-Time-To-Serve: 0.00624799728393555

[{"clientId":"38732907","id":"58","channel":"/slim/subscribe","successful":true}]
[22-09-28 17:55:27.1549] Slim::Web::Cometd::handler (149) Cometd request (192.168.1.12:40031): [
  {
    channel => "/slim/unsubscribe",
    clientId => 38_732_907,
    data => { unsubscribe => "/38732907/slim/menustatus/b8:25:eb:c1:71:f0" },
    id => 59,
  },
]
[22-09-28 17:55:27.1553] Slim::Web::Cometd::handler (503) Request::unsubscribe( CODE(0xeef3664) )
[22-09-28 17:55:27.1560] Slim::Web::Cometd::sendHTTPResponse (734) Sending Cometd response (192.168.1.12:40031):
HTTP/1.1 200 OK
Cache-Control: no-cache
Pragma: no-cache
Content-Length: 152
Content-Type: application/json
Expires: -1
X-Time-To-Serve: 0.00221705436706543

[{"clientId":"38732907","data":{"unsubscribe":"/38732907/slim/menustatus/b8:25:eb:c1:71:f0"},"id":"59","channel":"/slim/unsubscribe","successful":true}]
[22-09-28 17:55:27.1681] Slim::Web::Cometd::handler (149) Cometd request (192.168.1.12:40031): [
  {
    channel => "/slim/subscribe",
    clientId => 38_732_907,
    data => {
          request  => [
                        "b8:26:eb:c1:71:f0",
                        ["displaystatus", "subscribe:showbriefly"],
                      ],
          response => "/38732907/slim/displaystatus/b8:26:eb:c1:71:f0",
        },
    id => 60,
  },
]
[22-09-28 17:55:27.1688] Slim::Web::Cometd::handleRequest (909) Request for /38732907/slim/displaystatus/b8:26:eb:c1:71:f0 / 60 is not async
[22-09-28 17:55:27.1691] Slim::Web::Cometd::Manager::deliver_events (240) Sending event on channel /38732907/slim/displaystatus/b8:26:eb:c1:71:f0 to 38732907
[22-09-28 17:55:27.1697] Slim::Web::Cometd::Manager::deliver_events (254) Delivering events to 38732907:
[
  {
    channel => "/38732907/slim/displaystatus/b8:26:eb:c1:71:f0",
    data => {},
    ext => { priority => "" },
    id => 60,
  },
]
[22-09-28 17:55:27.1703] Slim::Web::Cometd::sendHTTPResponse (740) Sending Cometd chunk (192.168.1.12:39723):
[{"data":{},"id":"60","channel":"/38732907/slim/displaystatus/b8:26:eb:c1:71:f0","ext":{"priority":""}}]
[22-09-28 17:55:27.1709] Slim::Web::Cometd::sendHTTPResponse (734) Sending Cometd response (192.168.1.12:40031):
HTTP/1.1 200 OK
Cache-Control: no-cache
Pragma: no-cache
Content-Length: 81
Content-Type: application/json
Expires: -1
X-Time-To-Serve: 0.00406384468078613

[{"clientId":"38732907","id":"60","channel":"/slim/subscribe","successful":true}]
[22-09-28 17:55:27.1822] Slim::Web::Cometd::handler (149) Cometd request (192.168.1.12:40031): [
  {
    channel => "/slim/subscribe",
    clientId => 38_732_907,
    data => {
          request  => ["b8:26:eb:c1:71:f0", ["menustatus"]],
          response => "/38732907/slim/menustatus/b8:26:eb:c1:71:f0",
        },
    id => 61,
  },
]
[22-09-28 17:55:27.1826] Slim::Web::Cometd::handleRequest (786) Treating request as plain subscription: [["menustatus"]]
[22-09-28 17:55:27.1830] Slim::Web::Cometd::handleRequest (809) Subscribed for /38732907/slim/menustatus/b8:26:eb:c1:71:f0, callback CODE(0xeef217c)
[22-09-28 17:55:27.1836] Slim::Web::Cometd::sendHTTPResponse (734) Sending Cometd response (192.168.1.12:40031):
HTTP/1.1 200 OK
Cache-Control: no-cache
Pragma: no-cache
Content-Length: 81
Content-Type: application/json
Expires: -1
X-Time-To-Serve: 0.00272893905639648

[{"clientId":"38732907","id":"61","channel":"/slim/subscribe","successful":true}]
[22-09-28 17:55:27.1944] Slim::Web::Cometd::handler (149) Cometd request (192.168.1.12:40031): [
  {
    channel => "/slim/request",
    clientId => 38_732_907,
    data => {
          request  => [
                        "b8:26:eb:c1:71:f0",
                        ["status", "-", 1, "useContextMenu:1", "menu:menu"],
                      ],
          response => "/38732907/slim/playerstatus/b8:26:eb:c1:71:f0",
        },
    id => 62,
  },
]
[22-09-28 17:55:27.1955] Slim::Web::Cometd::handleRequest (909) Request for /38732907/slim/playerstatus/b8:26:eb:c1:71:f0 / 62 is not async
[22-09-28 17:55:27.1958] Slim::Web::Cometd::Manager::deliver_events (240) Sending event on channel /38732907/slim/playerstatus/b8:26:eb:c1:71:f0 to 38732907
[22-09-28 17:55:27.1984] Slim::Web::Cometd::Manager::deliver_events (254) Delivering events to 38732907:
[
  {
    channel => "/38732907/slim/playerstatus/b8:26:eb:c1:71:f0",
    data => {
          alarm_next               => 0,
          alarm_snooze_seconds     => 540,
          alarm_state              => "none",
          "alarm_timeout_seconds"  => 3600,
          alarm_version            => 2,
          base                     => {
                                        actions => {
                                              more => {
                                                    cmd => ["contextmenu"],
                                                    itemsParams => "params",
                                                    params => { context => "playlist", menu => "track" },
                                                    player => 0,
                                                    window => { isContextMenu => 1 },
                                                  },
                                            },
                                      },
          count                    => 0,
          "digital_volume_control" => 0,
          "mixer volume"           => 45,
          mode                     => "stop",
          player_connected         => 1,
          player_ip                => "192.168.1.11:58374",
          player_name              => "Max2Play",
          "playlist mode"          => "off",
          "playlist repeat"        => 2,
          "playlist shuffle"       => 0,
          playlist_tracks          => 0,
          power                    => 1,
          preset_data              => [
                                        {
                                          URL  => "https://www.mysqueezebox.com/public/opml/5f0ab48ae6c061cc6b7143091b68b90e062417b0/favorites.opml",
                                          text => "On mysqueezebox.com",
                                        },
                                        {},
                                        {},
                                        {},
                                        {},
                                        {},
                                        {},
                                        {},
                                        {},
                                        {},
                                      ],
          preset_loop              => [1, 0, 0, 0, 0, 0, 0, 0, 0, 0],
          seq_no                   => 0,
          signalstrength           => 0,
        },
    ext => { priority => "" },
    id => 62,
  },
]
[22-09-28 17:55:27.1989] Slim::Web::Cometd::sendHTTPResponse (740) Sending Cometd chunk (192.168.1.12:39723):
[{"data":{"seq_no":"0","mixer volume":"45","player_name":"Max2Play","playlist_tracks":"0","player_connected":"1","mode":"stop","alarm_state":"none","preset_loop":["1","0","0","0","0","0","0","0","0","0"],"power":"1","playlist mode":"off","playlist repeat":"2","base":{"actions":{"more":{"params":{"context":"playlist","menu":"track"},"itemsParams":"params","window":{"isContextMenu":"1"},"cmd":["contextmenu"],"player":"0"}}},"count":"0","preset_data":[{"URL":"https://www.mysqueezebox.com/public/opml/5f0ab48ae6c061cc6b7143091b68b90e062417b0/favorites.opml","text":"On mysqueezebox.com"},{},{},{},{},{},{},{},{},{}],"alarm_timeout_seconds":"3600","alarm_next":"0","signalstrength":"0","digital_volume_control":"0","playlist shuffle":"0","alarm_snooze_seconds":"540","player_ip":"192.168.1.11:58374","alarm_version":"2"},"id":"62","channel":"/38732907/slim/playerstatus/b8:26:eb:c1:71:f0","ext":{"priority":""}}]
[22-09-28 17:55:27.1995] Slim::Web::Cometd::sendHTTPResponse (734) Sending Cometd response (192.168.1.12:40031):
HTTP/1.1 200 OK
Cache-Control: no-cache
Pragma: no-cache
Content-Length: 79
Content-Type: application/json
Expires: -1
X-Time-To-Serve: 0.00651097297668457

[{"clientId":"38732907","id":"62","channel":"/slim/request","successful":true}]
[22-09-28 17:55:27.2113] Slim::Web::Cometd::handler (149) Cometd request (192.168.1.12:40031): [
  {
    channel => "/slim/request",
    clientId => 38_732_907,
    data => {
          request  => ["b8:26:eb:c1:71:f0", ["menu", 0, 512, "direct:1"]],
          response => "/38732907/slim/request/14",
        },
    id => 63,
  },
]
SamInPgh commented 1 year ago

I ran an additional test where the behavior was as expected and the top menu for Player 2 was displayed successfully. The log files appear to be essentially identical up until the end, where the avpTop menu is requested for the successful case and is not requested for the failed case. By the way, I'm not sure whether your code is actually using the contents of the "player" attribute populated in the avpTop menu other than checking for a non-zero value but I want to reiterate that it contains a pointer to an array of client id's ($client->id) for players using the plugin and not the array itself. Anyway, here are the log excerpts.

Succeeded:

[22-09-28 22:25:44.0259] Slim::Web::Cometd::sendHTTPResponse (734) Sending Cometd response (192.168.1.12:42913):
HTTP/1.1 200 OK
Cache-Control: no-cache
Pragma: no-cache
Content-Length: 79
Content-Type: application/json
Expires: -1
X-Time-To-Serve: 0.0501861572265625

[{"clientId":"01c2cc24","id":"26","channel":"/slim/request","successful":true}]
[22-09-28 22:25:44.0385] Slim::Web::Cometd::handler (149) Cometd request (192.168.1.12:42913): [
  {
    channel => "/slim/request",
    clientId => "01c2cc24",
    data => {
          request  => ["b8:26:eb:c1:71:f0", ["avpTop", 0, 512, "useContextMenu:1"]],
          response => "/01c2cc24/slim/request/4",
        },
    id => 27,
  },
]
[22-09-28 22:25:44.0390] Plugins::DenonAvpControl::Plugin::avpTop (504) Adding the menu elements to the audio menu.
[22-09-28 22:25:44.0392] Plugins::DenonAvpControl::Plugin::avpTop (505) Client UA string is: Squeezer-squeezer/2.2.10-beta-0
[22-09-28 22:25:44.0394] Plugins::DenonAvpControl::Plugin::avpTop (509) Will use AVR menu commands. 
[22-09-28 22:25:44.0397] Plugins::DenonAvpControl::Plugin::avpTop (796) Done setting up menu items: numitems=9
[22-09-28 22:25:44.0399] Slim::Web::Cometd::handleRequest (909) Request for /01c2cc24/slim/request/4 / 27 is not async
[22-09-28 22:25:44.0402] Slim::Web::Cometd::Manager::deliver_events (240) Sending event on channel /01c2cc24/slim/request/4 to 01c2cc24
[22-09-28 22:25:44.0449] Slim::Web::Cometd::Manager::deliver_events (254) Delivering events to 01c2cc24:
[
  {
    channel => "/01c2cc24/slim/request/4",
    data => {
          count     => 9,
          item_loop => [
                         {
                           actions => { "do" => { cmd => ["avpPowerToggle", 1, 1], player => 0 } },
                           icon => "plugins/DenonAvpControl/html/images/denon_control.png",
                           id => "avpInfo",
                           nextWindow => "refresh",
                           radio => 1,
                           text => "Model: AVR-3311USA\nZone: Main   Power: ON",
                           windowId => "avpInfo",
                         },
                         {
                           actions => {
                                 go => {
                                         cmd => ["avpChannels"],
                                         params => { menu => "avpChannels" },
                                         player => 0,
                                       },
                               },
                           icon => "plugins/DenonAvpControl/html/images/channel.png",
                           id => "channels",
                           text => "Channel Level Adjustment",
                         },
                         {
                           actions => {
                                 go => {
                                         cmd => ["avpQuickSelect"],
                                         params => { menu => "avpQuickSelect" },
                                         player => 0,
                                       },
                               },
                           icon => "plugins/DenonAvpControl/html/images/subwoofer.png",
                           id => "quickselect",
                           text => "Quick Select",
                         },
                         {
                           actions => {
                                 go => {
                                         cmd => ["avpSourceSelect"],
                                         params => { menu => "avpSourceSelect" },
                                         player => 0,
                                       },
                               },
                           icon => "plugins/DenonAvpControl/html/images/select_source.png",
                           id => "sourceSelect",
                           text => "Input Source Select",
                         },
                         {
                           actions => {
                                 go => { cmd => ["avpSM"], params => { menu => "avpSM" }, player => 0 },
                               },
                           icon => "plugins/DenonAvpControl/html/images/surround_modes.png",
                           id => "surroundmode",
                           text => "Surround Mode",
                         },
                         {
                           actions => {
                                 go => { cmd => ["avpRmEq"], params => { menu => "avpRmEq" }, player => 0 },
                               },
                           icon => "plugins/DenonAvpControl/html/images/room_eq.png",
                           id => "roomequalizer",
                           text => "Room Equalizer",
                         },
                         {
                           actions => {
                                 go => { cmd => ["avpDynEq"], params => { menu => "avpDynEq" }, player => 0 },
                               },
                           icon => "plugins/DenonAvpControl/html/images/dynamic_sound.png",
                           id => "dynamicequalizer",
                           text => "Dynamic Equalizer",
                         },
                         {
                           actions => {
                                 go => { cmd => ["avpRes"], params => { menu => "avpRes" }, player => 0 },
                               },
                           icon => "plugins/DenonAvpControl/html/images/restorer.png",
                           id => "restorer",
                           text => "Audio Restorer",
                         },
                         {
                           actions => {
                                 go => { cmd => ["avpRefLvl"], params => { menu => "avpRefLvl" }, player => 0 },
                               },
                           icon => "plugins/DenonAvpControl/html/images/sliders_sm.png",
                           id => "reflevel",
                           text => "Reference Level Offset",
                         },
                       ],
          offset    => 0,
          window    => { windowStyle => "icon_list" },
        },
    ext => { priority => "" },
    id => 27,
  },
]

Failed:

[22-09-28 17:55:27.2657] Slim::Web::Cometd::sendHTTPResponse (734) Sending Cometd response (192.168.1.12:40031):
HTTP/1.1 200 OK
Cache-Control: no-cache
Pragma: no-cache
Content-Length: 79
Content-Type: application/json
Expires: -1
X-Time-To-Serve: 0.0557379722595215

[{"clientId":"38732907","id":"63","channel":"/slim/request","successful":true}]
kaaholst commented 1 year ago

Found a bug, I updated the apk in the above link. Hope this helps, please retest.

Actually I was able to reproduce it here, only it's difficult to see in the UI, because the menu is the same.

SamInPgh commented 1 year ago

Great work, Kurt! Everything seems to be working perfectly now. Thanks again for your help!

kaaholst commented 1 year ago

Thanks for the help in finding and resolving this!

And we do receive the list of players in the player attribute. We use it to determine if the menu item is to be displayed and to check if the current menu is valid when switching player. Just to be clear, we receive it is a serialized as a JSON array.

SamInPgh commented 1 year ago

Ah! Thanks for the info. Good to know. And, as you might notice, I edited my previous comment to remove the part about the problem with highlighting the selected item on the next screen, etc. etc. etc. I realized that it was in reference to Material Skin and not Squeezer. I was actually hoping that I could edit it before you read it but no luck. ;-) Anyway, take care and feel free to close this epic issue.

SamInPgh commented 1 year ago

I just realized that there is another more serious problem caused by not returning to the main (or Settings) menu when switching players. If the user is, for example, multiple levels deep in a menu and switches players, it's true that the current menu gets refreshed. However, if the user then backs out to the previous menu via the "back" button, the screen contents are NOT updated or refreshed to reflect the new player. Unlike with the current menu, there is nothing I can do in the plugin to handle this situation, as the menu function does not seem to get invoked in this case. The cached contents of the menu are simply re-displayed. An example of this behavior in the default menu structure can be seen when selecting "Settings->Advanced->Squeezebox Information->xxxx Information", where 'xxxx' is the name of the current player. If the player is switched to 'yyyy', for example, while in that screen and then the user backs up one level to the previous screen, it still shows the name of the previous player, i.e. "xxxx Information". In the case of my plugin menu, the problem is more serious than this because the cached menu will now contain audio settings relating to the AVR connected to the previous player instead of the current one and, as I said, I have no way of controlling or handling that situation --- at least none that I know of.

I hope I have explained the problem clearly and that maybe you can find a way to send the user back to the main Settings menu when switching players while in a submenu of Settings, either through a user option or by default.

Thanks.

On Sun, Jun 26, 2022, 7:11 PM Sam Yahres @.***> wrote:

I see your point regarding the usefulness of keeping the current menu active when switching players in the situations you listed. Is there any way Squeezer could detect that the Denon plugin's menu system is in use when switching players and instead return to either the top or the Settings menu?

I use "nextWindow => refresh" in many of the menus in order to update the window contents after executing a command. As I said, I could add code in each of the ten submenus to make sure that the current player is still valid and take remedial action if it's not but I'd like to avoid that if possible.

On Sun, Jun 26, 2022, 5:32 PM Kurt Aaholst @.***> wrote:

Yes going to top menu is probably the safest and also easiest to implement.

However from a user perspective it's quite handy to be able to switch player when in the now playing screen, current playlist, alarm list or even when browsed the local music and want to play the current results on a different player. Especially since this functionality is already present in Squeezer.

An alternative could be to explore the nextWindow maybe in combination with goNow functionality of SBS SqueezePlay interface https://wiki.slimdevices.com/index.php/SBS_SqueezePlay_interface, which Squeezer and I believe most other controllers use.

— Reply to this email directly, view it on GitHub https://github.com/kaaholst/android-squeezer/issues/768#issuecomment-1166651191, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASZI4RHYRY2WVI3M2HBUGJDVRDD73ANCNFSM5ZFI5XQA . You are receiving this because you authored the thread.Message ID: @.***>

kaaholst commented 1 year ago

??? Isn't that the same comment as of June 29th. Does it still apply?

SamInPgh commented 1 year ago

That IS the same comment, and I have no idea how it got posted again. A glitch in the system? I'll look into it and change my password just in case.