GNU bug report logs - #18157
24.4.50; battery-pmset fails to report critical battery state

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 18157 in the body.
You can then email your comments to 18157 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Sebastian Wiesner <swiesner <at> lunaryorn.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.4.50; battery-pmset fails to report critical battery state
Date: Thu, 31 Jul 2014 15:16:44 +0200

`battery-pmset' fails to correctly report critical battery state:  Even
if the battery charge level is below `battery-load-critical', it reports
"low" battery state only.

The culprit is the `cond' expression in the body of `battery-pmset'
which compares the reported `load-percentage' against `battery-load-low'
first, and then against `battery-load-critical'.

Since the latter is typically lower, it will never be reached, because
`cond' already returns after the former succeeded.

To fix this issue, `batter-pmset' needs to check `battery-load-critical'
*first*, and then `battery-load-low'.

Message #8 received at 18157 <at> debbugs.gnu.org (full text, mbox):

From: Marcin Borkowski <mbork <at> mbork.pl>
To: Sebastian Wiesner <swiesner <at> lunaryorn.com>
Cc: mbork <at> mbork.pl, 18157 <at> debbugs.gnu.org
Subject: Re: bug#18157: 24.4.50;
 battery-pmset fails to report critical battery state
Date: Tue, 16 Feb 2016 15:56:56 +0100

On 2014-07-31, at 16:16, Sebastian Wiesner <swiesner <at> lunaryorn.com> wrote:

> `battery-pmset' fails to correctly report critical battery state:  Even
> if the battery charge level is below `battery-load-critical', it reports
> "low" battery state only.
>
> The culprit is the `cond' expression in the body of `battery-pmset'
> which compares the reported `load-percentage' against `battery-load-low'
> first, and then against `battery-load-critical'.
>
> Since the latter is typically lower, it will never be reached, because
> `cond' already returns after the former succeeded.
>
> To fix this issue, `batter-pmset' needs to check `battery-load-critical'
> *first*, and then `battery-load-low'.

Does this patch help?

--8<---------------cut here---------------start------------->8---
diff -u --label /usr/local/share/emacs/25.1.50/lisp/battery.el.gz --label \#\<buffer\ battery.el.gz\> /tmp/jka-com1889YvM /tmp/buffer-content-1889yDZ
--- /usr/local/share/emacs/25.1.50/lisp/battery.el.gz
+++ #<buffer battery.el.gz>
@@ -628,12 +628,12 @@
 	    (cond ((looking-at "; charging")
 		   (setq battery-status "charging"
 			 battery-status-symbol "+"))
-		  ((< (string-to-number load-percentage) battery-load-low)
-		   (setq battery-status "low"
-			 battery-status-symbol "-"))
 		  ((< (string-to-number load-percentage) battery-load-critical)
 		   (setq battery-status "critical"
 			 battery-status-symbol "!"))
+		  ((< (string-to-number load-percentage) battery-load-low)
+		   (setq battery-status "low"
+			 battery-status-symbol "-"))
 		  (t
 		   (setq battery-status "high"
 			 battery-status-symbol "")))

Diff finished.  Tue Feb 16 15:56:03 2016
--8<---------------cut here---------------end--------------->8---

Best,

--
mb

Message #11 received at 18157 <at> debbugs.gnu.org (full text, mbox):

From: Sebastian Wiesner <swiesner <at> lunaryorn.com>
To: Marcin Borkowski <mbork <at> mbork.pl>
Cc: 18157 <at> debbugs.gnu.org
Subject: Re: bug#18157: 24.4.50;
 battery-pmset fails to report critical battery state
Date: Tue, 16 Feb 2016 19:41:25 +0100

> Am 16.02.2016 um 15:56 schrieb Marcin Borkowski <mbork <at> mbork.pl>:
> 
> On 2014-07-31, at 16:16, Sebastian Wiesner <swiesner <at> lunaryorn.com> wrote:
> 
>> `battery-pmset' fails to correctly report critical battery state:  Even
>> if the battery charge level is below `battery-load-critical', it reports
>> "low" battery state only.
>> 
>> The culprit is the `cond' expression in the body of `battery-pmset'
>> which compares the reported `load-percentage' against `battery-load-low'
>> first, and then against `battery-load-critical'.
>> 
>> Since the latter is typically lower, it will never be reached, because
>> `cond' already returns after the former succeeded.
>> 
>> To fix this issue, `batter-pmset' needs to check `battery-load-critical'
>> *first*, and then `battery-load-low'.
> 
> Does this patch help?

I'm sorry, are you asking me?  It would be bold to believe that I still cared for this issue now, almost *two years* after I reported it.  

At a cursory look the fix surely looks correct, but I hope you understand that Emacs taking two years for what seems to be a trivial issue to fix does not particularly contribute to my enthusiasm, nor does it motivate me to test this patch.

> --8<---------------cut here---------------start------------->8---
> diff -u --label /usr/local/share/emacs/25.1.50/lisp/battery.el.gz --label \#\<buffer\ battery.el.gz\> /tmp/jka-com1889YvM /tmp/buffer-content-1889yDZ
> --- /usr/local/share/emacs/25.1.50/lisp/battery.el.gz
> +++ #<buffer battery.el.gz>
> @@ -628,12 +628,12 @@
> 	    (cond ((looking-at "; charging")
> 		   (setq battery-status "charging"
> 			 battery-status-symbol "+"))
> -		  ((< (string-to-number load-percentage) battery-load-low)
> -		   (setq battery-status "low"
> -			 battery-status-symbol "-"))
> 		  ((< (string-to-number load-percentage) battery-load-critical)
> 		   (setq battery-status "critical"
> 			 battery-status-symbol "!"))
> +		  ((< (string-to-number load-percentage) battery-load-low)
> +		   (setq battery-status "low"
> +			 battery-status-symbol "-"))
> 		  (t
> 		   (setq battery-status "high"
> 			 battery-status-symbol "")))
> 
> Diff finished.  Tue Feb 16 15:56:03 2016
> --8<---------------cut here---------------end--------------->8---
> 
> Best,
> 
> --
> mb

Message #14 received at 18157 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Sebastian Wiesner <swiesner <at> lunaryorn.com>
Cc: mbork <at> mbork.pl, 18157 <at> debbugs.gnu.org
Subject: Re: bug#18157: 24.4.50;
 battery-pmset fails to report critical battery state
Date: Tue, 16 Feb 2016 21:04:08 +0200

> From: Sebastian Wiesner <swiesner <at> lunaryorn.com>
> Date: Tue, 16 Feb 2016 19:41:25 +0100
> Cc: 18157 <at> debbugs.gnu.org
> 
> I hope you understand that Emacs taking two years for what seems to be a trivial issue to fix does not particularly contribute to my enthusiasm, nor does it motivate me to test this patch.

FWIW, I'm very grateful to Marcin and a few people like him for
investing their time into working on this and other bug reports, and
hope that more contributors like them will come on board and help in
this effort.

Thanks a lot, guys!

Message #17 received at 18157 <at> debbugs.gnu.org (full text, mbox):

From: Marcin Borkowski <mbork <at> mbork.pl>
To: Sebastian Wiesner <swiesner <at> lunaryorn.com>
Cc: 18157 <at> debbugs.gnu.org
Subject: Re: bug#18157: 24.4.50;
 battery-pmset fails to report critical battery state
Date: Wed, 17 Feb 2016 00:43:46 +0100

On 2016-02-16, at 19:41, Sebastian Wiesner <swiesner <at> lunaryorn.com> wrote:

>> Does this patch help?
>
> I'm sorry, are you asking me?  It would be bold to believe that I still cared for this issue now, almost *two years* after I reported it.  
>
> At a cursory look the fix surely looks correct, but I hope you understand that Emacs taking two years for what seems to be a trivial issue to fix does not particularly contribute to my enthusiasm, nor does it motivate me to test this patch.

I am also sorry if I sounded rude.  The question was directed to people
reading the bug-gnu-emacs mailing list as well as to you.  I am also
sorry that this was not fixed earlier, but I am bold enough to believe
that it is better to try to investigate it now than to leave it forever.

I am also sorry that I didn't test this myself.  However, I do not own
a Mac, so I could only transcribe your report into a (trivial,
admittedly) patch for others to test.  I do not consider my input here
too valuable (everyone could write this patch, even without any deeper
knowledge about Elisp, thanks to your detailed report and suggestion as
to were the correct fix was - thank you for that!), but I hoped that
what I may have /really/ fixed was the problem of your report not
gaining enough attention when it was sent.

Also, even if you don't care for this issue anymore, there may be other
people who do.  And you were the author of the report, so it was
a natural thing to send this message to you.  (In fact, this is what the
debbugs tool did, as it does by default when responding to bug reports.)

Also, if I should have behaved differently, please tell me what I could
have done better (apart from the not-so-optimal wording of my message).

Best,

-- 
Marcin Borkowski
http://octd.wmi.amu.edu.pl/en/Marcin_Borkowski
Faculty of Mathematics and Computer Science
Adam Mickiewicz University

Message #20 received at 18157 <at> debbugs.gnu.org (full text, mbox):

From: John Wiegley <jwiegley <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: mbork <at> mbork.pl, Sebastian Wiesner <swiesner <at> lunaryorn.com>,
 18157 <at> debbugs.gnu.org
Subject: Re: bug#18157: 24.4.50;
 battery-pmset fails to report critical battery state
Date: Fri, 19 Feb 2016 21:51:52 -0800

[Message part 1 (text/plain, inline)]

>>>>> Eli Zaretskii <eliz <at> gnu.org> writes:

> FWIW, I'm very grateful to Marcin and a few people like him for investing
> their time into working on this and other bug reports, and hope that more
> contributors like them will come on board and help in this effort.
> 
> Thanks a lot, guys!

I would like to second that thanks. We move at the speed of our volunteers.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

[signature.asc (application/pgp-signature, inline)]

Message #23 received at 18157 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Marcin Borkowski <mbork <at> mbork.pl>
Cc: 18157 <at> debbugs.gnu.org
Subject: Re: bug#18157: 24.4.50;
 battery-pmset fails to report critical battery state
Date: Sat, 20 Feb 2016 18:35:14 +1100

Marcin Borkowski <mbork <at> mbork.pl> writes:

> Does this patch help?
>
> diff -u --label /usr/local/share/emacs/25.1.50/lisp/battery.el.gz --label \#\<buffer\ battery.el.gz\> /tmp/jka-com1889YvM /tmp/buffer-content-1889yDZ
> --- /usr/local/share/emacs/25.1.50/lisp/battery.el.gz
> +++ #<buffer battery.el.gz>
> @@ -628,12 +628,12 @@
>  	    (cond ((looking-at "; charging")
>  		   (setq battery-status "charging"
>  			 battery-status-symbol "+"))
> -		  ((< (string-to-number load-percentage) battery-load-low)
> -		   (setq battery-status "low"
> -			 battery-status-symbol "-"))
>  		  ((< (string-to-number load-percentage) battery-load-critical)
>  		   (setq battery-status "critical"
>  			 battery-status-symbol "!"))
> +		  ((< (string-to-number load-percentage) battery-load-low)
> +		   (setq battery-status "low"
> +			 battery-status-symbol "-"))
>  		  (t
>  		   (setq battery-status "high"
>  			 battery-status-symbol "")))

Looks logical to me.  I've applied it to emacs-25.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #26 received at 18157 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Marcin Borkowski <mbork <at> mbork.pl>
Cc: Sebastian Wiesner <swiesner <at> lunaryorn.com>, 18157 <at> debbugs.gnu.org
Subject: Re: bug#18157: 24.4.50;
 battery-pmset fails to report critical battery state
Date: Sat, 20 Feb 2016 18:36:47 +1100

Marcin Borkowski <mbork <at> mbork.pl> writes:

> Also, if I should have behaved differently, please tell me what I could
> have done better (apart from the not-so-optimal wording of my message).

No, you've done exactly what we hope people would do.  Thank you for
investigating the problem and providing a patch.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.