openSUSE / jeos-firstboot

Lightweight firstboot wizard systemd service for SLE and openSUSE JeOS Images
MIT License
14 stars 13 forks source link

Improve dialogs, support manually entering SSID for WiFi #79

Closed gmoro closed 3 years ago

gmoro commented 3 years ago

Introduce wifi hidden SSID supports Improve menu appearance fixing bsc#1177188

gmoro commented 3 years ago

Two of those commits are from me, please fix the authorship. I just copied and paste this, do you really need the authorship?

The approach with () has some issues:

* There can be multiple hidden networks

Forgot to implement that

* `()` is a valid ESSID

Thats the main problem

* Hidden networks don't broadcast, so they might not show up at all

Hidden networks don't broadcast the SSID but they are always visible for iwlist, with an empty SSID I thought that was not a good enough solution, but now we have something to go from.

So instead I suggest to always add a special entry to enter a SSID manually. dialogs supports keys and values in dialogs, so you can use that to distinguish between the ESSIDs and the special option.

I did try to do that initially, but failed, do you have a good example of doing this special entries?

Vogtinator commented 3 years ago

Two of those commits are from me, please fix the authorship. I just copied and paste this,

Copy + paste the entire diff and commit message separately? If you use git cherry-pick or just work on the branch, it'll keep them with proper author and date.

do you really need the authorship?

Yes please.

The approach with () has some issues:

* There can be multiple hidden networks

Forgot to implement that

* `()` is a valid ESSID

Thats the main problem

* Hidden networks don't broadcast, so they might not show up at all

Hidden networks don't broadcast the SSID but they are always visible for iwlist, with an empty SSID

I read up on that and indeed, they still send beacons regularly.

I thought that was not a good enough solution, but now we have something to go from.

So instead I suggest to always add a special entry to enter a SSID manually. dialogs supports keys and values in dialogs, so you can use that to distinguish between the ESSIDs and the special option.

I did try to do that initially, but failed, do you have a good example of doing this special entries?

dialog --no-tags --menu "Choose wisely" 0 0 0 "SSID=SSID 1" "SSID 1" "SSID=SSID 2" "SSID 2" "manual" "Enter SSID manually"

gmoro commented 3 years ago

By the way, I need to retab the whole thing, it's a mess of mixed tabs and spaces, I guess should wait until the next release, what do you think?

Vogtinator commented 3 years ago

By the way, I need to retab the whole thing, it's a mess of mixed tabs and spaces, I guess should wait until the next release, what do you think?

IMO that can be done at any point, but in a separate PR. It can be mentioned in the .git-blame-ignore-revs file

gmoro commented 3 years ago

LGTM code wise.

Now raspberrywifi_get_wlan_networks never fails, because even when ip link or iwlist fail, it has the manual entry which makes $list non-empty. Maybe that should be caught explicitly? Though I'm not sure how

I guess that makes the previous behaviour wrong then, because an empty list is a valid thing, there might be no networks in range, we should really start checking return codes for the commands, only way to guarantee