Skip to content
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

feat: attempt to use channel of $WIFI_IFACE if setting fails #450

Closed

Conversation

sudoAlphaX
Copy link

This patch attempts to fallback to an existing $WIFI_IFACE_CHANNEL if
current $CHANNEL does not work. In cases, hardware reports as multiple
channels available to transmit, hence the script uses channel 1 as
default. But it fails when trying to transmit on channel 1.

This patch modifies can_transmit_to_channel function to check if
$CHANNEL is equal to $WIFI_IFACE_CHANNEL. If not, it sets $CHANNEL to be
the same as $WIFI_IFACE_CHANNEL and checks can_transmit_to_channel. If
passed, ap is created on the same channel as $WIFI_IFACE_CHANNEL. Else,
it safely fails.

This patch attempts to fallback to an existing $WIFI_IFACE_CHANNEL if
current $CHANNEL does not work. In cases, hardware reports as multiple
channels available to transmit, hence the script uses channel 1 as
default. But it fails when trying to transmit on channel 1.

This patch modifies can_transmit_to_channel function to check if
$CHANNEL is equal to $WIFI_IFACE_CHANNEL. If not, it sets $CHANNEL to be
the same as $WIFI_IFACE_CHANNEL and checks can_transmit_to_channel. If
passed, ap is created on the same channel as $WIFI_IFACE_CHANNEL. Else,
it safely fails.
This patch is an improvement to 88f7261 where instead of trying to use
$WIFI_IFACE_CHANNEL if $CHANNEL cannot transmit, it will instead try
only if channel is not explicitly specified (channel=default)
Copy link

@jfly jfly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have one of those cards that seems to only supports an AP on the current channel, as reported in #344. Unfortunately, your diff doesn't quite work for me.

[[ "${CHANNEL_INFO}" == *no\ IR* ]] && return 1
[[ "${CHANNEL_INFO}" == *disabled* ]] && return 1

if [[ -z "${CHANNEL_INFO}" || "${CHANNEL_INFO}" == *no\ IR* || "${CHANNEL_INFO}" == *disabled* ]]; then
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This (unfortunately) doesn't work for me. In my case, my card is currently connected to channel 8:

$ iw wlp0s20f3 info
Interface wlp0s20f3
	ifindex 2
	wdev 0x1
	addr 2c:33:58:fe:8e:ea
	type managed
	wiphy 0
	channel 8 (2447 MHz), width: 20 MHz, center1: 2447 MHz
	txpower 22.00 dBm
	multicast TXQ:
		qsz-byt	qsz-pkt	flows	drops	marks	overlmt	hashcol	tx-bytes	tx-packets
		0	0	0	0	0	0	0	0		0

Yet the output of iw phy phy0 info that you're parsing here doesn't seem to care, the default channel (channel 1) will happily skip this if statement and go on to return 0, which then fails because my card can't actually start a AP on channel 1 when it's currently connected to channel 8.

...
		Frequencies:
			* 2412.0 MHz [1] (22.0 dBm)
			* 2417.0 MHz [2] (22.0 dBm)
			* 2422.0 MHz [3] (22.0 dBm)
			* 2427.0 MHz [4] (22.0 dBm)
			* 2432.0 MHz [5] (22.0 dBm)
			* 2437.0 MHz [6] (22.0 dBm)
			* 2442.0 MHz [7] (22.0 dBm)
			* 2447.0 MHz [8] (22.0 dBm)
			* 2452.0 MHz [9] (22.0 dBm)
			* 2457.0 MHz [10] (22.0 dBm)
			* 2462.0 MHz [11] (22.0 dBm)
			* 2467.0 MHz [12] (22.0 dBm)
			* 2472.0 MHz [13] (22.0 dBm)
			* 2484.0 MHz [14] (disabled)

What exactly are we trying to check for here? If the chosen channel will work? Is there a better way of checking that? Happy to share output of more commands on my laptop if useful.

The code you've written here does work for me if I just remove this if statement entirely. I'm guessing that's not the right fix, though: we'd always go on to switch from the default channel to the wifi iface channel.

(For the record, things fail for me in a completely different way earlier on if I use iwconfig instead of iw. I can share information about that if it's useful)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just reused the if checks and wrapped them inside a single if condition using or operators. I do not exactly know what those original checks do:

   [[ -z "${CHANNEL_INFO}" ]] && return 1
        [[ "${CHANNEL_INFO}" == *no\ IR* ]] && return 1
        [[ "${CHANNEL_INFO}" == *disabled* ]] && return 1

It was because i wanted those checks to be on the same line to continue.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha. I guess this is a different issue. We don't have to solve it in this PR.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what i think is that the selected channel must exist, should not be a "no IR" and should not be disabled. else, can_transmit_to_channel returns 1 (fail). I do not know what "no IR" is.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I see, you are using channel 8. And there is no "no IR", nor "disabled" text in your output of iw phy phy0 info. There is no reason why the function should return 1.

I will refactor this pr. Please test that with your setup when I do. I'll be happy to resolve any issues.


# Attempt to use the channel of the interface
echo "Unable to transmit using channel $CHANNEL_NUM. Attempting $WIFI_IFACE_CHANNEL." >&2
CHANNEL=$WIFI_IFACE_CHANNEL
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This feels like a bit of a hack. I wouldn't expect a function named can_transmit_to_channel to have a side effect like this. Have you considered structuring this differently? Perhaps a "find_channel" function that calls can_transmit_to_channel internally, and does have this side-effect?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking back, i realize how bad this makes the can_transmit_to_channel function. I will change it soon and correct. But I will close this pr and open a new one because now i see that the entire pr is a hack. Sorry!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, please cc me on the next PR so I see it go by.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants