-
Notifications
You must be signed in to change notification settings - Fork 111
Description
The APuP pull request #1134 has been merged without addressing some of the comments, so that people could start using it.
So here I report all the comments that still need to be addressed:
#1134 (review)
commenting on:
| xpcall(function() proto.runOnDevice(linuxBaseIfname, args) end, |
Ok, maybe I understood:
configurewrites the configuration for the interface to get up at the next reboot or restart of the service whilerunOnDevicemakes the thing happen right now in a volatile way. I think this should be explained more extensively in the comments for the future readers.
#1134 (comment)
commenting on:
| function network.createStatic(linuxBaseIfname) |
Could you add as a comment in the code the difference between
network.createStaticandnetwork.createStaticIface?
From what I can understand, the difference is that createStaticIface creates the configuration file (the actual interface will appear upon reboot or restart of the networking service), with a custom netmask, gateway and IPv4 or IPv6. While this creates the interface in the running system (not in the configuration file) with fixed netmask, no gateway and the primary IPv4. Right? If this is right, I think that it should have a more informative name than justcreateStatic.
#1134 (comment)
commenting on:
| function network.createVlan(linuxBaseIfname, vid, vlanProtocol) |
Same comment as above: it would be convenient to rename this function so that the difference with
createVlanIfaceis more clear.
lime-curtigghio only has the
test.shand the readme files, right?
In this case, I would not place it in thepackagesfolder, as it does not have a Makefile.
Thetest.shfile could be renamed and go in thetestsfolder.
The readme could be renamed (lime-curtigghiois not clear to me, does it mean anything?) and go either to the website libremesh.org as a new page or in the root of this repository.
In this case, we have to redefine
ap_ssid.
I would propose this:
- remove both
apup_ssidandap_ssid(anyway ap_ssid was not a good description of what this variable was for, see In lime-defaultsap_ssidrequired even whenlist_modes 'ap'is removed #430 (comment))- the AP interface SSID, both for normal AP and for APuP, gets taken from a new
ssidvariable.- the %N1 things (e.g. Batman-adv VLAN ID) gets set in a new variable, maybe
broadcast_id, instead than being calculated from another setting.
So we would move from:config lime network [...] option main_ipv4_address '10.%N1.0.0/16' [...] option main_ipv6_address 'fd%N1:%N2%N3:%N4%N5::/64' [...] list protocols batadv:%N1 [...] option anygw_mac "aa:aa:aa:%N1:%N2:aa" [...] config lime wifi [...] option ap_ssid 'LibreMesh.org' [...] option apup_ssid 'LibreMesh.org'To:
config lime network option broadcast_id 13 [...] option main_ipv4_address '10.%D1.0.0/16' [...] option main_ipv6_address 'fd%D1:%D2%D3:%D4%D5::/64' [...] list protocols batadv:%D1 [...] option anygw_mac "aa:aa:aa:%D1:%D2:aa" [...] config lime wifi [...] option ssid 'LibreMesh.org' [...]Where
%D1isbroadcast_id % 256,%D2is the next 8 bits etc etc (so by default they would be all zeros except %D1 which is 13).