On 20/05/17 09:57, Aleksander Morgado wrote:
> The default setup with 100ms between GPS commands doesn't seem to be
> always enough for the Engine activation step:
>
> [1495016625.392972] (ttyACM1): --> 'AT^SGPSC="NMEA/Output","on"<CR>'
> [1495016625.503885] (ttyACM1): <-- '<CR><LF>^SGPSC:
> "Nmea/Output","on"<CR><LF><CR><LF><CR><LF>OK<CR><LF>'
> [1495016625.607650] (ttyACM1): --> 'AT^SGPSC="Power/Antenna","on"<CR>'
> [1495016625.697862] (ttyACM1): <-- '<CR><LF>^SGPSC:
> "Power/Antenna","on"<CR><LF><CR><LF>OK<CR><LF>'
> [1495016625.809393] (ttyACM1): --> 'AT^SGPSC="Engine","1"<CR>'
> [1495016625.895970] (ttyACM1): <-- '<CR><LF>+CME ERROR: 767<CR><LF>'
>
> We now setup up to 3 retries for the Engine activation step before
> returning an error, and we also update to 2000ms the wait time before
> the Engine activation command is run.
> ---
>
> Hey,
>
> This update just changes the default wait time before issuing the Engine=1
> command, instead of 500ms we now use 2000ms. I've done several tests and most
> of the times the Engine is correctly activated after the first 2s wait time,
> and some other times it requires one retry. Never seen it require more than
> one retry, but I guess it doesn't harm if we leave the max 3 retry attempts.
>
This has been merged to git master.
> ---
> plugins/cinterion/mm-common-cinterion.c | 43
> ++++++++++++++++++++++++---------
> 1 file changed, 32 insertions(+), 11 deletions(-)
>
> diff --git a/plugins/cinterion/mm-common-cinterion.c
> b/plugins/cinterion/mm-common-cinterion.c
> index 6e839c8d..3f2b3994 100644
> --- a/plugins/cinterion/mm-common-cinterion.c
> +++ b/plugins/cinterion/mm-common-cinterion.c
> @@ -533,6 +533,15 @@ mm_common_cinterion_disable_location_gathering
> (MMIfaceModemLocation *self,
>
> /*****************************************************************************/
> /* Enable location gathering (Location interface) */
>
> +/* We will retry the SGPSC command that enables the Engine */
> +#define MAX_SGPSC_ENGINE_RETRIES 3
> +
> +/* Cinterion asks for 100ms some time between GPS commands, but we'll give up
> + * to 2000ms before setting the Engine configuration as 100ms didn't seem
> always
> + * enough (we would get +CME ERROR: 767 errors reported). */
> +#define GPS_COMMAND_TIMEOUT_DEFAULT_MS 100
> +#define GPS_COMMAND_TIMEOUT_ENGINE_MS 2000
> +
> typedef enum {
> ENABLE_LOCATION_GATHERING_GPS_STEP_FIRST,
> ENABLE_LOCATION_GATHERING_GPS_STEP_SGPSS,
> @@ -545,6 +554,7 @@ typedef enum {
> typedef struct {
> MMModemLocationSource source;
> EnableLocationGatheringGpsStep gps_step;
> + guint sgpsc_engine_retries;
> } EnableLocationGatheringContext;
>
> static void
> @@ -564,16 +574,10 @@ mm_common_cinterion_enable_location_gathering_finish
> (MMIfaceModemLocation *sel
> static void enable_location_gathering_context_gps_step (GTask *task);
>
> static gboolean
> -enable_location_gathering_context_gps_step_next_cb (GTask *task)
> +enable_location_gathering_context_gps_step_schedule_cb (GTask *task)
> {
> - EnableLocationGatheringContext *ctx;
> -
> - ctx = (EnableLocationGatheringContext *) g_task_get_task_data (task);
> -
> - /* We jump to the next step */
> - ctx->gps_step++;
> + /* Run the scheduled step */
> enable_location_gathering_context_gps_step (task);
> -
> return G_SOURCE_REMOVE;
> }
>
> @@ -582,16 +586,33 @@ enable_sgpsc_or_sgpss_ready (MMBaseModem *self,
> GAsyncResult *res,
> GTask *task)
> {
> - GError *error = NULL;
> + EnableLocationGatheringContext *ctx;
> + GError *error = NULL;
> +
> + ctx = (EnableLocationGatheringContext *) g_task_get_task_data (task);
>
> if (!mm_base_modem_at_command_finish (self, res, &error)) {
> + /* The GPS setup may sometimes report "+CME ERROR 767" when enabling
> the
> + * Engine; so we'll run some retries of the same command ourselves.
> */
> + if (ctx->gps_step ==
> ENABLE_LOCATION_GATHERING_GPS_STEP_SGPSC_ENGINE) {
> + ctx->sgpsc_engine_retries++;
> + mm_dbg ("GPS Engine setup failed (%u/%u)",
> ctx->sgpsc_engine_retries, MAX_SGPSC_ENGINE_RETRIES);
> + if (ctx->sgpsc_engine_retries < MAX_SGPSC_ENGINE_RETRIES) {
> + g_clear_error (&error);
> + goto schedule;
> + }
> + }
> g_task_return_error (task, error);
> g_object_unref (task);
> return;
> }
>
> - /* Cinterion asks for 100ms between GPS commands... */
> - g_timeout_add (100, (GSourceFunc)
> enable_location_gathering_context_gps_step_next_cb, task);
> + /* Go on to next step */
> + ctx->gps_step++;
> +
> +schedule:
> + g_timeout_add (ctx->gps_step ==
> ENABLE_LOCATION_GATHERING_GPS_STEP_SGPSC_ENGINE ?
> GPS_COMMAND_TIMEOUT_ENGINE_MS : GPS_COMMAND_TIMEOUT_DEFAULT_MS,
> + (GSourceFunc)
> enable_location_gathering_context_gps_step_schedule_cb, task);
> }
>
> static void
> --
> 2.12.2
>
--
Aleksander
https://aleksander.es
_______________________________________________
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
