New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gluon-core: add preserve wifi channel feature #640

Merged
merged 1 commit into from Feb 24, 2016

Conversation

Projects
None yet
7 participants
@kokel
Contributor

kokel commented Feb 5, 2016

Old description:
This changes introduces the new uci section 'gluon-core.wireless' with a preserve_channel option per radio:

  • preserve_channel_radio0 (boolean)
  • preserve_channel_radio1 (boolean)

By setting these options to true the wifi channel of the regarding radio will be preserved during upgrades.

New description:
This change introduces the new uci section 'gluon-core.wireless' with a preserve_channels option:

  • preserve_channels (boolean)

By setting this option to 1 (true) wifi channels will be preserved during upgrades.

@Adorfer

This comment has been minimized.

Show comment
Hide comment
@Adorfer

Adorfer Feb 6, 2016

Contributor

I would like to have this in the next release, since it will provide great benefit for "airtime optimized" mesh networks where you tend to systematically install pairs of two 841N on different channels, just in order to have more reliable meshing if there is interference with other networks.

As a hotfix (to preserve the radio channel) you could do this as a "special hack" for nodes which already have to stay and you have no means of setting such a config directive.

https://github.com/eulenfunk/firmware/blob/master/templates/dus/keepradiochannel.diff

Contributor

Adorfer commented Feb 6, 2016

I would like to have this in the next release, since it will provide great benefit for "airtime optimized" mesh networks where you tend to systematically install pairs of two 841N on different channels, just in order to have more reliable meshing if there is interference with other networks.

As a hotfix (to preserve the radio channel) you could do this as a "special hack" for nodes which already have to stay and you have no means of setting such a config directive.

https://github.com/eulenfunk/firmware/blob/master/templates/dus/keepradiochannel.diff

@tcatm

This comment has been minimized.

Show comment
Hide comment
@tcatm

tcatm Feb 8, 2016

I don't like preserve_channel_radio0 as you have not much control of what radio0 and radio1 is (or whether it's radioX at all). Maybe a simpler approach like preserve_channels works better (or adding the options to the wireless config).

tcatm commented Feb 8, 2016

I don't like preserve_channel_radio0 as you have not much control of what radio0 and radio1 is (or whether it's radioX at all). Maybe a simpler approach like preserve_channels works better (or adding the options to the wireless config).

@NeoRaider

This comment has been minimized.

Show comment
Hide comment
@NeoRaider

NeoRaider Feb 8, 2016

Member

Well, the current code would work with any radio name, and as it is supposed to be set after flashing, the radio names would already be known. But I don't see a good usecase for setting this for each radio individually either...

Member

NeoRaider commented Feb 8, 2016

Well, the current code would work with any radio name, and as it is supposed to be set after flashing, the radio names would already be known. But I don't see a good usecase for setting this for each radio individually either...

Show outdated Hide outdated package/gluon-core/files/lib/gluon/upgrade/200-wireless
@@ -10,11 +10,21 @@ if not sysconfig.gluon_version then
uci:delete_all('wireless', 'wifi-iface')
end
local function get_channel(radio, config)
if uci:get_first('gluon-core', 'wireless', 'preserve_channel_' .. radio) and sysconfig.gluon_version and uci:get('wireless', radio) then

This comment has been minimized.

@NeoRaider

NeoRaider Feb 8, 2016

Member

This should not refer to sysconfig.gluon_version. Just return the current setting if there is a setting and preserve_channel is set; in any other case, return the default setting.

@NeoRaider

NeoRaider Feb 8, 2016

Member

This should not refer to sysconfig.gluon_version. Just return the current setting if there is a setting and preserve_channel is set; in any other case, return the default setting.

@kokel

This comment has been minimized.

Show comment
Hide comment
@kokel

kokel Feb 8, 2016

Contributor

Thanks for your feedback. I have merged your suggestions. For now there is one config option gluon-core.wireless.preserve_channels to preserve the wifi channel of all radios.

Please let me know if the alternate approach via wireless config (then this would be on a per radio basis) is cleaner or the preferred way, in your point of view. I don't care much about the approach, both are fine for me.

Contributor

kokel commented Feb 8, 2016

Thanks for your feedback. I have merged your suggestions. For now there is one config option gluon-core.wireless.preserve_channels to preserve the wifi channel of all radios.

Please let me know if the alternate approach via wireless config (then this would be on a per radio basis) is cleaner or the preferred way, in your point of view. I don't care much about the approach, both are fine for me.

@NeoRaider NeoRaider added this to the 2016.2 milestone Feb 8, 2016

@petabyteboy

This comment has been minimized.

Show comment
Hide comment
@petabyteboy

petabyteboy Feb 9, 2016

