[PATCH v2] atmodem: Signal quality on quectel serial modems
From: Lars Poeschel As the default way of getting the signal quality with +CIND is also unstable on quectel serial modems (the same as on quectel EC21). In fact the signal quality is only updated on cell changes. Those trigger a manual AT+CSQ in ofono and get an update this way, but the URCs do not work. So we implement a quectelish way here as well. The quectelish way is very similar to the way ifx modems work. We can reuse their csq_notify function. --- drivers/atmodem/network-registration.c | 24 +--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/drivers/atmodem/network-registration.c b/drivers/atmodem/network-registration.c index 78b1994c..c1309f61 100644 --- a/drivers/atmodem/network-registration.c +++ b/drivers/atmodem/network-registration.c @@ -780,15 +780,26 @@ static void ifx_xciev_notify(GAtResult *result, gpointer user_data) */ } -static void ifx_xcsq_notify(GAtResult *result, gpointer user_data) +static void ifx_quec_csq_notify(GAtResult *result, gpointer user_data) { struct ofono_netreg *netreg = user_data; + struct at_netreg_data *nd = ofono_netreg_get_data(netreg); int rssi, ber, strength; GAtResultIter iter; + const char *prefix; g_at_result_iter_init(&iter, result); - if (!g_at_result_iter_next(&iter, "+XCSQ:")) + switch (nd->vendor) { + case OFONO_VENDOR_QUECTEL_SERIAL: + prefix = "+CSQN:"; + break; + default: + prefix = "+XCSQ:"; + break; + } + + if (!g_at_result_iter_next(&iter, prefix)) return; if (!g_at_result_iter_next_number(&iter, &rssi)) @@ -2027,7 +2038,7 @@ static void at_creg_set_cb(gboolean ok, GAtResult *result, gpointer user_data) /* Register for specific signal strength reports */ g_at_chat_register(nd->chat, "+XCIEV:", ifx_xciev_notify, FALSE, netreg, NULL); - g_at_chat_register(nd->chat, "+XCSQ:", ifx_xcsq_notify, + g_at_chat_register(nd->chat, "+XCSQ:", ifx_quec_csq_notify, FALSE, netreg, NULL); g_at_chat_send(nd->chat, "AT+XCSQ=1", none_prefix, NULL, NULL, NULL); @@ -2118,6 +2129,13 @@ static void at_creg_set_cb(gboolean ok, GAtResult *result, gpointer user_data) g_at_chat_send(nd->chat, "AT+QINDCFG=\"act\",1", none_prefix, NULL, NULL, NULL); break; + case OFONO_VENDOR_QUECTEL_SERIAL: + g_at_chat_register(nd->chat, "+CSQN:", + ifx_quec_csq_notify, FALSE, netreg, NULL); + /* Register for specific signal strength reports */ + g_at_chat_send(nd->chat, "AT+QEXTUNSOL=\"SQ\",1", none_prefix, + NULL, NULL, NULL); + break; default: g_at_chat_send(nd->chat, "AT+CIND=?", cind_prefix, cind_support_cb, netreg, NULL); -- 2.27.0 ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH 1/2] atmodem: Detect usage of AT+CGEREP
Hi Denis, On Thu, Aug 20, 2020 at 09:41:28AM -0500, Denis Kenzior wrote: > > > So why not just run iter_next_range in a loop? it actually understands > > > both > > > lists and ranges. See cind_support_cb() for an example. > > > > Ok, I can do this i a loop if you want. > > > > > > + /* if min1 == max1 we had no range but a list and that > > > > +* means we are interested in the last number of that list*/ > > > > + if (min1 == max1) { > > > > + while (!g_at_result_iter_close_list(&iter)) { > > > > + if (!g_at_result_iter_next_number(&iter, &max1)) > > > > + break; > > > > + } > > > > + } > > > > + > > > > + if (!g_at_result_iter_skip_next(&iter)) { > > > > + two_arguments = false; > > > > + goto out; > > > > + } > > > > > > Not sure what this is doing? isn't +CGEREP just two lists ? According > > > to 27.007: > > > "+CGEREP: (list of supported s),(list of supported s)" > > > > Well, we have to deal with very different +CGEREP results. For example > > on the quectel EC21 I get this: > > > > "+CGEREP: (0-2),(0,1)" > > > > and on the quectel M95 I get this: > > > > "+CGEREP: (0,1)" > > > > So what the code does is this: > > It tries to parse a range with > > > > g_at_result_iter_next_range(&iter, &min1, &max1) > > > > Now two things can happen: It either parsed the range "(0-2)" then we > > have min1 != max1, or it tried to parse a list "(0,1)". This time > > iter_next_range() breaks on the comma ',' and exits with > > min1 == max1 == 0. Then we know, we did not find the maximum value yet > > and we enter the loop: > > > > ▸··▸···while (!g_at_result_iter_close_list(&iter)) { > > ▸··▸···▸···if (!g_at_result_iter_next_number(&iter, &max1)) > > ▸··▸···▸···▸···break; > > ▸··▸···} > > > > This does then loop to the end of the list until iter_close_list() > > becomes true, which is at the closing paranthesis ')'. max1 then > > contains the last item in that list (which we suspect to be the > > maximum value). > > So in theory this could be written like: > int max = -1; > iter_open_list(&iter); > > while (iter_next_range(&iter, &max1, &max2)) { > if (max2 > max) > max = max2; > } > > iter_close_list(&iter); I can do this and it does indeed work! :-) This may be my personal thing, but I find this not very intuitive and hard to read. To make this more intuitive can we: * move your proposed while loop to a new function * name that function ...iter_next_range_or_list (or something like that) * put that function into gatresult.c * and in the new at_cgerep_test_cb just use this function ? I can imagine this new function can be of use at other places as well. What do you think ? Regards, Lars ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org