From a3e38d43576b77385dad85b290cb3473366e7de0 Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Wed, 17 Dec 2025 17:30:47 -0300 Subject: [PATCH 01/15] fix: instanciate scheduler tasks ("master tasks") during lazy deploy They are actual components, not deployments, and are needed for dataflow analysis --- lib/syskit.rb | 1 + lib/syskit/deployment.rb | 124 ++++++------------ lib/syskit/models/configured_deployment.rb | 24 ++-- .../models/deployed_task_instanciation.rb | 116 ++++++++++++++++ lib/syskit/models/deployment.rb | 32 ++++- lib/syskit/models/deployment_group.rb | 8 ++ .../system_network_deployer.rb | 20 ++- lib/syskit/task_context.rb | 26 ++++ .../test_system_network_deployer.rb | 87 ++++++++++++ test/test_deployment.rb | 6 +- test/test_exceptions.rb | 4 +- 11 files changed, 333 insertions(+), 115 deletions(-) create mode 100644 lib/syskit/models/deployed_task_instanciation.rb diff --git a/lib/syskit.rb b/lib/syskit.rb index 1b0e0fc65..f0524cc0e 100644 --- a/lib/syskit.rb +++ b/lib/syskit.rb @@ -77,6 +77,7 @@ module ProcessManagers require "syskit/models/task_context" require "syskit/models/ruby_task_context" require "syskit/models/deployment" +require "syskit/models/deployed_task_instanciation" require "syskit/models/configured_deployment" require "syskit/models/deployment_group" diff --git a/lib/syskit/deployment.rb b/lib/syskit/deployment.rb index ea8dd2926..6d38b8046 100644 --- a/lib/syskit/deployment.rb +++ b/lib/syskit/deployment.rb @@ -97,8 +97,8 @@ def has_orocos_name?(orocos_name) end def instanciate_all_tasks - model.each_orogen_deployed_task_context_model.map do |act| - task(name_mappings[act.name]) + each_orogen_deployed_task_context_model.map do |act| + task(act.name) end end @@ -106,49 +106,20 @@ def instanciate_all_tasks # # It takes into account deployment prefix def each_orogen_deployed_task_context_model(&block) - model.each_orogen_deployed_task_context_model(&block) + orogen_model.task_activities.each(&block) end - # Either find the existing task that matches the given deployment specification, - # or creates and adds it. - # - # @param (see #task) - def find_or_create_task(name, syskit_task_model = nil, auto_conf: false) - orogen_task_deployment_model = deployed_orogen_model_by_name(name) - if orogen_master = orogen_task_deployment_model.master - mapped_master = name_mappings[orogen_master.name] - scheduler_task = find_or_create_task( - mapped_master, auto_conf: true - ) - candidates = scheduler_task.each_parent_task - else - candidates = each_executed_task - end - - # I don't know why name_mappings[orogen.name] would not be - # equal to 'name' and I couldn't find a reason for this in the - # git history when I refactored this. - # - # I keep it here for now, just in case, but that would need to - # be investigated - # - # TODO - mapped_name = name_mappings[orogen_task_deployment_model.name] - candidates.each do |task| - return task if task.orocos_name == mapped_name - end + # The deployment's orogen model with the name mappings applied + def orogen_model + return @orogen_model if @orogen_model - create_deployed_task( - orogen_task_deployment_model, - syskit_task_model, - scheduler_task, auto_conf: auto_conf - ) + @orogen_model = model.map_orogen_model(name_mappings) end def deployed_orogen_model_by_name(name) orogen_task_deployment = each_orogen_deployed_task_context_model - .find { |act| name == name_mappings[act.name] } + .find { |act| name == act.name } unless orogen_task_deployment available = each_orogen_deployed_task_context_model .map { |act| name_mappings[act.name] } @@ -168,8 +139,7 @@ def deployed_orogen_model_by_name(name) # Create and add a task model supported by this deployment # # @param [OroGen::Spec::TaskDeployment] orogen_task_deployment_model - # the orogen model that describes this - # deployment + # the orogen model that describes this deployment, already mapped # @param [Models::TaskContext,nil] syskit_task_model the expected # syskit task model, or nil if it is meant to use the basic model. # This is useful in specialized models (e.g. dynamic services) @@ -180,17 +150,11 @@ def deployed_orogen_model_by_name(name) # a configuration that matches the task's orocos name (if it exists). This # is mostly used for scheduling tasks, which are automatically instanciated # by Syskit. - # - # @see find_or_create_task task - def create_deployed_task( - orogen_task_deployment_model, - syskit_task_model, scheduler_task, auto_conf: false - ) - mapped_name = name_mappings[orogen_task_deployment_model.name] + def create_deployed_task(orogen_task_deployment_model, syskit_task_model) + mapped_name = orogen_task_deployment_model.name if ready? && !(remote_handles = remote_task_handles[mapped_name]) raise InternalError, - "no remote handle describing #{mapped_name} in #{self}" \ - "(got #{remote_task_handles.keys.sort.join(', ')})" + "cannot find remote task handles for #{mapped_name}" end if task_context_in_fatal?(mapped_name) @@ -199,29 +163,15 @@ def create_deployed_task( "from #{self}" end - base_syskit_task_model = model.resolve_syskit_model_for_deployed_task( - orogen_task_deployment_model + task = instanciate_deployed_task( + mapped_name, + orogen_model: orogen_task_deployment_model, + syskit_model: syskit_task_model, + plan: plan ) - syskit_task_model ||= base_syskit_task_model - unless syskit_task_model <= base_syskit_task_model - raise ArgumentError, - "incompatible explicit selection of task model " \ - "#{syskit_task_model} for the model of #{mapped_name} in #{self}, " \ - "expected #{base_syskit_task_model} or one of its subclasses" - end - task = syskit_task_model - .new(orocos_name: mapped_name, read_only: read_only?(mapped_name)) - plan.add(task) task.executed_by self - if scheduler_task - task.depends_on scheduler_task, role: "scheduler" - task.should_configure_after scheduler_task.start_event - end - - task.orogen_model = orogen_task_deployment_model task.initialize_remote_handles(remote_handles) if remote_handles - auto_select_conf(task) if auto_conf task end @@ -245,32 +195,32 @@ def task(name, syskit_task_model = nil) end orogen_task_deployment_model = deployed_orogen_model_by_name(name) + task = create_deployed_task(orogen_task_deployment_model, syskit_task_model) + task_setup_scheduler(task, existing_tasks: executed_tasks_by_name) + task + end - if (orogen_master = orogen_task_deployment_model.master) - scheduler_task = find_or_create_task( - orogen_master.name, auto_conf: true + # (see DeployedTaskInstanciation#task_setup_scheduler) + def task_setup_scheduler(task, existing_tasks: {}) + return unless (scheduler_task = super) + + scheduler_task.executed_by self + + if ready? && !scheduler_task.has_remote_information? + scheduler_task.initialize_remote_handles( + remote_task_handles.fetch(task.orocos_name) ) end - create_deployed_task( - orogen_task_deployment_model, - syskit_task_model, scheduler_task - ) + scheduler_task end - # Selects the configuration of a master task + include Models::DeployedTaskInstanciation + + # The tasks executed by this deployment, as a name to task hash # - # Master tasks are auto-injected in the network, and as such the - # user cannot select their configuration. This picks either - # ['default', task.orocos_name] if the master task's has a configuration - # section matching the task's name, or ['default'] otherwise. - private def auto_select_conf(task) - manager = task.model.configuration_manager - task.conf = - if manager.has_section?(task.orocos_name) - ["default", task.orocos_name] - else - ["default"] - end + # @return [Hash] + def executed_tasks_by_name + each_executed_task.to_h { |t| [t.orocos_name, t] } end ## diff --git a/lib/syskit/models/configured_deployment.rb b/lib/syskit/models/configured_deployment.rb index 7823ca84c..cba5a9232 100644 --- a/lib/syskit/models/configured_deployment.rb +++ b/lib/syskit/models/configured_deployment.rb @@ -49,6 +49,12 @@ def non_logger_task_names @name_mappings.values.grep_v(/_Logger$/) end + def read_only?(orocos_name) + read_only.include?(orocos_name) + end + + include DeployedTaskInstanciation + # @api private # # Resolves the read_only argument into an Array of task names to be set as @@ -102,13 +108,7 @@ def command_line(loader: Roby.app.default_pkgconfig_loader, **options) def orogen_model return @orogen_model if @orogen_model - @orogen_model = model.orogen_model.dup - orogen_model.task_activities.map! do |activity| - activity = activity.dup - activity.name = name_mappings[activity.name] || activity.name - activity - end - @orogen_model + @orogen_model = model.map_orogen_model(name_mappings) end # Enumerate the tasks that are deployed by this configured @@ -131,14 +131,8 @@ def each_deployed_task_model # Enumerate the oroGen specification for the deployed tasks # # @yieldparam [OroGen::Spec::TaskDeployment] - def each_orogen_deployed_task_context_model - return enum_for(__method__) unless block_given? - - model.each_orogen_deployed_task_context_model do |deployed_task| - task = deployed_task.dup - task.name = name_mappings[task.name] || task.name - yield(task) - end + def each_orogen_deployed_task_context_model(&block) + orogen_model.task_activities.each(&block) end # Create a new deployment task that can represent self in a plan diff --git a/lib/syskit/models/deployed_task_instanciation.rb b/lib/syskit/models/deployed_task_instanciation.rb new file mode 100644 index 000000000..01b295597 --- /dev/null +++ b/lib/syskit/models/deployed_task_instanciation.rb @@ -0,0 +1,116 @@ +# frozen_string_literal: true + +module Syskit + module Models + # Features that implement the instanciation of deployed tasks, compatible with + # both {ConfiguredDeployment} and {Syskit::Deployment} + module DeployedTaskInstanciation + # Create a task from this deployment's + # + # @param [String] orocos_name the mapped name of the deployed task + # @param [OroGen::Spec::DeployedTask,nil] orogen_model the orogen model of the + # deployed task, if known. Pass nil if it has to be resolved. + # @param [TaskContext,nil] syskit_model the syskit model that should be used + # to create the task instance. It may be a submodel of the deployment's task + # model. Pass nil to resolve from the deployment model + # @param [Roby::Plan,nil] plan the plan in which the task should be created + # @return [(OroGen::Spec::DeployedTask,TaskContext)] the resolved orogen + # models and + def instanciate_deployed_task( + orocos_name, orogen_model: nil, syskit_model: nil, plan: nil + ) + orogen_model, syskit_model = + instanciate_deployed_task_resolve_task_model( + orocos_name, orogen_model, syskit_model + ) + + args = { + orogen_model: orogen_model, + orocos_name: orocos_name, + read_only: read_only?(orocos_name) + } + args[:plan] = plan if plan + syskit_model.new(**args) + end + + # Create the 'scheduler' task of a task (if needed) + # + # OroGen components can be explicitly triggered by another component. This + # shows up as having a 'master' in the deployed task model. This method makes + # sure that the scheduler component is instanciated, and sets up proper + # relationship between the scheduled task and the scheduler task + def task_setup_scheduler(task, existing_tasks: {}) + return unless (orogen_master_m = task.orogen_model.master) + + mapped_master_name = orogen_master_m.name + unless (scheduler_task = existing_tasks[mapped_master_name]) + scheduler_task = + instanciate_deployed_task(mapped_master_name, plan: task.plan) + scheduler_task.select_conf_from_name + + existing_tasks = + existing_tasks.merge({ mapped_master_name => scheduler_task }) + end + + task_setup_scheduler(scheduler_task, existing_tasks: existing_tasks) + + task.depends_on scheduler_task, role: "scheduler" + task.should_configure_after scheduler_task.start_event + scheduler_task + end + + # Helper for {#instanciate_deployed_task} to resolve the syskit task model + # that should be used to represent the deployed task + # + # @param [String] orocos_name the mapped name of the deployed task + # @param [OroGen::Spec::DeployedTask,nil] orogen_model the orogen model of the + # deployed task, if known. Pass nil if it has to be resolved. + # @param [TaskContext,nil] syskit_model the syskit model that should be used + # to create the task instance. It may be a submodel of the deployment's task + # model. Pass nil to resolve from the deployment model + # @return [(OroGen::Spec::DeployedTask,TaskContext)] the resolved orogen + # models and + def instanciate_deployed_task_resolve_task_model( + orocos_name, orogen_model = nil, syskit_model = nil + ) + orogen_model ||= + each_orogen_deployed_task_context_model + .find { |m| m.name == orocos_name } + + unless orogen_model + raise ArgumentError, "no deployed task found for #{orocos_name}" + end + + base_syskit_model = resolve_syskit_model_for_deployed_task(orogen_model) + if syskit_model && !(syskit_model <= base_syskit_model) + raise ArgumentError, + "incompatible explicit selection of task model " \ + "#{syskit_model} for the model of #{orogen_model} in " \ + "#{self}, expected #{base_syskit_model} " \ + "or one of its subclasses" + end + + [orogen_model, syskit_model || base_syskit_model] + end + + # Resolve the syskit task context model that should be used to represent a + # deployed task + # + # @param [OroGen::Spec::DeployedTask] orogen_deployed_task the model, which + # name has been mapped + def resolve_syskit_model_for_deployed_task(orogen_deployed_task) + unmapped_name = name_mappings.rassoc(orogen_deployed_task.name)&.first + unless unmapped_name + raise ArgumentError, + "no mapping points to name #{orogen_deployed_task.name}, " \ + "known names: #{name_mappings}. This method expects the " \ + "mapped name as argument" + end + + model.resolve_syskit_model_for_deployed_task( + orogen_deployed_task, name: unmapped_name + ) + end + end + end +end diff --git a/lib/syskit/models/deployment.rb b/lib/syskit/models/deployment.rb index 7239a2eb6..966647ef3 100644 --- a/lib/syskit/models/deployment.rb +++ b/lib/syskit/models/deployment.rb @@ -181,9 +181,10 @@ def find_syskit_model_for_deployed_task(deployed_task) # @api private # # Resolve the Syskit task model for one of this deployment's tasks - def resolve_syskit_model_for_deployed_task(deployed_task) - task_name = deployed_task.name - if (registered = @task_name_to_syskit_model[task_name]) + def resolve_syskit_model_for_deployed_task( + deployed_task, name: deployed_task.name + ) + if (registered = @task_name_to_syskit_model[name]) return registered end @@ -195,7 +196,7 @@ def resolve_syskit_model_for_deployed_task(deployed_task) "Deployment.define_deployed_task" ) - @task_name_to_syskit_model[task_name] = + @task_name_to_syskit_model[name] = ::Syskit::TaskContext.model_for(deployed_task.task_model) end @@ -219,6 +220,29 @@ def each_deployed_task_model def each_orogen_deployed_task_context_model(&block) orogen_model.task_activities.each(&block) end + + # Return the deployment's orogen model with a given name mapping applied + def map_orogen_model(name_mappings) + mapped_model = orogen_model.dup + activity_mapping = {} + mapped_model.task_activities.map! do |activity| + mapped_activity = activity.dup + activity_mapping[activity] = mapped_activity + mapped_activity.name = name_mappings[activity.name] || activity.name + mapped_activity + end + + mapped_model.task_activities.each do |t| + if (mapped_master = activity_mapping[t.master]) + t.master = mapped_master + end + + t.slaves.replace( + t.slaves.map { |t| activity_mapping[t] } + ) + end + mapped_model + end end end end diff --git a/lib/syskit/models/deployment_group.rb b/lib/syskit/models/deployment_group.rb index a42a5761e..7ce1186fa 100644 --- a/lib/syskit/models/deployment_group.rb +++ b/lib/syskit/models/deployment_group.rb @@ -45,6 +45,14 @@ def instanciate(plan, permanent: true, deployment_tasks: {}) [deployment_task.task(mapped_task_name), deployment_task] end + def orocos_name + mapped_task_name + end + + def read_only? + configured_deployment.read_only?(mapped_task_name) + end + def orogen_model return @orogen_model if @orogen_model diff --git a/lib/syskit/network_generation/system_network_deployer.rb b/lib/syskit/network_generation/system_network_deployer.rb index 3ff982fc9..f74137bf2 100644 --- a/lib/syskit/network_generation/system_network_deployer.rb +++ b/lib/syskit/network_generation/system_network_deployer.rb @@ -242,17 +242,27 @@ def apply_selected_deployments(selected_deployments, deployment_tasks = {}) # Apply deployments selected during {#deploy} by setting the task's # orocos_name argument accordingly # - # @param [Component=>Deployment] selected_deployments the + # @param [Component=>DeploymentGroup::DeployedTask] selected_deployments the # component-to-deployment association def lazy_apply_selected_deployments(selected_deployments) - selected_deployments.each do |task, sel| - unless sel.mapped_task_name + with_master = selected_deployments.find_all do |task, sel| + unless sel.orocos_name raise "found selected deployment without a task name" end - orocos_name = sel.mapped_task_name - task.orocos_name ||= orocos_name + task.orocos_name ||= sel.orocos_name task.orogen_model = sel.orogen_model + task.orogen_model.master + end + return if with_master.empty? + + by_name = selected_deployments + .to_h { |task, _| [task.orocos_name, task] } + with_master.each do |task, sel| + deployment = sel.configured_deployment + scheduler_task = + deployment.task_setup_scheduler(task, existing_tasks: by_name) + by_name[scheduler_task.orocos_name] = scheduler_task end end diff --git a/lib/syskit/task_context.rb b/lib/syskit/task_context.rb index d76aec098..fbdcd51c8 100644 --- a/lib/syskit/task_context.rb +++ b/lib/syskit/task_context.rb @@ -226,6 +226,32 @@ def distance_to(other) execution_agent.distance_to(other.execution_agent) end + # Whether the remote task information has been set by our deployment + # + # The task's deployment must set remote information gathered during deployment + # execution by calling {#initialize_remote_handles}. This method verifies that + # this step has been done. + # + # It will not be done until the deployment is ready + def has_remote_information? + orocos_task + end + + # Automatically select the task configuration based on its orocos_name + # + # Some tasks (mainly master tasks) are auto-injected in the network, and as + # such the user cannot select their configuration. This picks either + # ['default', task.orocos_name] if the master task's has a configuration + # section matching the task's name, or ['default'] otherwise. + def select_conf_from_name + self.conf = + if model.configuration_manager.has_section?(orocos_name) + ["default", orocos_name] + else + ["default"] + end + end + # Verifies if a task could be replaced by this one # # @return [Boolean] true if #merge(other_task) can be called and diff --git a/test/network_generation/test_system_network_deployer.rb b/test/network_generation/test_system_network_deployer.rb index 6fec76c54..17dc4c66d 100644 --- a/test/network_generation/test_system_network_deployer.rb +++ b/test/network_generation/test_system_network_deployer.rb @@ -613,6 +613,93 @@ def make_candidates(count) end end + describe "scheduled tasks during lazy deployments" do + # Slave tasks are handled by Syskit::Deployment in eager deployment mode + + before do + @task_m = TaskContext.new_submodel + + @orogen_model = Models.create_orogen_deployment_model("deployment") + @orogen_scheduler = + @orogen_model.task("scheduler", @task_m.orogen_model) + @orogen_scheduled = + @orogen_model.task("scheduled", @task_m.orogen_model) + @orogen_scheduled.slave_of(@orogen_scheduler) + end + + it "adds its master task as dependency" do + scheduled_task = create_task("prefix_scheduled") + scheduler_tests_deploy + scheduler_task = scheduled_task.child_from_role("scheduler") + + assert_equal "prefix_scheduler", scheduler_task.orocos_name + end + + it "auto-selects the configuration of the master task" do + scheduled_task = create_task("prefix_scheduled") + syskit_stub_conf @task_m, "prefix_scheduler" + scheduler_tests_deploy + scheduler_task = scheduled_task.child_from_role("scheduler") + + assert_equal %w[default prefix_scheduler], scheduler_task.conf + end + + it "constrains the configuration of the slave task" do + scheduled_task = create_task("prefix_scheduled") + scheduler_tests_deploy + scheduler_task = scheduled_task.child_from_role("scheduler") + + assert scheduler_task.start_event.child_object?( + scheduled_task.start_event, + Roby::EventStructure::SyskitConfigurationPrecedence + ) + end + + it "reuses an existing scheduler task" do + scheduled_task = create_task("prefix_scheduled") + scheduler_task = create_task("prefix_scheduler") + scheduler_tests_deploy + + assert_same scheduler_task, + scheduled_task.child_from_role("scheduler") + end + + it "creates a scheduler task only once" do + orogen_scheduled2 = + @orogen_model.task("scheduled2", @task_m.orogen_model) + orogen_scheduled2.slave_of(@orogen_scheduler) + scheduled_task = create_task("prefix_scheduled") + scheduled2_task = create_task("prefix_scheduled2") + scheduler_tests_deploy + + scheduler = scheduled_task.child_from_role("scheduler") + assert_equal "prefix_scheduler", scheduler.orocos_name + assert_same scheduler, scheduled2_task.child_from_role("scheduler") + end + + it "creates scheduler tasks recursively" do + orogen_scheduler2 = + @orogen_model.task("scheduler2", @task_m.orogen_model) + @orogen_scheduler.slave_of(orogen_scheduler2) + scheduled_task = create_task("prefix_scheduled") + scheduler_tests_deploy + + scheduler = scheduled_task.child_from_role("scheduler") + scheduler2 = scheduler.child_from_role("scheduler") + assert_equal "prefix_scheduler2", scheduler2.orocos_name + end + + def create_task(orocos_name) + @task_m.new(plan: plan, orocos_name: orocos_name) + end + + def scheduler_tests_deploy + deployment_m = Deployment.new_submodel(orogen_model: @orogen_model) + default_deployment_group.use_deployment(deployment_m => "prefix_") + deployer.deploy(lazy: true) + end + end + def deployed_task_helper(model, name) Models::DeploymentGroup::DeployedTask.new(model, name) end diff --git a/test/test_deployment.rb b/test/test_deployment.rb index 71031303e..5410b56f0 100644 --- a/test/test_deployment.rb +++ b/test/test_deployment.rb @@ -185,7 +185,9 @@ def mock_raw_port(task, port_name) assert_equal "other_name", deployment_task.task("other_name").orocos_name end it "sets orogen_model on the new task" do - assert_equal orogen_deployed_task, deployment_task.task("mapped_task_name").orogen_model + model = deployment_task.task("mapped_task_name").orogen_model + assert_equal "mapped_task_name", model.name + assert_equal orogen_deployed_task.task_model, model.task_model end it "adds the deployment task as an execution agent for the new task" do flexmock(task_m).new_instances.should_receive(:executed_by).with(deployment_task).once @@ -199,7 +201,7 @@ def mock_raw_port(task, port_name) deployment_task.task("mapped_task_name") end it "does runtime initialization if it is already ready" do - task = flexmock(task_m.new) + task = flexmock(task_m.new(orocos_name: "mapped_task_name")) flexmock(task_m).should_receive(:new).and_return(task) remote_handle = flexmock(in_fatal: false) diff --git a/test/test_exceptions.rb b/test/test_exceptions.rb index ab6d3d066..26abf6218 100644 --- a/test/test_exceptions.rb +++ b/test/test_exceptions.rb @@ -215,8 +215,8 @@ module Syskit expected = <<~PP.chomp deployed task 'test_syskit_tests_empty' from deployment \ - 'syskit_tests_empty' defined in 'orogen_syskit_tests' on 'localhost' is \ - assigned to 2 tasks. Below is the list of \ + 'test_syskit_tests_empty' defined in 'orogen_syskit_tests' on \ + 'localhost' is assigned to 2 tasks. Below is the list of \ the dependent non-deployed actions. Right after the list is \ a detailed explanation of why the first two tasks are not merged: OroGen.orogen_syskit_tests.Empty(arg: 1, conf: ["default"], \ From 48c32fdea33e988a90835c967b34f696011b6ec7 Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Thu, 18 Dec 2025 11:10:12 -0300 Subject: [PATCH 02/15] chore: disable Style/InverseMethod for a class ordering test I did not change the !( ... <= ... ) into ( ... > ... ) as even if Ruby somehow pretends the order is total, it really is not. That is, I would expect A < B and A > B to both return false (or even nil) for unrelated classes. --- lib/syskit/models/deployed_task_instanciation.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/syskit/models/deployed_task_instanciation.rb b/lib/syskit/models/deployed_task_instanciation.rb index 01b295597..f3ebb05b6 100644 --- a/lib/syskit/models/deployed_task_instanciation.rb +++ b/lib/syskit/models/deployed_task_instanciation.rb @@ -82,7 +82,7 @@ def instanciate_deployed_task_resolve_task_model( end base_syskit_model = resolve_syskit_model_for_deployed_task(orogen_model) - if syskit_model && !(syskit_model <= base_syskit_model) + if syskit_model && !(syskit_model <= base_syskit_model) # rubocop:disable Style/InverseMethods raise ArgumentError, "incompatible explicit selection of task model " \ "#{syskit_model} for the model of #{orogen_model} in " \ From 98765ee1e94f323e4f8c8f3c7d12913b15d4fe95 Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Fri, 19 Dec 2025 11:37:48 -0300 Subject: [PATCH 03/15] fix: separate initial information gathering from resolution of triggering inputs in dataflow computation When master/slave configurations are involved, initial_information actually does nothing for slaves (they are processed with the master, added a comment to explain why). But then, we call triggering_inputs, which may call done_port_info on output ports for which there are no triggering inputs. Then when the master gets processed, the slave's initial_task_information tries to "create" the port info on a done port, which raises. This worked so far because, by happenstance, master tasks were always added before slave tasks in the final deployed network. The lazy deployments changed the order and now this fails. --- .../dataflow_computation.rb | 39 +++++++++-------- .../network_generation/dataflow_dynamics.rb | 5 +++ .../test_dataflow_dynamics.rb | 42 ++++++++++++++----- 3 files changed, 58 insertions(+), 28 deletions(-) diff --git a/lib/syskit/network_generation/dataflow_computation.rb b/lib/syskit/network_generation/dataflow_computation.rb index 2b0cf0c30..16eca19c5 100644 --- a/lib/syskit/network_generation/dataflow_computation.rb +++ b/lib/syskit/network_generation/dataflow_computation.rb @@ -192,27 +192,32 @@ def propagate(tasks) debug "" debug "== Gathering Initial Information" tasks.each do |task| - debug { "computing initial information for #{task}" } - - log_nest(4) do + debug "computing initial information for #{task}" + log_nest(2) do initial_information(task) - if connections = triggering_port_connections(task) - triggering_connections[task] = connections - triggering_dependencies[task] = connections.map do |port_name, triggers| - triggers.ports.map(&:first) - end + end + end - debug do - debug "#{connections.size} triggering connections for #{task}" - connections.each do |port_name, info| - debug " for #{port_name}" - log_nest(8) do - log_pp :debug, info - end - end - break + # This MUST be done after the loop above, don't merge them + tasks.each do |task| # rubocop:disable Style/CombinableLoops + connections = log_nest(2) { triggering_port_connections(task) } + next unless connections + + triggering_connections[task] = connections + triggering_dependencies[task] = + connections.map do |port_name, triggers| + triggers.ports.map(&:first) + end + + debug do + debug " #{connections.size} triggering connections for #{task}" + connections.each do |port_name, info| + debug " for #{port_name}" + log_nest(6) do + log_pp :debug, info end end + break end end diff --git a/lib/syskit/network_generation/dataflow_dynamics.rb b/lib/syskit/network_generation/dataflow_dynamics.rb index 028d5c1da..6e604aaff 100644 --- a/lib/syskit/network_generation/dataflow_dynamics.rb +++ b/lib/syskit/network_generation/dataflow_dynamics.rb @@ -369,6 +369,11 @@ def initial_slaves_information(task) # # Computes a task's initial information def initial_task_information(task) + # We need to resolve the slaves, as the master's task done_task_info + # will call done_task_info on the slaves as well + # + # Note that #initial_information is explicitly skipping tasks + # that have a master activity initial_slaves_information(task) set_port_info(task, nil, PortDynamics.new("#{task.orocos_name}.main")) diff --git a/test/network_generation/test_dataflow_dynamics.rb b/test/network_generation/test_dataflow_dynamics.rb index 951b1416f..7d0941169 100644 --- a/test/network_generation/test_dataflow_dynamics.rb +++ b/test/network_generation/test_dataflow_dynamics.rb @@ -134,7 +134,7 @@ module NetworkGeneration describe "master/slave deployments" do before do - task_m = Syskit::TaskContext.new_submodel do + task_m = @task_m = Syskit::TaskContext.new_submodel do output_port "out", "/double" end deployment_m = Syskit::Deployment.new_submodel do @@ -151,21 +151,41 @@ module NetworkGeneration end it "resolves if called for the slave after the master" do - flexmock(dynamics).should_receive(:set_port_info) - .with(@slave, nil, any).once - flexmock(dynamics).should_receive(:set_port_info) - dynamics.initial_information(@master) - dynamics.initial_information(@slave) + flexmock(dynamics) + .should_receive(:set_port_info) + .with(@slave, nil, any).once.pass_thru + dynamics.should_receive(:set_port_info).pass_thru + dynamics.propagate([@master, @slave]) assert dynamics.has_final_information_for_task?(@master) assert dynamics.has_final_information_for_task?(@slave) end it "resolves if called for the master after the slave" do - flexmock(dynamics).should_receive(:set_port_info) - .with(@slave, nil, any).once - flexmock(dynamics).should_receive(:set_port_info) - dynamics.initial_information(@slave) - dynamics.initial_information(@master) + flexmock(dynamics) + .should_receive(:set_port_info) + .with(@slave, nil, any).once.pass_thru + dynamics.should_receive(:set_port_info).pass_thru + dynamics.propagate([@slave, @master]) + assert dynamics.has_final_information_for_task?(@master) + assert dynamics.has_final_information_for_task?(@slave) + end + + it "resolves if called for the slave after the master on " \ + "a task whose outputs are not triggered by the task itself" do + @task_m.out_port.orogen_model.triggered_on_update = false + flexmock(dynamics) + .should_receive(:set_port_info) + .with(@slave, nil, any).once.pass_thru + dynamics.should_receive(:set_port_info).pass_thru + dynamics.propagate([@master, @slave]) + assert dynamics.has_final_information_for_task?(@master) + assert dynamics.has_final_information_for_task?(@slave) + end + + it "resolves if called for the master after the slave on " \ + "a task whose outputs are not triggered by the task itself" do + @task_m.out_port.orogen_model.triggered_on_update = false + dynamics.propagate([@slave, @master]) assert dynamics.has_final_information_for_task?(@master) assert dynamics.has_final_information_for_task?(@slave) end From 5bf679dfe5372fb213efdb0d600ce0ccba7fb75e Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Fri, 19 Dec 2025 11:37:48 -0300 Subject: [PATCH 04/15] fix: duplicate scheduler tasks appearing after lazy network adaptation of master/slave setups --- lib/syskit/deployment.rb | 6 +- lib/syskit/network_generation/engine.rb | 29 +-- .../runtime_network_adaptation.rb | 16 +- .../system_network_deployer.rb | 41 +++- test/network_generation/test_engine.rb | 94 ++++++++ .../test_runtime_network_adaptation.rb | 222 +++++++++++++++++- 6 files changed, 374 insertions(+), 34 deletions(-) diff --git a/lib/syskit/deployment.rb b/lib/syskit/deployment.rb index 6d38b8046..a207e510e 100644 --- a/lib/syskit/deployment.rb +++ b/lib/syskit/deployment.rb @@ -187,7 +187,7 @@ def read_only?(mapped_name) # model that should be used to create the task, if it is not the # same as the base model. This is used for specialized models (e.g. # dynamic services) - def task(name, syskit_task_model = nil) + def task(name, syskit_task_model = nil, setup_scheduler: true) if finishing? || finished? raise InvalidState, "#{self} is either finishing or already " \ @@ -196,7 +196,9 @@ def task(name, syskit_task_model = nil) orogen_task_deployment_model = deployed_orogen_model_by_name(name) task = create_deployed_task(orogen_task_deployment_model, syskit_task_model) - task_setup_scheduler(task, existing_tasks: executed_tasks_by_name) + if setup_scheduler + task_setup_scheduler(task, existing_tasks: executed_tasks_by_name) + end task end diff --git a/lib/syskit/network_generation/engine.rb b/lib/syskit/network_generation/engine.rb index 77a97d0e9..fec4415cd 100644 --- a/lib/syskit/network_generation/engine.rb +++ b/lib/syskit/network_generation/engine.rb @@ -539,20 +539,21 @@ def resolve( cleanup_resolution_errors: on_error != :commit ) merge_solver.merge_task_contexts_with_same_agent = early_deploy - required_instances, resolution_errors = resolve_system_network( - requirement_tasks, - garbage_collect: garbage_collect, - validate_abstract_network: validate_abstract_network, - validate_generated_network: validate_generated_network, - compute_deployments: compute_deployments, - default_deployment_group: default_deployment_group, - compute_policies: compute_policies, - validate_deployed_network: validate_deployed_network, - early_deploy: early_deploy, - capture_errors_during_network_resolution: - capture_errors_during_network_resolution, - cleanup_resolution_errors: cleanup_resolution_errors - ) + required_instances, resolution_errors = + resolve_system_network( + requirement_tasks, + garbage_collect: garbage_collect, + validate_abstract_network: validate_abstract_network, + validate_generated_network: validate_generated_network, + compute_deployments: compute_deployments, + default_deployment_group: default_deployment_group, + compute_policies: compute_policies, + validate_deployed_network: validate_deployed_network, + early_deploy: early_deploy, + capture_errors_during_network_resolution: + capture_errors_during_network_resolution, + cleanup_resolution_errors: cleanup_resolution_errors + ) # Can only be reached if the capture_error_during_network_resolution flag # is true diff --git a/lib/syskit/network_generation/runtime_network_adaptation.rb b/lib/syskit/network_generation/runtime_network_adaptation.rb index 2dea93e01..3371873d9 100644 --- a/lib/syskit/network_generation/runtime_network_adaptation.rb +++ b/lib/syskit/network_generation/runtime_network_adaptation.rb @@ -95,6 +95,9 @@ def import_from_runtime_plan # Find existing deployment tasks and finishing deployment tasks for the # system's used deployments + # + # @return an array of 3-tuples (deployment_task, existing_deployment_task, + # finishing_deployment) def used_deployments_find_existing( used_deployments, existing_deployments, finishing_deployments ) @@ -121,7 +124,6 @@ def finalize_used_deployments(used_deployments_with_existing) reused_deployed_tasks = Set.new selected_deployment_tasks = Set.new used_deployments_with_existing.each do |deployment, existing, finishing| - # Check for the corresponding task in the plan selected, new, reused = handle_required_deployment(deployment, existing, finishing) @@ -217,8 +219,10 @@ def used_deployment_instanciate(required) end deployed_tasks = required.tasks.map do |initial_deployed_task| - deployed_task = - deployment_task.task(initial_deployed_task.orocos_name) + deployed_task = deployment_task.task( + initial_deployed_task.orocos_name, + setup_scheduler: false + ) # !!! Cf. comment in SystemNetworkDeployer#apply_selected_deployments @merge_solver.apply_merge_group( @@ -509,8 +513,10 @@ def adapt_existing_create_new(task, existing_task, existing_deployment_task) end end - new_task = existing_deployment_task - .task(task.orocos_name, task.concrete_model) + new_task = + existing_deployment_task + .task(task.orocos_name, task.concrete_model, setup_scheduler: false) + debug do " created #{new_task} for #{task} (#{task.orocos_name})" end diff --git a/lib/syskit/network_generation/system_network_deployer.rb b/lib/syskit/network_generation/system_network_deployer.rb index f74137bf2..bef3b005c 100644 --- a/lib/syskit/network_generation/system_network_deployer.rb +++ b/lib/syskit/network_generation/system_network_deployer.rb @@ -86,8 +86,8 @@ def deploy(error_handler: RaiseErrorHandler.new, validate: true, interruption_point "syskit-netgen:select_deployments" if lazy - lazy_apply_selected_deployments(selected_deployments) - used_deployments = selected_deployments + used_deployments = + lazy_apply_selected_deployments(selected_deployments) else used_deployments = apply_selected_deployments(selected_deployments, deployment_tasks) @@ -236,9 +236,34 @@ def apply_selected_deployments(selected_deployments, deployment_tasks = {}) # subnet. This is not the goal here. merge_solver.apply_merge_group(task => deployed_task) used_deployments[deployed_task] = deployed_task_m + + used_deployments.merge!( + apply_selected_deployments_discover_schedulers( + deployed_task, deployed_task_m.configured_deployment + ) + ) end end + # Return entries compatible with used_deployments for a task's scheduler + # task(s) - resolved recursively + def apply_selected_deployments_discover_schedulers( + task, configured_deployment + ) + return {} unless task.orogen_model.master + + scheduler_task = task.scheduler_child + scheduler_name = scheduler_task.orocos_name + scheduler_deployed_task = Models::DeploymentGroup::DeployedTask.new( + configured_deployment, scheduler_name + ) + + recursive = apply_selected_deployments_discover_schedulers( + scheduler_task, configured_deployment + ) + { scheduler_task => scheduler_deployed_task }.merge(recursive) + end + # Apply deployments selected during {#deploy} by setting the task's # orocos_name argument accordingly # @@ -254,16 +279,24 @@ def lazy_apply_selected_deployments(selected_deployments) task.orogen_model = sel.orogen_model task.orogen_model.master end - return if with_master.empty? + return selected_deployments if with_master.empty? + used_deployments = selected_deployments.dup by_name = selected_deployments .to_h { |task, _| [task.orocos_name, task] } with_master.each do |task, sel| deployment = sel.configured_deployment scheduler_task = deployment.task_setup_scheduler(task, existing_tasks: by_name) - by_name[scheduler_task.orocos_name] = scheduler_task + scheduler_name = scheduler_task.orocos_name + by_name[scheduler_name] = scheduler_task + + scheduler_deployed_task = Models::DeploymentGroup::DeployedTask.new( + deployment, scheduler_name + ) + used_deployments[scheduler_task] = scheduler_deployed_task end + used_deployments end # Sanity checks to verify that the result of #deploy_system_network diff --git a/test/network_generation/test_engine.rb b/test/network_generation/test_engine.rb index ae318e61f..45cad030c 100644 --- a/test/network_generation/test_engine.rb +++ b/test/network_generation/test_engine.rb @@ -481,6 +481,100 @@ def deploy_dev_and_bus end end + describe "master/slave setups" do + before do + @task_m = task_m = TaskContext.new_submodel do + argument :name + end + @deployment_m = Deployment.new_submodel(name: "test") do + scheduled1 = task "scheduled1", task_m + scheduled2 = task "scheduled2", task_m + scheduler = task "scheduler", task_m + scheduled1.slave_of(scheduler) + scheduled2.slave_of(scheduler) + end + + @configured_deployment = + use_deployment(@deployment_m => "prefix_").first + end + + it "deploys slave tasks from scratch" do + syskit_deploy( + [@task_m.with_arguments(name: "1").prefer_deployed_tasks(/1/), + @task_m.with_arguments(name: "2").prefer_deployed_tasks(/2/)], + default_deployment_group: default_deployment_group + ) + + assert_master_slave_pattern_correct + end + + it "deploys a slave task when another of the same deployment exists" do + syskit_deploy( + [@task_m.with_arguments(name: "1").prefer_deployed_tasks(/1/)], + default_deployment_group: default_deployment_group + ) + + initial_tasks = plan.find_tasks(@task_m).to_a + execution_agent = initial_tasks.first.execution_agent + + syskit_deploy( + [@task_m.with_arguments(name: "1").prefer_deployed_tasks(/1/), + @task_m.with_arguments(name: "2").prefer_deployed_tasks(/2/)], + default_deployment_group: default_deployment_group + ) + + initial_tasks.each do |t| + assert plan.has_task?(t) + end + assert_master_slave_pattern_correct + end + + it "deploys slave tasks when the deploymentc exists" do + deployment_task = @configured_deployment.new(plan: plan) + + syskit_deploy( + [@task_m.with_arguments(name: "1").prefer_deployed_tasks(/1/), + @task_m.with_arguments(name: "2").prefer_deployed_tasks(/2/)], + default_deployment_group: default_deployment_group + ) + + assert_master_slave_pattern_correct(deployment_task: deployment_task) + end + + it "deploys slave tasks when the scheduler task exists" do + deployment_task = @configured_deployment.new(plan: plan) + scheduler_task = deployment_task.task("prefix_scheduler") + + syskit_deploy( + [@task_m.with_arguments(name: "1").prefer_deployed_tasks(/1/), + @task_m.with_arguments(name: "2").prefer_deployed_tasks(/2/)], + default_deployment_group: default_deployment_group + ) + + tasks = assert_master_slave_pattern_correct( + deployment_task: deployment_task + ) + assert_same scheduler_task, tasks[-1] + end + + def assert_master_slave_pattern_correct(deployment_task: nil) + tasks = plan.find_tasks(@task_m).sort_by(&:orocos_name) + assert_equal( + %w[prefix_scheduled1 prefix_scheduled2 prefix_scheduler], + tasks.map(&:orocos_name) + ) + tasks[0, 2].each do |scheduled_task| + assert_same tasks[-1], scheduled_task.scheduler_child + end + + deployment_task ||= tasks.first.execution_agent + tasks.each do |t| + assert_same deployment_task, t.execution_agent + end + tasks + end + end + describe "the hooks" do before do task_m = Syskit::TaskContext.new_submodel diff --git a/test/network_generation/test_runtime_network_adaptation.rb b/test/network_generation/test_runtime_network_adaptation.rb index 10cec067f..9f357f4db 100644 --- a/test/network_generation/test_runtime_network_adaptation.rb +++ b/test/network_generation/test_runtime_network_adaptation.rb @@ -62,6 +62,34 @@ module NetworkGeneration end end + it "successfully creates a new master/slave setup" do + task_m = TaskContext.new_submodel + deployment = Deployment.new_submodel do + scheduled = task "scheduled", task_m + scheduler = task "scheduler", task_m + scheduled.slave_of(scheduler) + end + + # NOTE: cannot call with 'scheduler', as creating 'scheduled' will + # automatically create the scheduler too + required_deployment, = + add_deployment_and_tasks(work_plan, deployment, %w[scheduled]) + adapter = create_adapter( + used_deployments_from_tasks(plan.find_tasks(task_m).to_a) + ) + adapter.finalize_deployed_tasks + + tasks = @work_plan.find_tasks(task_m).sort_by(&:orocos_name) + assert_equal %w[scheduled scheduler], tasks.map(&:orocos_name) + scheduled_task = tasks.first + scheduler_task = tasks.last + assert scheduled_task.depends_on?(scheduler_task) + assert_same required_deployment, + scheduler_task.execution_agent + assert_same required_deployment, + scheduler_task.execution_agent + end + it "updates an existing deployment, proxying the existing " \ "tasks and creating new ones" do deployment_m = create_deployment_model(task_count: 3) @@ -148,6 +176,75 @@ module NetworkGeneration adapter.finalize_deployed_tasks end end + + it "reuses an existing scheduler task" do + task_m = TaskContext.new_submodel + deployment_m = Deployment.new_submodel do + scheduled1 = task "scheduled1", task_m + scheduled2 = task "scheduled2", task_m + scheduler = task "scheduler", task_m + scheduled1.slave_of(scheduler) + scheduled2.slave_of(scheduler) + end + + _, (existing_scheduler,) = + add_deployment_and_tasks(plan, deployment_m, %w[scheduler]) + + add_deployment_and_tasks( + work_plan, deployment_m, %w[scheduled1 scheduled2] + ) + adapter = create_adapter( + used_deployments_from_tasks(work_plan.find_tasks(task_m).to_a) + ) + adapter.finalize_deployed_tasks + + tasks = @work_plan.find_tasks(task_m).sort_by(&:orocos_name) + assert_equal %w[scheduled1 scheduled2 scheduler], + tasks.map(&:orocos_name) + scheduler_task = tasks[-1] + assert_equal work_plan[existing_scheduler], scheduler_task + refute_nil scheduler_task.execution_agent + tasks[0, 2].each do |scheduled_task| + assert scheduled_task.depends_on?(scheduler_task) + assert_same scheduler_task.execution_agent, + scheduler_task.execution_agent + end + end + + it "properly instanciates a master/slave setup " \ + "if the deployment task already exists" do + task_m = TaskContext.new_submodel + deployment_m = Deployment.new_submodel do + scheduled1 = task "scheduled1", task_m + scheduled2 = task "scheduled2", task_m + scheduler = task "scheduler", task_m + scheduled1.slave_of(scheduler) + scheduled2.slave_of(scheduler) + end + + deployment, = add_deployment_and_tasks(plan, deployment_m, %w[]) + + add_deployment_and_tasks( + work_plan, deployment_m, %w[scheduled1 scheduled2] + ) + adapter = create_adapter( + used_deployments_from_tasks(work_plan.find_tasks(task_m).to_a) + ) + adapter.finalize_deployed_tasks + + tasks = @work_plan.find_tasks(task_m).sort_by(&:orocos_name) + assert_equal %w[scheduled1 scheduled2 scheduler], + tasks.map(&:orocos_name) + tasks.each do |t| + assert_equal work_plan[deployment], t.execution_agent + end + scheduler_task = tasks[-1] + tasks[0, 2].each do |scheduled_task| + assert scheduled_task.depends_on?(scheduler_task) + assert_same scheduler_task.execution_agent, + scheduler_task.execution_agent + end + end end describe "lazily deployed networks" do @@ -280,6 +377,109 @@ module NetworkGeneration adapter.finalize_deployed_tasks end end + + it "successfully creates a new master/slave setup" do + task_m = TaskContext.new_submodel + deployment_m = Deployment.new_submodel do + scheduled1 = task "scheduled1", task_m + scheduled2 = task "scheduled2", task_m + scheduler = task "scheduler", task_m + scheduled1.slave_of(scheduler) + scheduled2.slave_of(scheduler) + end + + tasks = add_lazy_tasks( + work_plan, deployment_m, %w[scheduled1 scheduled2 scheduler] + ) + tasks[0].depends_on(tasks.last) + tasks[1].depends_on(tasks.last) + adapter = create_adapter( + used_deployments_from_lazy(tasks, deployment_m) + ) + adapter.finalize_deployed_tasks + + tasks = @work_plan.find_tasks(task_m).sort_by(&:orocos_name) + assert_equal %w[scheduled1 scheduled2 scheduler], + tasks.map(&:orocos_name) + scheduler_task = tasks[-1] + refute_nil scheduler_task.execution_agent + tasks[0, 2].each do |scheduled_task| + assert scheduled_task.depends_on?(scheduler_task) + assert_same scheduler_task.execution_agent, + scheduler_task.execution_agent + end + end + + it "reuses an existing scheduler task" do + task_m = TaskContext.new_submodel + deployment_m = Deployment.new_submodel do + scheduled1 = task "scheduled1", task_m + scheduled2 = task "scheduled2", task_m + scheduler = task "scheduler", task_m + scheduled1.slave_of(scheduler) + scheduled2.slave_of(scheduler) + end + + _, (existing_scheduler,) = + add_deployment_and_tasks(plan, deployment_m, %w[scheduler]) + + tasks = add_lazy_tasks( + work_plan, deployment_m, %w[scheduled1 scheduled2 scheduler] + ) + tasks[0].depends_on(tasks.last) + tasks[1].depends_on(tasks.last) + adapter = create_adapter( + used_deployments_from_lazy(tasks, deployment_m) + ) + adapter.finalize_deployed_tasks + + tasks = @work_plan.find_tasks(task_m).sort_by(&:orocos_name) + assert_equal %w[scheduled1 scheduled2 scheduler], + tasks.map(&:orocos_name) + scheduler_task = tasks[-1] + assert_equal work_plan[existing_scheduler], scheduler_task + refute_nil scheduler_task.execution_agent + tasks[0, 2].each do |scheduled_task| + assert scheduled_task.depends_on?(scheduler_task) + assert_same scheduler_task.execution_agent, + scheduler_task.execution_agent + end + end + + it "properly instanciates a master/slave setup " \ + "if the deployment task already exists" do + task_m = TaskContext.new_submodel + deployment_m = Deployment.new_submodel do + scheduled1 = task "scheduled1", task_m + scheduled2 = task "scheduled2", task_m + scheduler = task "scheduler", task_m + scheduled1.slave_of(scheduler) + scheduled2.slave_of(scheduler) + end + + add_deployment_and_tasks(plan, deployment_m, %w[]) + + tasks = add_lazy_tasks( + work_plan, deployment_m, %w[scheduled1 scheduled2 scheduler] + ) + tasks[0].depends_on(tasks.last) + tasks[1].depends_on(tasks.last) + adapter = create_adapter( + used_deployments_from_lazy(tasks, deployment_m) + ) + adapter.finalize_deployed_tasks + + tasks = @work_plan.find_tasks(task_m).sort_by(&:orocos_name) + assert_equal %w[scheduled1 scheduled2 scheduler], + tasks.map(&:orocos_name) + scheduler_task = tasks[-1] + refute_nil scheduler_task.execution_agent + tasks[0, 2].each do |scheduled_task| + assert scheduled_task.depends_on?(scheduler_task) + assert_same scheduler_task.execution_agent, + scheduler_task.execution_agent + end + end end def create_deployment_model(task_count:) @@ -524,22 +724,26 @@ def create_adapter(used_deployments) end def used_deployments_from_tasks(tasks) + configured_deployments = tasks.each_with_object({}) do |task, per_name| + name = task.execution_agent.process_name + per_name[name] ||= flexmock(process_name: name) + end + tasks.each_with_object({}) do |task, used_deployments| + name = task.execution_agent.process_name used_deployments[task] = flexmock( - configured_deployment: flexmock( - process_name: task.execution_agent.process_name - ) + configured_deployment: configured_deployments.fetch(name) ) end end def used_deployments_from_lazy(tasks, deployment_m) + configured_deployment = Models::ConfiguredDeployment.new( + "localhost", deployment_m + ) tasks.each_with_object({}) do |task, used_deployments| - used_deployments[task] = flexmock( - configured_deployment: Models::ConfiguredDeployment.new( - "localhost", deployment_m - ) - ) + used_deployments[task] = + flexmock(configured_deployment: configured_deployment) end end @@ -561,7 +765,7 @@ def initialize(task_m, **arguments) event :ready - define_method :task do |task_name, task_model = nil, record: true| + define_method :task do |task_name, task_model = nil, record: true, **| task = @task_m.new(orocos_name: task_name) @created_tasks << [task_name, task_model, task] if record task.executed_by self From ccb046b41865eee822b6ebc5dc64d65cb1501df0 Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Fri, 5 Dec 2025 17:00:56 -0300 Subject: [PATCH 05/15] chore: cleanup naming of some more netgen-related timepoints --- lib/syskit/network_generation/async_fiber.rb | 4 +++- lib/syskit/network_generation/engine.rb | 12 ++++++------ .../network_generation/runtime_network_adaptation.rb | 1 + 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/syskit/network_generation/async_fiber.rb b/lib/syskit/network_generation/async_fiber.rb index 78ff9edbc..a6c2ab5f2 100644 --- a/lib/syskit/network_generation/async_fiber.rb +++ b/lib/syskit/network_generation/async_fiber.rb @@ -40,6 +40,8 @@ def initialize( resolution_control: Control.new(slice), event_logger: event_logger, resolver_options: resolver_options) + log_timepoint("syskit-netgen:async-fiber-start") + # Protect all Component instances against garbage collection # by adding them in a transaction. This is to make sure we don't # tear down, during network generation, subparts of the old network @@ -49,7 +51,7 @@ def initialize( @keepalive.wrap(component_task) unless component_task.finished? end - log_timepoint("syskit-netgen:async-fiber-start") + log_timepoint("syskit-netgen:created-keepalive-transaction") @fiber = Fiber.new do catch(:syskit_netgen_cancelled) do diff --git a/lib/syskit/network_generation/engine.rb b/lib/syskit/network_generation/engine.rb index fec4415cd..1759364cd 100644 --- a/lib/syskit/network_generation/engine.rb +++ b/lib/syskit/network_generation/engine.rb @@ -184,11 +184,11 @@ def apply_deployed_network_to_plan if @dataflow_dynamics @dataflow_dynamics.apply_merges(merge_solver) - log_timepoint "apply_merged_to_dataflow_dynamics" + log_timepoint "syskit-netgen:apply_merged_to_dataflow_dynamics" end Engine.deployment_postprocessing.each do |block| block.call(self, work_plan) - log_timepoint "postprocessing:#{block}" + log_timepoint "syskit-netgen:postprocessing:#{block}" end end @@ -597,13 +597,12 @@ def apply_system_network_to_plan( required_instances = required_instances.transform_values do |task| merge_solver.replacement_for(task) end - log_timepoint "apply_merge_to_stored_instances" fix_toplevel_tasks(required_instances) - log_timepoint "fix_toplevel_tasks" + log_timepoint "syskit-netgen:fixup" Engine.final_network_postprocessing.each do |block| block.call(self, work_plan) - log_timepoint "final_network_postprocessing:#{block}" + log_timepoint "syskit-netgen:final_network_postprocessing:#{block}" end # Finally, we should now only have deployed tasks. Verify it @@ -611,10 +610,11 @@ def apply_system_network_to_plan( if garbage_collect && validate_final_network validate_final_network(required_instances, work_plan, compute_deployments: compute_deployments) - log_timepoint "validate_final_network" + log_timepoint "syskit-netgen:validate_final_network" end commit_work_plan + log_timepoint "syskit-netgen:commit" end def discard_work_plan diff --git a/lib/syskit/network_generation/runtime_network_adaptation.rb b/lib/syskit/network_generation/runtime_network_adaptation.rb index 3371873d9..f90c4aaa0 100644 --- a/lib/syskit/network_generation/runtime_network_adaptation.rb +++ b/lib/syskit/network_generation/runtime_network_adaptation.rb @@ -39,6 +39,7 @@ def initialize( def apply result = finalize_deployed_tasks sever_old_plan_from_new_plan + log_timepoint "syskit-netgen:sever-old-from-new-plan" result end From 5c9be6b09125a197d31b7f0393499243f8d0a302 Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Tue, 16 Dec 2025 12:18:09 -0300 Subject: [PATCH 06/15] fix: make the async-fiber region a timepoint group --- lib/syskit/network_generation/async_fiber.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/syskit/network_generation/async_fiber.rb b/lib/syskit/network_generation/async_fiber.rb index a6c2ab5f2..74e40d35e 100644 --- a/lib/syskit/network_generation/async_fiber.rb +++ b/lib/syskit/network_generation/async_fiber.rb @@ -40,7 +40,7 @@ def initialize( resolution_control: Control.new(slice), event_logger: event_logger, resolver_options: resolver_options) - log_timepoint("syskit-netgen:async-fiber-start") + log_timepoint_group_start("syskit-netgen:async-fiber") # Protect all Component instances against garbage collection # by adding them in a transaction. This is to make sure we don't @@ -154,7 +154,7 @@ def result def finished! @finished = true @keepalive.discard_transaction unless @keepalive.finalized? - log_timepoint("syskit-netgen:async-fiber-finished") + log_timepoint_group_end("syskit-netgen:async-fiber") end # Wait for the resolution to finish and either apply the result or raise if From dcd18e136bfb65c55d4aa56643a22aafde60a787 Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Tue, 16 Dec 2025 13:19:45 -0300 Subject: [PATCH 07/15] chore: resume to the resolution fiber on .start This is inconsequential for live systems, but makes general performance debugging easier. --- lib/syskit/network_generation/async_fiber.rb | 8 ++++++- .../test_apply_requirement_modifications.rb | 23 ++++++++++++++----- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/lib/syskit/network_generation/async_fiber.rb b/lib/syskit/network_generation/async_fiber.rb index 74e40d35e..9b9b4bb14 100644 --- a/lib/syskit/network_generation/async_fiber.rb +++ b/lib/syskit/network_generation/async_fiber.rb @@ -89,7 +89,13 @@ def async_phase def self.start( plan, requirement_tasks = default_requirement_tasks, resolver_options: {} ) - new(plan, requirement_tasks, resolver_options: resolver_options) + async = new(plan, requirement_tasks, resolver_options: resolver_options) + async.start unless Syskit.conf.resolution_time_slice == 0 + async + end + + def start + @fiber.resume(false) end # Cancel this resolution diff --git a/test/runtime/test_apply_requirement_modifications.rb b/test/runtime/test_apply_requirement_modifications.rb index d2078e805..c979c2b21 100644 --- a/test/runtime/test_apply_requirement_modifications.rb +++ b/test/runtime/test_apply_requirement_modifications.rb @@ -55,6 +55,9 @@ module Runtime describe "with a cancelled resolution running" do before do + @__resolution_time_slice = Syskit.conf.resolution_time_slice + Syskit.conf.resolution_time_slice = 0 + @cmp_m = Composition.new_submodel plan.add_permanent_task( cmp = @cmp_m.to_instance_requirements.as_plan @@ -65,6 +68,10 @@ module Runtime execute { plan.syskit_cancel_async_resolution } end + after do + Syskit.conf.resolution_time_slice = @__resolution_time_slice + end + it "waits for the resolution end but does not apply the result" do execute { plan.syskit_join_current_resolution } @@ -94,12 +101,17 @@ module Runtime describe ".apply_requirement_modifications" do before do + @__fiber_slice = + Syskit.conf.resolution_time_slice @__capture_errors_feature_flag = Syskit.conf.capture_errors_during_network_resolution? + Syskit.conf.resolution_time_slice = 0 Syskit.conf.capture_errors_during_network_resolution = false end after do + Syskit.conf.resolution_time_slice = + @__fiber_slice Syskit.conf.capture_errors_during_network_resolution = @__capture_errors_feature_flag end @@ -278,16 +290,15 @@ module Runtime end execute { Runtime.apply_requirement_modifications(plan) } - loop do - break unless plan.syskit_has_async_resolution? - + deadline = Time.now + 1 + until Time.now > deadline execute { plan.syskit_join_current_resolution } execute { Runtime.apply_requirement_modifications(plan) } + success = reqs.all?(&:resolution_success?) + break if success end - reqs.each do |req| - assert req.resolution_success? - end + assert success end it "while using force, cancels pending resolutions and executes " \ From 5a4db2a652fc6ba68267ce03d02bac0a3ae9b90b Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Tue, 16 Dec 2025 13:20:42 -0300 Subject: [PATCH 08/15] fix: do not create logger tasks if there's nothing to log --- lib/syskit/deployment.rb | 8 +++--- lib/syskit/network_generation/logger.rb | 27 ++++++++++++------- .../process_managers/in_process/manager.rb | 8 +++--- .../process_managers/ruby_tasks/manager.rb | 4 +-- lib/syskit/roby_app/configuration.rb | 6 +++-- 5 files changed, 34 insertions(+), 19 deletions(-) diff --git a/lib/syskit/deployment.rb b/lib/syskit/deployment.rb index a207e510e..39248aa4d 100644 --- a/lib/syskit/deployment.rb +++ b/lib/syskit/deployment.rb @@ -311,7 +311,7 @@ def logger_name # # @return [TaskContext,nil] either the logging task, or nil if this # deployment has none - def logger_task + def logger_task(create: true) return unless logging_enabled? if arguments[:logger_task] @@ -323,14 +323,16 @@ def logger_task @logger_task = if logger_task&.fullfills?(LoggerService) logger_task - else + elsif create instanciate_default_logger_task(logger_name) end @logger_task&.default_logger = true end - @logger_task ||= process_server_config.default_logger_task(plan) + return @logger_task if @logger_task + + @logger_task = process_server_config.default_logger_task(plan, create: create) end # Instanciates a new default logger diff --git a/lib/syskit/network_generation/logger.rb b/lib/syskit/network_generation/logger.rb index 9a36a14c8..affa8cc68 100644 --- a/lib/syskit/network_generation/logger.rb +++ b/lib/syskit/network_generation/logger.rb @@ -142,19 +142,25 @@ def self.add_logging_to_network(engine, work_plan) next unless deployment.log_port?(p) log_port_name = "#{t.orocos_name}.#{p.name}" - connections[[p.name, log_port_name]] = Hash[fallback_policy: fallback_policy] + connections[[p.name, log_port_name]] = + Hash[fallback_policy: fallback_policy] required_logging_ports << [log_port_name, t, p] end required_connections << [t, connections] end - unless (logger_task = deployment.logger_task) - warn "deployment #{deployment.process_name} has no usable " \ - "logger (default logger name would be " \ - "#{deployment.process_name}_Logger)" + logger_task = deployment.logger_task( + create: !required_logging_ports.empty? + ) + unless logger_task + unless required_logging_ports.empty? + warn "deployment #{deployment.process_name} has no usable " \ + "logger (default logger name would be " \ + "#{deployment.process_name}_Logger)" + end next end - logger_task = work_plan[deployment.logger_task] + logger_task = work_plan[logger_task] # Disconnect current log connections, we're going to # reestablish the ones we want later on. We leave other @@ -184,9 +190,12 @@ def self.add_logging_to_network(engine, work_plan) # # Otherwise, Logger#configure will take care of it for # us - required_logging_ports.each do |port_name, logged_task, logged_port| - logger_task.create_logging_port(port_name, logged_task, logged_port) - end + required_logging_ports + .each do |port_name, logged_task, logged_port| + logger_task.create_logging_port( + port_name, logged_task, logged_port + ) + end end required_connections.each do |task, connections| connections = diff --git a/lib/syskit/process_managers/in_process/manager.rb b/lib/syskit/process_managers/in_process/manager.rb index f18508a3c..25118a483 100644 --- a/lib/syskit/process_managers/in_process/manager.rb +++ b/lib/syskit/process_managers/in_process/manager.rb @@ -131,8 +131,8 @@ def stop(process_name) DEFAULT_LOGGER_NAME = "syskit_in_process_logger" - def default_logger_task(plan, app: Roby.app) - self.class.default_logger_task(plan, app: app) + def default_logger_task(plan, app: Roby.app, create: true) + self.class.default_logger_task(plan, app: app, create: create) end def self.find_default_logger_task(plan, app: Roby.app) @@ -152,11 +152,13 @@ def self.find_default_logger_deployment_task(plan, app: Roby.app) end # The logger task - def self.default_logger_task(plan, app: Roby.app) + def self.default_logger_task(plan, app: Roby.app, create: true) return unless (deployment = app.syskit_in_process_logger_deployment) if (t = find_default_logger_task(plan)) return t + elsif !create + return end deployment_t = find_default_logger_deployment_task(plan, app: app) diff --git a/lib/syskit/process_managers/ruby_tasks/manager.rb b/lib/syskit/process_managers/ruby_tasks/manager.rb index 80c7829d3..042ee5630 100644 --- a/lib/syskit/process_managers/ruby_tasks/manager.rb +++ b/lib/syskit/process_managers/ruby_tasks/manager.rb @@ -111,8 +111,8 @@ def dead_deployment(deployment_name, status = Status.new(true)) terminated_deployments[deployment] = status end - def default_logger_task(plan, app: Roby.app) - InProcess::Manager.default_logger_task(plan, app: app) + def default_logger_task(plan, app: Roby.app, create: true) + InProcess::Manager.default_logger_task(plan, app: app, create: create) end end end diff --git a/lib/syskit/roby_app/configuration.rb b/lib/syskit/roby_app/configuration.rb index 56d35547f..5a9465e8a 100644 --- a/lib/syskit/roby_app/configuration.rb +++ b/lib/syskit/roby_app/configuration.rb @@ -909,8 +909,10 @@ def register_on_name_server? register_on_name_server end - def default_logger_task(plan) - client.default_logger_task(plan) if client.respond_to?(:default_logger_task) + def default_logger_task(plan, create: true) + if client.respond_to?(:default_logger_task) + client.default_logger_task(plan, create: create) + end end end From 7b7dae3f88b896564544d1eaee8b4dfd66ba45d1 Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Fri, 5 Dec 2025 17:00:56 -0300 Subject: [PATCH 09/15] chore: micro-optimize the "through_method_missing" accesses --- .../actions/interface_model_extension.rb | 48 +++++++++---------- lib/syskit/actions/profile.rb | 30 +++++++----- lib/syskit/bound_data_service.rb | 16 ++----- lib/syskit/component.rb | 22 +++++---- .../models/data_monitoring_table.rb | 7 ++- .../models/fault_response_table_extension.rb | 18 ++----- .../coordination/models/port_handling.rb | 8 +--- lib/syskit/coordination/port_handling.rb | 8 +--- lib/syskit/instance_requirements.rb | 25 +++------- lib/syskit/models/bound_data_service.rb | 14 +----- lib/syskit/models/component.rb | 27 ++++------- lib/syskit/models/composition.rb | 12 +---- lib/syskit/models/faceted_access.rb | 14 +----- lib/syskit/models/placeholder.rb | 16 +------ lib/syskit/models/port_access.rb | 12 +---- lib/syskit/port_access.rb | 12 +---- lib/syskit/queries/abstract_component_base.rb | 18 ++++--- lib/syskit/robot/master_device_instance.rb | 7 ++- lib/syskit/robot/robot_definition.rb | 16 ++----- lib/syskit/task_context.rb | 12 +---- lib/syskit/test/profile_test.rb | 19 +------- 21 files changed, 112 insertions(+), 249 deletions(-) diff --git a/lib/syskit/actions/interface_model_extension.rb b/lib/syskit/actions/interface_model_extension.rb index 0d99e2bb6..23b692557 100644 --- a/lib/syskit/actions/interface_model_extension.rb +++ b/lib/syskit/actions/interface_model_extension.rb @@ -185,20 +185,6 @@ def use_profile_object( register_action_from_profile(definition.to_action_model) end end - - include MetaRuby::DSLs::FindThroughMethodMissing - - def has_through_method_missing?(name) - MetaRuby::DSLs.has_through_method_missing?( - profile, name, "_tag" => :has_tag? - ) || super - end - - def find_through_method_missing(name, args) - MetaRuby::DSLs.find_through_method_missing( - profile, name, args, "_tag" => :find_tag - ) || super - end end # @api private @@ -217,32 +203,44 @@ def setup_main_profile(profile) end end - Roby::Actions::Models::Library.include LibraryExtension - Roby::Actions::Interface.extend LibraryExtension - Roby::Actions::Interface.extend InterfaceModelExtension - # @api private # - # Module injected in {Roby::Actions::Interface}, i.e. instance-level methods - module InterfaceExtension - def profile - self.class.profile - end + # Definition of the delegation of the `_tag` methods to `profile` + module FindTag + HAS_THROUGH_METHOD_MISSING = { "_tag" => :has_tag? }.freeze + FIND_THROUGH_METHOD_MISSING = { "_tag" => :find_tag }.freeze def has_through_method_missing?(name) MetaRuby::DSLs.has_through_method_missing?( - profile, name, "_tag" => :has_tag? + profile, name, HAS_THROUGH_METHOD_MISSING ) || super end def find_through_method_missing(name, args) MetaRuby::DSLs.find_through_method_missing( - profile, name, args, "_tag" => :find_tag + profile, name, args, FIND_THROUGH_METHOD_MISSING ) || super end include MetaRuby::DSLs::FindThroughMethodMissing end + + # @api private + # + # Module injected in {Roby::Actions::Interface}, i.e. instance-level methods + module InterfaceExtension + def profile + self.class.profile + end + end + + Roby::Actions::Models::Library.include LibraryExtension + Roby::Actions::Models::Library.include FindTag + Roby::Actions::Interface.extend LibraryExtension + Roby::Actions::Interface.extend InterfaceModelExtension + Roby::Actions::Interface.extend FindTag + Roby::Actions::Interface.include InterfaceExtension + Roby::Actions::Interface.include FindTag end end diff --git a/lib/syskit/actions/profile.rb b/lib/syskit/actions/profile.rb index e1669460f..48a51bf7d 100644 --- a/lib/syskit/actions/profile.rb +++ b/lib/syskit/actions/profile.rb @@ -701,25 +701,31 @@ def find_device_requirements_by_name(name) end end + HAS_THROUGH_METHOD_MISSING = { + "_tag" => :has_tag?, + "_def" => :has_definition?, + "_dev" => :has_device?, + "_task" => :has_deployed_task?, + "_deployment_group" => :has_deployment_group? + }.freeze + def has_through_method_missing?(m) MetaRuby::DSLs.has_through_method_missing?( - self, m, - "_tag" => :has_tag?, - "_def" => :has_definition?, - "_dev" => :has_device?, - "_task" => :has_deployed_task?, - "_deployment_group" => :has_deployment_group? + self, m, HAS_THROUGH_METHOD_MISSING ) || super end + FIND_THROUGH_METHOD_MISSING = { + "_tag" => :find_tag, + "_def" => :find_definition_by_name, + "_dev" => :find_device_requirements_by_name, + "_task" => :find_deployed_task_by_name, + "_deployment_group" => :find_deployment_group_by_name + }.freeze + def find_through_method_missing(m, args) MetaRuby::DSLs.find_through_method_missing( - self, m, args, - "_tag" => :find_tag, - "_def" => :find_definition_by_name, - "_dev" => :find_device_requirements_by_name, - "_task" => :find_deployed_task_by_name, - "_deployment_group" => :find_deployment_group_by_name + self, m, args, FIND_THROUGH_METHOD_MISSING ) || super end diff --git a/lib/syskit/bound_data_service.rb b/lib/syskit/bound_data_service.rb index bb5bd55ba..d528f80b9 100644 --- a/lib/syskit/bound_data_service.rb +++ b/lib/syskit/bound_data_service.rb @@ -159,19 +159,9 @@ def to_instance_requirements req end - def has_through_method_missing?(m) - MetaRuby::DSLs.has_through_method_missing?( - self, m, - "_srv" => :has_data_service? - ) || super - end - - def find_through_method_missing(m, args) - MetaRuby::DSLs.find_through_method_missing( - self, m, args, - "_srv" => :find_data_service - ) || super - end + MetaRuby::DSLs::FindThroughMethodMissing.standard( + self, { "_srv" => "data_service" } + ) DRoby = Struct.new :component, :model do def proxy(peer) diff --git a/lib/syskit/component.rb b/lib/syskit/component.rb index f3a9ff401..249870614 100644 --- a/lib/syskit/component.rb +++ b/lib/syskit/component.rb @@ -700,21 +700,27 @@ def deregister_data_reader(name) @data_readers.each(&:disconnect) end + HAS_THROUGH_METHOD_MISSING = { + "_srv" => :has_data_service?, + "_writer" => :find_registered_data_writer, + "_reader" => :find_registered_data_reader + }.freeze + + FIND_THROUGH_METHOD_MISSING = { + "_srv" => :find_data_service, + "_writer" => :find_registered_data_writer, + "_reader" => :find_registered_data_reader + }.freeze + def has_through_method_missing?(m) MetaRuby::DSLs.has_through_method_missing?( - self, m, - "_srv" => :has_data_service?, - "_writer" => :find_registered_data_writer, - "_reader" => :find_registered_data_reader + self, m, HAS_THROUGH_METHOD_MISSING ) || super end def find_through_method_missing(m, args) MetaRuby::DSLs.find_through_method_missing( - self, m, args, - "_srv" => :find_data_service, - "_writer" => :find_registered_data_writer, - "_reader" => :find_registered_data_reader + self, m, args, FIND_THROUGH_METHOD_MISSING ) || super end diff --git a/lib/syskit/coordination/models/data_monitoring_table.rb b/lib/syskit/coordination/models/data_monitoring_table.rb index a18bf38ce..578d26e97 100644 --- a/lib/syskit/coordination/models/data_monitoring_table.rb +++ b/lib/syskit/coordination/models/data_monitoring_table.rb @@ -99,15 +99,18 @@ def attach_to(query) attachment_points << query end + HAS_THROUGH_METHOD_MISSING = { "_port" => :has_port? }.freeze + FIND_THROUGH_METHOD_MISSING = { "_port" => :find_port }.freeze + def has_through_method_missing?(m) MetaRuby::DSLs.has_through_method_missing?( - root, m, "_port" => :has_port? + root, m, HAS_THROUGH_METHOD_MISSING ) || super end def find_through_method_missing(m, args) MetaRuby::DSLs.find_through_method_missing( - root, m, args, "_port" => :find_port + root, m, args, FIND_THROUGH_METHOD_MISSING ) || super end diff --git a/lib/syskit/coordination/models/fault_response_table_extension.rb b/lib/syskit/coordination/models/fault_response_table_extension.rb index 93e8171b0..b1b810c45 100644 --- a/lib/syskit/coordination/models/fault_response_table_extension.rb +++ b/lib/syskit/coordination/models/fault_response_table_extension.rb @@ -77,26 +77,14 @@ def find_monitor(name) nil end - def has_through_method_missing?(m) - MetaRuby::DSLs.has_through_method_missing?( - self, m, "_monitor" => :find_monitor - ) || super - end - - def find_through_method_missing(m, args) - MetaRuby::DSLs.find_through_method_missing( - self, m, args, "_monitor" => :find_monitor - ) || super - end - - include MetaRuby::DSLs::FindThroughMethodMissing + MetaRuby::DSLs::FindThroughMethodMissing.standard(self, %w[monitor]) def respond_to_missing?(m, include_private) - arguments[m] || super + arguments.key?(m) || super end def method_missing(m, *args, &block) - if arg = arguments[m] + if (arg = arguments[m]) Roby::Coordination::Models::Variable.new(m) else super diff --git a/lib/syskit/coordination/models/port_handling.rb b/lib/syskit/coordination/models/port_handling.rb index b0298d5f4..6d6ce5cdf 100644 --- a/lib/syskit/coordination/models/port_handling.rb +++ b/lib/syskit/coordination/models/port_handling.rb @@ -25,13 +25,7 @@ def self_port_to_component_port(port) model.self_port_to_component_port(port) end - def has_through_method_missing?(m) - MetaRuby::DSLs.has_through_method_missing?(self, m, "_port" => :has_port?) || super - end - - def find_through_method_missing(m, args) - MetaRuby::DSLs.find_through_method_missing(self, m, args, "_port" => :find_port) || super - end + MetaRuby::DSLs::FindThroughMethodMissing.standard(self, %w[port]) end end end diff --git a/lib/syskit/coordination/port_handling.rb b/lib/syskit/coordination/port_handling.rb index 54030d44e..0d3e2ff65 100644 --- a/lib/syskit/coordination/port_handling.rb +++ b/lib/syskit/coordination/port_handling.rb @@ -31,13 +31,7 @@ def self_port_to_component_port(port) end end - def has_through_method_missing?(m) - MetaRuby::DSLs.has_through_method_missing?(self, m, "_port" => :has_port?) || super - end - - def find_through_method_missing(m, args) - MetaRuby::DSLs.find_through_method_missing(self, m, args, "_port" => :find_port) || super - end + MetaRuby::DSLs::FindThroughMethodMissing.standard(self, %w[port]) end class OutputPort < Syskit::OutputPort diff --git a/lib/syskit/instance_requirements.rb b/lib/syskit/instance_requirements.rb index 809bce6f5..cf2429296 100644 --- a/lib/syskit/instance_requirements.rb +++ b/lib/syskit/instance_requirements.rb @@ -1349,25 +1349,12 @@ def each_child end end - def has_through_method_missing?(name) - MetaRuby::DSLs.has_through_method_missing?( - self, name, - "_srv" => :has_data_service?, - "_child" => :has_child?, - "_port" => :has_port? - ) || super - end - - def find_through_method_missing(name, args) - MetaRuby::DSLs.find_through_method_missing( - self, name, args, - "_srv" => :find_data_service, - "_child" => :find_child, - "_port" => :find_port - ) || super - end - - include MetaRuby::DSLs::FindThroughMethodMissing + MetaRuby::DSLs::FindThroughMethodMissing.standard( + self, + "_srv" => "data_service", + "_child" => "child", + "_port" => "port" + ) # Generates the InstanceRequirements object that represents +self+ # best diff --git a/lib/syskit/models/bound_data_service.rb b/lib/syskit/models/bound_data_service.rb index f0f2eeea5..1220994c1 100644 --- a/lib/syskit/models/bound_data_service.rb +++ b/lib/syskit/models/bound_data_service.rb @@ -398,18 +398,8 @@ def find_data_service(name) nil end - def has_through_method_missing?(m) - MetaRuby::DSLs.has_through_method_missing?( - self, m, "_srv" => :has_data_service? - ) || super - end - - def find_through_method_missing(m, args) - MetaRuby::DSLs.find_through_method_missing( - self, m, args, - "_srv" => :find_data_service - ) || super - end + MetaRuby::DSLs::FindThroughMethodMissing + .standard(self, { "_srv" => "data_service" }) # Whether two services are the same service bound to two different interfaces # diff --git a/lib/syskit/models/component.rb b/lib/syskit/models/component.rb index dd278e2af..833395462 100644 --- a/lib/syskit/models/component.rb +++ b/lib/syskit/models/component.rb @@ -1399,25 +1399,14 @@ def data_writer(port, as:, **policy) data_writers[as] = port.to_bound_data_accessor(as, self, **policy) end - def has_through_method_missing?(name) - MetaRuby::DSLs.has_through_method_missing?( - self, name, - "_srv" => :find_data_service, - "_reader" => :find_data_reader, - "_writer" => :find_data_writer - ) || super - end - - def find_through_method_missing(name, args) - MetaRuby::DSLs.find_through_method_missing( - self, name, args, - "_srv" => :find_data_service, - "_reader" => :find_data_reader, - "_writer" => :find_data_writer - ) || super - end - - include MetaRuby::DSLs::FindThroughMethodMissing + MetaRuby::DSLs::FindThroughMethodMissing.standard( + self, + { + "_srv" => :data_service, + "_reader" => :data_reader, + "_writer" => :data_writer + } + ) ruby2_keywords def method_missing(name, *args, &block) # rubocop:disable Style/MissingRespondToMissing if name == :orogen_model diff --git a/lib/syskit/models/composition.rb b/lib/syskit/models/composition.rb index 9ae108b7d..1d22b8dca 100644 --- a/lib/syskit/models/composition.rb +++ b/lib/syskit/models/composition.rb @@ -1254,17 +1254,7 @@ def setup_submodel(submodel, register_specializations: true, submodel end - def has_through_method_missing?(m) - MetaRuby::DSLs.has_through_method_missing?( - self, m, "_child" => :find_child - ) || super - end - - def find_through_method_missing(m, args) - MetaRuby::DSLs.find_through_method_missing( - self, m, args, "_child" => :find_child - ) || super - end + MetaRuby::DSLs::FindThroughMethodMissing.standard(self, %w[child]) # Helper method for {#promote_exported_output} and # {#promote_exported_input} diff --git a/lib/syskit/models/faceted_access.rb b/lib/syskit/models/faceted_access.rb index 22eda3fb9..4f8521130 100644 --- a/lib/syskit/models/faceted_access.rb +++ b/lib/syskit/models/faceted_access.rb @@ -134,19 +134,7 @@ def to_s "#{object}.as(#{required.each_required_model.map(&:to_s).sort.join(',')})" end - def has_through_method_missing?(m) - MetaRuby::DSLs.has_through_method_missing?( - self, m, "_port" => :has_port? - ) || super - end - - def find_through_method_missing(m, args) - MetaRuby::DSLs.find_through_method_missing( - self, m, args, "_port" => :find_port - ) || super - end - - include MetaRuby::DSLs::FindThroughMethodMissing + MetaRuby::DSLs::FindThroughMethodMissing.standard(self, %w[port]) end end end diff --git a/lib/syskit/models/placeholder.rb b/lib/syskit/models/placeholder.rb index dffcc4dec..e49be80a6 100644 --- a/lib/syskit/models/placeholder.rb +++ b/lib/syskit/models/placeholder.rb @@ -146,21 +146,7 @@ def placeholder? true end - def has_through_method_missing?(m) - MetaRuby::DSLs.has_through_method_missing?( - self, m, - "_port" => :has_port? - ) || super - end - - def find_through_method_missing(m, args) - MetaRuby::DSLs.find_through_method_missing( - self, m, args, - "_port" => :find_port - ) || super - end - - include MetaRuby::DSLs::FindThroughMethodMissing + MetaRuby::DSLs::FindThroughMethodMissing.standard(self, %w[port]) # Encapsulation of the methods that allow to create placeholder # models diff --git a/lib/syskit/models/port_access.rb b/lib/syskit/models/port_access.rb index 3521b6514..cba2b9e85 100644 --- a/lib/syskit/models/port_access.rb +++ b/lib/syskit/models/port_access.rb @@ -9,17 +9,7 @@ module PortAccess # to the corresponding Models::Port instance attribute(:ports) { {} } - def has_through_method_missing?(m) - MetaRuby::DSLs.has_through_method_missing?( - self, m, "_port" => :has_port? - ) || super - end - - def find_through_method_missing(m, args) - MetaRuby::DSLs.find_through_method_missing( - self, m, args, "_port" => :find_port - ) || super - end + MetaRuby::DSLs::FindThroughMethodMissing.standard(self, %w[port]) # Returns the port object that maps to the given name, or nil if it # does not exist. diff --git a/lib/syskit/port_access.rb b/lib/syskit/port_access.rb index 509dddec2..443094bd3 100644 --- a/lib/syskit/port_access.rb +++ b/lib/syskit/port_access.rb @@ -89,16 +89,6 @@ def has_input_port?(name, including_dynamic = true) !!find_input_port(name) end - def has_through_method_missing?(m) - MetaRuby::DSLs.has_through_method_missing?( - self, m, "_port" => :has_port? - ) || super - end - - def find_through_method_missing(m, args) - MetaRuby::DSLs.find_through_method_missing( - self, m, args, "_port" => :find_port - ) || super - end + MetaRuby::DSLs::FindThroughMethodMissing.standard(self, %w[port]) end end diff --git a/lib/syskit/queries/abstract_component_base.rb b/lib/syskit/queries/abstract_component_base.rb index 88c41785e..f82fa065e 100644 --- a/lib/syskit/queries/abstract_component_base.rb +++ b/lib/syskit/queries/abstract_component_base.rb @@ -80,19 +80,25 @@ def data_service_matcher_by_name(name) include MetaRuby::DSLs::FindThroughMethodMissing + HAS_THROUGH_METHOD_MISSING = { + "_srv" => :data_service_by_name?, + "_port" => :port_by_name? + }.freeze + def has_through_method_missing?(m) MetaRuby::DSLs.has_through_method_missing?( - self, m, - "_srv" => :data_service_by_name?, - "_port" => :port_by_name? + self, m, HAS_THROUGH_METHOD_MISSING ) || super end + FIND_THROUGH_METHOD_MISSING = { + "_srv" => :find_data_service_matcher_by_name, + "_port" => :find_port_matcher_by_name + }.freeze + def find_through_method_missing(m, args) MetaRuby::DSLs.find_through_method_missing( - self, m, args, - "_srv" => :find_data_service_matcher_by_name, - "_port" => :find_port_matcher_by_name + self, m, args, FIND_THROUGH_METHOD_MISSING ) || super end end diff --git a/lib/syskit/robot/master_device_instance.rb b/lib/syskit/robot/master_device_instance.rb index ea7189ea9..361bba6d8 100644 --- a/lib/syskit/robot/master_device_instance.rb +++ b/lib/syskit/robot/master_device_instance.rb @@ -355,15 +355,18 @@ def slave(slave_service, as: nil) robot.devices["#{name}.#{srv.name}"] = device_instance end + HAS_THROUGH_METHOD_MISSING = { "_dev" => :has_slave? }.freeze + FIND_THROUGH_METHOD_MISSING = { "_dev" => :slave }.freeze + def has_through_method_missing?(m) MetaRuby::DSLs.has_through_method_missing?( - self, m, "_dev" => :has_slave? + self, m, HAS_THROUGH_METHOD_MISSING ) || super end def find_through_method_missing(m, args) MetaRuby::DSLs.find_through_method_missing( - self, m, args, "_dev" => :slave + self, m, args, FIND_THROUGH_METHOD_MISSING ) || super end diff --git a/lib/syskit/robot/robot_definition.rb b/lib/syskit/robot/robot_definition.rb index 253eaada1..601175c78 100644 --- a/lib/syskit/robot/robot_definition.rb +++ b/lib/syskit/robot/robot_definition.rb @@ -255,19 +255,9 @@ def to_dependency_injection @di.dup end - def has_through_method_missing?(m) - MetaRuby::DSLs.has_through_method_missing?( - self, m, "_dev" => :has_device? - ) || super - end - - def find_through_method_missing(m, args) - MetaRuby::DSLs.find_through_method_missing( - self, m, args, "_dev" => :find_device - ) || super - end - - include MetaRuby::DSLs::FindThroughMethodMissing + MetaRuby::DSLs::FindThroughMethodMissing.standard( + self, { "_dev" => "device" } + ) end end end diff --git a/lib/syskit/task_context.rb b/lib/syskit/task_context.rb index fbdcd51c8..989a582e4 100644 --- a/lib/syskit/task_context.rb +++ b/lib/syskit/task_context.rb @@ -1564,16 +1564,6 @@ def removed_sink(source) relation_graph_for(Flows::DataFlow).modified_tasks << self end - def has_through_method_missing?(name) - MetaRuby::DSLs.has_through_method_missing?( - self, name, "_property" => :has_property? - ) || super - end - - def find_through_method_missing(name, args) - MetaRuby::DSLs.find_through_method_missing( - self, name, args, "_property" => :find_property - ) || super - end + MetaRuby::DSLs::FindThroughMethodMissing.standard(self, %w[property]) end end diff --git a/lib/syskit/test/profile_test.rb b/lib/syskit/test/profile_test.rb index 4e79ba5fe..f138c27b5 100644 --- a/lib/syskit/test/profile_test.rb +++ b/lib/syskit/test/profile_test.rb @@ -54,23 +54,8 @@ def find_device(name) subject_syskit_model.robot.devices[name] end - def has_through_method_missing?(m) - MetaRuby::DSLs.has_through_method_missing?( - self, m, - "_def" => :find_definition, - "_dev" => :find_device - ) || super - end - - def find_through_method_missing(m, args) - MetaRuby::DSLs.find_through_method_missing( - self, m, args, - "_def" => :find_definition, - "_dev" => :find_device - ) || super - end - - include MetaRuby::DSLs::FindThroughMethodMissing + MetaRuby::DSLs::FindThroughMethodMissing + .standard(self, { "_def" => "definition", "_dev" => "device"}) end def has_through_method_missing?(m) From 92f6db70b1bd9b00205c43a5aba1fa4b63ee6cdf Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Mon, 15 Dec 2025 17:12:48 -0300 Subject: [PATCH 10/15] chore: create new task contexts directly on the target plan Doing new-then-add creates a TemplatePlan first, which is then copied to the real plan. The copy operation is relatively cheap, but unnecessary --- lib/syskit/models/component.rb | 8 ++++---- lib/syskit/models/composition.rb | 2 +- lib/syskit/models/deployment.rb | 5 ++--- lib/syskit/models/deployment_group.rb | 8 ++------ 4 files changed, 9 insertions(+), 14 deletions(-) diff --git a/lib/syskit/models/component.rb b/lib/syskit/models/component.rb index 833395462..a24a6f63a 100644 --- a/lib/syskit/models/component.rb +++ b/lib/syskit/models/component.rb @@ -186,8 +186,7 @@ def instanciate( plan, _context = DependencyInjectionContext.new, task_arguments: {}, ** ) - plan.add(task = new(**task_arguments)) - task + new(plan: plan, **task_arguments) end # The model next in the ancestry chain, or nil if +self+ is root @@ -638,11 +637,12 @@ def with_dynamic_service(dynamic_service_name, **options) # @param dyn_options options passed to the dynamic service block # through {DynamicDataService#instanciate} # @return [BoundDynamicDataService] the newly created service - def require_dynamic_service(dynamic_service_name, as:, **dyn_options) + def require_dynamic_service( + dynamic_service_name, force: false, as:, **dyn_options) service_name = as.to_str dyn = dynamic_service_by_name(dynamic_service_name) - if (srv = find_data_service(service_name)) + if !force && (srv = find_data_service(service_name)) return srv if srv.fullfills?(dyn.service_model) raise ArgumentError, diff --git a/lib/syskit/models/composition.rb b/lib/syskit/models/composition.rb index 1d22b8dca..1a2affd0d 100644 --- a/lib/syskit/models/composition.rb +++ b/lib/syskit/models/composition.rb @@ -1038,7 +1038,7 @@ def instanciate( # rubocop:disable Metrics/ParameterLists end # First of all, add the task for +self+ - plan.add(self_task = new(**task_arguments)) + self_task = new(plan: plan,**task_arguments) conf = if self_task.has_argument?(:conf) self_task.conf(self_task.arguments[:conf]) else diff --git a/lib/syskit/models/deployment.rb b/lib/syskit/models/deployment.rb index 966647ef3..2a6dd356e 100644 --- a/lib/syskit/models/deployment.rb +++ b/lib/syskit/models/deployment.rb @@ -37,9 +37,8 @@ def deployment_name orogen_model.name end - def instanciate(plan, arguments = {}) - plan.add(task = new(arguments)) - task + def instanciate(plan, **arguments) + new(plan: plan, **arguments) end # @api private diff --git a/lib/syskit/models/deployment_group.rb b/lib/syskit/models/deployment_group.rb index 7ce1186fa..82d2690a5 100644 --- a/lib/syskit/models/deployment_group.rb +++ b/lib/syskit/models/deployment_group.rb @@ -34,14 +34,10 @@ class DeploymentGroup def instanciate(plan, permanent: true, deployment_tasks: {}) deployment_task = ( deployment_tasks[[configured_deployment]] ||= - configured_deployment.new + configured_deployment.new(plan: plan) ) - if permanent - plan.add_permanent_task(deployment_task) - else - plan.add(deployment_task) - end + plan.add_permanent_task(deployment_task) if permanent [deployment_task.task(mapped_task_name), deployment_task] end From c056681069a3bc9ba14c67399ac0c49ec8c19b6e Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Mon, 8 Dec 2025 15:36:57 -0300 Subject: [PATCH 11/15] fix: remove unnecessary and confusing `nil` argument to each_data_service --- lib/syskit/models/component.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/syskit/models/component.rb b/lib/syskit/models/component.rb index a24a6f63a..556245a7e 100644 --- a/lib/syskit/models/component.rb +++ b/lib/syskit/models/component.rb @@ -159,7 +159,7 @@ def each_slave_data_service(master_service) return enum_for(:each_slave_data_service, master_service) end - each_data_service(nil) do |_name, service| + each_data_service do |_name, service| next unless (m = service.master) yield(service) if m.full_name == master_service.full_name @@ -173,7 +173,7 @@ def each_slave_data_service(master_service) def each_root_data_service return enum_for(:each_root_data_service) unless block_given? - each_data_service(nil) do |_name, service| + each_data_service do |_name, service| yield(service) if service.master? end end From a918c173dfcc67b18543a82644835e180acd4149 Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Tue, 9 Dec 2025 21:37:17 -0300 Subject: [PATCH 12/15] chore: enable caching for the data_service inherited attribute --- lib/syskit/models/component.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/syskit/models/component.rb b/lib/syskit/models/component.rb index 556245a7e..e6a67453e 100644 --- a/lib/syskit/models/component.rb +++ b/lib/syskit/models/component.rb @@ -42,7 +42,7 @@ def can_use_template? # # @key_name full_name # @return [Hash] - inherited_attribute(:data_service, :data_services, map: true) { {} } + inherited_attribute(:data_service, :data_services, map: true, cache: true) { {} } # List of modules that should be applied on the underlying # {Orocos::RubyTasks::StubTaskContext} when running tests in @@ -55,8 +55,8 @@ def can_use_template? def clear_model super - data_services.clear - dynamic_services.clear + data_services_clear + dynamic_services_clear # NOTE: the placeholder_models cache is cleared separately. The # reason is that we need to clear it on permanent and # non-permanent models alike, including component models that @@ -796,7 +796,7 @@ def provides_resolve_name(as:, slave_of: nil) def provides_validate_possible_overload(model, full_name) # Get the source name and the source model - if data_services[full_name] + if self_data_services[full_name] raise ArgumentError, "there is already a data service named '#{full_name}' " \ "defined on '#{short_name}'" @@ -818,7 +818,7 @@ def provides_validate_possible_overload(model, full_name) def promote_service_if_needed(service) return service if service.component_model == self - data_services[service.full_name] = service.attach(self) + service.attach(self) end # @api private @@ -841,7 +841,7 @@ def provides_compute_port_mappings(service, given_srv_to_self = {}) def register_bound_data_service(full_name, service) include service.model - data_services[full_name] = service + data_service_set(full_name, service) Models.debug do Models.debug "#{short_name} provides #{service}" From e944fd6872a71473e2657f93f70c0266a49513c5 Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Wed, 10 Dec 2025 17:03:14 -0300 Subject: [PATCH 13/15] fix: replace find_data_service by the much more efficient has_data_service? when applicable --- lib/syskit/component.rb | 4 ++-- lib/syskit/models/component.rb | 3 ++- lib/syskit/task_context.rb | 2 +- test/test_component.rb | 4 +++- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/syskit/component.rb b/lib/syskit/component.rb index 249870614..ce04c0519 100644 --- a/lib/syskit/component.rb +++ b/lib/syskit/component.rb @@ -142,7 +142,7 @@ def each_data_service end def has_data_service?(service_name) - model.find_data_service(service_name) + model.has_data_service?(service_name) end # Finds a data service by its name @@ -385,7 +385,7 @@ def merge(merged_task) def duplicate_missing_services_from(task) missing_services = task.model.each_data_service.find_all do |_, srv| - !model.find_data_service(srv.full_name) + !model.has_data_service?(srv.full_name) end missing_services.each do |_, srv| diff --git a/lib/syskit/models/component.rb b/lib/syskit/models/component.rb index e6a67453e..13f35390c 100644 --- a/lib/syskit/models/component.rb +++ b/lib/syskit/models/component.rb @@ -1209,7 +1209,7 @@ def can_merge_service?(self_srv, target_srv) def apply_missing_dynamic_services_from(from, specialize_if_needed = true) missing_services = from.each_data_service.find_all do |_, srv| - !find_data_service(srv.full_name) + !has_data_service?(srv.full_name) end if missing_services.empty? @@ -1233,6 +1233,7 @@ def apply_missing_dynamic_services_from(from, specialize_if_needed = true) end base_model end + base_model end # Returns the component model that is the merge model of self and diff --git a/lib/syskit/task_context.rb b/lib/syskit/task_context.rb index 989a582e4..1f2360755 100644 --- a/lib/syskit/task_context.rb +++ b/lib/syskit/task_context.rb @@ -304,7 +304,7 @@ def can_be_deployed_by?(task) has_service_to_add_through_reconfiguration = each_required_dynamic_service.any? do |srv| srv.model.addition_requires_reconfiguration? && - !task.find_data_service(srv.name) + !task.has_data_service?(srv.name) end return false if has_service_to_add_through_reconfiguration diff --git a/test/test_component.rb b/test/test_component.rb index 3c30ed329..3875a7f95 100644 --- a/test/test_component.rb +++ b/test/test_component.rb @@ -285,7 +285,9 @@ merged_task.specialize merged_task.require_dynamic_service "dyn", as: "srv" task.specialize - flexmock(task.model).should_receive(:find_data_service).with("srv").and_return(true) + flexmock(task.model) + .should_receive(:has_data_service?) + .with("srv").and_return(true) flexmock(task.model).should_receive(:provides_dynamic).never task.merge(merged_task) end From 5e42000361db422617a3f44e86e045a219ebcb3f Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Mon, 15 Dec 2025 17:11:02 -0300 Subject: [PATCH 14/15] fix: replace each/match by find when looking for slave device services --- lib/syskit/models/bound_data_service.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/syskit/models/bound_data_service.rb b/lib/syskit/models/bound_data_service.rb index 1220994c1..d46fc8acb 100644 --- a/lib/syskit/models/bound_data_service.rb +++ b/lib/syskit/models/bound_data_service.rb @@ -385,17 +385,19 @@ def each_required_model extend InstanceRequirements::Auto def has_data_service?(name) - component_model.each_slave_data_service(self) do |slave_m| - return true if slave_m.name == name + unless (m = component_model.find_data_service("#{self.name}.#{name}")) + return false end - false + + m.master == self end def find_data_service(name) - component_model.each_slave_data_service(self) do |slave_m| - return slave_m if slave_m.name == name + unless (m = component_model.find_data_service("#{self.name}.#{name}")) + return end - nil + + m if m.master == self end MetaRuby::DSLs::FindThroughMethodMissing From 8cd31327eb416aea50b4ca07ddbdc62e7b1ace3c Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Mon, 15 Dec 2025 17:11:25 -0300 Subject: [PATCH 15/15] chore: style --- lib/syskit/models/component.rb | 37 ++++++++++++++++------------------ 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/lib/syskit/models/component.rb b/lib/syskit/models/component.rb index 13f35390c..7ad9797a9 100644 --- a/lib/syskit/models/component.rb +++ b/lib/syskit/models/component.rb @@ -1212,26 +1212,23 @@ def apply_missing_dynamic_services_from(from, specialize_if_needed = true) !has_data_service?(srv.full_name) end - if missing_services.empty? - self - else - # We really really need to specialize self. The reason is - # that self.model, even though it has private - # specializations, might be a reusable model from the system - # designer's point of view. With the singleton class, we - # know that it is not - base_model = if specialize_if_needed then specialize - else - self - end - missing_services.each do |_, srv| - dynamic_service_options = - { as: srv.name }.merge(srv.dynamic_service_options) - base_model.require_dynamic_service( - srv.dynamic_service.name, **dynamic_service_options - ) - end - base_model + return self if missing_services.empty? + + # We really really need to specialize self. The reason is + # that self.model, even though it has private + # specializations, might be a reusable model from the system + # designer's point of view. With the singleton class, we + # know that it is not + base_model = if specialize_if_needed then specialize + else + self + end + missing_services.each do |_, srv| + dynamic_service_options = + { as: srv.name }.merge(srv.dynamic_service_options) + base_model.require_dynamic_service( + srv.dynamic_service.name, force: true, **dynamic_service_options + ) end base_model end