itm / wsn-device-drivers

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

Timed out operation can harm next running operation #81

Closed mlegenhausen closed 12 years ago

mlegenhausen commented 12 years ago

Problem: When an operation timed out and is canceled, it can happen, that the finally block for leaving the programming mode is executed while the next operation is already running. This let this operation fail.

Solution: Add a global lock to the SerialPortConnection which includes a prepare() and release() method which has to be called before any operation on the serialport is happening.

Advanded solution: Add annotation support to the Operation container. This will allow to hook in annotation inceptors that can run code before and after the run method of the OperationRunnable is called. This will also allow to enter the programming mode once instead for each sub operation. A very useful annotation would be @Program. This will allow to use the lock without calling prepare and release in the operation itself, cause it is handled by the interceptor.

danbim commented 12 years ago

Must the leave-programming-mode operation be executed in a final block? The advanced solution (although not fully understood) seems a little overengineered to me. Please bear in mind that the overall architecture should still be easily understood. Therefore, I vote for the simple solution.

mlegenhausen commented 12 years ago

Yes it has to be, cause when the operation is timed out and the programing mode is not left the program on the device is not executed and a manually leave of the programming mode is not possible. Cause the SerialPortConnection is shared between several operations and is not able to handle concurrent access I think a lock is needed (to make it completely save! See the problem with the finally-blocks).

A current realisation idea is to add these two methods to the serialportconnection and to use the AOP capabilities of Guice. http://code.google.com/p/google-guice/wiki/AOP

And a big advantage is that the redundant code in all the operations is gone + a performance benefit.

mlegenhausen commented 12 years ago

A timed out operation now maximum harms the next operation, cause of maybe occuring timing problems. But the will not harm the whole queue. Can may be optimized later.

There is now a @Program Annotation that can be used to switch a method in programming mode. Reduces the amount of boiler plate code.

danbim commented 12 years ago

What about naming the @Program annotation @ProgrammingMode? Why is the annotation on the run-method and not on the operation class? Is this intentionally? I would have thought that if I'm going to write an operation, abstracted as a separate operation class I would put the annotation there.

Please add some documentation to the annotation, how to use it and how the system is working. Otherwise it's hard to understand for contributers and project partners not involved in the discussions.

danbim commented 12 years ago

When the whole feature is done, please write an email to the testbed-runtime mailing list explaining these changes so that everybody is notified of these architectural changes.

mlegenhausen commented 12 years ago

Renaming can be done of cause for better understanding.

At first it was intended as class annotation but it was to inflexible. A method annotation has the advantage that only certain methods can be annotated which is used for example in the program operation. Where the reset operation is not executed in programming mode. So you can delay the entrance of the programming mode as long as possible.

A class annotation has to be evaluated again. This could mean all methods of the class has to executed in programming mode.

mlegenhausen commented 12 years ago

See Issue #83