@@ -69,25 +69,28 @@ module Roby # The planner model we should use argument :planner_model # The planner method name - argument :method_name + argument :planning_method # The planner method options argument :method_options # Filters the options in +options+, splitting between the options that # are specific to the planning task and those that are to be forwarded # to the planner itself - def self.filter_options(options) # :nodoc: + def self.filter_options(options) task_arguments, planning_options = Kernel.filter_options options, :period => nil, :lookahead => 1, :planner_model => nil, + :planning_method => nil, :planned_model => Roby::Task, :method_name => nil, :method_options => {}, :planning_owners => nil - if !task_arguments[:method_name] - raise ArgumentError, "required argument :method_name missing" + task_arguments[:planning_method] ||= task_arguments[:method_name] + + if !task_arguments[:planning_method] + raise ArgumentError, "you should provide either a method name or a method object" elsif !task_arguments[:planner_model] raise ArgumentError, "required argument :planner_model missing" elsif task_arguments[:lookahead] < 0 @@ -135,7 +138,7 @@ module Roby # +context+ is forwarded to the planned task def append_pattern(*context) # Create the new pattern - task_arguments = arguments.slice(:planner_model, :planned_model, :method_name) + task_arguments = arguments.slice(:planner_model, :planned_model, :planning_method) task_arguments[:method_options] = method_options.dup task_arguments[:method_options][:pattern_id] = @pattern_id @pattern_id += 1 lib/roby/planning/loops.rb @@ -133,6 +133,10 @@ module Roby def to_s; "#{name}:#{id}(#{options})" end end + class FreeMethod < MethodDefinition + def call(planner); planner.instance_eval(&body) end + end + # The model of a planning method. This does not define an actual # implementation of the method, only the model methods should abide to. class MethodModel @@ -653,9 +657,8 @@ module Roby elsif planning_options[:args] && !planning_options[:args].empty? raise ArgumentError, "provided method-specific options through both :args and the option hash" end - @arguments.push(method_options) - Planning.debug { "planning #{name}[#{arguments}]" } + Planning.debug { "planning #{name}[#{method_options}]" } # Check for recursion if (options[:id] && @stack.include?([name, options[:id]])) || (!options[:id] && @stack.find { |n, _| n == name }) @@ -678,7 +681,7 @@ module Roby all_returns.compact! for return_type in all_returns - if task = find_reusable_task(return_type) + if task = find_reusable_task(return_type, method_options) return task end end @@ -689,7 +692,7 @@ module Roby end # Call the methods - call_planning_methods(Hash.new, options, *methods) + call_planning_methods(Hash.new, method_options, *methods) rescue Interrupt raise @@ -698,21 +701,18 @@ module Roby e.method_name = name e.method_options = options raise e - - ensure - @arguments.pop end - def find_reusable_task(return_type) + def find_reusable_task(return_type, method_options) query = plan.find_tasks. - which_fullfills(return_type, arguments). + which_fullfills(return_type, method_options). self_owned. not_abstract. not_finished. roots(TaskStructure::Hierarchy) for candidate in query - Planning.debug { "selecting task #{candidate} instead of planning #{return_type}[#{arguments}]" } + Planning.debug { "selecting task #{candidate} instead of planning #{return_type}[#{method_options}]" } return candidate end nil @@ -724,9 +724,10 @@ module Roby # Tries to find a successfull development in the provided method list. # # It raises NotFound if none of the methods returned successfully - def call_planning_methods(errors, options, method, *methods) + def call_planning_methods(errors, method_options, method, *methods) begin @stack.push [method.name, method.id] + @arguments.push(method_options) Planning.debug { "calling #{method.name}:#{method.id} with arguments #{arguments}" } begin result = method.call(self) @@ -762,6 +763,7 @@ module Roby result ensure + @arguments.pop @stack.pop end @@ -771,7 +773,7 @@ module Roby if methods.empty? raise NotFound.new(self, errors) else - call_planning_methods(errors, options, *methods) + call_planning_methods(errors, method_options, *methods) end end @@ -782,20 +784,22 @@ module Roby def make_loop(options = {}, &block) raise ArgumentError, "no block given" unless block - options.merge! :planner_model => self.class, :method_name => 'loops' - _, planning_options = PlanningLoop.filter_options(options) - loop_id = Planner.next_id if !@stack.empty? loop_id = "#{@stack.last[1]}_#{loop_id}" end + loop_method = FreeMethod.new 'loops', {}, lambda(&block) + + options[:planner_model] = self.class + options[:planning_method] = loop_method + _, planning_options = PlanningLoop.filter_options(options) planning_options[:id] = loop_id planning_options[:reuse] = false - m = self.class.method('loops', planning_options, &block) + loop_method.options.merge!(planning_options) options[:method_options] ||= {} options[:method_options].merge!(arguments || {}) - options[:method_options][:id] = m.id + options[:method_options][:id] = loop_method.id PlanningLoop.new(options) end end lib/roby/planning/model.rb @@ -8,22 +8,34 @@ module Roby class PlanningTask < Roby::Task attr_reader :planner, :transaction - arguments :planner_model, :method_name, + arguments :planner_model, :planning_method, :method_options, :planned_model, :planning_owners + attr_reader :method_name + def self.filter_options(options) task_options, method_options = Kernel.filter_options options, :planner_model => nil, - :method_name => nil, + :planning_method => nil, :method_options => {}, + :method_name => nil, # kept for backward compatibility :planned_model => nil, :planning_owners => nil + if task_options[:method_name] + method_name = task_options[:planning_method] = task_options[:method_name] + elsif task_options[:planning_method].respond_to?(:to_str) || task_options[:planning_method].respond_to?(:to_sym) + method_name = task_options[:method_name] = task_options[:planning_method].to_s + end + if !task_options[:planner_model] raise ArgumentError, "missing required argument 'planner_model'" - elsif !task_options[:method_name] - raise ArgumentError, "missing required argument 'method_name'" + elsif !task_options[:planning_method] + raise ArgumentError, "missing required argument 'planning_method'" + elsif !(method_name || task_options[:planning_method].kind_of?(Roby::Planning::MethodDefinition)) + raise ArgumentError, "the planning_method argument is neither a method object nor a name" + end task_options[:planned_model] ||= nil task_options[:method_options] ||= Hash.new @@ -34,16 +46,23 @@ module Roby def initialize(options) task_options = PlanningTask.filter_options(options) + @method_name = task_options[:method_name] super(task_options) end def planned_model - arguments[:planned_model] ||= planner_model.model_of(method_name, method_options).returns || Roby::Task + arguments[:planned_model] ||= if method_name + planner_model.model_of(method_name, method_options).returns + else + planning_method.returns + end + + arguments[:planned_model] ||= Roby::Task end def to_s - "#{super}[#{method_name}:#{method_options}] -> #{@planned_task || "nil"}" + "#{super}[#{planning_method}:#{method_options}] -> #{@planned_task || "nil"}" end def planned_task @@ -88,7 +107,11 @@ module Roby end def planning_thread(context) - result_task = planner.send(method_name, method_options.merge(:context => context)) + result_task = if method_name + planner.send(method_name, method_options.merge(:context => context)) + else + planner.send(:call_planning_methods, Hash.new, method_options.merge(:context => context), planning_method) + end # Don't replace the planning task with ourselves if the # transaction specifies another planning task @@ -145,6 +168,7 @@ module Roby class TransactionProxy < Roby::Transactions::Task proxy_for PlanningTask def_delegator :@__getobj__, :planner + def_delegator :@__getobj__, :planning_method def_delegator :@__getobj__, :method_name def_delegator :@__getobj__, :method_options end lib/roby/planning/task.rb @@ -34,7 +34,8 @@ class TC_PlanningLoop < Test::Unit::TestCase :planning_owners => nil, :planner_model => planner_model, :planned_model => SimpleTask, - :method_name => :task, + :planning_method => :task, + :method_name => 'task', :method_options => {} } end @@ -348,47 +349,49 @@ class TC_PlanningLoop < Test::Unit::TestCase # end #end - #def test_make_loop - # planner_model = Class.new(Planning::Planner) do - # include Test::Unit::Assertions - - # @result_task = nil - # attr_reader :result_task - # method(:task) { @result_task = SimpleTask.new(:id => arguments[:task_id])} - # method(:looping_tasks) do - # t1 = make_loop(:period => 0, :child_argument => 2) do - # # arguments of 'my_looping_task' shall be forwarded - # raise unless arguments[:parent_argument] == 1 - # raise unless arguments[:child_argument] == 2 - # task(:task_id => 'first_loop') - # end - # t2 = make_loop do - # task(:task_id => 'second_loop') - # end - # # Make sure the two loops are different - # assert(t1.method_options[:id] != t2.method_options[:id]) - # [t1, t2] - # end - # end + def test_make_loop + planner_model = Class.new(Planning::Planner) do + include Test::Unit::Assertions + + @result_task = nil + attr_reader :result_task + method(:task) { @result_task = SimpleTask.new(:id => arguments[:task_id])} + method(:looping_tasks) do + t1 = make_loop(:period => 0, :child_argument => 2) do + # arguments of 'my_looping_task' shall be forwarded + raise unless arguments[:parent_argument] == 1 + raise unless arguments[:child_argument] == 2 + task(:task_id => 'first_loop') + end + t2 = make_loop do + task(:task_id => 'second_loop') + end + # Make sure the two loops are different + assert(t1.method_options[:id] != t2.method_options[:id]) + [t1, t2] + end + end - # planner = planner_model.new(plan) - # t1, t2 = planner.looping_tasks(:parent_argument => 1) - # plan.insert(t1) - # plan.insert(t2) + planner = planner_model.new(plan) + t1, t2 = planner.looping_tasks(:parent_argument => 1) + assert(t1.fully_instanciated?, t1.arguments.keys - t1.class.arguments.to_a) + assert(t2.fully_instanciated?) + plan.insert(t1) + plan.insert(t2) - # t1.start! - # planned_task = planning_task_result(t1.last_planning_task) - # assert_equal('first_loop', planned_task.arguments[:id]) + t1.start! + planned_task = planning_task_result(t1.last_planning_task) + assert_equal('first_loop', planned_task.arguments[:id]) - # t2.start! - # planned_task = planning_task_result(t2.last_planning_task) - # assert_equal('second_loop', planned_task.arguments[:id]) + t2.start! + planned_task = planning_task_result(t2.last_planning_task) + assert_equal('second_loop', planned_task.arguments[:id]) - # t3 = planner.make_loop(:period => 0, :parent_argument => 1, :child_argument => 2) do - # task(:task_id => 'third_loop') - # end - # plan.insert(t3) - # t3.start! - # assert_equal('third_loop', planning_task_result(t3.last_planning_task).arguments[:id]) - #end + t3 = planner.make_loop(:period => 0, :parent_argument => 1, :child_argument => 2) do + task(:task_id => 'third_loop') + end + plan.insert(t3) + t3.start! + assert_equal('third_loop', planning_task_result(t3.last_planning_task).arguments[:id]) + end end test/planning/test_loops.rb @@ -103,4 +103,24 @@ class TC_PlanningTask < Test::Unit::TestCase assert_kind_of(SimpleTask, new_task, planning_task) assert_equal(101, new_task.arguments[:id]) end + + def test_method_object + planner_model = Class.new(Planning::Planner) + + FlexMock.use do |mock| + mock.should_receive(:method_called).with(:context => nil, :arg => 10).once + + body = lambda do + mock.method_called(arguments) + Roby::Task.new(:id => 'result_of_lambda') + end + m = FreeMethod.new 'test_object', {:id => 10}, body + planning_task = PlanningTask.new(:planner_model => planner_model, :planning_method => m, :arg => 10) + plan.permanent(planning_task) + planning_task.start! + new_task = planning_task_result(planning_task) + assert_equal 'result_of_lambda', new_task.arguments[:id] + end + end end + test/planning/test_task.rb 08149dcad6899c94e5beb169420c9d54701726ef Sylvain Joyeux sylvain.joyeux@dfki.de http://github.com/doudou/roby/commit/8ad08be480bb26259f5b33f8e859dcb4c2bf37d9 8ad08be480bb26259f5b33f8e859dcb4c2bf37d9 2008-06-02T08:21:20-07:00 2008-06-02T08:21:20-07:00 [core] fix memory leak in make_loop Until now, the make_loop construct was defining a method per call on the planner class directly. It means basically that it would create as much methods as its parent planning method is called, with no way to "forget" them. This commit fixes that by allowing to use planning with free method objects (FreeMethod): a single block can be enclosed as a method definition not tied to a particular planner, and then applied by using #call_planning_methods. 01a4e20b7cc96bb04c7b21340eea8cadcb934475 Sylvain Joyeux sylvain.joyeux@dfki.de