diff mbox

[3/3] channel and frequency added in interface capability properties on dbus

Message ID 1482155837-8217-3-git-send-email-avichal.a@samsung.com
State Changes Requested
Headers show

Commit Message

Avichal Agarwal Dec. 19, 2016, 1:57 p.m. UTC
Signed-off-by: Avichal Agarwal <avichal.a@samsung.com>
Signed-off-by: Mayank Haarit <mayank.h@samsung.com>
Signed-off-by: Kyeong-Chae Lim <kcya.lim@samsung.com>
---
 wpa_supplicant/dbus/dbus_new_handlers.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Dan Williams Dec. 19, 2016, 3:44 p.m. UTC | #1
On Mon, 2016-12-19 at 19:27 +0530, Avichal Agarwal wrote:
> Signed-off-by: Avichal Agarwal <avichal.a@samsung.com>
> Signed-off-by: Mayank Haarit <mayank.h@samsung.com>
> Signed-off-by: Kyeong-Chae Lim <kcya.lim@samsung.com>
> ---
>  wpa_supplicant/dbus/dbus_new_handlers.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/wpa_supplicant/dbus/dbus_new_handlers.c
> b/wpa_supplicant/dbus/dbus_new_handlers.c
> index e11dd36..d0d3c88 100644
> --- a/wpa_supplicant/dbus/dbus_new_handlers.c
> +++ b/wpa_supplicant/dbus/dbus_new_handlers.c
> @@ -2360,6 +2360,7 @@ DBusMessage *
> wpas_dbus_handler_set_pkcs11_engine_and_module_path(
>  }
>  
>  
> +
>  /**
>   * wpas_dbus_getter_capabilities - Return interface capabilities
>   * @iter: Pointer to incoming dbus message iter
> @@ -2642,6 +2643,21 @@ dbus_bool_t wpas_dbus_getter_capabilities(
>  			goto nomem;
>  	}
>  
> +	if (res >= 0) {
> +		int  buf[1024];

Whitespace issues here; only one space between the type and variable
name.

> +		 int len= wpas_get_channel_freq(wpa_s, buf,2);

Formatting here; space between variable name and the '=' sign.  Also
spaces after ',' in argument lists.

> +		if(!wpa_dbus_dict_append_int32_array(&iter_dict,
> "Frequency", buf,len))

"Channels" is plural below, so perhaps this should be "Frequencies"?
 Also, these aren't ever going to be negative, so perhaps we should be
using uint32 lists instead of int32.

> +				goto nomem;
> +
> +	}
> +	if (res >= 0) {
> +			int  buf[1024];
> +			int len=wpas_get_channel_freq(wpa_s, buf,1)

Same whitespace/spacing comments here.

> +			if(!wpa_dbus_dict_append_int32_array(&iter_d
> ict, "Channels", buf,len))
> +					goto nomem;
> +
> +		}
> +
>  	if (!wpa_dbus_dict_close_write(&variant_iter, &iter_dict) ||
>  	    !dbus_message_iter_close_container(iter, &variant_iter))
>  		goto nomem;
> @@ -2654,6 +2670,8 @@ nomem:
>  }
>  
>  
> +
> +
>  /**
>   * wpas_dbus_getter_state - Get interface state
>   * @iter: Pointer to incoming dbus message iter
Dan Williams Dec. 19, 2016, 4:20 p.m. UTC | #2
On Mon, 2016-12-19 at 09:44 -0600, Dan Williams wrote:
> On Mon, 2016-12-19 at 19:27 +0530, Avichal Agarwal wrote:
> > 
> > Signed-off-by: Avichal Agarwal <avichal.a@samsung.com>
> > Signed-off-by: Mayank Haarit <mayank.h@samsung.com>
> > Signed-off-by: Kyeong-Chae Lim <kcya.lim@samsung.com>
> > ---
> >  wpa_supplicant/dbus/dbus_new_handlers.c | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> > 
> > diff --git a/wpa_supplicant/dbus/dbus_new_handlers.c
> > b/wpa_supplicant/dbus/dbus_new_handlers.c
> > index e11dd36..d0d3c88 100644
> > --- a/wpa_supplicant/dbus/dbus_new_handlers.c
> > +++ b/wpa_supplicant/dbus/dbus_new_handlers.c
> > @@ -2360,6 +2360,7 @@ DBusMessage *
> > wpas_dbus_handler_set_pkcs11_engine_and_module_path(
> >  }
> >  
> >  
> > +
> >  /**
> >   * wpas_dbus_getter_capabilities - Return interface capabilities
> >   * @iter: Pointer to incoming dbus message iter
> > @@ -2642,6 +2643,21 @@ dbus_bool_t wpas_dbus_getter_capabilities(
> >  			goto nomem;
> >  	}
> >  
> > +	if (res >= 0) {
> > +		int  buf[1024];
> 
> Whitespace issues here; only one space between the type and variable
> name.
> 
> > 
> > +		 int len= wpas_get_channel_freq(wpa_s, buf,2);
> 
> Formatting here; space between variable name and the '=' sign.  Also
> spaces after ',' in argument lists.
> 
> > 
> > +		if(!wpa_dbus_dict_append_int32_array(&iter_dict,
> > "Frequency", buf,len))
> 
> "Channels" is plural below, so perhaps this should be "Frequencies"?
>  Also, these aren't ever going to be negative, so perhaps we should
> be
> using uint32 lists instead of int32.

Here's a different thought.  The supplicant internally keeps the
channel/frequency in the same object (hostap_channel_data) and we could
do the same thing here for the dbus interface.

Having these two pieces of info as separate items in the D-Bus
interface means you cannot distinguish between the overlapping channel
#s from the different bands (channel 132 at either 5660 or 3660 MHz) or
802.11b from 802.11ad channels.

There are two ways to tie channel/frequency together in the interface:

1) array-of-dicts: a "Channels" property would be an array of dicts
(dbus signature "aa{sv}"), and each element in the array would be a
dict of channel properties.  This is most extendable to the future
should we want to add more options to each channel's properties, like
max TX power and whatnot.

2) array-of-structs: a "Channels" property would be an array of structs
(dbus signature "a(uu)") where each element is a struct of type "uu"
with the first uint32 being the frequency and the second being the
channel #.  I like this less because it's less future-proof.

Dan

> > 
> > +				goto nomem;
> > +
> > +	}
> > +	if (res >= 0) {
> > +			int  buf[1024];
> > +			int len=wpas_get_channel_freq(wpa_s,
> > buf,1)
> 
> Same whitespace/spacing comments here.
> 
> > 
> > +			if(!wpa_dbus_dict_append_int32_array(&iter
> > _d
> > ict, "Channels", buf,len))
> > +					goto nomem;
> > +
> > +		}
> > +
> >  	if (!wpa_dbus_dict_close_write(&variant_iter, &iter_dict)
> > ||
> >  	    !dbus_message_iter_close_container(iter,
> > &variant_iter))
> >  		goto nomem;
> > @@ -2654,6 +2670,8 @@ nomem:
> >  }
> >  
> >  
> > +
> > +
> >  /**
> >   * wpas_dbus_getter_state - Get interface state
> >   * @iter: Pointer to incoming dbus message iter
> 
> _______________________________________________
> Hostap mailing list
> Hostap@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/hostap
diff mbox

Patch

diff --git a/wpa_supplicant/dbus/dbus_new_handlers.c b/wpa_supplicant/dbus/dbus_new_handlers.c
index e11dd36..d0d3c88 100644
--- a/wpa_supplicant/dbus/dbus_new_handlers.c
+++ b/wpa_supplicant/dbus/dbus_new_handlers.c
@@ -2360,6 +2360,7 @@  DBusMessage * wpas_dbus_handler_set_pkcs11_engine_and_module_path(
 }
 
 
+
 /**
  * wpas_dbus_getter_capabilities - Return interface capabilities
  * @iter: Pointer to incoming dbus message iter
@@ -2642,6 +2643,21 @@  dbus_bool_t wpas_dbus_getter_capabilities(
 			goto nomem;
 	}
 
+	if (res >= 0) {
+		int  buf[1024];
+		 int len= wpas_get_channel_freq(wpa_s, buf,2);
+		if(!wpa_dbus_dict_append_int32_array(&iter_dict, "Frequency", buf,len))
+				goto nomem;
+
+	}
+	if (res >= 0) {
+			int  buf[1024];
+			int len=wpas_get_channel_freq(wpa_s, buf,1);
+			if(!wpa_dbus_dict_append_int32_array(&iter_dict, "Channels", buf,len))
+					goto nomem;
+
+		}
+
 	if (!wpa_dbus_dict_close_write(&variant_iter, &iter_dict) ||
 	    !dbus_message_iter_close_container(iter, &variant_iter))
 		goto nomem;
@@ -2654,6 +2670,8 @@  nomem:
 }
 
 
+
+
 /**
  * wpas_dbus_getter_state - Get interface state
  * @iter: Pointer to incoming dbus message iter