[PATCH v3 0/1] qmicli: add support for the LOC service

[Aleksander Morgado]

Hey Thomas

On Sun, Jun 24, 2018 at 4:40 PM, Thomas Weißschuh <thomas at t-8ch.de> wrote:
> This patchset adds qmicli support for the LOC service.
> The logic is structured around a state machine which manages the execution of
> non-mandatory steps like start and stop.
>
> Changes in v2:
>
> * Dropped a patch which was merged with another patch series
> * Invocation to LocStart and LocStopped must now be specified explicitly
> * Renamed --loc-continuous to --loc-follow
> * Implemented the parallel printing of both location and satellite reports
> * NMEA mode requires follow mode
>
> Changes in v3:
>
> * Cancelling of --loc-follow operations has been fixed
>
> Thomas Weißschuh (1):
>   qmicli: add support for the LOC service
>

You're not going to believe this, but yesterday I remembered about
your last patch and ended up taking it and started reworking it. See:
https://gitlab.freedesktop.org/mobile-broadband/libqmi/commits/loc

I initially took your patch and started to add follow-up commits
fixing things myself, but then ended up thinking in what I believe was
a better way to handle these commands and drafted some different
changes. I've also extended the position and satellite info reports to
actually print the contents, and fixed some TLVs from the position
report.

Let me explain my changes, and let me try to convince you :)

This is the main implementation I had, highly based on your original patch:
https://gitlab.freedesktop.org/mobile-broadband/libqmi/commit/06767e0e13a26ddabdbbeecb7d88f3c2e2c6a9ed

Until now all qmicli commands have been done trying to map one to one
each QMI request in the database. This was easy until we started to
see that QMI services relied more and more on indications to get
responses and such. In the specific case of LOC, this meant that we
had to invent ourselves some "requests" to map the ones that didn't
exist. That's how we got to have e.g. the suggested
--loc-position-report, which isn't really a request; we just "start"
and listen for indications.

But then I thought that we shouldn't be implementing much more logic
than the bare minimum. I don't want to have qmicli expose much logic
that shouldn't have, it should be as closest as possible to the
original LOC database.
So, my suggestion was to have these LOC options:

  --loc-session-id=[ID]
                        Session ID for the LOC session
  --loc-start
                        Start location gathering
  --loc-stop
                        Stop location gathering
  --loc-get-position
                        Get position reported by the location module
  --loc-get-satellite-info
                        Show satellite report
  --loc-timeout=[SECS]
                        Maximum time to wait for information in
`--loc-get-position' and `--loc-get-satellite-info' (default 30s)
  --loc-follow-position
                        Follow all position updates reported by the
location module indefinitely
  --loc-follow-satellite-info
                        Follow all satellite info updates reported by
the location module indefinitely
  --loc-follow-nmea
                        Follow all NMEA trace updates reported by the
location module indefinitely

The loc-start and loc-stop actions start or stop independently the
engine. The caveat here is that for loc-start to do anything
meaningful, we MUST NOT release-cid, as the indications will be
emitted only if the start was executed on that same CID.
Then, we would have get-position and get-satellite-info that would
return the contents of the first indication received.
And then we have the follow options, which will print on the screen
all indications received until user hits Ctrl+C.

My changes e.g. try to avoid needing to run the start implicitly when
--follow options are requested, so the user would need to do as
follows:

sudo qmicli -p -d /dev/cdc-wdm4 --loc-start --client-no-release-cid
sudo qmicli -p -d /dev/cdc-wdm4 --loc-follow-position
--loc-follow-satellite-info --client-no-release-cid --client-cid=1
(Ctrl+C)
sudo qmicli -p -d /dev/cdc-wdm4 --loc-stop --client-cid=1

This doesn't prevent us from e.g. setting up a script to do this at a
higher level (kind of what qmi-network does with --wds-start-network).

I totally wouldn't mind if you reviewed/tested these changes. I've
been testing them with a modem here, and now was working on making
this work in MBIM mode (e.g. running QMI LOC over MBIM).

-- 
Aleksander
https://aleksander.es