[PATCH] ublox: use ID_MM_UBLOX_PORT_READY_DELAY udev flag as init

[Aleksander Morgado]

Hey!

On Wed, May 2, 2018 at 10:34 PM, Matthew Starr <mstarr at hedonline.com> wrote:
> Added reading the ID_MM_UBLOX_PORT_READY_DELAY udev flag value and
> using it as an init delay when a value is set.
>
> The 20 second delay for the TOBY-L4 +READ URC has been reimplemented
> using the new ID_MM_UBLOX_PORT_READY_DELAY udev value.

This looks simple enough I think yes.
Some small comments below.

> ---
>  plugins/ublox/77-mm-ublox-port-types.rules |  2 ++
>  plugins/ublox/mm-plugin-ublox.c            | 16 ++++++++++------
>  2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/plugins/ublox/77-mm-ublox-port-types.rules b/plugins/ublox/77-mm-ublox-port-types.rules
> index 31128dac..168d1571 100644
> --- a/plugins/ublox/77-mm-ublox-port-types.rules
> +++ b/plugins/ublox/77-mm-ublox-port-types.rules
> @@ -11,9 +11,11 @@ SUBSYSTEMS=="usb", ATTRS{bInterfaceNumber}=="?*", ENV{.MM_USBIFNUM}="$attr{bInte
>  #  ttyACM0 (if #2): secondary (ignore)
>  #  ttyACM1 (if #4): debug port (ignore)
>  #  ttyACM2 (if #6): primary
> +#      Wait up to 20s for the +READY URC
>  #  ttyACM3 (if #8): AT port for FOTA (ignore)
>  ATTRS{idVendor}=="1546", ATTRS{idProduct}=="1010", ENV{.MM_USBIFNUM}=="02", ENV{ID_MM_PORT_IGNORE}="1"
>  ATTRS{idVendor}=="1546", ATTRS{idProduct}=="1010", ENV{.MM_USBIFNUM}=="04", ENV{ID_MM_PORT_IGNORE}="1"
> +ATTRS{idVendor}=="1546", ATTRS{idProduct}=="1010", ENV{.MM_USBIFNUM}=="06", ENV{ID_MM_UBLOX_PORT_READY_DELAY}="20"
>  ATTRS{idVendor}=="1546", ATTRS{idProduct}=="1010", ENV{.MM_USBIFNUM}=="08", ENV{ID_MM_PORT_IGNORE}="1"
>
>  # TOBY-R2 port types
> diff --git a/plugins/ublox/mm-plugin-ublox.c b/plugins/ublox/mm-plugin-ublox.c
> index f4553a60..fdff239b 100644
> --- a/plugins/ublox/mm-plugin-ublox.c
> +++ b/plugins/ublox/mm-plugin-ublox.c
> @@ -50,9 +50,6 @@ create_modem (MMPlugin     *self,
>  /*****************************************************************************/
>  /* Custom init context */
>
> -/* Wait up to 20s for the +READY URC */
> -#define READY_WAIT_TIME_SECS 20
> -
>  typedef struct {
>      MMPortSerialAt *port;
>      GRegex         *ready_regex;
> @@ -147,8 +144,13 @@ wait_for_ready (GTask *task)
>                                                     task,
>                                                     NULL);
>
> +    mm_dbg ("(%s/%s) waiting %d seconds for init timeout",
> +            mm_port_probe_get_port_subsys (probe),
> +            mm_port_probe_get_port_name   (probe),
> +            mm_kernel_device_get_property_as_int (mm_port_probe_peek_port(probe), "ID_MM_UBLOX_PORT_READY_DELAY"));
> +
>      /* Otherwise, let the custom init timeout in some seconds. */
> -    ctx->timeout_id = g_timeout_add_seconds (READY_WAIT_TIME_SECS, (GSourceFunc) ready_timeout, task);
> +    ctx->timeout_id = g_timeout_add_seconds (mm_kernel_device_get_property_as_int (mm_port_probe_peek_port(probe), "ID_MM_UBLOX_PORT_READY_DELAY"), (GSourceFunc) ready_timeout, task);

How about having a single call reading the udev tag value in
ublox_custom_init() and passing the value to wait_for_ready()?

E.g.
static void
wait_for_ready (GTask *task,
                guint wait_timeout_secs)
{
...
}

>  }
>
>  static void
> @@ -225,8 +227,10 @@ ublox_custom_init (MMPortProbe         *probe,
>          return;
>      }
>
> -    /* Device hotplugged, wait for READY URC */
> -    wait_for_ready (task);
> +    if (mm_kernel_device_get_property_as_int (mm_port_probe_peek_port(probe), "ID_MM_UBLOX_PORT_READY_DELAY") != 0) {

Given that we're reading an int, possibly better > 0 instead of != 0.

> +           /* Device hotplugged and has a defined ready delay, wait for READY URC */
> +           wait_for_ready (task);
> +    }
>  }
>
>  /*****************************************************************************/
> --
> 2.14.1
>
> _______________________________________________
> ModemManager-devel mailing list
> ModemManager-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel



-- 
Aleksander
https://aleksander.es