Here is a proposal for a patch for the changes we discussed.
Change recommends are welcome,
Matthias
P.S. note:
this will need a few adaptations for packages that already use dynamic
props.
The Return value of the user function has no to be simply true (or
false) and is not longer a call to the base class.
If you agree to the patch i take care of modifying all affected packages.
--
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
>From 19c254e06668ef0490df30113eeba906e4bf86fa Mon Sep 17 00:00:00 2001
From: Matthias Goldhoorn <[email protected]>
Date: Wed, 8 Jan 2014 13:08:50 +0100
Subject: [PATCH] Modified handling of dynamic-properties:
The old behaviour was that a dynamic property get called every time.
Now the set_<name>_internal function get's called instead of the
propery itself. This is done within the caller thread to prevent
a lock if the task is scheduled external.
If the task is already configured then the user setter function get's
called WModified handling of dynamic-properties:
The old behavior was that a dynamic property get called every time.
Now the set_<name>_internal function get's called instead of the
propery itself. This is done within the caller thread to prevent
a lock if the task is scheduled external.
If the task is already configured then the user setter function get's
Called WITHIN the user thread (which means the scheduler must run).
The user has to make sure that ALL dynamic properties get's read within the
Configure hook. The setter function does NOT get called if the task is so far
unconfigured.
ITHING the user thread (which means the sceduler must run).
The user has to make sure that ALL dynamic properties get's read within the
configure hook. The setter function does NOT get called if the task is so far
unconfigured.
---
lib/orogen/gen/tasks.rb | 55 ++++++++++++++++++++++++++-------
lib/orogen/spec/configuration_object.rb | 21 ++++++++++---
lib/orogen/templates/tasks/TaskBase.cpp | 3 ++
3 files changed, 62 insertions(+), 17 deletions(-)
diff --git a/lib/orogen/gen/tasks.rb b/lib/orogen/gen/tasks.rb
index 1e52338..da25b41 100644
--- a/lib/orogen/gen/tasks.rb
+++ b/lib/orogen/gen/tasks.rb
@@ -34,12 +34,28 @@ def register_for_generation
constructor(constructor.join("\n"))
- if dynamic? && (setter_operation.task == task)
+ if dynamic? && (setter_operation[0].task == task)
task.add_code_to_base_method_before "updateDynamicProperties"," if(!set#{name.capitalize}(_#{name}.get())) return false;\n"
- setter_operation.base_body = " //Update the RTT value of this property\n _#{name}.set(value); \n return true;"
- setter_operation.body = " //TODO Add your code here \n\n //Call the base function, DO-NOT remove\n return(#{task.name}Base::set#{name.capitalize}(value));"
+ setter_operation[0].base_body= <<EOF
+// The following steps happen within the base Implementation:
+// if the task is not configured yet, update the classical property and return true
+// if the task is configured OR running so far, call the user method to update the value
+// take care that this update will cause a thread change, and execution goes into the thread
+// of the task context.
+// if the user method return false, we return false too and do NOT update the classical value
+ if(isConfigured()){
+ RTT::OperationCaller< #{setter_operation[0].signature(false)} > userMethod = this->getOperation("#{setter_operation[1].name}");
+ assert(userMethod.ready());
+ if(!userMethod(#{setter_operation[0].argument_signature(true,false)})){
+ return false;
+ }
+ }
+ _#{name}.set(value);
+ return true;
+EOF
+ setter_operation[0].hidden = true
+ setter_operation[1].body = " return true;"
end
-
end
end
@@ -60,10 +76,27 @@ def register_for_generation
initializer("_#{name}(\"#{name}\")").
constructor(constructor.join("\n"))
- if dynamic? && (setter_operation.task == task)
- task.add_code_to_base_method_before "updateDynamicAttributes"," if(!set#{name.capitalize}(_#{name}.get())) return false;\n"
- setter_operation.base_body = " //Update the RTT value of this attribute\n _#{name}.set(value); \n return true;"
- setter_operation.body = " //TODO Add your code here \n\n //Call the base function, DO-NOT remove\n return(#{task.name}Base::set#{name.capitalize}(value));"
+ if dynamic? && (setter_operation[0].task == task)
+ task.add_code_to_base_method_before "updateDynamicProperties"," if(!set#{name.capitalize}(_#{name}.get())) return false;\n"
+ setter_operation[0].base_body= <<EOF
+// The following steps happen within the base Implementation:
+// if the task is not configured yet, update the classical property and return true
+// if the task is configured OR running so far, call the user method to update the value
+// take care that this update will cause a thread change, and execution goes into the thread
+// of the task context.
+// if the user method return false, we return false too and do NOT update the classical value
+ if(isConfigured()){
+ RTT::OperationCaller< #{setter_operation[0].signature(false)} > userMethod = this->getOperation("#{setter_operation[1].name}");
+ assert(userMethod.ready());
+ if(!userMethod(#{setter_operation[0].argument_signature(true,false)})){
+ return false;
+ }
+ }
+ _#{name}.set(value);
+ return true;
+EOF
+ setter_operation[0].hidden = true
+ setter_operation[1].body = " return true;"
end
end
end
@@ -160,7 +193,7 @@ def used_types
end
# Returns the argument part of the C++ signature for this callable
- def argument_signature(with_names = true)
+ def argument_signature(with_names = true, with_types = true)
arglist = arguments.map do |name, type, doc, qualified_type|
# Auto-add const-ref for non-trivial types
arg =
@@ -170,9 +203,7 @@ def argument_signature(with_names = true)
qualified_type
end
- if with_names then "#{arg} #{name}"
- else arg
- end
+ "#{arg if with_types} #{name if with_names}"
end
arglist.join(", ")
diff --git a/lib/orogen/spec/configuration_object.rb b/lib/orogen/spec/configuration_object.rb
index 5715948..df034d5 100644
--- a/lib/orogen/spec/configuration_object.rb
+++ b/lib/orogen/spec/configuration_object.rb
@@ -18,9 +18,10 @@ class ConfigurationObject
def dynamic?; !!@setter_operation end
# An operation that can be used to set the property. This is non-nil
- # only for dynamic properties
+ # only for dynamic properties. First one is the internal operation
+ # second one the public for user-code implementation.
#
- # @return [Orocos::Spec::Operation]
+ # @return [Orocos::Spec::Operation,Orocos::Spec::Operation]
attr_accessor :setter_operation
# The name of the type this property is using, for consistency with
@@ -57,9 +58,19 @@ def initialize(task, name, type, default_value)
end
def dynamic
- @setter_operation = task.find_operation("set#{name.capitalize}")
- if !@setter_operation
- @setter_operation = task.operation("set#{name.capitalize}").
+ @setter_operation = Array.new(2)
+ @setter_operation[0] = task.find_operation("set#{name.capitalize}_internal")
+ if !@setter_operation[0]
+ @setter_operation[0] = task.operation("set#{name.capitalize}_internal").
+ returns("bool").
+ argument("value", type_name).
+ runs_in_caller_thread.
+ doc("Internal Dynamic Property setter of #{name}")
+ end
+
+ @setter_operation[1] = task.find_operation("set#{name.capitalize}")
+ if !@setter_operation[1]
+ @setter_operation[1] = task.operation("set#{name.capitalize}").
returns("bool").
argument("value", type_name).
doc("Dynamic Property setter of #{name}")
diff --git a/lib/orogen/templates/tasks/TaskBase.cpp b/lib/orogen/templates/tasks/TaskBase.cpp
index 757dfd5..1a14a18 100644
--- a/lib/orogen/templates/tasks/TaskBase.cpp
+++ b/lib/orogen/templates/tasks/TaskBase.cpp
@@ -1,6 +1,9 @@
/* Generated from orogen/lib/orogen/templates/tasks/TAskBase.cpp */
#include "tasks/<%= task.basename %>Base.hpp"
+<% if !task.has_dynamic_properties? %>
+#include <rtt/OperationCaller.hpp>
+<% end %>
using namespace <%= component.name %>;
--
1.8.5.1
>From f1f13867f2a549e9dff7fb5af7b1ac9f317f61b9 Mon Sep 17 00:00:00 2001
From: Matthias Goldhoorn <[email protected]>
Date: Wed, 8 Jan 2014 13:14:44 +0100
Subject: [PATCH] Renamed dynamit setter function, see changes in orogen
---
lib/orocos/task_context.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/orocos/task_context.rb b/lib/orocos/task_context.rb
index f16d716..7e98640 100644
--- a/lib/orocos/task_context.rb
+++ b/lib/orocos/task_context.rb
@@ -14,7 +14,7 @@ def dynamic?; !!@dynamic_operation end
def initialize(task, name, orocos_type_name)
super
- if task.has_operation?(opname = "set#{name.capitalize}")
+ if task.has_operation?(opname = "set#{name.capitalize}_internal")
@dynamic_operation = task.operation(opname)
end
end
--
1.8.5.1
_______________________________________________
Rock-dev mailing list
[email protected]
http://www.dfki.de/mailman/cgi-bin/listinfo/rock-dev
