Hi,
Very sorry for the delay. Patrik is on vacation, and on my side I forgot
this thread.
Your API looks like a good start, but there is room for simplification:
less callbacks mostly,
and functions etc...
/* Data structure */
typedef enum {
WIFI_DIRECT_ERROR_NONE = 0, /**< Successful */
WIFI_DIRECT_ERROR_NOT_PERMITTED = -EPERM, /** Operation not
permitted(1) */
WIFI_DIRECT_ERROR_OUT_OF_MEMORY = -ENOMEM, /** Out of memory(12) */
WIFI_DIRECT_ERROR_RESOURCE_BUSY = -EBUSY, /** Device or resource
busy(16) */
WIFI_DIRECT_ERROR_INVALID_PARAMETER = -EINVAL, /** Invalid function
parameter(22) */
WIFI_DIRECT_ERROR_CONNECTION_TIME_OUT = -ETIMEDOUT, /**< Connection
timed out(110) */
WIFI_DIRECT_ERROR_NOT_INITIALIZED = -0x00008000|0x0201, /**< Not
initialized */
WIFI_DIRECT_ERROR_COMMUNICATION_FAILED = -0x00008000|0x0202, /**< I/O
error */
WIFI_DIRECT_ERROR_WIFI_USED = -0x00008000|0x0203, /**< WiFi is being
used */
WIFI_DIRECT_ERROR_MOBILE_AP_USED = -0x00008000|0x0204, /**< Mobile AP
is being used */
WIFI_DIRECT_ERROR_CONNECTION_FAILED = -0x00008000|0x0205, /**<
Connection failed */
WIFI_DIRECT_ERROR_AUTH_FAILED = -0x00008000|0x0206, /**<
Authentication failed */
How will you know that authentication has failed? ConnMan will just tell
you "Failed" that's it.
WIFI_DIRECT_ERROR_OPERATION_FAILED = -0x00008000|0x0207, /**<
Operation failed */
WIFI_DIRECT_ERROR_ALREADY_INITIALIZED = -0x00008000|0x0209, /**<
Already initialized client */
So it's not an error then ;)
You wifi_direct_init(), if already called once, should just return
successfully without doing anything.
WIFI_DIRECT_ERROR_CONNECTION_CANCELED, /**< Connection canceled by
local device */
If something/someone is canceling a connection, it's not an error.
} wifi_direct_error_e;
Anyway, why not using standard linux errors? You add some complexity to
yourself here.
If every callback/functions are properly documented: dev will know what
-EBUSY will mean in
the context of a specific function etc...
typedef enum {
WIFI_DIRECT_DEVICE_STATE_ACTIVATED,
WIFI_DIRECT_DEVICE_STATE_DEACTIVATED,
} wifi_direct_device_state_e;
You mean the technology here, since you don't have any access to any
"device".
Just remove "device" from the name.
typedef enum {
WIFI_DIRECT_CONNECTION_IDLE,
WIFI_DIRECT_CONNECTION_CONNECTING,
WIFI_DIRECT_CONNECTION_CONNECTED,
WIFI_DIRECT_CONNECTION_IPASSIGNED,
Connected/Ipassigned are equal. There is no proper P2P connection
without an IP.
ConnMan will drop the connection request if it cannot get any IP.
WIFI_DIRECT_CONNECTION_DISCONNECTING,
WIFI_DIRECT_CONNECTION_DISCONNECTED,
} wifi_direct_connection_state_e;
typedef enum {
WIFI_DIRECT_CATEGORY_COMPUTER_PC,
WIFI_DIRECT_CATEGORY_COMPUTER_SERVER,
WIFI_DIRECT_CATEGORY_COMPUTER_MEDIA_CTR,
WIFI_DIRECT_CATEGORY_COMPUTER_UMPC,
WIFI_DIRECT_CATEGORY_COMPUTER_NOTEBOOK,
WIFI_DIRECT_CATEGORY_COMPUTER_DESKTOP,
WIFI_DIRECT_CATEGORY_COMPUTER_MID,
WIFI_DIRECT_CATEGORY_COMPUTER_NETBOOK,
WIFI_DIRECT_CATEGORY_INPUT_KEYBOARD,
WIFI_DIRECT_CATEGORY_INPUT_MOUSE,
WIFI_DIRECT_CATEGORY_INPUT_JOYSTICK,
WIFI_DIRECT_CATEGORY_INPUT_TRACKBALL,
WIFI_DIRECT_CATEGORY_INPUT_CONTROLLER,
WIFI_DIRECT_CATEGORY_INPUT_REMOTE,
WIFI_DIRECT_CATEGORY_INPUT_TOUCHSCREEN,
WIFI_DIRECT_CATEGORY_INPUT_BIO_READER,
WIFI_DIRECT_CATEGORY_INPUT_BAR_READER,
WIFI_DIRECT_CATEGORY_PRINTER_PRINTER,
WIFI_DIRECT_CATEGORY_PRINTER_SCANNER,
WIFI_DIRECT_CATEGORY_PRINTER_FAX,
WIFI_DIRECT_CATEGORY_PRINTER_COPIER,
WIFI_DIRECT_CATEGORY_PRINTER_ALLINONE,
WIFI_DIRECT_CATEGORY_CAMERA_DIGITAL_STILL,
WIFI_DIRECT_CATEGORY_CAMERA_VIDEO,
WIFI_DIRECT_CATEGORY_CAMERA_WEBCAM,
WIFI_DIRECT_CATEGORY_CAMERA_SECONDARYURITY,
WIFI_DIRECT_CATEGORY_STORAGE_NAS,
WIFI_DIRECT_CATEGORY_NETWORK_INFRA_AP,
WIFI_DIRECT_CATEGORY_NETWORK_INFRA_ROUTER,
WIFI_DIRECT_CATEGORY_NETWORK_INFRA_SWITCH,
WIFI_DIRECT_CATEGORY_NETWORK_INFRA_GATEWAY,
WIFI_DIRECT_CATEGORY_DISPLAY_TV,
WIFI_DIRECT_CATEGORY_DISPLAY_PIC_FRAME,
WIFI_DIRECT_CATEGORY_DISPLAY_PROJECTOR,
WIFI_DIRECT_CATEGORY_DISPLAY_MONITOR,
WIFI_DIRECT_CATEGORY_MULTIMEDIA_DAR,
WIFI_DIRECT_CATEGORY_MULTIMEDIA_PVR,
WIFI_DIRECT_CATEGORY_MULTIMEDIA_MCX,
WIFI_DIRECT_CATEGORY_MULTIMEDIA_STB,
WIFI_DIRECT_CATEGORY_MULTIMEDIA_MSMAME,
WIFI_DIRECT_CATEGORY_MULTIMEDIA_PVP,
WIFI_DIRECT_CATEGORY_GAME_XBOX,
WIFI_DIRECT_CATEGORY_GAME_XBOX_360,
WIFI_DIRECT_CATEGORY_GAME_PS,
WIFI_DIRECT_CATEGORY_GAME_CONSOLE,
WIFI_DIRECT_CATEGORY_GAME_PORTABLE,
WIFI_DIRECT_CATEGORY_PHONE_WM,
WIFI_DIRECT_CATEGORY_PHONE_SINGLE,
WIFI_DIRECT_CATEGORY_PHONE_DUAL,
WIFI_DIRECT_CATEGORY_PHONE_SM_SINGLE,
WIFI_DIRECT_CATEGORY_PHONE_SM_DUAL,
WIFI_DIRECT_CATEGORY_AUDIO_TUNER,
WIFI_DIRECT_CATEGORY_AUDIO_SPEAKER,
WIFI_DIRECT_CATEGORY_AUDIO_PMP,
WIFI_DIRECT_CATEGORY_AUDIO_HEADSET,
WIFI_DIRECT_CATEGORY_AUDIO_HEADPHONE,
WIFI_DIRECT_CATEGORY_AUDIO_MIC,
} wifi_direct_category_e;
Currently ConnMan does not provide peer's device type/secondary type.
We have to figure out a way to do it properly.
typedef enum {
WIFI_DIRECT_WPS_TYPE_NONE = 0x00, /**< No WPS type */
WIFI_DIRECT_WPS_TYPE_PBC = 0x01, /**< Push Button Configuration */
WIFI_DIRECT_WPS_TYPE_PIN_DISPLAY = 0x02, /**< Display PIN code */
WIFI_DIRECT_WPS_TYPE_PIN_KEYPAD = 0x04, /**< Provide the keypad to
input the PIN */
Don't make such differentiation. It's up to the user to enter the PIN.
(UI can propose a default one if necessary)
We won't use PIN generation from wpa_supplicant.
} wifi_direct_wps_type_e;
typedef enum {
WIFI_DIRECT_SERVICE_ALL,
WIFI_DIRECT_SERVICE_BONJOUR,
WIFI_DIRECT_SERVICE_UPNP,
WIFI_DIRECT_SERVICE_WSDISCOVERY,
WIFI_DIRECT_SERVICE_WIFIDISPLAY,
WIFI_DIRECT_SERVICE_VENDORSPEC = 0xff,
} wifi_direct_service_type_e;
You will have to parse the IE's of each peer service. That's ok, apps
needs a proper peer service library.
typedef enum {
WIFI_DIRECT_DISPLAY_SOURCE,
WIFI_DIRECT_DISPLAY_PRIMARY_SINK,
WIFI_DIRECT_DISPLAY_SECONDARY_SINK,
WIFI_DIRECT_DISPLAY_DUAL_ROLE,
} wifi_direct_display_type_e;
typedef struct {
char *identify:
char *name;
char *ip_addr;
char *netmask;
wifi_direct_category_e category;
wifi_direct_peer_state_e state;
} wifi_direct_peer_info_s;
/******************************************************************************
* Notification callback function type
* Discover notification can occurs at peers are found
*
*/
typedef bool (*wifi_direct_discovered_peer_cb) (wifi_direct_peer_info_s *peer,
void *user_data);
What's the difference with wifi_direct_peers_changed_cb below?
You might drop that one.
/******************************************************************************
* Notification callback function type
* Connected peer notification can occurs at a peers connected
*
*/
typedef bool (*wifi_direct_connected_peer_cb) (wifi_direct_peer_info_s *peer,
void *user_data)
Maybe you don't want such callback also.
Again wifi_direct_connection_state_changed_cb is good enough here
/******************************************************************************
* Notification callback function type
* Activate notification can occurs at WiFi Direct activated
*
*/
typedef void (*wifi_direct_device_state_changed_cb) (int error_code,
wifi_direct_device_state_e device_state,
void *user_data);
You mean the technology here, since you don't have any access to any
"device".
Just remove "device" from the name.
/******************************************************************************
* Notification callback function type
* Connection notification can occurs at WiFi Direct connection state changed
*
*/
typedef void (*wifi_direct_connection_state_changed_cb) (int error_code,
wifi_direct_connection_state_e connection_state,
wifi_direct_peer_info_s *peer, void *user_data);
/******************************************************************************
* Notification callback function type
* Peer changed notification can occurs at remote peers added or removed
*
*/
typedef void (*wifi_direct_peers_changed_cb) (void *user_data);
/******************************************************************************
* Notification callback function type
* Incoming connection notification can occurs at a incoming request received
*
*/
typedef void (*wifi_direct_incoming_connection_cb) (wifi_direct_wps_type_e type,
wifi_direct_peer_info_s *peer, void *user_data);
What about out-going connection that needs a PIN input?
Either add a new callback for PIN input for out-going connection, or try
to mix both use case in one.
/* Inteface */
/******************************************************************************
* This API will do some initialization the various variables
*
*
*/
int wifi_direct_initialize(void);
/******************************************************************************
* This API will do some deinitialization.
*
*
*/
int wifi_direct_deinitialize(void);
/******************************************************************************
* This API shall register the activation callback function to monitor WiFi
* Direct state.
*
*/
int wifi_direct_set_device_state_changed_cb(
wifi_direct_device_state_changed_cb cb,
void *user_data);
/******************************************************************************
* This API shall unregister the activation callback function.
*
*
*/
int wifi_direct_unset_device_state_changed_cb(void);
I would remove that function. Just allow
wifi_direct_set_device_state_changed_cb()
to accept a NULL pointer as a cb. Thus this will be your unset function
as well.
It's just a matter of documenting the function so developer knows about
that behavior.
/******************************************************************************
* This API shall register the connection callback function to monitor WiFi
* Direct connection state.
*
*/
int wifi_direct_set_connection_state_changed_cb(
wifi_direct_connection_state_changed_cb cb,
void *user_data);
/******************************************************************************
* This API shall unregister the connection callback function.
*
*
*/
int wifi_direct_unset_connection_state_changed_cb(void);
Same here.
/******************************************************************************
* This API shall register the peers changed callback function to monitor WiFi
* Direct remote peers changed. the callback will be invoke after scan.
*
*/
int wifi_direct_set_peers_changed_cb(wifi_direct_peers_changed_cb cb,
void *user_data);
/******************************************************************************
* This API shall unregister the peers changed callback function.
*
*
*/
int wifi_direct_unset_peers_changed_cb(void);
Same here
/******************************************************************************
* This API shall register a incoming connection callback function.
* The callback will be invoke when a incoming connection received.
*
*/
int wifi_direct_set_incoming_connection_cb(
wifi_direct_incoming_connection_cb cb,
void *user_data);
/******************************************************************************
* This API shall unregister the incoming connection callback function.
*
*
*/
int wifi_direct_unset_incoming_connection_cb(void);
Same here.
/******************************************************************************
* This API shall get all the peers, and invoke the callback with the peers
* as a parameters.
*
*/
int wifi_direct_foreach_discovered_peers(wifi_direct_discovered_peer_cb cb,
void *user_data);
/******************************************************************************
* This API shall get all the connect peers, and invoke the callback with the
peers
* as a parameters.
*
*/
int wifi_direct_foreach_connected_peers(wifi_direct_connected_peer_cb cb,
void *user_data);
Hum here you work differently: no unset functions (which should be the way),
but when does the dev needs to call that? The foreach is semantically a
bit misleading,
moreover if you compare with previous functions.
Instead of _foreach_ I would stick with _set_.
/******************************************************************************
* This API shall activate WiFi direct
*
*
*/
int wifi_direct_activate(void);
/******************************************************************************
* This API shall deactivate WiFi direct
*
*
*/
int wifi_direct_deactivate(void);
See, here you did not put "device" which is the way to go ;)
/******************************************************************************
* This API shall start a P2P Scan process to find the peers
*
*
*/
int wifi_direct_start_discovery(void);
/******************************************************************************
* This API shall start or accept a p2p request.
* 1 Start a P2P outgoing request with PBC mothod.
* 2 Accept a P2P incoming request with PBC mothod.
s/mothod/method
* 3 Invite a peer into current P2P connection.
*/
int wifi_direct_connect(const wifi_direct_peer_info_s *peer);
/******************************************************************************
* This API shall start a p2p request with PIN dispaly. Application will
* get the PIN code form the*pin* parameters, and display to Users.
*
*/
int wifi_direct_connect_with_display_pin(const wifi_direct_peer_info_s *peer,
char **pin);
You can remove that one since connman won't provide you a generated PIN.
/******************************************************************************
* This API shall accept a p2p request with PIN keypad. Application should
* set the PIN code which get from remote peer's display.
*
*/
int wifi_direct_connect_with_enter_pin(const wifi_direct_peer_info_s *peer,
const char *pin);
/******************************************************************************
* This API shall ending a P2P connection
* 1 Reject a incoming P2P request.
* 2 Cancel a outgoing connecting request.
* 3 Destory a founded P2P connection with a peer.
*/
int wifi_direct_disconnect(const wifi_direct_peer_info_s *peer);
/******************************************************************************
* This API shall ending all the P2P connection with local device.
Remove "with local device". It's P2P so it's with local device.
*
*
*/
int wifi_direct_disconnect_all(void);
/******************************************************************************
* This API shall set local device name
*
*
*/
int wifi_direct_set_device_name(const char *device_name);
Remove it. We won't expose this from DBus. Currently, ConnMan sets the
hostname as
the device name or "ConnMan" (if the hostname is missing which _should
not_ happen).
We could get a /etc/main.conf entry to set a specific one if really
necessary but I am not found of this either.
/******************************************************************************
* This API shall get local device name
*
*
*/
int wifi_direct_get_device_name(char **device_name);
Remove it as well. App should get the hostname.
/******************************************************************************
* This API shall set local device category
*
*
*/
int wifi_direct_set_category(wifi_direct_category_e category);
/******************************************************************************
* This API shall get local device category
*
*
*/
Remove those both. The device category is something related to system
configuration and integration.
No app will ever need to set this, and getting it either won't help much
for anything.
int wifi_direct_get_category(wifi_direct_category_e *category);
/******************************************************************************
* This API shall get local device state
No device word here, just the state.
*
*
*/
int wifi_direct_get_device_state(wifi_direct_state_e *state);
Why not just enum wifi_direct_state_e wifi_direct_get_state(void); ?
Much simpler.
/******************************************************************************
* This API shall get local device connection state
*
*
*/
int wifi_direct_get_device_connect_state(wifi_direct_connection_state_e *state);
Same issue with signature.
Btw, you get the technology state here, but is it that important? Isn't
it better for the app to get
the peer list as usual and check the state. I don't see a use case
currently where an app would
be happy just to know p2p tech is connected, without knowing to which peer.
Just wondering.
/******************************************************************************
* This API shall register a WiFi Direct service
*
*
*/
int wifi_direct_service_add(wifi_direct_service_type_e type, char *data1,
char *data2);
What are data1 and data2?
/******************************************************************************
* This API shall unregister a WiFi Direct service
*
*
*/
int wifi_direct_service_del(wifi_direct_service_type_e type, char *data1,
char *data2);
Same here
/******************************************************************************
* This API shall do a WiFi Display initialization with type, port, hdcp
*
*
*/
int wifi_direct_init_wifi_display(wifi_direct_display_type_e type, int port,
int hdcp);
/******************************************************************************
* This API shall do a WiFi Display deinitialization.
*
*
*/
int wifi_direct_deinit_wifi_display(void);
/******************************************************************************
* This API shall get WiFi Display port.
*
*
*/
int wifi_direct_get_display_port(int *port);
/******************************************************************************
* This API shall get WiFi Display device type.
*
*
*/
int wifi_direct_get_display_type(wifi_direct_display_type_e *type);
All this last function are Wifi Display related, not sure at all you
want to put it in the same basket
as wifi direct library.
Tomasz
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
_______________________________________________
Dev mailing list
[email protected]
https://lists.tizen.org/listinfo/dev
