"Program guide in live tv" keybinding overrides guide jump point

[anonymous]

Attached is an alternative approach to #3363 that was closed.

This patch may be affected by issues identified in #3636.

The approach is to filter the unwanted jumppoints out rather than patching the jumppoint code. Maybe jumppoints should be filtered iff a local keybinding is defined?

I will rework the patch if a different approach is desired.

[Roo]

Filter jumppoints in MythMainWindow::TranslateKeyPress?

[Roo]

updated patch to add filterJumpPoint parameter to translateKeyPress

[Roo]

I have updated the patch and built on my laptop, the patch appears to be working well.

translate-key-presses_v2.diff doesn't have the modifications to the TranslateKeyPress? calls, I will add them separately to the ticket, one for each change, starting with the "Program Guide" jump point.

I would be grateful for any feedback on this.

[Roo]

Fixes Program Guide jump point

[Roo]

If you want to block the "Program Finder" jump point from the "TV Playback" context then just add to the QStringList in tv_play.cpp.

QStringList excludedJumpPoints(QStringList()<<"Program Guide"<<"Program Finder"); 

[matt.doran@…]

I've tested these patches and it works well.

It would be good if pressing M in the global guide switched to live tv and changed to the selected channel.

[Roo]

Alternative approach to problem

[Roo <roo.watt@…>]

I attached an alternative approach to solving the problem as it seems that the previous method may not have been acceptable.

Cheers.

[Roo <roo.watt@…>]

Squashes finder jumppoint when in guide and vice versa

[Roo <roo.watt@…>]

V4 of the patch addresses the problem where the finder/guide jump points should be blocked when in the finder or guide.

I have mapped the finder/guide jumppoint to toggle between the finder and guide when shown, exiting from the guide or finder will return you to live/recorded tv now.

[ylee@…]

A slightly-modified version of Roo's v4 patch

[ylee@…]

I have uploaded a slightly-modified version of Roo's v4 patch. For me, running 0.20.x, v4 (which I realize was written against SVN) never worked quite right. While, within Program Finder, pushing the Program Guide jumppoint would properly take me to Guide, going from Guide to Finder didn't work because I have TV Settings|Program Guide|Allow channel jumping in guide enabled, and so would see "6" as the start of a channel number.

The attached patch, v5, has the following changes:

• The Program Guide and Program Finder jumppoints act as toggles. Pushing Guide within Guide, or Finder within Finder, exits (back to either playback or the main menu, as appropriate). Although I've never used one, I'm told that some people prefer this behavior as they are used to it from using various cable settop-box DVRs' remotes.
• Within Guide, the Finder jumppoint does not do anything; the same with the Guide jumppoint within Finder. I realize this is not optimal, but I couldn't figure out how to jump from Guide to Finder or vice versa without stuffing a keybinding into QString. (I did not want to hardwire the "#" or "S" keys, for obvious reasons.) The "FINDER" and "GUIDE" special bindings did not work in this context. I hope someone else will quickly figure out the obvious functionality I am surely overlooking and fix this issue.

[Roo <roo.watt@…>]

See #4448 for a fix for the Allow channel jumping problem. The alternate patch just needs a tweak to use the new action from the other ticket.

[Roo <roo.watt@…>]

Incorporates patch from #4448

[Roo <roo.watt@…>]

translate-key-presses_alterante_approach_v6.diff hopefully addresses the issue reported by ylee.

v6 incorporates the patch on #4448 so the FINDER & GUIDE actions can be used instead of 4 & 6.

[ylee@…]

Replying to Roo <roo.watt@gmail.com>:

translate-key-presses_alterante_approach_v6.diff hopefully addresses the issue reported by ylee.

It does; thank you.

v6 incorporates the patch on #4448 so the FINDER & GUIDE actions can be used instead of 4 & 6.

I've found the following peculiarities with v6 on 0.20.2:

• (Live TV) FINDER doesn't work. GUIDE, then FINDER, works.
• (Anywhere) When ESCAPEing from FINDER, if the previous page was GUIDE, mythfrontend returns to GUIDE instead; another ESCAPEing is required. This happens whether GUIDE was triggered via jumppoint or from the main menu.

[Roo <roo.watt@…>]

Replying to ylee@pobox.com:

Replying to Roo <roo.watt@gmail.com>:

