Ciao Aleksander, On 22 May 2017 at 13:18, Aleksander Morgado <aleksan...@aleksander.es> 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: > <debug> (ttyACM3) device open count is 1 (close) > <debug> (ttyACM0) device open count is 0 (close) > <debug> (ttyACM0) closing serial port... > <debug> (ttyACM0) serial port closed > <debug> (ttyACM3) device open count is 0 (close) > <debug> (ttyACM3) closing serial port... > <debug> (ttyACM3) serial port closed > <info> Modem /org/freedesktop/ModemManager1/Modem/0: state changed > (disabling -> disabled) > <debug> [device /sys/devices/pci0000:00/0000:00:1a.7/usb1/1-3] > unexported modem from path '/org/freedesktop/ModemManager1/Modem/0' > <debug> (ttyACM3) forced to close port > <debug> (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: > <info> Modem /org/freedesktop/ModemManager1/Modem/0: 3GPP > Registration state changed (home -> unknown) > <debug> (ttyACM3) device open count is 2 (close) > <debug> (ttyACM0) device open count is 1 (close) > <debug> (ttyACM3) device open count is 1 (close) > <info> Modem /org/freedesktop/ModemManager1/Modem/0: state changed > (disabling -> disabled) > <debug> [device /sys/devices/pci0000:00/0000: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