bash loop for checking service and network connection

Question

I want to start omxplayer, check if there is the process is up, check if there is internet connection start with play a streaming, if there is not play a local loop, meanwhile try to check if the internet connection is up, if there is internet connection stop local file and play streaming video. I write this... any suggestion? Thanks

#!/bin/bash
killall -9 omxplayer.bin
SERVICE='omxplayer'
while true; do
if ps ax | grep -v grep | grep $SERVICE > /dev/null
then
echo "Running" # sleep 5
else
sleep 5
log=log_file_pette.txt
# create log file or overrite if already present
printf "Log File Streaming- " >> $log

# append date to log file
date >> $log

# send email with date restart
echo `date` | mailx -s "Subject: Start streaming Ad" [email protected]
# if there is no internet and no ping
if ping -q -c 1 -W 1 google.com >/dev/null
then
killall -9 omxplayer.bin
$SERVICE -o alsa https://xxxxxxxxxxxx/playlist.m3u8 &
else
# create log file or overrite if already present
printf "Log File Local- " >> $log

# append date to log file
date >> $log
killall -9 omxplayer.bin
$SERVICE -o alsa looptv.mp4 
sleep 5
fi
fi
done
```

Answer 1

Use indentation to make the flow of logic easier to see

With indentation added to the original code, it's a lot easier to understand the branches of the logic:

killall -9 omxplayer.bin
SERVICE='omxplayer'
while true; do
    if ps ax | grep -v grep | grep $SERVICE > /dev/null
    then
        echo "Running" # sleep 5
    else
        sleep 5
        log=log_file_pette.txt
        # create log file or overrite if already present
        printf "Log File Streaming- " >> $log

        # append date to log file
        date >> $log

        # send email with date restart
        echo `date` | mailx -s "Subject: Start streaming Ad" [email protected]
        # if there is no internet and no ping
        if ping -q -c 1 -W 1 google.com >/dev/null
        then
            killall -9 omxplayer.bin
            $SERVICE -o alsa https://xxxxxxxxxxxx/playlist.m3u8 &
        else
            # create log file or overrite if already present
            printf "Log File Local- " >> $log

            # append date to log file
            date >> $log
            killall -9 omxplayer.bin
            $SERVICE -o alsa looptv.mp4 
            sleep 5
        fi
    fi
done

Use functions to avoid duplicated logic

This snippet appears twice, with one word replaced:

# create log file or overrite if already present
printf "Log File Streaming- " >> $log

# append date to log file
date >> $log

It would be good to create a helper function for it, with the changing part a variable:

log_with_label() {
    local label=$1
    echo "Log File $label- $(date)" >> "$log"
}

Then you can call this with log_with_label "Streaming" and log_with_label "Local".

Even the simple killall -9 omxplayer.bin would be good in a helper function:

kill_player() {
    killall -9 omxplayer.bin
}

The comments... are lying...

Some of the comments are confusing, because they don't tell the truth.

No sleeping happens here:

echo "Running" # sleep 5

The file will not be overwritten here. This will append to the file if exists:

# create log file or overrite if already present
printf "Log File Streaming- " >> $log

Use double-quotes around variables in shell commands

Instead of these:

if ps ax | grep -v grep | grep $SERVICE > /dev/null
printf "Log File Streaming- " >> $log
$SERVICE -o alsa https://xxxxxxxxxxxx/playlist.m3u8 &

write like this:

if ps ax | grep -v grep | grep "$SERVICE" > /dev/null
printf "Log File Streaming- " >> "$log"
"$SERVICE" -o alsa https://xxxxxxxxxxxx/playlist.m3u8 &

Avoid deeply nested code

Looking at this code:

while true; do
    if ps ax | grep -v grep | grep $SERVICE > /dev/null
    then
        echo "Running" # sleep 5
    else

        # ... many, many lines here....

    fi
done

By the time the I reach the fi, I don't remember what the base condition was. It would be good to reduce the nesting with a continue in the if branch. (I'm wondering if you wanted to put a sleep 5 in there, in code, instead of a comment...)

while true; do
    if ps ax | grep -v grep | grep $SERVICE > /dev/null
    then
        echo "Running" # sleep 5
        continue
    fi

    # ... many, many lines here....

done

Use shellcheck.net

shellcheck.net is a that finds bugs in your shell scripts. It's good to fix all the violations raised by it.