[Rock-dev] Regarding dynamic properties

[Matthias Goldhoorn]

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:  matthias.goldhoorn at informatik.uni-bremen.de

  Weitere Informationen: http://www.informatik.uni-bremen.de/robotik