Summary: | after resume from hibernate, critical battery action (hibernate) does not work anymore | ||
---|---|---|---|
Product: | upower | Reporter: | poinck <ich> |
Component: | general | Assignee: | Richard Hughes <richard> |
Status: | RESOLVED FIXED | QA Contact: | |
Severity: | minor | ||
Priority: | medium | CC: | bugzilla, sustmidown |
Version: | unspecified | ||
Hardware: | x86-64 (AMD64) | ||
OS: | Linux (All) | ||
Whiteboard: | |||
i915 platform: | i915 features: | ||
Attachments: |
possible workaround to restart upower after resume from hibernate
fix for the bug fix for the bug v2 |
Description
poinck
2014-12-28 13:16:20 UTC
Created attachment 113012 [details]
possible workaround to restart upower after resume from hibernate
Howto use workaround:
(as root):
cp upower-restart-after-hibernate.service /etc/systemd/system
systemctl enable upower-restart-after-hibernate.service
systemctl status upower-restart-after-hibernate.service
It works for me.
Can you get a full debug log from upowerd when reproducing the problem? Testing with 0.99.2 would be appreciated as well. Created attachment 130886 [details] [review] fix for the bug I found the problem. The take_action_timeout_cb() function returns G_SOURCE_REMOVE which makes GLib destroy the timeout. However the action_timeout_id stayed != 0 so when warning level turned to "action" again the daemon assumed that the timeout is already set and did nothing. (It only logged: "Not taking action, timeout id already set") Comment on attachment 130886 [details] [review] fix for the bug Review of attachment 130886 [details] [review]: ----------------------------------------------------------------- I was about to review the patch saying it needed a test case, but you got everything right the first time, coding style, commit message formatting, test case. Stellar work :) Just a couple of nits, and that's good to commit. Either I or one of the other developers on upower should take care of committing this shortly when the changes have been made. And feel free to look into any other problems you see in upower :) ::: src/linux/integration-test @@ +252,4 @@ > with open(self.log.name) as f: > return text in f.read() > > + def have_text_in_log_n_times(self, text, n): Could you make this return the number of times the string is mentioned instead? This would allow us to implement the above function as a special case (count == 1) and have the assertion failures mention the number of times the string is mentioned, instead of simply failing. @@ +794,5 @@ > + self.testbed.uevent(bat0, 'change') > + time.sleep(0.5) > + > + self.assertEqual(self.get_dbus_display_property('WarningLevel'), UP_DEVICE_LEVEL_ACTION) > + self.assertEventually(lambda: self.have_text_in_log_n_times("About to call logind method Hibernate", 2), timeout=300) Can you please avoid using the lambda here? I'd rather the code was as straight forward as possible. It does increase the runtime slightly, but we're still talking tens of seconds to run the whole test suite. Created attachment 132983 [details] [review] fix for the bug v2 Sorry for not replying for so long time. I only came here to see why my bugfix is not included in the new release (0.99.5). I was not in CC list so I was not notified about your comment. I expected Bugzilla to notify me about new comments automatically. :( > Stellar work :) Thank you for the compliments. > Could you make this return the number of times the string is mentioned instead? > This would allow us to implement the above function as a special case (count == 1) > and have the assertion failures mention the number of times the string is mentioned, > instead of simply failing. Done. Just note that "have_text_in_log()" is actually "count > 0" and not "count == 1" as you mentioned to keep the old behavior. > Can you please avoid using the lambda here? I'd rather the code was as straight > forward as possible. It does increase the runtime slightly, but we're still > talking tens of seconds to run the whole test suite. I changed it to explicit timeout of 20s (+ 0.5s to avoid race conditions). I used the lambda only to avoid duplicating the UP_DAEMON_ACTION_DELAY constant value in tests as this is basically a violation of "Don't Repeat Yourself" rule. (But in case anybody changes the constant either the test will fail and he/she will be notified or in case of lowering the value the test will run longer than it could but it will work.) The new patch (v2) is in the attachment. Committed to git master, thanks very much! |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.