[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(, result);
- if (!g_at_result_iter_next(, "+XCSQ:"))
+ switch (nd->vendor) {
+ case OFONO_VENDOR_QUECTEL_SERIAL:
+ prefix = "+CSQN:";
+ break;
+ default:
+ prefix = "+XCSQ:";
+ break;
+ }
+
+ if (!g_at_result_iter_next(, prefix))
return;
if (!g_at_result_iter_next_number(, ))
@@ -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()) {
> > > > + if (!g_at_result_iter_next_number(, ))
> > > > + break;
> > > > + }
> > > > + }
> > > > +
> > > > + if (!g_at_result_iter_skip_next()) {
> > > > + 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(, , )
> >
> > 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()) {
> > ▸··▸···▸···if (!g_at_result_iter_next_number(, ))
> > ▸··▸···▸···▸···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();
>
> while (iter_next_range(, , )) {
> if (max2 > max)
> max = max2;
> }
>
> iter_close_list();
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
