[PATCH] ublox: use ID_MM_UBLOX_PORT_READY_DELAY udev flag as init

[Matthew Starr]

> -----Original Message-----
> From: Aleksander Morgado [mailto:aleksander at aleksander.es]
> Sent: Monday, May 07, 2018 2:31 PM
> On Mon, May 7, 2018 at 6:47 PM, Matthew Starr <mstarr at hedonline.com>
> wrote:
> >> -----Original Message-----
> >> From: Aleksander Morgado [mailto:aleksander at aleksander.es]
> >> Sent: Monday, May 07, 2018 10:59 AM
> >> On Mon, May 7, 2018 at 4:32 PM, Matthew Starr
> <mstarr at hedonline.com>
> >> wrote:
> >> >> -----Original Message-----
> >> >> From: Aleksander Morgado [mailto:aleksander at aleksander.es]
> >> >> Sent: Saturday, May 05, 2018 7:31 AM
> >> >>
> >> >> >> > > 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)
> >> >> >> > > {
> >> >> >> > > ...
> >> >> >> > > }
> >> >> >> > >
> >> >> >> >
> >> >> >> > The only issue with that is wait_for_ready() is also called by the
> >> >> >> quick_at_ready() function which is a callback function, so I can't
> pass in
> >> >> the
> >> >> >> wait_timeout_secs to that instance.  I was thinking instead of
> adding
> >> >> >> wait_timeout_secs to the CustomInitContext struct since that is
> >> already
> >> >> >> available to the wait_for_ready() function no matter how it is
> called.
> >> Then
> >> >> >> ctx->wait_timeout_secs can be set in the ublox_custom_init()
> >> function,
> >> >> >> unless you have another recommendation.
> >> >> >>
> >> >> >> Oh, that's totally fine. Just read it once and store it in the
> >> >> >> CustomInitContext.
> >> >> >>
> >> >> >
> >> >> > One additional item is I found to reliably get the TOBY-R200 to
> initialize
> >> with
> >> >> ModemManager I needed to have it use a 2 second delay using the
> >> >> ID_MM_UBLOX_PORT_READY_DELAY udev flag in the 77-mm-ublox-
> port-
> >> >> types.rules file.  Without this the modem is sometimes never initialized
> by
> >> >> MM.  I am planning on adding that to the new patch I will be sending
> too.
> >> >> >
> >> >>
> >> >> Humm.... but does the TOBY-R200 send the +READY URC before those
> 2s?
> >> >> I don't understand why that would be needed really. Could you send
> >> >> some logs with/without the 2s timeout to see the differences?
> >> >>
> >> >
> >> > The TOBY-R200 does not send a +READY URC message.  The root issue is
> >> that when the USB serial devices (ttyACM) appear they are not really
> ready
> >> for communication yet.  This delay interface just happened to be an easy
> way
> >> to apply a delay at startup, but I can see how this is outside the code's
> >> intended use.
> >> >
> >> > Apparently several of the modems that u-blox has can send greeting
> text
> >> which by default is turned off, and when turned on the default greeting is
> a
> >> blank string.  Since the defaults have to be assumed, the use of the
> greeting
> >> on other u-blox modems will not be useful to determine when the
> modem is
> >> ready to communicate.
> >> >
> >> > If you want I can take out the 2 second delay from the patch and
> resubmit
> >> without that.
> >> >
> >>
> >> I believe the patch should go without 2 second delay, yes.
> >
> > I will resubmit the patch without it.
> >
> >>
> >> > I attached the logs with and without the delay for you to review.  You
> can
> >> see in the one without the delay MM never is able to get the modem to
> >> respond to the AT command even after 10 minutes of trying.
> >> >
> >>
> >> I'm extremely confused by this log (the one "without delay"). Is this
> >> log obtained running from git master without any other patch on top?
> >> Did you manually modify anything in the code to get this log? I would
> >> expect to see "AT" commands *sent* to the device via the ttyACMx
> >> interfaces and I'm not seeing those in the logs.
> >>
> >
> > This is the master branch of MM with my patch (but without the 2 second
> delay on the TOBY-R200).  I thought this was weird too.
> >
> > It looks like the +READY URC delay of 20 seconds that my patch replaces
> was avoiding/masking this issue with the TOBY-R200 modem.
> >
> > I connected to the modem directly using picocom when this issue occurs
> with MM and the modem responds to "AT" with "OK".  Also other AT
> commands I tried worked fine.  I tried this on all 3 USB serial lines that have
> AT interfaces.
> >
> > Adding in a small delay before probing the modem seems to resolve the
> issue, or at least is a work around since I am not sure what is causing the
> issue.
> >
> 
> Oh, I know what the issue is :) I believe it's actually a problem
> introduced by your patch. You're only calling wait_for_ready() in
> ublox_custom_init() if the timeout > 0. If there is no timeout
> defined, your logic just exits without finishing the async operation
> and so without doing the port probing.
> 
> Your new code should look like this at the end of ublox_custom_init():
> 
> ...
>     if (ctx->wait_timeout_secs > 0) {
>         /* Device hotplugged and has a defined ready delay, wait for
> READY URC */
>         wait_for_ready (task);
>         return;
>     }
> 
>     /* Otherwise, no need for any custom wait */
>     g_task_return_boolean (task, TRUE);
>     g_object_unref (task);
> }
> 
> Could you try that with the TOBY-R200?
> 

That fixed it.  I will include this in another submission of the patch.

Regards,
Matthew Starr