translate-key-presses_alterante_approach_v6.diff hopefully addresses the issue reported by ylee.

It does; thank you.

Cool.

I've found the following peculiarities with v6 on 0.20.2:

• (Live TV) FINDER doesn't work. GUIDE, then FINDER, works.
• (Anywhere) When ESCAPEing from FINDER, if the previous page was GUIDE, mythfrontend returns to GUIDE instead; another ESCAPEing is required. This happens whether GUIDE was triggered via jumppoint or from the main menu.

This behaviour is the same as in current trunk I believe. I have noticed this behaviour but thought it best to keep the patch to the single issue.

[cpinkham]

(In [15414]) Copy the GUIDE and FINDER keys to the "TV Frontend" context to allow these keys to be used on the Guide Grid and Program Finder screens instead of requiring the user to hit "6" and "4" on these screens respectively to go to the other screen.

Closes #4448 and references #3642 since this patch was included as part of the v6 patch on that ticket. I'm not applying the patch in #3642 yet because I haven't looked at it enough to know if that is the best way to accomplish the task.

[cpinkham]

Replying to Roo <roo.watt@gmail.com>:

Replying to ylee@pobox.com:

• (Anywhere) When ESCAPEing from FINDER, if the previous page was GUIDE, mythfrontend returns to GUIDE instead; another ESCAPEing is required. This happens whether GUIDE was triggered via jumppoint or from the main menu.

This behaviour is the same as in current trunk I believe. I have noticed this behaviour but thought it best to keep the patch to the single issue.

This is because GuideGrid::showProgFinder() does not emit accept() to exit the GuideGrid? after showing the Program Finder. ProgFinder::showGuide() does emit accept() after showing the Guide Grid which causes the Program Finder to be exitted. I'm not sure right now which way should be the standard right now, but they both should act similar.

[ylee@…]

Replying to cpinkham:

This is because GuideGrid::showProgFinder() does not emit accept() to exit the GuideGrid? after showing the Program Finder. ProgFinder::showGuide() does emit accept() after showing the Guide Grid which causes the Program Finder to be exitted. I'm not sure right now which way should be the standard right now, but they both should act similar.

I vote for GuideGrid::showProgFinder() to emit accept(); the way ProgFinder::showGuide() behaves, in which one ESCAPE is sufficient, is clearly the norm within MythTV.

[Roo <roo.watt@…>]

v7, updated for #4448

[cpinkham]

Roo, can you try the patch attached called 'localAction_over_jumppoint_v1.diff'. I believe it accomplishes what you are trying to do in a better way.

I believe this is a more generic solution. It was easier to code it up as a proof of concept than trying to explain it. This will let you assign the same key to a global jumppoint and a local action. The jumppoint is now registered with a 'localAction' keyword. If the localAction of the jumppoint is found to be one of the actions for the key in the local context, then the jump point is not used. I tested this by setting 'S' as the key on the 'Program Guide' jumppoint. From the main menu or the watch recordings screen, the jumppoint would be processed if I hit 'S' and I would jump to the Program Guide. If I was watching a recording, then TranslateKeyPress?() would see that that the key I was using had a localAction defined on the jumppoint and this key was also bound to that local action and would use the local action instead of the jumppoint. This method is easier to extend to other actions, they just have to register the localAction keyword using REG_JUMPLOC instead of REG_JUMP. I haven't made any DB modifications as part of this patch, I have those lines #ifdef'ed out right now in the patch. You should still be able to test the patch though, because the only info that comes out of the DB is the keylist for the jumppoint.

[cpinkham]

Add localAction to jumppoint info to allow using local action instead of jumppoint bound to the same key

[Roo <roo.watt@…>]

Replying to cpinkham:

Roo, can you try the patch attached called 'localAction_over_jumppoint_v1.diff'. I believe it accomplishes what you are trying to do in a better way.

So do I, some comments follow:

• I like the generality of your approach, much nicer :)
• I wonder if there is a need to extend the jumppoint table with the extra column? Just leave the localactions defined in code, if in the future the local actions are changed (in code) then the db wouldn't need to be updated.
• If the keybinding and the jumppoint are set to different keys then the jumppoint still gets used, bit awkward in the setup aspect.

