Bosse_B
Posts: 836
Joined: Thu Jan 30, 2014 9:53 am

script problem (again) - scratching my head...

Wed Jun 05, 2019 9:35 pm

I am trying to write a file copy script to handle video uploads to my Kodi media centers.
I want to tell my script to upload to my website (this works fine) and to none, one or both media centers using a second parameter.

This is the relevant part of my script dealing with this:

Code: Select all

#/bin/bash
# Read command line arguments
if [ -z "$1" ]; then
  echo "Please enter an existing file name as arg 1!"
  echo "Usage:"
  echo "putvideo <video filename> <target mediacenters>"
  exit
else
  FILENAME="$1"
  if [ ! -f "$FILENAME" ]; then
    echo "File $FILENAME not found!"
    exit
  fi
fi

if [ -z "$2" ]; then
  kodiflag="$2"
else
  kodiflag="0"
fi

....

#Now copy files to Kodi (only if flag is set)
if [ "$kodiflag" = "1" ] || [ "$kodiflag" = "3" ]; then
  echo "Copy the video file to Kodi"
  COMMAND="cp $FILENAME $KODIDISK/$DIRNAME/"
  $COMMAND
fi

if [ "$kodiflag" = "2" ] || [ "$kodiflag" = "3" ]; then
  echo "Copy to new Kodi too"
  COMMAND="cp $FILENAME $KODIDISK2/$DIRNAME/"
  $COMMAND
fi
But calling this script using parameters like this:

Code: Select all

putvideo inputfile.mp4 1
results in no copy at all even though I expected the first if to trigger...

Apparently the logic (assembled from reading posts via Google search) does not work but I don't know why...

Any ideas?
Bo Berglund
Sweden

swampdog
Posts: 266
Joined: Fri Dec 04, 2015 11:22 am

Re: script problem (again) - scratching my head...

Thu Jun 06, 2019 1:20 am

#!/bin/bash
^^^ no ! (bang) in yours.

if [ -z "$2" ]; then
kodiflag="$2"
^^^if nul then set nul?

I tend to use the same basic template..

Code: Select all

#!/bin/bash

NAM=`basename "$0"`

fcp_usage ()
{
cat<<EOF
${NAM}: [ --help | -h ]
${NAM}: [ start | 1 ]
${NAM}: [ stop | 0 ]
${NAM}: [ foo ] [arg1] [arg2]
EOF
}

fcp_1 ()
{
 echo "activate something"
}

fcp_0 ()
{
 echo "deactivate something"
}

fcp_foo ()
{
 [ $# -lt 2 ] && {
        fcp_usage 1>&2
        exit 1
 }
 echo "$@"
}

fcp_main ()
{
 case "$1" in
        start | 1)
        fcp_1
        ;;

        stop | 0)
        fcp_0
        ;;

        foo)
        shift
        fcp_foo "$@"
        ;;

        *)
        fcp_usage 1>&2
        exit 1
 esac
}

case "$1" in
        --help | -h)
        fcp_usage
        exit 0
        ;;

        *)
        fcp_main "$@"
        ;;
esac
It lends itself to quick modification..

Code: Select all

#!/bin/bash

NAM=`basename "$0"`

KODIDISK=("/tmp/one" "/tmp/two")

fcp_usage ()
{
cat<<EOF
${NAM}: [ --help | -h ]
${NAM}: [ video filename ] [ target mediacentre ]
EOF
}

fcp_cp ()
{
 echo cp "$@"
}

fcp_main ()
{
 local  filename="$1"
 local  kodiflag="$2"

 [ -z "$filename" ] || [ ! -f $filename ] && {
        echo "$NAM: No filename!" 1>&2
        fcp_usage 1>&2
        exit 1
 }

 if [ 1 == $(( kodiflag & 0x01 )) ]
 then
        fcp_cp "$filename" "${KODIDISK[0]}"
 fi
 if [ 2 == $(( kodiflag & 0x02 )) ]
 then
        fcp_cp "$filename" "${KODIDISK[1]}"
 fi
}

case "$1" in
        --help | -h)
        fcp_usage
        exit 0
        ;;

        *)
        fcp_main "$@"
        ;;
esac
Hope that helps.

hortimech
Posts: 331
Joined: Wed Apr 08, 2015 5:52 pm

Re: script problem (again) - scratching my head...

Thu Jun 06, 2019 8:29 am

Very nice changes, but it isn't portable, what if the OP is using dash ? The first problem that would be hit is that dash doesn't use arrays :)
Try this instead:

Code: Select all

#!/bin/bash

NAM=$(basename "$0")

KODIDISK1="/tmp/one"
KODIDISK2="/tmp/two"

fcp_usage ()
{
cat<<EOF
${NAM}: [ --help | -h ]
${NAM}: [ video filename ] [ target mediacentre ]
EOF
}

