<div dir="ltr"><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">Awesome! Thank you! Carlo</div></div><div class="gmail_extra"> <div class="gmail_quote">On 29 May 2017 at 12:27, Aleksander Morgado <<a href="mailto:aleksander@aleksander.es" target="_blank">aleksander@aleksander.es</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 29/05/17 09:48, Carlo Lobrano wrote: > Currently Dell plugin implements a retry logic of three commands > (AT+GMI, AT+CGMI, ATI1I2I3) to detect modem's vendor string. > Moreover, since Telit modems always reply to the first command, to speed > the probing time up, those modem are tagged with an Udev rule so that we > can avoid sending the other two commands at all. > > However, the retry logic is in case a port needs some time to reply, so > it makes sense to apply it only to the first command. Then if the port still > does not respond with any kind of reply, that probably means that it is not > AT capable and we can skip the other AT commands as well. > > Then, this patch: > - sets a maximum number of timeouts for AT+GMI to 3. After this > timeouts, the port is considered not AT-capable. > - sets AT+CGMI and ATI1I2I3 to be sent only once. > - removes Dell udev rule for tagging Telit Modems. > > --- > > Patch v2: > > As per Aleksander's code review > > - mm_port_probe_set_result_at set to false when run out of retries for manufacturer (and yes, I do confirm that without this line, I saw other "AT" commands sent to the ports). > - fixed debug logs > > @Aleksander I'm going to reply to this email with the logs you requested. > Pushed to git master, thanks! The logs look good to be, btw, thanks for sending them. <div><div class="h5"> > --- > plugins/dell/77-mm-dell-port-types.rules | 4 ---- > plugins/dell/mm-plugin-dell.c | 29 ++++++++++++++++++----------- > 2 files changed, 18 insertions(+), 15 deletions(-) > > diff --git a/plugins/dell/77-mm-dell-port-types.rules b/plugins/dell/77-mm-dell-port-types.rules > index bbb59b9..206fb6e 100644 > --- a/plugins/dell/77-mm-dell-port-types.rules > +++ b/plugins/dell/77-mm-dell-port-types.rules > @@ -8,9 +8,5 @@ GOTO="mm_dell_port_types_end" > LABEL="mm_dell_vendorcheck" > SUBSYSTEMS=="usb", ATTRS{bInterfaceNumber}=="?*", ENV{.MM_USBIFNUM}="$attr{bInterfaceNumber}" > > -# DW5580 is a Dell-branded Telit modem > -# tag is needed here for dynamic port recognition > -ATTRS{idVendor}=="413c", ATTRS{idProduct}=="81ba", ENV{ID_MM_TELIT_PORTS_TAGGED}="1" > - > GOTO="mm_dell_port_types_end" > LABEL="mm_dell_port_types_end" > diff --git a/plugins/dell/mm-plugin-dell.c b/plugins/dell/mm-plugin-dell.c > index f9dfd1a..fbb0a9c 100644 > --- a/plugins/dell/mm-plugin-dell.c > +++ b/plugins/dell/mm-plugin-dell.c > @@ -45,6 +45,8 @@ > #include "mm-broadband-modem-mbim.h" > #endif > > +#define MAX_PORT_PROBE_TIMEOUTS 3 > + > G_DEFINE_TYPE (MMPluginDell, mm_plugin_dell, MM_TYPE_PLUGIN) > > MM_PLUGIN_DEFINE_MAJOR_VERSION > @@ -70,6 +72,7 @@ typedef struct { > guint gmi_retries; > guint cgmi_retries; > guint ati_retries; > + guint timeouts; > } CustomInitContext; > > static void > @@ -140,6 +143,8 @@ static void custom_init_step (CustomInitContext *ctx); > static void > custom_init_step_next_command (CustomInitContext *ctx) > { > + ctx->timeouts = 0; > + > if (ctx->gmi_retries > 0) > ctx->gmi_retries = 0; > else if (ctx->cgmi_retries > 0) > @@ -169,6 +174,7 @@ response_ready (MMPortSerialAt *port, > return; > } > /* Directly retry same command on timeout */ > + ctx->timeouts++; > custom_init_step (ctx); > g_error_free (error); > return; > @@ -269,6 +275,13 @@ custom_init_step (CustomInitContext *ctx) > } > #endif > > + if (ctx->timeouts >= MAX_PORT_PROBE_TIMEOUTS) { > + mm_dbg ("(Dell) couldn't detect real manufacturer in (%s): too many timeouts", > + mm_port_get_device (MM_PORT (ctx->port))); > + mm_port_probe_set_result_at (ctx->probe, FALSE); > + goto out; > + } > + > if (ctx->gmi_retries > 0) { > ctx->gmi_retries--; > mm_port_serial_at_command (ctx->port, > @@ -309,9 +322,10 @@ custom_init_step (CustomInitContext *ctx) > return; > } > > - /* Finish custom_init */ > - mm_dbg ("(Dell) couldn't flip secondary port to AT in (%s): all retries consumed", > + mm_dbg ("(Dell) couldn't detect real manufacturer in (%s): all retries consumed", > mm_port_get_device (MM_PORT (ctx->port))); > +out: > + /* Finish custom_init */ > g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE); > custom_init_context_complete_and_free (ctx); > } > @@ -337,15 +351,8 @@ dell_custom_init (MMPortProbe *probe, > ctx->port = g_object_ref (port); > ctx->cancellable = cancellable ? g_object_ref (cancellable) : NULL; > ctx->gmi_retries = 3; > - ctx->cgmi_retries = 3; > - ctx->ati_retries = 3; > - > - /* Dell-branded Telit modems always answer to +GMI > - * Avoid +CGMI and ATI sending for minimizing port probing time */ > - if (mm_kernel_device_get_global_property_as_boolean (port_device, "ID_MM_TELIT_PORTS_TAGGED")) { > - ctx->cgmi_retries = 0; > - ctx->ati_retries = 0; > - } > + ctx->cgmi_retries = 1; > + ctx->ati_retries = 1; > > custom_init_step (ctx); > } > -- </div></div>Aleksander <a href="https://aleksander.es" rel="noreferrer" target="_blank">https://aleksander.es</a> </blockquote></div> </div>