D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes added a comment.


  In D16425#488963 , @davidedmundson 
wrote:
  
  > I'd quite like to get this in as I'll end up moving part of this - and 
we've got too much bikeshedding here.
  >
  > > i18n("While asleep, hibernate after 3 hours")
  >
  > If you don't know the timeout, don't write it. It's better to be vague than 
lie.
  >
  > "Suspend, then hibernate after a period of inactivitiy"
  
  
  I have updated the message. I cannot land it so I need help with that

REPOSITORY
  R122 Powerdevil

BRANCH
  arcpatch-D16425_1

REVISION DETAIL
  https://phabricator.kde.org/D16425

To: avaldes, broulik, ngraham, meven, davidedmundson, sitter
Cc: sitter, davidedmundson, ericadams, jobauer, reverendhomer, meven, soriano, 
abalaji, graesslin, ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes updated this revision to Diff 60926.
avaldes added a comment.


  Summary:
  See bug 399727  for a good 
description of what this code is for.
  
  The new ui will show a new option like the following image F6349860: 
screenshot.png 
  
  Reviewers: broulik, ngraham
  
  Subscribers: soriano, abalaji, graesslin, ngraham, plasma-devel
  
  Tags: #plasma 
  
  Differential Revision: https://phabricator.kde.org/D16425

REPOSITORY
  R122 Powerdevil

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16425?vs=59241=60926

BRANCH
  arcpatch-D16425_1

REVISION DETAIL
  https://phabricator.kde.org/D16425

AFFECTED FILES
  daemon/actions/bundled/suspendsession.cpp
  daemon/actions/bundled/suspendsession.h
  daemon/actions/bundled/suspendsessionconfig.cpp
  daemon/actions/bundled/suspendsessionconfig.h
  daemon/backends/upower/login1suspendjob.cpp
  daemon/backends/upower/powerdevilupowerbackend.cpp
  daemon/backends/upower/upowersuspendjob.cpp
  daemon/org.freedesktop.PowerManagement.xml
  daemon/powerdevilbackendinterface.h
  daemon/powerdevilfdoconnector.cpp
  daemon/powerdevilfdoconnector.h
  daemon/powerdevilpowermanagement.cpp
  daemon/powerdevilpowermanagement.h

To: avaldes, broulik, ngraham, meven, davidedmundson, sitter
Cc: sitter, davidedmundson, ericadams, jobauer, reverendhomer, meven, soriano, 
abalaji, graesslin, ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes marked 3 inline comments as done.
avaldes added a comment.


  In D16425#475148 , @ericadams 
wrote:
  
  > I apologize if this is the wrong place to add this comment but I have a 
laptop where this applies and would benefit me. I am happy to help test this if 
you find it helpful. Thanks.
  
  
  Feel free to download this patch (download raw diff 
) and apply it to powerdevil 
repo . You need to build the 
package and replace the one in your system.
  
  If you use arch or a derivative I can help you with that.

REPOSITORY
  R122 Powerdevil

BRANCH
  arcpatch-D16425_1

REVISION DETAIL
  https://phabricator.kde.org/D16425

To: avaldes, broulik, ngraham
Cc: ericadams, jobauer, reverendhomer, meven, soriano, abalaji, graesslin, 
ngraham, plasma-devel, LeGast00n, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes updated this revision to Diff 59241.
avaldes added a comment.


  Summary:
  See bug 399727  for a good 
description of what this code is for.
  
  The new ui will show a new option like the following image F6349860: 
screenshot.png 
  
  Reviewers: broulik, ngraham
  
  Subscribers: soriano, abalaji, graesslin, ngraham, plasma-devel
  
  Tags: #plasma 
  
  Differential Revision: https://phabricator.kde.org/D16425

REPOSITORY
  R122 Powerdevil

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16425?vs=59170=59241

BRANCH
  arcpatch-D16425_1

REVISION DETAIL
  https://phabricator.kde.org/D16425

AFFECTED FILES
  daemon/actions/bundled/suspendsession.cpp
  daemon/actions/bundled/suspendsession.h
  daemon/actions/bundled/suspendsessionconfig.cpp
  daemon/actions/bundled/suspendsessionconfig.h
  daemon/backends/upower/login1suspendjob.cpp
  daemon/backends/upower/powerdevilupowerbackend.cpp
  daemon/backends/upower/upowersuspendjob.cpp
  daemon/org.freedesktop.PowerManagement.xml
  daemon/powerdevilbackendinterface.h
  daemon/powerdevilfdoconnector.cpp
  daemon/powerdevilfdoconnector.h
  daemon/powerdevilpowermanagement.cpp
  daemon/powerdevilpowermanagement.h

To: avaldes, broulik, ngraham
Cc: jobauer, reverendhomer, meven, soriano, abalaji, graesslin, ngraham, 
plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes added inline comments.

