--- plugins/ublox.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/plugins/ublox.c b/plugins/ublox.c index b65bc52a..9ee38a6b 100644 --- a/plugins/ublox.c +++ b/plugins/ublox.c @@ -102,13 +102,6 @@ static void ublox_remove(struct ofono_modem *modem) g_free(data); } -static GAtChat *open_device(struct ofono_modem *modem, - const char *key, char *debug) -{ - return at_util_open_device(modem, key, ublox_debug, debug, - NULL); -} - static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data) { struct ofono_modem *modem = user_data; @@ -236,17 +229,20 @@ static int ublox_enable(struct ofono_modem *modem) DBG("%p", modem); - data->aux = open_device(modem, "Aux", "Aux: "); + data->aux = at_util_open_device(modem, "Aux", + ublox_debug, "Aux: ", NULL); /* If this is a serial modem then the device may be behind * the 'Device' attribute instead... */ if (data->aux == NULL) { - data->aux = open_device(modem, "Device", "Aux: "); + data->aux = at_util_open_device(modem, "Device", + ublox_debug, "Aux: ", NULL); if (data->aux == NULL) return -EINVAL; } - data->modem = open_device(modem, "Modem", "Modem: "); + data->modem = at_util_open_device(modem, "Modem", + ublox_debug, "Modem: ", NULL); if (data->modem) { g_at_chat_set_slave(data->modem, data->aux); g_at_chat_send(data->modem, "ATE0 +CMEE=1", none_prefix, -- 2.20.1

uBlox devices present their USB interfaces well before those interfaces are ready to respond to any commands. The documentation says to monitor the 'greeting text' to detect readiness, but this 'greeting text' is not actually specified for any device other than the TOBY L4. What seems to work is to probe the device with 'AT' commands until the device responds, and then to wait an additional second before proceeding. The TOBY L4 reliably sends its 'greeting text' (+AT: READY) within this interval. It would be more rigorous to actually wait for the 'READY' indication for the TOBY L4, but that would require knowing the device model before the device model is actually queried. This is doable via the USB product ID, but overkill when the above heuristic seems to work reliably. Before this patch, the ublox plugin was trying to achieve something like the above with the g_at_chat_set_wakeup_command() function, but that had some issues: i) it did not work reliably, in particular failing badly on the TOBY L4 with responses getting out of sync with commands ii) it was an inappropriate use of the wakeup_command which is intended for devices that may sleep when there is no communication during some interval This patch adds an init sequence that probes the device for readiness before continuing with initialization. --- plugins/ublox.c | 121 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 107 insertions(+), 14 deletions(-) diff --git a/plugins/ublox.c b/plugins/ublox.c index 9ee38a6b..656dc553 100644 --- a/plugins/ublox.c +++ b/plugins/ublox.c @@ -29,6 +29,7 @@ #include <glib.h> #include <gatchat.h> #include <gattty.h> +#include <ell/ell.h> #define OFONO_API_SUBJECT_TO_CHANGE #include <ofono/plugin.h> @@ -66,6 +67,10 @@ struct ublox_data { const struct ublox_model *model; int flags; + + struct l_timeout *init_timeout; + int init_count; + guint init_cmd; }; static void ublox_debug(const char *str, void *user_data) @@ -223,6 +228,82 @@ fail: ofono_modem_set_powered(modem, FALSE); } +static void init_cmd_cb(gboolean ok, GAtResult *result, void *user_data) +{ + struct ofono_modem *modem = user_data; + struct ublox_data *data = ofono_modem_get_data(modem); + + DBG("%p", modem); + + if (!ok) + goto fail; + + /* When the 'init command' succeeds, we insert an additional + * delay of 1 second before proceeding with the actual + * intialization of the device. We reuse the init_timeout + * instance for this, just clearing the command to indicate + * that additional retries aren't necessary. + */ + data->init_cmd = 0; + data->init_count = 0; + l_timeout_modify_ms(data->init_timeout, 1000); + + return; + +fail: + l_timeout_remove(data->init_timeout); + data->init_timeout = NULL; + + g_at_chat_unref(data->aux); + data->aux = NULL; + g_at_chat_unref(data->modem); + data->modem = NULL; + ofono_modem_set_powered(modem, FALSE); +} + +static void init_timeout_cb(struct l_timeout *timeout, void *user_data) +{ + struct ofono_modem *modem = user_data; + struct ublox_data *data = ofono_modem_get_data(modem); + + DBG("%p", modem); + + /* As long as init_cmd is set we need to either keep retrying + * or fail everything after excessive retries + */ + if (data->init_cmd && data->init_count++ < 20) { + g_at_chat_retry(data->aux, data->init_cmd); + l_timeout_modify_ms(timeout, 1000); + return; + } + + l_timeout_remove(data->init_timeout); + data->init_timeout = NULL; + + if (data->init_cmd) { + ofono_error("failed to init modem after 20 attempts"); + goto fail; + } + + g_at_chat_send(data->aux, "ATE0", none_prefix, + NULL, NULL, NULL); + g_at_chat_send(data->aux, "AT+CMEE=1", none_prefix, + NULL, NULL, NULL); + + if (g_at_chat_send(data->aux, "AT+CGMM", NULL, + query_model_cb, modem, NULL) > 0) + return; + +fail: + g_at_chat_unref(data->aux); + data->aux = NULL; + g_at_chat_unref(data->modem); + data->modem = NULL; + ofono_modem_set_powered(modem, FALSE); + return; +} + + static int ublox_enable(struct ofono_modem *modem) { struct ublox_data *data = ofono_modem_get_data(modem); @@ -250,22 +331,34 @@ static int ublox_enable(struct ofono_modem *modem) g_at_chat_send(data->modem, "AT&C0", NULL, NULL, NULL, NULL); } - /* The modem can take a while to wake up if just powered on. */ - g_at_chat_set_wakeup_command(data->aux, "AT\r", 1000, 11000); - - g_at_chat_send(data->aux, "ATE0", none_prefix, - NULL, NULL, NULL); - g_at_chat_send(data->aux, "AT+CMEE=1", none_prefix, - NULL, NULL, NULL); - - if (g_at_chat_send(data->aux, "AT+CGMM", NULL, - query_model_cb, modem, NULL) > 0) - return -EINPROGRESS; + /* + * uBlox devices present their USB interfaces well before those + * interfaces are actually ready to use. The specs say to monitor + * the 'greeting text' to detect whether the device is ready to use; + * unfortunately, other than for the TOBY L4, the greeting text is + * not actually specified. + * + * What has been determined experimentally to work is to probe with + * an 'AT' command until it responds and then wait an additional + * second before continuing with device initialization. Even for + * the TOBY L4 where one should wait for the '+AT: READY' URC + * before intialization, this seems to be sufficient; the 'READY' + * indication always arrives within this time. + * + * (It would be more rigorous to actually wait for the 'READY' + * indication, but that would require knowing the device model + * before the device model is actually queried. Do-able via + * USB Product ID, but overkill when the above seems to work + * reliably.) + */ - g_at_chat_unref(data->aux); - data->aux = NULL; + data->init_count = 0; + data->init_cmd = g_at_chat_send(data->aux, "AT", none_prefix, + init_cmd_cb, modem, NULL); + data->init_timeout = l_timeout_create_ms(500, init_timeout_cb, modem, + NULL); - return -EINVAL; + return -EINPROGRESS; } static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data) -- 2.20.1

On 24/09/2019 17:05, Denis Kenzior wrote: Yes... init_cmd gets cleared in init_cmd_cb when the command succeeds. We reuse the callback to get an extra 1 second delay (which we require) before continuing with the initialization. There's a comment about this in init_cmd_cb above. Yes. Yes, but it's not needed anywhere else... are you requiring a static inline function for this or can I just resubmit this without the return statement? /Jonas

Hi Jonas, Den tis 24 sep. 2019 kl 13:33 skrev Jonas Bonn <jonas(a)norrbonn.se&gt;: ....and in that case we could not restart ofono.... I have seen this for L2 as well. I have a local patch which is ugly so I did not push it here, where I flushed the commands. It seems like the L2 buffers up the command and replies to all of the later. Since we times out and forgets about commands and don't use echo we get out of sync and model detection fails. +1, very annoying since it happens after some URCs too. Gah saw you had a new version sent out, will review the code there. BR Richard