Opened 12 years ago

Closed 11 years ago

#3910 closed patch (wontfix)

Mythshutdown cleanup

Reported by: Matthew Wire <devel@…> Owned by: danielk
Priority: minor Milestone: unknown
Component: mythtv Version: head
Severity: medium Keywords: mythshutdown mythcontext isrunning globalsetting
Cc: Ticket locked: no

Description

3 patches:
Add getGlobalSetting and setGlobalSetting into mythcontext.cpp.
Add isRunning to util.cpp.
Remove functions from mythshutdown and use new library ones.

Attachments (15)

libmyth-mythcontext_globalSetting-31082007.diff (3.2 KB) - added by Matthew Wire <devel@…> 12 years ago.
setGlobalSetting and getGlobalSetting functions
libmyth-util_isRunning31082007.diff (1.1 KB) - added by Matthew Wire <devel@…> 12 years ago.
isRunning function
mythshutdown-31082007.diff (5.3 KB) - added by Matthew Wire <devel@…> 12 years ago.
mythshutdown using new library functions
libmyth-mythcontext_globalSettings-02092007.diff (1.2 KB) - added by Matthew Wire <devel@…> 12 years ago.
Add saveGlobalSetting function to mythcontext
libmyth-util_isRunning02092007.diff (1.1 KB) - added by Matthew Wire <devel@…> 12 years ago.
Add isRunning function to util.cpp
mythshutdown-02092007.diff (5.2 KB) - added by Matthew Wire <devel@…> 12 years ago.
Remove setGlobalSetting and getGlobalSetting and use mythcontext instead
mythshutdown-util-isRunning-18112007.patch (1.9 KB) - added by Matthew Wire <devel@…> 12 years ago.
Move isRunning function to libmyth/util.cpp
mythshutdown-mythcontext-get_setSettings-18112007.patch (6.1 KB) - added by Matthew Wire <devel@…> 12 years ago.
Use functions in mythcontext instead of copies for get/set Settings
#3910 mythshutdown-mythcontext-get_setSettings-11122007.patch (6.1 KB) - added by Matthew Wire <devel@…> 12 years ago.
Patch refresh for latest svn
#3910 mythshutdown-util-isRunning-11122007.patch (1.9 KB) - added by Matthew Wire <devel@…> 12 years ago.
Patch refresh for latest svn
05-3910-mythshutdown-util-isRunning.2.patch (2.1 KB) - added by Matthew Wire <devel@…> 11 years ago.
Updated for trunk
07-3910-mythdb-dbsettings-cleanup.patch (14.7 KB) - added by Matthew Wire <devel@…> 11 years ago.
Cleanup of database access functions
06-3910-mythshutdown-dbsettings.patch (3.9 KB) - added by Matthew Wire <devel@…> 11 years ago.
Removal of database access functions from mythshutdown
21-3910-mythshutdown-dbsettings.patch (3.9 KB) - added by Matthew Wire <devel@…> 11 years ago.
22-3910-mythshutdown-util-isRunning.2.patch (2.1 KB) - added by Matthew Wire <devel@…> 11 years ago.

Download all attachments as: .zip

Change History (25)

Changed 12 years ago by Matthew Wire <devel@…>

setGlobalSetting and getGlobalSetting functions

Changed 12 years ago by Matthew Wire <devel@…>

isRunning function

Changed 12 years ago by Matthew Wire <devel@…>

Attachment: mythshutdown-31082007.diff added

mythshutdown using new library functions

comment:1 Changed 12 years ago by Matthew Wire <devel@…>

I'm trying to work on some shutdown/wakeup features at the moment and part of this includes centralising all the shutdown/wakeup functions into a shutdownwakeup class within libmyth. The changes on this patch facilite part of this.

comment:2 in reply to:  1 Changed 12 years ago by paulh

Replying to Matthew Wire <devel@mrwire.co.uk>:

I'm trying to work on some shutdown/wakeup features at the moment and part of this includes centralising all the shutdown/wakeup functions into a shutdownwakeup class within libmyth. The changes on this patch facilite part of this.

Can you be a little more specific about where you are going with all this. What is the final goal? I'm not sure Isaac and the other devs will want to add extra stuff to libmyth unless there is a very good reason to do so.

Changed 12 years ago by Matthew Wire <devel@…>

Add saveGlobalSetting function to mythcontext

Changed 12 years ago by Matthew Wire <devel@…>

Add isRunning function to util.cpp

Changed 12 years ago by Matthew Wire <devel@…>

Attachment: mythshutdown-02092007.diff added

Remove setGlobalSetting and getGlobalSetting and use mythcontext instead

comment:3 Changed 12 years ago by Matthew Wire <devel@…>

Sorry, that comment was rather vague. I'll follow up with an email on the dev list. That'll teach me to submit late at night.
I've also refreshed all three patches. The mythcontext patch is now only four lines since I realised that the existed functions could be re-used with one wrapper function.


libmyth-mythcontext_globalSettings adds saveGlobalSetting function to mythcontext since there is currently no function that will do this.

libmyth-util_isRunning moves the isRunning function from mythshutdown to util.cpp. This function could be useful elsewhere in myth (this is the only part of the patch that I need for the shutdown/wakeup class).

mythshutdown removes getGlobalSetting and setGlobalSetting from mythshutdown and uses equivalent functions in mythcontext instead - reducing unnecessary code duplication. It also adds a dependency on util.h for the isRunning function.

Changed 12 years ago by Matthew Wire <devel@…>

Move isRunning function to libmyth/util.cpp

Changed 12 years ago by Matthew Wire <devel@…>

Use functions in mythcontext instead of copies for get/set Settings

Changed 12 years ago by Matthew Wire <devel@…>

Patch refresh for latest svn

Changed 12 years ago by Matthew Wire <devel@…>

Patch refresh for latest svn

comment:4 Changed 12 years ago by Matthew Wire <devel@…>

Updated patches for latest svn.

The mythcontext patch removes duplicate database set/get methods from mythshutdown and uses the ones in mythcontext instead.
The util patch moves the isRunning function from mythshutdown to libmyth so it can be used for other things (such as #4184).

Neither of these patches provide any new functionality in themselves, they are code cleanups which make future patches easier.

comment:5 Changed 12 years ago by danielk

Owner: changed from Isaac Richards to danielk
Status: newassigned
Version: unknownhead

comment:6 Changed 12 years ago by paulh

Matthew, I don't think getGlobalSetting("MythShutdownLock?", "0") and gContext->GetSetting?("MythShutdownLock?", "0") are equivalent the last one will prefer the setting for the local host if one is available. New setups shouldn't have any local host settings for that setting but my ancient DB on my dev box certainly does have - probably left over from some long gone version of Myth.

Changed 11 years ago by Matthew Wire <devel@…>

Updated for trunk

Changed 11 years ago by Matthew Wire <devel@…>

Cleanup of database access functions

Changed 11 years ago by Matthew Wire <devel@…>

Removal of database access functions from mythshutdown

comment:7 Changed 11 years ago by Matthew Wire <devel@…>

Updated all patches for trunk and refactored where necessary.

07-3910-mythdb-dbsettings-cleanup.patch cleans up mythdb settings access code, reducing two almost identical functions down to a single function that does all the database access (GetSettingOnHost?). In it's default mode (tryGlobal is true) it will behave as before and look for a host setting before falling back to the global settings table (Paul, this fixes your issue that you raised). GetGlobalSetting? and SaveGlobalSetting? are added as wrapper functions for use by mythshutdown.

06-3910-mythshutdown-dbsettings requires the above patch. This patches mythshutdown to remove it's own copies of the Get/Save? settings code and use the ones in mythdb instead.

Changed 11 years ago by Matthew Wire <devel@…>

Changed 11 years ago by Matthew Wire <devel@…>

comment:8 Changed 11 years ago by Matthew Wire <devel@…>

Updated patches (21 and 22) so they apply cleanly.
Note: mythshutdown patch on #5935 should be applied before these two.
Note(2): mythdb-dbsettings-cleanup should be applied before mythshutdown-dbsettings.

comment:9 Changed 11 years ago by danielk

(In [19505]) Refs #3910. Add doxygen docs for isRunning() function.

comment:10 Changed 11 years ago by danielk

Resolution: wontfix
Status: assignedclosed

Matthew, I'm sorry you have put so much effort into this. I'm not going to apply the isRunning patch because moving it to util.h might encourage others to use it, and it is just not a good idea (see the comment I added to it in [19505].) And the DB settings code changes may be fine, but I don't think the small code savings in mythshutdown are worth a potentially destabilizing change to one of the core methods of MythContext. I hope this isn't too discouraging, we really like it when people send patches and keep them up to date to boot!

Note: See TracTickets for help on using tickets.