patternshellMinor
Shell script to detect router slider status
Viewed 0 times
scriptroutersliderstatusshelldetect
Problem
I have an openwrt router (TP-Link MR3040) and on boot I have it check the slider (AP, WISP, 3G/4G mode). The goal was to have it execute the current "slider status" vs the old one, or do nothing if it's the same (before reboot). I just want a critique on making it more elegant.
#!/bin/sh
test=$(cat /root/logs/sliderstatus)
status=""
if grep -qe "sw1.*in hi" /sys/kernel/debug/gpio ; then
if grep -qe "sw2.*in hi" /sys/kernel/debug/gpio ; then
# AP
logger "Configure AP"
status="AP"
else
# WISP
logger "Configure WISP"
status="CLIENT"
fi
else
# 3G
logger "Configure 3G"
status="CUSTOM"
fi
if [ $status != $test ] ; then
case $status in
CLIENT) echo "status is client, executing now"
#sh /root/scripts/shell/CLIENT_MODE.sh &
;;
AP) echo "status is AP, executing now"
#sh /root/scripts/shell/AP_MODE.sh &
;;
CUSTOM) echo "status is CUSTOM, executing now"
;;
*) echo "ERROR status does not match ERROR"
;;
esac
elif [ $status == $test ] ; then
echo " Status: $status and Slider $test are the same"
else
echo "You shouldn't have seen this"
fiSolution
First of all, there are so many pitfalls associated with
If you write the script in a better scripting language you can get rid of the duplicate call to
If you must write in /bin/sh, then run your code through one of the following static analyzers to help you find potential coding problems:
For instance, ShellCheck found these issues:
In perl your script would look like this:
It's still basically the same code, but has these advantages:
Finally, I have a question about your patterns for
/bin/sh programming that I prefer to write all except the most trivial scripts in a language like perl, python or even awk. I realize that availability is a concern, but all of those languages are pretty standard now.If you write the script in a better scripting language you can get rid of the duplicate call to
grep which is one thing that I presume bothers you about the code.If you must write in /bin/sh, then run your code through one of the following static analyzers to help you find potential coding problems:
- http://www.shellcheck.net
checkbashismson Debian / Ubuntu systems
For instance, ShellCheck found these issues:
if [ $status != $test ] ; then
^---- should use "$test"
elif [ $status == $test ] ; then
^---- == not supported in POSIX shIn perl your script would look like this:
#!/usr/bin/env perl
use strict;
use warnings;
use File::Slurp;
my $test = read_file( '/root/logs/sliderstatus' );
chomp $test;
my ($found1, $found2);
open(my $fh, ") {
if (m/\(sw1\s*\)\s+in\s+hi/) { $found1 = 1 }
if (m/\(sw2\s*\)\s+in\s+hi/) { $found2 = 1 }
}
close($fh);
my ($status, $script);
if ($found1 && $found2) {
$status = "AP";
$script = "/root/scripts/shell/AP_MODE.sh";
} elsif ($found1) {
$status = "CLIENT";
$script = "/root/scripts/shell/CLIENT_MODE.sh";
} else {
$status = "CUSTOM";
# don't set $script
}
if ($status ne $test) {
print "Status is $status, executing now.\n";
if ($script) {
system("$script &");
}
} else {
print "Status $status and slider $test are the same.\n";
}It's still basically the same code, but has these advantages:
- It only reads
/sys/kernel/debug/gpioonce
- It's a lot safer
- It's easier to express more complex logic
- You have more versatile data structures available to you - e.g. real arrays and hash maps.
Finally, I have a question about your patterns for
sw1 and sw2. What does the output of /sys/kernel/debug/gpio look like? Presumably the inputs are number something like sw1, sw2, sw3, etc. Could there be a sw10? There should be a delimiter in the output you can use to make sure you are matching the full switch number.Code Snippets
if [ $status != $test ] ; then
^---- should use "$test"
elif [ $status == $test ] ; then
^---- == not supported in POSIX sh#!/usr/bin/env perl
use strict;
use warnings;
use File::Slurp;
my $test = read_file( '/root/logs/sliderstatus' );
chomp $test;
my ($found1, $found2);
open(my $fh, "</sys/kernel/debug/gpio");
while (<$fh>) {
if (m/\(sw1\s*\)\s+in\s+hi/) { $found1 = 1 }
if (m/\(sw2\s*\)\s+in\s+hi/) { $found2 = 1 }
}
close($fh);
my ($status, $script);
if ($found1 && $found2) {
$status = "AP";
$script = "/root/scripts/shell/AP_MODE.sh";
} elsif ($found1) {
$status = "CLIENT";
$script = "/root/scripts/shell/CLIENT_MODE.sh";
} else {
$status = "CUSTOM";
# don't set $script
}
if ($status ne $test) {
print "Status is $status, executing now.\n";
if ($script) {
system("$script &");
}
} else {
print "Status $status and slider $test are the same.\n";
}Context
StackExchange Code Review Q#102343, answer score: 4
Revisions (0)
No revisions yet.