On 09.01.2014 09:52, Sylvain Joyeux wrote: > Comments: > > The separation between the internal method called in caller thread and the > user method in callee thread looks overkill. Other suggestions? > > More rational reasons as to why it is bad: > - it has a race condition. Since the operation and the task are in two > different threads, isConfigured() might be e.g. true when you test it and > false one line of code later. Missed this one thank's > - when the component is not running, there can be no lock contention that > blocks the caller for a long period of time (the component is doing > nothing). Even if it was the case, one can call operations asynchronously. Don't get this, i assume that you mean the task could not block if it's not running. If the deployed task uses the task-scheduler, then behavior was indeed that the task blocks because it's not executed at all. This causes the normal configuration methods (throught syskit/orocos.rb) to hang.
> - when the component is running, the lock contention can still be present in > your version as you call the user operation Known, but orogen calls (during configure) the right one.What the user still does is out of out scope. > - what you did is extremely expensive compilation-time wise as you have to > have two operations and do a C++ operation call. As Mentioned before, other suggestions? > - it is ugly on the component interface side as it makes the internal > operation visible (instead of having a single setXXX operation) No idea howto archive a thread change otherwise > > Quick code review: > - you introduce a change of behaviour in argument_signature: you add either > a > leading or trailing space in the returned string if one of the with_ > parameters are false. which should make no difference?! (could use strip otherwise) > - please use __orogen_ as prefix for the internal method (instead of the > _internal suffix), it would match the 'hidden' __orogen_getTID operation > that > is already added by orogen. ack > > Finally, I did not get the change of interface for the user method(s), and why > it was necessary Before there was a call to the base-class implementation done, the base function is now not needed anymore because the property get's updates by the internal function. So the user-code could simply return true which makes the core more straight forward. -- Dipl.-Inf. Matthias Goldhoorn Space and Underwater Robotic Universität Bremen FB 3 - Mathematik und Informatik AG Robotik Robert-Hooke-Straße 1 28359 Bremen, Germany Zentrale: +49 421 178 45-6611 Besuchsadresse der Nebengeschäftstelle: Robert-Hooke-Straße 5 28359 Bremen, Germany Tel.: +49 421 178 45-4193 Empfang: +49 421 178 45-6600 Fax: +49 421 178 45-4150 E-Mail: [email protected] Weitere Informationen: http://www.informatik.uni-bremen.de/robotik _______________________________________________ Rock-dev mailing list [email protected] http://www.dfki.de/mailman/cgi-bin/listinfo/rock-dev