Contributor

Another great addition to this would be the possibility to turn this on via the site by default.

Contributor

petabyteboy commented Feb 9, 2016

Another great addition to this would be the possibility to turn this on via the site by default.

@jplitza

This comment has been minimized.

Show comment
Hide comment
@jplitza

jplitza Feb 9, 2016

Member

@plumpudding Why would that make any sense? The different channel would have to be configured via the command line or via an additional package anyway. In the same way would this setting be set.

Member

jplitza commented Feb 9, 2016

@plumpudding Why would that make any sense? The different channel would have to be configured via the command line or via an additional package anyway. In the same way would this setting be set.

@kokel

This comment has been minimized.

Show comment
Hide comment
@kokel

kokel Feb 9, 2016

Contributor

@plumpudding @jplitza I could imagine one possible use case for the site.conf switch ... the bofh in freifunk community x wants to reset all user defined wifi channels 😝 😇

Contributor

kokel commented Feb 9, 2016

@plumpudding @jplitza I could imagine one possible use case for the site.conf switch ... the bofh in freifunk community x wants to reset all user defined wifi channels 😝 😇

@nazco

This comment has been minimized.

Show comment
Hide comment
@nazco

nazco Feb 9, 2016

Contributor

Great feature!

Contributor

nazco commented Feb 9, 2016

Great feature!

Show outdated Hide outdated docs/features/wlan-configuration.rst
@@ -24,3 +24,11 @@ existing configuration will also be set for the new configuration.
This allows upgrades to change from IBSS to 11s and vice-versa while retaining the
"wireless meshing is enabled/disabled" property configured by the user regardless
of the used mode.
During upgrades the wifi channel of the 2,4GHz and 5GHz radio will be restored to the channel

This comment has been minimized.

@NeoRaider

NeoRaider Feb 9, 2016

Member

Should be "2.4GHz", not "2,4GHz".

@NeoRaider

NeoRaider Feb 9, 2016

Member

Should be "2.4GHz", not "2,4GHz".

Show outdated Hide outdated docs/features/wlan-configuration.rst
During upgrades the wifi channel of the 2,4GHz and 5GHz radio will be restored to the channel
configured in the site.conf. If your use case is to be in need to preserve a user defined
wifi channel during upgrades you can configure this via the uci section ``gluon-core.wireless``::

This comment has been minimized.

@NeoRaider

NeoRaider Feb 9, 2016

Member

Very weird grammar, just say: "If you need to preserve [...]"

@NeoRaider

NeoRaider Feb 9, 2016

Member

Very weird grammar, just say: "If you need to preserve [...]"

Show outdated Hide outdated package/gluon-core/files/lib/gluon/upgrade/200-wireless
@@ -10,11 +10,21 @@ if not sysconfig.gluon_version then
uci:delete_all('wireless', 'wifi-iface')
end
local function get_channel(radio, config)
if uci:get_first('gluon-core', 'wireless', 'preserve_channels') then
return uci:get('wireless', radio, 'channel')

This comment has been minimized.

@NeoRaider

NeoRaider Feb 9, 2016

Member

