itm / wsn-device-drivers

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

Use device.shutdown() instead of connection.shutdown() #20

Closed psotres closed 13 years ago

psotres commented 13 years ago

Device interface should include a shutdown() method that should be used instead of connection.shutdown() or Closeables.closeQuietly(Connection) when deleting the device.

mlegenhausen commented 13 years ago

When I understand you right you simply want a clean up method in the device to make some tear down operations?

psotres commented 13 years ago

In GenericDeviceExample you call a shutdown() method where you use Closeables.closeQuietly(connection). When a multiplexed device is used, there are some things to do prior to decide if you have to call this function, so it will better to call device.shutdown() and let each device decide how it shutdowns

I'm not sure what TR does when disconnecting or shutting down a device, but if it does something similar to GenericDeviceExample, it should use something like a device.shutdown() instead of connection.shutdown().

Just to sumarize,

I suggest Device interface should implement a shutdown() method and let each device implementation decide how to clean up. This method should be used whenever a device has to be shutdown.

mlegenhausen commented 13 years ago

You can do this by listening to the Connection. When the connection is closed you get an event. When it is not granular enough, we can add more listener stages like beforeConnectionClosed(). Cause for me there is no cause to add a close method to the device. Cause normally there is nothing that has to be shutdown.

psotres commented 13 years ago

Maybe I didn't explain very well, sorry. I was meaning exactly the opposite: you cannot close a connection (a RXTX real serial port connection, I mean) of a multiplexed device in a normal way because other devices can be listening at the exact same connection too.

I don't exactly know when TR really closes the connection of a device. If it only does it when shutting down the whole system there shouldn't be any problem. However, if it closes the connection when disabling a device (just like the GenericDeviceExample on wsn-device-drivers-core does), it's probably that for multiplexed devices this behavior won't work.

The equivalent of closing the connection for a non-multiplexed device in a multiplexed one is something like "remove the current device of the demultiplexer routing table, then check if any other device if still listening in this routing table and, if not, close the connection"

This is the reason because I believe it has more sense to implement that shutting down functionality at device level instead of handle it in a raw way in a higher level, but I'm seeing it from the point of view of how I implemented lower layers of a multiplexed driver, maybe with your modifications this is not a problem anymore.

mlegenhausen commented 13 years ago

I talked about that with Florian and we decided to make it the following way. The Multiplexer simply creates dedicated virtual connections for the devices. So you can treat each device as it is simply connected to a SerialPort.

Example:

SerialPortConnection connection = new SerialPortConnection();
connection.connect("COM1");

Multiplexer multiplexer = new Multiplexer(connection);
WasmoteConnection con1 = multiplexer.createConnection("NodeId1", "Mac1");

WaspmoteDevice device = new WaspmoteDevice(con1);
// Use the Device as normally.

con1.close(); // The new name of the shutdown method.

So we simply added a thin layer the rest behaives like the other drivers.

mlegenhausen commented 13 years ago

Cause for me there is no sense at the moment to add a close method to the device, cause the device is only to be intended to be a operation factory and should normally not have a state, I will close this issue.