| Message ID | 20161011113740.28392-2-chunkeey@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Mathias Kresin |
| Headers | show |
On 11 October 2016 at 13:37, Christian Lamparter <chunkeey@googlemail.com> wrote: > Previously, wifi detect simply dumped its generated wireless > configuration out to STDOUT. A second step was needed to > append the configuration to /etc/config/wireless (or create > it, if it didn't exist). > > With this patch, The wifi detection script will now use uci > to update the wireless configuration directly. > > Note: uci writes a "cfg123456" printout to stdout when it > generates a name for the unnamed section ("add wireless wifi-iface"). This rather sounds like a bug in uci, at least I would expect it to generate no output if it is invoked with "-q", neither on stdout nor stderr. > Since we currently pipe wifi detect into /etc/config/wireless, we > have to redirected this output, so it doesn't end up in the wireless > configuration file. I wonder if it wouldn't be better to keep "detect" output to stdout, and add a new command that modifies the uci config (e.g. "update" or "refresh"). People might rely/expect the current behavior, and get confused when it stops working. Jonas
On 10/12/2016 01:42 PM, Jonas Gorski wrote: > On 11 October 2016 at 13:37, Christian Lamparter > <chunkeey@googlemail.com> wrote: >> Previously, wifi detect simply dumped its generated wireless >> configuration out to STDOUT. A second step was needed to >> append the configuration to /etc/config/wireless (or create >> it, if it didn't exist). >> >> With this patch, The wifi detection script will now use uci >> to update the wireless configuration directly. >> >> Note: uci writes a "cfg123456" printout to stdout when it >> generates a name for the unnamed section ("add wireless wifi-iface"). > > This rather sounds like a bug in uci, at least I would expect it to > generate no output if it is invoked with "-q", neither on stdout nor > stderr. > >> Since we currently pipe wifi detect into /etc/config/wireless, we >> have to redirected this output, so it doesn't end up in the wireless >> configuration file. > > I wonder if it wouldn't be better to keep "detect" output to stdout, > and add a new command that modifies the uci config (e.g. "update" or > "refresh"). People might rely/expect the current behavior, and get > confused when it stops working. > > > Jonas > Maybe we could just rename wifi-detect to something different, so nobody would expect the old behaviour? Keeping two separate code paths doesn't seem like a good idea to me. Regards, Matthias
On Wednesday, October 12, 2016 2:05:15 PM CEST Matthias Schiffer wrote: > On 10/12/2016 01:42 PM, Jonas Gorski wrote: > > On 11 October 2016 at 13:37, Christian Lamparter > > <chunkeey@googlemail.com> wrote: > >> Previously, wifi detect simply dumped its generated wireless > >> configuration out to STDOUT. A second step was needed to > >> append the configuration to /etc/config/wireless (or create > >> it, if it didn't exist). > >> > >> With this patch, The wifi detection script will now use uci > >> to update the wireless configuration directly. > >> > >> Note: uci writes a "cfg123456" printout to stdout when it > >> generates a name for the unnamed section ("add wireless wifi-iface"). > > > > This rather sounds like a bug in uci, at least I would expect it to > > generate no output if it is invoked with "-q", neither on stdout nor > > stderr. There's a description in uci's list.c [0] that states the purpose of the printout. "[...] This is used as reference when locating or updating the section from apps/scripts. To make multiple concurrent versions somewhat safe for updating, the name is generated from a hash of its type and name/value pairs of its option, and it is prefixed by a counter value. If the order of the unnamed sections changes for some reason, updates to them will be rejected." The cfg123456 string produced by the fprintf(stdout, ...) in uci_do_add [1]. > >> Since we currently pipe wifi detect into /etc/config/wireless, we > >> have to redirected this output, so it doesn't end up in the wireless > >> configuration file. > > > > I wonder if it wouldn't be better to keep "detect" output to stdout, > > and add a new command that modifies the uci config (e.g. "update" or > > "refresh"). People might rely/expect the current behavior, and get > > confused when it stops working. > > > Maybe we could just rename wifi-detect to something different, so nobody > would expect the old behaviour? Keeping two separate code paths doesn't > seem like a good idea to me. I renamed it to "wifi config" for now. I've played around with making a "compat" 'wifi detect' that simply prints the current config to stdout. i.e.: wifi_detect() { wifi_config &> /dev/null tmp=$(mktemp -u); cp /etc/config/wireless "$tmp"; cat "$tmp"; rm -f "$tmp" } this will work for wifi detect > /etc/config/wireless. But it also will cause to spawn duplicates if ">>" is used. Any clever ideas? Otherwise, I can drop the uci changes. They are certainly nice to have, but we can manage without them. Regards, Christian --- [0] <https://git.lede-project.org/?p=project/uci.git;a=blob;f=list.c;h=2b859a62c1b7717d0dc3c1502772d97768810218;hb=fe45f97302cb9cab70b6dcadfc5fde260b2212e9#l156> [1] <https://git.lede-project.org/?p=project/uci.git;a=blob;f=cli.c;h=deb670b62583679234fc68477d4fe6be38b84f71;hb=fe45f97302cb9cab70b6dcadfc5fde260b2212e9#l460>
diff --git a/package/kernel/mac80211/files/lib/wifi/mac80211.sh b/package/kernel/mac80211/files/lib/wifi/mac80211.sh index 9b15de5..253de40 100644 --- a/package/kernel/mac80211/files/lib/wifi/mac80211.sh +++ b/package/kernel/mac80211/files/lib/wifi/mac80211.sh @@ -92,7 +92,7 @@ detect_mac80211() { htmode="VHT80" } - [ -n $htmode ] && append ht_capab " option htmode $htmode" "$N" + [ -n $htmode ] && ht_capab="set wireless.radio${devidx}.htmode=$htmode" if [ -x /usr/bin/readlink -a -h /sys/class/ieee80211/${dev} ]; then path="$(readlink -f /sys/class/ieee80211/${dev}/device)" @@ -104,30 +104,29 @@ detect_mac80211() { case "$path" in platform*/pci*) path="${path##platform/}";; esac - dev_id=" option path '$path'" + dev_id="set wireless.radio${devidx}.path='$path'" else - dev_id=" option macaddr $(cat /sys/class/ieee80211/${dev}/macaddress)" + dev_id="set wireless.radio${devidx}.macaddr=$(cat /sys/class/ieee80211/${dev}/macaddress)" fi - cat <<EOF -config wifi-device radio$devidx - option type mac80211 - option channel ${channel} - option hwmode 11${mode_band} -$dev_id -$ht_capab - # REMOVE THIS LINE TO ENABLE WIFI: - option disabled 1 - -config wifi-iface - option device radio$devidx - option network lan - option mode ap - option ssid LEDE - option encryption none - + uci -q batch > /dev/null <<-EOF + set wireless.radio${devidx}=wifi-device + set wireless.radio${devidx}.type=mac80211 + set wireless.radio${devidx}.channel=${channel} + set wireless.radio${devidx}.hwmode=11${mode_band} + ${dev_id} + ${ht_capab} + set wireless.radio${devidx}.disabled=1 + + add wireless wifi-iface + set wireless.@wifi-iface[-1].device=radio${devidx} + set wireless.@wifi-iface[-1].network=lan + set wireless.@wifi-iface[-1].mode=ap + set wireless.@wifi-iface[-1].ssid=LEDE + set wireless.@wifi-iface[-1].encryption=none EOF - devidx=$(($devidx + 1)) - done -} + uci commit -q &> /dev/null + devidx=$(($devidx + 1)) + done +}
Previously, wifi detect simply dumped its generated wireless configuration out to STDOUT. A second step was needed to append the configuration to /etc/config/wireless (or create it, if it didn't exist). With this patch, The wifi detection script will now use uci to update the wireless configuration directly. Note: uci writes a "cfg123456" printout to stdout when it generates a name for the unnamed section ("add wireless wifi-iface"). Since we currently pipe wifi detect into /etc/config/wireless, we have to redirected this output, so it doesn't end up in the wireless configuration file. Signed-off-by: Christian Lamparter <chunkeey@gmail.com> --- package/kernel/mac80211/files/lib/wifi/mac80211.sh | 45 +++++++++++----------- 1 file changed, 22 insertions(+), 23 deletions(-)