search

freifunk-berlin / firmware

DEPRECATED: Build system for Berlin firmware. Please user the pinned falter-repos instead
https://berlin.freifunk.net
GNU General Public License v3.0
73 stars 34 forks source link

SAm0815_Hedy-alpha: Duplicate fields for ESSID break LuCi #500

Closed torte71 closed 6 years ago

torte71 commented 6 years ago

Branch: SAm0815_Hedy-alpha (checked out via git today, 2017-12-26 15:00; built for PI3; TARGET=brcm2708-bcm2710) Same behaviour in this image: https://buildbot.berlin.freifunk.net/buildbot/unstable/ar71xx-generic/572/tunnel-berlin/freifunk-berlin-1.0.0-alpha-sam0815-43478e9-tl-wr842n-v3-sysupgrade.bin

In the interface settings for the wireless adapters, the entry field for "ESSID" appears twice. After submitting, there will be two entries in /etc/config/wireless:

instead of

The "list" instead of "option" breaks several pages in LuCi (wireless config, network config, ...), because the lua scripts expect a string return value but get a table, what leads to several errors.

SvenRoederer commented 6 years ago

Can you please check if this also happens to older versions of "SAm0815_Hedy-alpha" branch? they can be found on https://buildbot.berlin.freifunk.net/buildbot/unstable/ar71xx-generic/ and then look for Version.txt in every separate build-number. Version.txt has a line informing on the exact branch and revision used during build.

torte71 commented 6 years ago

Starts between [1]:547 (rev d172dd7) - good, no error [2]:549 (rev 4d49f1b) - bad, duplicate ESSID field

To reproduce:

[1] 547: https://buildbot.berlin.freifunk.net/buildbot/unstable/ar71xx-generic/547/VERSION.txt

https://github.com/freifunk-berlin/firmware https://wiki.freifunk.net/Berlin:Firmware Firmware: git branch "SAm0815_experimental", revision d172dd7 OpenWRT: repository from https://git.lede-project.org/source.git, git branch "d573b1287e1b4ace7680d5dbfde20ba25e428b17", revision reboot-5486-gd573b12 Feed packages: repository from https://github.com/openwrt/packages.git, git branch "6789b0ffb8636ebe1034374fc6cc9bb1533680f7", revision 6789b0f Feed luci: repository from https://github.com/openwrt/luci.git, git branch "8e6b1a6d8628d6da4820a4400eddd8a1da84fa27", revision 8e6b1a6 Feed routing: repository from https://github.com/openwrt-routing/packages.git, git branch "5457ae65243958b64ffea96e28d868a3d6820954", revision 5457ae6 Feed packages_berlin: repository from https://github.com/freifunk-berlin/firmware-packages.git, git branch "19cb3cddd2a0df6346586653d97f766953471717", revision for-0.2.0-166-g19cb3cd

[2] 549: https://buildbot.berlin.freifunk.net/buildbot/unstable/ar71xx-generic/549/VERSION.txt

https://github.com/freifunk-berlin/firmware https://wiki.freifunk.net/Berlin:Firmware Firmware: git branch "issue414_v2", revision 4d49f1b OpenWRT: repository from https://git.lede-project.org/source.git, git branch "367b4563b4cd466188bd4c8cf24ff0a125029c1c", revision v17.01.4-10-g367b456 Feed packages: repository from https://github.com/openwrt/packages.git, git branch "cd5c448758f30868770b9ebf8b656c1a4211a240", revision cd5c448 Feed luci: repository from https://github.com/openwrt/luci.git, git branch "7fc88b4e6eb8abf4b53f74f95306dd84b57e9a53", revision 7fc88b4 Feed routing: repository from https://github.com/openwrt-routing/packages.git, git branch "d11075cd40a88602bf4ba2b275f72100ddcb4767", revision d11075c Feed packages_berlin: repository from https://github.com/freifunk-berlin/firmware-packages.git, git branch "19cb3cddd2a0df6346586653d97f766953471717", revision for-0.2.0-166-g19cb3cd

torte71 commented 6 years ago

Have you been that quick? I just noticed, that build #576 (revision 1ddb59a) didn't show the duplicate field (on the gl-ar150). So I flashed that revision to the wr842 that I've used for the tests before, and the duplicate field has gone there, too.

SvenRoederer commented 6 years ago

differences between these builds are hard to track.

can you test with builds from the exact same branch-name, but different ages?

SvenRoederer commented 6 years ago

I just noticed, that build #576 (revision 1ddb59a) didn't show the duplicate field

this is based on LEDE-17.01 with corresponding feeds. But I wonder on the output of git diff 4d49f1b..1ddb59a as it looks like there is somehow LEDE-master involved.

