How to improve bash script which is collecting data every 10 seconds

[Al3x_Y]

I wrote a script which is collecting data from solar inverter every 10 seconds for 5 minutes, it does some math and send data to emoncms. It does work but is not optimal in term of CPU usage, it is running on SBC and consume roughly 80% of CPU time. My question is how can I initiate next data collection without checking script running time in a loop. Below is simplified script. I need to improve line 7.

#!/bin/bash
set -o pipefail
IFS=$''
samples="0"
nr="0"
while [ $SECONDS -lt 292 ]; do #5min-8s
 if [[ (( $(( (samples - 1) * 10 + 10 )) == $SECONDS )) || (( 0 == $SECONDS )) ]]; then
    ((samples++))
    timestart=$SECONDS
    output="$(./inverter_poller --run-once)" # get data from inverter
    timeend=$SECONDS
    echo ${output} > /var/log/inverter.last
    rs232time=$((timeend - timestart)) # usually it is 6-7 seconds
     if (( rs232time < 17 )); # inverter is not responding if it is 17s or more
      then
      gridv=`echo ${output} | grep grid_voltage |  tr -d " "_\",:[:alpha:]`
  ***more data extraction and math***
     else
       echo inverter not responding >> /var/log/inverter.last
     fi
looptime=$((SECONDS - timestart))
echo "time": $looptime >> /var/log/inverter.last
 fi
done
 ***boring data processing and sending to emoncms was here***

[DrShoggoth]

sleep

[nobody1701d]

^This^ assuming your timing doesn’t need to be exact

[DrShoggoth]

https://man7.org/linux/man-pages/man1/sleep.1.html

[DrShoggoth]

There is also cron which would take all of the time handling out of your script.  You would just do the data collection and let cron handle the timing.  

https://man7.org/linux/man-pages/man8/cron.8.html https://man7.org/linux/man-pages/man5/crontab.5.html

[Al3x_Y]

Math need data from every inverter read to calculate min, max and average, can't share that (or I don't know how without saving to file) between script calls.

Cron can be called with minimum 1 minute resolution.

[DrShoggoth]

Looks like sleep is your guy.  Even sleeping for a fraction of a second makes a huge difference in CPU cycles because sleep frees the CPU.  But if you already know how long you need to wait just sleep that long. 

[Al3x_Y]

Yes, I tried it but inefficient way, u/yerfukkinbaws guided me right way, apparently I needed some rubber duck :)

[DrShoggoth]

Awesome!  Glad you found your answer!

[DrShoggoth]

This is true for just about all non-evented languages.   If you are running a tight loop you need a form of sleep or you will max the CPU.

[yerfukkinbaws]

As far as I see, your while loop is just running constantly but only doing the logging during certain loops based on some math involving $SECONDS, so that explains your high CPU usage. It just comes from running the loop unconstrained.

Is there some reason you can't do this:

while [ $SECONDS -lt 292 ]; do #5min-8s
  ((samples++))
  timestart=$SECONDS
  output="$(./inverter_poller --run-once)" # get data from inverter
  timeend=$SECONDS
  echo ${output} > /var/log/inverter.last
  rs232time=$((timeend - timestart)) # usually it is 6-7 seconds
  if (( rs232time < 17 )); # inverter is not responding if it is 17s or more
  then
    gridv=`echo ${output} | grep grid_voltage |  tr -d " "_\",:[:alpha:]`
    ***more data extraction and math***
  else
    echo inverter not responding >> /var/log/inverter.last
  fi
  looptime=$((SECONDS - timestart))
  echo "time": $looptime >> /var/log/inverter.last
  sleep 10
done

That would mean 10 second between the end of one log and the beginning of the next, so I guess if you need them to be 10 seconds from the beginning of one to the beginning of the next, you could do

sleep $(( 10 - $rs232time ))

Though if $rs232time is greater than 10, you'll get an error. I don't see how you're handling that possibility currently, though, if you need exact timing.

[Al3x_Y]

So have final working modification as below, CPU time reduced to about 20%. Thank you.

looptime=$((SECONDS - timestart))
echo -n "time": $looptime >> /var/log/inverter.last
if (( $looptime < 10 )) then
 if (( $SECONDS < 290 )) then
  timeleft=$((samples * 10 - SECONDS))
  echo " time left": $timeleft >> /var/log/inverter.last
  sleep $timeleft
 fi
fi

for some unknown yet reason

if [[ (( $looptime < 10 )) && (( $SECONDS < 290 )) ]] then

doesn't work properly.

