[PATCH v2] broadband-bearer: once connected, set flow control settings

[Dan Williams]

On Tue, 2017-04-18 at 17:49 +0200, Aleksander Morgado wrote:
> On Tue, Apr 18, 2017 at 4:58 PM, Dan Williams <dcbw at redhat.com>
> wrote:
> > > > > I made the flow control setting a readable property in the
> > > > > Modem
> > > > > object, and a RW property in the Bearer object. The property
> > > > > is
> > > > > inherited by the bearer object when it is created, i.e. in
> > > > > mm_broadband_bearer_new() we just read the property from the
> > > > > modem
> > > > > and we set it in the bearer creation.
> > > > > 
> > > > > I thought of g_object_bind_property()-ing them whenever the
> > > > > "modem"
> > > > > object was set as property in the "bearer" object, but then
> > > > > realized
> > > > > that happens in "MMBaseBearer" level not in
> > > > > "MMBroadbandBearer",
> > > > > so
> > > > > decided not to complicate it more; especially since the
> > > > > property
> > > > > won't change any more.
> > > > > 
> > > > > What do you think?
> > > > 
> > > > Looks OK to me, though if we're not exposing the flow control
> > > > via
> > > > the
> > > > D-Bus API on the Modem object, should we just not bother making
> > > > it
> > > > a
> > > > property there?  It's not used anywhere yet...
> > > 
> > > After moving the flow control setting to the bearer creation
> > > method I
> > > needed a way to receive the actual setting value; and definitely
> > > didn't want to modify the mm_broadband_bearer_new() parameters
> > > with a
> > > new MMFlowControl one (as that method is used in lots of plugins
> > > as
> > > well). So, at the end I did need a way to get information from
> > > the
> > > modem object from within the bearer object, so instead of having
> > > a
> > > mm_broadband_modem_get_connected_flow_control() method as in the
> > > first
> > > patch, I ended up with the read-only property... too convoluted
> > > maybe?
> > 
> > mm_broadband_bearer_new() is only used in 9 places though
> > (including
> > the 2 hits in mm-broadband-bearer.c/h itself).  Not too many for me
> > to
> > say we shouldn't bother changing it?
> 
> Well, the thing is that we would then have 9 places calling
> mm_broadband_modem_get_connected_flow_control() instead of one (as
> the
> modem subclasses in the plugin don't have direct access to
> self->priv->flow_control in the MMBroadbandModem... don't know, I'm
> not very convinced with any of the solutions, truth be told.

Ok, I'm convinced.  Just go with your patch.

Dan