On Tue, 2017-04-18 at 17:49 +0200, Aleksander Morgado wrote: > On Tue, Apr 18, 2017 at 4:58 PM, Dan Williams <d...@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 _______________________________________________ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
