[4/5] fff-nodewatcher: Consolidate code in nodewatcher.d/10-systemdata.sh

Submitted by Adrian Schmutzler on Jan. 7, 2020, 1:40 p.m.

Details

Message ID 20200107134024.1755-4-freifunk@adrianschmutzler.de
State Accepted
Headers show

Commit Message

Adrian Schmutzler Jan. 7, 2020, 1:40 p.m. 
This consolidates the code in nodewatcher.d/10-systemdata.sh by:

- Slightly reordering data retrieval
- Moving XML node assembly to corresponding data retrieval, making
  the whole file easier to read
- Changing some if statements to shorter binary condition shortcuts
- Reduce the number of variables by merging some code into the XML
  node assembly

Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
---
 src/packages/fff/fff-nodewatcher/Makefile     |   2 +-
 .../usr/lib/nodewatcher.d/10-systemdata.sh    | 107 ++++++++----------
 2 files changed, 50 insertions(+), 59 deletions(-)

Patch hide | download patch | download mbox 

diff --git a/src/packages/fff/fff-nodewatcher/Makefile b/src/packages/fff/fff-nodewatcher/Makefile
index e4a07983..19dd9cd1 100644
--- a/src/packages/fff/fff-nodewatcher/Makefile
+++ b/src/packages/fff/fff-nodewatcher/Makefile
@@ -1,7 +1,7 @@ 
 include $(TOPDIR)/rules.mk
 
 PKG_NAME:=fff-nodewatcher
-PKG_RELEASE:=57
+PKG_RELEASE:=58
 
 PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
 
diff --git a/src/packages/fff/fff-nodewatcher/files/usr/lib/nodewatcher.d/10-systemdata.sh b/src/packages/fff/fff-nodewatcher/files/usr/lib/nodewatcher.d/10-systemdata.sh
index a61e1e57..f4a6980e 100755
--- a/src/packages/fff/fff-nodewatcher/files/usr/lib/nodewatcher.d/10-systemdata.sh
+++ b/src/packages/fff/fff-nodewatcher/files/usr/lib/nodewatcher.d/10-systemdata.sh
@@ -3,7 +3,7 @@ 
 # License; GPL v3
 
 SCRIPT_STATUS_FILE=$(uci get nodewatcher.@script[0].status_text_file)