But when build567 fixed the issue, I'm fine with closing this

torte71 commented 6 years ago

Fixing this bug is so 2017. Closing is OK.

ghost commented 6 years ago

Don't close this yet: some changes in the wireless config reintroduce the bug. Disabling intern-ch13.freifunk.net works just fine, but changing berin.freifunk.net's channel breaks LuCi.

Some of the errors thrown

http://frei.funk/cgi-bin/luci/admin/network/wireless

/usr/lib/lua/luci/model/network.lua:1656: bad argument #3 to '?' (string expected, got table)
stack traceback:
    [C]: in function 'error'
    /usr/lib/lua/luci/util.lua:40: in function </usr/lib/lua/luci/util.lua:29>
    /usr/lib/lua/luci/model/network.lua:1656: in function 'shortname'
    /usr/lib/lua/luci/controller/admin/network.lua:55: in function 'e'
    /usr/lib/lua/luci/dispatcher.lua:464: in function 'createtree'
    /usr/lib/lua/luci/dispatcher.lua:171: in function 'dispatch'
    /usr/lib/lua/luci/dispatcher.lua:109: in function </usr/lib/lua/luci/dispatcher.lua:108>

http://frei.funk/cgi-bin/luci/admin/system/admin

/usr/lib/lua/luci/dispatcher.lua:380: Failed to execute cbi dispatcher target for entry '/admin/system/admin'.
The called action terminated with an exception:
/usr/lib/lua/luci/template.lua:55: Failed to execute template 'cbi/map'.
A runtime error occured: /usr/lib/lua/luci/template.lua:55: Failed to execute template 'cbi/tsection'.
A runtime error occured: /usr/lib/lua/luci/template.lua:55: Failed to execute template 'cbi/ucisection'.
A runtime error occured: /usr/lib/lua/luci/template.lua:55: Failed to execute template 'cbi/network_netlist'.
A runtime error occured: /usr/lib/lua/luci/model/network.lua:1213: bad argument #4 to '?' (string expected, got table)
stack traceback:
    [C]: in function 'assert'
    /usr/lib/lua/luci/dispatcher.lua:380: in function 'dispatch'
    /usr/lib/lua/luci/dispatcher.lua:109: in function 
root@butters:~# cat /etc/config/wireless

config wifi-device 'radio0'
        option type 'mac80211'
        option path 'platform/soc/3f300000.mmc/mmc_host/mmc1/mmc1:0001/mmc1:0001:1'
        option htmode 'HT20'
        option country 'DE'
        option doth '0'
        option diversity '1'
        option hwmode '11g'
        option channel '9'

config wifi-iface
        option network 'wireless0'
        option encryption 'none'
        option device 'radio0'
        option bssid 'D2:CA:FF:EE:BA:BE'
        option ssid 'intern-ch13.freifunk.net'
        option mcast_rate '6000'
        option mode 'adhoc'
        option ifname 'wlan0-adhoc-2'
        option disabled '1'

config wifi-iface
        option ifname 'wlan0-dhcp-2'
        option network 'dhcp'
        option encryption 'none'
        option device 'radio0'
        option mode 'ap'
        list ssid 'berlin.freifunk.net'
        list ssid 'berlin.freifunk.net'

I'm building 43478e9b04a779050ead3315d01d683bb68d0915 for the rpi-3.

torte71 commented 6 years ago

@SvenRoederer: I found it. But as I don't want to act like a bull in a china shop, I could need your advice, what would be the best way to patch one of our patches.

The duplicate ESSID field is caused by this patch: https://github.com/freifunk-berlin/firmware/blob/master/patches/200-luci_fix_meshid.patch It doesn't take into account, that there is already the ESSID field defined some lines above in the original code and adds it again. I don't know, why this didn't cause any problems earlier - maybe the old openwrt stuff just reacted differently to duplicates.

This is, how the patch should look for lede >= 17.01:

diff --git a/modules/luci-mod-admin-full/luasrc/model/cbi/admin_network/wifi.lua b/modules/luci-mod-admin-full/luasrc/model/cbi/admin_network/wifi.lua
index 6c89d7e..c46ef42 100644
--- a/feeds/luci/modules/luci-mod-admin-full/luasrc/model/cbi/admin_network/wifi.lua
+++ b/feeds/luci/modules/luci-mod-admin-full/luasrc/model/cbi/admin_network/wifi.lua
@@ -405,15 +405,27 @@ s:tab("encryption", translate("Wireless Security"))
 s:tab("macfilter", translate("MAC-Filter"))
 s:tab("advanced", translate("Advanced Settings"))

