0

I used the solution from this answer in a script I'm working on, but it doesn't seem to be working for me. I'm on Bash 5. Here's a simplified test case, I would expect the do_thing function to be called at minutes that are multiples of 5 (except when $min and $minutes are the same, since in the real script I have do_thing triggering at the start of the main loop), but running it also triggers on those multiples + 1, so clearly I have a little off-by-one or math error somewhere but I'm not seeing it. Any ideas?

#!/usr/bin/env bash

minutes=6
sec=00
min="$minutes"

do_thing() {
  echo 'done thing'
}

while [ "$min" -ge 0 ]; do
  remainder=$(( min % 5 ))
  # TODO: this is broken
  [ "$remainder" -eq 0 ] && [ "$min" -ne "$minutes" ] && do_thing
  while [ $sec -ge 0 ]; do
    echo "$min $sec"
    ((sec=sec-1))
    sleep 1
  done
  sec=59
  ((min=min-1))
done
4
  • 1
    what do you mean by it triggers multiple + 1? it triggers 5:59 and 0:59 since you count down seconds only after do_thing... (put echo done thing $min $sec then you know) Aug 5, 2020 at 18:42
  • As @frostschutz says, you need to call do_thing() when [[ $sec == "0" ]]. Or rewrite it to count down in seconds. If the inner loop is just a busy-wait, why not just sleep 300?
    – Rich
    Aug 5, 2020 at 19:13
  • @Rich the full script accepts user input in minutes, it's a meditation timer where do_thing is a function that plays a bell in the background.
    – Zac Anger
    Aug 5, 2020 at 21:08
  • 🤦 thanks, both of you, I don't know how I missed that. @frostschutz if you put your comment in an answer, I can mark it as accepted.
    – Zac Anger
    Aug 5, 2020 at 21:13

1 Answer 1

2

First thing to debugging any bash script is to add set -x at the very top. This will let you see what the script is doing, what each test looks like on each iteration of a loop, etc.

This is where I'd put it:

#!/usr/bin/env bash

# NOTE: Remove or comment the below line when you're done debugging!
set -x

minutes=6
sec=00
...

Sprinkling echo "myvar = $myvar" throughout your code while you're debugging can also help you to track a variable's value at key points in your script so you can compare it to what you think it should be.

The flaw in your logic is that once you get the condition that lets you out of the "minute counter" loop, you're immediately reducing min by 1 so when you recalculate the remainder you're always off by one. You're decrementing your "counter" before you use it for the calculation you want!

Logically, you want to test for seconds = 0, then calculate remainder, then set up for the next loop:

#!/usr/bin/env bash

#set -x

minutes=6
sec=00
min="$minutes"

do_thing() {
  echo 'done thing'
}

while [ "$min" -ge 0 ]; do
  while [ $sec -ge 0 ]; do
    echo "$min $sec"
    ((sec=sec-1))
    sleep 1
  done
  remainder=$(( min % 5 ))
  # Added echo for debugging
  echo "remainder = $remainder"
  [ "$remainder" -eq 0 ] && [ "$min" -ne "$minutes" ] && do_thing
  sec=59
  ((min=min-1))
done

One other thing stands out here - Be very careful with && and || - they are nice for quick and dirty, but for complex logic they can fool you!

These logical operators are not a substitute for a proper test, even though they behave that way sometimes - one difference from using for example an if []; then... test is that they consume your exit code silently. Read up on them, and don't use more than one of them consecutively without also using () to enforce your desired order of operations.

If you want to take my advice and avoid && here, here's one way (a nested `if is another, but I prefer not to use those if I don't have to):

  ...
  remainder=$(( min % 5 ))
  # Added echo for debugging
  echo "remainder = $remainder"
  if ([ "$remainder" -eq 0 ] && [ "$min" -ne "$minutes" ]); then
    do_thing
  fi
  sec=59
  ((min=min-1))
  ...
3
  • Thanks! I always forget about set -x, even though I've been using Linux for close to a decade. The tip about using () to force order of operations is also good.
    – Zac Anger
    Aug 6, 2020 at 1:14
  • You're welcome! :) Aug 6, 2020 at 15:50
  • 1
    I used to think Bash was not a "proper" programming language since it's not intended to be compiled, but it's truly an incredibly powerful programming tool. I even hear about seasoned Bash veterans who are still learning new facets of the shell. Aug 6, 2020 at 16:07

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .