2
\$\begingroup\$

Very new to Raspberry Pi and bash, but working on a headless server that will run a streaming app from USB audio card.

The plan is to take a USB card with settings file, and when inserted, the usb will automount, check for settings file, load the streamer using config file and wait for the audio input stream to start

All I have to date is generic udev rules and mount/unmount scripts ready to add code to see if the config file exists and start processing. I heavily borrowed ideas, code and udev manual SuperUser Answer for udev as well as the Mount Manager example

Many examples used serial numbers and volume labels - I do NOT want anything hardcoded. The goal was to be able to mount any loaded USB drive on to a script defined /media/ and use same script to unmount it if it was removed.

This has only been tested on Raspberry Pi 3 running Raspbian Jessie Lite. The only prep is to create the log and script folders defined by the config parameters at the top of the init script (and chmod 0755 scripts(

Barebones comments so far, but building documentation as I go along.

Any feedback, warnings and suggestions are much appreciated. I am very new at this and have been relying on others' examples and many man pages

/etc/udev/rules.d/85-usb-loader.rules

# /etc/udev/rules.d/85-usb-loader.rules
# ADD rule: 
# if USB inserted, 
# and flash drive loaded as sd#
# pass on dev id and device formatting type
# run short script to fork another processing script
# run script to initiate another script (first script must finish quickly)
# to mkdir and mount, process file
#
# REMOVE rule: 
# if USB removed, 
# unmount devices loaded by ADD rule
# remove mount folder created by ADD rule 
# 
# ** NOTE ** only add initial action parameter to ADD rule
#
# reload rules on PI by: 
# > sudo udevadm control -R
#
ACTION=="add", KERNEL=="sd*[0-9]", SUBSYSTEMS=="usb", RUN+="/home/pi/scripts/usb-initloader.sh ADD %k $env{ID_FS_TYPE}"
ACTION=="remove", KERNEL=="sd*[0-9]", SUBSYSTEMS=="usb", RUN+="/home/pi/scripts/usb-initloader.sh %k"

/home/pi/scripts/usb-initloader.sh

#!/bin/bash
# /home/pi/scripts/usb-initloader.sh
#
# make sure to chmod 0755 on these scripts
#
# script that runs when usb device is inserted
# checks to see if file is on USB drive and copies it over
# eventually run a python script that loads the config file
#
# ** DEVICE ADDED **
# should be called from a udev rule like:
# passes kernel device and filesystem type
# ACTION=="add", KERNEL=="sd*[0-9]", SUBSYSTEMS=="usb", RUN+="/home/pi/scripts/usb-initloader.sh ADD %k %E{key}"
# Mounts usb device on /media/<dev>
# Logs changes to /var/log/syslog
# use tail /var/log/syslog to look at latest events in log
#
# ** DEVICE REMOVED **
# or remove - where we only need the kernel device
# should be called from a udev rule like:
# ACTION=="remove", KERNEL=="sd*[0-9]", SUBSYSTEMS=="usb", RUN+="/home/pi/scripts/usb-initloader.sh %k"
#
#
# CONFIGURATION
#
LOG_DIR=/home/pi/logs
LOG_FILE="${LOG_DIR}/usb-automount.log"
MOUNT_DIR=/media
# Call new script and leave this one (with trailing "&")
if [ "1ドル" == "ADD" ]; then
 DEVICE="2ドル" # USB device name (formed with kernel passed from rule)
 DEVTYPE="3ドル" # USB device formatting type
 /home/pi/scripts/usb-automount.sh "$LOG_FILE" "$MOUNT_DIR" "$DEVICE" "$DEVTYPE" &
else
 DEVICE="1ドル" # USB device name (formed with kernel passed from rule)
 /home/pi/scripts/usb-unloader.sh "$LOG_FILE" "$MOUNT_DIR" "$DEVICE" &
fi

/home/pi/scripts/usb-unloader.sh

#!/bin/bash
# /home/pi/scripts/usb-unloader.sh
# Called from /home/pi/scripts/usb-initloader.sh
# make sure to chmod 0755 on file
# UnMounts usb device on /media/<device>
# Logs changes to /var/log/syslog and local log folder
# use tail /var/log/syslog to look at latest events in log
# CONFIGURATION
LOG_FILE="1ドル"
MOUNT_DIR="2ドル"
DEVICE="3ドル" # USB device name (from kernel parameter passed from rule)
# check input
if [ -z "$LOG_FILE" ]; then
 exit 1
fi
if [ -z "$MOUNT_DIR" ]; then
 exit 1
fi
if [ -z "$DEVICE" ]; then
 exit 1
fi
dt=$(date '+%d/%m/%Y %H:%M:%S')
echo "--- USB Auto UnLoader --- $dt" >> $LOG_FILE
sudo umount "/dev/$DEVICE" >> $LOG_FILE 2>&1
sudo rmdir "$MOUNT_DIR/$DEVICE" >> $LOG_FILE 2>&1
device_is_mounted=`grep ${DEVICE} /etc/mtab`
if [ "$device_is_mounted" == "" ]; then
 echo "/dev/${DEVICE} successfully UnMounted" >> $LOG_FILE 2>&1
fi

/home/pi/scripts/usb-automount.sh

#!/bin/sh
#
# Script: /home/pi/scripts/usb-automount.sh
# make sure to chmod 0755 on this script
# In case process of mounting takes too long for udev
# we call this script from /home/pi/scripts/usb-initloader.sh
#
# USAGE: usb-automount.sh DEVICE FILESYSTEM
# DEVICE is the actual device node at /dev/DEVICE (returned by udev rules %k parameter)
# FILESYSTEM is the FileSystem type returned by rules (returned by udev rules %E{ID_FS_TYPE} or $env{ID_FS_TYPE}
#
# This script takes a device name, file system type, creates /media/DEVICE and mounts the partition. 
#
# Adapted for Raspberry Pi - Raspbian O/S
# from previous code found at:
# https://superuser.com/questions/53978/automatically-mount-external-drives-to-media-label-on-boot-without-a-user-logge
# and mount manager example at
# http://solvedforhome.com/?p=2806&v=3a52f3c22ed6
LOG_FILE="1ドル"
MOUNT_DIR="2ドル"
DEVICE="3ドル" # USB device name (from kernel parameter passed from rule)
FILESYSTEM="4ドル"
# check input
if [ -z "$LOG_FILE" ]; then
 exit 1
fi
if [ -z "$MOUNT_DIR" ]; then
 exit 1
fi
if [ -z "$DEVICE" ]; then
 exit 1
fi
if [ -z "$FILESYSTEM" ]; then
 exit 1
fi
# Allow time for device to be added
sleep 1
dt=$(date '+%d/%m/%Y %H:%M:%S')
echo "--- USB AutoLoader --- $dt" >> $LOG_FILE
# test that this device isn't already mounted
device_is_mounted=`grep ${DEVICE} /etc/mtab`
if [ -n "$device_is_mounted" ]; then
 echo "Error: seems /dev/${DEVICE} is already mounted" >> $LOG_FILE 2>&1
 exit 1
fi
# test mountpoint - it shouldn't exist
if [ ! -e "$MOUNT_DIR/${DEVICE}" ]; then
 # make the mountpoint
 sudo mkdir "$MOUNT_DIR/${DEVICE}" >> $LOG_FILE 2>&1
 # make sure the Pi user owns this folder
 sudo chown -R pi:pi "$MOUNT_DIR/${DEVICE}" >> $LOG_FILE 2>&1
 # mount the device base on USB file system
 case "$FILESYSTEM" in
 vfat) sudo mount -t vfat -o utf8,uid=pi,gid=pi "/dev/${DEVICE}" "$MOUNT_DIR/${DEVICE}" >> $LOG_FILE 2>&1
 ;;
 # use locale setting for ntfs
 ntfs) sudo mount -t auto -o uid=pi,gid=pi,locale=en_US.UTF-8 "/dev/${DEVICE}" "$MOUNT_DIR/${DEVICE}" >> $LOG_FILE 2>&1
 ;;
 # ext2/3/4 do not like uid option
 ext*) sudo mount -t auto -o sync,noatime "/dev/${DEVICE}" "$MOUNT_DIR/${DEVICE}" >> $LOG_FILE 2>&1
 ;;
 esac
 # Allow time for device to be mounted
 sleep 2
 device_is_mounted=`grep ${DEVICE} /etc/mtab`
 if [ -n "$device_is_mounted" ]; then
 echo "/dev/${DEVICE} successfully mounted" >> $LOG_FILE 2>&1
 fi
 # all done here, return successful
 exit 0
fi
exit 1
asked Jul 3, 2016 at 3:14
\$\endgroup\$
3
  • \$\begingroup\$ Hi, I rolled back to the previous version. It is our policy to roll back changes that invalidate answers. If you would like another round of code review of the revised code, the recommended practice is to post that as a new question, linking back to the previous version, with a brief summary of what has changed \$\endgroup\$ Commented Jul 6, 2016 at 5:39
  • \$\begingroup\$ Ahh okay - that makes sense. I'll do that. I'm going to add some file processing to it to extend it a bit. \$\endgroup\$ Commented Jul 6, 2016 at 16:20
  • \$\begingroup\$ Revised code and review at codereview.stackexchange.com/questions/134233/… \$\endgroup\$ Commented Oct 18, 2018 at 18:02

1 Answer 1

1
\$\begingroup\$

This is really nice Bash code, you're off to a really great start!

Repeated >> $LOG_FILE 2>&1

The first thing that catches the eye is the repeated redirections. Not only this is not fun to write, a lot of noise to read, and when you add something to the script later, you might forget to consistently add the redirection.

One easy solution is to wrap the entire code in a { ... } block, for example:

{
 dt=$(date '+%d/%m/%Y %H:%M:%S')
 echo "--- USB AutoLoader --- $dt"
 # test that this device isn't already mounted
 device_is_mounted=`grep ${DEVICE} /etc/mtab`
 if [ -n "$device_is_mounted" ]; then
 echo "Error: seems /dev/${DEVICE} is already mounted"
 exit 1
 fi
 # ... and so on ...
} >> $LOG_FILE 2>&1

