[PATCH] ublox: use ID_MM_UBLOX_PORT_READY_DELAY udev flag as init

[Aleksander Morgado]

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?

-- 
Aleksander
https://aleksander.es