I wonder if it is worth registering a "context,action" pair, then TranslateKeyPress() could check the context and return the matching local action instead of the jumppoint.

This would have the added advantage that the local keybinding doesn't have to be the same as the jumppoint. Actually, there would be no need to have the "TV Frontend" GUIDE action bound if you have the GUIDE jumppoint set. This was one of the things I liked about the alternate approach.

Anyway, thanks for looking into this. I am keen for this to make 0.21 as it does come up a fair bit on the mailing lists.

[cpinkham]

A modified version of this patch was applied in [15736]. So, I'm closing this ticket but wanted to address the questions raised by Roo.

Replying to Roo <roo.watt@gmail.com>:

Replying to cpinkham:

• I wonder if there is a need to extend the jumppoint table with the extra column?

Just leave the localactions defined in code, if in the future the local actions are changed (in code) then the db wouldn't need to be updated.

I took this commented out code out of my patch, so the DB isn't touched.

• If the keybinding and the jumppoint are set to different keys then the jumppoint

still gets used, bit awkward in the setup aspect.

I don't see this as a problem. If the user doesn't define them both the same then it could lead to situations where we aren't sure what they actually want to do. Consider if I assign 'S' to GUIDE and '#' to FINDER in the TV Playback context, but I assign '#' to the Program Guide jumppoint and 'S' to the Program Finder jumppoint. This is a contrived example, but holds true for any case there the same key is used for a different local action and jumppoint. If the user hits '#' while watching a recording, did they want the Program Finder because of the local key binding for the context or did they want the Program Guide because of the jumppoint? That is the reason that I require both keys to be the same for the jumppoint and the local action.

I wonder if it is worth registering a "context,action" pair, then TranslateKeyPress()

could check the context and return the matching local action instead of the jumppoint.

I didn't want to tie it to that. If the localAction is defined in the current context and the keys match, then the localAction is preferred. No need to specify a lists of contexts that apply. If we didn't want to be able to use the localAction in the current context, we wouldn't have added the handling code for it, so I think it makes sense to prefer the localAction if it exists in the current context and the keys match.

This would have the added advantage that the local keybinding doesn't have to be the same as the jumppoint. Actually, there would be no need to have the "TV Frontend" GUIDE action bound if you have the GUIDE jumppoint set. This was one of the things I liked about the alternate approach.

I considered allowing the localAction to be used if there were no keys bound to it, rather than requiring the key be the same as the jumppoint, but that will require more invasive changes because we currently don't have a way to lookup an action to see what actions are defined in a given context or what keys associate with that action. This is possible, we just don't track things this way right now. The KeyContext? class in mythmainwindow.cpp would have to be modified to support this if you want to take a look at implementing it. The changes are minor, but given the fact that localActions in question already have default key assignments, it would be rare for them to be empty.

[Roo <roo.watt@…>]

Replying to cpinkham:

Replying to cpinkham:

• I wonder if there is a need to extend the jumppoint table with the extra column?

Just leave the localactions defined in code, if in the future the local actions are changed (in code) then the db wouldn't need to be updated.

I took this commented out code out of my patch, so the DB isn't touched.

Cool

• If the keybinding and the jumppoint are set to different keys then the jumppoint

still gets used, bit awkward in the setup aspect.

I don't see this as a problem. If the user doesn't define them both the same then it could lead to situations where we aren't sure what they actually want to do.

Either do I really, giving the choice to users is good, having thought about it. I was playing devils advocate.

This would have the added advantage that the local keybinding doesn't have to be the same as the jumppoint. Actually, there would be no need to have the "TV Frontend" GUIDE action bound if you have the GUIDE jumppoint set. This was one of the things I liked about the alternate approach.

I considered allowing the localAction to be used if there were no keys bound to it, rather than requiring the key be the same as the jumppoint, but that will require more invasive changes because we currently don't have a way to lookup an action to see what actions are defined in a given context or what keys associate with that action. This is possible, we just don't track things this way right now.

That was why I suggested registering the context as well as the localaction as a pair, thought that could be used instead.

The KeyContext? class in mythmainwindow.cpp would have to be modified to support this if you want to take a look at implementing it. The changes are minor, but given the fact that localActions in question already have default key assignments, it would be rare for them to be empty.

I will look into this if either, you want me to, or later if the need arises.

Thanks for detailed response by the way.