Re: [PATCH 2/7] extended support for LTE and NR. Some minor fixes. part 2 of 7

[Giacinto Cifelli <gciofono@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2459 bytes --]

Hi Denis,

sorry for writing on top, but I don't see a good place in the middle.

The idea would be to just store the values in the driver but not in the
module,
and then send them before attaching, so that the verification can be
consistent and all,
just like for the gprs-context atom.

so the idea would be to check if a new method, that I have tentatively
called set_property(),
exists. If so, use the reviewed stile above, otherwise fallback to the
current function.

Giacinto

On Thu, Sep 20, 2018 at 6:03 PM Denis Kenzior <denkenz@gmail.com> wrote:

> Hi Giacinto,
>
> > static DBusMessage *lte_set_property(DBusConnection *conn,
> > DBusMessage *msg, void *data)
> > {
> > [...]
> > if (lte->driver->set_property)
> > lte->driver->set_property(...);
>

> What is this buying you?
>
> > else if (lte->driver->set_default_attach_info)
> > lte->driver->set_default_attach_info(...);
> > else
> > return __ofono_error_not_implemented(msg);
> > [...]
> >
> > in src/modem.c:
> >
> > static void sim_state_watch(enum ofono_sim_state new_state, void *user)
> > {
> > [...]
> > case OFONO_SIM_STATE_READY:
> > modem_change_state(modem, MODEM_STATE_OFFLINE);
> >
> > ofono_lte_set_reginfo(modem);
>
> How does this help you?  You might be in the OFFLINE state for a while
> and the LTE settings might still be messed with in that time.
>
> >
> > /* Modem is always online, proceed to online state. */
> > if (modem_is_always_online(modem) == TRUE)
> > set_online(modem, TRUE);
> >
> > if (modem->online == TRUE)
> > modem_change_state(modem, MODEM_STATE_ONLINE);
> > else if (modem->get_online)
> > modem->driver->set_online(modem, 1, common_online_cb,
> > modem);
> >
> > modem->get_online = FALSE;
> >
> > break;
> > }
> >
> > I see that if(modem_is_always_online) maybe it will come too late, but
> > in that case also the current solution
> > with set_default_attach_info suffers from the same limitation, so no
> > regression.
>
> The modem_is_always_online case is a bit strange, it is for really
> primitive devices which likely won't have an LTE atom anyway.
>
> >
> > Of course then, again in src/lte.c, the function ofono_lte_set_reginfo
> > will look for an lte atom, if present will call it without parameter for
> > sending the AT+CGDCONT and AT+CGAUTH to the modem.
> >
> > Would this work for you?
> >
>
> I don't see how this helps.
>
> Regards,
> -Denis
>

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 3205 bytes --]