[PATCH 2/2] Added WDS Bind Mux Data Port message
Aleksander Morgado
aleksander at aleksander.es
Mon Feb 20 19:31:54 UTC 2017
Hey,
On Thu, Feb 16, 2017 at 7:48 AM, Carlo Lobrano <c.lobrano at gmail.com> wrote:
> This message is used to bind a muxed data port to a controller device.
> The Muxed data port has to be managed by qmi_wwan driver.
>
> The Muxed data port is identified by:
> - mux_id: the numeric ID given to qmi_wwan once created
> - interface number: the interface number of the qmi controller device on
> the modem
>
> Once the binding is completed, all the commands sent (and I expect also
> received, but I could not test it) using the same Client ID are for the
> binded data port instead of the real one.
> ---
> data/qmi-service-wds.json | 34 ++++++++-
> src/libqmi-glib/qmi-enums-wds.h | 19 +++++
> src/libqmi-glib/qmi-enums.h | 10 ++-
> src/qmicli/qmicli-wds.c | 155 ++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 215 insertions(+), 3 deletions(-)
>
> diff --git a/data/qmi-service-wds.json b/data/qmi-service-wds.json
> index dc29998..d037481 100644
> --- a/data/qmi-service-wds.json
> +++ b/data/qmi-service-wds.json
> @@ -1707,6 +1707,38 @@
> "format" : "guint32" },
> { "name" : "APN",
> "format" : "string" } ] },
> - "prerequisites": [ { "common-ref" : "Success" } ] } ] }
> + "prerequisites": [ { "common-ref" : "Success" } ] } ] },
>
> +// *********************************************************************************
> + { "name" : "Bind Mux Data Port",
> + "type" : "Message",
> + "service" : "WDS",
> + "id" : "0x00A2",
> + "since" : "1.18",
> + "input" : [ { "name" : "Endpoint Info",
> + "id" : "0x10",
> + "mandatory" : "no",
> + "type" : "TLV",
> + "since" : "1.18",
> + "format" : "sequence",
> + "contents" : [ { "name" : "Endpoint Type",
> + "format" : "guint32",
> + "public-format" : "QmiDataEndpointType"},
> + { "name" : "Interface Number",
> + "format" : "guint32" }]},
> + { "name" : "Mux ID",
> + "id" : "0x11",
> + "type" : "TLV",
> + "since" : "1.18",
> + "mandatory" : "no",
> + "format" : "guint8"},
> + { "name" : "Client Type",
> + "id" : "0x13",
> + "type" : "TLV",
> + "since" : "1.18",
> + "mandatory" : "no",
> + "format" : "guint32",
> + "public-format" : "QmiWdsClientType"}
> + ],
> + "output" : [ { "common-ref" : "Operation Result" } ] }
> ]
> diff --git a/src/libqmi-glib/qmi-enums-wds.h b/src/libqmi-glib/qmi-enums-wds.h
> index 49edb82..e3bd6e2 100644
> --- a/src/libqmi-glib/qmi-enums-wds.h
> +++ b/src/libqmi-glib/qmi-enums-wds.h
> @@ -1904,4 +1904,23 @@ typedef enum { /*< underscore_name=qmi_wds_qos_class_identifier >*/
> * Since: 1.18
> */
>
> +/**
> + * QmiWdsClientType:
> + * @QMI_CLIENT_TYPE_UNDEFINED: no client type defined
> + * @WDS_CLIENT_TYPE_TETHERED: client type tethered
QMI_ prefix missing.
> + *
> + * Client Type
> + *
> + * Since: 1.18
> + */
> +typedef enum { /*< underscore_name=qmi_wds_client_type > */
> + WDS_CLIENT_TYPE_TETHERED = 0x01,
> + WDS_CLIENT_TYPE_UNDEFINED = 0xFF,
QMI_ prefix missing.
> +} QmiWdsClientType;
> +
> +/**
> + * qmi_wds_client_type_get_string:
> + *
> + * Since: 1.18
> + */
> #endif /* _LIBQMI_GLIB_QMI_ENUMS_WDS_H_ */
> diff --git a/src/libqmi-glib/qmi-enums.h b/src/libqmi-glib/qmi-enums.h
> index 77ee5c3..bda2aed 100644
> --- a/src/libqmi-glib/qmi-enums.h
> +++ b/src/libqmi-glib/qmi-enums.h
> @@ -151,11 +151,17 @@ typedef enum {
> * @QMI_DATA_EP_TYPE_HSUSB: Data Endpoint Type HSUSB.
> *
> * Data Endpoint Type.
> + *
> + * Since: 1.18
> */
Please add this to the previous patch.
> -typedef enum {
> +typedef enum { /*< underscore_name=qmi_data_endpoint_type > */
> QMI_DATA_EP_TYPE_HSUSB = 0X02,
> QMI_DATA_EP_TYPE_UNDEFINED = 0XFF,
> } QmiDataEndpointType;
>
> -
> +/**
> + * qmi_data_endpoint_type_get_string:
> + *
> + * Since: 1.18
> + */
Ah, here it is, please add this to the previous patch.
> #endif /* _LIBQMI_GLIB_QMI_ENUMS_H_ */
> diff --git a/src/qmicli/qmicli-wds.c b/src/qmicli/qmicli-wds.c
> index 224cb7c..add736e 100644
> --- a/src/qmicli/qmicli-wds.c
> +++ b/src/qmicli/qmicli-wds.c
> @@ -35,6 +35,9 @@
> #include "qmicli.h"
> #include "qmicli-helpers.h"
>
> +#define QMI_WDS_MUX_ID_UNDEFINED 0xFF
> +#define QMI_WDS_ENDPOINT_INTERFACE_NUMBER_UNDEFINED -1
> +
> /* Context */
> typedef struct {
> QmiDevice *device;
> @@ -68,6 +71,7 @@ static gchar *set_autoconnect_settings_str;
> static gboolean get_supported_messages_flag;
> static gboolean reset_flag;
> static gboolean noop_flag;
> +static gchar *bind_mux_str;
>
> static GOptionEntry entries[] = {
> { "wds-start-network", 0, 0, G_OPTION_ARG_STRING, &start_network_str,
> @@ -138,6 +142,10 @@ static GOptionEntry entries[] = {
> "Reset the service state",
> NULL
> },
> + { "wds-bind-mux-data-port", 0, 0, G_OPTION_ARG_STRING, &bind_mux_str,
> + "Bind qmux data port to controller device (allowed keys: mux-id, ep-iface-number) to be used with `--client-no-release-cid'",
> + "[\"key=value,...\"]"
> + },
> { "wds-noop", 0, 0, G_OPTION_ARG_NONE, &noop_flag,
> "Just allocate or release a WDS client. Use with `--client-no-release-cid' and/or `--client-cid'",
> NULL
> @@ -171,6 +179,7 @@ qmicli_wds_options_enabled (void)
>
> n_actions = (!!start_network_str +
> !!stop_network_str +
> + !!bind_mux_str +
> get_current_settings_flag +
> get_packet_service_status_flag +
> get_packet_statistics_flag +
> @@ -1552,6 +1561,136 @@ noop_cb (gpointer unused)
> return FALSE;
> }
>
> +typedef struct {
> + guint32 mux_id;
> + guint8 ep_type;
> + guint32 ep_iface_number;
> + guint32 client_type;
> +} BindMuxDataPortProperties;
> +
Single whiteline to separate here, not two.
> +
> +static gboolean
> +bind_mux_data_port_properties_handle (const gchar *key,
> + const gchar *value,
> + GError **error,
> + gpointer user_data)
> +{
> + BindMuxDataPortProperties *props = user_data;
A whiteline missing here.
> + if (!value || !value[0]) {
> + g_set_error (error,
> + QMI_CORE_ERROR,
> + QMI_CORE_ERROR_FAILED,
> + "key '%s' requires a value",
> + key);
> + return FALSE;
> + }
> +
> + if (g_ascii_strcasecmp (key, "mux-id") == 0) {
> + props->mux_id = atoi(value);
> + return TRUE;
> + }
> +
> + if (g_ascii_strcasecmp (key, "ep-iface-number") == 0) {
> + props->ep_iface_number = atoi(value);
> + return TRUE;
> + }
> +
> + g_set_error (error,
> + QMI_CORE_ERROR,
> + QMI_CORE_ERROR_FAILED,
> + "Unrecognized option '%s'",
> + key);
> +
> + return FALSE;
> +}
> +
> +static QmiMessageWdsBindMuxDataPortInput *
> +bind_mux_data_port_input_create (const gchar *str)
> +{
> + QmiMessageWdsBindMuxDataPortInput *input = NULL;
> + GError *error = NULL;
> + BindMuxDataPortProperties props = {
> + .mux_id = QMI_WDS_MUX_ID_UNDEFINED,
> + .ep_type = QMI_DATA_EP_TYPE_HSUSB,
> + .ep_iface_number = QMI_WDS_ENDPOINT_INTERFACE_NUMBER_UNDEFINED,
> + .client_type = WDS_CLIENT_TYPE_TETHERED,
> + };
> +
> + if (!str[0])
> + return NULL;
> +
> + /* New key=value format */
This is a new command, there is no old/new format; the key=value one
is the only one supported.
> + if (strchr (str, '=')) {
> + if (!qmicli_parse_key_value_string (str,
> + &error,
> + bind_mux_data_port_properties_handle,
> + &props)) {
> + g_printerr ("error: could not parse input string '%s'\n", error->message);
> + g_error_free (error);
> + return NULL;
> + }
> + }
else error? We do require the key=value format. Maybe it's worth doing
a quick check before qmicli_parse_key_value_string() to see if
character '=' exists in the string and error out directly there if it
doesn't exist.
> +
> +
> + if ((props.mux_id == QMI_WDS_MUX_ID_UNDEFINED) ^
> + (props.ep_iface_number == QMI_WDS_ENDPOINT_INTERFACE_NUMBER_UNDEFINED)) {
> + g_printerr ("error: Mux ID and Endpoint Iface Number are both needed\n");
> + return NULL;
> + }
> +
I don't get the bitwise XOR logic in the check. If you do want to have
both given, why not:
if (props.mux_id == UNDEFINED || props.ep_iface_number == UNDEFINED) {
// error
}
> + input = qmi_message_wds_bind_mux_data_port_input_new ();
> +
> + if (!qmi_message_wds_bind_mux_data_port_input_set_endpoint_info (input, props.ep_type, props.ep_iface_number, &error)) {
> + g_printerr ("error: couldn't set endpoint info: '%s'\n", error->message);
> + goto error_out;
> + }
> +
> + if (!qmi_message_wds_bind_mux_data_port_input_set_mux_id (input, props.mux_id, &error)) {
> + g_printerr ("error: couldn't set mux ID %d: '%s'\n", props.mux_id, error->message);
> + goto error_out;
> + }
> +
> + if (!qmi_message_wds_bind_mux_data_port_input_set_client_type (input, props.client_type , &error)) {
> + g_printerr ("error: couldn't set client type: '%s'\n", error->message);
> + goto error_out;
> + }
> +
> + return input;
> +
> +error_out:
> + if (error)
> + g_error_free (error);
> + qmi_message_wds_bind_mux_data_port_input_unref (input);
> + return NULL;
> +}
> +
> +static void
> +bind_mux_data_port_ready (QmiClientWds *client,
> + GAsyncResult *res) {
> + QmiMessageWdsBindMuxDataPortOutput *output;
> + GError *error = NULL;
> + g_print ("[%s]\n", __FUNCTION__);
Please remove that g_print(), looks like a development trace, right?
> +
> + output = qmi_client_wds_bind_mux_data_port_finish (client, res, &error);
> + if (!output) {
> + g_printerr ("error: operation failed: %s\n", error->message);
> + g_error_free (error);
> + operation_shutdown (FALSE);
> + return;
> + }
> +
> + if (!qmi_message_wds_bind_mux_data_port_output_get_result (output, &error)) {
> + g_printerr ("error: couldn't bind mux data port: %s\n", error->message);
> + g_error_free (error);
> + qmi_message_wds_bind_mux_data_port_output_unref (output);
> + operation_shutdown (FALSE);
> + return;
> + }
> +
> + qmi_message_wds_bind_mux_data_port_output_unref (output);
> + operation_shutdown (TRUE);
> +}
> +
> void
> qmicli_wds_run (QmiDevice *device,
> QmiClientWds *client,
> @@ -1610,6 +1749,22 @@ qmicli_wds_run (QmiDevice *device,
> return;
> }
>
> + /* Request to bind mux port? */
> + if (bind_mux_str) {
> + QmiMessageWdsBindMuxDataPortInput *input;
> + g_print ("Bind mux data port");
g_debug() instead, or just remove it.
> +
> + input = bind_mux_data_port_input_create (bind_mux_str);
> +
> + qmi_client_wds_bind_mux_data_port (client,
> + input,
> + 10,
> + ctx->cancellable,
> + (GAsyncReadyCallback) bind_mux_data_port_ready,
> + NULL);
input unref missing here.
> + return;
> + }
> +
> /* Request to get current settings? */
> if (get_current_settings_flag) {
> QmiMessageWdsGetCurrentSettingsInput *input;
> --
> 2.7.4
>
--
Aleksander
https://aleksander.es
More information about the libqmi-devel
mailing list