You are not logged in.
Pages: 1
To try and come to grips with bash scripting, I have been working on a basic status script for my laptop. What I have at the moment has all the functionality that I need and, more importantly, it works.
However, as it runs in a loop and as you never know what you don't know, I would really appreciate any comments about the approach I have used.
Please don't rewrite the script, just point out things I might want to look at to make it more efficient and/or more elegant/correct. Apart from Procyon's line, most of it is stuff I have cobbled together. The $BAT stuff seems particularly kludgy...
Brutal criticism is welcome, as long as it is constructive
# edit - so much for extensive testing. Moved the $CHG line into the loop where it could actually update
#!/bin/bash
# Status script for wmfs
RED="\\#BF4D80\\"
YEL="\\#C4A1E6\\"
BLU="\\#477AB3\\"
GRN="\\#53A6A6\\"
CYN="\\#6096BF\\"
MAG="\\#7E62B3\\"
GRY="\\#666666\\"
WHT="\\#C0C0C0\\"
while true;
do
# Collect system information
CHG=`acpi -V | awk '{ gsub(/,/, "");} NR==1 {print $4}'`
BAT=`acpi -V | if grep -q "on-line"; then echo $BLU"AC"; else echo $RED$CHG; fi`
MEM=`free -m | awk '/Mem/ {print $3}'`
# CPU line courtesy Procyon: https://bbs.archlinux.org/viewtopic.php?pid=661592
CPU=`eval $(awk '/^cpu /{print "previdle=" $5 "; prevtotal=" $2+$3+$4+$5 }' /proc/stat); sleep 0.4;
eval $(awk '/^cpu /{print "idle=" $5 "; total=" $2+$3+$4+$5 }' /proc/stat);
intervaltotal=$((total-${prevtotal:-0}));
echo "$((100*( (intervaltotal) - ($idle-${previdle:-0}) ) / (intervaltotal) ))"`
HD=`df -h | awk '/^ |sd/ {if (NF==5) print $4; else if (NF==6) print $5}'`
PCM=`exec "/home/jason/Scripts/pacman-up.pl"`
INT=`host google.com>/dev/null; if [ $? -eq 0 ]; then echo $GRN"ON"; else echo $RED"NO"; fi`
DATE=`date "+%I:%M"`
# Pipe to status bar
wmfs -s 0 "$GRY[BAT $BAT$GRY] [CPU $YEL$CPU%$GRY MEM $CYN$MEM$GRY] [HDD $MAG$HD$GRY] [PAC $BLU$PCM$GRY] [NET $INT$GRY] • $WHT$DATE"
sleep 5
done
Last edited by jasonwryan (2010-12-28 06:52:25)
Offline
You might want to check out Perl. Brutal enough?
Offline
LOL. Given my ineptitude with bash, your optimism with regard to my checking out Perl is not so much brutal as delusional
Offline
INT=`host google.com>/dev/null; if [ $? -eq 0 ]; then echo $GRN"ON"; else echo $RED"NO"; fi`
If you don't care about the exact return val and you really only care about pass or fail, you can simplify:
host google.com >/dev/null && echo "${GRN}NO" || echo "${RED}NO"
df has a POSIX compliance flag (-P) that should make your life easier. Everything is printed on the same line, meaning that you don't need to worry about counting columns, i.e.:
df -P | awk '/^\/dev/{print $5}'
Offline
Thanks falconindy!
Does that mean you are giving me a pass on the $BAT line?
Offline
For the $BAT ligne you could do the following:
BAT=$(grep -q "on-line" <(acpi -V) && echo $BLU"AC" || echo $RED$CHG)
@see man bash "Process Substitution" section for the '<(acpi -V)'.
Last edited by Chippeur (2010-12-28 10:42:42)
Sorry in advance for my poor english...
Offline
I haven't had a super-close look at your script, but is there a reason you're not using conky? My understanding is that it would be more efficient than running a bunch of shell commands every couple of seconds. Are you doing this in bash just to learn bash? IANAP, and bash is a fine language, but it seems more idiomatic to me to either write something like this in a higher-level language or do it in conky.
Offline
I think you need a battery percentage indicator, so I awk'ed this up:
awk 'BEGIN{v1=0; v2=0} /rem/ {v1=$3}; /full/ {v2=$4};END {v3=v1*100/v2; printf "%d%\n", v3'} /proc/acpi/battery/BAT0/{info,state}
Last edited by Ashren (2010-12-28 14:20:58)
Offline
I haven't had a super-close look at your script, but is there a reason you're not using conky?
First line of this thread:
To try and come to grips with bash scripting, I have been working on a basic status script for my laptop.
ᶘ ᵒᴥᵒᶅ
Offline
First line of this thread:
Yes, thank you, I saw that, but my question still stands. One of his questions was on how to make this more efficient, and while I am no expert on these matters, I am led to understand that using bash scripts for this sort of thing is not the most efficient use of resources. It seems to me that if you want to learn (more about) bash, there are more suitable applications. I don't mean this as a criticism, I'm just curious what the OP's thoughts are on the choice of language/tool. Was conky/higher-level language considered and rejected? Is it simply that this is a pet project to learn bash and the OP doesn't mind wasting a few cpu cycles using (what I understand to be) a less efficient tool? Does the OP not find the difference in resource usage significant? These are the kinds of questions I am driving at.
Offline
It's 'piped' in the wmfs status bar. This is apparently the way wmfs works, there is no gain to also run Conky I think. (I don't know wmfs maybe I'm wrong)
Last edited by Chippeur (2010-12-28 14:33:38)
Sorry in advance for my poor english...
Offline
Thanks falconindy!
Does that mean you are giving me a pass on the $BAT line?
It didn't stand out to be as being quite as hideous, but you could condense it as well, either by chippeur's bash-ier method or a more portable solution:
BAT=$({ acpi -V | grep -q "on-line"; } && echo $BLU"AC" || echo $RED$CHG)
Which isn't any better or worse, persay.
Offline
Hah. I did not know the acpi command for some reason. It certainly makes my battery awk redundant.
Well, here's an awk version of the $BAT line:
BAT=`acpi -V | awk '/on-line/ {if ($3 == "on-line") {print "'"$BLU"'""AC"} else {print "'"$RED$CHG"'"}}'`
I know, I know ... awk isn't bash.
Offline
Picking nits:
* Backticks are deprecated, use $() instead.
* Use lower case var names unless they're global variables
Instead of looping every x seconds, one could use inotifywait or incron to write the values to a named pipe.
Last edited by steve___ (2010-12-28 16:02:12)
Offline
BAT=$({ acpi -V | grep -q "on-line"; } && echo $BLU"AC" || echo $RED$CHG)
Which isn't any better or worse, persay.
Wouldn't using the fd be more efficient? It would save the pipe.
Offline
@jasonwryan
You could lose the dependency on acpi and get the values for temp and bat yourself:
temp=$(</sys/class/thermal/thermal_zone0/temp)
temp=$(($temp/1000))
Offline
Backticks aren't going away, now matter how many people say they're deprecated. Better reasons to avoid using them instead of $() is readability and the ability to nest commands without going insane trying to figure out the backslash escapes and ending up back at the readability issue.
Using the process substituion is no more or less efficient than the pipe. Using the bash method:
1) bash forks, exec's a process (acpi) and writes the data to /dev/fd/XX
2) bash forks, exec's a process (grep) with stdin tied to the FD opened in step 1
Using the pipe:
1) bash forks, exec's a process (acpi) with stdout tied to an unnamed pipe
2) bash forks, exec's a process (grep) with stdin tied to the unnamed pipe
Process substitution is a convenience method of getting around the subshell created in a 'foo | while read...' loop. You could just as easily write the output of foo to a file, redirect the file to the while loop and then discard the file.
Offline
Wow: thank you all for the input.
Chippeur: the process substitution pointer is exactly the type of thing I am after - cheers.
hellomynameisphil: apparently the only way to learn bash is to actually write bash scripts, and as tinkering around on my laptop is the only opportunity I have to do this, ditching conky seemed as good a place as any The efficiency I was asking about was within bash, not necessarily the overhead re bash v conky - or some other language (as dmz suggested).
Ashren: I'm tempted to go with your awk line, just to try and make it a clean sweep
falconindy: your bash-fu is formidable. I am in awk.
steve___: again, those pointers are the sorts of things I hoped for when I posted: thank you.
Offline
I also think you should use $() instead of ``, it's so much more readable.
I think that the "CPU=`eval ..." stuff is awful. You should avoid using eval.
As an alternative, take a look at the following script. It's an infinite loop, you can C-c to stop it.
#!/bin/bash
previous_total=0
previous_stats=(0 0 0 0)
while :
do
current_total=0
read -a current_stats < <(awk '/^cpu /{print $2" "$3" "$4" "$5}' /proc/stat)
for n in ${current_stats[@]}; do ((current_total+=$n)); done
sleep 2
((idle=${current_stats[3]}-${previous_stats[3]}))
((total=current_total-previous_total))
((cpu=100*(total-idle)/total))
previous_stats=(${current_stats[@]})
previous_total=$current_total
echo "cpu $cpu%"
done
Last edited by rockin turtle (2010-12-29 09:47:48)
Offline
Thanks rockin turtle. I tried your suggestion, but couldn't get it to play nicely with the rest of the
script. So I am using Procyon's. I'm not sure what is wrong with eval, but I will live with it for the time being.
I have made quite a few changes based on everyone's input. Thank you all.
The finished product? At least for now:
#!/bin/bash
# Status script for wmfs
RED="\\#BF4D80\\"
YEL="\\#C4A1E6\\"
BLU="\\#477AB3\\"
GRN="\\#53A6A6\\"
CYN="\\#6096BF\\"
MAG="\\#7E62B3\\"
GRY="\\#666666\\"
WHT="\\#C0C0C0\\"
# Collect system information
CHG=$(acpi -V | awk '{ gsub(/,/, "");} NR==1 {print $4}')
BAT=$(grep -q "on-line" <(acpi -V) && echo $BLU"AC" || echo $RED$CHG)
MEM=$(awk '/Mem/ {print $3}' <(free -m))
# CPU line courtesy Procyon: https://bbs.archlinux.org/viewtopic.php?pid=661592
CPU=$(eval $(awk '/^cpu /{print "previdle=" $5 "; prevtotal=" $2+$3+$4+$5 }' /proc/stat); sleep 0.4;
eval $(awk '/^cpu /{print "idle=" $5 "; total=" $2+$3+$4+$5 }' /proc/stat);
intervaltotal=$((total-${prevtotal:-0}));
echo "$((100*( (intervaltotal) - ($idle-${previdle:-0}) ) / (intervaltotal) ))")
HD=$(awk '/^\/dev/{print $5}' <(df -P))
PCM=$("$HOME/Scripts/pacman-up.pl")
INT=$(host google.com>/dev/null && echo $GRN"ON" || echo $RED"NO")
DTE=$(date "+%I:%M")
# Pipe to status bar
wmfs -s "$GRY[BAT $BAT$GRY] [CPU $YEL$CPU%$GRY MEM $CYN$MEM$GRY] [HDD $MAG$HD$GRY] [PAC $BLU$PCM$GRY] [NET $INT$GRY] • $WHT$DTE"
Offline
Eval is a common misspelling of evil. It's prone to code injection, blah blah blah blah.
http://mywiki.wooledge.org/BashFAQ/048
I use it. It's extremely useful in a select number of cases, but one should be aware of the pitfalls associated with it.
@jasonwryan: in your case, it's fine. Unless the kernel decides to start writing something like '523489;rm -rf /' to /proc/stat you'll be alright.
Offline
@jasonwryan: in your case, it's fine. Unless the kernel decides to start writing something like '523489;rm -rf /' to /proc/stat you'll be alright.
I suspect I'll be fine even if that does happen.
Thanks falconindy.
Offline
It's a bit tricky, but you could save a lot of PIDs by trying to shape the entire line in 1 awk script for example with wmfs $(awk ... < <(acpi -V; df -H; free -m)). Or even launching wmfs from within awk. Not that that really matters probably, but I have some old scripts that use hundreds of PIDs per minute which I'm too lazy to fix and I feel kind of bad about it.
You can also get the information from /proc instead of those commands, like /proc/meminfo, /proc/acpi/ac_adapter/ACAD/state. As for conserving PIDs it doesn't really combine with the above method, because you still have to cat it I think, it's a bit easier on resources though: time free -m &>/dev/null; time cat /proc/meminfo &>/dev/null
Here is an alternative for the CPU percentage script using fewer PIDs and no eval:
read cpu a b c previdle rest < /proc/stat
prevtotal=$((a+b+c+previdle))
sleep 0.5
read cpu a b c idle rest < /proc/stat
total=$((a+b+c+idle))
CPU=$((100*( (total-prevtotal) - (idle-previdle) ) / (total-prevtotal) ))
in awk, maybe this is usable with the altogether method too:
CPU=$(awk 'BEGIN{i=0}
{sum[i]=$2+$3+$4+$5; idle[i++]=$5}
END {printf "%d\n", 100*( (sum[1]-sum[0]) - (idle[1]-idle[0]) ) / (sum[1]-sum[0])}
' <( head -n 1 /proc/stat; sleep 0.5; head -n 1 /proc/stat))
Offline
Isn't /proc being phased out for /sys ?
cat /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_cur_freq
Offline
Pages: 1