fcp_main ()
{
 local  filename="$1"
 local  kodiflag="$2"

 [ -z "$filename" ] && {
        echo "$NAM: No filename!" 1>&2
        fcp_usage 1>&2
        exit 1
 }

 [ ! -f "$filename" ] && {
        echo "$NAM: Incorrect filename!" 1>&2
        fcp_usage 1>&2
        exit 1
 }

 if [ 1 = $(( kodiflag & 0x01 )) ]; then
        cp "$filename" "${KODIDISK1}"
 fi
 if [ 2 = $(( kodiflag & 0x02 )) ]; then
        cp "$filename" "${KODIDISK2}"
 fi
}

case "$1" in
        --help | -h)
        fcp_usage
        exit 0
        ;;

        *)
        fcp_main "$@"
        ;;
esac
[?code]

Bosse_B
Posts: 836
Joined: Thu Jan 30, 2014 9:53 am

Re: script problem (again) - scratching my head...

Thu Jun 06, 2019 7:54 pm

swampdog wrote:
Thu Jun 06, 2019 1:20 am
#!/bin/bash
^^^ no ! (bang) in yours.
Didn't notice. Several of my scripts have this error but work anyway
if [ -z "$2" ]; then
kodiflag="$2"
^^^if nul then set nul?
Oh Gosh!
it should have been:

Code: Select all

if [ -z "$2" ]; then
  kodiflag="0"
else
  kodiflag="$2"
fi

Hope that helps.
I think you spotted two of my bugs.
I am now on a location where the Kodi boxes are not reachable so I will have to test some other way.
Back latest on Sunday.
Bo Berglund
Sweden

swampdog
Posts: 266
Joined: Fri Dec 04, 2015 11:22 am

Re: script problem (again) - scratching my head...

Thu Jun 06, 2019 9:57 pm

@hortimech

#!/bin/dash ? ;-)

Shell script portability is a nightmare because you're reduced to using bourne shell (aka #!/bin/sh) and once you get complex enough to be pulling in sub scripts it becomes a royal pita. There's so many pitfalls that its either best (a) not to bother (b) install the same shell across all systems. eg:

admin@pi05:/wrk/T $ file `which sh`
/bin/sh: symbolic link to dash

..which seems innocuous but opens a can of worms as you're running dash in 'sh' mode and there'll be subtle differences between it and systems which implement a standalone /bin/sh like the BSD's and Aix. We haven't even touched on command differences - "ps -ef" on bsd is not the same as on linux.

ShiftPlusOne
Raspberry Pi Engineer & Forum Moderator
Raspberry Pi Engineer & Forum Moderator
Posts: 6074
Joined: Fri Jul 29, 2011 5:36 pm
Location: The unfashionable end of the western spiral arm of the Galaxy

Re: script problem (again) - scratching my head...

Thu Jun 06, 2019 10:14 pm

Hope this doesn't derail the thread, but I'd just like to suggest 'shellcheck', which is a brilliant tool that finds most of the mistakes people make in shell scripts, including portability issues.

hortimech
Posts: 331
Joined: Wed Apr 08, 2015 5:52 pm

Re: script problem (again) - scratching my head...

Fri Jun 07, 2019 6:58 am

Yes , I always run my scripts through shellcheck, that's why it was '/bin/dash', I suppose I could have used '/bin/sh' instead, the result would have been the same, the suggested script wasn't portable.

Why did I set it to '/bin/dash' ?
Just to test the script, my default shell is '/bin/bash' so 'as is' the script had a few problems (backticks are so last year etc), but change the shebang to '/bin/dash' and shellcheck tells you that arrays are a no-no
If it was a complex script, then using arrays may have been needed, but it was such a simple script, that they were not needed.

Bosse_B
Posts: 836
Joined: Thu Jan 30, 2014 9:53 am

Re: script problem (again) - scratching my head...

Fri Jun 07, 2019 10:40 pm

hortimech wrote:
Fri Jun 07, 2019 6:58 am
Yes , I always run my scripts through shellcheck
No such command on RPi2, RPi3 nor Ubuntu18...
So must be installed.
Is it worth it for a very casual scripter?
Bo Berglund
Sweden

hortimech
Posts: 331
Joined: Wed Apr 08, 2015 5:52 pm

Re: script problem (again) - scratching my head...

Sat Jun 08, 2019 6:53 am

Well, no and yes :D
You don't actually need it, if your script works for you, then you are okay without it, but if you want to learn how to use shell script better, then it can point you to any errors.

swampdog
Posts: 266
Joined: Fri Dec 04, 2015 11:22 am

Re: script problem (again) - scratching my head...

Fri Jun 14, 2019 8:23 pm

'shellcheck' does look quite useful. I'm amazed I've gone all these years without knowing of it. It can't do any harm to run it and because you can disable warnings to taste you can see the wood for the trees. In my case, SC2006, depreciated backticks rather than $(..), and it's not to know I deliberately don't quote variables which hold numbers.

Return to “General discussion”