Re: Port forced closed after DEACT
Hi! Could it be that the log is really from a MM 1.10 run instead of a MM > 1.12 run? The fix to ignore disconnection reports when PPP is used was > only added in MM 1.12.0 IIRC. > > that was indeed the case, problem solved! :D Thanks! ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: Port forced closed after DEACT
Hi, I thought this should have been fixed in > 5f29bd64de8127cb326488d68a2a2b64a45e1f45, but I don't see the relevant > "PPP is required for connection, will ignore disconnection reports" > message that should happen once the modem gets connected in PPP mode. > > If you have the setup with you, could you check why this message is > not being logged? that log is under the condition of ipv4/ipv6_config method being PPP, is it the same method printed by print_bearer_info, right? However, I see the log "Launching 3GPP connection attempt with APN ...", so I expect that the rest of the call chain is called up to `dial_3gpp_ready` which sets the method to PPP for AT ports. I still don't understand why the "PPP is required" log does not show up. In MM 1.12.8, I cannot also find the following message, which is in the log instead (disclaimer, I have no access to the very same environment right now) "Found PDP context with CID 1 and PDP type ipv4v6 for APN 'internet'" I guess it is a composed message, but even so, I cannot find it. Was it maybe in an older version? ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: Port forced closed after DEACT
Hi Aleksander, On Fri, 5 Jun 2020, 10:22 am Aleksander Morgado, wrote: > Hey! > > > I am facing an unexpected - as far as I can tell - behavior with > ModemManager 1.12.8 and network manager is 1.22.10 on Ubuntu 20.04, the > modem is LE910 V2. > > > > During and ongoing connection, lasting since 38 minutes, the tty port is > force closed after the following message > > > > (ttyACM3): <-- '+CGEV: ME DEACT IP, "10.29.174.52", > 1' > > > > Is it normal that a deactivation leads to a port close? > > > > No it isn't, this happened due to the mixup of the CLOCAL settings, > full relevant logs: > > Jun 3 16:49:22 orndura ModemManager[418]: (ttyACM3): <-- > '+CGEV: ME DEACT IP, "10.29.174.52", 1' > Jun 3 16:49:22 orndura ModemManager[418]: mobile equipment > request to deactivate context (type IP, address 10.29.174.52, cid 1) > Jun 3 16:49:22 orndura NetworkManager[411]: [1591192162.3478] > modem["ttyACM0"]: modem state changed, 'connected' --> 'registered' > (reason: user-requested) > Jun 3 16:49:22 orndura ModemManager[418]: Bearer > /org/freedesktop/ModemManager1/Bearer/1: explicitly disconnected > > MM detects the disconnection and by itself reports the modem > disconnected, it shouldn't have done that, it should have ignored the > disconnection report detected by itself and wait for pppd to detect it > itself. > > Jun 3 16:49:22 orndura NetworkManager[411]: [1591192162.3479] > device (ttyACM0): state change: activated -> failed (reason > 'modem-no-carrier', sys-iface-state: 'managed') > Jun 3 16:49:22 orndura ModemManager[418]: (ttyACM0): port now > disconnected > Jun 3 16:49:22 orndura NetworkManager[411]: [1591192162.3501] > active-connection[0x19bb480]: set state deactivated (was activated) > Jun 3 16:49:22 orndura ModemManager[418]: Modem > /org/freedesktop/ModemManager1/Modem/1: state changed (connected -> > registered) > Jun 3 16:49:22 orndura NetworkManager[411]: [1591192162.3508] > manager: NetworkManager state is now DISCONNECTED > Jun 3 16:49:22 orndura NetworkManager[411]: [1591192162.3515] > active-connection[0x19bb480]: check-master-ready: not signalling > (state deactivated, no master) > Jun 3 16:49:22 orndura pppd[2774]: Modem hangup > Jun 3 16:49:22 orndura NetworkManager[411]: Modem hangup > > CHUP happens in the TTY, which MM also detects because it was already > also monitoring the port. > > Jun 3 16:49:22 orndura pppd[2774]: nm-ppp-plugin: (nm_phasechange): > status 8 / phase 'network' > Jun 3 16:49:22 orndura ModemManager[418]: (ttyACM3): <-- > '' > Jun 3 16:49:22 orndura ModemManager[418]: (ttyACM0) > unexpected port hangup! > Jun 3 16:49:22 orndura ModemManager[418]: (ttyACM0) forced to > close port > > MM treats the HUP as a port removal, which shouldn't have happened if > the disconnection was reported by NM after pppd exit, as CLOCAL > settings would have been reset. > > I thought this should have been fixed in > 5f29bd64de8127cb326488d68a2a2b64a45e1f45, but I don't see the relevant > "PPP is required for connection, will ignore disconnection reports" > message that should happen once the modem gets connected in PPP mode. > > If you have the setup with you, could you check why this message is > not being logged? > > -- > Aleksander > https://aleksander.es Thanks for the analysis! We were thinking about something related to clocal, but not this detail, indeed. I'll double check with the setup and keep, and report it back Carlo I'll double check the setup, and > > ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: Telit LE910-V2
Hi Ondřej, On Wed, 7 Aug 2019 at 13:24, Ondřej Krajsa wrote: > Hi, > I have problems with TELIT LE910v2 modem, OS is Debian, ModemManager 1.10 > from repository. > The first is as follows: > Sometimes the modem refuses to connect to a bearer, with a error: > > error: couldn't connect the bearer: > 'GDBus.Error:org.freedesktop.libmbim.Error.Status.Failure: Failure' > This error seems to come from the libmbim, could you share the actual commands you use and the logs with debug enabled? > The strange thing is that when I make a call (and hangup) to the modem > (SIM), it connects immediately. > > I use: > mmcli -m 0 --create-bearer = "apn = internet" > mmcli -b 0 -c > > The second problem is that it refuses to connect to the "non-standard" APN > via LTE, it is necessary to use a CID other than 1. Is it possible to set > it up somehow? > What do you mean by "non-standard"? If, as it seems, the modemmanager is using the MBIM protocol to communicate with the modem, CID number should not be a problem, unless a combination of AT + MBIM command has been used (which should be avoided, only AT commands or only MBIM ones). Lilkewise the first problem, could you share the actual commands used and the debug logs? Plus - just for debugging - what's the output of the command "AT+CGDCONT?" in this use case > > Best regards, > Ondrej > ___ > ModemManager-devel mailing list > ModemManager-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel Regards, Carlo ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: MM support for access technology change
Hi Aleksander, Jose Carlo & Daniele in CC, to see if they can shed some light. > I'm looking into it Carlo ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH] telit: use parent logic to load unlock retries
On Jan 4 2018, at 2:05 pm, Carlo Lobrano wrote: > I'll have a look at that today or tomorrow at the most, thanks a lot! > I tested the patch on both LE910 and HE910 (just to test a modem that supports csim lock and one that does not) and it looks very good. Carlo ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH] telit: use parent logic to load unlock retries
Hi Aleksander, On Jan 1 2018, at 7:35 pm, Aleksander Morgado wrote: > The generic broadband modem provides a common method to load unlock > retries based on CSIM queries. We modify the Telit plugin to use the > generic method but keeping the CSIM locking/unlocking logic in place. > --- > > Hey Carlo & everyone, > > The following patch tries to re-use the logic that loads unlock retries from > the parent generic implementation. I left the CSIM locking/unlocking feature > in the Telit plugin. > > What do you think? > > BTW, totally untested, I don't have a Telit modem with me right now. I'll have a look at that today or tomorrow at the most, thanks a lot! ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH] mm-iface-modem: add check_for_sim_swap method and enable steps
Hey, > +Carlo in CC for review/test as he's probably interested in this change. It makes totally sense and looks good to me Carlo ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: RFC: New device filter policies
> This is actually a good test, thank you! > > > > > You're welcome :) ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: RFC: New device filter policies
Hi Aleksander, On Nov 20 2017, at 2:33 pm, Carlo Lobrano wrote: > Hi Aleksander, > > > I plan to get this merged to git master sometime this week; tests and > > comments welcome! > > I plan to have some testing done in the next couple of days > > Carlo I just tested the following modems: HE910, LE910, LE910 V2 (with AT and MBIM protocols), LM940 (which is QMI). Modems are all detected the same way: **mmcli** output is the same for all of the modems using both **master** and *device-filter-policies* branches and the latter both in default and strict mode. This also means that the primary protocol used is also the same for each modem. Probed devices (I looked for log message "probe required"): master and device-filter-policies in **default mode** probe the same ports, while in **strict mode** modemmanager skips only the ports that in master/device-filter-policies default mode are not used. Below an example from LM940 logs + grep 'probe required' lm940-master.log ModemManager ... (Telit) [cdc-wdm1] probe required: 'qmi' ModemManager ... (Telit) [ttyUSB5] probe required: 'at' ModemManager ... (Telit) [ttyUSB4] probe required: 'at' ModemManager ... (Telit) [ttyUSB3] probe required: 'at' ModemManager ... (Telit) [ttyUSB2] probe required: 'at' ModemManager ... (Telit) [ttyUSB1] probe required: 'at' ModemManager ... (Iridium) [ttyS4] probe required: 'at, at-vendor, at-product' ModemManager ... (u-blox) [ttyS4] probe required: 'at, at-vendor' ModemManager ... (Telit) [ttyS4] probe required: 'at, at-vendor' ModemManager ... (Cinterion) [ttyS4] probe required: 'at, at-vendor' ModemManager ... (Nokia) [ttyS4] probe required: 'at, at-vendor, at-icera' ModemManager ... (Via CBP7) [ttyS4] probe required: 'at, at-vendor, at-product' ModemManager ... (Generic) [ttyS4] probe required: 'at, qcdm' + grep 'probe required' lm940-device-filter-policies-default.log ModemManager ... (Telit) [cdc-wdm1] probe required: 'qmi' ModemManager ... (Telit) [ttyUSB5] probe required: 'at' ModemManager ... (Telit) [ttyUSB4] probe required: 'at' ModemManager ... (Telit) [ttyUSB3] probe required: 'at' ModemManager ... (Telit) [ttyUSB2] probe required: 'at' ModemManager ... (Telit) [ttyUSB1] probe required: 'at' ModemManager ... (Iridium) [ttyS4] probe required: 'at, at-vendor, at-product' ModemManager ... (u-blox) [ttyS4] probe required: 'at, at-vendor' ModemManager ... (Telit) [ttyS4] probe required: 'at, at-vendor' ModemManager ... (Cinterion) [ttyS4] probe required: 'at, at-vendor' ModemManager ... (Nokia) [ttyS4] probe required: 'at, at-vendor, at-icera' ModemManager ... (Via CBP7) [ttyS4] probe required: 'at, at-vendor, at-product' ModemManager ... (Generic) [ttyS4] probe required: 'at, qcdm' + grep 'probe required' lm940-device-filter-policies-strict.log ModemManager ... (Telit) [cdc-wdm1] probe required: 'qmi' ModemManager ... (Telit) [ttyUSB5] probe required: 'at' ModemManager ... (Telit) [ttyUSB4] probe required: 'at' ModemManager ... (Telit) [ttyUSB3] probe required: 'at' ModemManager ... (Telit) [ttyUSB2] probe required: 'at' ModemManager ... (Telit) [ttyUSB1] probe required: 'at So, regarding modem recognition, everything looks fine to me. Carlo ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: RFC: New device filter policies
Hi Aleksander, > I plan to get this merged to git master sometime this week; tests and > comments welcome! I plan to have some testing done in the next couple of days Carlo ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH v4] telit-plugin: handle QSS unsolicited due to power state transitions
> Of course it looks great, I just want to have a try tomorrow on a modem
to double check it.
Double checked, all green. Thanks a lot!
On 4 September 2017 at 18:41, Carlo Lobrano wrote:
> Of course it looks great, I just want to have a try tomorrow on a modem to
> double check it.
>
> On 4 September 2017 at 18:29, Carlo Lobrano wrote:
>
>> > Using its own GTask ends up requiring the extra _ready() method, but
>> that
>> > makes the flow much more clear and consistent I think.
>>
>> yeah, I agree too
>>
>> > I've pushed your patch to the "clobrano/qss-unsolicited-power" branch
>> > in my github repo:
>> > https://github.com/aleksander0m/ModemManager/commits/
>> clobrano/qss-unsolicited-power
>> >
>> > I've fixed up several whitespace issues in your patch, and then pushed
>> > several other commits on top. Could you give them a look to make sure
>> > they're ok?
>>
>> sure, I'm going to check it, thanks for the explanation and the fixes!
>>
>>
>>
>> On 4 September 2017 at 18:09, Aleksander Morgado <
>> aleksan...@aleksander.es> wrote:
>>
>>> Hey!
>>>
>>> On Mon, Sep 4, 2017 at 8:13 AM, Carlo Lobrano
>>> wrote:
>>> > When transitioning between power-low and power-on modes, Telit modems
>>> > switch the SIM off/on, which leads to the emission of #QSS unsolicited
>>> not
>>> > related to actual SIM swaps.
>>> >
>>> > To handle this #QSS unsolicited, this patch:
>>> >
>>> > * disables reacting on #QSS unsolicited when modem_power_down is
>>> received
>>> > * implements modem_after_power_up that:
>>> > - checks whether the SIM has been changed, matching cached SIM
>>> > Identifier with the value in the current SIM. If SIM Identifier,
>>> > is different, sim hot swap ports detected is called.
>>> > - re-enables reacting on #QSS unsolicited
>>> >
>>> > ---
>>> >
>>> > Hi Aleksander,
>>> >
>>> > I should have fixed all the problems, but I'm still not sure about
>>> > base-sim's implementation of mm_base_sim_load_sim_identifier and
>>> > mm_base_sim_load_sim_identifier_finish.
>>> >
>>> > I saw that, generally, similar functions have their own ..._ready
>>> counterpart,
>>> > and maybe a GTask that takes care of caller's callback,
>>> > but I think I shouldn't need it and let the caller's callback do all
>>> the
>>> > job. However, as I said, I'm not really sure this is right.
>>> >
>>>
>>> What you did is technically right, although I always prefer to avoid
>>> putting much logic in the finish() method (as it's really duplicating
>>> logic), so I always have the async method manage its own GTask. In
>>> this way, the finish() method doesn't need to do extra checks like "do
>>> I need to do special things based on how the GTask was created?" Using
>>> its own GTask ends up requiring the extra _ready() method, but that
>>> makes the flow much more clear and consistent I think.
>>>
>>> I've pushed your patch to the "clobrano/qss-unsolicited-power" branch
>>> in my github repo:
>>> https://github.com/aleksander0m/ModemManager/commits/clobran
>>> o/qss-unsolicited-power
>>>
>>> I've fixed up several whitespace issues in your patch, and then pushed
>>> several other commits on top. Could you give them a look to make sure
>>> they're ok? The last patch does the own-GTask think discussed above.
>>>
>>>
>>> > ---
>>> > plugins/telit/mm-broadband-modem-telit.c | 181
>>> ++-
>>> > src/mm-base-sim.c| 51 +
>>> > src/mm-base-sim.h| 80 +++---
>>> > 3 files changed, 271 insertions(+), 41 deletions(-)
>>> >
>>> > diff --git a/plugins/telit/mm-broadband-modem-telit.c
>>> b/plugins/telit/mm-broadband-modem-telit.c
>>> > index a9fc5f0..b30e7c9 100644
>>> > --- a/plugins/telit/mm-broadband-modem-telit.c
>>> > +++ b/plugins/telit/mm-broadband-modem-telit.c
>>> > @@ -58,6 +58,7 @@ struct _MMBroadbandModemTelitPrivate {
>>> > MMTelitCsimLockState csim_lock_state;
>>> > GTask *csim_lock_task;
>>>
Re: [PATCH v4] telit-plugin: handle QSS unsolicited due to power state transitions
Of course it looks great, I just want to have a try tomorrow on a modem to
double check it.
On 4 September 2017 at 18:29, Carlo Lobrano wrote:
> > Using its own GTask ends up requiring the extra _ready() method, but
> that
> > makes the flow much more clear and consistent I think.
>
> yeah, I agree too
>
> > I've pushed your patch to the "clobrano/qss-unsolicited-power" branch
> > in my github repo:
> > https://github.com/aleksander0m/ModemManager/commits/clobrano/qss-
> unsolicited-power
> >
> > I've fixed up several whitespace issues in your patch, and then pushed
> > several other commits on top. Could you give them a look to make sure
> > they're ok?
>
> sure, I'm going to check it, thanks for the explanation and the fixes!
>
>
>
> On 4 September 2017 at 18:09, Aleksander Morgado > wrote:
>
>> Hey!
>>
>> On Mon, Sep 4, 2017 at 8:13 AM, Carlo Lobrano
>> wrote:
>> > When transitioning between power-low and power-on modes, Telit modems
>> > switch the SIM off/on, which leads to the emission of #QSS unsolicited
>> not
>> > related to actual SIM swaps.
>> >
>> > To handle this #QSS unsolicited, this patch:
>> >
>> > * disables reacting on #QSS unsolicited when modem_power_down is
>> received
>> > * implements modem_after_power_up that:
>> > - checks whether the SIM has been changed, matching cached SIM
>> > Identifier with the value in the current SIM. If SIM Identifier,
>> > is different, sim hot swap ports detected is called.
>> > - re-enables reacting on #QSS unsolicited
>> >
>> > ---
>> >
>> > Hi Aleksander,
>> >
>> > I should have fixed all the problems, but I'm still not sure about
>> > base-sim's implementation of mm_base_sim_load_sim_identifier and
>> > mm_base_sim_load_sim_identifier_finish.
>> >
>> > I saw that, generally, similar functions have their own ..._ready
>> counterpart,
>> > and maybe a GTask that takes care of caller's callback,
>> > but I think I shouldn't need it and let the caller's callback do all the
>> > job. However, as I said, I'm not really sure this is right.
>> >
>>
>> What you did is technically right, although I always prefer to avoid
>> putting much logic in the finish() method (as it's really duplicating
>> logic), so I always have the async method manage its own GTask. In
>> this way, the finish() method doesn't need to do extra checks like "do
>> I need to do special things based on how the GTask was created?" Using
>> its own GTask ends up requiring the extra _ready() method, but that
>> makes the flow much more clear and consistent I think.
>>
>> I've pushed your patch to the "clobrano/qss-unsolicited-power" branch
>> in my github repo:
>> https://github.com/aleksander0m/ModemManager/commits/
>> clobrano/qss-unsolicited-power
>>
>> I've fixed up several whitespace issues in your patch, and then pushed
>> several other commits on top. Could you give them a look to make sure
>> they're ok? The last patch does the own-GTask think discussed above.
>>
>>
>> > ---
>> > plugins/telit/mm-broadband-modem-telit.c | 181
>> ++-
>> > src/mm-base-sim.c| 51 +
>> > src/mm-base-sim.h| 80 +++---
>> > 3 files changed, 271 insertions(+), 41 deletions(-)
>> >
>> > diff --git a/plugins/telit/mm-broadband-modem-telit.c
>> b/plugins/telit/mm-broadband-modem-telit.c
>> > index a9fc5f0..b30e7c9 100644
>> > --- a/plugins/telit/mm-broadband-modem-telit.c
>> > +++ b/plugins/telit/mm-broadband-modem-telit.c
>> > @@ -58,6 +58,7 @@ struct _MMBroadbandModemTelitPrivate {
>> > MMTelitCsimLockState csim_lock_state;
>> > GTask *csim_lock_task;
>> > guint csim_lock_timeout_id;
>> > +gboolean parse_qss;
>> > };
>> >
>> > /***
>> **/
>> > @@ -151,10 +152,15 @@ telit_qss_unsolicited_handler (MMPortSerialAt
>> *port,
>> > }
>> >
>> > if (cur_qss_status != prev_qss_status)
>> > -mm_dbg ("QSS handler: status changed '%s -> %s",
>> > +mm_dbg ("QSS handler: status changed '%s -> %s'",
>> >
Re: [PATCH v4] telit-plugin: handle QSS unsolicited due to power state transitions
> Using its own GTask ends up requiring the extra _ready() method, but that
> makes the flow much more clear and consistent I think.
yeah, I agree too
> I've pushed your patch to the "clobrano/qss-unsolicited-power" branch
> in my github repo:
>
https://github.com/aleksander0m/ModemManager/commits/clobrano/qss-unsolicited-power
>
> I've fixed up several whitespace issues in your patch, and then pushed
> several other commits on top. Could you give them a look to make sure
> they're ok?
sure, I'm going to check it, thanks for the explanation and the fixes!
On 4 September 2017 at 18:09, Aleksander Morgado
wrote:
> Hey!
>
> On Mon, Sep 4, 2017 at 8:13 AM, Carlo Lobrano wrote:
> > When transitioning between power-low and power-on modes, Telit modems
> > switch the SIM off/on, which leads to the emission of #QSS unsolicited
> not
> > related to actual SIM swaps.
> >
> > To handle this #QSS unsolicited, this patch:
> >
> > * disables reacting on #QSS unsolicited when modem_power_down is received
> > * implements modem_after_power_up that:
> > - checks whether the SIM has been changed, matching cached SIM
> > Identifier with the value in the current SIM. If SIM Identifier,
> > is different, sim hot swap ports detected is called.
> > - re-enables reacting on #QSS unsolicited
> >
> > ---
> >
> > Hi Aleksander,
> >
> > I should have fixed all the problems, but I'm still not sure about
> > base-sim's implementation of mm_base_sim_load_sim_identifier and
> > mm_base_sim_load_sim_identifier_finish.
> >
> > I saw that, generally, similar functions have their own ..._ready
> counterpart,
> > and maybe a GTask that takes care of caller's callback,
> > but I think I shouldn't need it and let the caller's callback do all the
> > job. However, as I said, I'm not really sure this is right.
> >
>
> What you did is technically right, although I always prefer to avoid
> putting much logic in the finish() method (as it's really duplicating
> logic), so I always have the async method manage its own GTask. In
> this way, the finish() method doesn't need to do extra checks like "do
> I need to do special things based on how the GTask was created?" Using
> its own GTask ends up requiring the extra _ready() method, but that
> makes the flow much more clear and consistent I think.
>
> I've pushed your patch to the "clobrano/qss-unsolicited-power" branch
> in my github repo:
> https://github.com/aleksander0m/ModemManager/commits/clobrano/qss-
> unsolicited-power
>
> I've fixed up several whitespace issues in your patch, and then pushed
> several other commits on top. Could you give them a look to make sure
> they're ok? The last patch does the own-GTask think discussed above.
>
>
> > ---
> > plugins/telit/mm-broadband-modem-telit.c | 181
> ++-
> > src/mm-base-sim.c| 51 +
> > src/mm-base-sim.h| 80 +++---
> > 3 files changed, 271 insertions(+), 41 deletions(-)
> >
> > diff --git a/plugins/telit/mm-broadband-modem-telit.c
> b/plugins/telit/mm-broadband-modem-telit.c
> > index a9fc5f0..b30e7c9 100644
> > --- a/plugins/telit/mm-broadband-modem-telit.c
> > +++ b/plugins/telit/mm-broadband-modem-telit.c
> > @@ -58,6 +58,7 @@ struct _MMBroadbandModemTelitPrivate {
> > MMTelitCsimLockState csim_lock_state;
> > GTask *csim_lock_task;
> > guint csim_lock_timeout_id;
> > +gboolean parse_qss;
> > };
> >
> > /***
> **/
> > @@ -151,10 +152,15 @@ telit_qss_unsolicited_handler (MMPortSerialAt
> *port,
> > }
> >
> > if (cur_qss_status != prev_qss_status)
> > -mm_dbg ("QSS handler: status changed '%s -> %s",
> > +mm_dbg ("QSS handler: status changed '%s -> %s'",
> > mm_telit_qss_status_get_string (prev_qss_status),
> > mm_telit_qss_status_get_string (cur_qss_status));
> >
> > +if (self->priv->parse_qss == FALSE) {
> > +mm_dbg ("QSS: message ignored");
> > +return;
> > +}
> > +
> > if ((prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status !=
> QSS_STATUS_SIM_REMOVED) ||
> > (prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status ==
> QSS_STATUS_SIM_REMOVED)) {
> >
[PATCH v4] telit-plugin: handle QSS unsolicited due to power state transitions
When transitioning between power-low and power-on modes, Telit modems
switch the SIM off/on, which leads to the emission of #QSS unsolicited not
related to actual SIM swaps.
To handle this #QSS unsolicited, this patch:
* disables reacting on #QSS unsolicited when modem_power_down is received
* implements modem_after_power_up that:
- checks whether the SIM has been changed, matching cached SIM
Identifier with the value in the current SIM. If SIM Identifier,
is different, sim hot swap ports detected is called.
- re-enables reacting on #QSS unsolicited
---
Hi Aleksander,
I should have fixed all the problems, but I'm still not sure about
base-sim's implementation of mm_base_sim_load_sim_identifier and
mm_base_sim_load_sim_identifier_finish.
I saw that, generally, similar functions have their own ..._ready counterpart,
and maybe a GTask that takes care of caller's callback,
but I think I shouldn't need it and let the caller's callback do all the
job. However, as I said, I'm not really sure this is right.
---
plugins/telit/mm-broadband-modem-telit.c | 181 ++-
src/mm-base-sim.c| 51 +
src/mm-base-sim.h| 80 +++---
3 files changed, 271 insertions(+), 41 deletions(-)
diff --git a/plugins/telit/mm-broadband-modem-telit.c
b/plugins/telit/mm-broadband-modem-telit.c
index a9fc5f0..b30e7c9 100644
--- a/plugins/telit/mm-broadband-modem-telit.c
+++ b/plugins/telit/mm-broadband-modem-telit.c
@@ -58,6 +58,7 @@ struct _MMBroadbandModemTelitPrivate {
MMTelitCsimLockState csim_lock_state;
GTask *csim_lock_task;
guint csim_lock_timeout_id;
+gboolean parse_qss;
};
/*/
@@ -151,10 +152,15 @@ telit_qss_unsolicited_handler (MMPortSerialAt *port,
}
if (cur_qss_status != prev_qss_status)
-mm_dbg ("QSS handler: status changed '%s -> %s",
+mm_dbg ("QSS handler: status changed '%s -> %s'",
mm_telit_qss_status_get_string (prev_qss_status),
mm_telit_qss_status_get_string (cur_qss_status));
+if (self->priv->parse_qss == FALSE) {
+mm_dbg ("QSS: message ignored");
+return;
+}
+
if ((prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status !=
QSS_STATUS_SIM_REMOVED) ||
(prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status ==
QSS_STATUS_SIM_REMOVED)) {
mm_info ("QSS handler: SIM swap detected");
@@ -903,14 +909,175 @@ modem_load_unlock_retries (MMIfaceModem *self,
}
/*/
+/* Modem after power up (Modem interface) */
+typedef enum {
+AFTER_POWER_UP_STEP_FIRST,
+AFTER_POWER_UP_STEP_GET_SIM_IDENTIFIER,
+AFTER_POWER_UP_STEP_ENABLE_QSS_PARSING,
+AFTER_POWER_UP_STEP_LAST
+} ModemAfterPowerUpStep;
+
+typedef struct {
+gboolean has_sim_changed;
+guint retries;
+ModemAfterPowerUpStep step;
+} AfterPowerUpContext;
+
+static void after_power_up_step (GTask *task);
+
+static gboolean
+modem_after_power_up_finish (MMIfaceModem *self,
+ GAsyncResult *res,
+ GError **error)
+{
+return g_task_propagate_boolean (G_TASK (res), error);
+}
+
+static void
+load_sim_identifier_ready (MMBaseSim *sim,
+ GAsyncResult *res,
+ GTask *task)
+{
+AfterPowerUpContext *ctx;
+GError *error = NULL;
+gchar *current_simid;
+gchar *cached_simid;
+
+ctx = g_task_get_task_data (task);
+
+cached_simid = (gchar *)mm_gdbus_sim_get_sim_identifier (MM_GDBUS_SIM
(sim));
+current_simid = mm_base_sim_load_sim_identifier_finish (sim, res, &error);
+
+if (error) {
+if (g_error_matches (error, MM_CORE_ERROR, MM_CORE_ERROR_UNSUPPORTED))
{
+mm_warn ("could not load SIM identifier: %s", error->message);
+g_clear_error (&error);
+goto out;
+}
+
+if (ctx->retries-- > 0) {
+mm_warn ("could not load SIM identifier: %s (%d retries left)",
+ error->message, ctx->retries);
+g_clear_error (&error);
+g_timeout_add_seconds (1, (GSourceFunc) after_power_up_step, task);
+return;
+}
+
+mm_warn ("could not load SIM identifier: %s", error->message);
+g_clear_error (&error);
+goto out;
+}
+
+if (g_strcmp0 (current_simid, cached_simid) != 0) {
+mm_warn ("sim identifier has changed: possible SIM swap during power
down/low");
+ctx->has_sim_changed = TRUE;
+}
+
+out:
+g_free (current_simid);
+ctx->step++;
+after_power_up_step (task);
+}
+
+static void
+after_power_up_step (GTask *task)
+{
+AfterPowerUpContext *ctx;
+MMBroadbandModemTelit *self;
+MMBaseSim *sim = NULL;
+
+ctx = g_task_get_task_data (task);
+self
Re: [PATCH v3] telit-plugin: handle QSS unsolicited due to power state transitions
Please, ignore the previous v3 patch... I wasn't compiling the right
project and missed some errors.
On 4 September 2017 at 07:59, Carlo Lobrano wrote:
> When transitioning between power-low and power-on modes, Telit modems
> switch the SIM off/on, which leads to the emission of #QSS unsolicited not
> related to actual SIM swaps.
>
> To handle this #QSS unsolicited, this patch:
>
> * disables reacting on #QSS unsolicited when modem_power_down is received
> * implements modem_after_power_up that:
> - checks whether the SIM has been changed, matching cached SIM
> Identifier with the value in the current SIM. If SIM Identifier,
> is different, sim hot swap ports detected is called.
> - re-enables reacting on #QSS unsolicited
> ---
>
> Hi Aleksander,
>
> I should have fixed all the problems, but I'm still not sure about
> base-sim's implementation of mm_base_sim_load_sim_identifier and
> mm_base_sim_load_sim_identifier_finish.
> I saw that, generally, similar functions have their own ..._ready
> counterpart,
> and maybe a GTask that takes care of caller's callback,
> but I think I shouldn't need it and let the caller's callback do all the
> job. However, as I said, I'm not really sure this is right.
>
> ---
> plugins/telit/mm-broadband-modem-telit.c | 181
> ++-
> src/mm-base-sim.c| 51 +
> src/mm-base-sim.h| 80 +++---
> 3 files changed, 271 insertions(+), 41 deletions(-)
>
> diff --git a/plugins/telit/mm-broadband-modem-telit.c
> b/plugins/telit/mm-broadband-modem-telit.c
> index a9fc5f0..ac2c620 100644
> --- a/plugins/telit/mm-broadband-modem-telit.c
> +++ b/plugins/telit/mm-broadband-modem-telit.c
> @@ -58,6 +58,7 @@ struct _MMBroadbandModemTelitPrivate {
> MMTelitCsimLockState csim_lock_state;
> GTask *csim_lock_task;
> guint csim_lock_timeout_id;
> +gboolean parse_qss;
> };
>
> /***
> **/
> @@ -151,10 +152,15 @@ telit_qss_unsolicited_handler (MMPortSerialAt *port,
> }
>
> if (cur_qss_status != prev_qss_status)
> -mm_dbg ("QSS handler: status changed '%s -> %s",
> +mm_dbg ("QSS handler: status changed '%s -> %s'",
> mm_telit_qss_status_get_string (prev_qss_status),
> mm_telit_qss_status_get_string (cur_qss_status));
>
> +if (self->priv->parse_qss == FALSE) {
> +mm_dbg ("QSS: message ignored");
> +return;
> +}
> +
> if ((prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status !=
> QSS_STATUS_SIM_REMOVED) ||
> (prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status ==
> QSS_STATUS_SIM_REMOVED)) {
> mm_info ("QSS handler: SIM swap detected");
> @@ -903,14 +909,175 @@ modem_load_unlock_retries (MMIfaceModem *self,
> }
>
> /***
> **/
> +/* Modem after power up (Modem interface) */
> +typedef enum {
> +AFTER_POWER_UP_STEP_FIRST,
> +AFTER_POWER_UP_STEP_GET_SIM_IDENTIFIER,
> +AFTER_POWER_UP_STEP_ENABLE_QSS_PARSING,
> +AFTER_POWER_UP_STEP_LAST
> +} ModemAfterPowerUpStep;
> +
> +typedef struct {
> +gboolean has_sim_changed;
> +guint retries;
> +ModemAfterPowerUpStep step;
> +} AfterPowerUpContext;
> +
> +static void after_power_up_step (GTask *task);
> +
> +static gboolean
> +modem_after_power_up_finish (MMIfaceModem *self,
> + GAsyncResult *res,
> + GError **error)
> +{
> +return g_task_propagate_boolean (G_TASK (res), error);
> +}
> +
> +static void
> +load_sim_identifier_ready (MMBaseSim *sim,
> + GAsyncResult *res,
> + GTask *task)
> +{
> +AfterPowerUpContext *ctx;
> +GError *error = NULL;
> +gchar *current_simid;
> +gchar *cached_simid;
> +
> +ctx = g_task_get_task_data (task);
> +
> +cached_simid = (gchar *)mm_gdbus_sim_get_sim_identifier
> (MM_GDBUS_SIM (sim));
> +current_simid = mm_base_sim_load_sim_identifier_finish (sim, res,
> &error);
> +
> +if (error) {
> +if (g_error_matches (error, MM_CORE_ERROR,
> MM_CORE_ERROR_UNSUPPORTED)) {
> +mm_warn ("could not load SIM identifier: %s", error->message);
> +g_clear_error (error);
> +goto out;
> +}
> +
> +if (ctx->retries-- > 0) {
> +
[PATCH v3] telit-plugin: handle QSS unsolicited due to power state transitions
When transitioning between power-low and power-on modes, Telit modems
switch the SIM off/on, which leads to the emission of #QSS unsolicited not
related to actual SIM swaps.
To handle this #QSS unsolicited, this patch:
* disables reacting on #QSS unsolicited when modem_power_down is received
* implements modem_after_power_up that:
- checks whether the SIM has been changed, matching cached SIM
Identifier with the value in the current SIM. If SIM Identifier,
is different, sim hot swap ports detected is called.
- re-enables reacting on #QSS unsolicited
---
Hi Aleksander,
I should have fixed all the problems, but I'm still not sure about
base-sim's implementation of mm_base_sim_load_sim_identifier and
mm_base_sim_load_sim_identifier_finish.
I saw that, generally, similar functions have their own ..._ready counterpart,
and maybe a GTask that takes care of caller's callback,
but I think I shouldn't need it and let the caller's callback do all the
job. However, as I said, I'm not really sure this is right.
---
plugins/telit/mm-broadband-modem-telit.c | 181 ++-
src/mm-base-sim.c| 51 +
src/mm-base-sim.h| 80 +++---
3 files changed, 271 insertions(+), 41 deletions(-)
diff --git a/plugins/telit/mm-broadband-modem-telit.c
b/plugins/telit/mm-broadband-modem-telit.c
index a9fc5f0..ac2c620 100644
--- a/plugins/telit/mm-broadband-modem-telit.c
+++ b/plugins/telit/mm-broadband-modem-telit.c
@@ -58,6 +58,7 @@ struct _MMBroadbandModemTelitPrivate {
MMTelitCsimLockState csim_lock_state;
GTask *csim_lock_task;
guint csim_lock_timeout_id;
+gboolean parse_qss;
};
/*/
@@ -151,10 +152,15 @@ telit_qss_unsolicited_handler (MMPortSerialAt *port,
}
if (cur_qss_status != prev_qss_status)
-mm_dbg ("QSS handler: status changed '%s -> %s",
+mm_dbg ("QSS handler: status changed '%s -> %s'",
mm_telit_qss_status_get_string (prev_qss_status),
mm_telit_qss_status_get_string (cur_qss_status));
+if (self->priv->parse_qss == FALSE) {
+mm_dbg ("QSS: message ignored");
+return;
+}
+
if ((prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status !=
QSS_STATUS_SIM_REMOVED) ||
(prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status ==
QSS_STATUS_SIM_REMOVED)) {
mm_info ("QSS handler: SIM swap detected");
@@ -903,14 +909,175 @@ modem_load_unlock_retries (MMIfaceModem *self,
}
/*/
+/* Modem after power up (Modem interface) */
+typedef enum {
+AFTER_POWER_UP_STEP_FIRST,
+AFTER_POWER_UP_STEP_GET_SIM_IDENTIFIER,
+AFTER_POWER_UP_STEP_ENABLE_QSS_PARSING,
+AFTER_POWER_UP_STEP_LAST
+} ModemAfterPowerUpStep;
+
+typedef struct {
+gboolean has_sim_changed;
+guint retries;
+ModemAfterPowerUpStep step;
+} AfterPowerUpContext;
+
+static void after_power_up_step (GTask *task);
+
+static gboolean
+modem_after_power_up_finish (MMIfaceModem *self,
+ GAsyncResult *res,
+ GError **error)
+{
+return g_task_propagate_boolean (G_TASK (res), error);
+}
+
+static void
+load_sim_identifier_ready (MMBaseSim *sim,
+ GAsyncResult *res,
+ GTask *task)
+{
+AfterPowerUpContext *ctx;
+GError *error = NULL;
+gchar *current_simid;
+gchar *cached_simid;
+
+ctx = g_task_get_task_data (task);
+
+cached_simid = (gchar *)mm_gdbus_sim_get_sim_identifier (MM_GDBUS_SIM
(sim));
+current_simid = mm_base_sim_load_sim_identifier_finish (sim, res, &error);
+
+if (error) {
+if (g_error_matches (error, MM_CORE_ERROR, MM_CORE_ERROR_UNSUPPORTED))
{
+mm_warn ("could not load SIM identifier: %s", error->message);
+g_clear_error (error);
+goto out;
+}
+
+if (ctx->retries-- > 0) {
+mm_warn ("could not load SIM identifier: %s (%d retries left)",
+ error->message, ctx->retries);
+g_clear_error (&error);
+g_timeout_add_seconds (1, (GSourceFunc) after_power_up_step, task);
+return;
+}
+
+mm_warn ("could not load SIM identifier: %s", error->message);
+g_clear_error (&error);
+goto out;
+}
+
+if (g_strcmp0 (current_simid, cached_simid) != 0) {
+mm_warn ("sim identifier has changed: possible SIM swap during power
down/low");
+ctx->has_sim_changed = TRUE;
+}
+
+out:
+g_free (current_simid);
+ctx->step++;
+after_power_up_step (task);
+}
+
+static void
+after_power_up_step (GTask *task)
+{
+AfterPowerUpContext *ctx;
+MMBroadbandModemTelit *self;
+MMBaseSim *sim = NULL;
+
+ctx = g_task_get_task_data (task);
+self = g
[PATCH v2] telit-plugin: handle QSS unsolicited due to power state transitions
When transitioning between power-low and power-on modes, Telit modems
switch the SIM off/on, which leads to the emission of #QSS unsolicited not
related to an actual SIM swap.
To handle this use case, this patch:
* disables reacting on #QSS unsolicited when modem_power_down is received
* implements modem_after_power_up that:
- checks whether the SIM has been changed, matching cached SIM Identifier
with the value in the current SIM. If SIM Identifier, is different,
sim_hot_swap_ports_detected is called.
- re-enables reacting on #QSS unsolicited
---
Hi Aleksander,
here is my second patch version for the problem.
Not 100% sure this is what you expected, please let me know what do you think.
Thanks,
Carlo
---
plugins/telit/mm-broadband-modem-telit.c | 200 ++-
1 file changed, 196 insertions(+), 4 deletions(-)
diff --git a/plugins/telit/mm-broadband-modem-telit.c
b/plugins/telit/mm-broadband-modem-telit.c
index a9fc5f0..2f3dcca 100644
--- a/plugins/telit/mm-broadband-modem-telit.c
+++ b/plugins/telit/mm-broadband-modem-telit.c
@@ -58,6 +58,7 @@ struct _MMBroadbandModemTelitPrivate {
MMTelitCsimLockState csim_lock_state;
GTask *csim_lock_task;
guint csim_lock_timeout_id;
+gboolean parse_qss;
};
/*/
@@ -151,10 +152,15 @@ telit_qss_unsolicited_handler (MMPortSerialAt *port,
}
if (cur_qss_status != prev_qss_status)
-mm_dbg ("QSS handler: status changed '%s -> %s",
+mm_dbg ("QSS handler: status changed '%s -> %s'",
mm_telit_qss_status_get_string (prev_qss_status),
mm_telit_qss_status_get_string (cur_qss_status));
+if (self->priv->parse_qss == FALSE) {
+mm_dbg ("QSS: ignore QSS is enabled");
+return;
+}
+
if ((prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status !=
QSS_STATUS_SIM_REMOVED) ||
(prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status ==
QSS_STATUS_SIM_REMOVED)) {
mm_info ("QSS handler: SIM swap detected");
@@ -903,14 +909,193 @@ modem_load_unlock_retries (MMIfaceModem *self,
}
/*/
+/* Modem power up (Modem interface) */
+static void
+modem_power_up (MMIfaceModem *self,
+GAsyncReadyCallback callback,
+gpointer user_data)
+{
+mm_base_modem_at_command (MM_BASE_MODEM (self),
+ "+CFUN=1",
+ 5,
+ FALSE,
+ (GAsyncReadyCallback) callback,
+ user_data);
+}
+
+/*/
+/* Modem after power up (Modem interface) */
+typedef enum {
+AFTER_POWER_UP_STEP_FIRST,
+AFTER_POWER_UP_STEP_GET_SIM_IDENTIFIER,
+AFTER_POWER_UP_STEP_ENABLE_QSS_PARSING,
+AFTER_POWER_UP_STEP_LAST
+} ModemAfterPowerUpStep;
+
+typedef struct {
+MMBroadbandModemTelit *self;
+gboolean has_sim_changed;
+guint retries;
+ModemAfterPowerUpStep step;
+GError *error;
+} AfterPowerUpContext;
+
+static void after_power_up_step (GTask *task);
+
+static void
+after_power_up_context_free (AfterPowerUpContext *ctx)
+{
+g_clear_object (&(ctx->error));
+g_object_unref (ctx->self);
+g_slice_free (AfterPowerUpContext, ctx);
+}
+
+static void
+load_sim_identifier_ready (MMBaseSim *self,
+ GAsyncResult *res,
+ GTask *task)
+{
+AfterPowerUpContext *ctx;
+GError *error = NULL;
+gchar *current_simid;
+gchar *cached_simid;
+
+ctx = g_task_get_task_data (task);
+
+cached_simid = (gchar *)mm_gdbus_sim_get_sim_identifier (MM_GDBUS_SIM
(self));
+current_simid = MM_BASE_SIM_GET_CLASS (self)->load_sim_identifier_finish
(self, res, &error);
+if (error) {
+if (ctx->retries > 0) {
+ctx->retries--;
+mm_warn ("Couldn't read SIM identifier: '%s', %d retries left",
error->message, ctx->retries);
+
+g_clear_error (&error);
+g_timeout_add_seconds (1, (GSourceFunc) after_power_up_step, task);
+return;
+}
+
+mm_warn ("Couldn't read SIM identifier: '%s'", error->message);
+g_clear_error (&error);
+goto out;
+}
+
+if (g_strcmp0 (current_simid, cached_simid) != 0) {
+mm_warn ("sim identifier has changed, possible SIM swap during power
down/low");
+ctx->has_sim_changed = TRUE;
+}
+
+out:
+g_free (current_simid);
+g_object_unref (self);
+ctx->step++;
+after_power_up_step (task);
+}
+
+static void
+after_power_up_step (GTask *task)
+{
+AfterPowerUpContext *ctx;
+MMBaseSim *sim;
+
+ctx = g_task_get_task_data (task);
+
+g_object_get (MM_BROADBAND_MODEM_TELIT (ctx->self),
+
Re: [PATCH v2] broadband-modem: do not release SIM swap port contexts on disable
> Pushed to git master after fixing up the change in the file mode line,
> which I assume wasn't on purpose :)
Indeed, I don't even know how I made it :D
Thanks
On 29 August 2017 at 11:20, Aleksander Morgado
wrote:
> On Tue, Aug 29, 2017 at 9:15 AM, Carlo Lobrano
> wrote:
> > Currently when the modem is disabled, it releases SIM hot swap ports
> context,
> > and is not able to receive any notification about the SIM status.
> >
> > This patch keeps these ports opened when Modem is disabled and released
> them
> > only when SIM swap is detected or when modem is released.
> > ---
> >
> > If I understood correctly, the only change was to be done in broadband
> modem's finalize.
> > I also looked at enabled_ports_ctx as reference and it seems there are
> not other places to
> > relese the port contexts.
> >
>
> Pushed to git master after fixing up the change in the file mode line,
> which I assume wasn't on purpose :)
>
> Thanks!
>
> > ---
> >
> > src/mm-broadband-modem.c | 10 --
> > 1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
> > index 552bdc5..b298721 100644
> > --- a/src/mm-broadband-modem.c
> > +++ b/src/mm-broadband-modem.c
> > @@ -1,5 +1,4 @@
> > -/* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4
> -*- */
> > -/*
> > +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4
> -*- *//*
> > * This program is free software; you can redistribute it and/or modify
> > * it under the terms of the GNU General Public License as published by
> > * the Free Software Foundation; either version 2 of the License, or
> > @@ -8640,10 +8639,6 @@ disabling_stopped (MMBroadbandModem *self,
> > self->priv->enabled_ports_ctx = NULL;
> > }
> >
> > -if (self->priv->sim_hot_swap_ports_ctx) {
> > -ports_context_unref (self->priv->sim_hot_swap_ports_ctx);
> > -self->priv->sim_hot_swap_ports_ctx = NULL;
> > -}
> > return TRUE;
> > }
> >
> > @@ -10661,6 +10656,9 @@ finalize (GObject *object)
> > if (self->priv->enabled_ports_ctx)
> > ports_context_unref (self->priv->enabled_ports_ctx);
> >
> > +if (self->priv->sim_hot_swap_ports_ctx)
> > +ports_context_unref (self->priv->sim_hot_swap_ports_ctx);
> > +
> > if (self->priv->modem_3gpp_registration_regex)
> > mm_3gpp_creg_regex_destroy (self->priv->modem_3gpp_
> registration_regex);
> >
> > --
> > 2.9.3
> >
> > ___
> > ModemManager-devel mailing list
> > ModemManager-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
>
>
>
> --
> Aleksander
> https://aleksander.es
>
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
[PATCH v2] broadband-modem: do not release SIM swap port contexts on disable
Currently when the modem is disabled, it releases SIM hot swap ports context,
and is not able to receive any notification about the SIM status.
This patch keeps these ports opened when Modem is disabled and released them
only when SIM swap is detected or when modem is released.
---
If I understood correctly, the only change was to be done in broadband modem's
finalize.
I also looked at enabled_ports_ctx as reference and it seems there are not
other places to
relese the port contexts.
---
src/mm-broadband-modem.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
index 552bdc5..b298721 100644
--- a/src/mm-broadband-modem.c
+++ b/src/mm-broadband-modem.c
@@ -1,5 +1,4 @@
-/* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
-/*
+/* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- *//*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
@@ -8640,10 +8639,6 @@ disabling_stopped (MMBroadbandModem *self,
self->priv->enabled_ports_ctx = NULL;
}
-if (self->priv->sim_hot_swap_ports_ctx) {
-ports_context_unref (self->priv->sim_hot_swap_ports_ctx);
-self->priv->sim_hot_swap_ports_ctx = NULL;
-}
return TRUE;
}
@@ -10661,6 +10656,9 @@ finalize (GObject *object)
if (self->priv->enabled_ports_ctx)
ports_context_unref (self->priv->enabled_ports_ctx);
+if (self->priv->sim_hot_swap_ports_ctx)
+ports_context_unref (self->priv->sim_hot_swap_ports_ctx);
+
if (self->priv->modem_3gpp_registration_regex)
mm_3gpp_creg_regex_destroy (self->priv->modem_3gpp_registration_regex);
--
2.9.3
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH] broadband-modem: Do not release SIM swap port contexts on disable
> If removed from here (which I agree we should) we also need to clear the
context when the modem goes away, e.g. in finalize(), or we would be
leaking the context and the port references. Could you also add that in the
patch?
I've missed that, sure I'll do it
On 29 August 2017 at 07:34, Aleksander Morgado
wrote:
> On 28/08/17 14:18, Carlo Lobrano wrote:
> > Currently when the modem is disabled, it also releases SIM hot swap
> ports context,
> > and it is not able to receive any notification about the SIM status
> anymore.
> >
> > This patch keeps these ports opened when Modem is disabled and released
> them
> > only when SIM swap is detected.
> >
> > ---
> >
> > Hi Aleksander,
> >
> > as you might remember we talked about this issue while discussing
> "Handling QSS unsolicited in power state transitions" (it's the email's
> subject).
> > I'm back working on this topic and saw that this part hasn't been
> submitted yet.
> >
>
>
> If removed from here (which I agree we should) we also need to clear the
> context when the modem goes away, e.g. in finalize(), or we would be
> leaking the context and the port references. Could you also add that in the
> patch?
>
> > ---
> >
> > src/mm-broadband-modem.c | 4
> > 1 file changed, 4 deletions(-)
> >
> > diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
> > index 552bdc5..b482705 100644
> > --- a/src/mm-broadband-modem.c
> > +++ b/src/mm-broadband-modem.c
> > @@ -8640,10 +8640,6 @@ disabling_stopped (MMBroadbandModem *self,
> > self->priv->enabled_ports_ctx = NULL;
> > }
> >
> > -if (self->priv->sim_hot_swap_ports_ctx) {
> > -ports_context_unref (self->priv->sim_hot_swap_ports_ctx);
> > -self->priv->sim_hot_swap_ports_ctx = NULL;
> > -}
> > return TRUE;
> > }
> >
> >
>
>
> --
> Aleksander
> https://aleksander.es
>
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
[PATCH] broadband-modem: Do not release SIM swap port contexts on disable
Currently when the modem is disabled, it also releases SIM hot swap ports
context,
and it is not able to receive any notification about the SIM status anymore.
This patch keeps these ports opened when Modem is disabled and released them
only when SIM swap is detected.
---
Hi Aleksander,
as you might remember we talked about this issue while discussing "Handling QSS
unsolicited in power state transitions" (it's the email's subject).
I'm back working on this topic and saw that this part hasn't been submitted yet.
---
src/mm-broadband-modem.c | 4
1 file changed, 4 deletions(-)
diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
index 552bdc5..b482705 100644
--- a/src/mm-broadband-modem.c
+++ b/src/mm-broadband-modem.c
@@ -8640,10 +8640,6 @@ disabling_stopped (MMBroadbandModem *self,
self->priv->enabled_ports_ctx = NULL;
}
-if (self->priv->sim_hot_swap_ports_ctx) {
-ports_context_unref (self->priv->sim_hot_swap_ports_ctx);
-self->priv->sim_hot_swap_ports_ctx = NULL;
-}
return TRUE;
}
--
2.9.3
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: Handling QSS unsolicited in power state transitions
OK, I'll need some time to understand at which level make the changes, but I think it's fine On 2 August 2017 at 19:19, Aleksander Morgado wrote: > >> Note that this issue is again unrelated to the actual hot sim swapping > >> logic implemented; but now that we're starting to support hot sim > >> swapping, we should probably also check that usecase. At the end Telit > >> isn't the only module that may power off SIM Card during low power > >> mode. > >> So, how about a manual check to validate whether the SIM is different > >> or not?, e.g comparing whether the old property values we got before > >> the low power mode are still the same ones now after the power up... > > > > I understand, but I think that it is not strictly related to power low as > > well and it should not be taken into account there. Maybe a SIM power OFF > > mode? I do not know if this make any sense, but I see it more logic. > > > > Ideally, the modem should notify that is going to shut the SIM OFF, then > the > > Core can manage it, e.g. checking if something changed as you said, when > the > > SIM will be swiched ON again. > > > > Don't think we can rely on all modems reporting that the SIM is going > off or on, that is why i was thinking on a more generic solution based > on comparing SIM details, it's probably a good fallback. > > -- > Aleksander > https://aleksander.es > ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: Handling QSS unsolicited in power state transitions
> This is getting a bit out of hands :) I felt the same :) > So, yes, if the module powers off the SIM slot during CFUN=4 we won't > get #QSS events, that's understandable. But... what would happen if we > go to low power mode and switch the SIM card with a different one? We > would be gettting #QSS=1 later on after CFUN=1 and we would assume > nothing had happened, and that would be wrong as the SIM is different. We would be getting it in any case, even keeping the same SIM, so it does not give us any valuable information and can be ignored as well as the #QSS=0. > Note that this issue is again unrelated to the actual hot sim swapping > logic implemented; but now that we're starting to support hot sim > swapping, we should probably also check that usecase. At the end Telit > isn't the only module that may power off SIM Card during low power > mode. > So, how about a manual check to validate whether the SIM is different > or not?, e.g comparing whether the old property values we got before > the low power mode are still the same ones now after the power up... I understand, but I think that it is not strictly related to power low as well and it should not be taken into account there. Maybe a SIM power OFF mode? I do not know if this make any sense, but I see it more logic. Ideally, the modem should notify that is going to shut the SIM OFF, then the Core can manage it, e.g. checking if something changed as you said, when the SIM will be swiched ON again. ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH] sim hot swap: improved error management
> Pushed to git master, thanks.
Thanks!
> I suppose there is a bit of a race in the code, because:
>
> First the current SIM insertion state is checked (with AT#QSS?), and
> stored, and afterwards unsolicited notifications are enabled "AT#QSS=1".
>
> If a SIM eject happens between the two, then the SIM removed event
> wouldn't be caught, and the internal stored state would be incorrect -
> the fix would be to issue and parse the AT#QSS? command after the
> AT#QSS=1 command. This could also serve to verify that the modem did
> enable hot swap notifiation. In practise this is highly unlikely, but
> just thought I'd mention it.
and thanks Tim, you're right, I'll work on this as well
On 1 August 2017 at 10:34, Aleksander Morgado
wrote:
> On 25/07/17 09:03, Carlo Lobrano wrote:
> > Currently, when SIM hot swap fails in either mm-iface or plugin, the
> > ModemManager still opens ports context and prints a message saying that
> > SIM hot swap is supported and that it's waiting for SIM insertion,
> > instead of clearly saying that SIM hot swap is not working.
> >
> > This patch:
> >
> > 1. introduces a new property MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED
> >which is FALSE by default and set to TRUE only when
> >setup_sim_hot_swap_finish() succeded.
> > 2. subordinates the completion of SIM hot swap setup (in
> >mm-broadband-modem) and the related messages to the the value of
> >MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED
> >
> > Finally, this patch replaces the MBIM's sim_hot_swap_on private property
> > with the new property MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED, since
> they have the
> > same meaning.
> > ---
> >
> > Please ignore the previous patch, since it was rebased on the wrong
> branch.
> > This one is rebased on master branch at commit
> > aa0a6bed363419659101084b3861a51144eee859
> >
>
> Pushed to git master, thanks.
>
> > ---
> > plugins/telit/mm-broadband-modem-telit.c | 1 +
> > src/mm-broadband-modem-mbim.c| 21 ---
> > src/mm-broadband-modem.c | 94
> ++--
> > src/mm-iface-modem.c | 14 -
> > src/mm-iface-modem.h | 1 +
> > 5 files changed, 93 insertions(+), 38 deletions(-)
> >
> > diff --git a/plugins/telit/mm-broadband-modem-telit.c
> b/plugins/telit/mm-broadband-modem-telit.c
> > index e7650c0..edd9093 100644
> > --- a/plugins/telit/mm-broadband-modem-telit.c
> > +++ b/plugins/telit/mm-broadband-modem-telit.c
> > @@ -1377,6 +1377,7 @@ mm_broadband_modem_telit_new (const gchar *device,
> > MM_BASE_MODEM_VENDOR_ID, vendor_id,
> > MM_BASE_MODEM_PRODUCT_ID, product_id,
> > MM_IFACE_MODEM_SIM_HOT_SWAP_SUPPORTED, TRUE,
> > + MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED, FALSE,
> > NULL);
> > }
> >
> > diff --git a/src/mm-broadband-modem-mbim.c
> b/src/mm-broadband-modem-mbim.c
> > index c69893f..7f5d2fc 100644
> > --- a/src/mm-broadband-modem-mbim.c
> > +++ b/src/mm-broadband-modem-mbim.c
> > @@ -89,8 +89,6 @@ struct _MMBroadbandModemMbimPrivate {
> > MbimDataClass available_data_classes;
> > MbimDataClass highest_available_data_class;
> >
> > -/* For checking whether the SIM has been hot swapped */
> > -gboolean sim_hot_swap_on;
> > MbimSubscriberReadyState last_ready_state;
> > };
> >
> > @@ -1990,6 +1988,7 @@ basic_connect_notification_subscriber_ready_status
> (MMBroadbandModemMbim *self,
> > (self->priv->last_ready_state ==
> > MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED
> &&
> > ready_state !=
> > MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED))
> {
> > /* SIM has been removed or reinserted, re-probe to ensure
> correct interfaces are exposed */
> > +mm_dbg ("SIM hot swap detected");
> > mm_broadband_modem_update_sim_hot_swap_detected
> (MM_BROADBAND_MODEM (self));
> > }
> >
> > @@ -2286,10 +2285,15 @@ cleanup_unsolicited_events_3gpp
> (MMIfaceModem3gpp *_self,
> > gpointer user_data)
> > {
> > MMBroadbandModemMbim *self = MM_BROADBAND_MODEM_MBIM (_self);
> > +gboolean is_sim_hot_swap_configured = FALSE;
> > +
> > +g_object_get (self,
> > + MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED,
> &is_sim_hot_swap_configured,
>
Re: Handling QSS unsolicited in power state transitions
It seems that sometimes the decision to send the #QSS=0 is taken with some delay (with HE910) and in these cases it arrives quite late (10-13 seconds) and disabling the notifications and then re-enabling them later does not avoid it. Considered this, I would go with ignoring #QSS unsolicited between +CFUN=4 and +CFUN=1. The vast majority of telit modems switches the SIM off so this unsolicited is expected (actually, only the latest LE910 V2 FW do not do that) and, of course, it's the only one we will receive 'till +CFUN=1. Moreover ignoring QSS requires a lot less code than disabling/enabling its emission. On Tue, 1 Aug 2017 at 13:14 Carlo Lobrano wrote: > Yes, I agree. That's why it's on a different patch > > On Tue, 1 Aug 2017, 13:12 Aleksander Morgado, > wrote: > >> On Tue, Aug 1, 2017 at 12:31 PM, Carlo Lobrano >> wrote: >> >> Aren't the sim hot swap unsolicited messages received always, >> regardless >> >> of whether the modem is enabled or disabled? >> > >> > disabling_stopped function releases both ports contexts, so we don't >> receive >> > unsolicited on SIM swap ports. >> > >> >> Wait, I don't think that's ok. The SIM swap ports context is allocated >> during initialization, and should exist until the object is destroyed. >> We shouldn't be removing this context when the disable() happens, >> otherwise we wouldn't get SIM hot swap events when the modem is >> disabled. >> >> -- >> Aleksander >> https://aleksander.es >> > ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: Handling QSS unsolicited in power state transitions
Yes, I agree. That's why it's on a different patch On Tue, 1 Aug 2017, 13:12 Aleksander Morgado, wrote: > On Tue, Aug 1, 2017 at 12:31 PM, Carlo Lobrano > wrote: > >> Aren't the sim hot swap unsolicited messages received always, regardless > >> of whether the modem is enabled or disabled? > > > > disabling_stopped function releases both ports contexts, so we don't > receive > > unsolicited on SIM swap ports. > > > > Wait, I don't think that's ok. The SIM swap ports context is allocated > during initialization, and should exist until the object is destroyed. > We shouldn't be removing this context when the disable() happens, > otherwise we wouldn't get SIM hot swap events when the modem is > disabled. > > -- > Aleksander > https://aleksander.es > ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: Handling QSS unsolicited in power state transitions
> Aren't the sim hot swap unsolicited messages received always, regardless of whether the modem is enabled or disabled? disabling_stopped function releases both ports contexts, so we don't receive unsolicited on SIM swap ports. > The only issue is when getting #QSS=0 after a +CFUN=4, right? Because we would be getting a SIM removal event that isn't really such thing, it was just the SIM card being powered off as well. Is there maybe a command to get into low-power mode that shuts down RF but leaves SIM card powered? This is correct, but we don't have commands that disable RF and leaves SIM card powered. > A wild idea, but would it be possible to wrap around the +CFUN calls between #QSS indication disabled and enabled? > E.g. > Disable #QSS indications (not just ignore them in MM, but explicitly tell the modem to not send them) > +CFUN=4 > Enable #QSS indications right away > (maybe the #QSS won't arrive here as it was disabled during CFUN?) > > Would that make sense? Or will the #QSS be sent anyway, even if it was disabled during the +CFUN being run? That's actually a good idea to test. I've thought about disabling on CFUN=4 and re-enabling on CFUN=1, but not around the same command. On 1 August 2017 at 10:32, Aleksander Morgado wrote: > > On 01/08/17 10:04, Carlo Lobrano wrote: > > last week I was working to handle a behavior that most of the Telit modem > > have, that is, they emit *#QSS* unsolicited in consequence of *+CFUN* > > commands. Most of the times, the modems are already in *+CFUN=1* at the > > start so we did not notice it in a normal power up, but if we put the > modem > > in power low state, the *+CFUN=4* triggers a *#QSS=0* that the > ModemManager > > considers as a SIM swap. Note that this is receved as soon as the modem > is > > enabled again, because currently we release SIM swap ports while > disabling > > the modem, and, if we set power state on, ***together with the #QSS=1 due > > to +CFUN=1*** causing a crash. > > > > Aren't the sim hot swap unsolicited messages received always, regardless > of whether the modem is enabled or disabled? > > > Generally speaking this behavior isn't wrong per se (a part from the > crash, > > that can be fixed), because the SIM is actually turned off, but in this > > configuration we cannot re-enable the modem, while sending a *+CFUN=1* > the > > modem would be usable again. > > > > So, I made a couple of patches, (1) avoid releasing SIM swap ports on > modem > > disable, so we can still receive SIM notifications at the right moment, > and > > (2) ignore QSS unsolicited when changing *+CFUN* state and only for a > short > > amount of time. The problem is that there isn't a specific timeout within > > which this unsolicited must arrive and according to my tests it is quite > > variable (about between 3 and 10 seconds). > > > > The only issue is when getting #QSS=0 after a +CFUN=4, right? Because we > would be getting a SIM removal event that isn't really such thing, it was > just the SIM card being powered off as well. Is there maybe a command to > get into low-power mode that shuts down RF but leaves SIM card powered? > > > One solution is then to ignore *#QSS* for like 10 seconds, while the > other > > one is to keep ignoring them in the whole time between *+CFUN=4* and* > > +CFUN=1*. This last solution is more consistent, but it won't let us get > > SIM removal/insertion when the modem is in power-state-low. > > A wild idea, but would it be possible to wrap around the +CFUN calls > between #QSS indication disabled and enabled? > E.g. > Disable #QSS indications (not just ignore them in MM, but explicitly > tell the modem to not send them) > +CFUN=4 > Enable #QSS indications right away > (maybe the #QSS won't arrive here as it was disabled during CFUN?) > > Would that make sense? Or will the #QSS be sent anyway, even if it was > disabled during the +CFUN being run? > > -- > Aleksander > https://aleksander.es > ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Handling QSS unsolicited in power state transitions
Hi all, last week I was working to handle a behavior that most of the Telit modem have, that is, they emit *#QSS* unsolicited in consequence of *+CFUN* commands. Most of the times, the modems are already in *+CFUN=1* at the start so we did not notice it in a normal power up, but if we put the modem in power low state, the *+CFUN=4* triggers a *#QSS=0* that the ModemManager considers as a SIM swap. Note that this is receved as soon as the modem is enabled again, because currently we release SIM swap ports while disabling the modem, and, if we set power state on, ***together with the #QSS=1 due to +CFUN=1*** causing a crash. Generally speaking this behavior isn't wrong per se (a part from the crash, that can be fixed), because the SIM is actually turned off, but in this configuration we cannot re-enable the modem, while sending a *+CFUN=1* the modem would be usable again. So, I made a couple of patches, (1) avoid releasing SIM swap ports on modem disable, so we can still receive SIM notifications at the right moment, and (2) ignore QSS unsolicited when changing *+CFUN* state and only for a short amount of time. The problem is that there isn't a specific timeout within which this unsolicited must arrive and according to my tests it is quite variable (about between 3 and 10 seconds). One solution is then to ignore *#QSS* for like 10 seconds, while the other one is to keep ignoring them in the whole time between *+CFUN=4* and* +CFUN=1*. This last solution is more consistent, but it won't let us get SIM removal/insertion when the modem is in power-state-low. I'd like to know your opinion about this. Thank you! Carlo ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
[PATCH v3] telit-plugin: ignore QSS when SIM-ME interface is locked
With some modems, the lock/unlock of the SIM-ME interface with +CSIM=1/0
command is followed by #QSS unsolicited messages. With the current
implementation, this messages are mistaken for SIM swap events and so the
modem is first dropped and then re-probed.
With this patch, the plugin takes into account the SIM-ME lock state when
parsing #QSS unsolicited, so that the QSS handler can correctly
elaborate the messages that are not related to SIM swap events.
---
I spotted an error in version 2: csim_lock_task was saved in priv only if CSIM
was locked
---
plugins/telit/mm-broadband-modem-telit.c | 85
plugins/telit/mm-modem-helpers-telit.h | 9
2 files changed, 85 insertions(+), 9 deletions(-)
diff --git a/plugins/telit/mm-broadband-modem-telit.c
b/plugins/telit/mm-broadband-modem-telit.c
index e7650c0..2c6f7a9 100644
--- a/plugins/telit/mm-broadband-modem-telit.c
+++ b/plugins/telit/mm-broadband-modem-telit.c
@@ -44,6 +44,8 @@ G_DEFINE_TYPE_EXTENDED (MMBroadbandModemTelit,
mm_broadband_modem_telit, MM_TYPE
G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM,
iface_modem_init)
G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM_3GPP,
iface_modem_3gpp_init));
+#define CSIM_UNLOCK_MAX_TIMEOUT 3
+
typedef enum {
FEATURE_SUPPORT_UNKNOWN,
FEATURE_NOT_SUPPORTED,
@@ -53,6 +55,9 @@ typedef enum {
struct _MMBroadbandModemTelitPrivate {
FeatureSupport csim_lock_support;
MMTelitQssStatus qss_status;
+MMTelitCsimLockState csim_lock_state;
+GTask *csim_lock_task;
+guint csim_lock_timeout_id;
};
/*/
@@ -107,6 +112,7 @@ typedef struct {
} QssSetupContext;
static void qss_setup_step (GTask *task);
+static void pending_csim_unlock_complete (MMBroadbandModemTelit *self);
static void
telit_qss_unsolicited_handler (MMPortSerialAt *port,
@@ -122,14 +128,36 @@ telit_qss_unsolicited_handler (MMPortSerialAt *port,
prev_qss_status = self->priv->qss_status;
self->priv->qss_status = cur_qss_status;
+if (self->priv->csim_lock_state >= CSIM_LOCK_STATE_LOCK_REQUESTED) {
+
+if (prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status ==
QSS_STATUS_SIM_REMOVED) {
+mm_dbg ("QSS handler: #QSS=0 after +CSIM=1 -> CSIM locked!");
+self->priv->csim_lock_state = CSIM_LOCK_STATE_LOCKED;
+}
+
+if (prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status !=
QSS_STATUS_SIM_REMOVED) {
+mm_dbg ("QSS handler: #QSS>=1 after +CSIM=0 -> CSIM unlocked!");
+self->priv->csim_lock_state = CSIM_LOCK_STATE_UNLOCKED;
+
+if (self->priv->csim_lock_timeout_id) {
+g_source_remove (self->priv->csim_lock_timeout_id);
+self->priv->csim_lock_timeout_id = 0;
+}
+
+pending_csim_unlock_complete (self);
+}
+
+return;
+}
+
if (cur_qss_status != prev_qss_status)
-mm_dbg ("QSS: status changed '%s -> %s",
+mm_dbg ("QSS handler: status changed '%s -> %s",
mm_telit_qss_status_get_string (prev_qss_status),
mm_telit_qss_status_get_string (cur_qss_status));
if ((prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status !=
QSS_STATUS_SIM_REMOVED) ||
(prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status ==
QSS_STATUS_SIM_REMOVED)) {
-mm_info ("QSS: SIM swap detected");
+mm_info ("QSS handler: SIM swap detected");
mm_broadband_modem_update_sim_hot_swap_detected (MM_BROADBAND_MODEM
(self));
}
}
@@ -610,8 +638,9 @@ csim_unlock_ready (MMBaseModem *_self,
if (!response) {
if (g_error_matches (error,
MM_MOBILE_EQUIPMENT_ERROR,
- MM_MOBILE_EQUIPMENT_ERROR_NOT_SUPPORTED))
+ MM_MOBILE_EQUIPMENT_ERROR_NOT_SUPPORTED)) {
self->priv->csim_lock_support = FEATURE_NOT_SUPPORTED;
+}
mm_warn ("Couldn't unlock SIM card: %s", error->message);
g_error_free (error);
}
@@ -703,6 +732,8 @@ csim_lock_ready (MMBaseModem *_self,
g_object_unref (task);
return;
}
+} else {
+self->priv->csim_lock_state = CSIM_LOCK_STATE_LOCK_REQUESTED;
}
if (self->priv->csim_lock_support != FEATURE_NOT_SUPPORTED) {
@@ -755,6 +786,37 @@ handle_csim_locking (GTask*task,
}
static void
+pending_csim_unlock_complete (MMBroadbandModemTelit *self)
+{
+LoadUnlockRetriesContext *ctx;
+
+ctx = g_task_get_task_data (self->priv->csim_lock_task);
+
+if (ctx->succeded_requests == 0) {
+g_task_return_new_error (self->priv->csim_lock_task, MM_CORE_ERROR,
MM_CORE_ERROR_FAILED,
+ "Could not get any of the SIM unlock retries
values");
+} else {
+g_task_return_pointer (self->priv->csim_l
[PATCH v2] telit-plugin: ignore QSS when SIM-ME interface is locked
With some modems, the lock/unlock of the SIM-ME interface with +CSIM=1/0
command is followed by #QSS unsolicited messages. With the current
implementation, this messages are mistaken for SIM swap events and so the
modem is first dropped and then re-probed.
With this patch, the plugin takes into account the SIM-ME lock state when
parsing #QSS unsolicited, so that the QSS handler can correctly
elaborate the messages that are not related to SIM swap events.
---
plugins/telit/mm-broadband-modem-telit.c | 85
plugins/telit/mm-modem-helpers-telit.h | 9
2 files changed, 85 insertions(+), 9 deletions(-)
diff --git a/plugins/telit/mm-broadband-modem-telit.c
b/plugins/telit/mm-broadband-modem-telit.c
index e7650c0..11d93dd 100644
--- a/plugins/telit/mm-broadband-modem-telit.c
+++ b/plugins/telit/mm-broadband-modem-telit.c
@@ -44,6 +44,8 @@ G_DEFINE_TYPE_EXTENDED (MMBroadbandModemTelit,
mm_broadband_modem_telit, MM_TYPE
G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM,
iface_modem_init)
G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM_3GPP,
iface_modem_3gpp_init));
+#define CSIM_UNLOCK_MAX_TIMEOUT 3
+
typedef enum {
FEATURE_SUPPORT_UNKNOWN,
FEATURE_NOT_SUPPORTED,
@@ -53,6 +55,9 @@ typedef enum {
struct _MMBroadbandModemTelitPrivate {
FeatureSupport csim_lock_support;
MMTelitQssStatus qss_status;
+MMTelitCsimLockState csim_lock_state;
+GTask *csim_lock_task;
+guint csim_lock_timeout_id;
};
/*/
@@ -107,6 +112,7 @@ typedef struct {
} QssSetupContext;
static void qss_setup_step (GTask *task);
+static void pending_csim_unlock_complete (MMBroadbandModemTelit *self);
static void
telit_qss_unsolicited_handler (MMPortSerialAt *port,
@@ -122,14 +128,36 @@ telit_qss_unsolicited_handler (MMPortSerialAt *port,
prev_qss_status = self->priv->qss_status;
self->priv->qss_status = cur_qss_status;
+if (self->priv->csim_lock_state >= CSIM_LOCK_STATE_LOCK_REQUESTED) {
+
+if (prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status ==
QSS_STATUS_SIM_REMOVED) {
+mm_dbg ("QSS handler: #QSS=0 after +CSIM=1 -> CSIM locked!");
+self->priv->csim_lock_state = CSIM_LOCK_STATE_LOCKED;
+}
+
+if (prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status !=
QSS_STATUS_SIM_REMOVED) {
+mm_dbg ("QSS handler: #QSS>=1 after +CSIM=0 -> CSIM unlocked!");
+self->priv->csim_lock_state = CSIM_LOCK_STATE_UNLOCKED;
+
+if (self->priv->csim_lock_timeout_id) {
+g_source_remove (self->priv->csim_lock_timeout_id);
+self->priv->csim_lock_timeout_id = 0;
+}
+
+pending_csim_unlock_complete (self);
+}
+
+return;
+}
+
if (cur_qss_status != prev_qss_status)
-mm_dbg ("QSS: status changed '%s -> %s",
+mm_dbg ("QSS handler: status changed '%s -> %s",
mm_telit_qss_status_get_string (prev_qss_status),
mm_telit_qss_status_get_string (cur_qss_status));
if ((prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status !=
QSS_STATUS_SIM_REMOVED) ||
(prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status ==
QSS_STATUS_SIM_REMOVED)) {
-mm_info ("QSS: SIM swap detected");
+mm_info ("QSS handler: SIM swap detected");
mm_broadband_modem_update_sim_hot_swap_detected (MM_BROADBAND_MODEM
(self));
}
}
@@ -610,8 +638,9 @@ csim_unlock_ready (MMBaseModem *_self,
if (!response) {
if (g_error_matches (error,
MM_MOBILE_EQUIPMENT_ERROR,
- MM_MOBILE_EQUIPMENT_ERROR_NOT_SUPPORTED))
+ MM_MOBILE_EQUIPMENT_ERROR_NOT_SUPPORTED)) {
self->priv->csim_lock_support = FEATURE_NOT_SUPPORTED;
+}
mm_warn ("Couldn't unlock SIM card: %s", error->message);
g_error_free (error);
}
@@ -703,6 +732,8 @@ csim_lock_ready (MMBaseModem *_self,
g_object_unref (task);
return;
}
+} else {
+self->priv->csim_lock_state = CSIM_LOCK_STATE_LOCK_REQUESTED;
}
if (self->priv->csim_lock_support != FEATURE_NOT_SUPPORTED) {
@@ -755,6 +786,37 @@ handle_csim_locking (GTask*task,
}
static void
+pending_csim_unlock_complete (MMBroadbandModemTelit *self)
+{
+LoadUnlockRetriesContext *ctx;
+
+ctx = g_task_get_task_data (self->priv->csim_lock_task);
+
+if (ctx->succeded_requests == 0) {
+g_task_return_new_error (self->priv->csim_lock_task, MM_CORE_ERROR,
MM_CORE_ERROR_FAILED,
+ "Could not get any of the SIM unlock retries
values");
+} else {
+g_task_return_pointer (self->priv->csim_lock_task, g_object_ref
(ctx->retries), g_object_unref);
+}
+
+g_clear_object (&self->pr
Re: [PATCH] telit-plugin: ignore QSS when SIM-ME interface is locked
That's right, thank you
On Wed, 26 Jul 2017 at 12:21 Aleksander Morgado
wrote:
> On Wed, Jul 26, 2017 at 10:36 AM, Carlo Lobrano
> wrote:
> >>> +csim_unlock_complete (self->priv->csim_lock_task);
> >> Reset the csim_lock_task pointer here to NULL, please.
> >
> > Can I do this inside csim_unlock_complete or there is a reason to do it
> > outside this function?
> >
>
> If you do it inside the function, you should be passing the address of
> the GTask pointer, i.e.:
> csim_unlock_complete (GTask **task);
> So that you can do *task = NULL; inside the function. That's fine.
>
> Although, maybe, given that the GTask is inside the private structure
> of the modem object, it may make more sense if you pass the modem
> object directly, and access the private info within the function,
> something like:
>
> static void
> pending_csim_unlock_complete (MMBroadbandModemTelit *self)
> {
> // whatever
> g_task_return_something();
> g_clear_object (&self->priv->csim_lock_task);
> }
>
>
>
>
>
> --
> Aleksander
> https://aleksander.es
>
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH] telit-plugin: ignore QSS when SIM-ME interface is locked
>> +csim_unlock_complete (self->priv->csim_lock_task);
> Reset the csim_lock_task pointer here to NULL, please.
Can I do this inside csim_unlock_complete or there is a reason to do it
outside this function?
On Tue, 25 Jul 2017 at 14:28 Carlo Lobrano wrote:
> > Not sure you need to receive the GTask here from the timeout
> > user_data, as you already have it in the private info. Instead, you
> > could pass a reference to the MMBroadbandModemTelit object as
> > user_data and receive "self" here.
>
> Totally agree. I was unsure how to manage the two references and I didn't
> think I actually don't need it.
>
>
>
> On Tue, 25 Jul 2017 at 14:10 Aleksander Morgado
> wrote:
>
>> Hey,
>>
>> On Mon, Jul 24, 2017 at 6:23 PM, Carlo Lobrano
>> wrote:
>> > With some modems, the lock/unlock of the SIM-ME interface with +CSIM=1/0
>> > command is followed by #QSS unsolicited messages. With the current
>> > implementation, this messages are mistaken for SIM swap events and so
>> the
>> > modem is first dropped and then re-probed.
>> >
>> > With this patch, the plugin takes into account the SIM-ME lock state
>> when
>> > parsing #QSS unsolicited, so that the QSS handler can correctly
>> > elaborate the messages that are not related to SIM swap events.
>> > ---
>> > plugins/telit/mm-broadband-modem-telit.c | 90
>>
>> > plugins/telit/mm-modem-helpers-telit.h | 9
>> > 2 files changed, 90 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/plugins/telit/mm-broadband-modem-telit.c
>> b/plugins/telit/mm-broadband-modem-telit.c
>> > index e7650c0..3facc3e 100644
>> > --- a/plugins/telit/mm-broadband-modem-telit.c
>> > +++ b/plugins/telit/mm-broadband-modem-telit.c
>> > @@ -44,6 +44,8 @@ G_DEFINE_TYPE_EXTENDED (MMBroadbandModemTelit,
>> mm_broadband_modem_telit, MM_TYPE
>> > G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM,
>> iface_modem_init)
>> > G_IMPLEMENT_INTERFACE
>> (MM_TYPE_IFACE_MODEM_3GPP, iface_modem_3gpp_init));
>> >
>> > +#define CSIM_UNLOCK_MAX_TIMEOUT 3
>> > +
>> > typedef enum {
>> > FEATURE_SUPPORT_UNKNOWN,
>> > FEATURE_NOT_SUPPORTED,
>> > @@ -53,6 +55,9 @@ typedef enum {
>> > struct _MMBroadbandModemTelitPrivate {
>> > FeatureSupport csim_lock_support;
>> > MMTelitQssStatus qss_status;
>> > +MMTelitCsimLockState csim_lock_state;
>> > +GTask *csim_lock_task;
>> > +guint csim_lock_timeout_id;
>> > };
>> >
>> >
>> /*/
>> > @@ -107,6 +112,7 @@ typedef struct {
>> > } QssSetupContext;
>> >
>> > static void qss_setup_step (GTask *task);
>> > +static void csim_unlock_complete (GTask *task);
>> >
>> > static void
>> > telit_qss_unsolicited_handler (MMPortSerialAt *port,
>> > @@ -122,14 +128,36 @@ telit_qss_unsolicited_handler (MMPortSerialAt
>> *port,
>> > prev_qss_status = self->priv->qss_status;
>> > self->priv->qss_status = cur_qss_status;
>> >
>> > +if (self->priv->csim_lock_state >= CSIM_LOCK_STATE_LOCK_REQUESTED)
>> {
>> > +
>> > +if (prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status
>> == QSS_STATUS_SIM_REMOVED) {
>> > +mm_dbg ("QSS handler: #QSS=0 after +CSIM=1 -> CSIM
>> locked!");
>> > +self->priv->csim_lock_state = CSIM_LOCK_STATE_LOCKED;
>> > +}
>> > +
>> > +if (prev_qss_status == QSS_STATUS_SIM_REMOVED &&
>> cur_qss_status != QSS_STATUS_SIM_REMOVED) {
>> > +mm_dbg ("QSS handler: #QSS>=1 after +CSIM=0 -> CSIM
>> unlocked!");
>> > +self->priv->csim_lock_state = CSIM_LOCK_STATE_UNLOCKED;
>> > +
>> > +if (self->priv->csim_lock_timeout_id) {
>> > +g_source_remove (self->priv->csim_lock_timeout_id);
>> > +self->priv->csim_lock_timeout_id = 0;
>> > +}
>> > +
>> > +csim_unlock_complete (self->priv->csim_lock_task);
>>
>> Reset the csim_lock_task pointer here to NULL, please.
>>
>> > +}
>> > +
>&g
Re: [PATCH] telit-plugin: ignore QSS when SIM-ME interface is locked
> Not sure you need to receive the GTask here from the timeout
> user_data, as you already have it in the private info. Instead, you
> could pass a reference to the MMBroadbandModemTelit object as
> user_data and receive "self" here.
Totally agree. I was unsure how to manage the two references and I didn't
think I actually don't need it.
On Tue, 25 Jul 2017 at 14:10 Aleksander Morgado
wrote:
> Hey,
>
> On Mon, Jul 24, 2017 at 6:23 PM, Carlo Lobrano
> wrote:
> > With some modems, the lock/unlock of the SIM-ME interface with +CSIM=1/0
> > command is followed by #QSS unsolicited messages. With the current
> > implementation, this messages are mistaken for SIM swap events and so the
> > modem is first dropped and then re-probed.
> >
> > With this patch, the plugin takes into account the SIM-ME lock state when
> > parsing #QSS unsolicited, so that the QSS handler can correctly
> > elaborate the messages that are not related to SIM swap events.
> > ---
> > plugins/telit/mm-broadband-modem-telit.c | 90
>
> > plugins/telit/mm-modem-helpers-telit.h | 9
> > 2 files changed, 90 insertions(+), 9 deletions(-)
> >
> > diff --git a/plugins/telit/mm-broadband-modem-telit.c
> b/plugins/telit/mm-broadband-modem-telit.c
> > index e7650c0..3facc3e 100644
> > --- a/plugins/telit/mm-broadband-modem-telit.c
> > +++ b/plugins/telit/mm-broadband-modem-telit.c
> > @@ -44,6 +44,8 @@ G_DEFINE_TYPE_EXTENDED (MMBroadbandModemTelit,
> mm_broadband_modem_telit, MM_TYPE
> > G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM,
> iface_modem_init)
> > G_IMPLEMENT_INTERFACE
> (MM_TYPE_IFACE_MODEM_3GPP, iface_modem_3gpp_init));
> >
> > +#define CSIM_UNLOCK_MAX_TIMEOUT 3
> > +
> > typedef enum {
> > FEATURE_SUPPORT_UNKNOWN,
> > FEATURE_NOT_SUPPORTED,
> > @@ -53,6 +55,9 @@ typedef enum {
> > struct _MMBroadbandModemTelitPrivate {
> > FeatureSupport csim_lock_support;
> > MMTelitQssStatus qss_status;
> > +MMTelitCsimLockState csim_lock_state;
> > +GTask *csim_lock_task;
> > +guint csim_lock_timeout_id;
> > };
> >
> >
> /*/
> > @@ -107,6 +112,7 @@ typedef struct {
> > } QssSetupContext;
> >
> > static void qss_setup_step (GTask *task);
> > +static void csim_unlock_complete (GTask *task);
> >
> > static void
> > telit_qss_unsolicited_handler (MMPortSerialAt *port,
> > @@ -122,14 +128,36 @@ telit_qss_unsolicited_handler (MMPortSerialAt
> *port,
> > prev_qss_status = self->priv->qss_status;
> > self->priv->qss_status = cur_qss_status;
> >
> > +if (self->priv->csim_lock_state >= CSIM_LOCK_STATE_LOCK_REQUESTED) {
> > +
> > +if (prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status
> == QSS_STATUS_SIM_REMOVED) {
> > +mm_dbg ("QSS handler: #QSS=0 after +CSIM=1 -> CSIM
> locked!");
> > +self->priv->csim_lock_state = CSIM_LOCK_STATE_LOCKED;
> > +}
> > +
> > +if (prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status
> != QSS_STATUS_SIM_REMOVED) {
> > +mm_dbg ("QSS handler: #QSS>=1 after +CSIM=0 -> CSIM
> unlocked!");
> > +self->priv->csim_lock_state = CSIM_LOCK_STATE_UNLOCKED;
> > +
> > +if (self->priv->csim_lock_timeout_id) {
> > +g_source_remove (self->priv->csim_lock_timeout_id);
> > +self->priv->csim_lock_timeout_id = 0;
> > +}
> > +
> > +csim_unlock_complete (self->priv->csim_lock_task);
>
> Reset the csim_lock_task pointer here to NULL, please.
>
> > +}
> > +
> > +return;
> > +}
> > +
> > if (cur_qss_status != prev_qss_status)
> > -mm_dbg ("QSS: status changed '%s -> %s",
> > +mm_dbg ("QSS handler: status changed '%s -> %s",
> > mm_telit_qss_status_get_string (prev_qss_status),
> > mm_telit_qss_status_get_string (cur_qss_status));
> >
> > if ((prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status !=
> QSS_STATUS_SIM_REMOVED) ||
> > (prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status ==
> QSS_STATUS_SIM_REMOVED)) {
> > -mm_info ("QSS: SIM swap detected&qu
Re: [PATCH] sim hot swap: improved error management
> More importantly, after SIM hot swap is set up, an 'ATZ' command is > issued which resets the modem to power on state and so turns off sim hot > swap notification. I can't reproduce this issue. I see the ATZ command, but I still receive each QSS notification. By the way, do you mind if I report the observations on the patch "Ignore QSS..." in the other thread? It's a bit confusing to talk about it here. On Tue, 25 Jul 2017 at 13:49 Aleksander Morgado wrote: > > > > Finally, this patch replaces the MBIM's sim_hot_swap_on private property > > with the new property MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED, since they > have the > > same meaning. > > Eric, could you validate the patch with your MBIM modem as well? > > -- > Aleksander > https://aleksander.es > ___ > ModemManager-devel mailing list > ModemManager-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel > ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH] sim hot swap: improved error management
> Thanks for the patch. It doesn't seem to be working here unfortunately. this is the patch to improve SIM hot swap error management in both AT and MBIM based modems :) The patch for GE910-QUAD is in thread "[PATCH] telit-plugin: ignore QSS when SIM-ME interface is locked" On Tue, 25 Jul 2017 at 10:59 Tim Small wrote: > Hi Carlo, > > Thanks for the patch. It doesn't seem to be working here unfortunately. > It looks like AT+CSIM=1 is issued, but the corresponding AT+CSIM=0 > command isn't, so the SIM is still locked for direct interaction, so > subsequent AT commands are getting SIM not present responses. It looks > like the trouble starts when an ATV1 command is issued in the middle of > the CSIM PUK enquiries? > > Just to clarify this is a Telit GE910-QUAD (most recent firmware) with a > single serial connection (via a USB to UART adaptor at the moment since > I'm debugging with my PC), without using CMUX mode. > > Full trace here, with excerpts included below... > > > https://gist.github.com/tim-seoss/d175482c56072a0d303d315ca7353eb5#file-sim-hot-swap-debug-L795 > > Cheers, > > Tim. > > 19.623477] SIM is ready, running after SIM unlock step... > 19.623513] (ttyUSB0) device open count is 1 (close) > 21.306803] (ttyUSB0) device open count is 2 (open) > 21.306889] (ttyUSB0): --> 'AT+CSIM=1' > 21.329374] (ttyUSB0): <-- '' > 21.329663] (ttyUSB0): <-- 'O' > 21.329813] (ttyUSB0): <-- 'K' > 21.329858] (ttyUSB0) device open count is 3 (open) > 21.329887] (ttyUSB0) device open count is 2 (close) > 21.329933] (ttyUSB0): --> 'AT+CSIM=10,002100' > 21.351378] (ttyUSB0): <-- '#QSS: 0' > 21.351811] QSS: status changed 'sim-inserted -> sim-removed > 21.352036] QSS: SIM swap detected > 21.352490] load PIN unlock retries got no response: AT command was > cancelled > 21.352742] (ttyUSB0) device open count is 3 (open) > 21.352935] (ttyUSB0) device open count is 2 (close) > 21.353081] [device /sys/devices/pci:00/:00:14.0/usb1/1-9] > creating modem with plugin 'Telit' and '1' ports > 21.353650] (ttyUSB0) type 'at' claimed by > /sys/devices/pci:00/:00:14.0/usb1/1-9 > 21.353902] (/sys/devices/pci:00/:00:14.0/usb1/1-9) tty/ttyUSB0 > at (primary) > 21.354059] (/sys/devices/pci:00/:00:14.0/usb1/1-9) tty/ttyUSB0 > data (primary) > 21.357925] (ttyUSB0) opening serial port... > 21.358122] (ttyUSB0): couldn't set serial port closing_wait to none: > Inappropriate ioctl for device > 21.358157] (ttyUSB0): setting up baudrate: 115200 > 21.358181] (ttyUSB0) device open count is 1 (open) > 21.358193] (ttyUSB0): running init sequence... > 21.358218] (ttyUSB0) device open count is 2 (open) > 21.358244] (ttyUSB0) device open count is 3 (open) > 21.358263] Modem recreated for device > '/sys/devices/pci:00/:00:14.0/usb1/1-9' > 21.358546] loading current capabilities... > 21.358574] (ttyUSB0) device open count is 4 (open) > 21.358596] (ttyUSB0): --> 'AT+CSIM=10,002C000100' > 21.358614] (ttyUSB0): --> 'ATE0' > 21.366384] (ttyUSB0): <-- '' > 21.366594] (ttyUSB0): <-- '' > 21.366728] (ttyUSB0): <-- '+C' > 21.366813] (ttyUSB0): <-- 'S' > 21.366907] (ttyUSB0): <-- 'I' > 21.366984] (ttyUSB0): <-- 'M' > 21.367066] (ttyUSB0): <-- ':' > 21.367152] (ttyUSB0): <-- ' ' > 21.367237] (ttyUSB0): <-- '4' > 21.367330] (ttyUSB0): <-- ',' > 21.367432] (ttyUSB0): <-- '"' > 21.367511] (ttyUSB0): <-- '6' > 21.367593] (ttyUSB0): <-- '3' > 21.367674] (ttyUSB0): <-- 'C' > 21.367756] (ttyUSB0): <-- '3' > 21.367839] (ttyUSB0): <-- '"' > 21.367925] (ttyUSB0): <-- '' > 21.368013] (ttyUSB0): <-- '' > 21.368096] (ttyUSB0): <-- '' > 21.368174] (ttyUSB0): <-- '' > 21.368286] (ttyUSB0): <-- 'O' > 21.368498] (ttyUSB0): <-- 'K' > 21.368582] (ttyUSB0): <-- '' > 21.368627] (ttyUSB0): --> 'ATV1' > 21.381552] load PUK unlock retries got no response: AT command was > cancelled > 21.381615] (ttyUSB0) device open count is 1 (close) > 21.381641] load PIN2 unlock retries got no response: No AT port > available to run command > 21.381665] load PUK2 unlock retries got no response: No AT port > available to run command > 21.381685] Couldn't unlock SIM card: No AT port available to run command > 21.381713] Couldn't load unlock retries: 'Could not get any of the SIM > unlock retries values' > 21.381858] loading SIM identifier... > 21.382001] loading SIM identifier... > 21.382025] couldn't load SIM identifier: 'No AT port available to run > command' > 21.382042] loading IMSI... > 21.382063] couldn't load IMSI: 'No AT port available to run command' > 21.382082] loading Operator ID... > 21.382103] couldn't load Operator identifier: 'No AT port available to > run command' > 21.382116] loading Operator Name... > 21.382139] couldn't load Operator name: 'No AT port available to run > command' > 21.382190] loading own numbers... > 21.382213] couldn't load list of Own Numbers: 'No AT port available to > run command' > 21.382260] couldn't load current Bands: 'No AT port available to run > command' > 21.382325] couldn't initialize the modem: 'Operation
[PATCH] sim hot swap: improved error management
Currently, when SIM hot swap fails in either mm-iface or plugin, the
ModemManager still opens ports context and prints a message saying that
SIM hot swap is supported and that it's waiting for SIM insertion,
instead of clearly saying that SIM hot swap is not working.
This patch:
1. introduces a new property MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED
which is FALSE by default and set to TRUE only when
setup_sim_hot_swap_finish() succeded.
2. subordinates the completion of SIM hot swap setup (in
mm-broadband-modem) and the related messages to the the value of
MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED
Finally, this patch replaces the MBIM's sim_hot_swap_on private property
with the new property MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED, since they have
the
same meaning.
---
Please ignore the previous patch, since it was rebased on the wrong branch.
This one is rebased on master branch at commit
aa0a6bed363419659101084b3861a51144eee859
---
plugins/telit/mm-broadband-modem-telit.c | 1 +
src/mm-broadband-modem-mbim.c| 21 ---
src/mm-broadband-modem.c | 94 ++--
src/mm-iface-modem.c | 14 -
src/mm-iface-modem.h | 1 +
5 files changed, 93 insertions(+), 38 deletions(-)
diff --git a/plugins/telit/mm-broadband-modem-telit.c
b/plugins/telit/mm-broadband-modem-telit.c
index e7650c0..edd9093 100644
--- a/plugins/telit/mm-broadband-modem-telit.c
+++ b/plugins/telit/mm-broadband-modem-telit.c
@@ -1377,6 +1377,7 @@ mm_broadband_modem_telit_new (const gchar *device,
MM_BASE_MODEM_VENDOR_ID, vendor_id,
MM_BASE_MODEM_PRODUCT_ID, product_id,
MM_IFACE_MODEM_SIM_HOT_SWAP_SUPPORTED, TRUE,
+ MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED, FALSE,
NULL);
}
diff --git a/src/mm-broadband-modem-mbim.c b/src/mm-broadband-modem-mbim.c
index c69893f..7f5d2fc 100644
--- a/src/mm-broadband-modem-mbim.c
+++ b/src/mm-broadband-modem-mbim.c
@@ -89,8 +89,6 @@ struct _MMBroadbandModemMbimPrivate {
MbimDataClass available_data_classes;
MbimDataClass highest_available_data_class;
-/* For checking whether the SIM has been hot swapped */
-gboolean sim_hot_swap_on;
MbimSubscriberReadyState last_ready_state;
};
@@ -1990,6 +1988,7 @@ basic_connect_notification_subscriber_ready_status
(MMBroadbandModemMbim *self,
(self->priv->last_ready_state ==
MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED &&
ready_state != MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED)) {
/* SIM has been removed or reinserted, re-probe to ensure correct
interfaces are exposed */
+mm_dbg ("SIM hot swap detected");
mm_broadband_modem_update_sim_hot_swap_detected (MM_BROADBAND_MODEM
(self));
}
@@ -2286,10 +2285,15 @@ cleanup_unsolicited_events_3gpp (MMIfaceModem3gpp
*_self,
gpointer user_data)
{
MMBroadbandModemMbim *self = MM_BROADBAND_MODEM_MBIM (_self);
+gboolean is_sim_hot_swap_configured = FALSE;
+
+g_object_get (self,
+ MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED,
&is_sim_hot_swap_configured,
+ NULL);
self->priv->setup_flags &= ~PROCESS_NOTIFICATION_FLAG_SIGNAL_QUALITY;
self->priv->setup_flags &= ~PROCESS_NOTIFICATION_FLAG_CONNECT;
-if (self->priv->sim_hot_swap_on)
+if (is_sim_hot_swap_configured)
self->priv->setup_flags &= ~PROCESS_NOTIFICATION_FLAG_SUBSCRIBER_INFO;
self->priv->setup_flags &= ~PROCESS_NOTIFICATION_FLAG_PACKET_SERVICE;
common_setup_cleanup_unsolicited_events (self, FALSE, callback, user_data);
@@ -2500,7 +2504,6 @@ enable_subscriber_info_unsolicited_events_ready
(MMBroadbandModemMbim *self,
if (!common_enable_disable_unsolicited_events_finish (self, res, &error)) {
mm_dbg ("Failed to enable subscriber info events: %s", error->message);
-self->priv->sim_hot_swap_on = FALSE;
g_task_return_error (task, error);
g_object_unref (task);
return;
@@ -2519,7 +2522,6 @@ setup_subscriber_info_unsolicited_events_ready
(MMBroadbandModemMbim *self,
if (!common_setup_cleanup_unsolicited_events_finish (self, res, &error)) {
mm_dbg ("Failed to set up subscriber info events: %s", error->message);
-self->priv->sim_hot_swap_on = FALSE;
g_task_return_error (task, error);
g_object_unref (task);
return;
@@ -2541,7 +2543,6 @@ modem_setup_sim_hot_swap (MMIfaceModem *_self,
task = g_task_new (self, NULL, callback, user_data);
-self->priv->sim_hot_swap_on = TRUE;
self->priv->setup_flags |= PROCESS_NOTIFICATION_FLAG_SUBSCRIBER_INFO;
common_setup_cleanup_unsolicited_events (self,
TRUE,
@@ -2566,10 +2567,15 @@ modem_3gpp_disable_unsolicited_events (MMIfaceModem3gpp
*_self,
Re: sim hot swap problem with Telit GE910-QUAD
> I'll be back with the patch Sorry, I made a mistake and created a new thread. The patch is in "[PATCH] telit-plugin: ignore QSS when SIM-ME interface is locked" On Mon, 24 Jul 2017 at 12:27 Carlo Lobrano wrote: > That sounds perfect. > I'll be back with the patch > > On Mon, 24 Jul 2017 at 12:16 Aleksander Morgado > wrote: > >> On Mon, Jul 24, 2017 at 12:02 PM, Carlo Lobrano >> wrote: >> >> What would happen if you completely disable and enable the handlers > >> with >> >> the method I'm suggesting, and then you just let the #QSS=1 > >> indication >> >> happen? If the modem already had a SIM inserted, an extra > #QSS=1 >> wouldn't >> >> harm, right? Won't it be just ignored when it's >> >> processed? >> > >> > You're right, we don't need to process #QSS=1, but we absolutely need to >> > wait for it before sending any other SIM command. >> > >> > Since #QSS=1 arrives in a couple of seconds, even a sleep will do, but >> if >> > anything goes wrong we will get stuck in the next command (not sure if >> it is >> > a bug, but it looks like it to me). >> > >> >> I see, so yes, we do need to wait for the #QSS. >> >> > In the meantime I tried the my proposal and I'm not really sure about >> how to >> > wait for the unsolicited. I cannot let the thread go on, otherwise we >> call >> > +CRSM and get stuck, but if I put a simple sleep, we do not process any >> > unsolicited... >> > >> >> So, you're running an async operation (GSimpleAsyncResult or GTask). >> The logic won't go on unless you complete it, and you want to complete >> it only after receiving the indication or a give timeout happens. >> >> You solve this by setting up a timeout, e.g. >> g_timeout_add_seconds(3...) that will be triggered if you don't get >> the unsolicited indication. If the timeout happens, you complete the >> async method (you stored previously the GSimpleAsyncResult or GTask in >> private info). If the unsolicited message arrives, you also complete >> the async method, but you also cancel the timeout (so you also stored >> the timeout id in private info). The difference between a plain >> sleep() and a timeout via g_timeout_add_seconds() is that the former >> blocks the running thread, while the latter leaves the main loop >> running during the timeout (so unsolicited messages are processed). >> >> -- >> Aleksander >> https://aleksander.es >> > ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
[PATCH] telit-plugin: ignore QSS when SIM-ME interface is locked
With some modems, the lock/unlock of the SIM-ME interface with +CSIM=1/0
command is followed by #QSS unsolicited messages. With the current
implementation, this messages are mistaken for SIM swap events and so the
modem is first dropped and then re-probed.
With this patch, the plugin takes into account the SIM-ME lock state when
parsing #QSS unsolicited, so that the QSS handler can correctly
elaborate the messages that are not related to SIM swap events.
---
plugins/telit/mm-broadband-modem-telit.c | 90
plugins/telit/mm-modem-helpers-telit.h | 9
2 files changed, 90 insertions(+), 9 deletions(-)
diff --git a/plugins/telit/mm-broadband-modem-telit.c
b/plugins/telit/mm-broadband-modem-telit.c
index e7650c0..3facc3e 100644
--- a/plugins/telit/mm-broadband-modem-telit.c
+++ b/plugins/telit/mm-broadband-modem-telit.c
@@ -44,6 +44,8 @@ G_DEFINE_TYPE_EXTENDED (MMBroadbandModemTelit,
mm_broadband_modem_telit, MM_TYPE
G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM,
iface_modem_init)
G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM_3GPP,
iface_modem_3gpp_init));
+#define CSIM_UNLOCK_MAX_TIMEOUT 3
+
typedef enum {
FEATURE_SUPPORT_UNKNOWN,
FEATURE_NOT_SUPPORTED,
@@ -53,6 +55,9 @@ typedef enum {
struct _MMBroadbandModemTelitPrivate {
FeatureSupport csim_lock_support;
MMTelitQssStatus qss_status;
+MMTelitCsimLockState csim_lock_state;
+GTask *csim_lock_task;
+guint csim_lock_timeout_id;
};
/*/
@@ -107,6 +112,7 @@ typedef struct {
} QssSetupContext;
static void qss_setup_step (GTask *task);
+static void csim_unlock_complete (GTask *task);
static void
telit_qss_unsolicited_handler (MMPortSerialAt *port,
@@ -122,14 +128,36 @@ telit_qss_unsolicited_handler (MMPortSerialAt *port,
prev_qss_status = self->priv->qss_status;
self->priv->qss_status = cur_qss_status;
+if (self->priv->csim_lock_state >= CSIM_LOCK_STATE_LOCK_REQUESTED) {
+
+if (prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status ==
QSS_STATUS_SIM_REMOVED) {
+mm_dbg ("QSS handler: #QSS=0 after +CSIM=1 -> CSIM locked!");
+self->priv->csim_lock_state = CSIM_LOCK_STATE_LOCKED;
+}
+
+if (prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status !=
QSS_STATUS_SIM_REMOVED) {
+mm_dbg ("QSS handler: #QSS>=1 after +CSIM=0 -> CSIM unlocked!");
+self->priv->csim_lock_state = CSIM_LOCK_STATE_UNLOCKED;
+
+if (self->priv->csim_lock_timeout_id) {
+g_source_remove (self->priv->csim_lock_timeout_id);
+self->priv->csim_lock_timeout_id = 0;
+}
+
+csim_unlock_complete (self->priv->csim_lock_task);
+}
+
+return;
+}
+
if (cur_qss_status != prev_qss_status)
-mm_dbg ("QSS: status changed '%s -> %s",
+mm_dbg ("QSS handler: status changed '%s -> %s",
mm_telit_qss_status_get_string (prev_qss_status),
mm_telit_qss_status_get_string (cur_qss_status));
if ((prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status !=
QSS_STATUS_SIM_REMOVED) ||
(prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status ==
QSS_STATUS_SIM_REMOVED)) {
-mm_info ("QSS: SIM swap detected");
+mm_info ("QSS handler: SIM swap detected");
mm_broadband_modem_update_sim_hot_swap_detected (MM_BROADBAND_MODEM
(self));
}
}
@@ -610,8 +638,9 @@ csim_unlock_ready (MMBaseModem *_self,
if (!response) {
if (g_error_matches (error,
MM_MOBILE_EQUIPMENT_ERROR,
- MM_MOBILE_EQUIPMENT_ERROR_NOT_SUPPORTED))
+ MM_MOBILE_EQUIPMENT_ERROR_NOT_SUPPORTED)) {
self->priv->csim_lock_support = FEATURE_NOT_SUPPORTED;
+}
mm_warn ("Couldn't unlock SIM card: %s", error->message);
g_error_free (error);
}
@@ -703,6 +732,8 @@ csim_lock_ready (MMBaseModem *_self,
g_object_unref (task);
return;
}
+} else {
+self->priv->csim_lock_state = CSIM_LOCK_STATE_LOCK_REQUESTED;
}
if (self->priv->csim_lock_support != FEATURE_NOT_SUPPORTED) {
@@ -755,6 +786,42 @@ handle_csim_locking (GTask*task,
}
static void
+csim_unlock_complete (GTask *task)
+{
+MMBroadbandModemTelit *self;
+LoadUnlockRetriesContext *ctx;
+
+self = MM_BROADBAND_MODEM_TELIT (g_task_get_source_object (task));
+
+ctx = g_task_get_task_data (task);
+if (ctx->succeded_requests == 0) {
+g_task_return_new_error (task, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+ "Could not get any of the SIM unlock retries
values");
+} else {
+g_task_return_pointer (task, g_object_ref (ctx->retries),
g_object_unref);
+}
+
+g_object_unref (task)
Re: [PATCH v2] sim hot swap: improved error management
> Thanks for that - it seems to work for me. I had to manually apply one > of the hunks, so maybe I'm applying to the wrong branch? It should be right, since I rebased on master branch as well. Maybe I made a mistake, let me double check On Mon, 24 Jul 2017 at 15:44 Tim Small wrote: > On 24/07/17 12:54, Carlo Lobrano wrote: > > Currently, when SIM hot swap fails in either mm-iface or plugin, the > > ModemManager still opens ports context and prints a message saying that > > SIM hot swap is supported and that it's waiting for SIM insertion, > > instead of clearly saying that SIM hot swap is not working. > > > > This patch: > > > Hi Carlo, > > Thanks for that - it seems to work for me. I had to manually apply one > of the hunks, so maybe I'm applying to the wrong branch? > > src/mm-broadband-modem.c > @@ -10304,34 +10317,47 @@ > > was the hunk in question, and I'm using > https://anongit.freedesktop.org/git/ModemManager/ModemManager.git master > branch. > > Cheers, > > Tim. > > -- > South East Open Source Solutions Limited > Registered in England and Wales with company number 06134732. > Registered Office: 2 Powell Gardens, Redhill, Surrey, RH1 1TQ > VAT number: 900 6633 53 http://seoss.co.uk/ +44-(0)1273-808309 > <+44%201273%20808309> > ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
[PATCH v2] sim hot swap: improved error management
Currently, when SIM hot swap fails in either mm-iface or plugin, the
ModemManager still opens ports context and prints a message saying that
SIM hot swap is supported and that it's waiting for SIM insertion,
instead of clearly saying that SIM hot swap is not working.
This patch:
1. introduces a new property MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED
which is FALSE by default and set to TRUE only when
setup_sim_hot_swap_finish() succeded.
2. subordinates the completion of SIM hot swap setup (in
mm-broadband-modem) and the related messages to the the value of
MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED
Finally, this patch replaces the MBIM's "sim_hot_swap_on" private property
with the new property MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED, since they have
the
same meaning.
---
plugins/telit/mm-broadband-modem-telit.c | 1 +
src/mm-broadband-modem-mbim.c| 21 +---
src/mm-broadband-modem.c | 91 ++--
src/mm-iface-modem.c | 14 -
src/mm-iface-modem.h | 1 +
5 files changed, 92 insertions(+), 36 deletions(-)
diff --git a/plugins/telit/mm-broadband-modem-telit.c
b/plugins/telit/mm-broadband-modem-telit.c
index a8ee886..9f5b588 100644
--- a/plugins/telit/mm-broadband-modem-telit.c
+++ b/plugins/telit/mm-broadband-modem-telit.c
@@ -1373,6 +1373,7 @@ mm_broadband_modem_telit_new (const gchar *device,
MM_BASE_MODEM_VENDOR_ID, vendor_id,
MM_BASE_MODEM_PRODUCT_ID, product_id,
MM_IFACE_MODEM_SIM_HOT_SWAP_SUPPORTED, TRUE,
+ MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED, FALSE,
NULL);
}
diff --git a/src/mm-broadband-modem-mbim.c b/src/mm-broadband-modem-mbim.c
index 7c25c42..46d3693 100644
--- a/src/mm-broadband-modem-mbim.c
+++ b/src/mm-broadband-modem-mbim.c
@@ -89,8 +89,6 @@ struct _MMBroadbandModemMbimPrivate {
MbimDataClass available_data_classes;
MbimDataClass highest_available_data_class;
-/* For checking whether the SIM has been hot swapped */
-gboolean sim_hot_swap_on;
MbimSubscriberReadyState last_ready_state;
};
@@ -1990,6 +1988,7 @@ basic_connect_notification_subscriber_ready_status
(MMBroadbandModemMbim *self,
(self->priv->last_ready_state ==
MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED &&
ready_state != MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED)) {
/* SIM has been removed or reinserted, re-probe to ensure correct
interfaces are exposed */
+mm_dbg ("SIM hot swap detected");
mm_broadband_modem_update_sim_hot_swap_detected (MM_BROADBAND_MODEM
(self));
}
@@ -2286,10 +2285,15 @@ cleanup_unsolicited_events_3gpp (MMIfaceModem3gpp
*_self,
gpointer user_data)
{
MMBroadbandModemMbim *self = MM_BROADBAND_MODEM_MBIM (_self);
+gboolean is_sim_hot_swap_configured = FALSE;
+
+g_object_get (self,
+ MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED,
&is_sim_hot_swap_configured,
+ NULL);
self->priv->setup_flags &= ~PROCESS_NOTIFICATION_FLAG_SIGNAL_QUALITY;
self->priv->setup_flags &= ~PROCESS_NOTIFICATION_FLAG_CONNECT;
-if (self->priv->sim_hot_swap_on)
+if (is_sim_hot_swap_configured)
self->priv->setup_flags &= ~PROCESS_NOTIFICATION_FLAG_SUBSCRIBER_INFO;
self->priv->setup_flags &= ~PROCESS_NOTIFICATION_FLAG_PACKET_SERVICE;
common_setup_cleanup_unsolicited_events (self, FALSE, callback, user_data);
@@ -2500,7 +2504,6 @@ enable_subscriber_info_unsolicited_events_ready
(MMBroadbandModemMbim *self,
if (!common_enable_disable_unsolicited_events_finish (self, res, &error)) {
mm_dbg ("Failed to enable subscriber info events: %s", error->message);
-self->priv->sim_hot_swap_on = FALSE;
g_task_return_error (task, error);
g_object_unref (task);
return;
@@ -2519,7 +2522,6 @@ setup_subscriber_info_unsolicited_events_ready
(MMBroadbandModemMbim *self,
if (!common_setup_cleanup_unsolicited_events_finish (self, res, &error)) {
mm_dbg ("Failed to set up subscriber info events: %s", error->message);
-self->priv->sim_hot_swap_on = FALSE;
g_task_return_error (task, error);
g_object_unref (task);
return;
@@ -2541,7 +2543,6 @@ modem_setup_sim_hot_swap (MMIfaceModem *_self,
task = g_task_new (self, NULL, callback, user_data);
-self->priv->sim_hot_swap_on = TRUE;
self->priv->setup_flags |= PROCESS_NOTIFICATION_FLAG_SUBSCRIBER_INFO;
common_setup_cleanup_unsolicited_events (self,
TRUE,
@@ -2566,10 +2567,15 @@ modem_3gpp_disable_unsolicited_events (MMIfaceModem3gpp
*_self,
gpointer user_data)
{
MMBroadbandModemMbim *self = MM_BROADBAND_MODEM_MBIM (_self);
+gboolean is_sim_hot_swap_configured = FALSE;
Re: sim hot swap problem with Telit GE910-QUAD
That sounds perfect. I'll be back with the patch On Mon, 24 Jul 2017 at 12:16 Aleksander Morgado wrote: > On Mon, Jul 24, 2017 at 12:02 PM, Carlo Lobrano > wrote: > >> What would happen if you completely disable and enable the handlers > > with > >> the method I'm suggesting, and then you just let the #QSS=1 > indication > >> happen? If the modem already had a SIM inserted, an extra > #QSS=1 > wouldn't > >> harm, right? Won't it be just ignored when it's > >> processed? > > > > You're right, we don't need to process #QSS=1, but we absolutely need to > > wait for it before sending any other SIM command. > > > > Since #QSS=1 arrives in a couple of seconds, even a sleep will do, but if > > anything goes wrong we will get stuck in the next command (not sure if > it is > > a bug, but it looks like it to me). > > > > I see, so yes, we do need to wait for the #QSS. > > > In the meantime I tried the my proposal and I'm not really sure about > how to > > wait for the unsolicited. I cannot let the thread go on, otherwise we > call > > +CRSM and get stuck, but if I put a simple sleep, we do not process any > > unsolicited... > > > > So, you're running an async operation (GSimpleAsyncResult or GTask). > The logic won't go on unless you complete it, and you want to complete > it only after receiving the indication or a give timeout happens. > > You solve this by setting up a timeout, e.g. > g_timeout_add_seconds(3...) that will be triggered if you don't get > the unsolicited indication. If the timeout happens, you complete the > async method (you stored previously the GSimpleAsyncResult or GTask in > private info). If the unsolicited message arrives, you also complete > the async method, but you also cancel the timeout (so you also stored > the timeout id in private info). The difference between a plain > sleep() and a timeout via g_timeout_add_seconds() is that the former > blocks the running thread, while the latter leaves the main loop > running during the timeout (so unsolicited messages are processed). > > -- > Aleksander > https://aleksander.es > ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: sim hot swap problem with Telit GE910-QUAD
> What would happen if you completely disable and enable the handlers > with the method I'm suggesting, and then you just let the #QSS=1 > indication happen? If the modem already had a SIM inserted, an extra > #QSS=1 wouldn't harm, right? Won't it be just ignored when it's > processed? You're right, we don't need to process #QSS=1, but we absolutely need to wait for it before sending any other SIM command. Since #QSS=1 arrives in a couple of seconds, even a sleep will do, but if anything goes wrong we will get stuck in the next command (not sure if it is a bug, but it looks like it to me). In the meantime I tried the my proposal and I'm not really sure about how to wait for the unsolicited. I cannot let the thread go on, otherwise we call +CRSM and get stuck, but if I put a simple sleep, we do not process any unsolicited... On Mon, 24 Jul 2017 at 10:59 Aleksander Morgado wrote: > Hey, > > > > >> I just reproduced the issue on my hardware, so I can develop a patch for > >> this > > > > > > > > after some analysis, I think that the fix for this issue should count two > > steps > > > > 1. When +CSIM=1 is sent, the QSS handler should ignore QSS unsolicited > -> I > > brefly solved this with a "is_csim_locked" boolean in > MMBroadbandModemTelit > > priv, set by csim_lock_ready and read by the handler. > > > > 2. As well as +CSIM=1 causes a #QSS=0, even +CSIM=0 causes a #QSS=1, but > > this time we need to wait for this unsolicited to come before accepting > any > > other call to the SIM. Considering that the very next call is +CSRM, we > need > > to stop soon after unlocking SIM interface. > > > > I think I can do this in csim_unlock_ready, waiting 'till the QSS handler > > receives #QSS=1 (and up to some maximum amount of time, like 3-5 seconds, > > just to be sure), but since this will stop the entire process I > preferred to > > share this idea with you before working on it. > > > > what do you think? > > I was going to suggest you use > mm_port_serial_at_enable_unsolicited_msg_handler() as that allows you > to temporarily disable/enable an already configured unsolicited > message handler, but in this case you do want to use the handler as > you're expecting the #QSS=1 after +CSIM=0... > > What would happen if you completely disable and enable the handlers > with the method I'm suggesting, and then you just let the #QSS=1 > indication happen? If the modem already had a SIM inserted, an extra > #QSS=1 wouldn't harm, right? Won't it be just ignored when it's > processed? > > -- > Aleksander > https://aleksander.es > ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: sim hot swap problem with Telit GE910-QUAD
Hi Aleksander, Tim, I just reproduced the issue on my hardware, so I can develop a patch for > this > after some analysis, I think that the fix for this issue should count two steps 1. When +CSIM=1 is sent, the QSS handler should ignore QSS unsolicited -> I brefly solved this with a "*is_csim_locked*" boolean in MMBroadbandModemTelit priv, set by *csim_lock_ready* and read by the handler. 2. As well as +CSIM=1 causes a #QSS=0, even +CSIM=0 causes a #QSS=1, but this time we need to wait for this unsolicited to come before accepting any other call to the SIM. Considering that the very next call is +CSRM, we need to stop soon after unlocking SIM interface. I think I can do this in *csim_unlock_ready*, waiting 'till the QSS handler receives #QSS=1 (and up to some maximum amount of time, like 3-5 seconds, just to be sure), but since this will stop the entire process I preferred to share this idea with you before working on it. what do you think? ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: sim hot swap problem with Telit GE910-QUAD
> Carlo, what do you think? I just reproduced the issue on my hardware, so I can develop a patch for this On 21 July 2017 at 09:58, Aleksander Morgado wrote: > >> Oh wait, what happened here?... How did we "recreate" the modem object > >> and we're still sending AT+CSIM commands? > > > > Would you like me to dig deeper on this? Any hints? > > > > Before doing any further investigation I think we should go on and > implement the change to ignore #QSS indications in between CSIM=1 and > CSIM=0. > Carlo, what do you think? > > -- > Aleksander > https://aleksander.es > ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: sim hot swap problem with Telit GE910-QUAD
Hi Tim, > I think that the commands which mm issue causes it to close and reopen > the SIM and this takes longer than mm is giving it. How should I work > around this? This modem has an unsolicited signal (QSS) that shows the presence and status of the SIM. According to this signal, it seems that your SIM is detaching/attaching frequently (an unsolicited QSS 0 means SIM removed, while 1 means SIM inserted) ModemManager[27568]: (ttyUSB0): <-- '' ModemManager[27568]: (ttyUSB0): <-- '' ModemManager[27568]: (ttyUSB0): <-- '#' ModemManager[27568]: (ttyUSB0): <-- 'Q' ModemManager[27568]: (ttyUSB0): <-- 'S' ModemManager[27568]: (ttyUSB0): <-- 'S' ModemManager[27568]: (ttyUSB0): <-- ':' ModemManager[27568]: (ttyUSB0): <-- ' ' ModemManager[27568]: (ttyUSB0): <-- '1' ModemManager[27568]: (ttyUSB0): <-- '' ModemManager[27568]: (ttyUSB0): <-- '' ModemManager[27568]: QSS: status changed 'sim-removed -> sim-inserted ModemManager[27568]: QSS: SIM swap detected then again ModemManager[27568]: (ttyUSB0): <-- '#QSS: 0' After this last status change, even other commands return with "SIM missing" ModemManager[27568]: (ttyUSB0): --> 'AT+CPIN?' ModemManager[27568]: (ttyUSB0): <-- '' ModemManager[27568]: (ttyUSB0): <-- '+C' ModemManager[27568]: (ttyUSB0): <-- 'M' ModemManager[27568]: (ttyUSB0): <-- 'E' ModemManager[27568]: (ttyUSB0): <-- ' ' ModemManager[27568]: (ttyUSB0): <-- 'E' ModemManager[27568]: (ttyUSB0): <-- 'R' ModemManager[27568]: (ttyUSB0): <-- 'R' ModemManager[27568]: (ttyUSB0): <-- 'O' ModemManager[27568]: (ttyUSB0): <-- 'R' ModemManager[27568]: (ttyUSB0): <-- ':' ModemManager[27568]: (ttyUSB0): <-- ' ' ModemManager[27568]: (ttyUSB0): <-- '1' ModemManager[27568]: (ttyUSB0): <-- '0' ModemManager[27568]: (ttyUSB0): <-- '' ModemManager[27568]: (ttyUSB0): <-- '' ModemManager[27568]: Got failure code 10: SIM not inserted according to this, ModemManager behavior is correct. It should be investigated why those notifications come. Could you check the status of your SIM or SIM holder? On 19 July 2017 at 18:25, Tim Small wrote: > Hello, > > When probing a Telit GE910-QUAD (with a SIM inserted) mm always gives me > something like this in the debug logs: > > ModemManager[26945]: (ttyUSB0) device open count is 2 (close) > ModemManager[26945]: (ttyUSB0): --> 'AT#QSS?' > ModemManager[26945]: (ttyUSB0): <-- '' > ModemManager[26945]: (ttyUSB0): <-- '#' > ModemManager[26945]: (ttyUSB0): <-- 'Q' > ModemManager[26945]: (ttyUSB0): <-- 'S' > ModemManager[26945]: (ttyUSB0): <-- 'S' > ModemManager[26945]: (ttyUSB0): <-- ':' > ModemManager[26945]: (ttyUSB0): <-- ' ' > ModemManager[26945]: (ttyUSB0): <-- '1' > ModemManager[26945]: (ttyUSB0): <-- ',' > ModemManager[26945]: (ttyUSB0): <-- '0' > ModemManager[26945]: (ttyUSB0): <-- '' > ModemManager[26945]: (ttyUSB0): <-- '' > ModemManager[26945]: (ttyUSB0): <-- '' > ModemManager[26945]: (ttyUSB0): <-- '' > ModemManager[26945]: (ttyUSB0): <-- 'OK' > ModemManager[26945]: (ttyUSB0): <-- '' > ModemManager[26945]: (ttyUSB0): <-- '' > ModemManager[26945]: QSS: current status is 'sim-removed' > ModemManager[26945]: (ttyUSB0) device open count is 3 (open) > ModemManager[26945]: (ttyUSB0) device open count is 2 (close) > ModemManager[26945]: (ttyUSB0): --> 'AT#QSS=1' > ModemManager[26945]: (ttyUSB0): <-- '' > ModemManager[26945]: (ttyUSB0): <-- 'OK' > ModemManager[26945]: (ttyUSB0): <-- '' > ModemManager[26945]: (ttyUSB0): <-- '' > ModemManager[26945]: Iface modem: SIM hot swap setup succeded > ModemManager[26945]: checking if unlock required... > ModemManager[26945]: (ttyUSB0) device open count is 3 (open) > ModemManager[26945]: (ttyUSB0) device open count is 2 (close) > ModemManager[26945]: (ttyUSB0): --> 'AT+CPIN?' > ModemManager[26945]: (ttyUSB0): <-- '' > ModemManager[26945]: (ttyUSB0): <-- '+' > ModemManager[26945]: (ttyUSB0): <-- 'C' > ModemManager[26945]: (ttyUSB0): <-- 'M' > ModemManager[26945]: (ttyUSB0): <-- 'E' > ModemManager[26945]: (ttyUSB0): <-- ' ' > ModemManager[26945]: (ttyUSB0): <-- 'E' > ModemManager[26945]: (ttyUSB0): <-- 'R' > ModemManager[26945]: (ttyUSB0): <-- 'R' > ModemManager[26945]: (ttyUSB0): <-- 'O' > ModemManager[26945]: (ttyUSB0): <-- 'R' > ModemManager[26945]: (ttyUSB0): <-- ':' > ModemManager[26945]: (ttyUSB0): <-- ' ' > ModemManager[26945]: (ttyUSB0): <-- '1' > ModemManager[26945]: (ttyUSB0): <-- '0' > ModemManager[26945]: (ttyUSB0): <-- '' > ModemManager[26945]: (ttyUSB0): <-- '' > ModemManager[26945]: Got failure code 10: SIM not inserted > ModemManager[26945]: Couldn't check if unlock required: 'SIM not > inserted' > ModemManager[26945]: Modem: state changed (unknown -> failed) > ModemManager[26945]: Modem couldn't be initialized: Couldn't > check unlock status: SIM not inserted > ModemManager[26945]: (ttyUSB0) device open count is 1 (close
Re: Telit LE910 failing to find network
Hi Kelvin, I tried upgrading to modemmanager-1.6.4 and similar results. > > could you please attach the full debug logs, so we can have a complete picture of what's going on? Moreover, what's the PID of your modem? By default LE910 uses QMI protocol (cdc-wdmX device, instead of ttyUSB). ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH] sim hot swap: improved error management
Hi Aleksander, Any chance you can update also the MBIM implementation to use > MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED in the same way? > sure, I can have a look at this this afternoon or next week at the most ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
[PATCH] sim hot swap: improved error management
Currently, when SIM hot swap fails in either mm-iface or plugin, the
ModemManager still opens ports context and prints a message saying that
SIM hot swap is supported and that it's waiting for SIM insertion,
instead of clearly saying that SIM hot swap is not working.
This patch:
1. introduces a new property MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED
which is FALSE by default and set to TRUE only when
setup_sim_hot_swap_finish() succeded.
2. subordinates the completion of SIM hot swap setup (in
mm-broadband-modem) and the related messages to the the value of
MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED
---
plugins/telit/mm-broadband-modem-telit.c | 1 +
src/mm-broadband-modem.c | 91 ++--
src/mm-iface-modem.c | 14 -
src/mm-iface-modem.h | 1 +
4 files changed, 78 insertions(+), 29 deletions(-)
diff --git a/plugins/telit/mm-broadband-modem-telit.c
b/plugins/telit/mm-broadband-modem-telit.c
index a8ee886..9f5b588 100644
--- a/plugins/telit/mm-broadband-modem-telit.c
+++ b/plugins/telit/mm-broadband-modem-telit.c
@@ -1373,6 +1373,7 @@ mm_broadband_modem_telit_new (const gchar *device,
MM_BASE_MODEM_VENDOR_ID, vendor_id,
MM_BASE_MODEM_PRODUCT_ID, product_id,
MM_IFACE_MODEM_SIM_HOT_SWAP_SUPPORTED, TRUE,
+ MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED, FALSE,
NULL);
}
diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
index 08aa23a..9f842c9 100644
--- a/src/mm-broadband-modem.c
+++ b/src/mm-broadband-modem.c
@@ -116,6 +116,7 @@ enum {
PROP_MODEM_VOICE_CALL_LIST,
PROP_MODEM_SIMPLE_STATUS,
PROP_MODEM_SIM_HOT_SWAP_SUPPORTED,
+PROP_MODEM_SIM_HOT_SWAP_CONFIGURED,
PROP_FLOW_CONTROL,
PROP_LAST
};
@@ -134,6 +135,7 @@ struct _MMBroadbandModemPrivate {
PortsContext *sim_hot_swap_ports_ctx;
gboolean modem_init_run;
gboolean sim_hot_swap_supported;
+gboolean sim_hot_swap_configured;
/*<--- Modem interface --->*/
/* Properties */
@@ -10242,27 +10244,38 @@ initialize_step (InitializeContext *ctx)
* (we may be re-running the initialization step after SIM-PIN unlock)
*/
if (!ctx->self->priv->sim_hot_swap_ports_ctx) {
gboolean is_sim_hot_swap_supported = FALSE;
+gboolean is_sim_hot_swap_configured = FALSE;
g_object_get (ctx->self,
MM_IFACE_MODEM_SIM_HOT_SWAP_SUPPORTED,
&is_sim_hot_swap_supported,
NULL);
+g_object_get (ctx->self,
+ MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED,
&is_sim_hot_swap_configured,
+ NULL);
+
if (is_sim_hot_swap_supported) {
-PortsContext *ports;
-GError *error = NULL;
-mm_dbg ("Creating ports context for SIM hot swap");
+if (!is_sim_hot_swap_configured) {
+mm_warn ("SIM hot swap supported but not configured.
Skipping opening ports");
+} else {
+PortsContext *ports;
+GError *error = NULL;
-ports = g_new0 (PortsContext, 1);
-ports->ref_count = 1;
+mm_dbg ("Creating ports context for SIM hot swap");
-if (!open_ports_enabling (ctx->self, ports, FALSE, &error)) {
-mm_warn ("Couldn't open ports during Modem SIM hot swap
enabling: %s", error? error->message : "unknown reason");
-g_error_free (error);
-} else
-ctx->self->priv->sim_hot_swap_ports_ctx =
ports_context_ref (ports);
+ports = g_new0 (PortsContext, 1);
+ports->ref_count = 1;
-ports_context_unref (ports);
+if (!open_ports_enabling (ctx->self, ports, FALSE,
&error)) {
+mm_warn ("Couldn't open ports during Modem SIM hot
swap enabling: %s", error? error->message : "unknown reason");
+g_error_free (error);
+} else {
+ctx->self->priv->sim_hot_swap_ports_ctx =
ports_context_ref (ports);
+}
+
+ports_context_unref (ports);
+}
}
} else
mm_dbg ("Ports context for SIM hot swap already available");
@@ -10291,34 +10304,47 @@ initialize_step (InitializeContext *ctx)
} else {
/* Fatal SIM, firmware, or modem failure :-( */
gboolean is_sim_hot_swap_supported = FALSE;
+gboolean is_sim_hot_swap_configured = FALSE;
+
MMModemStateFailedReason reason =
mm_gdbus_modem_get_state_failed_reason (
(MmGdbusModem*)ctx->self->priv->modem_d
Re: Misleading message if SIM Hot swap configuration fails
Hi Alexander, yes you understood correctly and I agree with the suggestion. I'll work on it. Thanks On 6 July 2017 at 18:44, Aleksander Morgado wrote: > On Thu, Jul 6, 2017 at 5:10 PM, Carlo Lobrano wrote: > > looking at sim hot swap code, I noticed that even if sim swap > configuration > > fails at some point (in core or in the modem), we still print the message > > "SIM is missing, but the modem supports SIM hot swap. Waiting for > SIM...", > > which is kind of misleading in that case. > > > > I'm working on a patch and wondering whether there is any mechanism to > > notify errors from iface-modem to broadband-modem, to not only notify the > > right message but to prevent opening the dedicated ports. > > If I understand you correctly, you wouldn't want to issue that message > or process the extra open ports context if the modem does support > hot-swap BUT hot-swap configuration failed, right? I guess we could > have a new property in addition to > MM_IFACE_MODEM_SIM_HOT_SWAP_SUPPORTED, something like > MM_IFACE_MODEM_SIM_HOT_SWAP_READY which would be set by mm-iface-modem > to TRUE only of setup_sim_hot_swap_finish() succeeded. > > What do you think? > > > -- > Aleksander > https://aleksander.es > ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Misleading message if SIM Hot swap configuration fails
Hey, looking at sim hot swap code, I noticed that even if sim swap configuration fails at some point (in core or in the modem), we still print the message "SIM is missing, but the modem supports SIM hot swap. Waiting for SIM...", which is kind of misleading in that case. I'm working on a patch and wondering whether there is any mechanism to notify errors from iface-modem to broadband-modem, to not only notify the right message but to prevent opening the dedicated ports. Thanks, Carlo ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: MC7455 sporadically wrong primary port
On 4 July 2017 at 17:37, Karoly Pados wrote: > As you can see the modem has multiple ports. The modem is the only USB > device in the whole system (except for the on-board USB wifi adapter, which > is non-removable). The phenomenon I'm seeing is that the "primary port" > above is often something else than cdc-wdm0, sometimes even a ttyUSBX, and > in these cases the modem is unusable for data. The only workaround I have > is to keep resetting the modem until the correct primary port is picked. > Ok, I don't know this model, but I suppose that it does support MBIM or QMI (so the cdc-wdm port), but that somethings something goes wrong and it switches to AT protocol (and this is why it uses ttyUSB0). To have more information, you should run ModemManager in debug mode and provide the logs ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: MC7455 sporadically wrong primary port
Hi Karoly, which modem are you using? cdc-wdmX and tty ports are used for totally different communication protocols. Like, if you're modem supports QMI or MBIM, it's ok to use cdc-wdmX, otherwise is ok to use tty. Also consider that is up to the kernel to assign the number at the end of cdc-wdmX and that number depends on which other hardware is connected to USBs. i.e. when I have my usb earphones connected, the kernel assigns cdc-wdm1 to the modem instead of cdc-wdm0. Carlo On 4 July 2017 at 15:36, Karoly Pados wrote: > Hello, > > Sometimes MM picks the wrong primary port. What I observed is that > everything works if it picks cdc-wdm0. But pretty often it picks cdc-wdm1, > in which case NetworkManager is unable to establish a data connection. > Sometimes, much more rarely, it even picks ttyUSB0, and NM is then also > unable to do anything useful with the modem. > > Now I'm not 100% sure that this is the cause for our problems, so my first > question: Is it plausible at all that anything other than cdc-wdm0 as a > primary port is a problem? > > If so: > - What can I provide to help solve this in code? > - Is there a way around it? (for example, a way to force MM to pick a > specific port as primary?) > > Thank you, > Karoly > ___ > ModemManager-devel mailing list > ModemManager-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel > ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH] mm-broadband-modem-mbim: support hot swapping
Hi,
> If I'm not mistaken, whenever a sim insert/removal event is detected, we
should just call
> mm_broadband_modem_update_sim_hot_swap_detected(), which will trigger a
full modem re-probe.
yes, I confirm this. *mm_broadband_modem_update_sim_hot_swap_detected* will
trigger full re-probe and disable current modem.
Here is the code in telit plugin
133 *if* ((prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status
!= QSS_STATUS_SIM_REMOVED) ||
134 (prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status ==
QSS_STATUS_SIM_REMOVED)) {
135 mm_info ("QSS: SIM swap detected");
136 mm_broadband_modem_update_sim_hot_swap_detected (
MM_BROADBAND_MODEM (self));
137 }
The if condition is a bit complex here because we can have 4 different QSS
states, but if we are tracing just 2 or them (SIM IN vs SIM OUT), and if
I'm not wrong here, whenever last_ready_state and ready_state differ,
*mm_broadband_modem_update_sim_hot_swap_detected* should be called
if self->priv->last_ready_state != ready_state:
mm_broadband_modem_update_sim_hot_swap_detected (...)
On 29 June 2017 at 11:08, Aleksander Morgado
wrote:
> On 29/06/17 10:51, Aleksander Morgado wrote:
> >> +if (self->priv->last_ready_state !=
> >> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED
> &&
> >> +ready_state == MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED) {
> >> +/* SIM has been removed */
> >> +mm_iface_modem_update_failed_state (MM_IFACE_MODEM (self),
> >> +
> MM_MODEM_STATE_FAILED_REASON_SIM_MISSING);
> >> +} else if (self->priv->last_ready_state ==
> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED &&
> >> + ready_state !=
> >> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED)
> {
> >> +/* SIM has been reinserted */
> >> +mm_broadband_modem_update_sim_hot_swap_detected
> (MM_BROADBAND_MODEM (self));
> >> +}
> >>
> > If I'm not mistaken, whenever a sim insert/removal event is detected, we
> should just call mm_broadband_modem_update_sim_hot_swap_detected(), which
> will trigger a full modem re-probe. In this case the method is only being
> called for the case where the SIM is inserted, not for when the SIM is
> removed.
> >
>
> Also, could you provide MM debug logs showing the SIM card hot insertion
> and the SIM card hot removal?
>
> --
> Aleksander
> https://aleksander.es
>
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: Re: Issues with modem reset
Hi Aleksander, Piotr,> I'm checking the logs and if my understanding is correct it looks like> some logic from bearer disconnect from the connection before the modem> reset happens still executes after modem was lost and when "new" modem> is probed, it looks like it may get in the way of AT ports probing.> Especially this happens and afterwards it seems PORTCFG reply fails to> parse (or it's retried 3 times for other reason)I don't know if it's the bearer disconnection, but it seems that the modem release is mixed with the new probing: device_context_set_best_plugin(): [plugin manager] task 9,ttyACM0: best plugin matches device reported one: Telit device_context_continue(): [plugin Manager] task 9: still 2 running probes (2 active): ttyACM2, ttyACM1 _close_internal(): (ttyACM0) device open count is 0 (close) _close_internal(): (ttyACM0) closing serial port... _close_internal(): (ttyACM0) serial port closed port_serial_close_force(): (ttyACM0) forced to close port _close_internal(): (ttyACM0) device open count is 0 (close) _close_internal(): (ttyACM0) closing serial port... _close_internal(): (ttyACM0) serial port closed port_serial_close_force(): (ttyACM0) forced to close port finalize(): Modem (Telit) '/sys/devices/soc0/soc.1/210.aips-bus/2184200.usb/ci_hdrc.1/usb1/1-1/1-1.4' completely disposedin the meantime, even if ttyACM0 is replying to #PORTCFG?, the plugin seems unable to receve it and keep retring. debug_log(): (ttyACM0): --> 'AT#PORTCFG?'... debug_log(): (ttyACM0): <-- '' debug_log(): (ttyACM0): <-- '#PORTCFG: 8,8OK' getportcfg_ready(): telit: retrieving port mode layout mm_port_probe_set_result_at(): (tty/ttyACM0) port is AT-capable debug_log(): (ttyACM0): --> 'AT#PORTCFG?' debug_log(): (ttyACM0): <-- '' debug_log(): (ttyACM0): <-- '#PORTCFG: 8,8OK' getportcfg_ready(): telit: retrieving port mode layout mm_port_probe_set_result_at(): (tty/ttyACM0) port is AT-capable debug_log(): (ttyACM0): --> 'AT#PORTCFG?' debug_log(): (ttyACM0): <-- '' debug_log(): (ttyACM0): <-- '#PORTCFG: 8,8OK' getportcfg_ready(): telit: retrieving port mode layout[and here comes the disconnection]finally, the modem set only ttyACM3 as valid port, both at and data log_port(): (/sys/devices/soc0/soc.1/210.aips-bus/2184200.usb/ci_hdrc.1/usb1/1-1/1-1.4) tty/ttyACM3 at (primary) log_port(): (/sys/devices/soc0/soc.1/210.aips-bus/2184200.usb/ci_hdrc.1/usb1/1-1/1-1.4) tty/ttyACM3 data (primary)Not really sure what's happening here, but I do remember a similar issue where adding some new debug logs, it appeared that the reply sent to PORTCFG? parser was an empty string, the thread in this mailing list is "[PATCH] Hardening PORTCFG parse reply" dated mid July 2016. ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH] telit: enable 'SIM ready' notifications with #QSS=2
> In your case with the Telit modem and QSS:3; if we decide that it's > not worth waiting for QSS:3 because it takes too long and for our > purposes we can use it much earlier, then we could do the per-step > retries on SIM busy errors. The problem here is that, according to my tests, even if we try to get this info earlier, polling at some intervals, we end up waiting the same amount of time. > The idea of updating in the background and letting the remaining logic > to flow is a bit anarchic, as the user of the API cannot know when the > property values are done populated. Yes, you're right, I wasn't thinking about that. I've got the idea of dropping this change, actually. As I said, the goal was to have some logic to recover several situations of "sim busy", but I couldn't make a scalable solution that isn't tailored strictly around this specific case and, at the end of the day, it doesn't seem to provide a real improvement, specially if compared to the increased complexity. Finally, this issue affect only the SIMs of a specific vendor and the most important information (SIM and PUK) are always retrieved correctly. ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: QSS transitions additional patches
On 5 June 2017 at 08:49, Carlo Lobrano wrote: > sure, I'll do it today I replied in the patches' thread ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH 2/2] telit: rework QSS unsolicited handler enabling logic
Hi Aleksander,
I tested this patch and it looks good to me (I specially like the second
one). There is just a little mistake I made in my part of the patch. In the
"status changed" log I inverted current status with the previous one.
---
plugins/telit/mm-broadband-modem-telit.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/plugins/telit/mm-broadband-modem-telit.c
b/plugins/telit/mm-broadband-modem-telit.c
index 7c93c0d..e78a195 100644
--- a/plugins/telit/mm-broadband-modem-telit.c
+++ b/plugins/telit/mm-broadband-modem-telit.c
@@ -125,9 +125,9 @@ telit_qss_unsolicited_handler (MMPortSerialAt *port,
self->priv->qss_status = cur_qss_status;
if (cur_qss_status != prev_qss_status)
-mm_dbg ("QSS: status changed '%s -> %s",
-mm_telit_qss_status_get_string (cur_qss_status),
-mm_telit_qss_status_get_string (prev_qss_status));
+mm_dbg ("QSS: status changed %s -> %s",
+mm_telit_qss_status_get_string (prev_qss_status),
+mm_telit_qss_status_get_string (cur_qss_status));
if ((prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status !=
QSS_STATUS_SIM_REMOVED) ||
(prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status ==
QSS_STATUS_SIM_REMOVED)) {
--
Can I submit a little patch just for that or do you prefer doing otherwise?
On 2 June 2017 at 13:10, Aleksander Morgado
wrote:
> When the async method starts we store already the primary and the
> optional secondary port objects in the method context, keeping a full
> reference for each.
>
> When we parse the response for the enabling command, we just reuse the
> stored port objects, instead of re-querying the modem to get them, and
> this makes sure that the port objects where we want to set the
> unsolicited message handlers are still valid. If the ports are gone
> in the middle of the enabling operation, the handlers will be set
> without errors, even if the ports may likely get completely disposed
> when this async method context is disposed.
>
> Additionally, we make sure that we return an error also for the case
> where there is no secondary port and the enabling on the primary port
> failed.
>
> This patch also fixes the use of the at_command_full_finish() method to
> complete the at_command_full() async operation, to keep consistency.
> ---
> plugins/telit/mm-broadband-modem-telit.c | 102
> +--
> 1 file changed, 41 insertions(+), 61 deletions(-)
>
> diff --git a/plugins/telit/mm-broadband-modem-telit.c
> b/plugins/telit/mm-broadband-modem-telit.c
> index 2c1c5420..e609bab1 100644
> --- a/plugins/telit/mm-broadband-modem-telit.c
> +++ b/plugins/telit/mm-broadband-modem-telit.c
> @@ -103,6 +103,8 @@ typedef enum {
>
> typedef struct {
> QssSetupStep step;
> +MMPortSerialAt *primary;
> +MMPortSerialAt *secondary;
> GError *primary_error;
> GError *secondary_error;
> } QssSetupContext;
> @@ -138,6 +140,8 @@ telit_qss_unsolicited_handler (MMPortSerialAt *port,
> static void
> qss_setup_context_free (QssSetupContext *ctx)
> {
> +g_clear_object (&(ctx->primary));
> +g_clear_object (&(ctx->secondary));
> g_clear_error (&(ctx->primary_error));
> g_clear_error (&(ctx->secondary_error));
> g_slice_free (QssSetupContext, ctx);
> @@ -156,60 +160,37 @@ telit_qss_enable_ready (MMBaseModem *modem,
> GAsyncResult *res,
> GTask *task)
> {
> -GError *error = NULL;
> MMBroadbandModemTelit *self;
> QssSetupContext *ctx;
> -MMPortSerialAt *primary;
> -MMPortSerialAt *secondary;
> +MMPortSerialAt *port;
> +GError **error;
> GRegex *pattern;
>
> self = MM_BROADBAND_MODEM_TELIT (g_task_get_source_object (task));
> ctx = g_task_get_task_data (task);
>
> -mm_base_modem_at_command_finish (modem, res, &error);
> -if (error) {
> -if (ctx->step == QSS_SETUP_STEP_ENABLE_PRIMARY_PORT) {
> -mm_warn ("QSS: error enabling unsolicited on primary port:
> %s", error->message);
> -ctx->primary_error = error;
> -} else if (ctx->step == QSS_SETUP_STEP_ENABLE_SECONDARY_PORT) {
> -mm_warn ("QSS: error enabling unsolicited on secondary port:
> %s", error->message);
> -ctx->secondary_error = error;
> -} else {
> -g_assert_not_reached ();
> -}
> +if (ctx->step == QSS_SETUP_STEP_ENABLE_PRIMARY_PORT) {
> +port = ctx->primary;
> +error = &ctx->primary_error;
> +} else if (ctx->step == QSS_SETUP_STEP_ENABLE_SECONDARY_PORT) {
> +port = ctx->secondary;
> +error = &ctx->secondary_error;
> +} else
> +g_assert_not_reached ();
> +
> +if (!mm_base_modem_at_command_full_finish (modem, res, error)) {
> +mm_warn ("QSS: error enabling unsolicited on port %s: %s",
> mm_port_get_device (MM_PORT (port)), (*error)->message);
> goto next_step;
Re: QSS transitions additional patches
Hi On 2 June 2017 at 13:10, Aleksander Morgado wrote: > Hey Carlo, > > Your v3 looks good, but I ended up preparing a couple of additional > patches to go on top of it; could you review/test them? > > [PATCH 1/2] telit: make sure QSS status values read are always known > [PATCH 2/2] telit: rework QSS unsolicited handler enabling logic > > Cheers! > sure, I'll do it today ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
[PATCH v3] telit: manage QSS transistions
Currently, Telit's SIM swap implementation is stateless and based on #QSS unsolicited messages 0/1 (SIM_REMOVED/SIM_INSERTED). However, the user might have configured the modem in order to provide a more detailed information, with #QSS values 2/3 (SIM UNLOCKED/SIM READY). In this case and with current implementation, even receiving "#QSS: 3" will trigger the "SIM swap" logic. The same issue might occur in other use cases too, i.e. with SIM locked or when the message is received from both USB ports. This patch makes SIM swap implementation stateful, and it considers as an actual SIM swap, only transitions from #QSS: 0 to #QSS: 1/2/3 and vice versa. --- Hey, version 3 of the patch: - glib-mkenums - fixed error return in setting unsolicited handler on both ports - fixed #QSS? reply parser and its tests - removed unused code - improved logs Sorry, it's a long patch, I hope I didn't forget anything --- plugins/Makefile.am | 107 + plugins/telit/mm-broadband-modem-telit.c | 266 +++--- plugins/telit/mm-modem-helpers-telit.c| 20 ++ plugins/telit/mm-modem-helpers-telit.h| 12 + plugins/telit/tests/test-mm-modem-helpers-telit.c | 38 5 files changed, 327 insertions(+), 116 deletions(-) diff --git a/plugins/Makefile.am b/plugins/Makefile.am index 2a4ff2b..2b67349 100644 --- a/plugins/Makefile.am +++ b/plugins/Makefile.am @@ -225,45 +225,6 @@ NOVATEL_COMMON_COMPILER_FLAGS = -I$(top_srcdir)/plugins/novatel NOVATEL_COMMON_LIBADD_FLAGS = $(builddir)/libmm-utils-novatel.la -# common telit support - - -# Common telit helpers library -noinst_LTLIBRARIES += libhelpers-telit.la -libhelpers_telit_la_SOURCES = \ - telit/mm-modem-helpers-telit.c \ - telit/mm-modem-helpers-telit.h \ - $(NULL) - -noinst_PROGRAMS += test-modem-helpers-telit -test_modem_helpers_telit_SOURCES = \ - telit/tests/test-mm-modem-helpers-telit.c \ - $(NULL) -test_modem_helpers_telit_CPPFLAGS = \ - -I$(top_srcdir)/plugins/telit \ - $(NULL) -test_modem_helpers_telit_LDADD = \ - $(builddir)/libhelpers-telit.la \ - $(top_builddir)/src/libhelpers.la \ - $(top_builddir)/libmm-glib/libmm-glib.la \ - $(NULL) - -# Common telit modem support library -noinst_LTLIBRARIES += libmm-utils-telit.la -libmm_utils_telit_la_SOURCES = \ - telit/mm-common-telit.c \ - telit/mm-common-telit.h \ - telit/mm-broadband-modem-telit.c \ - telit/mm-broadband-modem-telit.h \ - $(NULL) - -TELIT_COMMON_COMPILER_FLAGS = -I$(top_srcdir)/plugins/telit -TELIT_COMMON_LIBADD_FLAGS = \ - $(builddir)/libhelpers-telit.la \ - $(builddir)/libmm-utils-telit.la \ - $(NULL) - - # plugin: generic @@ -818,6 +779,46 @@ libmm_plugin_via_la_LDFLAGS = $(PLUGIN_COMMON_LINKER_FLAGS) # plugin: telit +noinst_LTLIBRARIES += libhelpers-telit.la + +TELIT_ENUMS_INPUTS = \ + $(top_srcdir)/plugins/telit/mm-modem-helpers-telit.h \ + $(NULL) + +TELIT_ENUMS_GENERATED = \ + telit/mm-telit-enums-types.h \ + telit/mm-telit-enums-types.c \ + $(NULL) + +telit/mm-telit-enums-types.h: Makefile.am $(TELIT_ENUMS_INPUTS) $(top_srcdir)/build-aux/mm-enums-template.h + $(AM_V_GEN) \ + $(MKDIR_P) telit; \ + $(GLIB_MKENUMS) \ + --fhead "#include \"mm-modem-helpers-telit.h\"\n#ifndef __MM_TELIT_ENUMS_TYPES_H__\n#define __MM_TELIT_ENUMS_TYPES_H__\n" \ + --template $(top_srcdir)/build-aux/mm-enums-template.h \ + --ftail "#endif /* __MM_TELIT_ENUMS_TYPES_H__ */\n" \ + $(TELIT_ENUMS_INPUTS) > $@ + +telit/mm-telit-enums-types.c: Makefile.am $(top_srcdir)/build-aux/mm-enums-template.c telit/mm-telit-enums-types.h + $(AM_V_GEN) \ + $(MKDIR_P) telit; \ + $(GLIB_MKENUMS) \ + --fhead "#include \"mm-telit-enums-types.h\"" \ + --template $(top_srcdir)/build-aux/mm-enums-template.c \ + $(TELIT_ENUMS_INPUTS) > $@ + +libhelpers_telit_la_SOURCES = \ + telit/mm-modem-helpers-telit.c \ + telit/mm-modem-helpers-telit.h \ + $(NULL) + +nodist_libhelpers_telit_la_SOURCES = $(TELIT_ENUMS_GENERATED) + +libhelpers_telit_la_CPPFLAGS = $(PLUGIN_TELIT_COMPILER_FLAGS) + +BUILT_SOURCES += $(TELIT_ENUMS_GENERATED) +CLEANFILES+= $(TELIT_ENUMS_GENERATED) + pkglib_LTLIBRARIES += libmm-plugin-telit.la libmm_plugin_telit_la_SOURCES = \ telit/mm-plugin-telit.
[PATCH v2] telit: manage QSS transistions
Currently, Telit's SIM swap implementation is stateless and
based on #QSS unsolicited messages 0/1 (SIM_REMOVED/SIM_INSERTED).
However, the user might have configured the modem in order to provide a
more detailed information, with #QSS values 2/3 (SIM UNLOCKED/SIM READY).
In this case and with current implementation, even receiving "#QSS: 3" will
trigger the "SIM swap" logic. The same issue might occur in other use cases
too, i.e. with SIM locked or when the message is received from both USB
ports.
This patch makes SIM swap implementation stateful, and it considers as an
actual SIM swap, only transitions from #QSS: 0 to #QSS: 1/2/3 and vice
versa.
---
Changes according to code review:
- used GTask
- added helper method for "#QSS?" parsing
- added tests for "#QSS?" parser
- enabled unsolicited on both ports
---
plugins/telit/mm-broadband-modem-telit.c | 267 +++---
plugins/telit/mm-modem-helpers-telit.c| 21 ++
plugins/telit/mm-modem-helpers-telit.h| 11 +
plugins/telit/tests/test-mm-modem-helpers-telit.c | 40
4 files changed, 261 insertions(+), 78 deletions(-)
diff --git a/plugins/telit/mm-broadband-modem-telit.c
b/plugins/telit/mm-broadband-modem-telit.c
index 13ca4a5..b68ff6d 100644
--- a/plugins/telit/mm-broadband-modem-telit.c
+++ b/plugins/telit/mm-broadband-modem-telit.c
@@ -50,6 +50,7 @@ typedef enum {
struct _MMBroadbandModemTelitPrivate {
FeatureSupport csim_lock_support;
+QssStatus qss_status;
};
/*/
@@ -90,49 +91,57 @@ modem_after_sim_unlock (MMIfaceModem *self,
/*/
/* Setup SIM hot swap (Modem interface) */
+typedef enum {
+QSS_SETUP_STEP_FIRST,
+QSS_SETUP_STEP_QUERY,
+QSS_SETUP_STEP_ENABLE_PRIMARY_PORT,
+QSS_SETUP_STEP_ENABLE_SECONDARY_PORT,
+QSS_SETUP_STEP_LAST
+} QssSetupStep;
+
+static const gchar *qss_status_names [] = {
+[QSS_STATUS_SIM_REMOVED] = "QSS_STATUS_SIM_REMOVED",
+[QSS_STATUS_SIM_INSERTED] = "QSS_STATUS_SIM_INSERTED",
+[QSS_STATUS_SIM_INSERTED_AND_UNLOCKED] =
"QSS_STATUS_SIM_INSERTED_AND_UNLOCKED",
+[QSS_STATUS_SIM_INSERTED_AND_READY] =
"QSS_STATUS_SIM_INSERTED_AND_READY",
+};
+
+typedef struct {
+MMBroadbandModemTelit *self;
+QssSetupStep step;
+} QssSetupContext;
+
+static void qss_setup_step (GTask *task);
+
static void
telit_qss_unsolicited_handler (MMPortSerialAt *port,
GMatchInfo *match_info,
MMBroadbandModemTelit *self)
{
-guint qss;
+QssStatus cur_qss_status;
+QssStatus prev_qss_status;
-if (!mm_get_uint_from_match_info (match_info, 1, &qss))
+if (!mm_get_uint_from_match_info (match_info, 1, &cur_qss_status))
return;
-switch (qss) {
-case 0:
-mm_info ("QSS: SIM removed");
-break;
-case 1:
-mm_info ("QSS: SIM inserted");
-break;
-case 2:
-mm_info ("QSS: SIM inserted and PIN unlocked");
-break;
-case 3:
-mm_info ("QSS: SIM inserted and PIN locked");
-break;
-default:
-mm_warn ("QSS: unknown QSS value %d", qss);
-break;
-}
+prev_qss_status = self->priv->qss_status;
+self->priv->qss_status = cur_qss_status;
-mm_broadband_modem_update_sim_hot_swap_detected (MM_BROADBAND_MODEM
(self));
-}
+mm_dbg ("QSS: got '%s' status (previously it was '%s')",
+qss_status_names[cur_qss_status],
+qss_status_names[prev_qss_status]);
-typedef struct {
-MMBroadbandModemTelit *self;
-GSimpleAsyncResult *result;
-} ToggleQssUnsolicitedContext;
+if ((prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status !=
QSS_STATUS_SIM_REMOVED) ||
+(prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status ==
QSS_STATUS_SIM_REMOVED)) {
+mm_info ("QSS: SIM swap detected");
+mm_broadband_modem_update_sim_hot_swap_detected (MM_BROADBAND_MODEM
(self));
+}
+}
static void
-toggle_qss_unsolicited_context_complete_and_free (ToggleQssUnsolicitedContext
*ctx)
+qss_setup_context_free (QssSetupContext *ctx)
{
-g_simple_async_result_complete (ctx->result);
-g_object_unref (ctx->result);
-g_object_unref (ctx->self);
-g_slice_free (ToggleQssUnsolicitedContext, ctx);
+g_slice_free (QssSetupContext, ctx);
}
static gboolean
@@ -140,54 +149,104 @@ modem_setup_sim_hot_swap_finish (MMIfaceModem *self,
GAsyncResult *res,
GError **error)
{
-return !g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT
(res), error);
+return g_task_propagate_boolean (G_TASK (res), error);
+}
+
+static void
+telit_qss_query_ready (MMBaseModem *modem,
+ GAsyncResult *re
Re: [PATCH 2/2] dell: speed probing time up and reduce udev dependency
Awesome! Thank you!
Carlo
On 29 May 2017 at 12:27, Aleksander Morgado
wrote:
> On 29/05/17 09:48, Carlo Lobrano wrote:
> > Currently Dell plugin implements a retry logic of three commands
> > (AT+GMI, AT+CGMI, ATI1I2I3) to detect modem's vendor string.
> > Moreover, since Telit modems always reply to the first command, to speed
> > the probing time up, those modem are tagged with an Udev rule so that we
> > can avoid sending the other two commands at all.
> >
> > However, the retry logic is in case a port needs some time to reply, so
> > it makes sense to apply it only to the first command. Then if the port
> still
> > does not respond with any kind of reply, that probably means that it is
> not
> > AT capable and we can skip the other AT commands as well.
> >
> > Then, this patch:
> > - sets a maximum number of timeouts for AT+GMI to 3. After this
> > timeouts, the port is considered not AT-capable.
> > - sets AT+CGMI and ATI1I2I3 to be sent only once.
> > - removes Dell udev rule for tagging Telit Modems.
> >
> > ---
> >
> > Patch v2:
> >
> > As per Aleksander's code review
> >
> > - mm_port_probe_set_result_at set to false when run out of retries for
> manufacturer (and yes, I do confirm that without this line, I saw other
> "AT" commands sent to the ports).
> > - fixed debug logs
> >
> > @Aleksander I'm going to reply to this email with the logs you requested.
> >
>
> Pushed to git master, thanks!
>
> The logs look good to be, btw, thanks for sending them.
>
> > ---
> > plugins/dell/77-mm-dell-port-types.rules | 4
> > plugins/dell/mm-plugin-dell.c| 29
> ++---
> > 2 files changed, 18 insertions(+), 15 deletions(-)
> >
> > diff --git a/plugins/dell/77-mm-dell-port-types.rules
> b/plugins/dell/77-mm-dell-port-types.rules
> > index bbb59b9..206fb6e 100644
> > --- a/plugins/dell/77-mm-dell-port-types.rules
> > +++ b/plugins/dell/77-mm-dell-port-types.rules
> > @@ -8,9 +8,5 @@ GOTO="mm_dell_port_types_end"
> > LABEL="mm_dell_vendorcheck"
> > SUBSYSTEMS=="usb", ATTRS{bInterfaceNumber}=="?*",
> ENV{.MM_USBIFNUM}="$attr{bInterfaceNumber}"
> >
> > -# DW5580 is a Dell-branded Telit modem
> > -# tag is needed here for dynamic port recognition
> > -ATTRS{idVendor}=="413c", ATTRS{idProduct}=="81ba",
> ENV{ID_MM_TELIT_PORTS_TAGGED}="1"
> > -
> > GOTO="mm_dell_port_types_end"
> > LABEL="mm_dell_port_types_end"
> > diff --git a/plugins/dell/mm-plugin-dell.c
> b/plugins/dell/mm-plugin-dell.c
> > index f9dfd1a..fbb0a9c 100644
> > --- a/plugins/dell/mm-plugin-dell.c
> > +++ b/plugins/dell/mm-plugin-dell.c
> > @@ -45,6 +45,8 @@
> > #include "mm-broadband-modem-mbim.h"
> > #endif
> >
> > +#define MAX_PORT_PROBE_TIMEOUTS 3
> > +
> > G_DEFINE_TYPE (MMPluginDell, mm_plugin_dell, MM_TYPE_PLUGIN)
> >
> > MM_PLUGIN_DEFINE_MAJOR_VERSION
> > @@ -70,6 +72,7 @@ typedef struct {
> > guint gmi_retries;
> > guint cgmi_retries;
> > guint ati_retries;
> > +guint timeouts;
> > } CustomInitContext;
> >
> > static void
> > @@ -140,6 +143,8 @@ static void custom_init_step (CustomInitContext
> *ctx);
> > static void
> > custom_init_step_next_command (CustomInitContext *ctx)
> > {
> > +ctx->timeouts = 0;
> > +
> > if (ctx->gmi_retries > 0)
> > ctx->gmi_retries = 0;
> > else if (ctx->cgmi_retries > 0)
> > @@ -169,6 +174,7 @@ response_ready (MMPortSerialAt *port,
> > return;
> > }
> > /* Directly retry same command on timeout */
> > +ctx->timeouts++;
> > custom_init_step (ctx);
> > g_error_free (error);
> > return;
> > @@ -269,6 +275,13 @@ custom_init_step (CustomInitContext *ctx)
> > }
> > #endif
> >
> > +if (ctx->timeouts >= MAX_PORT_PROBE_TIMEOUTS) {
> > +mm_dbg ("(Dell) couldn't detect real manufacturer in (%s): too
> many timeouts",
> > +mm_port_get_device (MM_PORT (ctx->port)));
> > +mm_port_probe_set_result_at (ctx->probe, FALSE);
> > +goto out;
> > +}
> > +
> > if (ctx->gmi_retries > 0) {
> > ctx->gmi_retries--;
> > mm_port_serial_at_command (ctx->p
[PATCH 2/2] dell: speed probing time up and reduce udev dependency
Currently Dell plugin implements a retry logic of three commands
(AT+GMI, AT+CGMI, ATI1I2I3) to detect modem's vendor string.
Moreover, since Telit modems always reply to the first command, to speed
the probing time up, those modem are tagged with an Udev rule so that we
can avoid sending the other two commands at all.
However, the retry logic is in case a port needs some time to reply, so
it makes sense to apply it only to the first command. Then if the port still
does not respond with any kind of reply, that probably means that it is not
AT capable and we can skip the other AT commands as well.
Then, this patch:
- sets a maximum number of timeouts for AT+GMI to 3. After this
timeouts, the port is considered not AT-capable.
- sets AT+CGMI and ATI1I2I3 to be sent only once.
- removes Dell udev rule for tagging Telit Modems.
---
Patch v2:
As per Aleksander's code review
- mm_port_probe_set_result_at set to false when run out of retries for
manufacturer (and yes, I do confirm that without this line, I saw other "AT"
commands sent to the ports).
- fixed debug logs
@Aleksander I'm going to reply to this email with the logs you requested.
---
plugins/dell/77-mm-dell-port-types.rules | 4
plugins/dell/mm-plugin-dell.c| 29 ++---
2 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/plugins/dell/77-mm-dell-port-types.rules
b/plugins/dell/77-mm-dell-port-types.rules
index bbb59b9..206fb6e 100644
--- a/plugins/dell/77-mm-dell-port-types.rules
+++ b/plugins/dell/77-mm-dell-port-types.rules
@@ -8,9 +8,5 @@ GOTO="mm_dell_port_types_end"
LABEL="mm_dell_vendorcheck"
SUBSYSTEMS=="usb", ATTRS{bInterfaceNumber}=="?*",
ENV{.MM_USBIFNUM}="$attr{bInterfaceNumber}"
-# DW5580 is a Dell-branded Telit modem
-# tag is needed here for dynamic port recognition
-ATTRS{idVendor}=="413c", ATTRS{idProduct}=="81ba",
ENV{ID_MM_TELIT_PORTS_TAGGED}="1"
-
GOTO="mm_dell_port_types_end"
LABEL="mm_dell_port_types_end"
diff --git a/plugins/dell/mm-plugin-dell.c b/plugins/dell/mm-plugin-dell.c
index f9dfd1a..fbb0a9c 100644
--- a/plugins/dell/mm-plugin-dell.c
+++ b/plugins/dell/mm-plugin-dell.c
@@ -45,6 +45,8 @@
#include "mm-broadband-modem-mbim.h"
#endif
+#define MAX_PORT_PROBE_TIMEOUTS 3
+
G_DEFINE_TYPE (MMPluginDell, mm_plugin_dell, MM_TYPE_PLUGIN)
MM_PLUGIN_DEFINE_MAJOR_VERSION
@@ -70,6 +72,7 @@ typedef struct {
guint gmi_retries;
guint cgmi_retries;
guint ati_retries;
+guint timeouts;
} CustomInitContext;
static void
@@ -140,6 +143,8 @@ static void custom_init_step (CustomInitContext *ctx);
static void
custom_init_step_next_command (CustomInitContext *ctx)
{
+ctx->timeouts = 0;
+
if (ctx->gmi_retries > 0)
ctx->gmi_retries = 0;
else if (ctx->cgmi_retries > 0)
@@ -169,6 +174,7 @@ response_ready (MMPortSerialAt *port,
return;
}
/* Directly retry same command on timeout */
+ctx->timeouts++;
custom_init_step (ctx);
g_error_free (error);
return;
@@ -269,6 +275,13 @@ custom_init_step (CustomInitContext *ctx)
}
#endif
+if (ctx->timeouts >= MAX_PORT_PROBE_TIMEOUTS) {
+mm_dbg ("(Dell) couldn't detect real manufacturer in (%s): too many
timeouts",
+mm_port_get_device (MM_PORT (ctx->port)));
+mm_port_probe_set_result_at (ctx->probe, FALSE);
+goto out;
+}
+
if (ctx->gmi_retries > 0) {
ctx->gmi_retries--;
mm_port_serial_at_command (ctx->port,
@@ -309,9 +322,10 @@ custom_init_step (CustomInitContext *ctx)
return;
}
-/* Finish custom_init */
-mm_dbg ("(Dell) couldn't flip secondary port to AT in (%s): all retries
consumed",
+mm_dbg ("(Dell) couldn't detect real manufacturer in (%s): all retries
consumed",
mm_port_get_device (MM_PORT (ctx->port)));
+out:
+/* Finish custom_init */
g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
custom_init_context_complete_and_free (ctx);
}
@@ -337,15 +351,8 @@ dell_custom_init (MMPortProbe *probe,
ctx->port = g_object_ref (port);
ctx->cancellable = cancellable ? g_object_ref (cancellable) : NULL;
ctx->gmi_retries = 3;
-ctx->cgmi_retries = 3;
-ctx->ati_retries = 3;
-
-/* Dell-branded Telit modems always answer to +GMI
- * Avoid +CGMI and ATI sending for minimizing port probing time */
-if (mm_kernel_device_get_global_property_as_boolean (port_device,
"ID_MM_TELIT_PORTS_TAGGED")) {
-ctx->cgmi_retries = 0;
-ctx->ati_retries = 0;
-}
+ctx->cgmi_retries = 1;
+ctx->ati_retries = 1;
custom_init_step (ctx);
}
--
2.9.3
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH] telit: manage QSS transistions
> We're trying to move away from GSimpleAsyncResult, how about using > GTask? If you have any doubt or not sure how to do it, leave it with > GSimpleAsyncResult and I'll do the port to GTask as a follow up patch, > so that you can see what the changes were; it really is very easy, you > can check Ben's last emails doing lots of these GTask ports. Sure, I'll try myself first ;) > We usually set unsolicited messages in both primary and secondary > always, and given that now you've implemented stateful transitions for > QSS, we could have them in both. What do you think? makes sense, as well as using QSS_STATUS_SIM_UNKNOWN ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
[PATCH] telit: manage QSS transistions
Currently, Telit's SIM swap implementation is stateless and
based on #QSS unsolicited messages 0/1 (SIM_REMOVED/SIM_INSERTED).
However, the user might have configured the modem in order to provide a
more detailed information, with #QSS values 2/3 (SIM UNLOCKED/SIM READY).
In this case and with current implementation, even receiving "#QSS: 3" will
trigger the "SIM swap" logic. The same issue might occur in other use cases
too, i.e. with SIM locked or when the message is received from both USB
ports.
This patch makes SIM swap implementation stateful, and it considers as an
actual SIM swap, only transitions from #QSS: 0 to #QSS: 1/2/3 and vice
versa.
---
plugins/telit/mm-broadband-modem-telit.c | 162 ---
1 file changed, 125 insertions(+), 37 deletions(-)
diff --git a/plugins/telit/mm-broadband-modem-telit.c
b/plugins/telit/mm-broadband-modem-telit.c
index 13ca4a5..4dde862 100644
--- a/plugins/telit/mm-broadband-modem-telit.c
+++ b/plugins/telit/mm-broadband-modem-telit.c
@@ -48,8 +48,16 @@ typedef enum {
FEATURE_SUPPORTED
} FeatureSupport;
+typedef enum {
+QSS_STATUS_SIM_REMOVED,
+QSS_STATUS_SIM_INSERTED,
+QSS_STATUS_SIM_INSERTED_AND_UNLOCKED,
+QSS_STATUS_SIM_INSERTED_AND_READY
+} QssStatus;
+
struct _MMBroadbandModemTelitPrivate {
FeatureSupport csim_lock_support;
+QssStatus qss_status;
};
/*/
@@ -90,17 +98,34 @@ modem_after_sim_unlock (MMIfaceModem *self,
/*/
/* Setup SIM hot swap (Modem interface) */
+typedef enum {
+QSS_SETUP_STEP_FIRST,
+QSS_SETUP_STEP_QUERY,
+QSS_SETUP_STEP_SET_HANDLER,
+QSS_SETUP_STEP_LAST
+} QssSetupStep;
+
+typedef struct {
+MMBroadbandModemTelit *self;
+GSimpleAsyncResult *result;
+QssSetupStep step;
+} QssSetupContext;
+
+
+static void qss_setup_step (QssSetupContext *ctx);
+
static void
telit_qss_unsolicited_handler (MMPortSerialAt *port,
GMatchInfo *match_info,
MMBroadbandModemTelit *self)
{
-guint qss;
+QssStatus cur_qss_status;
+QssStatus prev_qss_status;
-if (!mm_get_uint_from_match_info (match_info, 1, &qss))
+if (!mm_get_uint_from_match_info (match_info, 1, &cur_qss_status))
return;
-switch (qss) {
+switch (cur_qss_status) {
case 0:
mm_info ("QSS: SIM removed");
break;
@@ -111,28 +136,30 @@ telit_qss_unsolicited_handler (MMPortSerialAt *port,
mm_info ("QSS: SIM inserted and PIN unlocked");
break;
case 3:
-mm_info ("QSS: SIM inserted and PIN locked");
+mm_info ("QSS: SIM inserted and ready");
break;
default:
-mm_warn ("QSS: unknown QSS value %d", qss);
+mm_warn ("QSS: unknown QSS value %d", cur_qss_status);
break;
}
-mm_broadband_modem_update_sim_hot_swap_detected (MM_BROADBAND_MODEM
(self));
-}
+prev_qss_status = self->priv->qss_status;
+self->priv->qss_status = cur_qss_status;
-typedef struct {
-MMBroadbandModemTelit *self;
-GSimpleAsyncResult *result;
-} ToggleQssUnsolicitedContext;
+if ((prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status !=
QSS_STATUS_SIM_REMOVED) ||
+(prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status ==
QSS_STATUS_SIM_REMOVED)) {
+mm_dbg ("QSS: SIM swapped (QSS status changed %d -> %d)",
prev_qss_status, cur_qss_status);
+mm_broadband_modem_update_sim_hot_swap_detected (MM_BROADBAND_MODEM
(self));
+}
+}
static void
-toggle_qss_unsolicited_context_complete_and_free (ToggleQssUnsolicitedContext
*ctx)
+toggle_qss_unsolicited_context_complete_and_free (QssSetupContext *ctx)
{
g_simple_async_result_complete (ctx->result);
g_object_unref (ctx->result);
g_object_unref (ctx->self);
-g_slice_free (ToggleQssUnsolicitedContext, ctx);
+g_slice_free (QssSetupContext, ctx);
}
static gboolean
@@ -144,19 +171,50 @@ modem_setup_sim_hot_swap_finish (MMIfaceModem *self,
}
static void
-telit_qss_toggle_ready (MMBaseModem *self,
+telit_qss_query_ready (MMBaseModem *self,
+ GAsyncResult *res,
+ QssSetupContext *ctx)
+{
+GError *error = NULL;
+const gchar *response;
+const gchar *str;
+QssStatus qss_status;
+guint qss_mode;
+
+response = mm_base_modem_at_command_finish (self, res, &error);
+if (error) {
+mm_warn ("Could not get \"#QSS?\" reply.");
+g_error_free (error);
+goto next_step;
+}
+
+str = mm_strip_tag (response, "#QSS: ");
+if (!sscanf (str, "%d,%d", &qss_mode, (guint*)&qss_status)) {
+mm_err ("Could not parse \"#QSS?\" response: %s", response);
+goto next_step;
+}
+
+ctx->self->priv->qss_status = qss_status;
+
Re: [PATCH] telit: enable 'SIM ready' notifications with #QSS=2
On May 25, 2017 3:17 PM, "Carlo Lobrano" wrote: Hi, > What I was thinking regarding this issue was to define a common error > id, like "MM_CORE_ERROR_RETRY_LATER". If the logic (e.g. MMIfaceModem > logic) receives such an error, it would re-schedule the execution of > that step later on, in X seconds, but would still continue with the > remaining steps in the logic it has. But this is really very specific to > what we're loading, as not all steps may be retried later. working on this again. I implemented the "retry in N seconds" logic inside mm-iface-modem.c taking "UPDATE_LOCK_INFO_CONTEXT_STEP_LOCK" as example, but now I'm considering whether (A) "UPDATE_LOCK_INFO_CONTEXT_STEP_AFTER_UNLOCK" should stop the whole "update_lock_info_context_step" process as "UPDATE_LOCK_INFO_CONTEXT_STEP_LOCK" does, or (B) I can let the process going on and update the unlock retries in the background. The differences I can see are that (A) with some SIMs the "update_lock_info_context_step" would take ~30 seconds to complete (it basically waits till QSS 3 arrives), while (B) some requests to the SIM might be blocked (because of CSIM lock). I've tried both, but I didn't see any other issues with this two solutions. What do you think? Moreover, I didn't really see any problem with CSIM lock, while waiting that much for having the modem available is a problem On 16 May 2017 at 12:33, Carlo Lobrano wrote: > > On 16 May 2017 at 10:53, Aleksander Morgado > wrote: > >> But g_simple_async_result_complete() can only be called once; as soon >> as you call it the async call would be completed, not sure what you do >> with the copy afterwards. Maybe I'm not understanding it properly? >> > > No, you got it right, I was looking for a way to re-use the object for > the pointer to the callback and some other data that at the point where I > could re-call the load_unlock_step were not available anymore. That said, I > kind of knew I was doing something wrong > > > ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH] telit: enable 'SIM ready' notifications with #QSS=2
Hi, > What I was thinking regarding this issue was to define a common error > id, like "MM_CORE_ERROR_RETRY_LATER". If the logic (e.g. MMIfaceModem > logic) receives such an error, it would re-schedule the execution of > that step later on, in X seconds, but would still continue with the > remaining steps in the logic it has. But this is really very specific to > what we're loading, as not all steps may be retried later. working on this again. I implemented the "retry in N seconds" logic inside mm-iface-modem.c taking "UPDATE_LOCK_INFO_CONTEXT_STEP_LOCK" as example, but now I'm considering whether (A) "UPDATE_LOCK_INFO_CONTEXT_STEP_AFTER_UNLOCK" should stop the whole "update_lock_info_context_step" process as "UPDATE_LOCK_INFO_CONTEXT_STEP_LOCK" does, or (B) I can let the process going on and update the unlock retries in the background. The differences I can see are that (A) with some SIMs the "update_lock_info_context_step" would take ~30 seconds to complete (it basically waits till QSS 3 arrives), while (B) some requests to the SIM might be blocked (because of CSIM lock). I've tried both, but I didn't see any other issues with this two solutions. What do you think? On 16 May 2017 at 12:33, Carlo Lobrano wrote: > > On 16 May 2017 at 10:53, Aleksander Morgado > wrote: > >> But g_simple_async_result_complete() can only be called once; as soon >> as you call it the async call would be completed, not sure what you do >> with the copy afterwards. Maybe I'm not understanding it properly? >> > > No, you got it right, I was looking for a way to re-use the object for > the pointer to the callback and some other data that at the point where I > could re-call the load_unlock_step were not available anymore. That said, I > kind of knew I was doing something wrong > > > ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH] broadband-modem: don't create sim hot swap ports context multiple times
Ciao Aleksander,
On 22 May 2017 at 13:18, Aleksander Morgado
wrote:
> ---
>
> Hey Carlo,
>
> It's never too late :)
>
> I couldn't reproduce the issue with the SIM hot swap changes you were
> trying, because none of my Telit modems here were able to support it
> properly, so I ended up re-reviewing the logs you sent and I believe I
> found the issue.
>
> When the SIM is already unlocked, the "initialization" sequence is run
> only once. When the SIM is locked, though, the "initialization" sequence is
> run twice: one until it detects that the SIM is locked, and the second one
> after the SIM is unlocked. The INITIALIZE_STEP_SIM_HOT_SWAP step was
> therefore running twice and unconditionally creating a new PortsContext
> each time, and that would have a nice side-effect apart from the memleak:
> the "port open count" was incorrectly increased by one for the TTYs...
>
> When SIM was removed ModemManager would close and remove all ports. These
> are the logs when we have run the initialization sequence only once, you
> can see that the open count goes to 0 for both ttyACM0 and ttyACM3:
> (ttyACM3) device open count is 1 (close)
> (ttyACM0) device open count is 0 (close)
> (ttyACM0) closing serial port...
> (ttyACM0) serial port closed
> (ttyACM3) device open count is 0 (close)
> (ttyACM3) closing serial port...
> (ttyACM3) serial port closed
> Modem /org/freedesktop/ModemManager1/Modem/0: state changed
> (disabling -> disabled)
> [device /sys/devices/pci:00/:00:1a.7/usb1/1-3]
> unexported modem from path '/org/freedesktop/ModemManager1/Modem/0'
> (ttyACM3) forced to close port
> (ttyACM0) forced to close port
>
> These are the logs when we have run the initialization sequence twice and
> the PortsContext was created twice, you can see how the open count didn't
> go to 0:
> Modem /org/freedesktop/ModemManager1/Modem/0: 3GPP
> Registration state changed (home -> unknown)
> (ttyACM3) device open count is 2 (close)
> (ttyACM0) device open count is 1 (close)
> (ttyACM3) device open count is 1 (close)
> Modem /org/freedesktop/ModemManager1/Modem/0: state changed
> (disabling -> disabled)
> [device /sys/devices/pci:00/:00:1a.7/usb1/1-3]
> unexported modem from path '/org/freedesktop/ModemManager1/Modem/0'
>
> When the modem was re-created you said you would get NULL strings as
> response in the AT commands sent, even if you saw the responses in the
> logs.. well, it was the port objects from the previous modem run, which
> were not fully closed, the ones receiving the responses :) Your new serial
> port objects in the new modem run would fail and get NULL strings as
> response as you said.
>
> Could you take a look at the patch, which just avoids to re-create the
> ports context, and see if it really fixes the issue you were seeing in your
> tests?
>
> Cheers!
>
This is weird, but I couldn't reproduce the "modem not recognized" part
myself.
What I can reproduce is the crash, and I can also confirm that this patch
fix it very well. Thank you!
>
> ---
> src/mm-broadband-modem.c | 10 +++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
> index 4a14d33d..68242e5c 100644
> --- a/src/mm-broadband-modem.c
> +++ b/src/mm-broadband-modem.c
> @@ -10154,7 +10154,9 @@ initialize_step (InitializeContext *ctx)
> return;
>
> case INITIALIZE_STEP_SIM_HOT_SWAP:
> -{
> +/* Create the SIM hot swap ports context only if not already done
> before
> + * (we may be re-running the initialization step after SIM-PIN
> unlock) */
> +if (!ctx->self->priv->sim_hot_swap_ports_ctx) {
> gboolean is_sim_hot_swap_supported = FALSE;
>
> g_object_get (ctx->self,
> @@ -10165,7 +10167,7 @@ initialize_step (InitializeContext *ctx)
> PortsContext *ports;
> GError *error = NULL;
>
> -mm_dbg ("Creating PortsContext for SIM hot swap.");
> +mm_dbg ("Creating ports context for SIM hot swap");
>
> ports = g_new0 (PortsContext, 1);
> ports->ref_count = 1;
> @@ -10178,7 +10180,9 @@ initialize_step (InitializeContext *ctx)
>
> ports_context_unref (ports);
> }
> -}
> +} else
> +mm_dbg ("Ports context for SIM hot swap already available");
> +
> /* Fall down to next step */
> ctx->step++;
>
> --
> 2.12.2
>
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: Generic +CMER setting logic
On 22 May 2017 at 17:15, Aleksander Morgado wrote: > Maybe, yes, although that shouldn't be an issue in most cases. I see. Anyway, the patch works fine for me, no issues observed. ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: Generic +CMER setting logic
Hey, > Carlo, any chance you can make a quick test with these patches to see > if they work as expected in the Telit modems? One of the changes I > see is that the Telit plugin previously did the AT+CMER command only > in the secondary port, while now we do it in both primary and > secondary, not sure if that's an issue? I guess the reason is that if the primary port is used for data (i.e. PPPD) notification won't arrive or will be buffered, according to the parameters passed to +CMER. If we send +CMER in both ports, are we gonna receive URCs twice? However, I'm not aware of any different behavior of Telit plugins respect this case, so if it's fine for the other plugins, I think it's ok for Telit one too. I'll try the patch and keep you updated On 21 May 2017 at 14:54, Aleksander Morgado wrote: > Hey Dan, Carlo & everyone, > > This patch series implements the checking of the AT+CMER command > parameters so that we can enable or disable them without error. The parser > only checks the 'mode' (1st field) and 'ind' (4th field) settings, as those > are the ones we request to update. > > Also modified the Cinterion and Telit plugins to avoid requiring custom > AT+CMER settings (they had one because some devices didn't support ind=1, > only ind=2). I've also seen the case where AT+CMER cannot even be disabled > (mode=1 the only one allowed, as in the EM7345), and in that case we just > skip the command. > > Carlo, any chance you can make a quick test with these patches to see if > they work as expected in the Telit modems? One of the changes I see is that > the Telit plugin previously did the AT+CMER command only in the secondary > port, while now we do it in both primary and secondary, not sure if that's > an issue? > > Cheers! > > [PATCH 1/4] helpers: new 'AT+CMER=?' parser > [PATCH 2/4] broadband-modem: query +CMER format before > enabling/disabling > [PATCH 3/4] cinterion: remove custom +CMER enabling > [PATCH 4/4] telit: remove custom +CMER enabling > ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH] telit: enable 'SIM ready' notifications with #QSS=2
On 16 May 2017 at 10:53, Aleksander Morgado wrote: > But g_simple_async_result_complete() can only be called once; as soon > as you call it the async call would be completed, not sure what you do > with the copy afterwards. Maybe I'm not understanding it properly? > No, you got it right, I was looking for a way to re-use the object for the pointer to the callback and some other data that at the point where I could re-call the load_unlock_step were not available anymore. That said, I kind of knew I was doing something wrong ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH] telit: enable 'SIM ready' notifications with #QSS=2
On 15 May 2017 at 17:15, Aleksander Morgado wrote: > Not sure I understand; how does the call complete if the > GSimpleAsyncResult isn't completed? > I save a copy of the instance in a GList, then complete the call ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH] telit: enable 'SIM ready' notifications with #QSS=2
> Hum... not sure this approach is the correct one. If you store the > GSimpleAsyncResult and not complete it the whole logic in the > MMIfaceModem will get stuck until it is completed afterwards. Ok, the approach might still be wrong (and even more wrong :D), but I only save the GSimpleAsyncResult for later, I do not stop the call, which is completed with the usual error messages. > What I was thinking regarding this issue was to define a common error > id, like "MM_CORE_ERROR_RETRY_LATER". If the logic (e.g. MMIfaceModem > logic) receives such an error, it would re-schedule the execution of > that step later on, in X seconds, but would still continue with the > remaining steps in the logic it has. But this is really very specific to > what we're loading, as not all steps may be retried later. > > For the specific case of loading unlock retries... does it make sense > for us to report e.g. that the modem is "locked" but without being able > to report the unlock retries left after some time later? From a user > point of view, I'm sure that an application that receives notification > that the modem is locked would expect right away to know the amount of > unlock retries left, if we report that several seconds later the > information will likely not be used at all. > > So I'm not sure whether this makes sense; waiting for "QSS: 3" > guarantees that we'll be able to read unlock retries; but maybe it's > just cleaner to perform several retries trying to load them as that may > be quicker than waiting for "QSS: 3"... it really is very unfortunate > that this notification takes so long to be emitted :/ I see. I've seen a similar approach in port probing, but I thought that retrying before #QSS:3 didn't have much sense. I like the idea of the error code though, and yes you're right saying that this information should arrive as soon as possible, however, consider that this problem occurs only with a specific SIM vendor and only for PIN2 and PUK2. I'm gonna develop this idea further, since I really like to have something generic and applicable also to other commands that return with SIM Busy error. On 15 May 2017 at 12:23, Aleksander Morgado wrote: > Hey hey, > > On 12/05/17 16:18, Carlo Lobrano wrote: > > > Maybe it wouldn't be a bad idea to keep track of which operations may > >> fail due to SIM being busy, and perform automatic retry later if we > >> get that specific error, something like that. > > Hey, > > > > I made a little proof of concept of this improvement. So far, it's > > restricted to *loading unlock retries again once #QSS:3* is received, > but I > > can't get to have the retries values updated at a higher level. This is > > likely due to *mm-iface-modem.c:load_unlock_retries_ready* callback not > > being called I guess, but I'm not really sure my approach is totally > > correct. > > > > I do the following: > > > > A. When mm_telit_parse_csim_response fails: > > 1. The *GSimpleAsyncResult* instance is saved > > 2. load_unlock_retries is marked to be called again later > > B. When QSS: 3 arrives: > > 1. a newly *LoadUnlockRetriesContext* is created using the saved > > *GSimpleAsyncResult* (the idea is to re-use the original callback > > reference, but not really sure it's correct). > > 2. *load_unlock_retries_step* is called again with the above > mentioned > > LoadUnlockRetriesContext instance > > > > Step 2 is done in a GSourceFunc called by the mainloop, using g_add_idle. > > Hum... not sure this approach is the correct one. If you store the > GSimpleAsyncResult and not complete it the whole logic in the > MMIfaceModem will get stuck until it is completed afterwards. The logic > in MMIfaceModem goes step by step, if one step takes 5 minutes to > complete, the next steps won't go on until that happens. > > What I was thinking regarding this issue was to define a common error > id, like "MM_CORE_ERROR_RETRY_LATER". If the logic (e.g. MMIfaceModem > logic) receives such an error, it would re-schedule the execution of > that step later on, in X seconds, but would still continue with the > remaining steps in the logic it has. But this is really very specific to > what we're loading, as not all steps may be retried later. > > For the specific case of loading unlock retries... does it make sense > for us to report e.g. that the modem is "locked" but without being able > to report the unlock retries left after some time later? From a user > point of view, I'm sure that an application that receives notification > that the modem is locked woul
Re: [PATCH 2/2] dell: speed probing time up and reduce udev dependency
On 15 May 2017 at 12:12, Aleksander Morgado wrote: > Also, any chance you can send debug logs with > the whole process once the patch updated? > Sure, no problem. I'll update the patch and provide the logs ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH] telit: enable 'SIM ready' notifications with #QSS=2
> Maybe it wouldn't be a bad idea to keep track of which operations may > fail due to SIM being busy, and perform automatic retry later if we > get that specific error, something like that. Hey, I made a little proof of concept of this improvement. So far, it's restricted to *loading unlock retries again once #QSS:3* is received, but I can't get to have the retries values updated at a higher level. This is likely due to *mm-iface-modem.c:load_unlock_retries_ready* callback not being called I guess, but I'm not really sure my approach is totally correct. I do the following: A. When mm_telit_parse_csim_response fails: 1. The *GSimpleAsyncResult* instance is saved 2. load_unlock_retries is marked to be called again later B. When QSS: 3 arrives: 1. a newly *LoadUnlockRetriesContext* is created using the saved *GSimpleAsyncResult* (the idea is to re-use the original callback reference, but not really sure it's correct). 2. *load_unlock_retries_step* is called again with the above mentioned LoadUnlockRetriesContext instance Step 2 is done in a GSourceFunc called by the mainloop, using g_add_idle. What I see is the load_unlock_retries_step logs, so this function is actually called (as it looks to me) correctly, but the output of "mmcli -m " still shows the same incomplete list of unlock retries as if the data have not been updated. ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
[PATCH 1/2] telit: removed ID_MM_TELIT_PORTS_TAGGED dependency
Currently, Telit plugin depends on ID_MM_TELIT_PORTS_TAGGED
environment variable, set by udev, for tagging modems that
support dynamic port config (#PORTCFG)
To remove this dependency from udev, Telit plugin now relies
only on the error management of the command AT#PORTCFG? itself
in order to see whether the modem supports it or not.
---
Sending again this part of the patch even without modification, so it should be
easier to apply both parts.
---
plugins/telit/77-mm-telit-port-types.rules | 9 -
plugins/telit/mm-common-telit.c| 10 ++
2 files changed, 2 insertions(+), 17 deletions(-)
diff --git a/plugins/telit/77-mm-telit-port-types.rules
b/plugins/telit/77-mm-telit-port-types.rules
index 01538cb..b052962 100644
--- a/plugins/telit/77-mm-telit-port-types.rules
+++ b/plugins/telit/77-mm-telit-port-types.rules
@@ -33,13 +33,4 @@ ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1010",
ENV{.MM_USBIFNUM}=="03", ENV{
# CE910-DUAL
ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1011", ENV{.MM_USBIFNUM}=="01",
ENV{ID_MM_TELIT_PORT_TYPE_MODEM}="1"
-# HE910, UE910, UL865 (dynamic port identification supported)
-ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="0021",
ENV{ID_MM_TELIT_PORTS_TAGGED}="1"
-
-# GE910 (dynamic port identification supported)
-ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="0022",
ENV{ID_MM_TELIT_PORTS_TAGGED}="1"
-
-# LE910 V2
-ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="0036",
ENV{ID_MM_TELIT_PORTS_TAGGED}="1"
-
LABEL="mm_telit_port_types_end"
diff --git a/plugins/telit/mm-common-telit.c b/plugins/telit/mm-common-telit.c
index 622f63d..8b470f4 100644
--- a/plugins/telit/mm-common-telit.c
+++ b/plugins/telit/mm-common-telit.c
@@ -336,12 +336,6 @@ telit_custom_init (MMPortProbe *probe,
ctx->getportcfg_done = FALSE;
ctx->getportcfg_retries = 3;
-/* If the device is tagged for supporting #PORTCFG do the custom init */
-if (mm_kernel_device_get_global_property_as_boolean (port_device,
"ID_MM_TELIT_PORTS_TAGGED")) {
-telit_custom_init_step (ctx);
-return;
-}
-
-g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
-telit_custom_init_context_complete_and_free (ctx);
+telit_custom_init_step (ctx);
+return;
}
--
2.9.3
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
[PATCH 2/2] dell: speed probing time up and reduce udev dependency
Currently Dell plugin implements a retry logic of three commands
(AT+GMI, AT+CGMI, ATI1I2I3) to detect modem's vendor string.
Moreover, since Telit modems always reply to the first command, to speed
the probing time up, those modem are tagged with an Udev rule so that we
can avoid sending the other two commands at all.
However, the retry logic is in case a port needs some time to reply, so
it makes sense to apply it only to the first command. Then if the port still
does not respond with any kind of reply, that probably means that it is not
AT capable and we can skip the other AT commands as well.
Then, this patch:
- sets a maximum number of timeouts for AT+GMI to 3. After this
timeouts, the port is considered not AT-capable.
- sets AT+CGMI and ATI1I2I3 to be sent only once.
- removes Dell udev rule for tagging Telit Modems.
---
Updated the patch according to Aleksander code review.
---
plugins/dell/77-mm-dell-port-types.rules | 4
plugins/dell/mm-plugin-dell.c| 26 --
2 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/plugins/dell/77-mm-dell-port-types.rules
b/plugins/dell/77-mm-dell-port-types.rules
index bbb59b9..206fb6e 100644
--- a/plugins/dell/77-mm-dell-port-types.rules
+++ b/plugins/dell/77-mm-dell-port-types.rules
@@ -8,9 +8,5 @@ GOTO="mm_dell_port_types_end"
LABEL="mm_dell_vendorcheck"
SUBSYSTEMS=="usb", ATTRS{bInterfaceNumber}=="?*",
ENV{.MM_USBIFNUM}="$attr{bInterfaceNumber}"
-# DW5580 is a Dell-branded Telit modem
-# tag is needed here for dynamic port recognition
-ATTRS{idVendor}=="413c", ATTRS{idProduct}=="81ba",
ENV{ID_MM_TELIT_PORTS_TAGGED}="1"
-
GOTO="mm_dell_port_types_end"
LABEL="mm_dell_port_types_end"
diff --git a/plugins/dell/mm-plugin-dell.c b/plugins/dell/mm-plugin-dell.c
index f9dfd1a..f982cfb 100644
--- a/plugins/dell/mm-plugin-dell.c
+++ b/plugins/dell/mm-plugin-dell.c
@@ -45,6 +45,8 @@
#include "mm-broadband-modem-mbim.h"
#endif
+#define MAX_PORT_PROBE_TIMEOUTS 3
+
G_DEFINE_TYPE (MMPluginDell, mm_plugin_dell, MM_TYPE_PLUGIN)
MM_PLUGIN_DEFINE_MAJOR_VERSION
@@ -70,6 +72,7 @@ typedef struct {
guint gmi_retries;
guint cgmi_retries;
guint ati_retries;
+guint timeouts;
} CustomInitContext;
static void
@@ -140,6 +143,8 @@ static void custom_init_step (CustomInitContext *ctx);
static void
custom_init_step_next_command (CustomInitContext *ctx)
{
+ctx->timeouts = 0;
+
if (ctx->gmi_retries > 0)
ctx->gmi_retries = 0;
else if (ctx->cgmi_retries > 0)
@@ -169,6 +174,7 @@ response_ready (MMPortSerialAt *port,
return;
}
/* Directly retry same command on timeout */
+ctx->timeouts++;
custom_init_step (ctx);
g_error_free (error);
return;
@@ -269,6 +275,12 @@ custom_init_step (CustomInitContext *ctx)
}
#endif
+if (ctx->timeouts >= MAX_PORT_PROBE_TIMEOUTS) {
+mm_dbg ("(Dell) couldn't flip secondary port to AT in (%s): too many
timeouts",
+mm_port_get_device (MM_PORT (ctx->port)));
+goto out;
+}
+
if (ctx->gmi_retries > 0) {
ctx->gmi_retries--;
mm_port_serial_at_command (ctx->port,
@@ -309,9 +321,10 @@ custom_init_step (CustomInitContext *ctx)
return;
}
-/* Finish custom_init */
mm_dbg ("(Dell) couldn't flip secondary port to AT in (%s): all retries
consumed",
mm_port_get_device (MM_PORT (ctx->port)));
+out:
+/* Finish custom_init */
g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
custom_init_context_complete_and_free (ctx);
}
@@ -337,15 +350,8 @@ dell_custom_init (MMPortProbe *probe,
ctx->port = g_object_ref (port);
ctx->cancellable = cancellable ? g_object_ref (cancellable) : NULL;
ctx->gmi_retries = 3;
-ctx->cgmi_retries = 3;
-ctx->ati_retries = 3;
-
-/* Dell-branded Telit modems always answer to +GMI
- * Avoid +CGMI and ATI sending for minimizing port probing time */
-if (mm_kernel_device_get_global_property_as_boolean (port_device,
"ID_MM_TELIT_PORTS_TAGGED")) {
-ctx->cgmi_retries = 0;
-ctx->ati_retries = 0;
-}
+ctx->cgmi_retries = 1;
+ctx->ati_retries = 1;
custom_init_step (ctx);
}
--
2.9.3
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH 2/2] dell: removed ID_MM_TELIT_PORTS_TAGGED dependency
> The retries logic is in case the port needs some time to reply, so > probably doing the retries only for the first command would make > sense, and this would get us 4 commands removed for all. > > But also, is there a modem that doesn't reply to AT+GMI at all? i.e. > just not replying anything? I doubt it, so we could also see if all > AT+GMI retries failed with a timeout error, and if so we just assume > it's not an AT port directly. This would actually have a better effect > than your patch, because we not only skip the CGMI or ATI commands, > but also the extra "AT"-only commands that I also see in the logs. > > What do you think? I think the last approach could be very useful here. I think that's a nice solution! I'll develop the new patch2 for this. Doe this affect also the part 1 of the patch? I mean, do you want me to submit again even part 1? On 10 May 2017 at 21:22, Aleksander Morgado wrote: > On Wed, May 10, 2017 at 4:34 PM, Carlo Lobrano > wrote: > >> Could you gather debug logs using the Telit/Dell modem in 2 different > >> runs: > >>* with the custom block setting cgmi_retries=0 and ati_retries=0 in > >> the Dell plugin (as in git master) > >>* and without it (e.g. just removing that block) > > > > > > > > Ok, this is equivalent to test it once with MM master, and once with MM > > master but without Dell udev rule to tag Telit modems, right? > > Yes > > > The results are attached to this email > > So, without the udev rule it takes 51s to probe all ports doing all > retries for all commands. With the udev rule in place it gets a bit > better, and takes 30s to do all port probing (you removed 6 AT > commands with the udev rule: 3 CGMI, 3ATI). > > I'm tempted to say that instead of: > ctx->gmi_retries = 3; > ctx->cgmi_retries = 3; > ctx->ati_retries = 3; > > We could do: > ctx->gmi_retries = 3; > ctx->cgmi_retries = 1; > ctx->ati_retries = 1; > > The retries logic is in case the port needs some time to reply, so > probably doing the retries only for the first command would make > sense, and this would get us 4 commands removed for all. > > But also, is there a modem that doesn't reply to AT+GMI at all? i.e. > just not replying anything? I doubt it, so we could also see if all > AT+GMI retries failed with a timeout error, and if so we just assume > it's not an AT port directly. This would actually have a better effect > than your patch, because we not only skip the CGMI or ATI commands, > but also the extra "AT"-only commands that I also see in the logs. > > What do you think? I think the last approach could be very useful here. > > -- > Aleksander > https://aleksander.es > ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: Re: SIM hot swap with SIM locked
> Hi Aleksander, > > You'll be able to reproduce it with the two patches attached to this > email. The first is basically the one you proposed for tracing #QSS: 3, > while the latter is a work in progress to fix the problem of sim locked > with some logic to manage QSS transitions and some debug logs > > On Fri, 17 Mar 2017 at 21:03 Aleksander Morgado > wrote: > >> On Fri, Mar 17, 2017 at 4:55 PM, Carlo Lobrano >> wrote: >> > mar 17 16:45:15 D2040 ModemManager[3946]: (ttyACM0): --> >> > 'AT+GCAP' >> > mar 17 16:45:15 D2040 ModemManager[3946]: (ttyACM0): <-- >> '' >> > mar 17 16:45:15 D2040 ModemManager[3946]: (ttyACM0): <-- '+GCAP: >> > +CGSM,+DS,+FCLASS,+MS,+ESOK' >> >> Ah, ok, I see the here now. >> >> > mar 17 16:45:16 D2040 ModemManager[3946]: parse_caps_gcap on >> > '(null)' >> >> This is very very weird. >> >> I really would like to try to reproduce this myself... >> >> -- >> Aleksander >> https://aleksander.es >> > > ___ > ModemManager-devel mailing list > ModemManager-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel > > Hi Aleksander, any update on this front? Let me know if you need any help for this Thanks! ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH 2/2] dell: removed ID_MM_TELIT_PORTS_TAGGED dependency
On 8 May 2017 at 09:55, Aleksander Morgado wrote:
> Wouldn't this happen in the same way for all non-AT ports in modems
> managed by the Telit plugin, now that ID_MM_TELIT_PORTS_TAGGED is
> removed from the Telit plugin as well?
>
In Telit plugin we only probe the port with ID_USB_INTERFACE_NUM == 00,
which we know that supports AT commands, see mm-common-telit.c:
286 /* Try to get a port configuration from the modem: usb interface 00
287 * is always linked to an AT port
288 */
289 port = mm_port_probe_peek_port (ctx->probe);
290 if (!ctx->getportcfg_done &&
291 g_strcmp0 (mm_kernel_device_get_property (port,
"ID_USB_INTERFACE_NUM"), "00") == 0) {
292
293 if (ctx->getportcfg_retries == 0)
294 goto out;
295 ctx->getportcfg_retries--;
296
297 mm_port_serial_at_command (
298 ctx->port,
299 "AT#PORTCFG?",
300 2,
301 FALSE, /* raw */
302 FALSE, /* allow_cached */
303 ctx->cancellable,
304 (GAsyncReadyCallback)getportcfg_ready,
305 ctx);
306 return;
307 }
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH 2/2] dell: removed ID_MM_TELIT_PORTS_TAGGED dependency
> > > I don't see any clear benefit in doing this; we're moving the match > with the PID from the udev rules file to the actual source code, which > means that if we ever get any other Dell-branded modem we'll need to > update the source code instead of adding a new udev rules file. But... > This follows the first part of the patch, where the ID_MM_TELIT_PORTS_TAGGED was removed, and then I though to remove it also from here and reduce the dependency from udev, but I understand its not a big change and can be dropped. Looking at the actual purpose of the code; it looks like it's for > minimizing probing time because Telit modems always reply well to > AT+GMI , and that is actually the first command we try... so do we > even need that logic there for anything? What happens if you remove > the block setting the cgmi_retries and at_retries to 0? Only AT ports reply to AT+GMI, while for the others (5, if I remember correctly) we need to try all the commands and the retries and wait for timeout to go on. ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
[PATCH 2/2] dell: removed ID_MM_TELIT_PORTS_TAGGED dependency
Currently Dell plugin depends on ID_MM_TELIT_PORTS_TAGGED
environment variable, set by udev, to speed up probing time
avoiding +CGMI and ATI sending.
To remove this dependency from udev, I moved the product ID
check from dell's udev rule to mm-plugin-dell.c file.
---
plugins/dell/77-mm-dell-port-types.rules | 2 +-
plugins/dell/mm-plugin-dell.c| 8 +++-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/plugins/dell/77-mm-dell-port-types.rules
b/plugins/dell/77-mm-dell-port-types.rules
index bbb59b9..5dcef47 100644
--- a/plugins/dell/77-mm-dell-port-types.rules
+++ b/plugins/dell/77-mm-dell-port-types.rules
@@ -10,7 +10,7 @@ SUBSYSTEMS=="usb", ATTRS{bInterfaceNumber}=="?*",
ENV{.MM_USBIFNUM}="$attr{bInte
# DW5580 is a Dell-branded Telit modem
# tag is needed here for dynamic port recognition
-ATTRS{idVendor}=="413c", ATTRS{idProduct}=="81ba",
ENV{ID_MM_TELIT_PORTS_TAGGED}="1"
+#ATTRS{idVendor}=="413c", ATTRS{idProduct}=="81ba",
ENV{ID_MM_TELIT_PORTS_TAGGED}="1"
GOTO="mm_dell_port_types_end"
LABEL="mm_dell_port_types_end"
diff --git a/plugins/dell/mm-plugin-dell.c b/plugins/dell/mm-plugin-dell.c
index f9dfd1a..71ca5a4 100644
--- a/plugins/dell/mm-plugin-dell.c
+++ b/plugins/dell/mm-plugin-dell.c
@@ -325,6 +325,7 @@ dell_custom_init (MMPortProbe *probe,
{
CustomInitContext *ctx;
MMKernelDevice *port_device;
+guint16 product_id;
port_device = mm_port_probe_peek_port (probe);
@@ -342,9 +343,14 @@ dell_custom_init (MMPortProbe *probe,
/* Dell-branded Telit modems always answer to +GMI
* Avoid +CGMI and ATI sending for minimizing port probing time */
-if (mm_kernel_device_get_global_property_as_boolean (port_device,
"ID_MM_TELIT_PORTS_TAGGED")) {
+product_id = mm_device_get_product (mm_port_probe_peek_device (probe));
+switch (product_id) {
+case 0x81ba:
ctx->cgmi_retries = 0;
ctx->ati_retries = 0;
+break;
+default:
+break;
}
custom_init_step (ctx);
--
2.9.3
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
[PATCH 1/2] telit: removed ID_MM_TELIT_PORTS_TAGGED dependency
Currently, Telit plugin depends on ID_MM_TELIT_PORTS_TAGGED
environment variable, set by udev, for tagging modems that
support dynamic port config (#PORTCFG)
To remove this dependency from udev, Telit plugin now relies
only on the error management of the command AT#PORTCFG? itself
in order to see whether the modem supports it or not.
---
plugins/telit/77-mm-telit-port-types.rules | 9 -
plugins/telit/mm-common-telit.c| 10 ++
2 files changed, 2 insertions(+), 17 deletions(-)
diff --git a/plugins/telit/77-mm-telit-port-types.rules
b/plugins/telit/77-mm-telit-port-types.rules
index 01538cb..b052962 100644
--- a/plugins/telit/77-mm-telit-port-types.rules
+++ b/plugins/telit/77-mm-telit-port-types.rules
@@ -33,13 +33,4 @@ ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1010",
ENV{.MM_USBIFNUM}=="03", ENV{
# CE910-DUAL
ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1011", ENV{.MM_USBIFNUM}=="01",
ENV{ID_MM_TELIT_PORT_TYPE_MODEM}="1"
-# HE910, UE910, UL865 (dynamic port identification supported)
-ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="0021",
ENV{ID_MM_TELIT_PORTS_TAGGED}="1"
-
-# GE910 (dynamic port identification supported)
-ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="0022",
ENV{ID_MM_TELIT_PORTS_TAGGED}="1"
-
-# LE910 V2
-ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="0036",
ENV{ID_MM_TELIT_PORTS_TAGGED}="1"
-
LABEL="mm_telit_port_types_end"
diff --git a/plugins/telit/mm-common-telit.c b/plugins/telit/mm-common-telit.c
index 622f63d..8b470f4 100644
--- a/plugins/telit/mm-common-telit.c
+++ b/plugins/telit/mm-common-telit.c
@@ -336,12 +336,6 @@ telit_custom_init (MMPortProbe *probe,
ctx->getportcfg_done = FALSE;
ctx->getportcfg_retries = 3;
-/* If the device is tagged for supporting #PORTCFG do the custom init */
-if (mm_kernel_device_get_global_property_as_boolean (port_device,
"ID_MM_TELIT_PORTS_TAGGED")) {
-telit_custom_init_step (ctx);
-return;
-}
-
-g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
-telit_custom_init_context_complete_and_free (ctx);
+telit_custom_init_step (ctx);
+return;
}
--
2.9.3
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: MMKernelDevice ID_USB_INTERFACE_NUM property isn't set anymore
>
> By chance I have another Ubuntu machine, which is 17.04 while the main one
> is 16.10, not sure that this matters, and in this one
>
> IS_USB_INTERFACE_NUM is correctly defined :|
And the /lib/udev/rules.d/60-serial.rules is present as well
I also double checked with a virtualized Fedora 18 and
IS_USB_INTERFACE_NUM is correctly set.
I do think this should be a problem with my main machine now
(sorry for the top posting)
On 3 May 2017 at 17:25, Carlo Lobrano wrote:
> By chance I have another Ubuntu machine, which is 17.04 while the main one
> is 16.10, not sure that this matters, and in this one IS_USB_INTERFACE_NUM
> is correctly defined :|
>
> On 3 May 2017 at 17:08, Dan Williams wrote:
>
>> On Wed, 2017-05-03 at 14:55 +0200, Aleksander Morgado wrote:
>> > On Wed, May 3, 2017 at 2:09 PM, Carlo Lobrano
>> > wrote:
>> > > > > > When using the "udev" backend, which you are, the property
>> > > > > > isn't
>> > > > > > "loaded" or "stored" anywhere in our code. We directly look
>> > > > > > for the
>> > > > > > property in the GUdevDevice underneath.
>> > > > > >
>> > > > > > Could you run udevadm info in the port to see if
>> > > > > > ID_USB_INTERFACE_NUM
>> > > > > > is
>> > > > > > set?
>> > > > >
>> > > > > Oh, I see now.
>> > > > > The property is actually unset. Is there anything else I can do
>> > > > > to have
>> > > > > this
>> > > > > information?
>> > > > >
>> > > > > $ udevadm info /dev/ttyACM0
>> > > > > P:
>> > > > >
>> > > > > /devices/pci:00/:00:14.0/usb3/3-3/3-3.2/3-3.2.4/3-
>> > > > > 3.2.4.3/3-3.2.4.3:1.0/tty/ttyACM0
>> > > > > N: ttyACM0
>> > > > > E: DEVNAME=/dev/ttyACM0
>> > > > > E:
>> > > > >
>> > > > > DEVPATH=/devices/pci:00/:00:14.0/usb3/3-3/3-3.2/3-
>> > > > > 3.2.4/3-3.2.4.3/3-3.2.4.3:1.0/tty/ttyACM0
>> > > > > E: ID_MM_CANDIDATE=1
>> > > > > E: ID_MM_TELIT_PORTS_TAGGED=1
>> > > > > E: ID_MM_TELIT_TAGGED=1
>> > > > > E: MAJOR=166
>> > > > > E: MINOR=0
>> > > > > E: SUBSYSTEM=tty
>> > > > > E: TAGS=:systemd:
>> > > > > E: USEC_INITIALIZED=75726372964
>> > > >
>> > > > Do you have a /lib/udev/rules.d/60-serial.rules in your system
>> > > > that
>> > > > sets ID_USB_INTERFACE_NUM? Maybe we shouldn't rely on having
>> > > > that?
>> > > >
>> > >
>> > > No, I do not have that 60-serial.rules file, but I did obtained the
>> > > result I
>> > > wanted adding the following rule to 77-mm-telit-port-types.rules
>> > >
>> > > SUBSYSTEMS=="usb", ATTRS{bInterfaceNumber}=="?*",
>> > > ENV{ID_USB_INTERFACE_NUM}="$attr{bInterfaceNumber}"
>> >
>> > We need to fix this; we shouldn't be relying on a systemd property. I
>> > originally thought this was provided by udev itself... although with
>> > the systemd/udev development being done together, it may actually be
>> > a
>> > packaging decision; no idea.
>>
>> I think it originally was a udev property. And I see it's still
>> provided by systemd-udev, so I'd expect it to be present if udev is
>> installed...
>>
>> Dan
>>
>> > One option would be to define ID_MM_USB_INTERFACE_NUM in all plugin
>> > udev rules (telit, option, huawei, cinterion), and update the plugins
>> > source code to use the new ID_MM_ prefixed property.
>> >
>> > Another option would be to provide a common rules file that is run
>> > before all the others and defines ID_MM_USB_INTERFACE_NUM for all
>> > plugins... we could have a 77-mm-serial.rules file and rename all
>> > plugin rules to 78-mm-...
>> >
>> > What do you think?
>> >
>>
>
>
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: MMKernelDevice ID_USB_INTERFACE_NUM property isn't set anymore
By chance I have another Ubuntu machine, which is 17.04 while the main one
is 16.10, not sure that this matters, and in this one IS_USB_INTERFACE_NUM
is correctly defined :|
On 3 May 2017 at 17:08, Dan Williams wrote:
> On Wed, 2017-05-03 at 14:55 +0200, Aleksander Morgado wrote:
> > On Wed, May 3, 2017 at 2:09 PM, Carlo Lobrano
> > wrote:
> > > > > > When using the "udev" backend, which you are, the property
> > > > > > isn't
> > > > > > "loaded" or "stored" anywhere in our code. We directly look
> > > > > > for the
> > > > > > property in the GUdevDevice underneath.
> > > > > >
> > > > > > Could you run udevadm info in the port to see if
> > > > > > ID_USB_INTERFACE_NUM
> > > > > > is
> > > > > > set?
> > > > >
> > > > > Oh, I see now.
> > > > > The property is actually unset. Is there anything else I can do
> > > > > to have
> > > > > this
> > > > > information?
> > > > >
> > > > > $ udevadm info /dev/ttyACM0
> > > > > P:
> > > > >
> > > > > /devices/pci:00/:00:14.0/usb3/3-3/3-3.2/3-3.2.4/3-
> > > > > 3.2.4.3/3-3.2.4.3:1.0/tty/ttyACM0
> > > > > N: ttyACM0
> > > > > E: DEVNAME=/dev/ttyACM0
> > > > > E:
> > > > >
> > > > > DEVPATH=/devices/pci:00/:00:14.0/usb3/3-3/3-3.2/3-
> > > > > 3.2.4/3-3.2.4.3/3-3.2.4.3:1.0/tty/ttyACM0
> > > > > E: ID_MM_CANDIDATE=1
> > > > > E: ID_MM_TELIT_PORTS_TAGGED=1
> > > > > E: ID_MM_TELIT_TAGGED=1
> > > > > E: MAJOR=166
> > > > > E: MINOR=0
> > > > > E: SUBSYSTEM=tty
> > > > > E: TAGS=:systemd:
> > > > > E: USEC_INITIALIZED=75726372964
> > > >
> > > > Do you have a /lib/udev/rules.d/60-serial.rules in your system
> > > > that
> > > > sets ID_USB_INTERFACE_NUM? Maybe we shouldn't rely on having
> > > > that?
> > > >
> > >
> > > No, I do not have that 60-serial.rules file, but I did obtained the
> > > result I
> > > wanted adding the following rule to 77-mm-telit-port-types.rules
> > >
> > > SUBSYSTEMS=="usb", ATTRS{bInterfaceNumber}=="?*",
> > > ENV{ID_USB_INTERFACE_NUM}="$attr{bInterfaceNumber}"
> >
> > We need to fix this; we shouldn't be relying on a systemd property. I
> > originally thought this was provided by udev itself... although with
> > the systemd/udev development being done together, it may actually be
> > a
> > packaging decision; no idea.
>
> I think it originally was a udev property. And I see it's still
> provided by systemd-udev, so I'd expect it to be present if udev is
> installed...
>
> Dan
>
> > One option would be to define ID_MM_USB_INTERFACE_NUM in all plugin
> > udev rules (telit, option, huawei, cinterion), and update the plugins
> > source code to use the new ID_MM_ prefixed property.
> >
> > Another option would be to provide a common rules file that is run
> > before all the others and defines ID_MM_USB_INTERFACE_NUM for all
> > plugins... we could have a 77-mm-serial.rules file and rename all
> > plugin rules to 78-mm-...
> >
> > What do you think?
> >
>
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: MMKernelDevice ID_USB_INTERFACE_NUM property isn't set anymore
On 3 May 2017 at 14:55, Aleksander Morgado wrote:
> On Wed, May 3, 2017 at 2:09 PM, Carlo Lobrano wrote:
> >> >> When using the "udev" backend, which you are, the property isn't
> >> >> "loaded" or "stored" anywhere in our code. We directly look for the
> >> >> property in the GUdevDevice underneath.
> >> >>
> >> >> Could you run udevadm info in the port to see if ID_USB_INTERFACE_NUM
> >> >> is
> >> >> set?
> >> >
> >> > Oh, I see now.
> >> > The property is actually unset. Is there anything else I can do to
> have
> >> > this
> >> > information?
> >> >
> >> > $ udevadm info /dev/ttyACM0
> >> > P:
> >> >
> >> > /devices/pci:00/:00:14.0/usb3/3-3/3-3.2/3-3.2.4/3-
> 3.2.4.3/3-3.2.4.3:1.0/tty/ttyACM0
> >> > N: ttyACM0
> >> > E: DEVNAME=/dev/ttyACM0
> >> > E:
> >> >
> >> > DEVPATH=/devices/pci:00/:00:14.0/usb3/3-3/3-3.2/3-
> 3.2.4/3-3.2.4.3/3-3.2.4.3:1.0/tty/ttyACM0
> >> > E: ID_MM_CANDIDATE=1
> >> > E: ID_MM_TELIT_PORTS_TAGGED=1
> >> > E: ID_MM_TELIT_TAGGED=1
> >> > E: MAJOR=166
> >> > E: MINOR=0
> >> > E: SUBSYSTEM=tty
> >> > E: TAGS=:systemd:
> >> > E: USEC_INITIALIZED=75726372964
> >>
> >> Do you have a /lib/udev/rules.d/60-serial.rules in your system that
> >> sets ID_USB_INTERFACE_NUM? Maybe we shouldn't rely on having that?
> >>
> >
> > No, I do not have that 60-serial.rules file, but I did obtained the
> result I
> > wanted adding the following rule to 77-mm-telit-port-types.rules
> >
> > SUBSYSTEMS=="usb", ATTRS{bInterfaceNumber}=="?*",
> > ENV{ID_USB_INTERFACE_NUM}="$attr{bInterfaceNumber}"
>
> We need to fix this; we shouldn't be relying on a systemd property. I
> originally thought this was provided by udev itself... although with
> the systemd/udev development being done together, it may actually be a
> packaging decision; no idea.
>
> One option would be to define ID_MM_USB_INTERFACE_NUM in all plugin
> udev rules (telit, option, huawei, cinterion), and update the plugins
> source code to use the new ID_MM_ prefixed property.
>
> Another option would be to provide a common rules file that is run
> before all the others and defines ID_MM_USB_INTERFACE_NUM for all
> plugins... we could have a 77-mm-serial.rules file and rename all
> plugin rules to 78-mm-...
>
> What do you think?
>
>
If the rule is the same for all the plugins (i.e. the one I wrote above),
I'd go with the single file 77-mm-serial.rules. Eventually, each plugin
can than adjust the behavior in its 78-mm... rule file
> --
> Aleksander
> https://aleksander.es
>
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: MMKernelDevice ID_USB_INTERFACE_NUM property isn't set anymore
On 3 May 2017 at 12:47, Aleksander Morgado wrote:
> On Wed, May 3, 2017 at 12:10 PM, Carlo Lobrano
> wrote:
> >> When using the "udev" backend, which you are, the property isn't
> >> "loaded" or "stored" anywhere in our code. We directly look for the
> >> property in the GUdevDevice underneath.
> >>
> >> Could you run udevadm info in the port to see if ID_USB_INTERFACE_NUM is
> >> set?
> >
> > Oh, I see now.
> > The property is actually unset. Is there anything else I can do to have
> this
> > information?
> >
> > $ udevadm info /dev/ttyACM0
> > P:
> > /devices/pci:00/:00:14.0/usb3/3-3/3-3.2/3-3.2.4/3-
> 3.2.4.3/3-3.2.4.3:1.0/tty/ttyACM0
> > N: ttyACM0
> > E: DEVNAME=/dev/ttyACM0
> > E:
> > DEVPATH=/devices/pci:00/:00:14.0/usb3/3-3/3-3.2/3-
> 3.2.4/3-3.2.4.3/3-3.2.4.3:1.0/tty/ttyACM0
> > E: ID_MM_CANDIDATE=1
> > E: ID_MM_TELIT_PORTS_TAGGED=1
> > E: ID_MM_TELIT_TAGGED=1
> > E: MAJOR=166
> > E: MINOR=0
> > E: SUBSYSTEM=tty
> > E: TAGS=:systemd:
> > E: USEC_INITIALIZED=75726372964
>
> Do you have a /lib/udev/rules.d/60-serial.rules in your system that
> sets ID_USB_INTERFACE_NUM? Maybe we shouldn't rely on having that?
>
>
No, I do not have that 60-serial.rules file, but I did obtained the result
I wanted adding the following rule to 77-mm-telit-port-types.rules
SUBSYSTEMS=="usb", ATTRS{bInterfaceNumber}=="?*",
ENV{ID_USB_INTERFACE_NUM}="$attr{bInterfaceNumber}"
--
> Aleksander
> https://aleksander.es
>
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: MMKernelDevice ID_USB_INTERFACE_NUM property isn't set anymore
> I answered in IRC already the other day, I see you didn't get my reply :) yeah, I'm sorry :) > When using the "udev" backend, which you are, the property isn't > "loaded" or "stored" anywhere in our code. We directly look for the > property in the GUdevDevice underneath. > > Could you run udevadm info in the port to see if ID_USB_INTERFACE_NUM is set? Oh, I see now. The property is actually unset. Is there anything else I can do to have this information? $ udevadm info /dev/ttyACM0 P: /devices/pci:00/:00:14.0/usb3/3-3/3-3.2/3-3.2.4/3-3.2.4.3/3-3.2.4.3:1.0/tty/ttyACM0 N: ttyACM0 E: DEVNAME=/dev/ttyACM0 E: DEVPATH=/devices/pci:00/:00:14.0/usb3/3-3/3-3.2/3-3.2.4/3-3.2.4.3/3-3.2.4.3:1.0/tty/ttyACM0 E: ID_MM_CANDIDATE=1 E: ID_MM_TELIT_PORTS_TAGGED=1 E: ID_MM_TELIT_TAGGED=1 E: MAJOR=166 E: MINOR=0 E: SUBSYSTEM=tty E: TAGS=:systemd: E: USEC_INITIALIZED=75726372964 On 2 May 2017 at 18:04, Aleksander Morgado wrote: > On Tue, May 2, 2017 at 4:49 PM, Carlo Lobrano wrote: > > > > first of all, I've already asked this question on IRC, but then I had to > > disconnect and I have no bouncer configured, so my apologize if someone > has > > already answered. > > > > I'm working on telit plugin port configuration (telit_custom_init_step > to be > > precise) and it seems that MMKernelDevice ID_USB_INTERFACE_NUM property > is > > never set. > > > > Tracking back this property, the only point where it's set is > > preload_interface_number called from preload_contents, but then I got > lost > > and not sure where to check > > > > I would appreciate any ideas > > I answered in IRC already the other day, I see you didn't get my reply :) > > When using the "udev" backend, which you are, the property isn't > "loaded" or "stored" anywhere in our code. We directly look for the > property in the GUdevDevice underneath. > > Could you run udevadm info in the port to see if ID_USB_INTERFACE_NUM is > set? > > -- > Aleksander > https://aleksander.es > ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
MMKernelDevice ID_USB_INTERFACE_NUM property isn't set anymore
Hi, first of all, I've already asked this question on IRC, but then I had to disconnect and I have no bouncer configured, so my apologize if someone has already answered. I'm working on telit plugin port configuration (telit_custom_init_step to be precise) and it seems that MMKernelDevice ID_USB_INTERFACE_NUM property is never set. Tracking back this property, the only point where it's set is preload_interface_number called from preload_contents, but then I got lost and not sure where to check I would appreciate any ideas ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
[PATCH] telit: give load lock retries steps a descriptive name
In order to make debug logging more clear, I replaced the step ID with a
descriptive step name.
---
plugins/telit/mm-broadband-modem-telit.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/plugins/telit/mm-broadband-modem-telit.c
b/plugins/telit/mm-broadband-modem-telit.c
index 9bd16cd..13ca4a5 100644
--- a/plugins/telit/mm-broadband-modem-telit.c
+++ b/plugins/telit/mm-broadband-modem-telit.c
@@ -487,6 +487,13 @@ typedef enum {
LOAD_UNLOCK_RETRIES_STEP_LAST
} LoadUnlockRetriesStep;
+static const gchar *step_lock_names[LOAD_UNLOCK_RETRIES_STEP_LAST] = {
+[LOAD_UNLOCK_RETRIES_STEP_PIN] = "PIN",
+[LOAD_UNLOCK_RETRIES_STEP_PUK] = "PUK",
+[LOAD_UNLOCK_RETRIES_STEP_PIN2] = "PIN2",
+[LOAD_UNLOCK_RETRIES_STEP_PUK2] = "PUK2",
+};
+
typedef struct {
MMBroadbandModemTelit *self;
GSimpleAsyncResult *result;
@@ -559,34 +566,32 @@ csim_query_ready (MMBaseModem *self,
response = mm_base_modem_at_command_finish (self, res, &error);
if (!response) {
-mm_warn ("load unlock retries: no respose for step %d: %s", ctx->step,
error->message);
+mm_warn ("load %s unlock retries got no response: %s",
step_lock_names[ctx->step], error->message);
g_error_free (error);
goto next_step;
}
if ( (unlock_retries = mm_telit_parse_csim_response (response, &error)) <
0) {
-mm_warn ("load unlock retries: parse error in step %d: %s.",
ctx->step, error->message);
+mm_warn ("load %s unlock retries parse error: %s.",
step_lock_names[ctx->step], error->message);
g_error_free (error);
goto next_step;
}
ctx->succeded_requests++;
+mm_dbg ("%s unlock retries left: %d", step_lock_names[ctx->step],
unlock_retries);
+
switch (ctx->step) {
case LOAD_UNLOCK_RETRIES_STEP_PIN:
-mm_dbg ("PIN unlock retries left: %d", unlock_retries);
mm_unlock_retries_set (ctx->retries, MM_MODEM_LOCK_SIM_PIN,
unlock_retries);
break;
case LOAD_UNLOCK_RETRIES_STEP_PUK:
-mm_dbg ("PUK unlock retries left: %d", unlock_retries);
mm_unlock_retries_set (ctx->retries, MM_MODEM_LOCK_SIM_PUK,
unlock_retries);
break;
case LOAD_UNLOCK_RETRIES_STEP_PIN2:
-mm_dbg ("PIN2 unlock retries left: %d", unlock_retries);
mm_unlock_retries_set (ctx->retries, MM_MODEM_LOCK_SIM_PIN2,
unlock_retries);
break;
case LOAD_UNLOCK_RETRIES_STEP_PUK2:
-mm_dbg ("PUK2 unlock retries left: %d", unlock_retries);
mm_unlock_retries_set (ctx->retries, MM_MODEM_LOCK_SIM_PUK2,
unlock_retries);
break;
default:
--
2.9.3
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH v4] telit: add error_code recognition to +CSIM parser
>
> Pushed to git master. I also pushed a follow up commit to use
> mm_get_uint_from_hex_str():
> https://cgit.freedesktop.org/ModemManager/ModemManager/commit/?id=
> d09bc8baaa9fe93a72bb715530b1403a7a81c891
>
> Thanks!
>
Awesome, thanks to you!
On 19 April 2017 at 10:46, Aleksander Morgado
wrote:
> On 19/04/17 10:00, Carlo Lobrano wrote:
> > - Refactored mm_telit_parse_csim_response in order to correctly
> > recognize the following +CSIM error codes:
> > * 6300 - Verification failed
> > * 6983 - Authentication method blocked
> > * 6984 - Reference data invalidated
> > * 6A86 - Incorrect parameters
> > * 6A88 - Reference data not found
> >
> > - Updated correspondent tests.
> > - Finally, some minor changes in other files for better error logging
> >
> > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=100374
>
> Pushed to git master. I also pushed a follow up commit to use
> mm_get_uint_from_hex_str():
> https://cgit.freedesktop.org/ModemManager/ModemManager/commit/?id=
> d09bc8baaa9fe93a72bb715530b1403a7a81c891
>
> Thanks!
>
> > ---
> > plugins/telit/mm-broadband-modem-telit.c | 6 +-
> > plugins/telit/mm-modem-helpers-telit.c| 98
> ---
> > plugins/telit/mm-modem-helpers-telit.h| 3 +-
> > plugins/telit/tests/test-mm-modem-helpers-telit.c | 73
> -
> > 4 files changed, 104 insertions(+), 76 deletions(-)
> >
> > diff --git a/plugins/telit/mm-broadband-modem-telit.c
> b/plugins/telit/mm-broadband-modem-telit.c
> > index 8b87310..d154fc6 100644
> > --- a/plugins/telit/mm-broadband-modem-telit.c
> > +++ b/plugins/telit/mm-broadband-modem-telit.c
> > @@ -541,13 +541,13 @@ csim_query_ready (MMBaseModem *self,
> > response = mm_base_modem_at_command_finish (self, res, &error);
> >
> > if (!response) {
> > -mm_warn ("No respose for step %d: %s", ctx->step,
> error->message);
> > +mm_warn ("load unlock retries: no respose for step %d: %s",
> ctx->step, error->message);
> > g_error_free (error);
> > goto next_step;
> > }
> >
> > -if ( (unlock_retries = mm_telit_parse_csim_response (ctx->step,
> response, &error)) < 0) {
> > -mm_warn ("Parse error in step %d: %s.", ctx->step,
> error->message);
> > +if ( (unlock_retries = mm_telit_parse_csim_response (response,
> &error)) < 0) {
> > +mm_warn ("load unlock retries: parse error in step %d: %s.",
> ctx->step, error->message);
> > g_error_free (error);
> > goto next_step;
> > }
> > diff --git a/plugins/telit/mm-modem-helpers-telit.c
> b/plugins/telit/mm-modem-helpers-telit.c
> > index c8c1f4b..1dd2542 100644
> > --- a/plugins/telit/mm-modem-helpers-telit.c
> > +++ b/plugins/telit/mm-modem-helpers-telit.c
> > @@ -17,6 +17,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #include
> > #define _LIBMM_INSIDE_MMCLI
> > @@ -113,54 +114,91 @@ mm_telit_get_band_flag (GArray *bands_array,
> >
> > /***
> **/
> > /* +CSIM response parser */
> > +#define MM_TELIT_MIN_SIM_RETRY_HEX 0x63C0
> > +#define MM_TELIT_MAX_SIM_RETRY_HEX 0x63CF
> >
> > gint
> > -mm_telit_parse_csim_response (const guint step,
> > - const gchar *response,
> > +mm_telit_parse_csim_response (const gchar *response,
> >GError **error)
> > {
> > -GRegex *r = NULL;
> > GMatchInfo *match_info = NULL;
> > -gchar *retries_hex_str;
> > -guint retries;
> > +GRegex *r = NULL;
> > +gchar *str_code = NULL;
> > +gint retries = -1;
> > +guint64 hex_code = 0x0;
> > +GError *inner_error = NULL;
> >
> > -r = g_regex_new ("\\+CSIM:\\s*[0-9]+,\\s*.*63C(.*)\"",
> G_REGEX_RAW, 0, NULL);
> > +r = g_regex_new ("\\+CSIM:\\s*[0-9]+,\\s*\".*([0-9a-fA-F]{4})\"",
> G_REGEX_RAW, 0, NULL);
> > +g_regex_match (r, response, 0, &match_info);
> >
> > -if (!g_regex_match (r, response, 0, &match_info)) {
> > -g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
> > - "Could not parse reponse '%s'", response);
> > -g_match_info_free (match_info);
> > -
[PATCH v4] telit: add error_code recognition to +CSIM parser
- Refactored mm_telit_parse_csim_response in order to correctly
recognize the following +CSIM error codes:
* 6300 - Verification failed
* 6983 - Authentication method blocked
* 6984 - Reference data invalidated
* 6A86 - Incorrect parameters
* 6A88 - Reference data not found
- Updated correspondent tests.
- Finally, some minor changes in other files for better error logging
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=100374
---
plugins/telit/mm-broadband-modem-telit.c | 6 +-
plugins/telit/mm-modem-helpers-telit.c| 98 ---
plugins/telit/mm-modem-helpers-telit.h| 3 +-
plugins/telit/tests/test-mm-modem-helpers-telit.c | 73 -
4 files changed, 104 insertions(+), 76 deletions(-)
diff --git a/plugins/telit/mm-broadband-modem-telit.c
b/plugins/telit/mm-broadband-modem-telit.c
index 8b87310..d154fc6 100644
--- a/plugins/telit/mm-broadband-modem-telit.c
+++ b/plugins/telit/mm-broadband-modem-telit.c
@@ -541,13 +541,13 @@ csim_query_ready (MMBaseModem *self,
response = mm_base_modem_at_command_finish (self, res, &error);
if (!response) {
-mm_warn ("No respose for step %d: %s", ctx->step, error->message);
+mm_warn ("load unlock retries: no respose for step %d: %s", ctx->step,
error->message);
g_error_free (error);
goto next_step;
}
-if ( (unlock_retries = mm_telit_parse_csim_response (ctx->step, response,
&error)) < 0) {
-mm_warn ("Parse error in step %d: %s.", ctx->step, error->message);
+if ( (unlock_retries = mm_telit_parse_csim_response (response, &error)) <
0) {
+mm_warn ("load unlock retries: parse error in step %d: %s.",
ctx->step, error->message);
g_error_free (error);
goto next_step;
}
diff --git a/plugins/telit/mm-modem-helpers-telit.c
b/plugins/telit/mm-modem-helpers-telit.c
index c8c1f4b..1dd2542 100644
--- a/plugins/telit/mm-modem-helpers-telit.c
+++ b/plugins/telit/mm-modem-helpers-telit.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include
#define _LIBMM_INSIDE_MMCLI
@@ -113,54 +114,91 @@ mm_telit_get_band_flag (GArray *bands_array,
/*/
/* +CSIM response parser */
+#define MM_TELIT_MIN_SIM_RETRY_HEX 0x63C0
+#define MM_TELIT_MAX_SIM_RETRY_HEX 0x63CF
gint
-mm_telit_parse_csim_response (const guint step,
- const gchar *response,
+mm_telit_parse_csim_response (const gchar *response,
GError **error)
{
-GRegex *r = NULL;
GMatchInfo *match_info = NULL;
-gchar *retries_hex_str;
-guint retries;
+GRegex *r = NULL;
+gchar *str_code = NULL;
+gint retries = -1;
+guint64 hex_code = 0x0;
+GError *inner_error = NULL;
-r = g_regex_new ("\\+CSIM:\\s*[0-9]+,\\s*.*63C(.*)\"", G_REGEX_RAW, 0,
NULL);
+r = g_regex_new ("\\+CSIM:\\s*[0-9]+,\\s*\".*([0-9a-fA-F]{4})\"",
G_REGEX_RAW, 0, NULL);
+g_regex_match (r, response, 0, &match_info);
-if (!g_regex_match (r, response, 0, &match_info)) {
-g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
- "Could not parse reponse '%s'", response);
-g_match_info_free (match_info);
-g_regex_unref (r);
-return -1;
+if (!g_match_info_matches (match_info)) {
+inner_error = g_error_new (MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+ "Could not recognize +CSIM response '%s'",
response);
+goto out;
}
-if (!g_match_info_matches (match_info)) {
-g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
- "Could not find matches in response '%s'", response);
-g_match_info_free (match_info);
-g_regex_unref (r);
-return -1;
+str_code = mm_get_string_unquoted_from_match_info (match_info, 1);
+if (str_code == NULL) {
+inner_error = g_error_new (MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+ "Could not find expected string code in
response '%s'", response);
+goto out;
}
-retries_hex_str = mm_get_string_unquoted_from_match_info (match_info, 1);
-g_assert (NULL != retries_hex_str);
+errno = 0;
+hex_code = g_ascii_strtoull (str_code, NULL, 16);
+if (hex_code == G_MAXUINT64 && errno == ERANGE) {
+inner_error = g_error_new (MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+ "Could not recognize expected hex code in
response '%s'", response);
+goto out;
+}
-/* convert hex value to uint */
-if (sscanf (retries_hex_str, "%x", &retries) != 1) {
- g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
- "Could not get retry value from match '%s'",
- retries_hex_str);
-g_match_info_free (match_info);
-g_regex_unref (r);
-
Re: [PATCH] telit: add error_code recognition to +CSIM parser
> > From a quick online search in TS 102.221, I see a lot of other generic > errors defined there as well, see multiple tables in section "10.2.1 > Status conditions returned by the UICC"; maybe we should extend the > list of possible errors supported with all know ones? > > http://www.etsi.org/deliver/etsi_ts/102200_102299/102221/ > 08.02.00_60/ts_102221v080200p.pdf > I should have added all the CSIM errors related to retries counter, but if some are missing, I can add them easily. On 18 April 2017 at 18:20, Aleksander Morgado wrote: > On Tue, Apr 18, 2017 at 6:01 PM, Dan Williams wrote: > >> > * 6300 - Verification failed > >> > * 6983 - Authentication method blocked > >> > * 6984 - Reference data invalidated > >> > * 6A86 - Incorrect parameters > >> > * 6A88 - Reference data not found > >> > >> From a quick online search in TS 102.221, I see a lot of other > >> generic > >> errors defined there as well, see multiple tables in section "10.2.1 > >> Status conditions returned by the UICC"; maybe we should extend the > >> list of possible errors supported with all know ones? > >> > >> http://www.etsi.org/deliver/etsi_ts/102200_102299/102221/08.02.00_60/ > >> ts_102221v080200p.pdf > > > > At some point we just need to write a generic CSIM/CRSM simfile driver, > > kinda like Ofono has. Nothing about CSIM is telit specific, and we > > might want to use that in other places too. > > Yes, totally agree, maybe we should focus on that next. > > > -- > Aleksander > https://aleksander.es > ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: [PATCH] telit: add error_code recognition to +CSIM parser
Hi Aleksander,
see below my reply
Il giovedì 6 aprile 2017, Aleksander Morgado ha
scritto:
> Hey Carlo,
>
> On 04/04/17 14:55, Carlo Lobrano wrote:
> > - Refactored mm_telit_parse_csim_response in order to correctly
> recognize the
> > following +CSIM error codes:
> >
> > * 6300 - Verification failed
> > * 6983 - Authentication method blocked
> > * 6984 - Reference data invalidated
> > * 6A86 - Incorrect parameters
> > * 6A88 - Reference data not found
> >
> > - Updated correspondent tests.
> > - Finally, some minor changes in other files for better error logging
> >
> > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=100374
> >
> > ---
> >
> > As a side note, I observed that sometimes the modem replies with error
> > code
> >
> > 6A86 - Incorrect parameters
> >
> > when #QSS: 3 has not been received yet. This seems to be a modem's bug
> > because the very same request is accepted as correct when issued later,
> > namely when the SIM is ready.
> >
>
> See comments inline below.
>
> > ---
> > plugins/telit/mm-broadband-modem-telit.c | 6 +-
> > plugins/telit/mm-modem-helpers-telit.c| 93
> ---
> > plugins/telit/mm-modem-helpers-telit.h| 3 +-
> > plugins/telit/tests/test-mm-modem-helpers-telit.c | 72
> +-
> > 4 files changed, 105 insertions(+), 69 deletions(-)
> >
> > diff --git a/plugins/telit/mm-broadband-modem-telit.c
> b/plugins/telit/mm-broadband-modem-telit.c
> > index cce0229..f316e30 100644
> > --- a/plugins/telit/mm-broadband-modem-telit.c
> > +++ b/plugins/telit/mm-broadband-modem-telit.c
> > @@ -541,13 +541,13 @@ csim_query_ready (MMBaseModem *self,
> > response = mm_base_modem_at_command_finish (self, res, &error);
> >
> > if (!response) {
> > -mm_warn ("No respose for step %d: %s", ctx->step,
> error->message);
> > +mm_warn ("load unlock retries: no respose for step %d: %s",
> ctx->step, error->message);
>
> I don't think we should be printing the "step number", especially not
> via warning. This information would be useful if instead we gave the
> actual lock name associated to the step, though.
>
> As the csim_query_ready() is being reused for multiple locks, we could
> have an array of strings specifying which lock is being processed at
> each step, e.g.:
>
> static const gchar *step_lock_names[LOAD_UNLOCK_RETRIES_STEP_LAST] = {
> [LOAD_UNLOCK_RETRIES_STEP_PIN] = "PIN",
> [LOAD_UNLOCK_RETRIES_STEP_PUK] = "PUK",
> [LOAD_UNLOCK_RETRIES_STEP_PIN2] = "PIN2",
> [LOAD_UNLOCK_RETRIES_STEP_PUK2] = "PUK2",
> };
>
> What do you think? I know this issue was already there, but just spotted
> it :) Maybe handled in a separate new patch better?
>
>
I agree, the "step" thing is just for developer (myself, specifically :D),
and I prefer the idea to use a separate patch.
>
> > g_error_free (error);
> > goto next_step;
> > }
> >
> > -if ( (unlock_retries = mm_telit_parse_csim_response (ctx->step,
> response, &error)) < 0) {
> > -mm_warn ("Parse error in step %d: %s.", ctx->step,
> error->message);
> > +if ( (unlock_retries = mm_telit_parse_csim_response (response,
> &error)) < 0) {
> > +mm_warn ("load unlock retries: parse error in step %d: %s.",
> ctx->step, error->message);
>
> Same here with the step number.
>
right
>
> > g_error_free (error);
> > goto next_step;
> > }
> > diff --git a/plugins/telit/mm-modem-helpers-telit.c
> b/plugins/telit/mm-modem-helpers-telit.c
> > index c8c1f4b..cb3ff24 100644
> > --- a/plugins/telit/mm-modem-helpers-telit.c
> > +++ b/plugins/telit/mm-modem-helpers-telit.c
> > @@ -17,6 +17,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #include
> > #define _LIBMM_INSIDE_MMCLI
> > @@ -113,54 +114,92 @@ mm_telit_get_band_flag (GArray *bands_array,
> >
> > /***
> **/
> > /* +CSIM response parser */
> > +#define MM_TELIT_MIN_SIM_RETRY_HEX 0x63C0
> > +#define MM_TELIT_MAX_SIM_RETRY_HEX 0x63CF
> >
> > gint
> > -mm_telit_parse_csim_response (const guint step,
> > - const gchar *response,
> > +mm_telit_parse_csim
Re: cinterion: modification to fetching unlock retries
Il martedì 18 aprile 2017, Aleksander Morgado ha scritto: > On Tue, Apr 18, 2017 at 9:59 AM, Carlo Lobrano > wrote: > > the examples in the docs do not show quoted strings, but I just gave a > try > > with a couple of modems and I saw no problems. > > Ok, so maybe we should really move the logic to the parent > MMBroadbandModem object and have all modems try the +CSIM based logic. > > Colin; does your modem accept +CSIM=1 / +CSIM=0 to lock/unlock the > CSIM access? It's optional in the Telit plugin, but may be worth > knowing what happens in the Cinterion modem. -- > Aleksander > https://aleksander.es Consider that we can move the "FeatureSupport" logic to the parent, too. If Cinterion do not support CSIM lock ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: cinterion: modification to fetching unlock retries
Hi Aleksander,
the examples in the docs do not show quoted strings, but I just gave a try
with a couple of modems and I saw no problems.
On 15 April 2017 at 00:14, Aleksander Morgado
wrote:
> On Fri, Apr 14, 2017 at 2:57 PM, Colin Helliwell
> wrote:
> >
> > static const UnlockRetriesMap unlock_retries_map_csim [] = {
> > { MM_MODEM_LOCK_SIM_PIN, "+CSIM=10,\"002100\"" },
> > { MM_MODEM_LOCK_SIM_PUK, "+CSIM=10,\"002C000100\"" },
> > { MM_MODEM_LOCK_SIM_PIN2,"+CSIM=10,\"0020008100\"" },
> > { MM_MODEM_LOCK_SIM_PUK2,"+CSIM=10,\"002C008100\"" },
> > };
>
> Carlo, does the Telit modem allow the quoted string given as second
> argument to +CSIM, instead of the currently unquoted ones?
>
> --
> Aleksander
> https://aleksander.es
>
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
[PATCH v3] telit: unsupported CSIM lock should not skip loading unlock retries
Some modems do not support CSIM lock/unlock, but they do support
querying SIM unlock retries through +CSIM command.
If CSIM lock returns with "unsupported command" do not propagate
the error and continue with the other CSIM queries instead, moreover the
CSIM lock feature is signed as FEATURE_UNSUPPORTED in Telit modem
private structure to prevent being sent again (e.g. calling CSIM
unlock AT command).
---
Fixed csim_lock_support never set to FEATURE_SUPPORTED
---
plugins/telit/mm-broadband-modem-telit.c | 94 ++--
plugins/telit/mm-broadband-modem-telit.h | 2 +
2 files changed, 80 insertions(+), 16 deletions(-)
diff --git a/plugins/telit/mm-broadband-modem-telit.c
b/plugins/telit/mm-broadband-modem-telit.c
index cce0229..0d8a34b 100644
--- a/plugins/telit/mm-broadband-modem-telit.c
+++ b/plugins/telit/mm-broadband-modem-telit.c
@@ -42,6 +42,15 @@ G_DEFINE_TYPE_EXTENDED (MMBroadbandModemTelit,
mm_broadband_modem_telit, MM_TYPE
G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM,
iface_modem_init)
G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM_3GPP,
iface_modem_3gpp_init));
+typedef enum {
+FEATURE_SUPPORT_UNKNOWN,
+FEATURE_NOT_SUPPORTED,
+FEATURE_SUPPORTED
+} FeatureSupport;
+
+struct _MMBroadbandModemTelitPrivate {
+FeatureSupport csim_lock_support;
+};
/*/
/* After Sim Unlock (Modem interface) */
@@ -521,10 +530,19 @@ csim_unlock_ready (MMBaseModem *self,
/* Ignore errors */
response = mm_base_modem_at_command_finish (self, res, &error);
if (!response) {
+if (g_error_matches (error,
+ MM_MOBILE_EQUIPMENT_ERROR,
+ MM_MOBILE_EQUIPMENT_ERROR_NOT_SUPPORTED)) {
+ctx->self->priv->csim_lock_support = FEATURE_NOT_SUPPORTED;
+}
mm_warn ("Couldn't unlock SIM card: %s", error->message);
g_error_free (error);
}
+if (ctx->self->priv->csim_lock_support != FEATURE_NOT_SUPPORTED) {
+ctx->self->priv->csim_lock_support = FEATURE_SUPPORTED;
+}
+
ctx->step++;
load_unlock_retries_step (ctx);
}
@@ -591,10 +609,22 @@ csim_lock_ready (MMBaseModem *self,
response = mm_base_modem_at_command_finish (self, res, &error);
if (!response) {
-g_prefix_error (&error, "Couldn't lock SIM card: ");
-g_simple_async_result_take_error (ctx->result, error);
-load_unlock_retries_context_complete_and_free (ctx);
-return;
+if (g_error_matches (error,
+ MM_MOBILE_EQUIPMENT_ERROR,
+ MM_MOBILE_EQUIPMENT_ERROR_NOT_SUPPORTED)) {
+ctx->self->priv->csim_lock_support = FEATURE_NOT_SUPPORTED;
+mm_warn ("Couldn't lock SIM card: %s. Continuing without CSIM
lock.", error->message);
+g_error_free (error);
+} else {
+g_prefix_error (&error, "Couldn't lock SIM card: ");
+g_simple_async_result_take_error (ctx->result, error);
+load_unlock_retries_context_complete_and_free (ctx);
+return;
+}
+}
+
+if (ctx->self->priv->csim_lock_support != FEATURE_NOT_SUPPORTED) {
+ctx->self->priv->csim_lock_support = FEATURE_SUPPORTED;
}
ctx->step++;
@@ -602,6 +632,40 @@ csim_lock_ready (MMBaseModem *self,
}
static void
+handle_csim_locking (LoadUnlockRetriesContext *ctx, gboolean is_lock)
+{
+switch (ctx->self->priv->csim_lock_support) {
+case FEATURE_SUPPORT_UNKNOWN:
+case FEATURE_SUPPORTED:
+if (is_lock) {
+mm_base_modem_at_command (MM_BASE_MODEM (ctx->self),
+ CSIM_LOCK_STR,
+ CSIM_QUERY_TIMEOUT,
+ FALSE,
+ (GAsyncReadyCallback)
csim_lock_ready,
+ ctx);
+} else {
+mm_base_modem_at_command (MM_BASE_MODEM (ctx->self),
+ CSIM_UNLOCK_STR,
+ CSIM_QUERY_TIMEOUT,
+ FALSE,
+ (GAsyncReadyCallback)
csim_unlock_ready,
+ ctx);
+}
+break;
+case FEATURE_NOT_SUPPORTED:
+mm_dbg ("CSIM lock not supported by this modem. Skipping %s
command",
+is_lock ? "lock" : "unlock");
+ctx->step++;
+load_unlock_retries_step (ctx);
+break;
+default:
+g_assert_not_reached ();
+break;
+}
+}
+
+static void
load_unlock_retries_step (LoadUnlockRetriesContext *ctx)
{
switch (ctx->step) {
@@ -609,12 +673
Re: [PATCH v2] telit: unsupported CSIM lock should not skip loading unlock retries
> Is this previous part really needed? Is there any case in which a LOCK
command would succeed but then UNLOCK command report a "not supported"
error? I guess it doesn't harm to have this, though, your call.
Right, there is no real need for this, but just for completeness I prefer
to keep it, if you don't mind
>
Looks like csim_lock_support isn't set anywhere to FEATURE_SUPPORTED? It
should be set here I believe.
Doh!
I'll fix it :D
On 10 April 2017 at 13:25, Aleksander Morgado
wrote:
> On 10/04/17 12:25, Carlo Lobrano wrote:
> > Some modems do not support CSIM lock/unlock, but they do support
> > querying SIM unlock retries through +CSIM command.
> >
> > If CSIM lock returns with "unsupported command" do not propagate
> > the error and continue with the other CSIM queries instead, moreover the
> > CSIM lock feature is signed as FEATURE_UNSUPPORTED in Telit modem
> > private structure to prevent being sent again (e.g. calling CSIM
> > unlock AT command).
> > ---
> >
> > Hi Aleksander,
> >
> > I added private structure to store whether csim lock is supported or not
> (and so
> > skip the command). Let me know if this is ok now.
> >
>
> See below.
>
> > Carlo
> >
> > ---
> > plugins/telit/mm-broadband-modem-telit.c | 86
> ++--
> > plugins/telit/mm-broadband-modem-telit.h | 2 +
> > 2 files changed, 72 insertions(+), 16 deletions(-)
> >
> > diff --git a/plugins/telit/mm-broadband-modem-telit.c
> b/plugins/telit/mm-broadband-modem-telit.c
> > index cce0229..841a48b 100644
> > --- a/plugins/telit/mm-broadband-modem-telit.c
> > +++ b/plugins/telit/mm-broadband-modem-telit.c
> > @@ -42,6 +42,15 @@ G_DEFINE_TYPE_EXTENDED (MMBroadbandModemTelit,
> mm_broadband_modem_telit, MM_TYPE
> > G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM,
> iface_modem_init)
> > G_IMPLEMENT_INTERFACE
> (MM_TYPE_IFACE_MODEM_3GPP, iface_modem_3gpp_init));
> >
> > +typedef enum {
> > +FEATURE_SUPPORT_UNKNOWN,
> > +FEATURE_NOT_SUPPORTED,
> > +FEATURE_SUPPORTED
> > +} FeatureSupport;
> > +
> > +struct _MMBroadbandModemTelitPrivate {
> > +FeatureSupport csim_lock_support;
> > +};
> >
> > /***
> **/
> > /* After Sim Unlock (Modem interface) */
> > @@ -521,6 +530,11 @@ csim_unlock_ready (MMBaseModem *self,
> > /* Ignore errors */
> > response = mm_base_modem_at_command_finish (self, res, &error);
> > if (!response) {
> > +if (g_error_matches (error,
> > + MM_MOBILE_EQUIPMENT_ERROR,
> > + MM_MOBILE_EQUIPMENT_ERROR_NOT_SUPPORTED))
> {
> > +ctx->self->priv->csim_lock_support = FEATURE_NOT_SUPPORTED;
> > +}
>
> Is this previous part really needed? Is there any case in which a LOCK
> command would succeed but then UNLOCK command report a "not supported"
> error? I guess it doesn't harm to have this, though, your call.
>
> > mm_warn ("Couldn't unlock SIM card: %s", error->message);
> > g_error_free (error);
> > }
> > @@ -591,10 +605,18 @@ csim_lock_ready (MMBaseModem *self,
> >
> > response = mm_base_modem_at_command_finish (self, res, &error);
> > if (!response) {
> > -g_prefix_error (&error, "Couldn't lock SIM card: ");
> > -g_simple_async_result_take_error (ctx->result, error);
> > -load_unlock_retries_context_complete_and_free (ctx);
> > -return;
> > +if (g_error_matches (error,
> > + MM_MOBILE_EQUIPMENT_ERROR,
> > + MM_MOBILE_EQUIPMENT_ERROR_NOT_SUPPORTED))
> {
> > +ctx->self->priv->csim_lock_support = FEATURE_NOT_SUPPORTED;
> > +mm_warn ("Couldn't lock SIM card: %s. Continuing without
> CSIM lock.", error->message);
> > +g_error_free (error);
> > +} else {
> > +g_prefix_error (&error, "Couldn't lock SIM card: ");
> > +g_simple_async_result_take_error (ctx->result, error);
> > +load_unlock_retries_context_complete_and_free (ctx);
> > +return;
> > +}
> > }
>
>
> Looks like csim_lock_support isn't set anywhere to
[PATCH v2] telit: unsupported CSIM lock should not skip loading unlock retries
Some modems do not support CSIM lock/unlock, but they do support
querying SIM unlock retries through +CSIM command.
If CSIM lock returns with "unsupported command" do not propagate
the error and continue with the other CSIM queries instead, moreover the
CSIM lock feature is signed as FEATURE_UNSUPPORTED in Telit modem
private structure to prevent being sent again (e.g. calling CSIM
unlock AT command).
---
Hi Aleksander,
I added private structure to store whether csim lock is supported or not (and so
skip the command). Let me know if this is ok now.
Carlo
---
plugins/telit/mm-broadband-modem-telit.c | 86 ++--
plugins/telit/mm-broadband-modem-telit.h | 2 +
2 files changed, 72 insertions(+), 16 deletions(-)
diff --git a/plugins/telit/mm-broadband-modem-telit.c
b/plugins/telit/mm-broadband-modem-telit.c
index cce0229..841a48b 100644
--- a/plugins/telit/mm-broadband-modem-telit.c
+++ b/plugins/telit/mm-broadband-modem-telit.c
@@ -42,6 +42,15 @@ G_DEFINE_TYPE_EXTENDED (MMBroadbandModemTelit,
mm_broadband_modem_telit, MM_TYPE
G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM,
iface_modem_init)
G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM_3GPP,
iface_modem_3gpp_init));
+typedef enum {
+FEATURE_SUPPORT_UNKNOWN,
+FEATURE_NOT_SUPPORTED,
+FEATURE_SUPPORTED
+} FeatureSupport;
+
+struct _MMBroadbandModemTelitPrivate {
+FeatureSupport csim_lock_support;
+};
/*/
/* After Sim Unlock (Modem interface) */
@@ -521,6 +530,11 @@ csim_unlock_ready (MMBaseModem *self,
/* Ignore errors */
response = mm_base_modem_at_command_finish (self, res, &error);
if (!response) {
+if (g_error_matches (error,
+ MM_MOBILE_EQUIPMENT_ERROR,
+ MM_MOBILE_EQUIPMENT_ERROR_NOT_SUPPORTED)) {
+ctx->self->priv->csim_lock_support = FEATURE_NOT_SUPPORTED;
+}
mm_warn ("Couldn't unlock SIM card: %s", error->message);
g_error_free (error);
}
@@ -591,10 +605,18 @@ csim_lock_ready (MMBaseModem *self,
response = mm_base_modem_at_command_finish (self, res, &error);
if (!response) {
-g_prefix_error (&error, "Couldn't lock SIM card: ");
-g_simple_async_result_take_error (ctx->result, error);
-load_unlock_retries_context_complete_and_free (ctx);
-return;
+if (g_error_matches (error,
+ MM_MOBILE_EQUIPMENT_ERROR,
+ MM_MOBILE_EQUIPMENT_ERROR_NOT_SUPPORTED)) {
+ctx->self->priv->csim_lock_support = FEATURE_NOT_SUPPORTED;
+mm_warn ("Couldn't lock SIM card: %s. Continuing without CSIM
lock.", error->message);
+g_error_free (error);
+} else {
+g_prefix_error (&error, "Couldn't lock SIM card: ");
+g_simple_async_result_take_error (ctx->result, error);
+load_unlock_retries_context_complete_and_free (ctx);
+return;
+}
}
ctx->step++;
@@ -602,6 +624,40 @@ csim_lock_ready (MMBaseModem *self,
}
static void
+handle_csim_locking (LoadUnlockRetriesContext *ctx, gboolean is_lock)
+{
+switch (ctx->self->priv->csim_lock_support) {
+case FEATURE_SUPPORT_UNKNOWN:
+case FEATURE_SUPPORTED:
+if (is_lock) {
+mm_base_modem_at_command (MM_BASE_MODEM (ctx->self),
+ CSIM_LOCK_STR,
+ CSIM_QUERY_TIMEOUT,
+ FALSE,
+ (GAsyncReadyCallback)
csim_lock_ready,
+ ctx);
+} else {
+mm_base_modem_at_command (MM_BASE_MODEM (ctx->self),
+ CSIM_UNLOCK_STR,
+ CSIM_QUERY_TIMEOUT,
+ FALSE,
+ (GAsyncReadyCallback)
csim_unlock_ready,
+ ctx);
+}
+break;
+case FEATURE_NOT_SUPPORTED:
+mm_dbg ("CSIM lock not supported by this modem. Skipping %s
command",
+is_lock ? "lock" : "unlock");
+ctx->step++;
+load_unlock_retries_step (ctx);
+break;
+default:
+g_assert_not_reached ();
+break;
+}
+}
+
+static void
load_unlock_retries_step (LoadUnlockRetriesContext *ctx)
{
switch (ctx->step) {
@@ -609,12 +665,7 @@ load_unlock_retries_step (LoadUnlockRetriesContext *ctx)
/* Fall back on next step */
ctx->step++;
case LOAD_UNLOCK_RETRIES_STEP_LOCK:
-mm_base_modem_at_command (MM_BASE_MODEM (ctx->self),
-
Re: [PATCH] telit: unsupported CSIM lock should not skip loading unlock retries
> How about we store the "not supported" information somewhere, like e.g.
> in a "FeatureSupport" enum value as we do in Huawei for other things:
>
https://cgit.freedesktop.org/ModemManager/ModemManager/tree/plugins/huawei/mm-broadband-modem-huawei.c#n79
Yep, I think it's a good idea, I can update the patch with this implemented.
Il giovedì 6 aprile 2017, Aleksander Morgado ha
scritto:
> On 06/04/17 14:37, Carlo Lobrano wrote:
> > Some modems do not support CSIM lock/unlock, but they do support
> > querying SIM unlock retries through +CSIM command.
> >
> > If CSIM lock returns with "unsupported command" do not propagate
> > the error and continue with the other CSIM queries instead.
> > ---
> >
> > Sorry for not having caught this problem earlier,
> > when reviewing the CSIM lock patch.
> >
>
> Yes, this is a good logic change that we should have done...
>
> How about we store the "not supported" information somewhere, like e.g.
> in a "FeatureSupport" enum value as we do in Huawei for other things:
> https://cgit.freedesktop.org/ModemManager/ModemManager/
> tree/plugins/huawei/mm-broadband-modem-huawei.c#n79
>
> i.e. when the object is created:
> self->priv->csim_lock_supported = FEATURE_SUPPORT_UNKNOWN;
>
> Them, after the first time we try the CSIM lock operation, we store the
> proper result (e.g. FEATURE_NOT_SUPPORTED or FEATURE_SUPPORTED).
>
> Then, the next time we are going to do a CSIM lock (or unlock)
> operation, we check whether the feature is supported or not, and if it
> isn't, we just ignore the operation right away.
>
> What do you think? This would help to also avoid trying the CSIM unlock
> operation if we already know CSIM lock is unsupported.
>
>
> > ---
> >
> > plugins/telit/mm-broadband-modem-telit.c | 15 +++
> > 1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/plugins/telit/mm-broadband-modem-telit.c
> b/plugins/telit/mm-broadband-modem-telit.c
> > index cce0229..3680a8a 100644
> > --- a/plugins/telit/mm-broadband-modem-telit.c
> > +++ b/plugins/telit/mm-broadband-modem-telit.c
> > @@ -591,10 +591,17 @@ csim_lock_ready (MMBaseModem *self,
> >
> > response = mm_base_modem_at_command_finish (self, res, &error);
> > if (!response) {
> > -g_prefix_error (&error, "Couldn't lock SIM card: ");
> > -g_simple_async_result_take_error (ctx->result, error);
> > -load_unlock_retries_context_complete_and_free (ctx);
> > -return;
> > +if (g_error_matches (error,
> > + MM_MOBILE_EQUIPMENT_ERROR,
> > + MM_MOBILE_EQUIPMENT_ERROR_NOT_SUPPORTED))
> {
> > +mm_warn ("Couldn't lock SIM card: %s. Continuing as
> well...", error->message);
> > +g_error_free (error);
> > +} else {
> > +g_prefix_error (&error, "Couldn't lock SIM card: ");
> > +g_simple_async_result_take_error (ctx->result, error);
> > +load_unlock_retries_context_complete_and_free (ctx);
> > +return;
> > +}
> > }
> >
> > ctx->step++;
> >
>
>
> --
> Aleksander
> https://aleksander.es
>
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
