matchai / spacefish

🚀🐟 The fish shell prompt for astronauts
https://spacefish.matchai.dev
MIT License
963 stars 79 forks source link

Random errors with battery section upon opening terminal #96

Closed Snuggle closed 5 years ago

Snuggle commented 5 years ago

Bug Report

Current Behavior image

Welcome to fish, the friendly interactive shell

~
➜ Could not send process 4477, 'grep' in job 10, 'upower -e | grep battery | head -1' from group 4158 to group 4472
setpgid: Operation not permitted
Could not send job 10 ('upower -e | grep battery | head -1') to foreground
tcsetpgrp: No such process
/org/freedesktop/UPower/devices/mouse_hidpp_battery_0
Could not send process 4478, 'head' in job 10, 'upower -e | grep battery | head -1' from group 4158 to group 4472
setpgid: Operation not permitted
<Could not send job W10> (' upower -e | grep battery | head -1f') to foregroundi
stcsetpgrp: No such process
h: Could not send job 10 (“upower -e | grep battery | head -1”) to foreground
~ setpgrp: No such process
✦ ➜

Expected Behavior Since my desktop has no battery, the battery section should exit without doing anything. I do have upower installed as a dependency for GNOME.

Relevant Fish Configuration

I do not have a fish config file or use any extra configuration, Spacefish installed using fisher.

Environment

Possible Solution

I wonder if this is an upstream bug with Fish, since the line of code is encased in (blah blah). It shouldn't print anything.

Additional context/Screenshots I just booted my desktop and opened a terminal window, this happened.

matchai commented 5 years ago

Hmm... 🤔 Would you be able to post the output of upower -e for you?

Can you try redirecting stderr to null within those parentheses?

set -l battery (upower -e | grep battery | head -1 ^/dev/null)
Snuggle commented 5 years ago
~
➜ upower -e
/org/freedesktop/UPower/devices/mouse_hidpp_battery_0
/org/freedesktop/UPower/devices/DisplayDevice

~
➜ upower -e | grep battery | head -1 ^/dev/null
/org/freedesktop/UPower/devices/mouse_hidpp_battery_0

~
➜ set -l battery (upower -e | grep battery | head -1 ^/dev/null)

~
➜ echo $battery
/org/freedesktop/UPower/devices/mouse_hidpp_battery_0

~
➜ set -l battery (upower -e | grep battery | head -1)

~
➜ echo $battery
/org/freedesktop/UPower/devices/mouse_hidpp_battery_0

Is it... Trying to use the battery for my wireless mouse? I use the Logitech MX Master. :thinking:

matchai commented 5 years ago

Oh wow! Looks like it is. If the stderr redirection silences the setpgid errors you were having, then feel free to make a PR with the fix.

With regards to upower identifying your mouse, you might want to make an issue on spacefish to create a whitelist of relevant device batteries prefixes (mouse_ should definitely be filtered out).

It looks like we overlooked testing for the battery section, so I think I'll go ahead and work on that soon.

Snuggle commented 5 years ago

If a whitelist is the best way to go, what comes up for actual laptops with a battery? Do you have a laptop that has a battery in upower?

matchai commented 5 years ago

The only laptops I have use pmset for power. 😕

salmanulfarzy commented 5 years ago

There is an issue in spaceship-prompt#526 regarding batteries of removable devices. We're waiting for PR.

What is the output of acpi -b ? Could you try moving acpi before upower in battery section?

Snuggle commented 5 years ago

It appears the one laptop I have that's running Fedora has no output for upower. :slightly_frowning_face:

If I install acpi, though, my Fedora laptop gets the following output:

➜ acpi -b
Battery 0: Charging, 99%, 00:05:20 until charged

I can test what happens on my desktop once I get back home later today!

salmanulfarzy commented 5 years ago

Huh.. Sorry about the duplicate comment. Thought it wasn't posted due to GitHub outage today.

matchai commented 5 years ago

Seeing as this issue is currently being discussed upstream, I'll close this for now. 👍

salmanulfarzy commented 5 years ago

@Snuggle Please let us know output of acpi -b on desktop when you get chance to run it.

Snuggle commented 5 years ago
~
➜ acpi -b
Battery 0: Discharging, 0%, rate information unavailable

I guess detect if battery level is >0%? Not the best solution but... It works for the most part.

Snuggle commented 5 years ago

Still getting this bug randomly.

image

matchai commented 5 years ago

Our use case seems pretty simple. It seems like it might be an issue with fish itself 😕 https://github.com/fish-shell/fish-shell/issues/3952 https://github.com/fish-shell/fish-shell/issues/4235

(whoops! hit the reopen button accidentally)

nicktimko commented 5 years ago

I don't know if this is a related issue, but when resizing my terminal window sometimes I'll get battery-related jobs that pop up and don't seem to go away. Guessing either this extension can poke at an underlying problem and/or the extension itself is maybe a bit overzealous in running some commands. Hard to repro, as I need to resize my terminal like a madman, but some fragmentary output is below.

✦3 ➜ <W> fish: Could not send job 12 (“upower -e | grep battery | head -1”) to foreground
tcsetpgrp: No such process
Could not send process Could not send process 96209621, ', 'head' in job grep' in job 12, 'upower -e | grep battery | head -1' from group 2772312, 'upower -e | grep battery | head -1' from group 27723 to group 9615
setpgid: Operation not permitted
Could not send job 12 ('upower -e | grep battery | head -1') to foreground
tcsetpgrp: No such process
 to group 9615
setpgid: Operation not permitted
Could not send job 12 ('upower -e | grep battery | head -1') to foreground
tcsetpgrp: No such process
~ rg/freedesktop/UPower/devices/battery_BAT0
~  ➜ 
~ 
✦4 ➜ fg
Send job 12, “upower -e | grep battery | head -1” to foreground
<W> fish: Could not send job 12 (“upower -e | grep battery | head -1”) to foreground
tcsetpgrp: No such process

~ 
✦4 ➜ jobs
Job Group   CPU State   Command
12  9615    0%  stopped upower -e | grep battery | head -1
11  8902    0%  stopped upower -e | grep battery | head -1
10  7380    0%  stopped upower -e | grep battery | head -1
9   646 0%  stopped echo $battery_data | grep percentage | awk '{print $2}'

Worst case it will actually crash my shell

✦10 ➜ 
<E> fish: More than one job in foreground: job 1: “upower -e | grep battery | head -1” job 2: “echo $battery_data | grep state | awk '{print $2}'”
<E> fish: Errors detected, shutting down. Break on sanity_lose() to debug.
<E> fish: More than one job in foreground: job 1: “echo $battery_data | grep state | awk '{print $2}'” job 2: “upower -e | grep battery | head -1”
<E> fish: Errors detected, shutting down. Break on sanity_lose() to debug.
<E> fish: More than one job in foreground: job 1: “upower -e | grep battery | head -1” job 2: “upower -e | grep battery | head -1”
<E> fish: Errors detected, shutting down. Break on sanity_lose() to debug.
nick@e5ck9j1:~$ 

The fish is 2.7.1 and I just installed spacefish today, so I don't know if some of the fixes discussed in your linked issues are pending release or what. set -gx SPACEFISH_BATTERY_SHOW false seems to make it less prone to happening.

This might be a bandaid on an infected wound, but from some of the remarks on the fish issues, I wonder if directly using /bin/grep the command instead of grep fish function would avoid some layers?