Re: API changes

[Will Stephenson <wstephenson kde org>]

On Thursday 13 March 2008, Dan Williams said:
> A few API changes, both in the Settings specification, and in D-Bus.
> Some of these are thanks to Will's documentation effort and mini-review
> of the API while he was doing that.

Great, glad to see the docu work is helping.

And here's a bunch more API question marks that Helmut and myself came up 
with:

They are organised by interface, and numbered, just grep for the #'s.

Will


--------------------------------

org.freedesktop.NetworkManager
Methods:
#1
ActivateDevice ( o: device, s: service_name, o: connection, o: 
specific_object ) → nothing

suggestion: move to org.freedesktop.NetworkManager.Device.Activate(s: 
service_name, o: connection, o: specific_object ) → nothing

rationale: Helmut told me that this had already been discussed, and that 
because you could imagine Connection.Activate( Device ) or Device.Activate( 
Connection) it was decided to put it on the manager.  If this was 
communicated properly,  this is a worst of all compromise.  Since Connection 
is on the settings service, the action can't be carried out there, so stay 
object-oriented and move this to Device.

As I'm wrapping NM in Solid for KDE 4, I'll probably move this method onto the 
Device class anyway, but moving it in NM would be better for both projects 
IMO.

#2
  errors:
    org.freedesktop.NetworkManager.Error.ConnectionActivating
    Another connection is already activating or the same connection is already 
active.

org.freedesktop.NetworkManager.Error.ConnectionActivating

Does this mean the same connection is already activating or is another 
connection activating on this device? 

Suggestion: split into 
org.freedesktop.NetworkManager.Error.ConnectionAlreadyActive
org.freedesktop.NetworkManager.Error.ConnectionActivating

Although I'm not sure when this error would be raised, since a client might 
want to interrupt a connection attempt (maybe started with bad parameter, 
maybe it's going wrong and failing to error).

#3
Sleep ( b: sleep ) → nothing
 Control the NetworkManager daemon's sleep state. When asleep, all interfaces 
that it manages are deactivated. 
Parameters
sleep - b
 Indicates whether the NetworkManager daemon should sleep or wake. 

suggestion: It's not clear from the method name what this does
SetSleepState( b: asleep ) perhaps?

#4
enum:
NM_STATE_CONNECTING = 2
 The NetworkManager daemon is connecting a device. 
What does this mean when one device is already active and another is 
connecting?

org.freedesktop.NetworkManager.AccessPoint
#5

roperties:
HwAddress - s - (read)
The hardware address of the access point. 
suggestion: make this uint64 type="t"?  IP addresses are uints, and uint64 
fits both 48 bit and 64 bit MAC addresses, and is smaller than the unicode 
string needed for a macaddress.

org.freedesktop.NetworkManager.Device.Wired
Properties:
#6
HwAddress - s - (read)
 Hardware address of the device. 
Suggestion: change to 't' as above

#7
org.freedesktop.NetworkManagerSettings
Signals:
NewConnection ( o: connection )
Suggestion: rename to ConnectionAdded to be consistent with 
NetworkManager.DeviceAdded

#8
Where is ConnectionRemoved on this interface?  See comment #11 below.


Methods:
#9
ListConnections ( ) → ao
Suggestion: rename to GetConnections to be consistent with other Get list type 
methods


org.freedesktop.NetworkManagerSettings.Connection
#10
Suggestion: for the settings maps, switch to u instead of s keys defined by 
well-known enums - faster, smaller, safer

Signals:
#11
Removed ( )
Emitted when this settings object was removed. 

IIUC this is emitted before the object is removed, or else DBus would likely 
have a fit.  If this is the case, then it has different semantics to 
NetworkManager.DeviceRemoved(o).  If prior to removal, should be called 
AboutToBeRemoved.  Clarify if what happens to this object after it emits this 
signal.

Suggestion: move to NetworkManagerSettings.ConnectionRemoved to be emitted 
after removal

org.freedesktop.NetworkManager.Device
Properties:
#12
Ip4Address - i - (read)
 The IPv4 address actually bound to the device. 
FIXME: wrong type - should be u.
#13
FIXME: devices can have >1 IP address so make this 'au' with the canonical 
address first, OR state that secondary IP addresses are listed in the 
Ip4Config.
#14
Ip4Config - o - (read)
FIXME: make clear that is is the data used to configure the device, not the 
actual configuration
QUESTION: since in the case of a dhcp IPv4 configuration, the referenced 
Ip4Config from the Settings will know nothing about the actual IP address, 
netmask etc, so where are these available apart from the Ip4Address above?

#15
HwAddress - s - (read)
 The hardware address of the device. 
suggestion: uint64 (type="t") as above

#16
org.freedesktop.NetworkManager.IP4Config
Address - u - (read)
IPv4 address. 
Suggestion: what about multiple addresses?change to 'au'

#17
Hostname - s - (read)
The hostname associated with this IPv4 address.
Suggestion: what about multiple hostnames? change to 'as' with canonical 
hostname first

org.freedesktop.NetworkManager.VPN.Manager
ListConnections ( ) → ao
#18
 Get the list of active VPN connections.
suggestion: rename to GetConnections for consistency with other get-list type 
methods

errors:
#19
org.freedesktop.NetworkManager.VPN.Error.BadArguments
org.freedesktop.NetworkManager.VPN.Error.WrongState
suggestion: too general to be useful - should have discrete specific errors to 
describe the problem - examples?

org.freedesktop.NetworkManager.VPN.Plugin
method:
#20
NeedSecrets (a{sa{sv}}) -> s
Suggestion: sounds like a signal. Rename to 
IdentifySecretSettingsForConnection.


NM_VPN_CONNECTION_STATE enum:
#21
NM_VPN_CONNECTION_STATE_CONNECT = 3
 The VPN connection is being established. 
suggestion: Should be CONNECTING so not confused with an action.

#22
NM_VPN_CONNECTION_STATE_IP_CONFIG_GET = 4
suggestion:
 The VPN connection is getting an IP address. FIXME: Should be 
NM_VPN_CONNECTION_STATE_IP_CONFIG_GETTING so isn't confused with an action