[PATCH 2/2] qmicli: add support LOC
Thomas Weißschuh
thomas at t-8ch.de
Mon Feb 26 07:00:37 UTC 2018
Hi,
replies are inline.
On Fri, 2018-02-23T13:27+0100, Aleksander Morgado wrote:
> 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.
Ok, will do.
> Then, I see that we are getting all info via indications instead of request/response, so maybe something like:
Just to clarify: The "Then" in your sentence is not meant to invalidate the
paragraph before? (loc-stop etc.)
> 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).
Will do.
> 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?
I completely agree.
> 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... :)
Ok.
> ) {
> > + 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.
Ok.
> ) {
> > + 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?
The `g_warn_if_reached()`?
It seems I do not yet handle the error case.
This will be fixed in the next version.
> > +}
> > +
> > +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