itm / wsn-device-drivers

Drivers for Wireless Sensor Network Devices
Other
6 stars 4 forks source link

Guicefy the Drivers #35

Closed mlegenhausen closed 13 years ago

mlegenhausen commented 13 years ago

Devide the components in such a way that you can define a device as a GuiceModule. This should allow to remove the Device Interface, cause the factories are provided by guice, allow a much better loosely coppling and hopefully a better testability ;)

danbim commented 13 years ago

Are you sure you mean to remove the Device interface? Or do you mean to remove the DeviceFactories?

mlegenhausen commented 13 years ago

When looking at the Device class you see all the create methods. When using guice these are not more necessary. All the other methods that are device specific can be put in another class or in the Connection. Instead of using the Device class we can then use the Injector.

danbim commented 13 years ago

Ah, right. I was thinking about DeviceAsync all the time ;-) Anyway, to be honest, I think the Device interface was obsolete anyway as the DeviceAsync interface (which could then be renamed to Device) provides both synchronous and asynchronous methods to interact with the device. (Synchronous through Future.get()...)

mlegenhausen commented 13 years ago

The target of this issue:

public class JennicDeviceModule extend AbstractModule {
    @Override
    protected void configure() {
        bind(Connection).to(iSenseSerialPortConnection.class).in(Singleton.class);
        bind(ProgramOperation.class).to(JennicProgramOperation.class);
        ...
        bind(OperationQueue.class).to(PausableExecutorOperationQueue.class).in(Singleton.class);
        bind(DeviceAsync.class).to(QueuedDeviceAsync.class).in(Singleton.class);
    }
}

This will replace the Device class and we can create the DeviceAsync as follows:

Injector injector = Guice.createInjector(new JennicDeviceModule());
injector.get(Connection.class).connect("COM21");
DeviceAsync deviceAsync = injector.get(DeviceAsync.class);

DeviceAsync can then be renamed to Device.

danbim commented 13 years ago

Btw... I always wonder why I, as the user have to deal with the Connection itself. My only interaction with it is to call connect(). Wouldn't it be easier to have that in the Device(Async) class which then creates the appropriate connection and calls the connect method?

mlegenhausen commented 13 years ago

You are right. We can add facade methods. Like connect and close. Close can then also shutdown the queue.so after module configuration you only have too deal with a slim interface. When you then want more you can use the getters.

mlegenhausen commented 13 years ago

Optimized example:

Injector injector = Guice.createInjector(new JennicDeviceModule());
DeviceAsync deviceAsync = injector.get(DeviceAsync.class);
deviceAsync.connect("COM21");
...
// Will shutdown queue and close connection.
deviceAsync.shutdown(true);

Or another/addional version:

DeviceAsync deviceAsync = new GuiceDeviceAsync(new JennicDeviceModule());

How about this?

danbim commented 13 years ago

I would still like to be able to optionally (!) pass the ExecutorService that is used internally. So, the code could like like this:

ExecutorService executorService = Executors.newCachedThreadPool();
Injector injector = Guice.createInjector(new JennicDeviceModule(executorService));
DeviceAsync deviceAsync = injector.get(DeviceAsync.class);
deviceAsync.connect("COM21");
...
// Will shutdown queue and close connection.
deviceAsync.shutdown(true);
// ... shutdown executorService manually when not used anymore ...
danbim commented 13 years ago

Also I would like to be able to get the correct device interface based on some string value like it is today with the DeviceFactory / ConnectionFactory as client applications rely on this. So I would ask for the Device(Async) interface with String "isense" and get a JennicDevice and so on...

mlegenhausen commented 13 years ago

I gucified all drivers. The null device is removed cause it is now useless. How about the Factories? When I understand you right you only needed a String to Guice Modul class? Or what are other wishes?

Another improvement: I will DeviceAsync extend from Connection, so we have a full Facade.

danbim commented 13 years ago

String to Guice Module class... yes, I guess that is what I need if understand correctly what you mean. Basically, what I need is the possibility to instantiate an (Async)Device which is then automatically of the right concrete type e.g., iSense. I have the possibility to pass a String e.g., "isense" to that "Factory"/Injector/Whatever to indicate what I want though.

mlegenhausen commented 13 years ago

Guicification is mostly done. When something is missing please open a new more specific issue. Devices can be created by String or DeviceType via the DeviceFactory.