Bug 100783

Created attachment 131018 [details] [review]
Patch highlighting the issue

The attached patch - based on one from Dan - both highlights the issue (printing "######## parallel enables!" when it occurs) and resolves it. Whether it is the most effective solution, I don't know.

Comment on attachment 131018 [details] [review]
Patch highlighting the issue

Review of attachment 131018 [details] [review]:
-----------------------------------------------------------------

::: src/mm-broadband-modem.c
@@ +9461,5 @@
> +    g_object_get (self,
> +                  MM_IFACE_MODEM_STATE, &state,
> +                  NULL);
> +
> +    if ((state < MM_MODEM_STATE_ENABLED) || (state > MM_MODEM_STATE_REGISTERED)) {

Why would it be an error if the final state is > REGISTERED? If it got connected while we were waiting for a final state, I'd say the modem is at least in the "enabled" state, which wouldn't be an error.

(In reply to Aleksander Morgado from comment #2)
> Comment on attachment 131018 [details] [review] [review]
> Patch highlighting the issue
> 
> Review of attachment 131018 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: src/mm-broadband-modem.c
> @@ +9461,5 @@
> > +    g_object_get (self,
> > +                  MM_IFACE_MODEM_STATE, &state,
> > +                  NULL);
> > +
> > +    if ((state < MM_MODEM_STATE_ENABLED) || (state > MM_MODEM_STATE_REGISTERED)) {
> 
> Why would it be an error if the final state is > REGISTERED? If it got
> connected while we were waiting for a final state, I'd say the modem is at
> least in the "enabled" state, which wouldn't be an error.

Just caution on my part. Dan originally suggested 'if (state != MM_MODEM_STATE_ENABLED)' , but I found this wasn't passing because state had got to REGISTERED. So, to be on the safe side (unsure of possible bad effects e.g. on DISCONNECTING), I just 'widened' it slightly.
As I say, the patch isn't necessarily *the* proposed solution - but does at least help illustrate the issue.

(In reply to colin.helliwell from comment #3)
> (In reply to Aleksander Morgado from comment #2)
> > Comment on attachment 131018 [details] [review] [review] [review]
> > Patch highlighting the issue
> > 
> > Review of attachment 131018 [details] [review] [review] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: src/mm-broadband-modem.c
> > @@ +9461,5 @@
> > > +    g_object_get (self,
> > > +                  MM_IFACE_MODEM_STATE, &state,
> > > +                  NULL);
> > > +
> > > +    if ((state < MM_MODEM_STATE_ENABLED) || (state > MM_MODEM_STATE_REGISTERED)) {
> > 
> > Why would it be an error if the final state is > REGISTERED? If it got
> > connected while we were waiting for a final state, I'd say the modem is at
> > least in the "enabled" state, which wouldn't be an error.
> 
> Just caution on my part. Dan originally suggested 'if (state !=
> MM_MODEM_STATE_ENABLED)' , but I found this wasn't passing because state had
> got to REGISTERED. So, to be on the safe side (unsure of possible bad
> effects e.g. on DISCONNECTING), I just 'widened' it slightly.
> As I say, the patch isn't necessarily *the* proposed solution - but does at
> least help illustrate the issue.

A modem in REGISTERED state is already ENABLED, so I wouldn't consider that an error, especially since the ENABLED->REGISTERED transition is automatic.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/mobile-broadband/ModemManager/issues/65.