INLINE COMMENTS

> abalaji wrote in suspendsessionconfig.cpp:45
> Move this to the initializer

something like this?

  : ActionConfig(parent),
m_suspendThenHibernateEnabled(nullptr)

REPOSITORY
  R122 Powerdevil

BRANCH
  arcpatch-D16425_1

REVISION DETAIL
  https://phabricator.kde.org/D16425

To: avaldes, broulik, ngraham
Cc: jobauer, reverendhomer, meven, soriano, abalaji, graesslin, ngraham, 
plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes marked 5 inline comments as done.
avaldes added a comment.


  Sorry for the constant spam of the original commit message, I cannot make 
arcanist to pick up a new commit for this differential without deleting the old 
one.
  
  Added changes based on code review comments

REPOSITORY
  R122 Powerdevil

BRANCH
  arcpatch-D16425_1

REVISION DETAIL
  https://phabricator.kde.org/D16425

To: avaldes, broulik, ngraham
Cc: jobauer, reverendhomer, meven, soriano, abalaji, graesslin, ngraham, 
plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes updated this revision to Diff 59170.
avaldes marked 2 inline comments as done.
avaldes added a comment.


  Added new Suspend then Hibernate option
  
  Summary:
  See bug 399727  for a good 
description of what this code is for.
  
  The new ui will show a new option like the following image F6349860: 
screenshot.png 
  
  Reviewers: broulik, ngraham
  
  Subscribers: soriano, abalaji, graesslin, ngraham, plasma-devel
  
  Tags: #plasma 
  
  Differential Revision: https://phabricator.kde.org/D16425

REPOSITORY
  R122 Powerdevil

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16425?vs=58969=59170

BRANCH
  arcpatch-D16425_1

REVISION DETAIL
  https://phabricator.kde.org/D16425

AFFECTED FILES
  daemon/actions/bundled/suspendsession.cpp
  daemon/actions/bundled/suspendsession.h
  daemon/actions/bundled/suspendsessionconfig.cpp
  daemon/actions/bundled/suspendsessionconfig.h
  daemon/backends/upower/login1suspendjob.cpp
  daemon/backends/upower/powerdevilupowerbackend.cpp
  daemon/backends/upower/upowersuspendjob.cpp
  daemon/org.freedesktop.PowerManagement.xml
  daemon/powerdevilbackendinterface.h
  daemon/powerdevilfdoconnector.cpp
  daemon/powerdevilfdoconnector.h
  daemon/powerdevilpowermanagement.cpp
  daemon/powerdevilpowermanagement.h

To: avaldes, broulik, ngraham
Cc: jobauer, reverendhomer, meven, soriano, abalaji, graesslin, ngraham, 
plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes marked 5 inline comments as done.
avaldes added a comment.


  Added changes based on recent comments.

INLINE COMMENTS

> abalaji wrote in suspendsessionconfig.cpp:106
> Stray line swap

Done on purpose to change the UI:

F6822188: suspendThenHibernate.png 

Automatically, m_comboBox, m_idleTime

> abalaji wrote in suspendsessionconfig.cpp:121
> Not sure if this is necessary, just `delete m_suspendThenHibernateEnabled` 
> might be enough.

yeah it's necessary, without it powerdevil crashes at launch

REPOSITORY
  R122 Powerdevil

BRANCH
  arcpatch-D16425_1

REVISION DETAIL
  https://phabricator.kde.org/D16425

To: avaldes, broulik, ngraham
Cc: jobauer, reverendhomer, meven, soriano, abalaji, graesslin, ngraham, 
plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes updated this revision to Diff 58969.
avaldes added a comment.


  Added new Suspend then Hibernate option
  
  Summary:
  See bug 399727  for a good 
description of what this code is for.
  
  The new ui will show a new option like the following image F6822188: 
suspendThenHibernate.png 
  
  FEATURE: 399727
  
  Reviewers: broulik, ngraham
  
  Subscribers: soriano, abalaji, graesslin, ngraham, plasma-devel
  
  Tags: #plasma 
  
  Differential Revision: https://phabricator.kde.org/D16425

REPOSITORY
  R122 Powerdevil

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16425?vs=58273=58969

BRANCH
  arcpatch-D16425_1

REVISION DETAIL
  https://phabricator.kde.org/D16425

AFFECTED FILES
  daemon/actions/bundled/suspendsession.cpp
  daemon/actions/bundled/suspendsession.h
  daemon/actions/bundled/suspendsessionconfig.cpp
  daemon/actions/bundled/suspendsessionconfig.h
  daemon/backends/upower/login1suspendjob.cpp
  daemon/backends/upower/powerdevilupowerbackend.cpp
  daemon/backends/upower/upowersuspendjob.cpp
  daemon/org.freedesktop.PowerManagement.xml
  daemon/powerdevilbackendinterface.h
  daemon/powerdevilfdoconnector.cpp
  daemon/powerdevilfdoconnector.h
  daemon/powerdevilpowermanagement.cpp
  daemon/powerdevilpowermanagement.h

