6

I'm getting nowhere with this.

I simply want to make a bash script that loops 10 times and breaks if the output contains the string "connection successful".

I tried something like ret=$? and then if [$ret -ne 0] but I kept getting the else statement happen, even if ret is 1.

So now I'm trying to use grep to search for the words "connection successful", but I don't know how to use the syntax for that.

So I want something like:

for i in {1..10}
do
  ret=bluetoothctl connect 26:EE:F1:58:92:AF | grep "connection successful"

  if [$ret -ne ""]; then
    break
  fi
done

but obviously with the right syntax for $ret=bluetoothctl connect 26:EE:F1:58:92:AF | grep "connection successful"

Any help would be greatly appreciated.

2
  • Spaces are required around [ and ]. Remember, [ is just an ordinary command, not shell syntax, so you need to separate it from its arguments with a space. [$ret would be like echo$ret.
    – Barmar
    Commented Aug 4, 2022 at 14:34
  • I'm not sure why you want to try 10 times and stop. If you just want to keep trying until the command is successful you can use the until keyword.
    – vschum
    Commented Aug 5, 2022 at 17:39

2 Answers 2

17

It is rarely necessary to store the output from grep when you just want to test whether some text matches a pattern. Instead, just invoke grep with its -q option and act on its exit status:

#!/bin/sh

tries=10

while [ "$tries" -gt 0 ]; do
    if bluetoothctl connect '26:EE:F1:58:92:AF' | grep -q 'connection successful'
    then
        break
    fi

    tries=$(( tries - 1 ))
done

if [ "$tries" -eq 0 ]; then
    echo 'failed to connect' >&2
    exit 1
fi

If bluetoothctl returns a sane exit status on failure and success, then you don't even need grep and can shorten the if-statement in the loop into the following:

if bluetoothctl connect '26:EE:F1:58:92:AF' >/dev/null
then
    break
fi

In fact, you might as well make bluetoothctl part of the loop condition (assuming you switch from a for loop to using a while loop like I'm showing here):

#!/bin/sh

tries=10

while [ "$tries" -gt 0 ] && ! bluetoothctl connect '26:EE:F1:58:92:AF'
do
    tries=$(( tries - 1 ))
done >/dev/null

if [ "$tries" -eq 0 ]; then
    echo 'failed to connect' >&2
    exit 1
fi

Consider using https://www.shellcheck.net to verify the syntax of your shell scripts. For the script in the question, it would point out that the test needs spaces within the [ ... ]:

  if [$ret -ne ""]; then
  ^-- SC1009 (info): The mentioned syntax error was in this if expression.
     ^-- SC1035 (error): You need a space after the [ and before the ].
     ^-- SC1073 (error): Couldn't parse this test expression. Fix to allow more checks.
                  ^-- SC1020 (error): You need a space before the ].
                  ^-- SC1072 (error): Missing space before ]. Fix any mentioned problems and try again.

Testing for a non-empty string is done with [ -n "$ret" ] or [ "$ret" != "" ]. Note that -ne is an arithmetic test.

You also do not assign the output of the pipeline correctly to ret. Unfortunately, the syntax is absolutely correct but does something completely different, so ShellCheck would not pick up on it. What you intended to use was

ret=$( bluetoothctl ... | grep ... )
9

Kusalananda gave tips on how to improve the code, but didn't exactly explain what you did wrong. Let's go over that:

I tried something like ret=$? and then if [$ret -ne 0] but I kept getting the else statement happen, even if ret is 1.

[ is a command, so you need to put a space between the command and its arguments, else the argument is going to be understood to be part of the command name. If ret is set to 1, bash should output the error [1: command not found, and if should see falsy because of the error. Similarly, [ requires the last argument to be ], and 0] isn't that. You also need a space there.

What you wanted was:

if [ "$ret" -ne 0 ]

Next part:

ret=bluetoothctl connect 26:EE:F1:58:92:AF | grep "connection successful"

What you're doing here is running the pipe connect 26:EE:F1:58:92:AF | grep "connection successful", while providing the connect command (which probably doesn't exist) the environment variable ret with value bluetoothctl.

In order to collect its output, you need to enclose the command in $():

  ret="$(bluetoothctl connect 26:EE:F1:58:92:AF | grep "connection successful")"

About this:

  if [$ret -ne ""]; then

You have the same issue of lacking spaces, but something further to understand is that basically most things are strings implicitly in bash, quotes are mostly just a way to be more explicit about it (besides also controlling some details on expansions). That means that [$ret -ne ""] evaluates pretty much exactly the same as the followings:

[$ret -ne ]
[$ret -ne ""''""'']
"["$ret "-ne" """]"
"["$ret"" "-ne""""" """]"""

which with $ret being empty, would equate to [ -ne ]. That evaluates to truthy because with just one argument (ignoring ]), [ checks whether it's a non-empty string, and -ne is.

Were $ret to be something else like foo bar baz, it'd be word-split because of the lack of quotes and you'd get the command [foo bar baz -ne ], which would have 4 arguments (including ]) and be falsy because bash wouldn't find the command [foo. Had you quoted $ret like ["$ret" -ne ""], then you'd get the equivalent of "[foo bar baz" -ne ], which would have 2 arguments (including ]) and also be falsy because bash wouldn't find the command [foo bar baz.

Another mistake you did here was using -ne, which is exclusively for numeric comparison. Even if you fixed the spacing and quoting issue, bash would give you the error integer expression expected. What you wanted was !=, which is for string comparison.

To sum it up and fix all that, what you wanted here was:

if [ "$ret" != "" ]; then

which can also be written as

if [ -n "$ret" ]; then

or

if [ "$ret" ]; then
4
  • [ can test for an empty string with -z - if [ -z "$ret" ].
    – minnmass
    Commented Aug 4, 2022 at 13:36
  • @minnmass Yes, but there's more than that to improve. It's better not to check the output like that but rather like how Kusalanda did it. I'm just focusing on explaining the mistakes, and providing their most direct fixes.
    – JoL
    Commented Aug 5, 2022 at 0:30
  • @JoL: I'm with you 100% that this was an X/Y question, and the solution you provided is better. But there are cases where checking for an empty string is the right thing (checking for empty variables comes to mind), so knowing how to do the thing asked about in the question can still be helpful. And, since part of the original question was "how do I check for an empty string", explicitly answering that in a POSIX-standard (and, IMO, more idiomatic) way is helpful.
    – minnmass
    Commented Aug 5, 2022 at 0:41
  • @minnmass Just so you know, != is also POSIX, so that's not really a valid argument. They also didn't ask how to check for an empty string; it's just one of the things they made a mistake on. I'll give you that != "" is slightly less idiomatic. I've added the use of -n (because that's what they need rather than -z) and [ "$ret" ] as alternatives.
    – JoL
    Commented Aug 5, 2022 at 2:40

You must log in to answer this question.

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