You are not logged in.

#1 2010-12-28 04:31:23

jasonwryan
Anarchist
From: .nz
Registered: 2009-05-09
Posts: 30,424
Website

Pointers re bash status script

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 smile

# edit - so much for extensive testing. Moved the $CHG line into the loop where it could actually update tongue

#!/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)


Arch + dwm   •   Mercurial repos  •   Surfraw

Registered Linux User #482438

Offline

#2 2010-12-28 04:57:45

dmz
Member
From: Sweden
Registered: 2008-08-27
Posts: 881
Website

Re: Pointers re bash status script

You might want to check out Perl. Brutal enough? smile

Offline

#3 2010-12-28 05:10:09

jasonwryan
Anarchist
From: .nz
Registered: 2009-05-09
Posts: 30,424
Website

Re: Pointers re bash status script

LOL. Given my ineptitude with bash, your optimism with regard to my checking out Perl is not so much brutal as delusional smile


Arch + dwm   •   Mercurial repos  •   Surfraw

Registered Linux User #482438

Offline

#4 2010-12-28 05:21:48

falconindy
Developer
From: New York, USA
Registered: 2009-10-22
Posts: 4,111
Website

Re: Pointers re bash status script

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

#5 2010-12-28 05:58:05

jasonwryan
Anarchist
From: .nz
Registered: 2009-05-09
Posts: 30,424
Website

Re: Pointers re bash status script

Thanks falconindy!

Does that mean you are giving me a pass on the $BAT line?


Arch + dwm   •   Mercurial repos  •   Surfraw

Registered Linux User #482438

Offline

#6 2010-12-28 10:40:56

Chippeur
Member
Registered: 2010-01-25
Posts: 19

Re: Pointers re bash status script

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

#7 2010-12-28 12:48:54

hellomynameisphil
Member
From: /home/phil/Vancouver
Registered: 2009-10-02
Posts: 257
Website

Re: Pointers re bash status script

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

#8 2010-12-28 13:37:38

Ashren
Member
From: Denmark
Registered: 2007-06-13
Posts: 1,229
Website

Re: Pointers re bash status script

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

#9 2010-12-28 13:38:35

litemotiv
Forum Fellow
Registered: 2008-08-01
Posts: 5,026

Re: Pointers re bash status script

hellomynameisphil wrote:

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:

jasonwryan wrote:

To try and come to grips with bash scripting, I have been working on a basic status script for my laptop.


ᶘ ᵒᴥᵒᶅ

Offline

#10 2010-12-28 14:16:57

hellomynameisphil
Member
From: /home/phil/Vancouver
Registered: 2009-10-02
Posts: 257
Website

Re: Pointers re bash status script

litemotiv wrote:

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

#11 2010-12-28 14:31:00

Chippeur
Member
Registered: 2010-01-25
Posts: 19

Re: Pointers re bash status script

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

#12 2010-12-28 14:36:02

falconindy
Developer
From: New York, USA
Registered: 2009-10-22
Posts: 4,111
Website

Re: Pointers re bash status script

jasonwryan wrote:

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

#13 2010-12-28 14:55:28

Ashren
Member
From: Denmark
Registered: 2007-06-13
Posts: 1,229
Website

Re: Pointers re bash status script

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

#14 2010-12-28 15:49:45

steve___
Member
Registered: 2008-02-24
Posts: 452

Re: Pointers re bash status script

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

#15 2010-12-28 15:52:48

steve___
Member
Registered: 2008-02-24
Posts: 452

Re: Pointers re bash status script

falconindy wrote:
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

#16 2010-12-28 15:59:36

steve___
Member
Registered: 2008-02-24
Posts: 452

Re: Pointers re bash status script

@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

#17 2010-12-28 16:05:26

falconindy
Developer
From: New York, USA
Registered: 2009-10-22
Posts: 4,111
Website

Re: Pointers re bash status script

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

#18 2010-12-28 18:06:03

jasonwryan
Anarchist
From: .nz
Registered: 2009-05-09
Posts: 30,424
Website

Re: Pointers re bash status script

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 smile 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 smile

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.


Arch + dwm   •   Mercurial repos  •   Surfraw

Registered Linux User #482438

Offline

#19 2010-12-29 09:43:31

rockin turtle
Member
From: Montana, USA
Registered: 2009-10-22
Posts: 227

Re: Pointers re bash status script

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

#20 2010-12-29 22:37:41

jasonwryan
Anarchist
From: .nz
Registered: 2009-05-09
Posts: 30,424
Website

Re: Pointers re bash status script

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"

Arch + dwm   •   Mercurial repos  •   Surfraw

Registered Linux User #482438

Offline

#21 2010-12-29 22:53:20

falconindy
Developer
From: New York, USA
Registered: 2009-10-22
Posts: 4,111
Website

Re: Pointers re bash status script

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

#22 2010-12-29 23:48:54

jasonwryan
Anarchist
From: .nz
Registered: 2009-05-09
Posts: 30,424
Website

Re: Pointers re bash status script

falconindy wrote:

@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. smile
Thanks falconindy.


Arch + dwm   •   Mercurial repos  •   Surfraw

Registered Linux User #482438

Offline

#23 2011-01-04 01:37:54

Procyon
Member
Registered: 2008-05-07
Posts: 1,819

Re: Pointers re bash status script

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

#24 2011-01-04 02:51:59

steve___
Member
Registered: 2008-02-24
Posts: 452

Re: Pointers re bash status script

Isn't /proc being phased out for /sys ?

cat /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_cur_freq

Offline

Board footer

Powered by FluxBB