-ssid = s:taboption("general", Value, "ssid", translate("<abbr title=\"Extended Service Set Identifier\">ESSID</abbr>"))
-ssid.datatype = "maxlength(32)"
-
 mode = s:taboption("general", ListValue, "mode", translate("Mode"))
 mode.override_values = true
 mode:value("ap", translate("Access Point"))
 mode:value("sta", translate("Client"))
 mode:value("adhoc", translate("Ad-Hoc"))

+ssid = s:taboption("general", Value, "ssid", translate("<abbr title=\"Extended Service Set Identifier\">ESSID</abbr>"))
+ssid.datatype = "maxlength(32)"
+ssid:depends({mode="ap"})
+ssid:depends({mode="sta"})
+ssid:depends({mode="adhoc"})
+ssid:depends({mode="ahdemo"})
+ssid:depends({mode="monitor"})
+ssid:depends({mode="ap-wds"})
+ssid:depends({mode="sta-wds"})
+ssid:depends({mode="wds"})
+
+
+meshid = s:taboption("general", Value, "mesh_id", translate("Mesh Id"))
+meshid:depends({mode="mesh"})
+
 bssid = s:taboption("general", Value, "bssid", translate("<abbr title=\"Basic Service Set Identifier\">BSSID</abbr>"))

 network = s:taboption("general", Value, "network", translate("Network"),

The builds for "SAm0815-experimental" were not affected, because the 200-...patch doesn't exist in this branch.

I've tested the new patch on the Hedy-1.0.0-rc1 branch (replaced the old patch, used the same name), it applies and the generated image works as it should (tested vpn03 on ar150).

What would be the way to proceed? Replace the 200-luci_fix_meshid.patch with the new content? Create a new 202-...patch and throw 200-...patch out of "series"? The former credits/signed-off-by should be kept, I think, but with some descriptions added? Hmpfff... many questions for a simple fix.

Btw.: As a weird side-effect, my newly built image for the ar150 now shows the 500-internal-server error, that I only knew from the pi-3 build before. Whereas the image from the buildbot #584 (freifunk-berlin-1.0.0-alpha-613b84d-gl-ar150-sysupgrade.bin) didn't show that behaviour.

SvenRoederer commented 6 years ago

The patch used for the firmware should be just the PR https://github.com/openwrt/luci/pull/1363 of @kishangondaliya . Not sure if I messed up importing it or if it was updated after I imported it.

@torte71 feel free to update my patch. I suggest to just replace the contents of the current patch with the code taken from the PR. Take care that you ensure the correct paths of the files to patch. Probably also add a reference in the patch-comments where to find the original PR. For a short test you can also download the original-openwrt-code of the Luci-page (https://github.com/kishangondaliya/luci/blob/lede-17.01/modules/luci-mod-admin-full/luasrc/model/cbi/admin_network/wifi.lua) manually add the lines of the PR and then 'diff' both files. The resulting diff (lede17.01-branch to patched code should be the contents of the patch we need finally.

P.S: probably I messed up the patch during backporting the original PR from master to lede-17.01 branch.

torte71 commented 6 years ago

OK, in my first shot, I changed the order a bit (I only looked at functionality rather than the original patch - and don't even feel very sorry about that :)). See https://github.com/openwrt/luci/pull/1363#issuecomment-355805730 But now I'll keep the original order - both ways work as expected, (afaics) because in the final generated document the javascript has already assigned the value that looks missing in the lua-source, so this is only cosmetical. I'll stick closely to the openwrt patch, and if they decide to change it, we can follow that, too.

Where should I apply it? Is "hedy-1.0.0-rc1" regarded as "closed", i.e. changes only go to "head"?

SvenRoederer commented 6 years ago

feel free to open an PR against Hedy-1.0.0-rc1 branch. So I'll merge your commit into this branch at first, then I'll push to master (after having a good build of rc1-branch)

torte71 commented 6 years ago

Ouch, my commit directly went here instead of my repository - this was not intentional.

SvenRoederer commented 6 years ago

So you learned about usage of git :-)

torte71 commented 6 years ago

For the moment, I've learned, always to use --verbose and --dry-run...

SvenRoederer commented 6 years ago

@torte71 just seen: https://github.com/openwrt/luci/commit/267bf83db020295179fad88c6859e50e3406665a ; let's see if @jow will cherry-pick for lede-17.01. If he will, we don't need to patch our code here

SvenRoederer commented 6 years ago

opened https://github.com/openwrt/luci/issues/1577 for cherry-picking in LuCI

SvenRoederer commented 6 years ago

@torte71 code for 802.11s-support has been cherry-picked. updated Hedy-1.0.0-rc1 build is on the way.

SvenRoederer commented 6 years ago

fixed upstream