[PATCH 2/2] qmicli: add support LOC

Aleksander Morgado aleksander at aleksander.es
Fri Feb 23 12:27:30 UTC 2018


On 03/02/18 11:55, Thomas WeiƟschuh wrote:
> ---
>  src/qmicli/Makefile.am  |   1 +
>  src/qmicli/qmicli-loc.c | 305 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/qmicli/qmicli.c     |  11 ++
>  src/qmicli/qmicli.h     |   7 ++
>  4 files changed, 324 insertions(+)
>  create mode 100644 src/qmicli/qmicli-loc.c
> 
> diff --git a/src/qmicli/Makefile.am b/src/qmicli/Makefile.am
> index eb63fa7..8a4fefa 100644
> --- a/src/qmicli/Makefile.am
> +++ b/src/qmicli/Makefile.am
> @@ -44,6 +44,7 @@ qmicli_SOURCES = \
>  	qmicli-wms.c \
>  	qmicli-wda.c \
>  	qmicli-voice.c \
> +	qmicli-loc.c \
>  	qmicli-charsets.c \
>  	qmicli-charsets.h
>  
> diff --git a/src/qmicli/qmicli-loc.c b/src/qmicli/qmicli-loc.c
> new file mode 100644
> index 0000000..134f954
> --- /dev/null
> +++ b/src/qmicli/qmicli-loc.c
> @@ -0,0 +1,305 @@
> +#include "config.h"
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <locale.h>
> +#include <string.h>
> +
> +#include <glib.h>
> +#include <gio/gio.h>
> +
> +#include <libqmi-glib.h>
> +
> +#include "qmicli.h"
> +#include "qmicli-helpers.h"
> +
> +
> +/* FIXME allow cancelling */
> +
> +static gboolean show_location = FALSE;
> +static gboolean show_satellites = FALSE;
> +static gboolean show_nmea = FALSE;
> +static gboolean print_continuous = FALSE;
> +
> +typedef struct {
> +    QmiClientLoc *client;
> +    GCancellable *cancellable;
> +    guint position_report_indication_id;
> +    guint nmea_indication_id;
> +    guint satellite_info_indication_id;
> +} Context;
> +
> +static GOptionEntry entries[] = {
> +    {
> +        "loc-location", 0, 0, G_OPTION_ARG_NONE, &show_location,
> +        "Show location",
> +        NULL,
> +    },
> +    {
> +        "loc-satellites", 0, 0, G_OPTION_ARG_NONE, &show_satellites,
> +        "Show satellite report",
> +        NULL,
> +    },
> +    {
> +        "loc-nmea", 0, 0, G_OPTION_ARG_NONE, &show_nmea,
> +        "Show NMEA string",
> +        NULL,
> +    },
> +    {
> +        "loc-continuous", 0, 0, G_OPTION_ARG_NONE, &print_continuous,
> +        "Print continuous updates",
> +        NULL,
> +    },

I would expect to see a "loc-start" command here (i.e. not automagically started by the other commands). Also, is there a way to "loc-stop"? The idea is that qmicli should provide actions as closest as possible to the actual QMI operations, nothing more than that.

Then, I see that we are getting all info via indications instead of request/response, so maybe something like:

loc-position-report     // following the "position-report" signal name
loc-gnss-satellite-info // following the "gnss-satellite-info" signal name
loc-nmea                // following the "nmea" signal name

And how about "loc-follow" instead of continuous? Just trying to find some name similar to the other option that enables some long-time monitoring (--wds-follow-network when given additionally to --wds-start-network).

One thing that we should definitely allow is to request all 3 things at the same time, e.g. if we want to leave monitoring for all events we could do:

qmicli --loc-position-report --loc-gnss-satellite-info --loc-nmea --loc-follow

Or if we just want to get max 1 of each, we would run the same without --loc-follow and stop as soon as we get one of each.

What do you think?

Some other comments inline below


> +    { NULL }
> +};
> +
> +GOptionGroup *
> +qmicli_loc_get_option_group (void)
> +{
> +    GOptionGroup *group;
> +
> +    group = g_option_group_new ("loc",
> +                                "LOC options",
> +                                "Show location options", NULL, NULL);
> +    g_option_group_add_entries (group, entries);
> +
> +    return group;
> +}
> +
> +gboolean
> +qmicli_loc_options_enabled (void)
> +{
> +    static guint n_actions = 0;
> +    static gboolean checked = FALSE;
> +
> +    if (checked)
> +        return !!n_actions;
> +
> +    n_actions = (
> +            !!show_location +
> +            !!show_satellites +
> +            !!show_nmea
> +            );
> +
> +    if (n_actions > 1) {
> +        g_printerr ("error: too many LOC actions requested\n");
> +        exit (EXIT_FAILURE);
> +    }
> +
> +    checked = TRUE;
> +    return !!n_actions;
> +}
> +
> +static Context *
> +context_new (QmiClientLoc *client, GCancellable *cancellable)
> +{
> +    Context *context;
> +
> +    context = g_slice_new0 (Context);
> +    context->cancellable = g_object_ref (cancellable);
> +    context->client = g_object_ref (client);
> +    return context;
> +}
> +
> +static void
> +context_free (Context *context)
> +{
> +    if (!context)
> +        return;
> +
> +    g_object_unref (context->client);
> +    g_object_unref (context->cancellable);
> +
> +    g_slice_free (Context, context);
> +}
> +
> +static void
> +operation_shutdown (gboolean operation_status, Context *ctx)
> +{
> +    if (ctx) {
> +        context_free (ctx);
> +    }
> +    qmicli_async_operation_done (operation_status, FALSE);
> +}
> +
> +static void
> +on_started (QmiClientLoc *client, GAsyncResult *res

The third parameter you receive in on_started() would be the Context itself, there is no need to call g_async_result_get_user_data() data afterwards, which feels weird and out of place... :)

) {
> +    QmiMessageLocStartOutput *output;
> +    GError *error = NULL;
> +    Context *ctx;
> +
> +   ctx = g_async_result_get_user_data(res);
> +
> +    output = qmi_client_loc_start_finish(client, res, &error);
> +    if (output == NULL) {
> +        g_printerr ("Could not start location tracking: %s\n", error->message);
> +        goto error;
> +    }
> +
> +    if (!qmi_message_loc_start_output_get_result (output, NULL)) {
> +        g_printerr ("Could not start location tracking!\n");
> +        goto error;
> +    }
> +
> +    qmi_message_loc_start_output_unref(output);
> +    return;
> +
> +error:
> +    qmi_message_loc_start_output_unref(output);
> +    operation_shutdown(FALSE, ctx);
> +}
> +
> +static void
> +on_registered (QmiClientLoc *client, GAsyncResult *res

Same here.

) {
> +   QmiMessageLocRegisterEventsOutput *output;
> +   QmiMessageLocStartInput *start_input;
> +   GError *error = NULL;
> +   Context *ctx;
> +
> +   ctx = g_async_result_get_user_data(res);
> +
> +   output = qmi_client_loc_register_events_finish(client, res, &error);
> +   if (output == NULL) {
> +       g_printerr ("Could not register message listener: %s\n", error->message);
> +       goto out;
> +   }
> +   if (!qmi_message_loc_register_events_output_get_result (output, NULL)) {
> +       g_printerr ("Could not register message listener!\n");
> +       goto out;
> +   }
> +
> +   start_input = qmi_message_loc_start_input_new ();
> +   qmi_message_loc_start_input_set_intermediate_report_state (start_input, QMI_LOC_INTERMEDIATE_REPORT_STATE_ENABLE, NULL);
> +   qmi_message_loc_start_input_set_minimum_interval_between_position_reports (start_input, 1000, NULL);
> +   qmi_message_loc_start_input_set_fix_recurrence_type (start_input, QMI_LOC_FIX_RECURRENCE_TYPE_REQUEST_PERIODIC_FIXES, NULL);
> +   qmi_message_loc_start_input_set_session_id (start_input, 0x0, NULL);
> +
> +   qmi_client_loc_start (client, start_input, 10, ctx->cancellable, (GAsyncReadyCallback) on_started, ctx);
> +
> +out:
> +   qmi_message_loc_register_events_output_unref(output);
> +}
> +
> +static void
> +report_received (gboolean success, Context *ctx) {
> +    if (success) {
> +        if (!print_continuous) {
> +            operation_shutdown(TRUE, ctx);
> +        }
> +        return;
> +    }
> +    g_warn_if_reached ();

Hum... what's the purpose of this?

> +}
> +
> +static void
> +position_report_received (QmiClientLoc       *object,
> +        QmiIndicationLocPositionReportOutput *output,
> +        Context                              *ctx) {
> +    gdouble lat, lon;
> +    QmiLocSessionStatus status;
> +
> +    qmi_indication_loc_position_report_output_get_latitude(output, &lat, NULL);
> +    qmi_indication_loc_position_report_output_get_longitude(output, &lon, NULL);
> +    qmi_indication_loc_position_report_output_get_session_status(output, &status, NULL);
> +
> +    if (status == QMI_LOC_SESSION_STATUS_SUCCESS) {
> +        g_print("Position: %f/%f\n", lat, lon);
> +        report_received(TRUE, ctx);
> +        return;
> +    }
> +
> +    if (status == QMI_LOC_SESSION_STATUS_IN_PROGRESS) {
> +        g_print("No position yet\n");
> +        return;
> +    }
> +
> +    g_print("Error %s\n", qmi_loc_session_status_get_string(status));
> +}
> +
> +static void
> +nmea_received (QmiClientLoc        *object,
> +        QmiIndicationLocNmeaOutput *output,
> +        Context                    *ctx) {
> +    const gchar *nmea = NULL;
> +
> +    qmi_indication_loc_nmea_output_get_nmea_string (output, &nmea, NULL);
> +
> +    if (nmea) {
> +        g_print("%s", nmea);
> +        report_received(TRUE, ctx);
> +    }
> +}
> +
> +static void
> +satellite_info_received (QmiClientLoc           *object,
> +        QmiIndicationLocGnssSatelliteInfoOutput *output,
> +        Context                                 *ctx) {
> +    GArray *satellite_infos = NULL;
> +
> +    qmi_indication_loc_gnss_satellite_info_output_get_satellite_info (output, &satellite_infos, NULL);
> +
> +    g_print("%d Satellites\n", satellite_infos ? satellite_infos->len : 0);
> +    report_received(TRUE, ctx);
> +}
> +
> +/*
> +static void
> +disconnect_signals (Context *ctx) {
> +    if (ctx->position_report_indication_id) {
> +        g_signal_handler_disconnect(ctx->client, ctx->position_report_indication_id);
> +    }
> +    if (ctx->nmea_indication_id) {
> +        g_signal_handler_disconnect(ctx->client, ctx->nmea_indication_id);
> +    }
> +    if (ctx->satellite_info_indication_id) {
> +        g_signal_handler_disconnect(ctx->client, ctx->satellite_info_indication_id);
> +    }
> +}
> +*/
> +
> +static void
> +cancel (Context *ctx) {
> +    gboolean success = !print_continuous;
> +    // FIXME
> +    operation_shutdown (success, NULL);
> +}
> +
> +void
> +qmicli_loc_run (QmiDevice *device,
> +                QmiClientLoc *client,
> +                GCancellable *cancellable) {
> +
> +    QmiMessageLocRegisterEventsInput *i;
> +    QmiLocEventRegistrationFlag indication_mask = 0;
> +    Context *ctx;
> +
> +    ctx = context_new(client, cancellable);
> +
> +    g_cancellable_connect(cancellable, G_CALLBACK (cancel), ctx, NULL);
> +
> +    if (show_location) {
> +        indication_mask = QMI_LOC_EVENT_REGISTRATION_FLAG_POSITION_REPORT;
> +    } else if (show_satellites) {
> +        indication_mask = QMI_LOC_EVENT_REGISTRATION_FLAG_GNSS_SATELLITE_INFO;
> +    } else if (show_nmea) {
> +        indication_mask = QMI_LOC_EVENT_REGISTRATION_FLAG_NMEA;
> +    }
> +
> +    ctx->position_report_indication_id = g_signal_connect (client,
> +            "position-report",
> +            G_CALLBACK (position_report_received), ctx);
> +    ctx->nmea_indication_id = g_signal_connect (client,
> +            "nmea",
> +            G_CALLBACK (nmea_received), ctx);
> +    ctx->satellite_info_indication_id = g_signal_connect (client,
> +            "gnss-satellite-info",
> +            G_CALLBACK (satellite_info_received), ctx);
> +
> +    i = qmi_message_loc_register_events_input_new ();
> +    qmi_message_loc_register_events_input_set_event_registration_mask(i, indication_mask, NULL);
> +    qmi_client_loc_register_events (client, i, 10, ctx->cancellable, (GAsyncReadyCallback) on_registered, ctx);
> +
> +    qmi_message_loc_register_events_input_unref (i);
> +
> +    return;
> +}
> diff --git a/src/qmicli/qmicli.c b/src/qmicli/qmicli.c
> index fecae8d..6993c9c 100644
> --- a/src/qmicli/qmicli.c
> +++ b/src/qmicli/qmicli.c
> @@ -372,6 +372,9 @@ allocate_client_ready (QmiDevice *dev,
>      case QMI_SERVICE_VOICE:
>          qmicli_voice_run (dev, QMI_CLIENT_VOICE (client), cancellable);
>          return;
> +    case QMI_SERVICE_LOC:
> +        qmicli_loc_run (dev, QMI_CLIENT_LOC (client), cancellable);
> +        return;
>      default:
>          g_assert_not_reached ();
>      }
> @@ -731,6 +734,12 @@ parse_actions (void)
>          actions_enabled++;
>      }
>  
> +    /* LOC options? */
> +    if (qmicli_loc_options_enabled ()) {
> +        service = QMI_SERVICE_LOC;
> +        actions_enabled++;
> +    }
> +
>      /* Cannot mix actions from different services */
>      if (actions_enabled > 1) {
>          g_printerr ("error: cannot execute multiple actions of different services\n");
> @@ -774,6 +783,8 @@ int main (int argc, char **argv)
>                                  qmicli_wda_get_option_group ());
>      g_option_context_add_group (context,
>                                  qmicli_voice_get_option_group ());
> +    g_option_context_add_group (context,
> +                                qmicli_loc_get_option_group ());
>      g_option_context_add_main_entries (context, main_entries, NULL);
>      if (!g_option_context_parse (context, &argc, &argv, &error)) {
>          g_printerr ("error: %s\n",
> diff --git a/src/qmicli/qmicli.h b/src/qmicli/qmicli.h
> index 7db7905..37b5f79 100644
> --- a/src/qmicli/qmicli.h
> +++ b/src/qmicli/qmicli.h
> @@ -90,4 +90,11 @@ void          qmicli_voice_run              (QmiDevice *device,
>                                               QmiClientVoice *client,
>                                               GCancellable *cancellable);
>  
> +/* Location group */
> +GOptionGroup *qmicli_loc_get_option_group (void);
> +gboolean      qmicli_loc_options_enabled  (void);
> +void          qmicli_loc_run              (QmiDevice *device,
> +                                                QmiClientLoc *client,
> +                                                GCancellable *cancellable);
> +
>  #endif /* __QMICLI_H__ */
> 


-- 
Aleksander
https://aleksander.es


More information about the libqmi-devel mailing list