Hi Lars,

On 5/28/20 4:32 AM, poesc...@lemonage.de wrote:
From: Lars Poeschel <poesc...@lemonage.de>

The Quectel EC21 does only work correctly, if the mux channel used for
aux is the first mux channel. It does only put it's URC messages in the
first mux channel, so this has to be the aux channel in our case.
To be flexible on the mux order we introduce an array here, that
contains the initialization data in it's needed order and is then simply
applied by a for-loop. Initialization of this array is done after we
queried the modem model.
---
  plugins/quectel.c | 72 ++++++++++++++++++++++++++++++++++++++---------
  1 file changed, 58 insertions(+), 14 deletions(-)


So this looks mostly reasonable, but see below:

diff --git a/plugins/quectel.c b/plugins/quectel.c
index 043d39f9..1cad35d6 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -78,6 +78,21 @@ static const uint8_t gsm0710_terminate[] = {
        0xf9, /* close flag */
  };
+enum mux_type {
+       QUECTEL_MUX_TYPE_AUX = 0,
+       QUECTEL_MUX_TYPE_MODEM,
+       QUECTEL_MUX_TYPE_MAX,
+};
+
+struct mux_initialization_data {
+       enum mux_type mux_type;
+       char *chat_debug;
+       const char *n_gsm_key;
+       const char *n_gsm_value;
+};
+
+static struct mux_initialization_data mux_order[QUECTEL_MUX_TYPE_MAX];
+

So the issue with this is that this driver now no-longer supports multiple modems of the same type active simultaneously (since all instances would be trying to write / read from this location).

A better approach would be to use two such variables, i.e. mux_order_ec21 and mux_order_default and then either store a pointer to the one to use inside quectel_data, or programmatically by looking up based on the quectel_data::model.

Alternatively, storing mux_order in quectel_data itself is also an option.

  enum quectel_model {
        QUECTEL_UNKNOWN,
        QUECTEL_UC15,
@@ -1014,6 +1037,17 @@ static void cgmm_cb(int ok, GAtResult *result, void 
*user_data)
                return;
        }
+ mux_order[0] = (struct mux_initialization_data)
+                       { QUECTEL_MUX_TYPE_MODEM,
+                       "Modem: ",
+                       "Modem",
+                       "/dev/gsmtty1"};
+       mux_order[1] = (struct mux_initialization_data)
+                       { QUECTEL_MUX_TYPE_AUX,
+                       "Aux: ",
+                       "Aux",
+                       "/dev/gsmtty2"};
+

This would then move into the static initializer above...

        if (strcmp(model, "UC15") == 0) {
                DBG("%p model UC15", modem);
                data->vendor = OFONO_VENDOR_QUECTEL;
@@ -1030,6 +1064,16 @@ static void cgmm_cb(int ok, GAtResult *result, void 
*user_data)
                DBG("%p model EC21", modem);
                data->vendor = OFONO_VENDOR_QUECTEL;
                data->model = QUECTEL_EC21;
+               mux_order[0] = (struct mux_initialization_data)
+                               { QUECTEL_MUX_TYPE_AUX,
+                               "Aux: ",
+                               "Aux",
+                               "/dev/gsmtty1"};
+               mux_order[1] = (struct mux_initialization_data)
+                               { QUECTEL_MUX_TYPE_MODEM,
+                               "Modem: ",
+                               "Modem",
+                               "/dev/gsmtty2"};
        } else {
                ofono_warn("%p unknown model: '%s'", modem, model);
                data->vendor = OFONO_VENDOR_QUECTEL;


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

Reply via email to