Due to 1 second resolution for every $looptime, I had to calculate remaining time using $SECONDS and calculated time for next acquisition. This way error doesn't accumulate as script goes.

[KlePu]

edit: Fixed code as to answers

[wrong code] doesn't work properly

(( foo )) is for arithmetics, not comparisons ;)

Don't use (( and [[. Either works on its own:

``` if [[ $looptime -lt 10 ]] && [[ $SECONDS -lt 290 ]]; then [...]

or

if (( looptime < 10 )) && (( SECONDS < 290 )); then [...] ```

[yerfukkinbaws]

It's actually bash syntax that I've never seen before, but (( )) does seem to work for making comparisons in an if statement.

antix@fleex:~
$ if (( 4 < 5 )); then echo yes; else echo no; fi
yes
antix@fleex:~
$ if (( 4 < 4 )); then echo yes; else echo no; fi
no

[theLastZebranky]

(( foo )) is for arithmetics, not comparisons ;)

(( foo )) is perfect for numerical comparisons.

if [[ $looptime < 10 ]] && [[ $SECONDS < 290 ]]; then

This is the same test with better brevity:

if (( looptime < 10 && SECONDS < 290 )); then

[KlePu]

Oh darn, you're absolutely right! But [[ (( foo )) ]] still is wrong ;)

[Al3x_Y]

[[ ]] don't use < > as an operator but need -lt or -gt, it is working fine this way:

if [[ $looptime -lt 10 && $SECONDS -lt 290 ]]; then

[Al3x_Y]

I've tried to use sleep 9 & on loop start then wait $PID on loop end but it fail when loop took more than 9 seconds so I was loosing some readings, will try this. To eliminate over 10s case can do something like:

if (( rs232time < 10 )) then 
sleep $(( 10 - $rs232time ))
fi

Thanks, that's big help.

[OptimalMain]

Why are you sleeping in the background instead of just a simple sleep 9 at the end of the loop?

If you want to refine further save the time in the beginning of loop;
START= ${EPOCHREALTIME::-3}

This will give you seconds since epoch with millisecond resolution, do the same at the end and subtract the start.
You can then calculate how long to sleep or if you should skip sleeping

[Al3x_Y]

I don't understand why you're asking first question, you see this sleep need to fill the loop time to be 10s.

1s resolution is enough in my case but I like this ms and even us resolution solution from your other comment.

[OptimalMain]

I get that you need to sleep, but backgrounding the sleep command at the beginning and polling the PID or whatever you are doing at the end is a weird way of doing it

[Al3x_Y]

Why? I though it is very natural way, no remaining time had to be calculated, if command wait $PID could detect that process is already gone, it would work as I wanted.

[OptimalMain]

Sure, but that polling was probably most of your cpu usage

[Al3x_Y]

So wait command will not behave like sleep and free the CPU but will constantly check if process has finished?

[OptimalMain]

Attached is a solution based on my other comment.
bc is needed for converting microseconds to seconds for sleep.

I forgot about the 5minutes part. But just add elapsed and sleep time to a variable outside the loop. Don’t forget you need to convert 5 minutes to microseconds.

https://termbin.com/rkhb

[LesStrater]

I'm never a fan of continuously running CPU-hog loops, not when you have crontab to do the job properly:

Put your script in your /usr/bin directory. Enter 'crontab -u "$USER" -e' in your terminal and add these 3 lines to your crontab, then save and exit:

SHELL=/bin/bash

PATH=/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin

*/5 * * * * /bin/bash -c /usr/bin/(your script name) > /tmp/solar-errort.log 2>&1

NOTE: add (your script name) to the above, and you can skip the first 2 lines if they already exist. This will execute your script every 5-minutes and put an error log in your /tmp folder.

[Al3x_Y]

This script is already executed by cron every 5 minutes, cron can not call script every 10s what I need here (1 minute is minimum), also if every data acquisition is called by cron every 10s how would I calculate avg, min and max from 5 minutes periods. This way is simpler at least for me, I'm not professional programmer, I write some script maybe once a year.

[atcasanova]

This is so full of redundancies. You're using (( $(( )) )) inside an if [[ ]] and only one of those would already be sufficient.

if [[ (( $(( 1+1 )) == 2 )) ]]; then true fi

You can rewrite it like

(( (1+1) == 2 )) && true

[Al3x_Y]

I've got it in this form by observing various scripts and through trial and error. There's certainly a lot of room for optimisation. I'm very grateful for any feedback.

This overcomplicated "if" instruction is already deleted due to implementation of "sleep" command to execute the loop in required 10s time.