3:28 p.m.
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(-)
diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index 0907635d..fd25d77c 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -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
7 a.m.
Hi Giacinto,
On 09/23/2018 12:28 AM, Giacinto Cifelli wrote:
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(-)
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
1:24 p.m.
On Mon, Sep 24, 2018 at 11:00 PM Denis Kenzior <denkenz(a)gmail.com> wrote:
Hi Giacinto,
On 09/23/2018 12:28 AM, Giacinto Cifelli wrote:
> 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(-)
>
This is all great and all, but where are my patches for actually fixing
the use of VENDOR_CINTERION for plugins/gemalto.c?
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.
Regards,
-Denis
1:48 p.m.
Hi Giacinto,
In the meanwhile, will you take this change (and related ones)?
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
1:50 p.m.
On Tue, Sep 25, 2018 at 5:48 AM Denis Kenzior <denkenz(a)gmail.com> wrote:
Hi Giacinto,
> In the meanwhile, will you take this change (and related ones)?
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.
But the VENDOR_CINTERION remains in the code, there are no changes for the
plugin.
I can only read code, not people's minds.
Regards,
-Denis
2:04 p.m.
Hi Giacinto,
But the VENDOR_CINTERION remains in the code, there are no changes
for
the plugin.
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
2:07 p.m.
On Tue, Sep 25, 2018 at 6:04 AM Denis Kenzior <denkenz(a)gmail.com> wrote:
Hi Giacinto,
> But the VENDOR_CINTERION remains in the code, there are no changes for
> the plugin.
>
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.
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?
Regards,
-Denis
2:16 p.m.
Hi Giacinto,
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?
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
2:24 p.m.
Hi Denis
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.
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,
-Denis
Regards,
Giacinto
2:27 p.m.
Hi Giacinto,
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.
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
1454
days inactive
1456
days old
9 comments
2 participants
participants (2)
-
Denis Kenzior -
Giacinto Cifelli