946257 - [wifi][gonk-kk] Porting WifiUtils

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Wifi, defect)

Description

[Kai-Zhen Li [:seinlin][:kli]]

WifiUtils is compiled failed in gonk-kk.

Comment 1

[Kai-Zhen Li [:seinlin][:kli]]

Attachment: Update NetUtils and WifiUtils to support Kitkat.

Hi, Vincent, some APIs are changed in kitkat. Could you help me to review this patch? Thank!

Comment 2

[Vincent Chang[:vchang][changyihsin]]

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.

Comment 3

[[:fabrice] Fabrice Desré]

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?

Comment 4

[Kai-Zhen Li [:seinlin][:kli]]

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.

Comment 5

[[:fabrice] Fabrice Desré]

(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.

Comment 6

[Andreas Gal :gal]

This needs to be landed. Kai-Zhen?

Comment 7

[Kai-Zhen Li [:seinlin][: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.

Comment 8

[Kai-Zhen Li [:seinlin][:kli]]

Attachment: Update NetUtils and WifiUtils for gonk-kk

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.

Comment 9

[[:fabrice] Fabrice Desré]

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

Comment 10

[Kai-Zhen Li [:seinlin][:kli]]

Attachment: Update NetUtils and WifiUtils for gonk-kk

nit got fixed.

Comment 11

[Marco Chen [:mchen]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a4d44de1d3a0

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a4d44de1d3a0