Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

[PVR] Feature: Confirm shutdown if PVR backend is not idle. #4342

Closed
wants to merge 1 commit into from

10 participants

@ksooo
Collaborator

User story: In case shutdown is requested by the user while PVR is recordig or a recording will start very soon (and XBMC machine potentially cannot restart fast enough), XBMC refuses to shutdown, but shows a Yes/No dialog to the user explaining that PVR is currently not idle and asking the user to shut down anyway. If user does nothing for 10 secs, the dialog will autoclose and shutdown will be cancelled.

Purpose: This is very useful to prevent accidentally stopped recordings.

BTW: This feature has been discussed from time to time in the forums and there were some (hacky) addons around to implement this, but no clean implementation (which IMO can only be done inXBMC itself.)

xbmc/ApplicationMessenger.cpp
@@ -222,6 +223,24 @@ void CApplicationMessenger::ProcessMessages()
}
}
+namespace
+{
+ bool lcl_isShutdownAllowed()
+ {
+ if (CSettings::Get().GetBool("pvrmanager.enabled")
+ && CSettings::Get().GetBool("pvrpowermanagement.enabled")
+ && CSettings::Get().GetBool("pvrpowermanagement.inhibitnonidleshutdown")
+ && !g_PVRManager.IsIdle())
+ {
+ CGUIDialogKaiToast::QueueNotification(CGUIDialogKaiToast::Error,
@t-nelson
t-nelson added a note

This should be a GUIDialogYesNo and ask the user what to do. Maybe they don't care about the recording.

@ksooo Collaborator
ksooo added a note

Okay. But this dialog must not stay forever on the screen waiting for user input. I use a single remote control (HDMI-CEC) to startup/shutdown my TV, AVR and XBMC box, all with a single click on the TV remote. If I shut down my system using the TV remote, the screen switches off first, then the AVR, last the XBMC box. And in this scenario, I simply cannot see the YesNo dialog... Thus, a YesNo dialog would be fine, but with "auto dismiss" after a certain amount of user inactivity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
xbmc/ApplicationMessenger.cpp
@@ -222,6 +223,24 @@ void CApplicationMessenger::ProcessMessages()
}
}
+namespace
@t-nelson
t-nelson added a note

Why?

@ksooo Collaborator
ksooo added a note

I understood that unnamed namespaces are exactly the way to go for real local functions. If functionality is actually file-local why expose it using a (static) member function?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
xbmc/ApplicationMessenger.cpp
@@ -222,6 +223,24 @@ void CApplicationMessenger::ProcessMessages()
}
}
+namespace
+{
+ bool lcl_isShutdownAllowed()
@t-nelson
t-nelson added a note

This should probably be a static in pvr manager.

lcl_?

@ksooo Collaborator
ksooo added a note

No problem with moving it to pvr manager.

lcl_ stands for "local". If this violates XBMC coding guide lines (I'm new to the project) I will throw it away (will do so anyway when putting code into static pvr meber function).

@davilla Collaborator
davilla added a note

grep -r lcl_ xbmc, none used in xbmc code. please follow the exiting coding convention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
language/German/strings.po
@@ -7469,6 +7469,18 @@ msgctxt "#19684"
msgid "Adult"
msgstr "Erotik"
+msgctxt "#19685"
@t-nelson
t-nelson added a note

Drop changes to this file. We only commit directly to English/strings.po, translations are handled on transifex.

@ksooo Collaborator
ksooo added a note

Okay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
system/settings/settings.xml
@@ -1369,6 +1369,14 @@
<formatlabel>14044</formatlabel>
</control>
</setting>
+ <setting id="pvrpowermanagement.inhibitnonidleshutdown" type="boolean" label="19685" help="37025">
@t-nelson
t-nelson added a note

I doubt the setting is really necessary.

@ksooo Collaborator
ksooo added a note

I think it is essential to have this as a setting. Consider a setup where the pvr backend is not on the same box as XBMC is. It is IMO not a good idea to activate (or even hardcode) the prevent-shutdown behavior for this scenario. You most probably have no the intention to prevent shut down of the XBMC box in case the PVR box (on the other machine) isn't idle, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ksooo
Collaborator

Squashed down to a single commit.

@t-nelson

With the change to DialogYesNo, the setting really is not necessary.

@t-nelson t-nelson added the The Hoff label
@ksooo
Collaborator

Okay. I will remove the setting.

language/English/strings.po
@@ -15497,3 +15511,8 @@ msgstr ""
msgctxt "#37024"
msgid "Select this if the audio out connection only supports multichannel audio as Dolby Digital 5.1, this allows multichannel audio such as AAC5.1 or FLAC5.1 to be listened to in 5.1 surround sound. Note - Not recommended on Pi as this requires a lot of CPU."
msgstr ""
+
+#: system/settings/settings.xml
+msgctxt "#37025"
+msgid "Confirm shutdown of XBM if PVR is recording or next recording will start within specified backend idle time period."
@MartijnKaijser Owner

typo in XBMC. Maybe just leave it our and just say
"Confirm shutdown if PVR is recording or next recording will start within specified backend idle time period."

@t-nelson
t-nelson added a note

The string is for the setting, which is being removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ksooo ksooo changed the title from [PVR] Feature: Confirm shutdown if PVR is not idle. Can be switched on/off via... to [PVR] Feature: Confirm shutdown if PVR backend is not idle.
@ksooo
Collaborator

Setting removed, rebased, squashed down to a single commit.

@xhaggi xhaggi added the PVR label
@MartijnKaijser MartijnKaijser added this to the Pending for inclusion milestone
@xhaggi
Collaborator

this makes sense if the backend is running on the sames machine, but the shutdown has no effect if not. is there a way to verify if the backend runs on the same system?

@ksooo
Collaborator

You're completely right. Unfortunately, atm I don't have a clue how to determine whether a PVR backend is remote or not. If I find out or somebody points me to the right direction I will implement this instantly.

@da-anda
Collaborator

do we fire an event/message/callback in case XBMC is going to shut down? If not, we probably should do so (not only PVR related). Then every addon could prevent the shutdown and it's up to the PVR addons to know when it's save or not

@MartijnKaijser

python has http://mirrors.xbmc.org/docs/python-docs/13.0-gotham/xbmc.html#Monitor-onAbortRequested (so likely JSON-RPC has it as well)
and an actual bool "xbmc.abortRequested" for python

@ksooo
Collaborator

All PVR addons I know are implemented in C++. How does this fit to the python approach? What shall an addon do in case onAbortReqested is called? Is there a way for the addon to actually prevent the shutdown or to signal a veto? Is abortRequested called only for shutdown or also for suspend/hibernete? IMO only for real shutdown....

A good approach to solve the whole problem (not just PVR) could be to iterate over all addons (actually not just addons, but all xbmc "components" interested in this event) in case shutdown is requested, collecting potential vetos from the components/addons. Only in case of no veto proceed with the shutdown. Does something like this (or parts for the puzzle) already exist in XBMC?

@xhaggi
Collaborator

What shall an addon do in case onAbortReqested is called?

decide if it's safe to shutdown xbmc.

Is there a way for the addon to actually prevent the shutdown or to signal a veto?

no. only python addons can do this.

A good approach to solve the whole problem (not just PVR) could be to iterate over all addons (actually not just addons, but all xbmc "components" interested in this event) in case shutdown is requested, collecting potential vetos from the components/addons. Only in case of no veto proceed with the shutdown.

yep, this should be the way.

Does something like this (or parts for the puzzle) already exist in XBMC?

IIRC only for python plugins.

@ksooo
Collaborator

Dow! Completely right, but by far to much effort at least for me looking at the time I got to work an XBMC. :-/

BTW: To keep things simple, in the first place my PR contained a UI setting for toggling the feature - reasoning was exactly that I had the "remote PVR scenario" in mind...

BTW2: Today, CApplication::CheckShutdown() utilizes g_PVRManager.IsIdle(). The latter does not care about the "remote scenario" either. Thus, my PR does not introduce any new issues with this scenario.

@topfs2
Collaborator

@opdenkamp @Jalle19 @xhaggi helix or helix + 1?

@opdenkamp
Collaborator

looks safe for helix to me, but it's a feature, so up to you.
but I don't want this enabled by default because it can't check if the backend is on localhost, which makes this a major annoyance if you have lots of frontends using a single shared backend (like me ;))

@ksooo
Collaborator

@opdenkamp: lol. Exactly for the remote backend PVR scenario I had a setting for this fréature in the initial PR. I was asked by @t-nelson to remove it. But I would be happy to reintroduce the setting... Just let me know.

@MartijnKaijser

@ksooo please extend the strings in the language file with a description so translators know what it means/used for. See https://github.com/xbmc/xbmc/blob/master/language/English/strings.po#L6837 for example

@ksooo
Collaborator

To prevent further back and forth: If I reintroduce the UI setting, will you accept my contribution?

@ksooo
Collaborator

Please consider: People (most have a standard setup - both pvr frontend and backend running on the same macheine) continue to ask for this feature for years now. My contribution might not be perfect wrt expert, non-local PVR backend setups, but it is definitely an improvement over exiting functionality where it is easy to kill running recordings by accidentally shutting down the machine kodi and pvr backend is running on.

@opdenkamp
Collaborator

all the PVR functionality is built in such way that there's a clear separation between backend and frontend for a reason. this needs to be the same. if you want it to only apply to backends running on localhost, then we need some way to check if that's the case or not (api call or a generic 'backend ip' setting for each add-on, or some other solution).

you don't know what most people use (but you're probably correct that it's localhost stuff), but even if you did, then you'll make things annoying for the rest who use a backend on another host. and I really don't consider setting up a single IP address very advanced, as that's the only difference between running local or remote in most cases.

so add a setting please.

@ksooo
Collaborator

@opdenkamp: full ack. Will reintroduce the setting. No problem at all as I have that code as a patch for my self-built system anyway.

@ksooo
Collaborator

@opdenkamp: UI setting reintroduced, squashed down to a single commit. Okay?

@ksooo
Collaborator

@Glenn-1990: Could you please read back in this conversation. It has been explained there what the problem with "only enabling this feature when the backend is in the same machine" is.

@ksooo
Collaborator

@opdenkamp, @Glenn-1990: What do you think about this? No need for a UI setting anymore. Requires PVR API version bump and trivial modification to all PVR client addons, though.

BTW: UI setting dead with this commit, but not yet removed from code to keep the diff small and readable.

@opdenkamp
Collaborator

no API changes at this point. I know that it's easy to solve, but not for Helix.

@ksooo
Collaborator

@opdenkamp: Does "no API changes at this point" mean you would accept the solution with the UI setting for Helix or is there no chance (at this point) to get this in for Helix?

@ksooo
Collaborator

Okay. Will remove the UI setting, improve the localhost detection code, extend the addons to support new GetBackendHostname() PVR API function. Feature is for Helix+1, not Helix due to the API change. Sad, but understandable.

@ksooo
Collaborator

Removed the UI setting again, improved the localhost detection code (still not perfect). Will asap extend the various PVR addons (opdenkamp/xbmc-pvr-addons repo) to support new GetBackendHostname() PVR API function funtion.

@ksooo
Collaborator

Rebased and squashed down to a single commit. Tested for some time now. No regressions/bugs found. Considering the PR "final" (despite from future rebasing for Helix+1).

@ksooo
Collaborator

@xhaggi no special reason, just personal taste. Is this a problem?

@xhaggi
Collaborator

In general it's no problem, it saves some lines of code and IMO it's better to read. Sorry, if I'm to nit-pick ;)

@MartijnKaijser

jenkins build this please
some final review comments?

@ksooo
Collaborator

Please be aware of opdenkamp/xbmc-pvr-addons#372 which is the counterpart for this PR (PVR API change).

@opdenkamp
Collaborator
xbmc/pvr/PVRManager.cpp
@@ -1374,6 +1374,24 @@ bool CPVRManager::IsIdle(void) const
return true;
}
+bool CPVRManager::CheckShutdown(bool bAskUser /* = true */) const
@FernetMenta Collaborator