To: avaldes, broulik, ngraham
Cc: jobauer, reverendhomer, meven, soriano, abalaji, graesslin, ngraham, 
plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes marked 3 inline comments as done.
avaldes added a comment.


  Updated with latest comments.
  
  Please land, I don't have permissions.

REPOSITORY
  R122 Powerdevil

BRANCH
  arcpatch-D16425_1

REVISION DETAIL
  https://phabricator.kde.org/D16425

To: avaldes, broulik, ngraham
Cc: reverendhomer, meven, soriano, abalaji, graesslin, ngraham, plasma-devel, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes updated this revision to Diff 58273.
avaldes added a comment.


  Added new Suspend then Hibernate option
  

  
  Summary:
  See bug 399727  for a good 
description of what this code is for.
  
  The new ui will show a new option like the following image F6349860: 
screenshot.png 
  

  
  Reviewers: broulik, ngraham
  
   
  
  Subscribers: soriano, abalaji, graesslin, ngraham, plasma-devel
  

  
  Tags: #plasma 
  

  
  Differential Revision: https://phabricator.kde.org/D16425

REPOSITORY
  R122 Powerdevil

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16425?vs=58091=58273

BRANCH
  arcpatch-D16425_1

REVISION DETAIL
  https://phabricator.kde.org/D16425

AFFECTED FILES
  daemon/actions/bundled/suspendsession.cpp
  daemon/actions/bundled/suspendsession.h
  daemon/actions/bundled/suspendsessionconfig.cpp
  daemon/actions/bundled/suspendsessionconfig.h
  daemon/backends/upower/login1suspendjob.cpp
  daemon/backends/upower/powerdevilupowerbackend.cpp
  daemon/backends/upower/upowersuspendjob.cpp
  daemon/org.freedesktop.PowerManagement.xml
  daemon/powerdevilbackendinterface.h
  daemon/powerdevilfdoconnector.cpp
  daemon/powerdevilfdoconnector.h
  daemon/powerdevilpowermanagement.cpp
  daemon/powerdevilpowermanagement.h

To: avaldes, broulik, ngraham
Cc: reverendhomer, meven, soriano, abalaji, graesslin, ngraham, plasma-devel, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes added a comment.


  > This patch doesn't seem to work for me. I have Arch Linux, 
powerdevil-5.15.5-1 and your patch applied. After clicking the check-box the 
"Apply" button doesn't get active. Therefore, this setting is not being saved.
  
  Did you enable the Suspend Session option too? you need to enable the Suspend 
Session before using the new checkbox, it is the same behavior of "Even when an 
external monitor is connected" checkbox. Is a limitation on how powerdevil UI 
is built.
  
  > Also, why does the checkbox label say "While asleep, hibernate after 3 
hours"? This value can be overriden in /etc/systemd/sleep.conf
  
  It is the default timeout, there's no easy way to change it using a UI or a 
parameter, unless we want to point the users to modify a root-owned file.

REPOSITORY
  R122 Powerdevil

REVISION DETAIL
  https://phabricator.kde.org/D16425

To: avaldes, broulik, ngraham
Cc: reverendhomer, meven, soriano, abalaji, graesslin, ngraham, plasma-devel, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes added a comment.


  In D16425#465321 , @ngraham wrote:
  
  > UI looks good enough for now. But is this the full diff? It seems like 
something got lost. The whole patch should include the changes from all commits 
in your branch, not just the last one.
  
  
  should be fixed now, I'm not sure what happened with arcanist.

REPOSITORY
  R122 Powerdevil

REVISION DETAIL
  https://phabricator.kde.org/D16425

To: avaldes, broulik, ngraham
Cc: meven, soriano, abalaji, graesslin, ngraham, plasma-devel, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes updated this revision to Diff 58091.
avaldes added a comment.


  Added new Suspend then Hibernate option
  

  
  Summary:
  See bug 399727  for a good 
description of what this code is for.
  The new ui will show a new option like the following image F6349860: 
screenshot.png 
  FEATURE: 399727
  
  Reviewers: broulik, ngraham
  
   
  
  Subscribers: soriano, abalaji, graesslin, ngraham, plasma-devel
  

  
  Tags: #plasma 
  

  
  Differential Revision: https://phabricator.kde.org/D16425

REPOSITORY
  R122 Powerdevil

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16425?vs=58044=58091

BRANCH
  arcpatch-D16425_1

REVISION DETAIL
  https://phabricator.kde.org/D16425