-SCRIPT_VERSION="57"
+SCRIPT_VERSION="58"
 
 debug() {
 	(>&2 echo "$1")
@@ -11,28 +11,31 @@  debug() {
 
 debug "$(date): Collecting basic system status data"
 
+SYSTEM_DATA="<status>online</status>"
+
 hostname="$(cat /proc/sys/kernel/hostname)"
 mac=$(awk '{ mac=toupper($1); gsub(":", "", mac); print mac }' /sys/class/net/br-mesh/address 2>/dev/null)
 [ "$hostname" = "OpenWrt" ] && hostname="$mac"
 [ "$hostname" = "FFF" ] && hostname="$mac"
+SYSTEM_DATA="$SYSTEM_DATA<hostname>$hostname</hostname>"
+
 description="$(uci -q get fff.system.description)"
-if [ -n "$description" ]; then
-	description="<description><![CDATA[$description]]></description>"
-fi
+[ -n "$description" ] && SYSTEM_DATA="$SYSTEM_DATA<description><![CDATA[$description]]></description>"
+
 latitude="$(uci -q get fff.system.latitude)"
 longitude="$(uci -q get fff.system.longitude)"
 if [ -n "$longitude" -a -n "$latitude" ]; then
-	geo="<geo><lat>$latitude</lat><lng>$longitude</lng></geo>";
+	SYSTEM_DATA="$SYSTEM_DATA<geo><lat>$latitude</lat><lng>$longitude</lng></geo>"
 fi
+
 position_comment="$(uci -q get fff.system.position_comment)"
-if [ -n "$position_comment" ]; then
-	position_comment="<position_comment><![CDATA[$position_comment]]></position_comment>"
-fi
+[ -n "$position_comment" ] && SYSTEM_DATA="$SYSTEM_DATA<position_comment><![CDATA[$position_comment]]></position_comment>"
+
 contact="$(uci -q get fff.system.contact)"
-if [ -n "$contact" ]; then
-	contact="<contact>$contact</contact>"
-fi
+[ -n "$contact" ] && SYSTEM_DATA="$SYSTEM_DATA<contact>$contact</contact>"
+
 uptime=$(awk '{ printf "<uptime>"$1"</uptime><idletime>"$2"</idletime>" }' /proc/uptime)
+SYSTEM_DATA="$SYSTEM_DATA$uptime"
 
 memory=$(awk '
 	/^MemTotal/ { printf "<memory_total>"$2"</memory_total>" }
@@ -40,36 +43,34 @@  memory=$(awk '
 	/^Buffers/ { printf "<memory_buffering>"$2"</memory_buffering>" }
 	/^MemFree/ { printf "<memory_free>"$2"</memory_free>" }
 ' /proc/meminfo)
+SYSTEM_DATA="$SYSTEM_DATA$memory"
+
 cpu=$(awk -F': ' '
 	/model/ { printf "<cpu>"$2"</cpu>" }
 	/system type/ { printf "<chipset>"$2"</chipset>" }
 	/platform/ { printf "<chipset>"$2"</chipset>" }
 ' /proc/cpuinfo)
-model="<model>$(cat /var/sysinfo/model)</model>"
-local_time="$(date +%s)"
+SYSTEM_DATA="$SYSTEM_DATA$cpu"
+
+SYSTEM_DATA="$SYSTEM_DATA<model>$(cat /var/sysinfo/model)</model>"
+
+SYSTEM_DATA="$SYSTEM_DATA<local_time>$(date +%s)</local_time>"
+
 load=$(awk '{ printf "<loadavg>"$3"</loadavg><processes>"$4"</processes>" }' /proc/loadavg)
+SYSTEM_DATA="$SYSTEM_DATA$load"
 
 debug "$(date): Collecting version information"
 
-batman_adv_version=$(cat /sys/module/batman_adv/version)
-kernel_version=$(uname -r)
-if [ -x /usr/bin/fastd ]; then
-	fastd_version="<fastd_version>$(/usr/bin/fastd -v | awk '{ print $2 }')</fastd_version>"
-fi
-nodewatcher_version=$SCRIPT_VERSION
-if [ -x /usr/sbin/babeld ]; then
-	babel_version="<babel_version>$(/usr/sbin/babeld -V 2>&1)</babel_version>"
-fi
+SYSTEM_DATA="$SYSTEM_DATA<batman_advanced_version>$(cat /sys/module/batman_adv/version)</batman_advanced_version>"
+SYSTEM_DATA="$SYSTEM_DATA<kernel_version>$(uname -r)</kernel_version>"
+SYSTEM_DATA="$SYSTEM_DATA<nodewatcher_version>$SCRIPT_VERSION</nodewatcher_version>"
 
-if [ -f "$SCRIPT_STATUS_FILE" ]; then
-	status_text="<status_text>$(cat "$SCRIPT_STATUS_FILE")</status_text>"
+if [ -x /usr/bin/fastd ]; then
+	SYSTEM_DATA="$SYSTEM_DATA<fastd_version>$(/usr/bin/fastd -v | awk '{ print $2 }')</fastd_version>"
 fi
 
-# Checks if fastd is running
-if pidof fastd >/dev/null ; then
-	vpn_active="<vpn_active>1</vpn_active>"
-else
-	vpn_active="<vpn_active>0</vpn_active>"
+if [ -x /usr/sbin/babeld ]; then
+	SYSTEM_DATA="$SYSTEM_DATA<babel_version>$(/usr/sbin/babeld -V 2>&1)</babel_version>"
 fi
 
 # example for /etc/openwrt_release:
@@ -80,8 +81,8 @@  fi
 #DISTRIB_TARGET="atheros/generic"
 #DISTRIB_DESCRIPTION="OpenWrt Attitude Adjustment 12.09-rc1"
 . /etc/openwrt_release
-distname=$DISTRIB_ID
-distversion=$DISTRIB_RELEASE
+SYSTEM_DATA="$SYSTEM_DATA<distname>$DISTRIB_ID</distname>"
+SYSTEM_DATA="$SYSTEM_DATA<distversion>$DISTRIB_RELEASE</distversion>"
 
 # example for /etc/firmware_release:
 #FIRMWARE_VERSION="95f36685e7b6cbf423f02cf5c7f1e785fd4ccdae-dirty"
@@ -89,34 +90,24 @@  distversion=$DISTRIB_RELEASE
 #OPENWRT_CORE_REVISION="35298"
 #OPENWRT_FEEDS_PACKAGES_REVISION="35298"
 . /etc/firmware_release
+SYSTEM_DATA="$SYSTEM_DATA<firmware_version>$FIRMWARE_VERSION</firmware_version>"
+SYSTEM_DATA="$SYSTEM_DATA<firmware_revision>$BUILD_DATE</firmware_revision>"
+SYSTEM_DATA="$SYSTEM_DATA<openwrt_core_revision>$OPENWRT_CORE_REVISION</openwrt_core_revision>"
+SYSTEM_DATA="$SYSTEM_DATA<openwrt_feeds_packages_revision>$OPENWRT_FEEDS_PACKAGES_REVISION</openwrt_feeds_packages_revision>"
 
-SYSTEM_DATA="<status>online</status>"
-SYSTEM_DATA=$SYSTEM_DATA"$status_text"
-SYSTEM_DATA=$SYSTEM_DATA"<hostname>$hostname</hostname>"
-SYSTEM_DATA=$SYSTEM_DATA"${description}"
-SYSTEM_DATA=$SYSTEM_DATA"${geo}"
-SYSTEM_DATA=$SYSTEM_DATA"${position_comment}"
-SYSTEM_DATA=$SYSTEM_DATA"${contact}"
-SYSTEM_DATA=$SYSTEM_DATA"<hood>$(uci -q get "system.@system[0].hood")</hood>"
-SYSTEM_DATA=$SYSTEM_DATA"<hoodid>$(uci -q get "system.@system[0].hoodid")</hoodid>"
-SYSTEM_DATA=$SYSTEM_DATA"<distname>$distname</distname>"
-SYSTEM_DATA=$SYSTEM_DATA"<distversion>$distversion</distversion>"
-SYSTEM_DATA=$SYSTEM_DATA"$cpu"
-SYSTEM_DATA=$SYSTEM_DATA"$model"
-SYSTEM_DATA=$SYSTEM_DATA"$memory"
-SYSTEM_DATA=$SYSTEM_DATA"$load"
-SYSTEM_DATA=$SYSTEM_DATA"$uptime"
-SYSTEM_DATA=$SYSTEM_DATA"<local_time>$local_time</local_time>"
-SYSTEM_DATA=$SYSTEM_DATA"<batman_advanced_version>$batman_adv_version</batman_advanced_version>"
-SYSTEM_DATA=$SYSTEM_DATA"<kernel_version>$kernel_version</kernel_version>"
-SYSTEM_DATA=$SYSTEM_DATA"$fastd_version"
-SYSTEM_DATA=$SYSTEM_DATA"<nodewatcher_version>$nodewatcher_version</nodewatcher_version>"
-SYSTEM_DATA=$SYSTEM_DATA"$babel_version"
-SYSTEM_DATA=$SYSTEM_DATA"<firmware_version>$FIRMWARE_VERSION</firmware_version>"
-SYSTEM_DATA=$SYSTEM_DATA"<firmware_revision>$BUILD_DATE</firmware_revision>"
-SYSTEM_DATA=$SYSTEM_DATA"<openwrt_core_revision>$OPENWRT_CORE_REVISION</openwrt_core_revision>"
-SYSTEM_DATA=$SYSTEM_DATA"<openwrt_feeds_packages_revision>$OPENWRT_FEEDS_PACKAGES_REVISION</openwrt_feeds_packages_revision>"
-SYSTEM_DATA=$SYSTEM_DATA"$vpn_active"
+debug "$(date): Collecting hood information and additional status data"
+
+SYSTEM_DATA="$SYSTEM_DATA<hood>$(uci -q get "system.@system[0].hood")</hood>"
+SYSTEM_DATA="$SYSTEM_DATA<hoodid>$(uci -q get "system.@system[0].hoodid")</hoodid>"
+
+if [ -s "$SCRIPT_STATUS_FILE" ]; then
+	SYSTEM_DATA="$SYSTEM_DATA<status_text>$(cat "$SCRIPT_STATUS_FILE")</status_text>"
+fi
+
+# Checks if fastd is running
+vpn_active=0
+pidof fastd >/dev/null && vpn_active=1
+SYSTEM_DATA="$SYSTEM_DATA<vpn_active>$vpn_active</vpn_active>"
 
 echo -n "<system_data>$SYSTEM_DATA</system_data>"

Comments

Fabian Blaese April 18, 2020, 5:33 p.m. 
Hallo Adrian,

On 07.01.20 14:40, Adrian Schmutzler wrote:
> diff --git a/src/packages/fff/fff-nodewatcher/files/usr/lib/nodewatcher.d/10-systemdata.sh b/src/packages/fff/fff-nodewatcher/files/usr/lib/nodewatcher.d/10-systemdata.sh
> index a61e1e57..f4a6980e 100755
> --- a/src/packages/fff/fff-nodewatcher/files/usr/lib/nodewatcher.d/10-systemdata.sh
> +++ b/src/packages/fff/fff-nodewatcher/files/usr/lib/nodewatcher.d/10-systemdata.sh
> [..]
> -batman_adv_version=$(cat /sys/module/batman_adv/version)
> -kernel_version=$(uname -r)
> -if [ -x /usr/bin/fastd ]; then
> -	fastd_version="<fastd_version>$(/usr/bin/fastd -v | awk '{ print $2 }')</fastd_version>"
> -fi
> -nodewatcher_version=$SCRIPT_VERSION
> -if [ -x /usr/sbin/babeld ]; then
> -	babel_version="<babel_version>$(/usr/sbin/babeld -V 2>&1)</babel_version>"
> -fi
> +SYSTEM_DATA="$SYSTEM_DATA<batman_advanced_version>$(cat /sys/module/batman_adv/version)</batman_advanced_version>"
batman_adv kann auch nicht vorhanden sein, das müsste analog zu babeld mit einem passenden if versehen werden.
Würde ich aber einfach als extra Patch senden.

Ansonsten:
Reviewed-by: Fabian Bläse <fabian@blaese.de>

Gruß
Fabian
Adrian Schmutzler April 18, 2020, 11:10 p.m. 
Hallo Fabian,

> > +SYSTEM_DATA="$SYSTEM_DATA<batman_advanced_version>$(cat /sys/module/batman_adv/version)</batman_advanced_version>" 
> batman_adv kann auch nicht vorhanden sein, das müsste analog zu babeld mit einem passenden if versehen werden. 
> Würde ich aber einfach als extra Patch senden. 

Das ist richtig, meine Version habe ich aber quasi nur von der alten Version übernommen (und hier nur zusammengefügt).

Insofern macht ein separater Patch Sinn.

Grüße

Adrian