gluon-status-page, gluon-mesh-batman-adv: make statuspage mesh-protoc… #1366

christf · 2018-04-14T10:36:10Z

…ol-agnostic

this PR will make the staus page backend independent from batctl.

this replaces the backend-part from #1053

neocturne

In addition to the noted changes, please also add an entry

packages 'status-page & (mesh-batman-adv-14 | mesh-batman-adv-15)' \
        'gluon-status-page-mesh-batman-adv'

to package/features, so that enabling the status-page and mesh-batman-adv-... features automatically selects the gluon-status-page-mesh-batman-adv package.

neocturne · 2018-04-14T17:04:16Z

package/gluon-mesh-batman-adv/src/Makefile

@@ -33,3 +33,4 @@ LDLIBS += $(LIBBATADV_LDLIBS)

 respondd.so: respondd.c
 $(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) -shared -fPIC -D_GNU_SOURCE -o $@ $^ $(LDLIBS) -lgluonutil -liwinfo -luci
+


It seems this line was added accidentally?

yeah.. done.

neocturne · 2018-04-14T17:04:35Z

package/gluon-status-page-mesh-batman-adv/Makefile

+define Package/gluon-status-page-mesh-batman-adv
+ TITLE:=Batman-data provider for gluon-status-page
+ SECTION:=gluon
+ CATEGORY:=Gluon


The above two lines can be dropped since 994c949

neocturne · 2018-04-14T17:11:27Z

package/gluon-status-page/files/lib/gluon/status-page/www/cgi-bin/dyn/neighbours-nodeinfo

@@ -6,6 +6,6 @@ badrequest() {
 exit 1
 }

-( batctl if | cut -d: -f1 | grep -qxF "$QUERY_STRING" ) 2>/dev/null || badrequest
+( ubus call network.interface dump | jsonfilter -e "@.interface[@.proto='gluon_mesh' && @.up=true].device"| grep -qxF "$QUERY_STRING" ) 2>/dev/null || badrequest


Hmm, I think the complexity of this command is high enough that we should encapsulate the whole thing up to the || in a small script provided by gluon-core. Something like gluon-is-meshif. I guess the grep could also be replaced by an extension of the jsonfilter expression.

agreed - how about we create gluon-get-mesh-ifs that contains everything up to the Igrep. This would be usable in a more universal way than when incorporating the grep for a specific interface....

Fine with me.

neocturne · 2018-04-14T17:12:53Z

package/gluon-status-page-mesh-batman-adv/src/Makefile

+
+neighbours-batadv: neighbours-batadv.c
+ $(CC) $(CFLAGS) $(CFLAGS_JSONC) $(LDFLAGS) $(LDFLAGS_JSONC) -o $@ $^ $(LDLIBS)
+


Ugh, I hate the amount of boilerplate code in this file... I'll clean this up very soon, in all Makefiles that use libbatadv.

neocturne · 2018-04-14T17:13:21Z

package/gluon-status-page/src/stations.c

