only show status when AC is offline

[mgor]

Hey,

I wanted the possibility to only show battery status when the AC is offline. I made it configurable with @batt_toggle_ac_status, if not set it will be set to false and it would always show the battery status (how it works now).

This is done with the helper method ac_online, if it returns true (0) all the prefixes will return an empty string (not showing anything in the status bar), if it returns false (1) it will show the information.

I only had possibility to test on Ubuntu (with upower and checking sysfs). So ac_online should be updated with the results of using pmset or acpi as well.

Also, I fixed a bunch of shellcheck warnings.

Thanks for a great plugin!

[martinbeentjes]

In your PR you are forcing all the modules of the plugin to automatically check something. I don't like this approach. I think it is better to leave the choice of actually using this new module to the user. And thus not adding the call to your code in every other module.

Also, is /org/freedesktop/UPower/devices/line_power_AC a general way of checking that AC information? Or is that specific to the distribution you are using?

What is the reason to change from -le to <?

One last thing: please fix the mixed indentation to only tabs.

[mgor]

Do you have a suggestion on how to accomplish this? In a first version of the PR I did the check in battery.tmux, and replace every prefix with a empty string (instead of the helper script), but then I relalized that the values will never be updated by tmux, since that part is only run once (at start). Scripts specified with #(...) will run every update interval.

I'm not sure about /org/freedesktop/UPower/devices/line_power_AC, I made the assumption that it would be the same on every distribution with upower and dbus(?) :-)

[martinbeentjes]

That statusbar of tmux is updated every five seconds (default interval), so that should also rerender all the information within the statusbar.

So, if you move the check of the AC to a script, that should just work fine.

[mgor]

Hm... I think there might be some misunderstanding of this PR does? Or how do you suggest that one helper script would change the output for the other helper scripts?

Consider the status bar being: #{battery_status_bg} #{battery_percentage} #{battery_icon}

If ac_online returns true (0), I don't want any output of either of these prefixes, and in battery.tmux these prefixes will be replaced by their respective helper script (hence, the check being made in each helper script).

If ac_online returns false (1), I want them to show the information.

[martinbeentjes]

So the basic goal of this helper script is to check if the machine is connected to AC, and if so: not show any information?

[mgor]

Yes, correct. But only if @batt_toggle_ac_status is set to true. In other words, if this configuration parameter isn't set, or if it is set to false it will behave as before.

[martinbeentjes]

First of all, I don't really like the naming of the option. It does not indicate to me what it exactly does. That is something that has to be changed before a possible merge.

Besides that, for now the PR adds the following: "Only show the battery information when the power adapter is not attached and @batt_toggle_ac_status to true." Can you confirm this?

Personally, I prefer to keep the battery information in the status bar even if the plug is attached. Why? Because I want to have a quick look at the percentage it is.

[mgor]

What's your suggestion on naming the option? I wanted to keep it as short as possible. batt_hide_when_ac_online?

Yes, I confirm that this is the case.

...and this is why it was added as an option, and doesn't change the default behaviour.

[mgor]

I hope that I've fixed the issues you've pointed out. However, it seems like there was already some mixed-indent in scripts/battery_status_bg.sh (which was fixed now).

[martinbeentjes]

What's your suggestion on naming the option? I wanted to keep it as short as possible. batt_hide_when_ac_online?

Maybe batt_toggle_plug_status is a bit more clear?

Yes, I confirm that this is the case.

Okay, good!

...and this is why it was added as an option, and doesn't change the default behaviour.

Yes, now it makes more sense.

---

So, to finish the PR in a good way, we need to answer and check the following:

• [ ] Is this functionality present in acpi?
• [ ] Is this functionality present in upower?

[mgor]

Maybe batt_toggle_plug_status is a bit more clear?

Fixed.

Is this functionality present in acpi?

I installed acpi and added support for it.

Is this functionality present in upower?

I'm guessing you mean pmset? Thanks to @troufster I got SSH access to a Apple laptop so I could add support for pmset as well.

[martinbeentjes]

Well, that was quicker than I expected! However, the output of the pmset -g does not give anything containing "AC Online"

$ pmset -g
System-wide power settings:
Currently in use:
 standbydelay         10800
 standby              1
 womp                 1
 halfdim              1
 hibernatefile        /var/vm/sleepimage
 powernap             1
 gpuswitch            2
 networkoversleep     0
 disksleep            10
 sleep                0
 autopoweroffdelay    28800
 hibernatemode        3
 autopoweroff         1
 ttyskeepawake        1
 displaysleep         15
 acwake               0
 lidwake              1

On macOS 10.12.2 Beta (16C48b)

---

I think you mean pmset -g batt 😉

[mgor]

Not sure which version of Mac OS X that was on the laptop I tried on, but the output was as follows:

MacBook-Pro:~ mgor$ pmset -g | awk '/Power/'
Battery Power           -1
AC Power                -1*

Which is why I didn't have pmset -g batt.

But with your output, I would suggest to chain that elif-case to:

pmset -g batt | egrep -q "^Now draining from.*AC Power"

(I hope that egrep is available in Mac OS X)

[martinbeentjes]

I think you tested it on an earlier version than Sierra. pmset has been updated, the output of the command is different on macOS Sierra compared to OS X El Capitan.