I think the function should still fall back to config.channel if uci:get('wireless', radio, 'channel') returns nil, so previously unconfigured radios (maybe because of missing drivers... not supported at the moment, but we'd like to make the config more flexible) will get the correct channel even if preserve_channels is set. You can just add or config.channel to the return line above.

@NeoRaider

NeoRaider Feb 9, 2016

Member

I think the function should still fall back to config.channel if uci:get('wireless', radio, 'channel') returns nil, so previously unconfigured radios (maybe because of missing drivers... not supported at the moment, but we'd like to make the config more flexible) will get the correct channel even if preserve_channels is set. You can just add or config.channel to the return line above.

@Adorfer

This comment has been minimized.

Show comment
Hide comment
@Adorfer

Adorfer Feb 9, 2016

Contributor

i want the optional "site.conf":
overwrite - take site.conf channel(s) by any means
preserve - keep existing chanels if already gluon on device (even if UCI not set)
node - do what's set in the node's UCI (and by default don't preserve)

Contributor

Adorfer commented Feb 9, 2016

i want the optional "site.conf":
overwrite - take site.conf channel(s) by any means
preserve - keep existing chanels if already gluon on device (even if UCI not set)
node - do what's set in the node's UCI (and by default don't preserve)

@NeoRaider

This comment has been minimized.

Show comment
Hide comment
@NeoRaider

NeoRaider Feb 9, 2016

Member

@Adorfer

When a user changes a setting, site.conf should not override it. There are settings for which we don't know if they were changed by the user; therefore we have to add settings like "preserve_channels".

We definitely don't want to add more complex settings to site.conf. We'd like to keep everything as simple as:

  • Defaults get set from site.conf
  • When the user changes things, they won't be changed on updates
  • When the site.conf defaults change, the settings that haven't been changed by the user should be updated where appropriate.

Following this scheme, I completely agree with @jplitza.

Member

NeoRaider commented Feb 9, 2016

@Adorfer

When a user changes a setting, site.conf should not override it. There are settings for which we don't know if they were changed by the user; therefore we have to add settings like "preserve_channels".

We definitely don't want to add more complex settings to site.conf. We'd like to keep everything as simple as:

  • Defaults get set from site.conf
  • When the user changes things, they won't be changed on updates
  • When the site.conf defaults change, the settings that haven't been changed by the user should be updated where appropriate.

Following this scheme, I completely agree with @jplitza.

@Adorfer

This comment has been minimized.

Show comment
Hide comment
@Adorfer

Adorfer Feb 9, 2016

Contributor

@NeoRaider
An Autoupdater is always a matter of trust.
the scenario for different wifi channels is mostly "airtime optimization" in an existing rollout.
But if you want to migrate a list of nodes to new domain, it's certainly neccesary to change radio settings, most likely including radio chanels.
Otherwise perhaps not "override node settings" but "do not preserve if sitecode changes."

Contributor

Adorfer commented Feb 9, 2016

@NeoRaider
An Autoupdater is always a matter of trust.
the scenario for different wifi channels is mostly "airtime optimization" in an existing rollout.
But if you want to migrate a list of nodes to new domain, it's certainly neccesary to change radio settings, most likely including radio chanels.
Otherwise perhaps not "override node settings" but "do not preserve if sitecode changes."

@NeoRaider

This comment has been minimized.

Show comment
Hide comment
@NeoRaider

NeoRaider Feb 9, 2016

Member

If a user sets the "preserve_channels" option, that's a willful decision to take care of the configuration themselves. You should never change any settings in spite of such a willful decision.

If some greater change requires modifying the configuration, then the users will need to fix it themselves. The same is the case if the autoupdater is disabled completely.

Member

NeoRaider commented Feb 9, 2016

If a user sets the "preserve_channels" option, that's a willful decision to take care of the configuration themselves. You should never change any settings in spite of such a willful decision.

If some greater change requires modifying the configuration, then the users will need to fix it themselves. The same is the case if the autoupdater is disabled completely.

@Adorfer

This comment has been minimized.

Show comment
Hide comment
@Adorfer

Adorfer Feb 10, 2016

Contributor

If some greater change requires modifying the configuration, then the users will need to fix it themselves.

Since it's our own nodes, we will then bake special firmware (as usual..)
And yes, i would like to have some kind of lightweight remote administratin interface for nodes which are my own. (and not doing this via ssh-scripts...) but that's another topic.

Contributor

Adorfer commented Feb 10, 2016

If some greater change requires modifying the configuration, then the users will need to fix it themselves.

Since it's our own nodes, we will then bake special firmware (as usual..)
And yes, i would like to have some kind of lightweight remote administratin interface for nodes which are my own. (and not doing this via ssh-scripts...) but that's another topic.

gluon-core: add preserve wifi channels feature
This new feature introduces the new uci section 'gluon-core.wireless' with a preserve_channels option:
 * preserve_channels (boolean)

By setting this option to 1 (true) wifi channels will be preserved during upgrades.
@kokel

This comment has been minimized.

Show comment
Hide comment
@kokel

kokel Feb 10, 2016

Contributor

@Adorfer I think your use cases are valid but can't be handled by this little feature. The migration of nodes to a complete different site configuration should be done in an extra package which implements the change of a complete different site.conf because it's not only the channel to be changed, than. This would be much more complex.

@NeoRaider I have merged your suggestions, thanks.

Contributor

kokel commented Feb 10, 2016

@Adorfer I think your use cases are valid but can't be handled by this little feature. The migration of nodes to a complete different site configuration should be done in an extra package which implements the change of a complete different site.conf because it's not only the channel to be changed, than. This would be much more complex.

@NeoRaider I have merged your suggestions, thanks.

NeoRaider added a commit that referenced this pull request Feb 24, 2016

Merge pull request #640 from kokel/preserve-wifi-channel
gluon-core: add preserve wifi channel feature

@NeoRaider NeoRaider merged commit 6961406 into freifunk-gluon:master Feb 24, 2016

@kokel kokel deleted the kokel:preserve-wifi-channel branch Feb 25, 2016

ecsv pushed a commit to FreifunkVogtland/gluon that referenced this pull request Jun 9, 2017

Merge pull request #640 from kokel/preserve-wifi-channel
gluon-core: add preserve wifi channel feature
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment