atmodem/sim: added vendor Gemalto

also renamed the cinterion_* functions in gemalto_*. Eventually, the vendor cinterion will be removed.

---
 drivers/atmodem/sim.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

index 0907635d..fd25d77c 100644
@@ -66,7 +66,7 @@ static const char *oercn_prefix[] = { "_OERCN:", NULL };
 static const char *cpinr_prefixes[] = { "+CPINR:", "+CPINRE:", NULL };
 static const char *epin_prefix[] = { "*EPIN:", NULL };
 static const char *simcom_spic_prefix[] = { "+SPIC:", NULL };
-static const char *cinterion_spic_prefix[] = { "^SPIC:", NULL };
+static const char *gemalto_spic_prefix[] = { "^SPIC:", NULL };
 static const char *pct_prefix[] = { "#PCT:", NULL };
 static const char *pnnm_prefix[] = { "+PNNM:", NULL };
 static const char *qpinc_prefix[] = { "+QPINC:", NULL };

@@ -1110,7 +1110,7 @@ error:
 	CALLBACK_WITH_FAILURE(cb, NULL, cbd->data);
 }
 
-static void cinterion_spic_cb(gboolean ok, GAtResult *result,
+static void gemalto_spic_cb(gboolean ok, GAtResult *result,
 							gpointer user_data)
 {
 	struct cb_data *cbd = user_data;

@@ -1228,8 +1228,9 @@ static void at_pin_retries_query(struct ofono_sim *sim,
 			return;
 		break;
 	case OFONO_VENDOR_CINTERION:
-		if (g_at_chat_send(sd->chat, "AT^SPIC", cinterion_spic_prefix,
-					cinterion_spic_cb, cbd, g_free) > 0)
+	case OFONO_VENDOR_GEMALTO:
+		if (g_at_chat_send(sd->chat, "AT^SPIC", gemalto_spic_prefix,
+					gemalto_spic_cb, cbd, g_free) > 0)
 			return;
 		break;
 	default:

2.17.1

Hi Giacinto,

This is all great and all, but where are my patches for actually fixing the use of VENDOR_CINTERION for plugins/gemalto.c?

Regards,
-Denis

Patience, I need to prepare everything beforehand.

VENDOR_CINTERION is selected in udevng it is for serial modems, it is mainly based on the TC65i model,
and apparently not properly tested for ages: back to our discussion about AT+VTS with quotes, I have discovered that
it was with quotes in ofono atmodem at the beginning, most likely for the TC65i, and then removed, but not entirely:
the string allocation in atmodem/voicecall.c still takes into account the room for the quotes in the code:
buf = g_try_new(char, len * 9 + 3);
the 9 in this command is the length of "+VTS=\"n\";".
(but the command above it has been updated!)

Most Gemalto modems have a serial port, that can be used in complement or instead of USB ports, and
in my code I have merged VENDOR_CINTERION and VENDOR_GEMALTO.

The plugin is the final part I am going to push. There are quite a few topics to discuss before I can do it.

In the meanwhile, will you take this change (and related ones)?

With this change, VENDOR_CINTERION works as before for now, but the internal prefixes need update.

Hi Giacinto,

No :)

Each change set needs to be self contained. I see no reason why you can't make VENDOR_CINTERION changes now in the relevant places. Even if you eventually plan to remove plugins/cinterion.c completely in the future.

I can only read code, not people's minds.

Regards,
-Denis

But the VENDOR_CINTERION remains in the code, there are no changes for the plugin.

Hi Giacinto,

That's exactly the point. You have taken out all VENDOR_CINTERION logic, yet plugins/gemalto.c still sets VENDOR_CINTERION for the sim atom. So now you broke that driver. Your change set is not self-contained. If I apply this patch and you get hit by a bus tomorrow, the plugin remains broken for everyone.

Regards,
-Denis

It is not taken out. It is:

       case OFONO_VENDOR_CINTERION:
       case OFONO_VENDOR_GEMALTO:
               if (g_at_chat_send(sd->chat, "AT^SPIC", gemalto_spic_prefix,
                                       gemalto_spic_cb, cbd, g_free) > 0)

both are there, and the functions are renamed, but still there. Or shall I duplicate the functions for the time being?

Hi Giacinto,

Ok, I stand corrected, I assumed you removed OFONO_VENDOR_CINTERION from the sms driver.

But still, can we strive for self-contained change sets? Just take out VENDOR_CINTERION use inside the sim driver and change over to VENDOR_GEMALTO for any modem drivers that need it. That makes things much easier to follow compared to doing only the sim driver changes now and then flipping from one VENDOR to another at some later point.

Regards,
-Denis

Hi Denis

But still, can we strive for self-contained change sets? Just take out

they are self-contained, but I see what you mean.
I will have to submit together then a file from atmodem and a change in plugin/cinterion.c, to be self-contained.
Another exception to the rule, I suppose, but I will do so.

Regards,

Regards,
Giacinto

Hi Giacinto,

rule, I suppose, but I will do so.
No, there's no exception here. Change set = set of patches. So you need to send me a series of 2 patches. First patch changes drivers/atmodem/sim.c. Second patch changes plugins/cinterion.c or plugins/gemalto.c or both.

Regards,
-Denis