[PATCH 7/7] flush unmanaged GPS port after GPS is stoped

[Aleksander Morgado]

Hey Tomas,

> ---
>  plugins/mbm/mm-broadband-modem-mbm.c | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/plugins/mbm/mm-broadband-modem-mbm.c b/plugins/mbm/mm-broadband-modem-mbm.c
> index 2f0a014..d881ddc 100644
> --- a/plugins/mbm/mm-broadband-modem-mbm.c
> +++ b/plugins/mbm/mm-broadband-modem-mbm.c
> @@ -1269,7 +1269,7 @@ gps_disabled_ready (MMBaseModem *self,
>                      GAsyncResult *res,
>                      LocationGatheringContext *ctx)
>  {
> -    MMPortSerialGps *gps_port;
> +    MMPortSerialGps *gps_port = NULL;
>      GError *error = NULL;
>  
>      if (!mm_base_modem_at_command_full_finish (self, res, &error))
> @@ -1277,15 +1277,27 @@ gps_disabled_ready (MMBaseModem *self,
>      else
>          g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
>  
> -    /* Only use the GPS port in NMEA/RAW setups */
> -    if (ctx->source & (MM_MODEM_LOCATION_SOURCE_GPS_NMEA |
> -                       MM_MODEM_LOCATION_SOURCE_GPS_RAW)) {
> -        /* Even if we get an error here, we try to close the GPS port */
> +    if ( ctx->source & MM_MODEM_LOCATION_SOURCE_GPS_UNMANAGED ) /* Unmanaged GPS port will be drained during close */ {

The comment should go in the previous line, not between the if() and the {.

Also no whitespaces after ( or before ).

> +        gps_port = mm_base_modem_peek_port_gps (self);
> +        if (!gps_port || !mm_port_serial_open (MM_PORT_SERIAL (gps_port), &error)) {
> +            if (error)
> +                g_simple_async_result_take_error (ctx->result, error);
> +            else
> +                g_simple_async_result_set_error (ctx->result,
> +                                                 MM_CORE_ERROR,
> +                                                 MM_CORE_ERROR_FAILED,
> +                                                 "Couldn't open raw GPS serial port");

This logic is re-setting the result in the GSimpleAsyncResult (it was
set always in the block above), and that's a problem.

But, I actually think we could ignore these 2 errors being handled here.
If we're not able to re-open the port to close it right away, that
shouldn't return an error in the location disable async command.

So, I'd suggest to do something like:

/* On GPS unmanaged, reopen the port so that the explicit close
 * afterwards drains the port. */
if (ctx->source & MM_MODEM_LOCATION_SOURCE_GPS_UNMANAGED) {
    gps_port = mm_base_modem_peek_port_gps (self);
    /* Don't try to close the GPS port later on if re-opening failed */
    if (gps_port && !mm_port_serial_open (MM_PORT_SERIAL (gps_port), NULL)
        gps_port = NULL;
} else ...


> +        }
> +    }
> +    else if (ctx->source & (MM_MODEM_LOCATION_SOURCE_GPS_NMEA | 
> +                            MM_MODEM_LOCATION_SOURCE_GPS_RAW )) /* need to close GPS port in NMEA/RAW setups */ {
>          gps_port = mm_base_modem_peek_port_gps (self);
> -        if (gps_port)
> -            mm_port_serial_close (MM_PORT_SERIAL (gps_port));
>      }
>  
> +    /* Even if we have an error here, we try to close the GPS port */
> +    if (gps_port)
> +        mm_port_serial_close (MM_PORT_SERIAL (gps_port));
> +
>      location_gathering_context_complete_and_free (ctx);
>  }
>  
> 


-- 
Aleksander
https://aleksander.es