@@ -27,6 +27,8 @@ static struct json_object *get_stations(const struct iwinfo_ops *iw, const char
 json_object_object_add(station, "signal", json_object_new_int(entry->signal));
 json_object_object_add(station, "noise", json_object_new_int(entry->noise));
 json_object_object_add(station, "inactive", json_object_new_int(entry->inactive));
+ json_object_object_add(station, "rx_rate", json_object_new_int(entry->rx_rate.rate));
+ json_object_object_add(station, "tx_rate", json_object_new_int(entry->tx_rate.rate));


This change looks unrelated, and we should not add these values unless the frontend also uses them.

in general I agree but since we are splitting backend and frontend here, I left it in the commit. I will remove it now, but I really do like those stats.

neocturne · 2018-04-14T17:21:31Z

package/gluon-status-page-mesh-batman-adv/Makefile

+ SECTION:=gluon
+ CATEGORY:=Gluon
+ PROVIDES:=gluon-status-page-data-provider
+ DEPENDS:=+gluon-status-page +gluon-mesh-batman-adv +libnl-tiny +libbatadv +libjson-c


gluon-mesh-batman-adv is a virtual package. Remove the + sign prefixing the dependency - never use selecting dependencies with virtual packages!

neocturne · 2018-04-14T17:24:44Z

package/gluon-status-page-mesh-batman-adv/Makefile

+ TITLE:=Batman-data provider for gluon-status-page
+ SECTION:=gluon
+ CATEGORY:=Gluon
+ PROVIDES:=gluon-status-page-data-provider


I think this PROVIDES is somewhat useless; as long as gluon-status-page-mesh-batman-adv depends on gluon-status-page, the result will be a circular dependency, which should always be avoided.

As the status page does not have a public API anymore, the status-page frontend for a specific mesh protocol will always be provided by the same package the backend comes in. As such, there is no common functionality in different backend packages that would justify a PROVIDES.

neocturne

Thinking about this again, I'm seeing the problem that now the frontent depends on parts of the backend that are moved to a separate package; I would like to avoid such dependencies.

In consequence, we actually need to move the frontend to a separate package first, before the backend may follow.

neocturne · 2018-04-15T07:44:58Z

package/gluon-core/files/usr/bin/get-mesh-ifs

@@ -0,0 +1,2 @@
+#!/bin/sh
+ubus call network.interface dump | jsonfilter -e "@.interface[@.proto='gluon_mesh' && @.up=true].device"


Maybe name it "gluon-mesh-interfaces"? In any case, it should start with "gluon-".

sure. I like verbs, so I would go with gluon-get-mesh-interfaces ok?

neocturne · 2018-04-15T07:46:39Z

package/gluon-l3roamd/files/etc/init.d/gluon-l3roamd

@@ -10,7 +10,7 @@ start_service () {
 procd_set_param stdout 1
 procd_set_param stderr 1
 procd_set_param respawn ${respawn_threshold:-3660} ${respawn_timeout:-5} ${respawn_retry:-0}
- interfaces=$(for dev in $( ubus call network.interface dump | jsonfilter -e "@.interface[@.proto='gluon_mesh' && @.up=true].device");do echo " -m $dev"; done;
+ interfaces=$(for dev in $(/usr/bin/get-mesh-ifs);do echo " -m $dev"; done;


Remove the full path here, just use the name of the script.

removed the path here but kept it in the scripts that are called by the statuspage.

Why, do those script not have a proper PATH set?

neocturne · 2018-04-15T09:40:00Z

I've prepared the frontend part in 35ade80.

…ol-agnostic

christf · 2018-04-15T10:04:59Z

rename and rebase is done.

neocturne · 2018-04-15T10:13:25Z

package/gluon-status-page/files/lib/gluon/status-page/www/cgi-bin/dyn/neighbours-nodeinfo

@@ -6,6 +6,6 @@ badrequest() {
 exit 1
 }

-( ubus call network.interface dump | jsonfilter -e "@.interface[@.proto='gluon_mesh' && @.up=true].device"| grep -qxF "$QUERY_STRING" ) 2>/dev/null || badrequest
+( /usr/bin/gluon-get-mesh-interfaces | grep -qxF "$QUERY_STRING" ) 2>/dev/null || badrequest


The other absolute paths in these files don't make much sense either, let's not introduce another one.

Sorry about changing my mind again, but I think I would prefer the verb "list" to "get" in the script name.

np. Adjusting now....

done. - for the paths I had to touch the files again anyways. I like list better than get too.

neocturne · 2018-04-15T10:17:30Z

The package/features update is still missing.

neocturne · 2018-04-15T10:23:55Z

And /lib/gluon/status-page/mesh.lua should be moved into the new package as well.

…list-mesh-ifs that lists all currently active mesh interfaces

…neighbours-nodeinfo and stations

neocturne · 2018-04-15T10:57:00Z

Thanks, merged.

neocturne requested changes Apr 14, 2018

View reviewed changes

neocturne requested changes Apr 15, 2018

View reviewed changes

gluon-status-page, gluon-mesh-batman-adv: make statuspage mesh-protoc…

e40f7bf

…ol-agnostic

neocturne reviewed Apr 15, 2018

View reviewed changes

christf added 2 commits April 15, 2018 12:37

gluon-core, gluon-status-page, gluon-l3roamd: introduce script gluon-…

e015999

…list-mesh-ifs that lists all currently active mesh interfaces

gluon-status-page: reduce usage of absolute paths in cgi-bin scripts …

fa57e2a

…neighbours-nodeinfo and stations

neocturne closed this Apr 15, 2018

rotanid added 0. type: enhancement The changeset is an enhancement 3. topic: package Topic: Gluon Packages labels Apr 16, 2018

rotanid added this to the 2018.1 milestone Apr 16, 2018

christf mentioned this pull request Apr 16, 2018

gluon-status-page: rework package to not depend on batman #1053

Closed

oszilloskop deleted the christf_statuspage-mesh-agnostic branch October 22, 2019 00:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gluon-status-page, gluon-mesh-batman-adv: make statuspage mesh-protoc… #1366

gluon-status-page, gluon-mesh-batman-adv: make statuspage mesh-protoc… #1366

christf commented Apr 14, 2018 •

edited

Loading

neocturne left a comment

neocturne Apr 14, 2018

christf Apr 14, 2018

neocturne Apr 14, 2018

christf Apr 14, 2018

neocturne Apr 14, 2018

christf Apr 14, 2018

neocturne Apr 14, 2018

christf Apr 14, 2018

neocturne Apr 14, 2018

neocturne Apr 14, 2018

christf Apr 14, 2018

neocturne Apr 14, 2018

christf Apr 14, 2018

neocturne Apr 14, 2018

neocturne left a comment

neocturne Apr 15, 2018

christf Apr 15, 2018

neocturne Apr 15, 2018

neocturne Apr 15, 2018

christf Apr 15, 2018

neocturne Apr 15, 2018

neocturne commented Apr 15, 2018

christf commented Apr 15, 2018

neocturne Apr 15, 2018

neocturne Apr 15, 2018

christf Apr 15, 2018

christf Apr 15, 2018

neocturne commented Apr 15, 2018

neocturne commented Apr 15, 2018

neocturne commented Apr 15, 2018

		@@ -33,3 +33,4 @@ LDLIBS += $(LIBBATADV_LDLIBS)

		respondd.so: respondd.c
		$(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) -shared -fPIC -D_GNU_SOURCE -o $@ $^ $(LDLIBS) -lgluonutil -liwinfo -luci


		neighbours-batadv: neighbours-batadv.c
		$(CC) $(CFLAGS) $(CFLAGS_JSONC) $(LDFLAGS) $(LDFLAGS_JSONC) -o $@ $^ $(LDLIBS)

		@@ -0,0 +1,2 @@
		#!/bin/sh
		ubus call network.interface dump \| jsonfilter -e "@.interface[@.proto='gluon_mesh' && @.up=true].device"

gluon-status-page, gluon-mesh-batman-adv: make statuspage mesh-protoc… #1366

gluon-status-page, gluon-mesh-batman-adv: make statuspage mesh-protoc… #1366

Conversation

christf commented Apr 14, 2018 • edited Loading

neocturne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neocturne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neocturne commented Apr 15, 2018

christf commented Apr 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neocturne commented Apr 15, 2018

neocturne commented Apr 15, 2018

neocturne commented Apr 15, 2018

christf commented Apr 14, 2018 •

edited

Loading