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
-
\$\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\$janos– janos2016年07月06日 05:39:54 +00:00Commented 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\$dbmitch– dbmitch2016年07月06日 16:20:43 +00:00Commented Jul 6, 2016 at 16:20
-
\$\begingroup\$ Revised code and review at codereview.stackexchange.com/questions/134233/… \$\endgroup\$dbmitch– dbmitch2018年10月18日 18:02:36 +00:00Commented Oct 18, 2018 at 18:02
1 Answer 1
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.
-
1\$\begingroup\$ Thanks so much. I'm going to print off your reply and have a closer look. Really appreciated. \$\endgroup\$dbmitch– dbmitch2016年07月05日 20:11:03 +00:00Commented 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\$dbmitch– dbmitch2016年07月06日 00:02:19 +00:00Commented 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\$janos– janos2016年07月06日 05:43:54 +00:00Commented 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\$dbmitch– dbmitch2016年07月06日 22:24:53 +00:00Commented Jul 6, 2016 at 22:24