Another, a bit cleaner solution is to wrap the code in a proper function, for example:

automount() {
 dt=$(date '+%d/%m/%Y %H:%M:%S')
 echo "--- USB AutoLoader --- $dt"
 # test that this device isn't already mounted
 device_is_mounted=`grep ${DEVICE} /etc/mtab`
 if [ -n "$device_is_mounted" ]; then
 echo "Error: seems /dev/${DEVICE} is already mounted"
 exit 1
 fi
 # ... and so on ...
}
automount >> $LOG_FILE 2>&1

Guard statements

The beginning of usb-automount.sh uses guard statements consistently, for example:

if [ -n "$device_is_mounted" ]; then
 echo "Error: seems /dev/${DEVICE} is already mounted" >> $LOG_FILE 2>&1
 exit 1
fi
# ... normal execution continues here ...

But then you break that pattern here:

# test mountpoint - it shouldn't exist
if [ ! -e "$MOUNT_DIR/${DEVICE}" ]; then
 # ...
 # all done here, return successful
 exit 0
fi
exit 1

The earlier pattern is better. It's consistent with the code above it, and it reduces the indentation level, resulting in flatter code that's often easier to read. That is, like this:

# test mountpoint - it shouldn't exist
if [ -e "$MOUNT_DIR/${DEVICE}" ]; then
 exit 1
fi
# ... normal execution continues here ...

In this case you don't even need the exit 0 at the end of the script, as the exit code of the script will be the exit code of the last statement.

Use $(...) instead of `...`

$(...) is the modern, recommended syntax, use it everywhere consistently.

That is, instead of this:

device_is_mounted=`grep ${DEVICE} /etc/mtab`

Write like this:

device_is_mounted=$(grep ${DEVICE} /etc/mtab)

Use the exit code of commands directly

A really cool thing about Bash conditionals is that they can use the exit code of a command directly. For example instead of this:

device_is_mounted=`grep ${DEVICE} /etc/mtab`
if [ -n "$device_is_mounted" ]; then
 echo "Error: seems /dev/${DEVICE} is already mounted" >> $LOG_FILE 2>&1
 exit 1
fi

You can write like this:

if grep -q ${DEVICE} /etc/mtab; then
 echo "Error: seems /dev/${DEVICE} is already mounted" >> $LOG_FILE 2>&1
 exit 1
fi

This works, because grep exits with success if it matched something. The -q flag is to make it quiet, otherwise the matched line would be part of the output, which you don't need and would be just noise.

Use more functions

The logic of checking if the device is mounted is repeated twice, and it's not exactly trivial to write. In such situations it's always good to introduce a helper function, for example:

is_mounted() {
 grep -q "1ドル" /etc/mtab
}

Notice how the function doesn't return anything explicitly: the exit code of the last command, in this case the grep, becomes the exit code of the function, and you can use the function in an if statement just like the grep itself earlier:

if is_mounted ${DEVICE}; then
 echo "Error: seems /dev/${DEVICE} is already mounted" >> $LOG_FILE 2>&1
 exit 1
fi

Indentation

It's a minor thing, but about halfway through usb-automount.sh the indentation is slightly off. It's good to keep it consistent.

answered Jul 5, 2016 at 19:36
\$\endgroup\$
4
  • 1
    \$\begingroup\$ Thanks so much. I'm going to print off your reply and have a closer look. Really appreciated. \$\endgroup\$ Commented Jul 5, 2016 at 20:11
  • \$\begingroup\$ doing some editing now and REALLY like the ideas of removing the repeated redirections. Besides being easier to read it helps me understand how bash functions are constructed and used. One clarification - when you say to use the more modern $(...) inseatd of quotes - would that being like when I was using "$DEVICE" instead of ${DEVICE}? \$\endgroup\$ Commented Jul 6, 2016 at 0:02
  • \$\begingroup\$ I added an example to clarify. My comment concerns subshells, ${...} is something entirely different, it's variable substitution, and there is no strong recommendation regarding that \$\endgroup\$ Commented Jul 6, 2016 at 5:43
  • \$\begingroup\$ Had to post a question in StackOverflow - I can't seem to get the function defined properly using your suggestion even though it passes ShellCheck. Get error is on line 24 syntax error near unexpected token ${\r'' autounload() { - right where I define the function - anyways. it's at stackoverflow.com/questions/38234756/bash-syntax-error \$\endgroup\$ Commented Jul 6, 2016 at 22:24

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.