this name is misleading. in what state does it return true or false. unlike CApplication::CheckShutdown which is a command this returns bool. we often use IsCondition for those type of things.
It is really hard to follow. you negate the result of this method CApplication::CheckShutdown and all statements here are negated too.

@ksooo Collaborator
ksooo added a note

Re functionality: I documented the new member function. See Application.h:

@brief Check whether kodi can be shutdown without accidentally stopping any active recordings.
@param bAskUser In case pvr backend is not idle, true to ask user what to do, false to return false.
@return True if pvr backend is not idle or user confirms to shutdown anyway.

Re "hard to follow": No problem. Will change the implementation of the new member function and of the using code to be more "positive".

No problem with renaming this member function. Do you have a suggestion for a better name?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
xbmc/ApplicationMessenger.cpp
@@ -261,6 +261,9 @@ void CApplicationMessenger::ProcessMessage(ThreadMessage *pMsg)
case TMSG_POWERDOWN:
{
+ if (!g_PVRManager.CheckShutdown())
@FernetMenta Collaborator

all those are not correct. we only want this to be checked if powermanagement.shutdowntime is true.

@ksooo Collaborator
ksooo added a note

Not really. Ghist of this feature is to prevent user initiated shutdowns. If we only check if "autoshutdown" (powermanagement.shutdowntime > 0) is set we would loose this.

@FernetMenta Collaborator

we need an option to disable this feature for users with a setup where this does harm.

@ksooo Collaborator
ksooo added a note
@opdenkamp Collaborator

hold on, I said that it can't be used by default, as your original PR did. that would make it very annoying for users with multiple frontends sharing a single backend. and since each new setting added kills a fluffy bunny, an implementation without introducing new settings definitely has my preference.

why would we need a new setting if we know the backend is on localhost and is recording?

@ksooo Collaborator
ksooo added a note

@opdenkamp: No offense intended. Just wanted to tell @FernetMenta that we had that settings discussion already.

@FernetMenta Collaborator

why would we need a new setting if we know the backend is on localhost and is recording?

I have a more matured solution outside of xbmc which manages the HTPC in cases like wake up for recordings, inactivity powerdown, etc. I need an option to disable this.

@ksooo Collaborator
ksooo added a note

@opdenkamp , @FernetMenta : Okay. Shall I reintroduce the gui setting, defaulted to deactivate the feature?

@FernetMenta Collaborator

yes but more important is a solution where to hook this function in.

@ksooo Collaborator
ksooo added a note

@opdenkamp , @FernetMenta Sure. But I'm lost at this point and need your advice how to do it right. Simply don't know enough about Kodi code base to find the right place for hooking the shutdown check in.

@FernetMenta Collaborator

In Application.cpp there are two places where CBuiltins::Execute is called. If I am no mistaken those calls do either come from action built into the skin or button presses. You could hook in if action.GetName starts with "Powerdown".

@ksooo Collaborator
ksooo added a note
@opdenkamp Collaborator

I have a more matured solution outside of xbmc which manages the HTPC in cases like wake up for recordings, inactivity powerdown, etc. I need an option to disable this.

Okay. Shall I reintroduce the gui setting, defaulted to deactivate the feature?

Sorry, but you'll have to convince me that we need a setting to prevent a one time dialog on shutdown.

All this does (or should do once it's changed) is asking the user if he really wants to shut down and interrupt a running recording. Even with your external management of system power, you will still interrupt the recording when you shut down the system?

@FernetMenta Collaborator

Now that we seem to have found a way how to detect user interaction via UI I don't need the setting. I never use XBMC's shutdown menu. I use kodi-send with Quit command. The script which sends this command takes care of recordings.

@opdenkamp Collaborator

okay great

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
xbmc/ApplicationMessenger.cpp
((7 lines not shown))
g_application.Stop(EXITCODE_QUIT);
}
break;
case TMSG_HIBERNATE:
{
+ if (!g_PVRManager.CheckShutdown())
@FernetMenta Collaborator

I think all those have to go away from here. If the system receives a message for hibernate because battery is weak is can't be ignored here. same is true for quit or suspend.

@ksooo Collaborator
ksooo added a note

You are right. I did not know that this code will be called for non-user initiated action. But, where to put the checks to only do it for user initiated action?

@opdenkamp Collaborator

you also can't infest appmessenger with a whole lot of pvr specific checks. these calls to g_PVRManager (which is a lazy man's macro @FernetMenta :P) have to be moved out of here anyway.

check CApplication::CheckShutdown() instead (think it was that one, haven't seen that code for quite some time)

@ksooo Collaborator
ksooo added a note

Calling CApplication::CheckShutdown() IMO does not fit because of its semantics. It does not only check but actually does if some conditions are met a CApplicationMessenger::Get().Shutdown().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@FernetMenta
Collaborator

the yes/no dialog must only be showed if shutdown was initiated by interactive user action. you can't bring up the dialog if quit was called by some lower layer or by a script using json/rpc api.

@ksooo
Collaborator

I think the problem with the dialog will be easily solved when the right place for doing the shutdown checks is found. Please refer to my other comment on this issue.

@FernetMenta
Collaborator

I had a look into the code and it seems that this problem is not trivial to solve. The skins call builtins like powerdown directly so there is no chance to detect if it was an ui command issued by a user. a possible solution might be to implements something like "userpowerdown" and use this in skins.

@ksooo
Collaborator

@FernetMenta: BTW, the dialog uses autoclose. Thus, no hangs in case of non-user initiated action.

@ksooo
Collaborator

@FernetMenta, @opdenkamp : Reworked as advised. Seems to work just fine.

What changed:

  • moved the powerdown checks away from CApplicationMessenger::ProcessMessage() to CApplication::OnAction(), CApplication::ExecuteXBMCAction()
  • additionally, the check needed to be introduced in CPeripheralCecAdapter::CecCommand(), CGUIWindowLoginScreen::OnAction()
  • Introduced helpers CApplication::IsSystemPowerdownAllowed, CBuiltins::IsSystemPowerdownCommand()

Please review.

xbmc/Application.cpp
@@ -2525,6 +2527,20 @@ bool CApplication::OnAppCommand(const CAction &action)
return true;
}
+bool CApplication::IsSystemPowerdownAllowed(void) const
@FernetMenta Collaborator

this method does only check PVRManager. hence it should belong to PVRManager not application.

@ksooo Collaborator
ksooo added a note

@FernetMenta : Idea was that this method could be used in the future to add powerdown checks for other subsystems. But atm I have no concrete scenarios for that. Thus, I can move this method to PVRManager if you want me to do that.

@FernetMenta Collaborator

I'd prefer moving it for now. If there will be a case in the future which justifies a method in Application (or elsewhere), it can be changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ksooo
Collaborator

@FernetMenta : Refactored. Implementation of IsSystemPowerdownYYY moved completely away from CApplication.

Anything else from the reviewers?

xbmc/interfaces/Builtins.cpp
((19 lines not shown))
+ return true;
+ }
+ else if (execute == "shutdown")
+ {
+ switch (CSettings::Get().GetInt("powermanagement.shutdownstate"))
+ {
+ // Note: Cases taken from CApplicationMessenger::ProcessMessage()
+ case POWERSTATE_SHUTDOWN:
+ case POWERSTATE_SUSPEND:
+ case POWERSTATE_HIBERNATE:
+ return true;
+
+ case POWERSTATE_QUIT:
+ case POWERSTATE_MINIMIZE:
+ case TMSG_RENDERER_FLUSH:
+ return false;
@FernetMenta Collaborator

is is good practice to add the default case.

@xhaggi Collaborator
xhaggi added a note

do we need the opposite part or just the default?

@ksooo Collaborator
ksooo added a note

As I wrote in the comment I raped the cases from CApplicationMessenger::ProcessState and by handling all of them here I wanted to make clear that nothing was forgotten.

@xhaggi Collaborator
xhaggi added a note

IMO it makes no sense to duplicate code if we don't need it for this kind of validation. At the latest when a new value is added which is not part of the shutdown commands you need to adjust it on two places, but it is not really necessary.

@ksooo Collaborator
ksooo added a note
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@FernetMenta
Collaborator

with the default case added it looks ok to me.

@ksooo
Collaborator

@FernetMenta : default added to CBuiltins::IsSystemPowerdownCommand().
@opdenkamp : now it's up to you to give your go/nogo.

@xhaggi xhaggi commented on the diff
xbmc/pvr/addons/PVRClients.cpp
((6 lines not shown))
+{
+ // timers going off soon?
+ const CDateTime now(CDateTime::GetUTCDateTime());
+ const CDateTimeSpan idle(
+ 0, 0, CSettings::Get().GetInt("pvrpowermanagement.backendidletime"), 0);
+ const CDateTime next(timers.GetNextEventTime());
+ const CDateTimeSpan delta(next - now);
+
+ return (delta <= idle);
+}
+
+bool CPVRClients::AllLocalBackendsIdle() const
+{
+ PVR_CLIENTMAP clients;
+ GetConnectedClients(clients);
+ for (PVR_CLIENTMAP_CITR itr = clients.begin(); itr != clients.end(); itr++)
@xhaggi Collaborator
xhaggi added a note

please use it instead of itr and ++it.

@ksooo Collaborator
ksooo added a note

@xhaggi : LOL. Actually I copied the code for the loop. It is done like this all over the place in CPVRClients. But yes , will change it, hoping that @opdenkamp has no problem with this.

@xhaggi Collaborator
xhaggi added a note

you can leave it for now. i will do a refactor later.

@ksooo Collaborator
ksooo added a note

@xhaggi: Okay. Thanks.

@opdenkamp Collaborator

yeah when i originally wrote that code, i assumed that the compiler would optimise that stuff out and never paid much attention to it. but it only seems to do that for primitives :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
xbmc/pvr/PVRManager.cpp
@@ -1414,6 +1414,28 @@ bool CPVRManager::IsIdle(void) const
return true;
}
+bool CPVRManager::IsSystemPowerdownOkay(bool bAskUser /*= true*/) const
@xhaggi Collaborator
xhaggi added a note

i don't like the wording. should we rename it to CanSystemPowerdown()

@ksooo Collaborator
ksooo added a note

@xhaggi: Good point. I don't like the name either, but had no better idea. Will change the method's name to CanSystemPowerdown.

@xhaggi Collaborator
xhaggi added a note

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
xbmc/pvr/PVRManager.cpp
@@ -1414,6 +1414,28 @@ bool CPVRManager::IsIdle(void) const
return true;
}
+bool CPVRManager::IsSystemPowerdownOkay(bool bAskUser /*= true*/) const
+{
+ if (IsStarted() && CSettings::Get().GetBool("pvrmanager.enabled"))
@xhaggi Collaborator
xhaggi added a note

no need to check for the setting here IMO

@ksooo Collaborator
ksooo added a note

Okay, will remove the check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
xbmc/pvr/addons/PVRClients.cpp
((16 lines not shown))
+
+bool CPVRClients::AllLocalBackendsIdle() const
+{
+ PVR_CLIENTMAP clients;
+ GetConnectedClients(clients);
+ for (PVR_CLIENTMAP_CITR itr = clients.begin(); itr != clients.end(); itr++)
+ {
+ CPVRTimers timers;
+ PVR_ERROR ret = itr->second->GetTimers(&timers);
+ if (ret == PVR_ERROR_NOT_IMPLEMENTED || ret != PVR_ERROR_NO_ERROR)
+ continue;
+
+ if (((timers.AmountActiveRecordings() > 0) // active recordins?
+ || (NextEventWithinBackendIdleTime(timers))) // next recording soon?
+ && g_application.getNetwork().IsLocalHost(
+ itr->second->GetBackendHostname())) // ... and a local backend?
@xhaggi Collaborator
xhaggi added a note

do not break the parameter to new line, less readable IMO and please remove the unnecessary brackets

@xhaggi Collaborator
xhaggi added a note

and no need for the comments

@ksooo Collaborator
ksooo added a note

Okay. Will compact the code like you suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
xbmc/windows/GUIWindowLoginScreen.cpp
@@ -141,7 +142,8 @@ bool CGUIWindowLoginScreen::OnAction(const CAction &action)
{
CStdString actionName = action.GetName();
StringUtils::ToLower(actionName);
- if (actionName.find("shutdown") != std::string::npos)
+ if ((actionName.find("shutdown") != std::string::npos) &&
@xhaggi Collaborator
xhaggi added a note

should we use the new method CBuiltins::IsSystemPowerdownCommand() instead?

@xhaggi Collaborator
xhaggi added a note

sorry ignore my comment :) I'm wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ksooo
Collaborator

All changes requested by @xhaggi are in now. Squashed down to a single commit. PR is waiting for final review of @opdenkamp.

xbmc/network/Network.cpp
((20 lines not shown))
#endif
- return hostStr;
+ return true;
+}
+
+bool CNetwork::IsLocalHost(const std::string& hostname)
+{
+ if (hostname.empty())
+ return false;
+
+ if ((hostname == "127.0.0.1")
@Jalle19 Collaborator
Jalle19 added a note

Technically, anything in 127.0.0.0/8 is localhost. Especially machines with multiple network interfaces tend to have more than one of these addresses.

@opdenkamp Collaborator

localhost is the whole 127.0.0.1/255.0.0.0 range

@Jalle19 Collaborator
Jalle19 added a note

That's what I said.

@Jalle19 Collaborator
Jalle19 added a note

In this case though it'll be the user who enters the address that we want to check here. I doubt anyone would enter e.g. 127.2.18.1, but since this method is generic it should probably handle all cases so we don't get burned in the future.

@opdenkamp Collaborator

yeah we both commented on the same time from the looks of it :P

@ksooo Collaborator
ksooo added a note

@opdenkamp, @jalle19 : Not quite sure what you mean. Could you sketch this on code, please? Maybe as a pseudo diff to the current implementation?

@opdenkamp Collaborator

i think something simple as checking for the string starting with "127." should be good enough here. if we really want to validate if it's actually a correct ip, then checking for a regex would be easiest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Jalle19 Jalle19 commented on the diff
xbmc/network/Network.cpp
((27 lines not shown))
+ if (hostname.empty())
+ return false;
+
+ if ((hostname == "127.0.0.1")
+ || (hostname == "::1")
+ || StringUtils::EqualsNoCase(hostname, "localhost"))
+ return true;
+
+ std::string myhostname;
+ if (GetHostName(myhostname)
+ && StringUtils::EqualsNoCase(hostname, myhostname))
+ return true;
+
+ std::vector<CNetworkInterface*>& ifaces = GetInterfaceList();
+ std::vector<CNetworkInterface*>::const_iterator iter = ifaces.begin();
+ while (iter != ifaces.end())
@Jalle19 Collaborator
Jalle19 added a note

This could possibly be done a bit more elegantly with std::find if CNetworkInterface had a comparison overload for const std::string &hostname.

@ksooo Collaborator
ksooo added a note

Feel free to optimize it once this PR got merged. I still want the PR not to get to big. Or is this a merge-killer?

@opdenkamp Collaborator

nah

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
xbmc/peripherals/devices/PeripheralCecAdapter.cpp
@@ -626,11 +627,14 @@ int CPeripheralCecAdapter::CecCommand(void *cbParam, const cec_command command)
(adapter->m_configuration.bPowerOffOnStandby == 1 || adapter->m_configuration.bShutdownOnStandby == 1) &&
(!adapter->m_standbySent.IsValid() || CDateTime::GetCurrentDateTime() - adapter->m_standbySent > CDateTimeSpan(0, 0, 0, SCREENSAVER_TIMEOUT)))
{
- adapter->m_bStarted = false;
- if (adapter->m_configuration.bPowerOffOnStandby == 1)
- CApplicationMessenger::Get().Suspend();
- else if (adapter->m_configuration.bShutdownOnStandby == 1)
- CApplicationMessenger::Get().Shutdown();
+ if (PVR::g_PVRManager.CanSystemPowerdown())
@opdenkamp Collaborator

please address this globally and not in cec specific code

@opdenkamp Collaborator

so in CApplication or something

@FernetMenta Collaborator

or call CApplication::ExecuteXBMCAction here

@opdenkamp Collaborator

yup, that'll be fine too

@ksooo Collaborator
ksooo added a note

Okay. Will change this to call CApplication::ExecuteXBMCAction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@opdenkamp opdenkamp commented on the diff
xbmc/pvr/PVRManager.cpp
@@ -1414,6 +1414,28 @@ bool CPVRManager::IsIdle(void) const
return true;
}
+bool CPVRManager::CanSystemPowerdown(bool bAskUser /*= true*/) const
@opdenkamp Collaborator

i really dislike multiple return statements in a method, so I use a bool bReturn in the pvr code.
please change this code to something like this:

bool CPVRManager::CanSystemPowerdown(bool bAskUser /*= true*/) const
{
  bool bReturn(true);
  if (IsStarted())
  {
    if (!m_addons->AllLocalBackendsIdle())
    {
      if (bAskUser)
      {
        // Inform user about PVR being busy. Ask if user wants to powerdown anyway.
        bool bCanceled = false;
        bReturn = CGUIDialogYesNo::ShowAndGetInput(19685, 19686, 0, 0, -1, -1, bCanceled, 10000);
      }
      else
        bRreturn = false; // do not powerdown (busy, but no user interaction requested).
    }
  }
  return bReturn; // powerdown okay.
}

(it's less code too ;-))

@ksooo Collaborator
ksooo added a note

Okay. Will change this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@opdenkamp
Collaborator

some minor comments, but other than those, it's fine with me.

note to the merge trigger happy team members: don't merge this yet because we need to pull updated pvr add-ons as part of this pr.

@ksooo
Collaborator

All changes requested by @opdenkamp and @Jalle19 are in now. Squashed PR down to a single commit. I hope, we are ready for merging now... Thanks to all reviewers.

@ksooo
Collaborator

@opdenkamp any chance to get this merged this weekend, given that I rebase tonight?

@ksooo ksooo [PVR] Feature: Confirm XBMC shutdown if any local PVR backend is not …
…idle.

- Bumped PVR addon API version to 1.9.3 (new function getBackendHostname())
- Fixed CNetwork::GetHostName signature and implementation.
- Added CNetwork::IsLocalHost.
- Changed CURL::IsLocalHost and CPVRClients::AllLocalBackendsIdle to use CNetwork::IsLocalHost.
b06cd7d
@ksooo
Collaborator

Rebased.

@opdenkamp opdenkamp referenced this pull request from a commit in opdenkamp/xbmc
@opdenkamp opdenkamp [PVR] bump add-ons (sync api with PR #4342)
closes #4342
919192b
@opdenkamp opdenkamp referenced this pull request
Merged

PR 4342 + add-on sync #6196

@opdenkamp opdenkamp closed this
@ksooo ksooo deleted the ksooo:no-shutdown-if-pvr-is-not-idle branch
@jediry jediry referenced this pull request from a commit in jediry/xbmc
@opdenkamp opdenkamp [PVR] bump add-ons (sync api with PR #4342)
closes #4342
6a68781
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 11, 2015
  1. @ksooo

    [PVR] Feature: Confirm XBMC shutdown if any local PVR backend is not …

    ksooo authored
    …idle.
    
    - Bumped PVR addon API version to 1.9.3 (new function getBackendHostname())
    - Fixed CNetwork::GetHostName signature and implementation.
    - Added CNetwork::IsLocalHost.
    - Changed CURL::IsLocalHost and CPVRClients::AllLocalBackendsIdle to use CNetwork::IsLocalHost.
This page is out of date. Refresh to see the latest.
View
14 language/English/strings.po
@@ -9295,7 +9295,19 @@ msgctxt "#19684"
msgid "Adult"
msgstr ""
-#empty strings from id 19685 to 19999
+#. Title for shutdown confirmation dialog
+#: xbmc/ApplicationMessenger.cpp
+msgctxt "#19685"
+msgid "Confirm shutdown"
+msgstr ""
+
+#. Text for shutdown confirmation dialog
+#: xbmc/ApplicationMessenger.cpp
+msgctxt "#19686"
+msgid "The PVR backend is busy. Shutdown anyway?"
+msgstr ""
+
+#empty strings from id 19687 to 19999
#: system/settings/settings.xml
msgctxt "#20000"
View
20 xbmc/Application.cpp
@@ -734,7 +734,9 @@ bool CApplication::Create()
std::string executable = CUtil::ResolveExecutablePath();
CLog::Log(LOGNOTICE, "The executable running is: %s", executable.c_str());
- CLog::Log(LOGNOTICE, "Local hostname: %s", m_network->GetHostName().c_str());
+ std::string hostname("[unknown]");
+ m_network->GetHostName(hostname);
+ CLog::Log(LOGNOTICE, "Local hostname: %s", hostname.c_str());
std::string lowerAppName = CCompileInfo::GetAppName();
StringUtils::ToLower(lowerAppName);
CLog::Log(LOGNOTICE, "Log File is located: %s%s.log", g_advancedSettings.m_logFolder.c_str(), lowerAppName.c_str());
@@ -2589,8 +2591,12 @@ bool CApplication::OnAction(const CAction &action)
// built in functions : execute the built-in
if (action.GetID() == ACTION_BUILT_IN_FUNCTION)
{
- CBuiltins::Execute(action.GetName());
- m_navigationTimer.StartZero();
+ if (!CBuiltins::IsSystemPowerdownCommand(action.GetName()) ||
+ g_PVRManager.CanSystemPowerdown())
+ {
+ CBuiltins::Execute(action.GetName());
+ m_navigationTimer.StartZero();
+ }
return true;
}
@@ -4723,7 +4729,7 @@ void CApplication::CheckShutdown()
|| m_musicInfoScanner->IsScanning()
|| m_videoInfoScanner->IsScanning()
|| g_windowManager.IsWindowActive(WINDOW_DIALOG_PROGRESS) // progress dialog is onscreen
- || (CSettings::Get().GetBool("pvrmanager.enabled") && !g_PVRManager.IsIdle()))
+ || !g_PVRManager.CanSystemPowerdown(false))
{
m_shutdownTimer.StartZero();
return;
@@ -4999,7 +5005,11 @@ bool CApplication::ExecuteXBMCAction(std::string actionStr)
// user has asked for something to be executed
if (CBuiltins::HasCommand(actionStr))
- CBuiltins::Execute(actionStr);
+ {
+ if (!CBuiltins::IsSystemPowerdownCommand(actionStr) ||
+ g_PVRManager.CanSystemPowerdown())
+ CBuiltins::Execute(actionStr);
+ }
else
{
// try translating the action from our ButtonTranslator
View
6 xbmc/GUIInfoManager.cpp
@@ -1921,7 +1921,11 @@ std::string CGUIInfoManager::GetLabel(int info, int contextWindow, std::string *
{
std::string friendlyName = CSettings::Get().GetString("services.devicename");
if (StringUtils::EqualsNoCase(friendlyName, CCompileInfo::GetAppName()))
- strLabel = StringUtils::Format("%s (%s)", friendlyName.c_str(), g_application.getNetwork().GetHostName().c_str());
+ {
+ std::string hostname("[unknown]");
+ g_application.getNetwork().GetHostName(hostname);
+ strLabel = StringUtils::Format("%s (%s)", friendlyName.c_str(), hostname.c_str());
+ }
else
strLabel = friendlyName;
}
View
8 xbmc/URL.cpp
@@ -19,6 +19,7 @@
*/
#include "URL.h"
+#include "Application.h"
#include "utils/RegExp.h"
#include "utils/log.h"
#include "utils/URIUtils.h"
@@ -29,6 +30,7 @@
#include "filesystem/StackDirectory.h"
#include "addons/Addon.h"
#include "utils/StringUtils.h"
+#include "network/Network.h"
#ifndef TARGET_POSIX
#include <sys\types.h>
#include <sys\stat.h>
@@ -679,14 +681,12 @@ std::string CURL::GetRedacted(const std::string& path)
bool CURL::IsLocal() const
{
- return (IsLocalHost() || m_strProtocol.empty());
+ return (m_strProtocol.empty() || IsLocalHost());
}
bool CURL::IsLocalHost() const
{
- // localhost is case-insensitive
- return (StringUtils::EqualsNoCase(m_strHostName, "localhost") ||
- m_strHostName == "127.0.0.1");
+ return g_application.getNetwork().IsLocalHost(m_strHostName);
}
bool CURL::IsFileOnly(const std::string &url)
View
9 xbmc/addons/include/xbmc_pvr_dll.h
@@ -595,6 +595,13 @@ extern "C"
time_t GetBufferTimeEnd();
/*!
+ * Get the hostname of the pvr backend server
+ * @return hostname as ip address or alias. If backend does not
+ * utilize a server, return empty string.
+ */
+ const char* GetBackendHostname();
+
+ /*!
* Called by XBMC to assign the function pointers of this add-on to pClient.
* @param pClient The struct to assign the function pointers to.
*/
@@ -674,6 +681,8 @@ extern "C"
pClient->GetPlayingTime = GetPlayingTime;
pClient->GetBufferTimeStart = GetBufferTimeStart;
pClient->GetBufferTimeEnd = GetBufferTimeEnd;
+
+ pClient->GetBackendHostname = GetBackendHostname;
};
};
View
5 xbmc/addons/include/xbmc_pvr_types.h
@@ -75,10 +75,10 @@ struct DemuxPacket;
#define PVR_STREAM_MAX_STREAMS 20
/* current PVR API version */
-#define XBMC_PVR_API_VERSION "1.9.2"
+#define XBMC_PVR_API_VERSION "1.9.3"
/* min. PVR API version */
-#define XBMC_PVR_MIN_API_VERSION "1.9.2"
+#define XBMC_PVR_MIN_API_VERSION "1.9.3"
#ifdef __cplusplus
extern "C" {
@@ -399,6 +399,7 @@ extern "C" {
time_t (__cdecl* GetPlayingTime)(void);
time_t (__cdecl* GetBufferTimeStart)(void);
time_t (__cdecl* GetBufferTimeEnd)(void);
+ const char* (__cdecl* GetBackendHostname)(void);
} PVRClient;
#ifdef __cplusplus
View
34 xbmc/interfaces/Builtins.cpp
@@ -96,6 +96,7 @@
#include <vector>
#include "settings/AdvancedSettings.h"
#include "settings/DisplaySettings.h"
+#include "powermanagement/PowerManager.h"
using namespace std;
using namespace XFILE;
@@ -244,6 +245,39 @@ bool CBuiltins::HasCommand(const std::string& execString)
return false;
}
+bool CBuiltins::IsSystemPowerdownCommand(const std::string& execString)
+{
+ std::string execute;
+ vector<string> params;
+ CUtil::SplitExecFunction(execString, execute, params);
+ StringUtils::ToLower(execute);
+
+ // Check if action is resulting in system powerdown.
+ if (execute == "reboot" ||
+ execute == "restart" ||
+ execute == "reset" ||
+ execute == "powerdown" ||
+ execute == "hibernate" ||
+ execute == "suspend" )
+ {
+ return true;
+ }
+ else if (execute == "shutdown")
+ {
+ switch (CSettings::Get().GetInt("powermanagement.shutdownstate"))
+ {
+ case POWERSTATE_SHUTDOWN:
+ case POWERSTATE_SUSPEND:
+ case POWERSTATE_HIBERNATE:
+ return true;
+
+ default:
+ return false;
+ }
+ }
+ return false;
+}
+
void CBuiltins::GetHelp(std::string &help)
{
help.clear();
View
1  xbmc/interfaces/Builtins.h
@@ -26,6 +26,7 @@ class CBuiltins
{
public:
static bool HasCommand(const std::string& execString);
+ static bool IsSystemPowerdownCommand(const std::string& execString);
static void GetHelp(std::string &help);
static int Execute(const std::string& execString);
};
View
40 xbmc/network/Network.cpp
@@ -159,19 +159,49 @@ int CNetwork::ParseHex(char *str, unsigned char *addr)
return len;
}
-std::string CNetwork::GetHostName(void)
+bool CNetwork::GetHostName(std::string& hostname)
{
char hostName[128];
if (gethostname(hostName, sizeof(hostName)))
- return "unknown";
+ return false;
- std::string hostStr;
#ifdef TARGET_WINDOWS
+ std::string hostStr;
g_charsetConverter.systemToUtf8(hostName, hostStr);
+ hostname = hostStr;
#else
- hostStr = hostName;
+ hostname = hostName;
#endif
- return hostStr;
+ return true;
+}
+
+bool CNetwork::IsLocalHost(const std::string& hostname)
+{
+ if (hostname.empty())
+ return false;
+
+ if (StringUtils::StartsWith(hostname, "127.")
+ || (hostname == "::1")
+ || StringUtils::EqualsNoCase(hostname, "localhost"))
+ return true;
+
+ std::string myhostname;
+ if (GetHostName(myhostname)
+ && StringUtils::EqualsNoCase(hostname, myhostname))
+ return true;
+
+ std::vector<CNetworkInterface*>& ifaces = GetInterfaceList();
+ std::vector<CNetworkInterface*>::const_iterator iter = ifaces.begin();
+ while (iter != ifaces.end())
@Jalle19 Collaborator
Jalle19 added a note

This could possibly be done a bit more elegantly with std::find if CNetworkInterface had a comparison overload for const std::string &hostname.

@ksooo Collaborator
ksooo added a note

Feel free to optimize it once this PR got merged. I still want the PR not to get to big. Or is this a merge-killer?

@opdenkamp Collaborator

nah

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ {
+ CNetworkInterface* iface = *iter;
+ if (iface && iface->GetCurrentIPAddress() == hostname)
+ return true;
+
+ ++iter;
+ }
+
+ return false;
}
CNetworkInterface* CNetwork::GetFirstConnectedInterface()
View
5 xbmc/network/Network.h
@@ -110,7 +110,7 @@ class CNetwork
virtual ~CNetwork();
// Return our hostname
- virtual std::string GetHostName(void);
+ virtual bool GetHostName(std::string& hostname);
// Return the list of interfaces
virtual std::vector<CNetworkInterface*>& GetInterfaceList(void) = 0;
@@ -146,6 +146,9 @@ class CNetwork
void StopServices(bool bWait);
static int ParseHex(char *str, unsigned char *addr);
+
+ // Return true if given name or ip address corresponds to localhost
+ bool IsLocalHost(const std::string& hostname);
};
#ifdef HAS_LINUX_NETWORK
View
4 xbmc/peripherals/devices/PeripheralCecAdapter.cpp
@@ -628,9 +628,9 @@ int CPeripheralCecAdapter::CecCommand(void *cbParam, const cec_command command)
{
adapter->m_bStarted = false;
if (adapter->m_configuration.bPowerOffOnStandby == 1)
- CApplicationMessenger::Get().Suspend();
+ g_application.ExecuteXBMCAction("Suspend");
else if (adapter->m_configuration.bShutdownOnStandby == 1)
- CApplicationMessenger::Get().Shutdown();
+ g_application.ExecuteXBMCAction("Shutdown");
}
break;
case CEC_OPCODE_SET_MENU_LANGUAGE:
View
20 xbmc/pvr/PVRManager.cpp
@@ -1414,6 +1414,26 @@ bool CPVRManager::IsIdle(void) const
return true;
}
+bool CPVRManager::CanSystemPowerdown(bool bAskUser /*= true*/) const
@opdenkamp Collaborator

i really dislike multiple return statements in a method, so I use a bool bReturn in the pvr code.
please change this code to something like this:

bool CPVRManager::CanSystemPowerdown(bool bAskUser /*= true*/) const
{
  bool bReturn(true);
  if (IsStarted())
  {
    if (!m_addons->AllLocalBackendsIdle())
    {
      if (bAskUser)
      {
        // Inform user about PVR being busy. Ask if user wants to powerdown anyway.
        bool bCanceled = false;
        bReturn = CGUIDialogYesNo::ShowAndGetInput(19685, 19686, 0, 0, -1, -1, bCanceled, 10000);
      }
      else
        bRreturn = false; // do not powerdown (busy, but no user interaction requested).
    }
  }
  return bReturn; // powerdown okay.
}

(it's less code too ;-))

@ksooo Collaborator
ksooo added a note

Okay. Will change this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+{
+ bool bReturn(true);
+ if (IsStarted())
+ {
+ if (!m_addons->AllLocalBackendsIdle())
+ {
+ if (bAskUser)
+ {
+ // Inform user about PVR being busy. Ask if user wants to powerdown anyway.
+ bool bCanceled = false;
+ bReturn = CGUIDialogYesNo::ShowAndGetInput(19685, 19686, 0, 0, -1, -1, bCanceled, 10000);
+ }
+ else
+ bReturn = false; // do not powerdown (busy, but no user interaction requested).
+ }
+ }
+ return bReturn;
+}
+
void CPVRManager::ShowPlayerInfo(int iTimeout)
{
if (IsStarted() && m_guiInfo)
View
12 xbmc/pvr/PVRManager.h
@@ -372,6 +372,18 @@ namespace PVR
bool IsIdle(void) const;
/*!
+ * @brief Check whether the system Kodi is running on can be powered down
+ * (shutdown/reboot/suspend/hibernate) without stopping any active
+ * recordings and/or without preventing the start of recordings
+ * sheduled for now + pvrpowermanagement.backendidletime.
+ * @param bAskUser True to informs user in case of potential
+ * data loss. User can decide to allow powerdown anyway. False to
+ * not to ask user and to not confirm power down.
+ * @return True if system can be safely powered down, false otherwise.
+ */
+ bool CanSystemPowerdown(bool bAskUser = true) const;
+
+ /*!
* @brief Set the current playing group, used to load the right channel.
* @param group The new group.
*/
View
13 xbmc/pvr/addons/PVRClient.cpp
@@ -132,6 +132,7 @@ void CPVRClient::ResetProperties(int iClientId /* = PVR_INVALID_CLIENT_ID */)
m_strConnectionString = DEFAULT_INFO_STRING_VALUE;
m_strFriendlyName = DEFAULT_INFO_STRING_VALUE;
m_strBackendName = DEFAULT_INFO_STRING_VALUE;
+ m_strBackendHostname.clear();
m_bIsPlayingTV = false;
m_bIsPlayingRecording = false;
memset(&m_addonCapabilities, 0, sizeof(m_addonCapabilities));
@@ -353,7 +354,7 @@ bool CPVRClient::CheckAPIVersion(void)
bool CPVRClient::GetAddonProperties(void)
{
- std::string strBackendName, strConnectionString, strFriendlyName, strBackendVersion;
+ std::string strBackendName, strConnectionString, strFriendlyName, strBackendVersion, strBackendHostname;
PVR_ADDON_CAPABILITIES addonCapabilities;
/* get the capabilities */
@@ -384,12 +385,17 @@ bool CPVRClient::GetAddonProperties(void)
try { strBackendVersion = m_pStruct->GetBackendVersion(); }
catch (std::exception &e) { LogException(e, "GetBackendVersion()"); return false; }
+ /* backend hostname */
+ try { strBackendHostname = m_pStruct->GetBackendHostname(); }
+ catch (std::exception &e) { LogException(e, "GetBackendHostname()"); return false; }
+
/* update the members */
m_strBackendName = strBackendName;
m_strConnectionString = strConnectionString;
m_strFriendlyName = strFriendlyName;
m_strBackendVersion = strBackendVersion;
m_addonCapabilities = addonCapabilities;
+ m_strBackendHostname = strBackendHostname;
return true;
}
@@ -410,6 +416,11 @@ const std::string& CPVRClient::GetBackendVersion(void) const
return m_strBackendVersion;
}
+const std::string& CPVRClient::GetBackendHostname(void) const
+{
+ return m_strBackendHostname;
+}
+
const std::string& CPVRClient::GetConnectionString(void) const
{
return m_strConnectionString;
View
6 xbmc/pvr/addons/PVRClient.h
@@ -129,6 +129,11 @@ namespace PVR
const std::string& GetBackendVersion(void) const;
/*!
+ * @brief the ip address or alias of the pvr backend server
+ */
+ const std::string& GetBackendHostname(void) const;
+
+ /*!
* @return The connection string reported by the backend.
*/
const std::string& GetConnectionString(void) const;
@@ -625,6 +630,7 @@ namespace PVR
PVR_ADDON_CAPABILITIES m_addonCapabilities; /*!< the cached add-on capabilities */
bool m_bGotAddonCapabilities; /*!< true if the add-on capabilities have already been fetched */
PVR_SIGNAL_STATUS m_qualityInfo; /*!< stream quality information */
+ std::string m_strBackendHostname; /*!< the cached backend hostname */
/* stored strings to make sure const char* members in PVR_PROPERTIES stay valid */
std::string m_strUserPath; /*!< @brief translated path to the user profile */
View
31 xbmc/pvr/addons/PVRClients.cpp
@@ -34,6 +34,7 @@
#include "pvr/recordings/PVRRecordings.h"
#include "pvr/timers/PVRTimers.h"
#include "cores/IPlayer.h"
+#include "network/Network.h"
using namespace ADDON;
using namespace PVR;
@@ -1371,3 +1372,33 @@ time_t CPVRClients::GetBufferTimeEnd() const
return time;
}
+
+bool CPVRClients::NextEventWithinBackendIdleTime(const CPVRTimers& timers)
+{
+ // timers going off soon?
+ const CDateTime now(CDateTime::GetUTCDateTime());
+ const CDateTimeSpan idle(
+ 0, 0, CSettings::Get().GetInt("pvrpowermanagement.backendidletime"), 0);
+ const CDateTime next(timers.GetNextEventTime());
+ const CDateTimeSpan delta(next - now);
+
+ return (delta <= idle);
+}
+
+bool CPVRClients::AllLocalBackendsIdle() const
+{
+ PVR_CLIENTMAP clients;
+ GetConnectedClients(clients);
+ for (PVR_CLIENTMAP_CITR itr = clients.begin(); itr != clients.end(); itr++)
@xhaggi Collaborator
xhaggi added a note

please use it instead of itr and ++it.

@ksooo Collaborator
ksooo added a note

@xhaggi : LOL. Actually I copied the code for the loop. It is done like this all over the place in CPVRClients. But yes , will change it, hoping that @opdenkamp has no problem with this.

@xhaggi Collaborator
xhaggi added a note

you can leave it for now. i will do a refactor later.

@ksooo Collaborator
ksooo added a note

@xhaggi: Okay. Thanks.

@opdenkamp Collaborator

yeah when i originally wrote that code, i assumed that the compiler would optimise that stuff out and never paid much attention to it. but it only seems to do that for primitives :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ {
+ CPVRTimers timers;
+ PVR_ERROR ret = itr->second->GetTimers(&timers);
+ if (ret == PVR_ERROR_NOT_IMPLEMENTED || ret != PVR_ERROR_NO_ERROR)
+ continue;
+
+ if (((timers.AmountActiveRecordings() > 0) || NextEventWithinBackendIdleTime(timers))
+ && g_application.getNetwork().IsLocalHost(itr->second->GetBackendHostname()))
+ return false;
+ }
+ return true;
+}
View
8 xbmc/pvr/addons/PVRClients.h
@@ -552,6 +552,12 @@ namespace PVR
bool GetPlayingClient(PVR_CLIENT &client) const;
+ /*!
+ * @brief Checks whether all local pvr backends (if any) are idle (no recording active, ...).
+ * @return True if all local backends are idle or no local backends are connected, false otherwise.
+ */
+ bool AllLocalBackendsIdle() const;
+
time_t GetPlayingTime() const;
time_t GetBufferTimeStart() const;
time_t GetBufferTimeEnd() const;
@@ -621,6 +627,8 @@ namespace PVR
int GetClientId(const ADDON::AddonPtr client) const;
+ static bool NextEventWithinBackendIdleTime(const CPVRTimers& timers);
+
bool m_bChannelScanRunning; /*!< true when a channel scan is currently running, false otherwise */
bool m_bIsSwitchingChannels; /*!< true while switching channels */
int m_playingClientId; /*!< the ID of the client that is currently playing */
View
4 xbmc/windows/GUIWindowLoginScreen.cpp
@@ -47,6 +47,7 @@
#include "guilib/LocalizeStrings.h"
#include "addons/AddonManager.h"
#include "view/ViewState.h"
+#include "pvr/PVRManager.h"
#define CONTROL_BIG_LIST 52
#define CONTROL_LABEL_HEADER 2
@@ -141,7 +142,8 @@ bool CGUIWindowLoginScreen::OnAction(const CAction &action)
{
std::string actionName = action.GetName();
StringUtils::ToLower(actionName);
- if (actionName.find("shutdown") != std::string::npos)
+ if ((actionName.find("shutdown") != std::string::npos) &&
+ PVR::g_PVRManager.CanSystemPowerdown())
CBuiltins::Execute(action.GetName());
return true;
}
Something went wrong with that request. Please try again.