[review] https://github.com/cbchan/ModemManager/tree/sim-mbim-unlock-retries

[Aleksander Morgado]

Hey,

On Tue, Aug 8, 2017 at 1:06 AM, Ben Chan <benchan at chromium.org> wrote:
> This branch contains a series of patches to address the following
> issue observed on MBIM modems:
>
> After MMSimMbim performs a MBIM_CID_PIN set operation, it calls
> mm_iface_modem_update_lock_info() (through its base class MMBaseSim)
> to refresh the unlock retries information, which results in a
> MBIM_CID_PIN query. However, a MBIM_CID_PIN query reports only the
> information of one PIN type and the PIN type can't be specified, we
> need to deduce the number of retries left for a specific PIN type from
> the response of a MBIM_CID_PIN set operation for that PIN type.
>
> https://github.com/linux-mobile-broadband/ModemManager/compare/master...cbchan:sim-mbim-unlock-retries

It really is unfortunate that the MBIM_CID_PIN query only reports the
information of the "currently applicable" PIN type, although the truth
is that that's the only useful one.

This change you're proposing makes the unlock retries updated on the
send PIN completion, e.g. if it fails we get right away the remaining
attempts of the next PIN applicable (either the same one or another
one). I'm not sure the PIN type included in the response is actually
the same one that was sent in the request, I understand it may not
need to be, although didn't test. E.g. I'm assuming if the last
SIM-PIN attempt is sent we may get back in the response SIM-PUK and
the number of attempts for that one (instead of SIM-PIN attempts 0).

Before this change, we had the unlock retries updated as part of
update_lock_info() triggered from the IfaceModem, and this happens
after sending PIN and in some other cases as well.

In either case, we're completely replacing the UnlockRetries exposed
in the IfaceModem interface; and actually, if you replace the
UnlockRetries in the send completion, wouldn't it be anyway
overwritten as part of update_lock_info() just a bit later?


-- 
Aleksander
https://aleksander.es