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

[Dan Williams]

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 at samsung.com>
> > Signed-off-by: Mayank Haarit <mayank.h at samsung.com>
> > Signed-off-by: Kyeong-Chae Lim <kcya.lim at 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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/hostap