added ToggleAddonEnabled to CBuiltins, to enable/disable an add-on #1425

Closed
wants to merge 3 commits into
from

Projects

None yet

3 participants

@opdenkamp
Member

toggles an add-on enabled/disabled

@opdenkamp
Member

@vdrfan updated after our chat on irc

@opdenkamp opdenkamp was assigned Sep 16, 2012
@opdenkamp
Member

could someone sign off on this one

@jmarshallnz
Member

What is the point of the builtin exactly? i.e. why do we need the builtin - who is calling it, and why exactly?

I'd reorder the commits as well so the enable/disable one is done first c4e6d86/43a48b5, which could be squashed, and then 4803228 (if deemed needed) on top.

I really don't like the includes of the PVR globals within AddonMgr though. It shouldn't need to know specifics about addons - ideally these would be handled instead with some sort of addon::ondisabled() or some such to isolate things.

@opdenkamp
Member

i can use it to enable/disable an add-on via the event server, easier for me when i'm testing on a system without a keyboard or remote connected. didn't see anything to do this with 1 command from the cmdline, so i added this one.

will reorder and squashing before pushing.

i'll have a look at some ondisabled() to do this.

thanks

@Montellese
Member

JSON-RPC will soon allow you to enable/disable (no toggle right now but could be added) an addon (just haven't merged the PR yet).

@opdenkamp
Member

right ok, then i'll remove the builtin and only fix the pvrmanager reset

@Montellese
Member

As an FYI I've merged the PR containing the Addons.SetAddonEnabled JSON-RPC method and I added a "toggle" functionality for easier use.

@opdenkamp
Member

thanks

@Montellese
Member

Does it make sense to drop the first commit, adjust the second one and then merge those two commits?

@opdenkamp
Member

i'll just drop this PR completely and just push the fix when i've written it

@opdenkamp opdenkamp closed this Oct 10, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment