Opened 9 years ago
Closed 7 years ago
Last modified 7 years ago
#12753 closed Patch - Feature (Won't Fix)
PR for Services API Adding GetKeyBindList and PutKeyBind to Myth Services PR #120
Reported by: | Owned by: | Bill Meek | |
---|---|---|---|
Priority: | minor | Milestone: | 0.28.2 |
Component: | MythTV - Services API - Backend | Version: | Master Head |
Severity: | medium | Keywords: | |
Cc: | Ticket locked: | no |
Description
https://github.com/MythTV/mythtv/pull/120
This adds two service API calls GetKeyBindList? and PutKeyBind?. The code here is largely modeled after the existing GetSettingList/PutSetting? and storageGroupDir / storageGroupDirList code. It was done after some discussion in #12738.
This is to make working with the new Frontend/SendKey? function easier, so you can determine what key is mapped to what, and adjust key bindings. It doesn't notify/reload the frontend (but neither does MythWeb).
There is at least two major items for review/comment: -PutSetting? only returns true if it has changed the key binding. It returns false if the key binding is invalid, or if it is already set to that key. I could just have it always return true without checking if any rows are updated, or if we want to be very correct I could first do a select on the key to make sure it exists, and return true as long as it exists.
-I merged jumppoints and key bindings together (from a function point of view). Jumppoints now show up as Context=JumpPoint?. Its a hack but it avoids having to make two more functions and more round trips for a client trying to get all the current binds.
-Also I debated added an IsAlive? type option to only return keybindings for hosts that are active (when hosts are not specified) I don't feel this is really needed however.
Change History (9)
comment:1 Changed 9 years ago by
Status: | new → infoneeded_new |
---|
comment:2 Changed 9 years ago by
I have updated the code to remove the trailing spaces, will try to pay attention to that in the future not sure where I am picking them up from.
Will wait to hear back about if using the FE classes for this is the best way to go before trying to update to the KeyBindings? class.
comment:3 Changed 9 years ago by
Status: | infoneeded_new → assigned |
---|
comment:4 Changed 9 years ago by
If this is going to allow changing bindings, it should also (somehow) tell any running frontend on the host for which the bindings are changed to reload its keys, similar to what's done in KeyBindings::CommitAction?() and KeyBindings::CommitJumppoint?() or a blanket reload as in mythfrontend's static ReloadKeys?() (in main.cpp). Without that, the frontend would be running with different keys until a restart. I'd guess this would entail adding a frontend services call to ask the frontend to reload keys, then making use of that call after modifying the keys.
comment:5 Changed 9 years ago by
Sure, mainly this replicates the current MythWeb implementation. stuartm was debating in the channel if we want to continue to allow remote keybind modification or not.
We may want to (if we are to notify the frontends) figure out a way to rate limit the reload requests, incase someone is changing a lot of bindings a once rather than sending dozens of reloads all at the same time.
comment:6 Changed 9 years ago by
In thinking about this a bit more I find it would be useful to be able to get and edit key bindings via the services API. Maintenance for the code should be fairly low, it allows for easier configuration of remotes like http://www.amazon.com/Ortek-Windows-Infrared-Receiver-Ultimate/dp/B00224ZDFY/ too, and the ability to do things like duplicate control setup (or partial setup) between frontends quite easily.
I think there is something of a goal to have 3rd party clients able to be first class members of mythtv without having to resort to mythprotocol or direct SQL manipulation.
It sounds like using this in concert with SendKey? is not really recommended, to which (assuming others agree) that means we need to fix SendAction? to make sure it works for all actions possible (reachable via SendKey? that is). If others agree I can try to fix that and submit a separate PR for it.
comment:7 Changed 8 years ago by
Milestone: | 0.28.1 → 0.28.2 |
---|
Moving remaining open 0.28.1 tickets to 0.28.2
comment:8 Changed 7 years ago by
Resolution: | → Won't Fix |
---|---|
Status: | assigned → closed |
Closing any remaining tickets for 0.28, if the issue persists, feel free to reopen and align to v29 or master
comment:9 Changed 7 years ago by
Owner: | changed from Bill Meek to Bill Meek |
---|
Mitch,
Did you consider, for example: KeyBindings::ReplaceActionKey rather to doing the UPDATE keybindings in Myth::PutKeyBind?
On a different note, for getting rid of trailing blanks,this may be of value if you use vim (from .vimrc):
I don't see trailing spaces mentioned in the Coding Standards, but I have seen commits removing them.