Re: Comments about the proposed new MM D-Bus API

[Dan Williams <dcbw redhat com>]

On Fri, 2011-05-20 at 00:05 +0200, Aleksander Morgado wrote:
> Hi Dan & list,
> 
> My comments about the proposed new D-Bus API:
> 
> 1) Regarding frequency/band management:
>  * There is now AllowedBands and SupportedBands properties in the API,
> but no GetBand() or SetBand(). So really seems the only way to set
> specific bands is through writing the AllowedBands property. I don't
> think its a good idea, as we really want proper error reporting when
> trying to set bands: device may not allow to reset bands while connected
> for example, or the specified band mask may not be supported by the

Yeah, that's a good point.  There might also be ways of working about
dbus-glib/GObject properties here and returning an error anyway; but in
the end I don't mind explicit getters/settings in addition to
properties.

> modem. Another option is to silently discard all those possible errors
> when trying to set an invalid band configuration in the property... :-/
>  * Regarding SupportedBands being a mask. Some devices do support wide
> bitmask-like band configurations; but some others just support a closed
> list of masks, like: "band1 and band2", "band3 and band4", "band1,
> band2, band3 and band4", and maybe they do not support "band1 and band3"
> kind of config. But indeed it's very difficult to provide a generic way
> of covering all cases, so the bitmask is probably the best approach. For
> the cases where specific band masks are supported, we could just perform
> a bitwise OR of all supported masks, like:
>    "band1 and band2" | \
>    "band3 and band4" | \
>    "band1, band2, band3 and band4" = "band1, band2, band3 and band4"
> And then, let SetBand() really check the passed specific band mask, and
> return an error if not allowed (like when passing "band1 and band3" in
> the above example.

Right; though I'd expect for many modems we'd simply leave the band
stuff unimplemented since they don't support setting specific bands, or
they'd take too much trouble to implement.

> 2) I would definitely remove GetInfo() from the modem API, and leave
> manufacturer, model and version/revision as independent read-only
> properties set during construction. This was also like that in the
> previous API, maybe there's a reason for that that I don't know.

Ok, that can be done.  One reason most of the items were function calls
in the first place was that they called directly into the modem; while
now ModemManager does a lot more inspection and caching up front.

> 3) I didn't dig much in the CDMA-specific API as I know nothing about
> CDMA, so no comments there.
> 
> 4) In general, given that this is not only a refactor as new things are
> being introduced in the API, I would suggest to keep an up-to-date
> migration table for all methods and properties, so that it can easily be
> checked what was migrated, what was removed and what was newly added.
> Once that list of methods/properties is done, we could also use it as a
> matrix to identify which features are supported by which plugin (e.g.,
> not all plugins support configuring bands). I wouldn't mind to setup
> that matrix in some wiki if you agree it's a good idea.

Good point.

Dan