Re: [PATCH] quectel: get devinfo

[Denis Kenzior]

Hi Sean,


Good plan :)
I have been looking at it. [0]


Yep, that looks like a good start.



Should I catch the callback directly in atmodem/devinfo.c instead?


You could try to intercept this in attr_cb itself.  But you'd need access to 
vendor selector somehow.  Perhaps a custom data structure instead of cb_data. 
Alternatively store the prefix in struct dev_data and set cb_data->user to 
struct dev_data.


Something like:

static void attr_cb()
{
...

if (at_util_parse_attr(...)) {
...
}

if (dev->vendor == QUECTEL &&
(!strcmp(dev->prefix, ...) ||
!strcmp(dev->prefix, ...))) {
// Strip Revision:
}

...
}



We could also just look for a "Revision: " in query_revision_cb(), but
it's kinda hacky


That's the other approach.  Introduce callback(s) for both revision and 
manufacturer commands and handle the quirk there.  Not sure what else we can do?


Regards,
-Denis
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org

Re: [PATCH] quectel: get devinfo

[Sean Nyekjaer]

On Mon, Dec 06, 2021 at 10:45:35AM -0600, Denis Kenzior wrote:
> Hi Sean,
> 
> On 12/6/21 3:50 AM, Sean Nyekjaer wrote:
> > Quectel devices returns "Revision:" before the manufacture and revision.
> > Via dbus:
> > "Manufacturer" s "Revision: MTK 0828"
> > "Revision" s "Revision: M95FAR02A08"
> > ---
> >   plugins/quectel.c | 2 ++
> >   1 file changed, 2 insertions(+)
> > 
> 
> Applied.  But you may want to add a quectel specific quirk handler in the
> devinfo driver that strips the 'Revision: ' prefix.  It should not be part
> of the response.
> 
> Regards,
> -Denis

Hi Denis,

Good plan :)
I have been looking at it. [0]

Should I catch the callback directly in atmodem/devinfo.c instead?

We could also just look for a "Revision: " in query_revision_cb(), but
it's kinda hacky

/Sean

[0]:


diff --git a/drivers/atmodem/devinfo.c b/drivers/atmodem/devinfo.c
index ff7386cd..b64d9977 100644
--- a/drivers/atmodem/devinfo.c
+++ b/drivers/atmodem/devinfo.c
@@ -37,6 +37,11 @@
 
 static const char *gcap_prefix[] = { "+GCAP:", NULL };
 
+struct dev_data {
+   GAtChat *chat;
+   unsigned int vendor;
+};
+
 static void attr_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
struct cb_data *cbd = user_data;
@@ -64,11 +69,11 @@ static void at_query_manufacturer(struct ofono_devinfo 
*info,
ofono_devinfo_query_cb_t cb, void *data)
 {
struct cb_data *cbd = cb_data_new(cb, data);
-   GAtChat *chat = ofono_devinfo_get_data(info);
+   struct dev_data *dev = ofono_devinfo_get_data(info);
 
cbd->user = "+CGMI:";
 
-   if (g_at_chat_send(chat, "AT+CGMI", NULL, attr_cb, cbd, g_free) > 0)
+   if (g_at_chat_send(dev->chat, "AT+CGMI", NULL, attr_cb, cbd, g_free) > 
0)
return;
 
g_free(cbd);
@@ -80,11 +85,11 @@ static void at_query_model(struct ofono_devinfo *info,
ofono_devinfo_query_cb_t cb, void *data)
 {
struct cb_data *cbd = cb_data_new(cb, data);
-   GAtChat *chat = ofono_devinfo_get_data(info);
+   struct dev_data *dev = ofono_devinfo_get_data(info);
 
cbd->user = "+CGMM:";
 
-   if (g_at_chat_send(chat, "AT+CGMM", NULL, attr_cb, cbd, g_free) > 0)
+   if (g_at_chat_send(dev->chat, "AT+CGMM", NULL, attr_cb, cbd, g_free) > 
0)
return;
 
g_free(cbd);
@@ -96,11 +101,11 @@ static void at_query_revision(struct ofono_devinfo *info,
ofono_devinfo_query_cb_t cb, void *data)
 {
struct cb_data *cbd = cb_data_new(cb, data);
-   GAtChat *chat = ofono_devinfo_get_data(info);
+   struct dev_data *dev = ofono_devinfo_get_data(info);
 
cbd->user = "+CGMR:";
 
-   if (g_at_chat_send(chat, "AT+CGMR", NULL, attr_cb, cbd, g_free) > 0)
+   if (g_at_chat_send(dev->chat, "AT+CGMR", NULL, attr_cb, cbd, g_free) > 
0)
return;
 
g_free(cbd);
@@ -112,11 +117,11 @@ static void at_query_serial(struct ofono_devinfo *info,
ofono_devinfo_query_cb_t cb, void *data)
 {
struct cb_data *cbd = cb_data_new(cb, data);
-   GAtChat *chat = ofono_devinfo_get_data(info);
+   struct dev_data *dev = ofono_devinfo_get_data(info);
 
cbd->user = "+CGSN:";
 
-   if (g_at_chat_send(chat, "AT+CGSN", NULL, attr_cb, cbd, g_free) > 0)
+   if (g_at_chat_send(dev->chat, "AT+CGSN", NULL, attr_cb, cbd, g_free) > 
0)
return;
 
g_free(cbd);
@@ -134,11 +139,16 @@ static void capability_cb(gboolean ok, GAtResult *result, 
gpointer user_data)
 static int at_devinfo_probe(struct ofono_devinfo *info, unsigned int vendor,
void *data)
 {
-   GAtChat *chat = g_at_chat_clone(data);
+   GAtChat *chat = data;
+   struct dev_data *dev;
+
+   dev = g_new0(struct dev_data, 1);
+   dev->chat = g_at_chat_clone(chat);
+   dev->vendor = vendor;
 
-   ofono_devinfo_set_data(info, chat);
+   ofono_devinfo_set_data(info, dev);
 
-   g_at_chat_send(chat, "AT+GCAP", gcap_prefix,
+   g_at_chat_send(dev->chat, "AT+GCAP", gcap_prefix,
capability_cb, info, NULL);
 
return 0;
@@ -146,11 +156,11 @@ static int at_devinfo_probe(struct ofono_devinfo *info, 
unsigned int vendor,
 
 static void at_devinfo_remove(struct ofono_devinfo *info)
 {
-   GAtChat *chat = ofono_devinfo_get_data(info);
+   struct dev_data *dev = ofono_devinfo_get_data(info);
 
-   ofono_devinfo_set_data(info, NULL);
+   g_at_chat_unref(dev->chat);
 
-   g_at_chat_unref(chat);
+   ofono_devinfo_set_data(info, NULL);
 }
 
 static const struct ofono_devinfo_driver driver = {
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org