Closed Bug 946257 Opened 6 years ago Closed 6 years ago

[wifi][gonk-kk] Porting WifiUtils

Categories

(Firefox OS Graveyard :: Wifi, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: seinlin, Assigned: seinlin)

References

Details

Attachments

(1 file, 2 obsolete files)

WifiUtils is compiled failed in gonk-kk.
Assignee: nobody → kli
Blocks: gonk-kk
Hi, Vincent, some APIs are changed in kitkat. Could you help me to review this patch? Thank!
Attachment #8345156 - Flags: review?(vchang)
Status: NEW → ASSIGNED
Comment on attachment 8345156 [details] [diff] [review]
Update NetUtils and WifiUtils to support Kitkat.

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

Hi kli, thank you for jumping to this. I am fine with the patch. 
But we need Fabrice to review this, too.

::: dom/wifi/WifiUtils.h
@@ +109,3 @@
>  
>    virtual int32_t
> +  do_wifi_connect_to_supplicant(const char* iface) = 0; // KK == ICS != JB 

Nit: Remove trailing whitespace.

@@ +112,3 @@
>  
>    virtual void
> +  do_wifi_close_supplicant_connection(const char* iface) = 0; // KK == ICS != JB 

Nit: Remove trailing whitespace.
Attachment #8345156 - Flags: review?(vchang)
Attachment #8345156 - Flags: review?(fabrice)
Attachment #8345156 - Flags: review+
Comment on attachment 8345156 [details] [diff] [review]
Update NetUtils and WifiUtils to support Kitkat.

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

I'd really like to see systemlibs.js being cleaned up!

::: dom/system/gonk/systemlibs.js
@@ +237,5 @@
> +                        ctypes.int.ptr,   // lease
> +                        ctypes.char.ptr,  // vendorinfo
> +                        ctypes.char.ptr,  // domain
> +                        ctypes.char.ptr); // mtu
> +    } else if (sdkVersion >= 18) { // 18 == JB 4.3

I think we have no JS callers for dhcp_do_request and dhcp_do_request_renew anymore. We should just remove this code.

::: dom/wifi/NetUtils.cpp
@@ +132,5 @@
>      char domains[PROPERTY_VALUE_MAX];
>      ret = dhcp_do_request(ifname, ipaddr, gateway, prefixLength, dns,
>                            server, lease, vendorinfo, domains);
> +  } else if (sdkVersion == 19) {
> +    // JB 4.4

can you add a link to androidxref like for the other versions?
Attachment #8345156 - Flags: review?(fabrice) → feedback+
I think loading few javascript source in b2g could save memory usage. I will revert systemlibs.js and file a new bug to remove the unused API in systemlibs.js. And the new bug will block 1.3+ to benefit low memory device such Tarako.
(In reply to Kai-Zhen Li from comment #4)
> I think loading few javascript source in b2g could save memory usage. I will
> revert systemlibs.js and file a new bug to remove the unused API in
> systemlibs.js. And the new bug will block 1.3+ to benefit low memory device
> such Tarako.

No need to block 1.3 on that. We'll cherry pick for tarako though.
This needs to be landed. Kai-Zhen?
Flags: needinfo?(kli)
(In reply to Andreas Gal :gal from comment #6)
> This needs to be landed. Kai-Zhen?

Yes, this needs to be landed. But I notice that some driver command is not available in kk, such as MACADDR. So I want to got it fixed before landed.
Flags: needinfo?(kli)
Hi Fabrice,Can we land this first? Because it block the gonk-kk build. Let's file a new bug to follow the inconsistent APIs/Commands of kk.
Attachment #8345156 - Attachment is obsolete: true
Attachment #8359224 - Flags: review?(fabrice)
Comment on attachment 8359224 [details] [diff] [review]
Update NetUtils and WifiUtils for gonk-kk

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

::: dom/system/gonk/systemlibs.js
@@ +186,5 @@
>      RESET_IPV6_ADDRESSES: 0x02,
>    };
>  
>    iface.RESET_ALL_ADDRESSES = iface.RESET_IPV4_ADDRESSES |
>                                iface.RESET_IPV6_ADDRESSES

let's add the missing semicolon at the end of the line!

::: dom/wifi/WifiUtils.h
@@ +109,3 @@
>  
>    virtual int32_t
> +  do_wifi_connect_to_supplicant(const char* iface) = 0; // KK == ICS != JB 

nit: trailing whitespace

@@ +112,3 @@
>  
>    virtual void
> +  do_wifi_close_supplicant_connection(const char* iface) = 0; // KK == ICS != JB 

nit: trailing whitespace
Attachment #8359224 - Flags: review?(fabrice) → review+
nit got fixed.
Attachment #8359224 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a4d44de1d3a0
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.