Re: Bluetooth support

[Dan Williams <dcbw redhat com>]

On Tue, 2009-01-13 at 19:11 +0200, Antti Kaijanmäki wrote:
> On Mon, 2009-01-12 at 13:14 -0500, Dan Williams wrote:
> > On Mon, 2009-01-12 at 09:29 +0200, Antti Kaijanmäki wrote:
> > > On Thu, 2009-01-08 at 13:26 -0500, Dan Williams wrote:
> > > > > DUN on the other hand can not be made as a new device class as it's just
> > > > > a way to get a serial device for NMGsmDevice and NMCdmaDevice. It would
> > > > > be clear to separate NMSerialDevice to NMHalSerialDevice,
> > > > > NMBluetoothSerialDevice, and NMPathSerialDevice (for arbitrary paths
> > > > > like '/dev/ttyS0' or '/dev/foo/bar/proprietary69'), but because
> > > > > NMGsmDevice and NMCdmaDevice are derived directly from NMSerialDevice we
> > > > > can't do the split; that would lead to NMBluetoothGsmDevice,
> > > > > NMHalGsmDevice, NMHalCdmaDevice, etc and also break the current device
> > > > > type enumeration.
> > > > > 
> > > > > Instead I suggest that we handle the different serial types inside
> > > > > NMSerialDevice. Let's add a 'type' option to serial configuration.
> > > > > NMSerialDevice would then handle these different types internally. The
> > > > > different types would be specified as an enumeration like {hal,
> > > > > bluetooth, path, irda, ...} The option would be optional and default to
> > > > > HAL.
> > > > 
> > > > Or, make the GSM and CDMA bits GInterfaces instead that any particular
> > > > device can implement...  That's the cleaner route actually.
> > > 
> > > Wouldn't we still end up with HalGsmDevice, BtGsmDevice, ..., which
> > > would implement either GSM or CDMA interfaces? The only difference
> > > between e.g. HalGsmDevice and BtGsmDevice is the way how serial device
> > > is discovered and initialized and all GSM funtionality would have to be
> > > duplicated.. 
> > 
> > No, because you would make the GSM and CDMA code a GInterface which the
> > straight serial or BT class would implement, but of course all the
> > common code can still live in the GInterfaces.  As you correctly point
> > out, the behavior is the same once you have a serial port, so I doubt
> > the BT class would need to override much of anything in the
> > NMGsmInterface or NMCdmaInterface code.  GInterfaces are slightly
> > different than Java, for example, in that the interface itself can also
> > have a bunch of code and the class that implements it only needs to
> > override that code if necessary.
> 
> OK, sounds good. I was thinking about Java style interfaces, but now I
> see your point.
> 
> 
> > > IMO the right place for abstraction would be NMSerialDevice; we could
> > > turn that to a GInterface. We can't derive NMGsmDevice and NMCdmaDevice
> > > directly from GInterface, but we could have the NMSerialDevice as a
> > > member of those both classes. Then on device creation time just specify
> > > which class implementing NMSerialDevice interface you actually want. Of
> > > course this would prevent us from casting e.g. NMGsmDevice directly to
> > > NMSerialDevice and thus break backward compatibility;
> > > instead of  NM_SERIAL_DEVICE (foo)->bar ();
> > > we would do NM_GSM_DEVICE (foo)->get_serial_device ()->bar(); 
> > 
> > NMSerialDevice may well be the best place to put the abstraction, but I
> > don't think it needs to be done as you've described it.  Most of
> > NMSerialDevice will go away pretty soon and be replaced with
> > ModemManager, thus there will be a *lot* less code there that needs to
> > be shared.  All that's left is PPP handling.  We may well be able to get
> > away with killing NMSerialDevice as a class in the first place, and
> > putting its functionality into a small helper file instead.
> 
> 
> > Then we're left with NMBluetoothDevice, from which is derived
> > NMBTGsmDevice and NMBTCdmaDevice, and the old NMGsmDevice and
> > NMCdmaDevice which all just talk to ModemManager for the device setup,
> > and share the common PPP handler code.
> 
> I attached a quick UML chart; is that something we are after here?
> First off there's new device classes NMDeviceKitDevice and
> NMBluetoothDevice that indicate where the devices are coming from. Of
> course that might be over engineering, but anyway..

I'll take a look...

> Now we are focusing on GSM and CDMA so there are NMGsmInterface and
> NMCdmaInterface which pretty much contain the code currently present in
> NMGsmDevice and NMCdmaDevice. Then there are corresponding
> NMBluetoothGsmDevice, NMDeviceKitGsmDevice, NMBluetoothCdmaDevice and
> NMDeviceKitCdmaDevice implementing the interfaces.

Yeah, though given that we've already had a few different HAL-type
things over the lifetime of NM, I'm a bit reluctant to use "DeviceKit"
in the HAL/DeviceKit/udev discovered devices.

> I don't know.. The chart seems kind off messy. Would there actually be
> any differences between, say, NMDeviceKitCdmaDevice and
> NMDeviceKitGsmDevice; both just look at the device properties from
> DeviceKit to determine the path to serial port, what else?

There may be later on, though if more functionality is moved to
ModemManager the difference between the two gets less.  Still makes some
sense to me to keep them separate for now, and they will be pretty small
if we do our jobs correctly.

> Oh, and btw, I added NMSerialGsmDevice and NMSerialCdmaDevice for serial
> devices with arbitrary serial port paths. Settings would be something
> like:

Arbitrary device node entry shouldn't be part of NM anywhere.  Nobody
should have to type that in, nor does a field like that have any place
in a UI.  The only type that *might* require that is old-school
serial-attached RS232 modems for 56k POTS dialup, and those should get
handled through directed probing instead.  NM would then pick up the
udev capabilities of the device automatically and add it to the internal
device list.

Dan