Closed Bug 790231 Opened 8 years ago Closed 7 years ago

[Wifi] Expand scan result flag

Categories

(Firefox OS Graveyard :: General, enhancement)

All
Gonk (Firefox OS)
enhancement
Not set

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 791506

People

(Reporter: masashi.honma, Unassigned)

Details

Attachments

(1 file, 2 obsolete files)

The flag area in scan result contains some useful information.
But current code uses only encryption mode.

This patch makes Gecko send all flags to Gaia.

I will use these flags to obtain WPS information.
Attached patch Patch file of this bug (obsolete) — Splinter Review
After this patch will be landed.

The pull request #4564 for Gaia needs to be landed.
Is there any comment on this ?

If you could not understand why this patch is needed, see pull request #5066 for Gaia.
Hello, Blake Kaplan.

If you have time, could you review this patch and Bug 791506 ?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment on attachment 660064 [details] [diff] [review]
Patch file of this bug

I am very much against this patch. Doing this ties the API very directly to wpa_supplicant. Looking at the pull request, why can't you simply add WPS to the list of capabilities (note that gaia will probably have to change to take that into account)?
Attachment #660064 - Flags: review-
> why can't you simply add WPS to the list of capabilities ?

First time, I have thought like you. But wpa_supplicant flags information has more useful information except WPS flags. For example "ESS flag".

If I only pass the WPS flag, I must change Gecko when I need to use another wpa_supplicant flag.

If I pass the all flags to Gaia, Gecko doesn't need more change about flags. I think changing Gecko has more impact than changing Gaia. Because Gecko is working on many platforms. So I think this patch is better way to reduce future code changes.

If changing Gecko for every flag addition is acceptable, I will change my patch as you say.
I made a new patch only changes WPS flag.
See new patch in Bug 791506.
Attached patch Patch after review (obsolete) — Splinter Review
Attachment #660064 - Attachment is obsolete: true
Flags: sec-review?
Flags: sec-review?
Attachment #666997 - Flags: review?(mrbkap)
Comment on attachment 666997 [details] [diff] [review]
Patch after review

Review of attachment 666997 [details] [diff] [review]:
-----------------------------------------------------------------

I like this patch except for the name of the new property.

How does "security" instead of "encryptions" sound to you? Kaze, I'd appreciate your suggestion as well.
Attachment #666997 - Flags: review?(mrbkap)
`encryptions' is misleading, I'd prefer `security' as well.
Attachment #666997 - Attachment is obsolete: true
> `encryptions' is misleading, I'd prefer `security' as well.

I modified my patch.
Comment on attachment 667462 [details] [diff] [review]
Patch after review 2

Is there a pull request ready to go on the gaia side so we can check this in without breaking things? This is ready to go with that.
Attachment #667462 - Flags: review+
> Is there a pull request ready to go on the gaia side so we can check this in
> without breaking things? This is ready to go with that.

I updated my PR #5528.
Hello Masashi-san,

I'm very sorry, but I had to back out this patch :(.  The gaia pull request was not merged in time, and without this patch breaks wifi.

We need to work with kaze to get the pull request merged.

https://hg.mozilla.org/integration/mozilla-inbound/rev/c924b386e2ea
OK. I will re-write the patch for master branch, and re-send it in few hours.
I re-wrote. See new PR #5697.
Looks like Masashi attached a fix for this bug in the patch he proposes for bug 791506. Closing.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 791506
You need to log in before you can comment on or make changes to this bug.