AFFECTED FILES
  daemon/actions/bundled/suspendsession.cpp
  daemon/actions/bundled/suspendsession.h
  daemon/actions/bundled/suspendsessionconfig.cpp
  daemon/actions/bundled/suspendsessionconfig.h
  daemon/backends/upower/login1suspendjob.cpp
  daemon/backends/upower/powerdevilupowerbackend.cpp
  daemon/backends/upower/upowersuspendjob.cpp
  daemon/org.freedesktop.PowerManagement.xml
  daemon/powerdevilbackendinterface.h
  daemon/powerdevilfdoconnector.cpp
  daemon/powerdevilfdoconnector.h
  daemon/powerdevilpowermanagement.cpp
  daemon/powerdevilpowermanagement.h

To: avaldes, broulik, ngraham
Cc: meven, soriano, abalaji, graesslin, ngraham, plasma-devel, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes added a comment.


  This is how it looks now @ngraham :
  
  F6822188: suspendThenHibernate.png 

REPOSITORY
  R122 Powerdevil

REVISION DETAIL
  https://phabricator.kde.org/D16425

To: avaldes, broulik, ngraham
Cc: meven, soriano, abalaji, graesslin, ngraham, plasma-devel, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes updated this revision to Diff 58044.
avaldes added a comment.


  Improved messages for suspend session

REPOSITORY
  R122 Powerdevil

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16425?vs=56024=58044

BRANCH
  arcpatch-D16425_1

REVISION DETAIL
  https://phabricator.kde.org/D16425

AFFECTED FILES
  daemon/actions/bundled/suspendsessionconfig.cpp

To: avaldes, broulik, ngraham
Cc: meven, soriano, abalaji, graesslin, ngraham, plasma-devel, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes added a comment.


  In D16425#464381 , @ngraham wrote:
  
  > This thing's UI really needs to be rewritten in QML. Once we do that and 
give it a proper FormLayout style, the string can be shorter, but for now, 
seeing it in context, I feel like  the string should be "Hibernate after 3 
hours of sleep".
  >
  > Also I wouldn't mind making the period configurable. If you don't want to 
do that in this patch though, that's fine.
  
  
  The period configuration can only be done by adding a .conf file to any of 
these locations with the parameter HibernateDelaySec, current dbus API doesn't 
have anything to pass or modify this value, so user intervention is required to 
change the default timeout of 3 hours.
  
/etc/systemd/sleep.conf
/etc/systemd/sleep.conf.d/*.conf
/run/systemd/sleep.conf.d/*.conf
/usr/lib/systemd/sleep.conf.d/*.conf
  
  @ngraham there is currently a limitation with my current patch, as a user you 
will always have to enable the suspend after a period of time (Suspend Session 
> After) to enable the Suspend-then-hibernate option. I think this is not a 
good default, I can also move this option to a new group similar to "Buttons 
event handling", which also modifies the suspend behaviour.

REPOSITORY
  R122 Powerdevil

REVISION DETAIL
  https://phabricator.kde.org/D16425

To: avaldes, broulik, ngraham
Cc: meven, soriano, abalaji, graesslin, ngraham, plasma-devel, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D21111: Added new option to use GlobalProtect with openconnect

[Alejandro Valdes]

avaldes added a comment.


  I can't land it, please merge @jgrulich

REPOSITORY
  R116 Plasma Network Management Applet

BRANCH
  globalprotect (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D2

To: avaldes, jgrulich
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D21111: Added new option to use GlobalProtect with openconnect

[Alejandro Valdes]

avaldes added a comment.


  In D2#463270 , @jgrulich wrote:
  
  > In D2#463216 , @avaldes 
wrote:
  >
  > > networkmanager-openconnect has added support in master for global protect 
,
 so now we can use it from plasma.
  > >
  > > This was tested in my machine with Plasma 5.15.5 and the lastest 
networkmanager-openconnect.
  > >
  > > The VPN works but I'm having an issue with  the existing configuration 
options:
  > >
  > > Unselecting the option "Use only for resources on this connection" in 
IPv4 > Routes doesn't work, I can access the network resources only available 
at the VPN but all my internet connections goes to the default internet route. 
I was able to solve this temporally by modifying the kernel routes. Is this 
intended?
  >
  >
  > Do you mean this change didn't apply and when you opened the configuration 
again was still selected? If the change did apply and after you open the 
configuration again it was set as it was supposed to be, then it's most likely 
a NM issue.
  >
  > > I also tried changing the priority of the VPN to 0 and the default 
network connection to 2 but it didn't work.
  >
  > It's autoconnect priority, this means that a connection with a higher 
priority will be automatically activated after restart instead of any other 
connection with a lower priority if available.
  
  
  No, my question is about VPN connectivity with NM, the connection is active 
but not all traffic goes through the VPN, I would expect that if that option in 
Routes (use only for resources of this network) should make the VPN as the main 
connection, but it doesn't.

REPOSITORY
  R116 Plasma Network Management Applet

REVISION DETAIL
  https://phabricator.kde.org/D2

To: avaldes, jgrulich
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D21111: Added new option to use GlobalProtect with openconnect

[Alejandro Valdes]

avaldes updated this revision to Diff 57861.
avaldes added a comment.


  - Changes based on code review

REPOSITORY
  R116 Plasma Network Management Applet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D2?vs=57837=57861

BRANCH
  globalprotect (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D2

AFFECTED FILES
  vpn/openconnect/CMakeLists.txt
  vpn/openconnect/openconnectprop.ui
  vpn/openconnect/openconnectwidget.cpp
  vpn/openconnect/plasmanetworkmanagement_openconnect_globalprotectui.desktop

To: avaldes, jgrulich
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D21111: Added new option to use GlobalProtect with openconnect

[Alejandro Valdes]

avaldes added a comment.


  networkmanager-openconnect has added support in master for global protect 
,
 so now we can use it from plasma.
  
  This was tested in my machine with Plasma 5.15.5 and the lastest 
networkmanager-openconnect.
  
  The VPN works but I'm having an issue with  the existing configuration 
options:
  
  Unselecting the option "Use only for resources on this connection" in IPv4 > 
Routes doesn't work, I can access the network resources only available at the 
VPN but all my internet connections goes to the default internet route. I was 
able to solve this temporally by modifying the kernel routes. Is this intended?
  
  I also tried changing the priority of the VPN to 0 and the default network 
connection to 2 but it didn't work.

REPOSITORY
  R116 Plasma Network Management Applet

REVISION DETAIL
  https://phabricator.kde.org/D2

To: avaldes, jgrulich
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D21111: Added new option to use GlobalProtect with openconnect

[Alejandro Valdes]

avaldes created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
Herald added a reviewer: jgrulich.
avaldes requested review of this revision.

REVISION SUMMARY
  This adds a new option to the dropdown and a new desktop file to create a 
connection to a Palo Alto / Global Protect network
  
  FEATURE: 405389

REPOSITORY
  R116 Plasma Network Management Applet

BRANCH
  globalprotect (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D2

AFFECTED FILES
  vpn/openconnect/CMakeLists.txt
  vpn/openconnect/openconnectprop.ui
  vpn/openconnect/openconnectwidget.cpp
  vpn/openconnect/plasmanetworkmanagement_openconnect_globalprotect.desktop

To: avaldes, jgrulich
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes added a comment.


  In D16425#449026 , @meven wrote:
  
  > In D16425#448999 , @avaldes 
wrote:
  >
  > > I applied this patch to v5.15.4 tag and tested with that and it works (my 
machine is running plasma 5.15.4), I can change the brightness with the 
keyboard and set the suspend-then-hibernate option.
  > >
  > > F6769609: image.png 
  >
  >
  > Very nice.
  >  The position of the checkbox in the settings seems odd to me.
  >  It would be great to align it below the "After" field.
  >  Or perhaps the screenshot just does not justice to the UI.
  
  
  This is how the module looks with the new option:
  F6770639: image.png 
  It looks the same as the "Even when an external monitor is connected", I 
tried using a HBox but it didn't change the alignment, maybe there is a 
restriction on how powerdevil builds the UI, but I'm no expert on that. if 
anyone has any idea on how to change it I'm open to suggestions.

REPOSITORY
  R122 Powerdevil

REVISION DETAIL
  https://phabricator.kde.org/D16425

To: avaldes, broulik, ngraham
Cc: meven, soriano, abalaji, graesslin, ngraham, plasma-devel, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes added a comment.


  In D16425#448828 , @ngraham wrote:
  
  > In D16425#448822 , @avaldes 
wrote:
  >
  > > In D16425#448820 , @ngraham 
wrote:
  > >
  > > > So is everything now working for you, or not?
  > >
  > >
  > > No, I can't use my keyboard to change brightness, but it might be related 
on how I test my changes. I need guidance on how can I test powerdevil changes 
locally.
  >
  >
  > It's a plasma component, so this might be helpful: 
https://community.kde.org/Guidelines_and_HOWTOs/Build_from_source/Test_plasma
  
  
  I applied this patch to v5.15.4 tag and tested with that and it works (my 
machine is running plasma 5.15.4), I can change the brightness with the 
keyboard and set the suspend-then-hibernate option.
  
  F6769609: image.png 

REPOSITORY
  R122 Powerdevil

REVISION DETAIL
  https://phabricator.kde.org/D16425

To: avaldes, broulik, ngraham
Cc: soriano, abalaji, graesslin, ngraham, plasma-devel, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes added a comment.


  In D16425#448820 , @ngraham wrote:
  
  > So is everything now working for you, or not?
  
  
  No, I can't use my keyboard to change brightness, but it might be related on 
how I test my changes. I need guidance on how can I test powerdevil changes 
locally.

REPOSITORY
  R122 Powerdevil

REVISION DETAIL
  https://phabricator.kde.org/D16425

To: avaldes, broulik, ngraham
Cc: soriano, abalaji, graesslin, ngraham, plasma-devel, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes updated this revision to Diff 56024.
avaldes added a comment.


  Removed CMakeList change

REPOSITORY
  R122 Powerdevil

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16425?vs=56023=56024

BRANCH
  suspend-then-hibernate (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D16425

AFFECTED FILES
  daemon/actions/bundled/suspendsession.cpp
  daemon/actions/bundled/suspendsession.h
  daemon/actions/bundled/suspendsessionconfig.cpp
  daemon/actions/bundled/suspendsessionconfig.h
  daemon/backends/upower/login1suspendjob.cpp
  daemon/backends/upower/powerdevilupowerbackend.cpp
  daemon/backends/upower/upowersuspendjob.cpp
  daemon/org.freedesktop.PowerManagement.xml
  daemon/powerdevilbackendinterface.h
  daemon/powerdevilfdoconnector.cpp
  daemon/powerdevilfdoconnector.h
  daemon/powerdevilpowermanagement.cpp
  daemon/powerdevilpowermanagement.h

To: avaldes, broulik, ngraham
Cc: soriano, abalaji, graesslin, ngraham, plasma-devel, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes added a comment.


  @ngraham I updated the patch with the comments, I'm not sure if I'm updating 
it corrrectly, I have tested it on my machine and it can suspend, but for some 
reason the dedicated keys to change the brightness are not working on my 
laptop. I'm still checking if this is related on how I build and test locally 
powerdevil or is related with my changes.

REPOSITORY
  R122 Powerdevil

REVISION DETAIL
  https://phabricator.kde.org/D16425

To: avaldes, broulik, ngraham
Cc: soriano, abalaji, graesslin, ngraham, plasma-devel, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes updated this revision to Diff 56023.
avaldes edited the summary of this revision.
avaldes added a comment.


  Rebasing changes to master, adding latest comments

REPOSITORY
  R122 Powerdevil

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16425?vs=44950=56023

BRANCH
  suspend-then-hibernate (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D16425

AFFECTED FILES
  CMakeLists.txt
  daemon/actions/bundled/suspendsession.cpp
  daemon/actions/bundled/suspendsession.h
  daemon/actions/bundled/suspendsessionconfig.cpp
  daemon/actions/bundled/suspendsessionconfig.h
  daemon/backends/upower/login1suspendjob.cpp
  daemon/backends/upower/powerdevilupowerbackend.cpp
  daemon/backends/upower/upowersuspendjob.cpp
  daemon/org.freedesktop.PowerManagement.xml
  daemon/powerdevilbackendinterface.h
  daemon/powerdevilfdoconnector.cpp
  daemon/powerdevilfdoconnector.h
  daemon/powerdevilpowermanagement.cpp
  daemon/powerdevilpowermanagement.h

To: avaldes, broulik, ngraham
Cc: soriano, abalaji, graesslin, ngraham, plasma-devel, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes updated this revision to Diff 44950.
avaldes added a comment.


  I have changed the message.
  
  Also, if anyone can help me, I'm having a segmentation fault with these 
changes when an application inhibits power saving. This does happen with or 
without having available hibernation in the machine. Is there any manual on how 
can I debug powerdevil? I wasn't able to find much when searching for it.
  
Application: org_kde_powerdevil (org_kde_powerdevil), signal: Segmentation 
fault
Using host libthread_db library "/usr/lib/libthread_db.so.1".
[Current thread is 1 (Thread 0x7fa4c8735840 (LWP 12369))]

Thread 5 (Thread 0x7fa4c4e18700 (LWP 12396)):
#0  0x7fa4ce93d7a4 in read () at /usr/lib/libc.so.6
#1  0x7fa4cced1781 in  () at /usr/lib/libglib-2.0.so.0
#2  0x7fa4ccf21a50 in g_main_context_check () at 
/usr/lib/libglib-2.0.so.0
#3  0x7fa4ccf22e86 in  () at /usr/lib/libglib-2.0.so.0
#4  0x7fa4ccf23f62 in g_main_loop_run () at /usr/lib/libglib-2.0.so.0
#5  0x7fa4c5d46c28 in  () at /usr/lib/libgio-2.0.so.0
#6  0x7fa4cceec3eb in  () at /usr/lib/libglib-2.0.so.0
#7  0x7fa4cdb66a9d in start_thread () at /usr/lib/libpthread.so.0
#8  0x7fa4ce94cb23 in clone () at /usr/lib/libc.so.6

Thread 4 (Thread 0x7fa4c5619700 (LWP 12393)):
#0  0x7fa4ce93d7a4 in read () at /usr/lib/libc.so.6
#1  0x7fa4cced1781 in  () at /usr/lib/libglib-2.0.so.0
#2  0x7fa4ccf21a50 in g_main_context_check () at 
/usr/lib/libglib-2.0.so.0
#3  0x7fa4ccf22e86 in  () at /usr/lib/libglib-2.0.so.0
#4  0x7fa4ccf22fce in g_main_context_iteration () at 
/usr/lib/libglib-2.0.so.0
#5  0x7fa4ccf23022 in  () at /usr/lib/libglib-2.0.so.0
#6  0x7fa4cceec3eb in  () at /usr/lib/libglib-2.0.so.0
#7  0x7fa4cdb66a9d in start_thread () at /usr/lib/libpthread.so.0
#8  0x7fa4ce94cb23 in clone () at /usr/lib/libc.so.6

Thread 3 (Thread 0x7fa4c675e700 (LWP 12388)):
#0  0x7fa4ce93d7a4 in read () at /usr/lib/libc.so.6
#1  0x7fa4cced1781 in  () at /usr/lib/libglib-2.0.so.0
#2  0x7fa4ccf21a50 in g_main_context_check () at 
/usr/lib/libglib-2.0.so.0
#3  0x7fa4ccf22e86 in  () at /usr/lib/libglib-2.0.so.0
#4  0x7fa4ccf22fce in g_main_context_iteration () at 
/usr/lib/libglib-2.0.so.0
#5  0x7fa4cee68fe4 in 
QEventDispatcherGlib::processEvents(QFlags) () 
at /usr/lib/libQt5Core.so.5
#6  0x7fa4cee148cc in 
QEventLoop::exec(QFlags) () at 
/usr/lib/libQt5Core.so.5
#7  0x7fa4cec5deb9 in QThread::exec() () at /usr/lib/libQt5Core.so.5
#8  0x7fa4cf0baba6 in  () at /usr/lib/libQt5DBus.so.5
#9  0x7fa4cec67f65 in  () at /usr/lib/libQt5Core.so.5
#10 0x7fa4cdb66a9d in start_thread () at /usr/lib/libpthread.so.0
#11 0x7fa4ce94cb23 in clone () at /usr/lib/libc.so.6

Thread 2 (Thread 0x7fa4c737d700 (LWP 12378)):
#0  0x7fa4ce941c21 in poll () at /usr/lib/libc.so.6
#1  0x7fa4ce053630 in  () at /usr/lib/libxcb.so.1
#2  0x7fa4ce0552db in xcb_wait_for_event () at /usr/lib/libxcb.so.1
#3  0x7fa4c82e3c5a in  () at /usr/lib/libQt5XcbQpa.so.5
#4  0x7fa4cec67f65 in  () at /usr/lib/libQt5Core.so.5
#5  0x7fa4cdb66a9d in start_thread () at /usr/lib/libpthread.so.0
#6  0x7fa4ce94cb23 in clone () at /usr/lib/libc.so.6

Thread 1 (Thread 0x7fa4c8735840 (LWP 12369)):
[KCrash Handler]
#6  0x7fa4cecb2523 in QHashData::nextNode(QHashData::Node*) () at 
/usr/lib/libQt5Core.so.5
#7  0x7fa4cfe0f8e5 in 
PowerDevil::PolicyAgent::onServiceUnregistered(QString const&) () at 
/usr/lib/libpowerdevilcore.so.2
#8  0x7fa4cfe3ac8a in  () at /usr/lib/libpowerdevilcore.so.2
#9  0x7fa4cee3fa7c in QMetaObject::activate(QObject*, int, int, void**) 
() at /usr/lib/libQt5Core.so.5
#10 0x7fa4cf118626 in QDBusServiceWatcher::serviceUnregistered(QString 
const&) () at /usr/lib/libQt5DBus.so.5
#11 0x7fa4cf118e8b in  () at /usr/lib/libQt5DBus.so.5
#12 0x7fa4cf119293 in 
QDBusServiceWatcher::qt_metacall(QMetaObject::Call, int, void**) () at 
/usr/lib/libQt5DBus.so.5
#13 0x7fa4cf0c69ef in  () at /usr/lib/libQt5DBus.so.5
#14 0x7fa4cee40352 in QObject::event(QEvent*) () at 
/usr/lib/libQt5Core.so.5
#15 0x7fa4cee15c39 in QCoreApplication::notifyInternal2(QObject*, 
QEvent*) () at /usr/lib/libQt5Core.so.5
#16 0x7fa4cee18ccc in 
QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () at 
/usr/lib/libQt5Core.so.5
#17 0x7fa4cee699d4 in  () at /usr/lib/libQt5Core.so.5
#18 0x7fa4ccf213cf in g_main_context_dispatch () at 
/usr/lib/libglib-2.0.so.0
#19 0x7fa4ccf22f89 in  () at /usr/lib/libglib-2.0.so.0
#20 0x7fa4ccf22fce in g_main_context_iteration () at 
/usr/lib/libglib-2.0.so.0
#21 0x7fa4cee68fc9 in 
QEventDispatcherGlib::processEvents(QFlags) () 
at 

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes added a comment.


  In D16425#349588 , @ngraham wrote:
  
  > We'll need a spinbox to display time options for the amount of delay before 
hibernating if this is the UI we go with. But I kinda like Martin's idea and 
present this in the form of an additional control sort of like this:
  >
  >   While suspended, hibernate after: [combobox with some carefully selected 
intervals, plus "Never", which is the default choice]
  >
  >
  > Also, how does this handle hardware or distro configurations that don't 
support hibernation? I might suggest that it would be best to hide the option 
entirely in such a case. On this subject, the HIG says:
  >
  > > If some of the program’s settings are only applicable in certain 
contexts, do not hide the inapplicable ones. Instead, disable them and hint to 
the user why they’re disabled. **Exception:** it is acceptable to hide settings 
for non-existent hardware. For example, it’s okay to hide the touchpad 
configuration when no touchpad is present.
  >
  > It's not //exctly// the same thing, but I think the principle applies 
here.
  
  
  If the hardware you are running doesn't support hibernation, logind 
canSupportThenHibernate method should return no, I tested this code in a 
machine without hibernation and it currently hides the new option. I added some 
checks before saving so we don't accidentally enable this suspend option by 
error.
  
  On the amount of time to hibernate, afaik this can only be configured by 
modifying /etc/systemd/sleep.conf and adding HibernateDelaySec= with the amount 
of seconds that you want to wait to hibernate, by default the wait time is 3 
hours. I have no knowledge of an API to configure this time.

REPOSITORY
  R122 Powerdevil

REVISION DETAIL
  https://phabricator.kde.org/D16425

To: avaldes, broulik
Cc: graesslin, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes updated this revision to Diff 44294.
avaldes added a comment.


  I have added a new checkbox inside the suspend session delay and in button 
event handling.
  
  I wasn't able to make it work with the button event handling, but the 
checkbox does work with the logic when Suspend Session is enabled.
  
  Let me know if you know of a better way to do this.
  
  Here's how it looks now: F6364813: sth-checkbox.png 


REPOSITORY
  R122 Powerdevil

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16425?vs=44208=44294

REVISION DETAIL
  https://phabricator.kde.org/D16425

AFFECTED FILES
  daemon/actions/bundled/handlebuttonevents.cpp
  daemon/actions/bundled/handlebuttonevents.h
  daemon/actions/bundled/handlebuttoneventsconfig.cpp
  daemon/actions/bundled/handlebuttoneventsconfig.h
  daemon/actions/bundled/suspendsession.cpp
  daemon/actions/bundled/suspendsession.h
  daemon/actions/bundled/suspendsessionconfig.cpp
  daemon/actions/bundled/suspendsessionconfig.h
  daemon/org.freedesktop.PowerManagement.xml
  daemon/powerdevilfdoconnector.cpp
  kcmodule/activities/activitywidget.cpp

To: avaldes, broulik
Cc: graesslin, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes added a comment.


  @ngraham afaik powerdevil asks logind for system capabilites, with the 
CanSuspend, CanHibernate, etc. defined here 
.
 I added a new option if logind reports yes for CanSuspendThenHibernate option.

REPOSITORY
  R122 Powerdevil

REVISION DETAIL
  https://phabricator.kde.org/D16425

To: avaldes, broulik
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes created this revision.
avaldes added a reviewer: broulik.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
avaldes requested review of this revision.

REVISION SUMMARY
  See bug 399727  for a good 
description of what this code is for.
  
  The new ui will show a new option like the following image F6349860: 
screenshot.png 

REPOSITORY
  R122 Powerdevil

REVISION DETAIL
  https://phabricator.kde.org/D16425

AFFECTED FILES
  daemon/actions/bundled/handlebuttoneventsconfig.cpp
  daemon/actions/bundled/suspendsession.cpp
  daemon/actions/bundled/suspendsession.h
  daemon/actions/bundled/suspendsessionconfig.cpp
  daemon/backends/upower/login1suspendjob.cpp
  daemon/backends/upower/powerdevilupowerbackend.cpp
  daemon/backends/upower/upowersuspendjob.cpp
  daemon/org.freedesktop.PowerManagement.xml
  daemon/powerdevilbackendinterface.h
  daemon/powerdevilfdoconnector.cpp
  daemon/powerdevilfdoconnector.h
  daemon/powerdevilpowermanagement.cpp
  daemon/powerdevilpowermanagement.h
  kcmodule/activities/activitywidget.cpp

To: avaldes, broulik
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart