Issues with new D-Bus API

[Marcel Holtmann]

Hi guys,

so I have been hacking on a client using the new D-Bus API and found a
couple of things that are sort of weird and could be done way simpler.
Some of them seems just copied from the old API and I think it is time
to fix this now before we actual get real users of this API.

1) DebugParams parameter as a struct is too complex

The specification describes this as debugging properties. The structure
elements are: debug level (i), show timestamps (b), show keys (b).

This is just again properties of properties and a bit too complex. I do
prefer if I can use tools like d-feet and just go in and change on value
here instead of dealing with D-Bus structs.

So my proposal would be to use "DebugLevel", "DebugShowTimestamps" and
"DebugShowKeys" native properties instead. In addition this allows to
switch timestamps on/off without reading out the whole debug struct
first.

2) The InterfaceAdded signal is actually InterfaceCreated

The specification talks about the InterfaceAdded signal and that is how
it should be. Even if the actual method is called CreateInterface. The
current codes emits InterfaceCreated signal. It propose fixing the code
to match up the specification and be in sync other signals like BSSAdded
and NetworkAdded.

3) CreateInterface properties are weird

The specification list the "Bridge_ifname" property. That should be
called "BridgeIfname" to match the name of the interface properties.

In addition I prefer if we can have "Ifindex" property and only "Ifname"
or "Ifindex are required to be provided. Same goes for including the
"Ifname" property for the interface.

Having the interface name is nice, but it is not guaranteed to be
stable. I looked through the wpa_supplicant code and it doesn't really
expose the index, but the nl80211 driver does. I am worried about
interface renames while WiFi operation is in place. The only stable
identifier is the index in Linux.

And just supporting both, "Ifname" and "Ifindex", doesn't bloat the API
at all. It actually does make it easier for the client.

4) Changes to Scanning property

The Scanning property of an interface only changes if wpa_supplicant
triggers the scanning. Not if you trigger it via iw or other tools. The
kernel emits a signal when a scanning is started. So at least with the
nl80211 we should be using that one and change Scanning property to
match the actual scanning state of the device.

5) StateChanged signal is pointless

The StateChanged signal is pretty much pointless. I would actually
prefer if we just send a PropertiesChanged signal with the State
property instead. I know that we potentially loose the old state, but
that is not a problem since that is available via the property in the
first place. And every client needs to track all state changes anyway.
So having the old state in that signal is at no real benefit. It just
makes the API uneven and the clients more complex.

6) Usefulness of ScanDone signal

The ScanDone signal is pretty nice to have, but currently it only shows
if the scan succeeded or failed. What about actually including the
information from the nl80211 driver on why the scan was triggered?

An alternative would be to make the Scan method actually return the
result. Especially with the BSSAdded and BSSRemoved doing all the work,
that might be a way better alternative. Essentially the client only
cares about the results of scans it triggered anyway.

7) Change of BSS properties

As mentioned earlier the BSS properties should be native properties and
not properties of properties.

In addition we should be removing "Quality" and "Noise" since they are
either pointless or the kernel is not exporting them anyway.

Rename "Level" into "Signal" to match what iw is showing. Does anything
using "RSSI" for this would be better? And lets make this a int16
instead of using a int32. That is legacy WEXT crap.

The "Frequency" property should be uint16 instead of int32. I really
have no idea why it is int32. That is just pointless.

Remove "Capabilities" and replace it with "Mode" as string. The valid
modes should match the interface capabilities. This makes it nicely
consistent. Also use "Privacy" as boolean to indicate encryption.

Is anybody really using "MaxRate" property. Wouldn't it better to export
"Rates" as array of uint16? And if it is sorted the client could easily
extract the max rate from there.

8) Network properties

That thing is still a mess and we should do something about it. Having
everything as strings is kinda bad. I do remember we talked about it,
but not being able to clearly match up a SSID from a network with a SSID
from a BSS. I do think we should have something cleaner and simpler
there and not necessary copy the syntax of the wpa_supplicant
configuration file. Especially the interface capabilities like KeyMgmt
should match the property names we use in the network configuration.

Also there should be an easy way to change a property later one. Like
for example change the "Priority". Then we can have multiple network
configuration and modify them at runtime. And don't need to remove and
re-add them.

9) Miscellaneous

I already mentioned that we should add the full dictionary of properties
as second argument to the signals InterfaceAdded, NetworkAdded and
BSSAdded. I do think that we can leave BlobAdded out of this since it
just is a data storage.

I haven't played with the WPS interface so far, but I will be doing that
soon.

Other than these things, I am more than happy with the new API and
actually like it a lot. It works pretty smoothly.

Regards

Marcel